fix(data-editor): make edit replay robust across structural changes - #10662
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
All contributors have signed the CLA ✍️ ✅ |
There was a problem hiding this comment.
1 issue found across 2 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="tests/_plugins/ui/_impl/test_data_editor.py">
<violation number="1" location="tests/_plugins/ui/_impl/test_data_editor.py:153">
P2: test_data_editor_replays_add_after_removing_all_rows constructs data_editor without the polars skipif that every other data_editor test uses (e.g. test_data_editor_initialization). In environments without polars, the identical construction is skipped in those tests, implying this one will fail. Add the same @pytest.mark.skipif(not DependencyManager.polars.has(), reason="Polars not installed") decorator.</violation>
</file>
Architecture diagram
sequenceDiagram
participant UI as Client / UI
participant DE as DataEditor Instance
participant TM as TableManager
participant Engine as apply_edits logic
Note over UI,Engine: Initialization
UI->>DE: __init__(data, editable_columns, ...)
DE->>TM: get_column_names()
TM-->>DE: column_names
DE->>DE: NEW: Store self._column_names
Note over UI,Engine: Edit Replay (e.g., Remove then Add Row)
UI->>DE: Trigger edit event (DataEdits)
DE->>Engine: apply_edits(data, edits, column_names)
Engine->>Engine: NEW: Resolve columns from column_names or schema
Engine->>Engine: NEW: Capture original_values prototype from data[0]
loop for each edit in DataEdits
alt Row Removal
Engine->>Engine: Remove row from data
Note right of Engine: data may now be empty []
else NEW: Positional/Column Edit
Engine->>Engine: Check column_set for column_id
opt column_id missing
Engine->>Engine: Append to columns list
Engine->>Engine: Update original_values prototype
end
alt rowIdx >= len(data)
Engine->>Engine: CHANGED: Extend data using preserved column list
end
Engine->>DE: _convert_value(edit_value, prototype_value, dtype)
DE-->>Engine: Casted value
Engine->>Engine: Update data[row_idx][column_id]
end
end
Engine-->>DE: Updated RowOrientedData
DE-->>UI: Return result (schema preserved)
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Coverage Report for ./frontend
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
3 issues found across 12 files (changes from recent commits).
You’re at about 90% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
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="marimo/_plugins/ui/_impl/data_editor.py">
<violation number="1" location="marimo/_plugins/ui/_impl/data_editor.py:170">
P2: When a sparse column's only representative value falls outside the ten sampled rows, replay loses its inferred type after the rows are removed. Derive conversion metadata from every column's non-null values, or preserve representative metadata before sampling, so post-deletion edits keep the original type.</violation>
<violation number="2" location="marimo/_plugins/ui/_impl/data_editor.py:170">
P2: When the only non-null exemplar falls outside the ten sampled rows, editing a null cell in an untyped column returns the raw string instead of preserving the column's value type. Infer conversion values across all rows, or otherwise guarantee that each column's representative is found.</violation>
</file>
<file name="frontend/src/plugins/impl/DataEditorPlugin.tsx">
<violation number="1" location="frontend/src/plugins/impl/DataEditorPlugin.tsx:50">
P2: Adding `columnNames` here makes the committed plugin OpenAPI schema stale. Regenerate and commit `frontend/plugins.openapi.yaml` so the plugin-schema synchronization test and published contract include this field.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
Tested manually and is working |
There was a problem hiding this comment.
Pull request overview
This PR fixes data-editor edit replay failures when the table becomes empty and improves determinism for structural edits by making table structure (column names/order, row count, dtype hints) independent of currently materialized cells on both backend and frontend.
Changes:
- Backend: refactors edit application through a single
_EditableTablemodel, adds scalar-list support, and preserves schema/column metadata across empty/sparse inputs. - Frontend: routes replay + live edits through a single pure
applyEditorEditstransition and threads explicitcolumnNamesthrough plugin state to stabilize column ordering. - Tests/OpenAPI: adds comprehensive regression coverage (empty replay, structural sequencing, precision handling) and exposes
columnNamesin the plugin schema.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/_plugins/ui/_impl/test_data_editor.py | Adds regression tests for empty-table appends, structural replay, conversion rules, and integer precision. |
| marimo/_plugins/ui/_impl/data_editor.py | Introduces _EditableTable-based edit application, scalar-list handling, and improved conversion semantics. |
| frontend/src/plugins/impl/DataEditorPlugin.tsx | Switches to editor-state-driven replay and passes explicit columnNames into the editor. |
| frontend/src/plugins/impl/data-editor/types.ts | Refines edit types and introduces EditorState/EditorRow types. |
| frontend/src/plugins/impl/data-editor/glide-utils.ts | Adds isValidCellValue and removes local mutation from paste (edits-only pathway). |
| frontend/src/plugins/impl/data-editor/glide-data-editor.tsx | Removes duplicated local replay/mutation and emits normalized edits for all operations. |
| frontend/src/plugins/impl/data-editor/editor-state.ts | New pure reducer-style state transition for applying edit logs. |
| frontend/src/plugins/impl/data-editor/data-utils.ts | Adds orderColumnFields, updates column operations to be name-/index-aware and preserve order. |
| frontend/src/plugins/impl/data-editor/tests/glide-utils.test.ts | Adds validation tests and updates paste behavior expectations. |
| frontend/src/plugins/impl/data-editor/tests/glide-data-editor.test.tsx | Updates editor tests to assert emitted edits instead of local mutation. |
| frontend/src/plugins/impl/data-editor/tests/editor-state.test.ts | New tests for deterministic replay and immutability of applyEditorEdits. |
| frontend/src/plugins/impl/data-editor/tests/data-utils.test.ts | Extends tests for ordering and updated column operations semantics. |
| frontend/src/plugins/impl/tests/DataEditorPlugin.test.ts | Adds async-load race tests to ensure latest edits win and stale loads are ignored. |
| frontend/plugins.openapi.yaml | Adds columnNames to the plugin schema. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This pull request was authored by a coding agent.
📝 Summary
Closes #10650
Deleting every row in a data editor could leave the frontend with no usable column metadata, so appending a row failed during replay. The same split replay paths also made structural edits vulnerable to stale column order, inconsistent conversion, and frontend/backend drift.
This change makes table structure explicit and independent of the cells that currently exist. The backend applies edits through one ordered table model that retains column names, row count, conversion examples, and dtype metadata across empty, sparse, scalar, row-oriented, and column-oriented inputs.
The frontend likewise routes both persisted replay and live cell, paste, row, and column operations through one pure editor-state transition. This removes the duplicate local mutation paths and ensures that the state shown to the user is produced by the same edit sequence sent to Python. Async CSV loads are committed through the loader's race guard and reconciled with the latest edit log, so stale requests cannot replace newer editor state.
Together, these changes fix the empty-table failure while making subsequent structural replay deterministic, type-safe, and easier to extend.
Fixed

📋 Pre-Review Checklist
✅ Merge Checklist