fix(studio): display diff for view-only notebook cell edits - #49901
Conversation
In the AI Assistant notebook-update proposal preview, a replace_cell operation that only changed a cell's view (table ↔ chart) or chart parameters (type, x/y columns, cumulative, scale, labels) showed a "Replaced" row but the expanded diff appeared empty. This was because the diff editor only compared the cell's SQL text, never considering view or chart configuration. Refactored metadata tracking to distinguish between a cell's source (database or time range) and view (table or chart config) independently: - getCellMetadata now returns structured NotebookCellFields with separate source and view fields instead of a single flattened text string - Added formatChartConfig and formatCellView helpers to describe chart cells - getEntryMetadata now diffs source and view independently; fields that are unchanged before and after are dropped entirely, so only changed fields appear in the diff header (e.g., "Table → Chart (bar, ...)" when only the view changed, with the unchanged database omitted) - If neither field changed, metadata is hidden entirely (instead of showing an unchanged line) - For non-replaced entries, formatCellFieldsText builds the description as before, omitting the view when it's just the default table view Added test cases covering: chart-view cells, view-only changes, chart- parameter-only changes, database-only changes, and fully-unchanged replacements. All 43 tests pass; tsc --noEmit shows no new type errors.
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe notebook preview now represents cell metadata with separate source and view fields. It formats table and chart views, includes log time ranges, and shows only changed metadata for replaced cells. ChangesNotebook metadata formatting
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change improves how notebook source and table or chart-view differences appear in the Studio AI Assistant preview without affecting notebook execution, authorization, stored data, or deployment behavior. No actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description explains the bug, root cause, solution, changed behavior, and test plan. It does not use every template heading and does not state the CONTRIBUTING.md confirmation, but it provides the required technical context and is mostly complete.
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
6 Skipped Deployments
|
tsc flagged excess properties when spreading the CellWire/AgentCell union return of the wireDatabaseCell/agentDatabaseCell test helpers, since TypeScript can't guarantee which union member is being spread. Build the chart-cell test fixtures as direct object literals instead.
There was a problem hiding this comment.
Preview test — ✅ all pass
Tested on the studio-staging preview by asking the Assistant for notebook edits on a real notebook. The "Test plan" items are all local (vitest / tsc) so I couldn't run those here; the checks below are the user-facing behaviors inferred from the summary.
| # | Test | Result |
|---|---|---|
| 1 | View-only change (table → bar chart) | ✅ "Replaced" row shows Table → Chart (bar, x: day, y: signups). Expanded diff shows identical SQL, as expected. |
| 2 | Chart-parameter-only change (bar → line) | ✅ Metadata reads Chart (bar, x: day, y: signups) → Chart (line, x: day, y: signups), no Database: shown. |
| 3 | SQL-only change (added, regression) | ✅ No header metadata at all; expanded row shows the SQL diff (1,5 → 1,10). |
| 4 | Added cells (added, regression) | ✅ Table cell shows Database: Primary; chart cell shows Database: Primary · Chart (bar, x: x, y: y). |
No new console errors from any of the proposal renders.
Notes:
- Minor, not blocking: the metadata span is capped at 45% width with
truncateand has no tooltip, so at the default panel width the chart → chart case (#2) reads asChart (bar, x: day, y: signups...and the "after" side is cut off. Might be worth atitleattribute or letting it wrap, but that's pre-existing layout rather than something this PR introduced.
🎭 Playwright Test Results (next)
Details
Flaky testsFeatures › database.spec.ts › Database Functions › CRUD operations works as expected Skipped testsFeatures › auth-users.spec.ts › should show web3 users as enabled when the matching web3 provider is enabled |
Braintrust eval report
|
2 participants
Summary
Fixed a bug in the AI Assistant notebook-update proposal preview where a
replace_celloperation that only changed a cell's view (table ↔ chart) or chart parameters (type, x/y columns, cumulative, scale, labels) would show as a "Replaced" row but the expanded diff would appear empty.Root cause: The diff editor only compared the cell's SQL text; view and chart configuration were never considered, so changes to those aspects showed no diff.
Solution:
getCellMetadatato return structuredNotebookCellFieldswith separatesource(database/time range) andview(table/chart) fields instead of a single concatenated stringformatChartConfigandformatCellViewhelpers to describe chart cellsgetEntryMetadatato diff source and view independently, showing only the fields that actually changed (e.g., "Table → Chart (bar, ...)" when only the view changed, with the unchanged database omitted)Test plan
viewfield, view-only changes surfacing without the unchanged database, chart-parameter-only changes surfacing without the unchanged database, database-only changes surfacing without the unchanged view, and fully-unchanged replacements hiding metadata entirelytsc --noEmiton apps/studio shows no new type errorsSummary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes