Make proposed AI edits UI consistent with editor styling - #10595
Open
akshayka wants to merge 4 commits into
Open
Make proposed AI edits UI consistent with editor styling#10595akshayka wants to merge 4 commits into
akshayka wants to merge 4 commits into
Conversation
AI-generated notebook cells should look like part of the editor and make the available actions clear. Replace the distracting proposal glows with the existing cell styling, keep the review controls inside each cell, and use explicit labels for proposed deletions. Do not offer to run a cell after the user accepts its deletion.
AI-generated cells and inline refactoring need the same review language and button treatment. Keep added-cell status concise, use explicit keep and revert actions for changes, and make normal edits, runs, and deletes clear the AI review state so the proposal UI does not persist after the user takes ownership of a cell.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
All contributors have signed the CLA ✍️ ✅ |
akshayka
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the frontend “staged AI cell” review experience so AI-proposed edits visually match standard editor cells, and so proposal review actions/labels better reflect “keep vs revert” semantics across add/change/delete proposals (including the inline refactor banner flow).
Changes:
- Replaces AI proposal “glow/opacity” styling with standard cell styling plus border-color cues, and introduces a consistent AI cell footer style.
- Updates per-cell and global proposal review labels/actions (e.g., “Keep change” / “Revert change”, “Keep all” / “Discard all”), and adjusts run-all behavior to skip deletion proposals.
- Clears staged/proposal state in more user-ownership flows (running, editing, deleting cells) so review markers don’t linger.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/css/app/Cell.css | Switches AI proposal visuals to border-based styling and adds .mo-ai-cell-footer styling. |
| frontend/src/components/editor/notebook-cell.tsx | Moves staged AI footer rendering into the main cell container so it participates in cell layout/styling. |
| frontend/src/components/editor/chrome/wrapper/pending-ai-cells.tsx | Updates global tray labels and prevents running staged deletion proposals. |
| frontend/src/components/editor/cell/useRunCells.ts | Clears staged AI state on run to reflect user ownership after execution. |
| frontend/src/components/editor/cell/useDeleteCell.tsx | Clears staged AI state when deleting one or many cells. |
| frontend/src/components/editor/cell/StagedAICell.tsx | Revises staged AI footer messaging and conditionally shows review actions based on proposal type. |
| frontend/src/components/editor/cell/code/cell-editor.tsx | Clears staged AI state on direct user edits in CodeMirror. |
| frontend/src/components/editor/ai/completion-handlers.tsx | Adds label customization support to completion action buttons. |
| frontend/src/components/editor/ai/ai-completion-editor.tsx | Aligns inline refactor banner styling/copy with the new “keep vs revert” terminology. |
Suppressed comments (1)
frontend/src/components/editor/cell/useDeleteCell.tsx:77
- Same as the single-cell delete: this
get+setpattern can drop concurrent staged AI updates. Use a functionalstore.setupdater to remove all deleted cellIds from the lateststagedAICellsAtomvalue atomically.
const stagedAICells = store.get(stagedAICellsAtom);
if (stagedAICells.size > 0) {
const nextStagedAICells = new Map(stagedAICells);
for (const cellId of cellIds) {
nextStagedAICells.delete(cellId);
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+53
to
+57
| }> = ({ | ||
| isLoading, | ||
| onAccept, | ||
| onDecline, | ||
| runCell, |
Comment on lines
+27
to
+32
| const stagedAICells = store.get(stagedAICellsAtom); | ||
| if (stagedAICells.has(cellId)) { | ||
| const nextStagedAICells = new Map(stagedAICells); | ||
| nextStagedAICells.delete(cellId); | ||
| store.set(stagedAICellsAtom, nextStagedAICells); | ||
| } |
|
|
||
| <p className="transition-opacity duration-200 text-muted-foreground"> | ||
| {isLoading ? "Generating fix..." : "Showing fix"} | ||
| {isLoading ? "Generating change..." : "AI changed this cell"} |
Comment on lines
+74
to
+82
| <CompletionActionsCellFooter | ||
| isLoading={false} | ||
| onAccept={() => handleCompletion("accept")} | ||
| onDecline={() => handleCompletion("reject")} | ||
| size="xs" | ||
| runCell={isDeletion ? undefined : runCell} | ||
| acceptLabel={isDeletion ? "Delete cell" : "Keep change"} | ||
| declineLabel={isDeletion ? "Keep cell" : "Revert change"} | ||
| /> |
Contributor
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
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/components/editor/ai/ai-completion-editor.tsx">
<violation number="1" location="frontend/src/components/editor/ai/ai-completion-editor.tsx:444">
P3: The visible text now says "Generating change..."/"AI changed this cell", but the Loading/CircleCheck icon `aria-label`s right above still say "Generating fix" and "Fix generated". Since this PR is about replacing the "fix" wording with "change", update both aria-labels to match so screen readers stay consistent.</violation>
</file>
<file name="frontend/src/components/editor/cell/StagedAICell.tsx">
<violation number="1" location="frontend/src/components/editor/cell/StagedAICell.tsx:80">
P3: When `isDeletion` is true, the destructive `Delete cell` action uses the green accept styling while the non-destructive `Keep cell` action uses the red reject styling. Give deletion actions explicit destructive and non-destructive variants so their colors match their consequences.</violation>
</file>
Architecture diagram
sequenceDiagram
participant UI as Notebook UI
participant Cell as Cell Component
participant CellEditor as Cell Editor (CodeMirror)
participant CellFooter as StagedAICellFooter
participant ActionBar as CompletionActions (Accept/Reject)
participant Store as Jotai Store (stagedAICellsAtom)
participant DeleteHook as useDeleteCell
participant RunHook as useRunCells
participant AIBanner as Inline AI Banner
participant CSS as CSS Styles
Note over UI,CSS: AI Proposal Cell Styling Flow
UI->>CSS: Apply mo-ai-generated-cell class
CSS->>UI: CHANGED: No glow/opacity, blue border instead
CSS->>UI: Apply mo-ai-deleted-cell class
CSS->>UI: CHANGED: No opacity/glow, red border + strikethrough
Note over CellFooter,ActionBar: Cell Footer (new layout)
Cell->>CellFooter: Render for staged AI cell
CellFooter->>CellFooter: Determine type (add/change/delete)
alt isAddition
CellFooter->>CellFooter: Show "AI-added cell" label, no action buttons
else isChange or isDeletion
CellFooter->>CellFooter: Show "AI changed this cell" or "AI suggests deleting this cell"
CellFooter->>ActionBar: Pass acceptLabel/declineLabel
alt isDeletion
ActionBar->>ActionBar: Labels: "Delete cell" / "Keep cell"
else isChange
ActionBar->>ActionBar: Labels: "Keep change" / "Revert change"
end
end
Note over CellEditor,Store: Editing clears proposal state
CellEditor->>CellEditor: Detect user input/delete/undo/redo event
CellEditor->>Store: CHANGED: Remove cellId from stagedAICellsAtom
Store-->>CellEditor: Updated state
Note over DeleteHook,Store: Deleting a proposed cell clears state
DeleteHook->>Store: Check if cellId is in stagedAICells
Store-->>DeleteHook: Cell found
DeleteHook->>Store: CHANGED: Remove cellId from stagedAICellsAtom
DeleteHook->>Store: CHANGED: For multi-delete, remove all cellIds
Note over RunHook,Store: Running a proposed cell clears state
RunHook->>Store: CHANGED: Remove each cellId from stagedAICellsAtom
RunHook->>RunHook: Run cells logic continues
Note over RunHook,PendingAICells: Global proposal tray
PendingAICells->>ActionBar: Pass "Keep all" / "Discard all" labels
PendingAICells->>RunHook: Run all cells (exclude deletion proposals)
RunHook->>Store: CHANGED: Remove user-accepted cells from staged state
Note over ActionBar,Store: Inline AI refactoring
ActionBar->>AIBanner: CHANGED: Border style + updated labels ("Keep change"/"Revert change")
AIBanner->>AIBanner: Updated loading text ("Generating change...")
AIBanner->>ActionBar: Accept/reject completion
Note over Store,CellFooter: State-driven re-render
Store-->>CellFooter: stagedAICellsAtom update
CellFooter->>CellFooter: Re-render based on current state
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| <p className="transition-opacity duration-200 text-muted-foreground"> | ||
| {isLoading ? "Generating fix..." : "Showing fix"} | ||
| {isLoading ? "Generating change..." : "AI changed this cell"} |
Contributor
There was a problem hiding this comment.
P3: The visible text now says "Generating change..."/"AI changed this cell", but the Loading/CircleCheck icon aria-labels right above still say "Generating fix" and "Fix generated". Since this PR is about replacing the "fix" wording with "change", update both aria-labels to match so screen readers stay consistent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/editor/ai/ai-completion-editor.tsx, line 444:
<comment>The visible text now says "Generating change..."/"AI changed this cell", but the Loading/CircleCheck icon `aria-label`s right above still say "Generating fix" and "Fix generated". Since this PR is about replacing the "fix" wording with "change", update both aria-labels to match so screen readers stay consistent.</comment>
<file context>
@@ -445,7 +441,7 @@ const CompletionBanner: React.FC<CompletionBannerProps> = ({
<p className="transition-opacity duration-200 text-muted-foreground">
- {isLoading ? "Generating fix..." : "Showing fix"}
+ {isLoading ? "Generating change..." : "AI changed this cell"}
</p>
</div>
</file context>
| onDecline={() => handleCompletion("reject")} | ||
| size="xs" | ||
| runCell={isDeletion ? undefined : runCell} | ||
| acceptLabel={isDeletion ? "Delete cell" : "Keep change"} |
Contributor
There was a problem hiding this comment.
P3: When isDeletion is true, the destructive Delete cell action uses the green accept styling while the non-destructive Keep cell action uses the red reject styling. Give deletion actions explicit destructive and non-destructive variants so their colors match their consequences.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/editor/cell/StagedAICell.tsx, line 80:
<comment>When `isDeletion` is true, the destructive `Delete cell` action uses the green accept styling while the non-destructive `Keep cell` action uses the red reject styling. Give deletion actions explicit destructive and non-destructive variants so their colors match their consequences.</comment>
<file context>
@@ -47,21 +48,40 @@ export const StagedAICellFooter: React.FC<{ cellId: CellId }> = ({
+ onDecline={() => handleCompletion("reject")}
+ size="xs"
+ runCell={isDeletion ? undefined : runCell}
+ acceptLabel={isDeletion ? "Delete cell" : "Keep change"}
+ declineLabel={isDeletion ? "Keep cell" : "Revert change"}
+ />
</file context>
Member
|
Seems that this removes the individual cell actions to Accept / Remove. Is that intentional? I think the glow does look nice, but maybe can be tweaked? I also think the label "AI added cell" feels unecessary, the icon alone feels nice, as a tooltip. |
Contributor
Author
Thanks for the feedback. @Light2Dark, yes, this removes the Accept and Remove individual cell actions. They were redundant. The cells are already added to your notebook, and you can remove by simply deleting them. The redundancy creates confusion and noise for the user. A glow might look good in isolation, but it doesn't fit the visual design language of our editor, and to me it reads as noise. I've updated the label to be just a tooltip |
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
**This pull request was authored by a coding agent.** ## Summary This pull request refines [#10595](#10595) by keeping AI proposal decisions explicit at each cell. Generated cells now expose the same per-cell Keep/Discard controls as updated and deleted cells, so users can review proposals individually or use the existing batch controls. The proposal UI also subscribes only to the staged state for its own cell, preventing unrelated cells from re-rendering when a proposal changes. ## Why AI editing is opt-in and should not add work to normal notebook editing. Explicit controls make proposal ownership predictable, while per-cell state selection keeps the review UI responsive in large notebooks. ## Screenshots ### Refined proposal UI <img width="972" height="770" alt="image" src="https://github.com/user-attachments/assets/9081a97d-d960-4d19-884a-4806874e58d8" /> <img width="957" height="160" alt="AI proposal review controls" src="https://github.com/user-attachments/assets/311e63f9-44d5-4686-96a4-0aaba7096fad" /> ### Before <img width="911" height="219" alt="AI proposal footer before this refinement" src="https://github.com/user-attachments/assets/1faa4a46-8b4a-46f5-affb-ab9c60a016f5" /> ### After <img width="968" height="299" alt="AI proposal footer after this refinement" src="https://github.com/user-attachments/assets/1c788465-2237-477e-9dfd-af68d83d9396" /> > Written by GPT-5.6 Sol on Cursor
Light2Dark
approved these changes
Aug 26, 2026
Bundle ReportChanges will increase total bundle size by 12.0kB (0.05%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: marimo-esmAssets Changed:
Files in
Files in
Files in
|
Member
|
@akshayka open for your review & merge |
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 pull request makes AI-generated notebook cells look consistent with the rest of the editor and clarifies how users review proposed changes.
It also updates the inline AI refactoring flow that opens with
Cmd+Shift+Eon macOS or the equivalent shortcut on other platforms.What changed
AI-added cellwithout adding separate Accept and Reject buttons to every new cell.Keep changeandRevert changeactions.Delete cellandKeep cellactions.Keep allandDiscard alllabels.Keep changeandRevert changelabels.Why
AI proposals are already inserted into the notebook document before the user reviews them. The review state only indicates that the proposal still needs attention. The updated UI uses the existing cell styling and reserves explicit review actions for changes that need a keep or revert decision.
Running or editing a proposed cell indicates that the user has reviewed the current code. Clearing the proposal state in those cases prevents the review marker from remaining after the user takes ownership of the cell.
With changes (this PR)
Without changes (before this PR)