feat(table): allow hosts to size-gate downloads#9510
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Member
Author
Contributor
|
@kirangadhave I have started the AI code review. It will take a few minutes to complete. |
Contributor
There was a problem hiding this comment.
No issues found across 8 files
Architecture diagram
sequenceDiagram
participant FE as Frontend (ExportMenu)
participant Atom as downloadSizeLimitAtom
participant BE as Backend (download_as)
participant DF as mo.dataframe/table (manager)
participant DB as Data
Note over FE,DB: Export / Clipboard flow with optional size-gating
FE->>BE: downloadAs({ format })
BE->>DF: to_csv/to_json/to_parquet()
DF->>DB: read data
DF-->>BE: payload bytes
BE->>BE: len(payload) – NEW: compute size_bytes
BE->>DF: mo_data.csv/json/parquet(url)
DF-->>BE: vfile with url
BE-->>FE: { url, filename, size_bytes }
alt Host has set downloadSizeLimitAtom
FE->>Atom: read policy (limitBytes, unavailableMessage)
alt size_bytes != null AND size_bytes > limitBytes
FE->>FE: toast("Data too large to download/copy")
Note over FE: Skip download/clipboard action
else size_bytes absent (older backend) or within limit
FE->>FE: proceed with download or clipboard copy
end
else Atom is null (default)
Note over FE: No gate – always proceed
FE->>FE: download or clipboard as normal
end
mscolnick
reviewed
May 11, 2026
4 tasks
d061dad to
3f25180
Compare
Expose a per-table JSON-serialized size in the render payload and a new `downloadSizeLimitAtom` so hosts (e.g. marimo-lsp in VS Code) can disable the Export button up-front when the data would not fit through their transport. JSON size is cached on the UI element via `memoize_last_value` keyed on manager identity, so it is recomputed only when filters/search produce a new manager. When the host has not seeded the atom (browser, islands), the gate is dormant and the button stays enabled regardless of size.
3f25180 to
7e99fce
Compare
Member
Author
Contributor
|
@kirangadhave I have started the AI code review. It will take a few minutes to complete. |
for more information, see https://pre-commit.ci
Contributor
There was a problem hiding this comment.
2 issues found across 9 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/plugins/impl/DataTablePlugin.tsx">
<violation number="1" location="frontend/src/plugins/impl/DataTablePlugin.tsx:556">
P2: `props.sizeBytes` is used inside `useAsyncData` but not tracked as a dependency, which can leave `data.sizeBytes` stale and cause incorrect size-gating decisions.</violation>
</file>
<file name="marimo/_plugins/ui/_impl/table.py">
<violation number="1" location="marimo/_plugins/ui/_impl/table.py:800">
P2: Avoid computing `size-bytes` during lazy initialization; this eagerly serializes the full table and negates the lazy-loading path.</violation>
</file>
Architecture diagram
sequenceDiagram
participant UI as Browser/VS Code Renderer
participant ExportMenu as ExportMenu Component
participant Atom as downloadSizeLimitAtom
participant Plugin as DataTablePlugin
participant Backend as Marimo Python Backend
participant Table as mo.ui.table
participant DataFrame as mo.ui.dataframe
participant Manager as TableManager
Note over UI,Manager: Download Size Gate Flow
UI->>Plugin: Render table with sizeBytes
Plugin->>ExportMenu: Pass sizeBytes prop
ExportMenu->>Atom: Read downloadSizeLimitAtom
alt Host sets limit (e.g., VS Code)
Atom-->>ExportMenu: { limitBytes, unavailableMessage }
alt sizeBytes > limitBytes
ExportMenu-->>UI: Disable Export button
ExportMenu-->>UI: Show tooltip with unavailableMessage
ExportMenu-->>ExportMenu: Handle export → skip download/copy
else sizeBytes ≤ limitBytes
ExportMenu-->>ExportMenu: Normal export flow continues
end
else Host doesn't set limit (default null)
Atom-->>ExportMenu: null
ExportMenu-->>ExportMenu: Gate disabled, normal flow continues
end
Note over UI,Backend: Download Request (when gate passes)
UI->>ExportMenu: user clicks export format
ExportMenu->>Plugin: downloadAs({ format })
Plugin->>Backend: RPC call to _download_as()
Backend->>Table: _download_as(args)
Table->>Manager: serialize data (csv/json/parquet)
Manager-->>Table: payload bytes
Table->>Table: Compute size_bytes = len(payload)
Table-->>Backend: DownloadAsResponse(url, filename, size_bytes)
Backend-->>Plugin: Response with size_bytes
Plugin-->>ExportMenu: Return url + filename
ExportMenu-->>UI: Trigger download / clipboard copy
Note over UI,Backend: Search/Filter Recompute
Plugin->>Backend: search({ query, ... })
Backend->>Table: _search(args)
Table->>Manager: Filter data
Manager-->>Table: Filtered result
Table->>Table: _get_json_size_bytes(result) (memoized)
Table-->>Backend: SearchTableResponse(data, total_rows, size_bytes)
Backend-->>Plugin: Response with size_bytes
Plugin-->>UI: Update sizeBytes prop on DataTable
Note over Table,DataFrame: DataFrame Plugin also participates
Backend->>DataFrame: init with size_bytes
DataFrame->>Plugin: Pass size_bytes prop
Plugin->>ExportMenu: Forward sizeBytes to ExportMenu
Note over Backend,Manager: Conservative size estimate
Note over Backend,Manager: JSON is largest format → if JSON fits, CSV/Parquet also fit
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
The size-bytes JSON computation runs on every table init; in lazy mode no search has happened yet so this serialized the full manager unnecessarily. Also adds props.sizeBytes to useAsyncData deps so the gate updates when the prop changes.
The dropdown gated open on a disabled flag while keeping aria-disabled only. Switch to a native disabled attribute on the Button and DropdownMenuTrigger, wrapped in a span so the Tooltip keeps a live hover target.
bec017e to
e355c4c
Compare
manzt
reviewed
May 12, 2026
| export const ExportMenu: React.FC<ExportActionProps> = (props) => { | ||
| const { locale } = useLocale(); | ||
| const [open, setOpen] = React.useState(false); | ||
| const policy = useAtomValue(downloadSizeLimitAtom); |
manzt
approved these changes
May 12, 2026
Collaborator
manzt
left a comment
There was a problem hiding this comment.
looks great. nice solution
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is step 1 toward fixing marimo-team/marimo-lsp#542; the companion change will live in marimo-lsp, which will set the atom to a concrete VS Code–appropriate limit at renderer init.
Summary
Adds
size_bytestoDownloadAsResponseand a newdownloadSizeLimitAtom(jotai) that gatesmo.ui.tableexports and clipboard copies when the serialized payload exceeds a host-supplied limit. The default policy isnull(gate dormant); hosts opt in by setting the atom at init time.This is a bandaid until streaming exports lands — the goal is to give VS Code's NotebookRenderer iframe (and any other sandboxed host) a way to refuse downloads it can't fulfill, rather than failing silently.
download_asnow returns(url, filename, size_bytes); the size is computed once from the already-serialized bytes per format (csv/json/parquet). Threaded through bothui.table._download_asandui.dataframe._download_as.ExportMenureadsdownloadSizeLimitAtom; when set and `size_bytes > limitBytes`, a danger toast fires and the download/copy is skipped. Fail-open when `size_bytes` is absent (older backend) or the atom is unset.DownloadAsSchemazod output mirrors the new optional field.This is blocking marimo-team/marimo-lsp#554
Test plan