feat(chart): UI to edit display_chart params and persist to db#700
Open
feat(chart): UI to edit display_chart params and persist to db#700
Conversation
Contributor
🚀 Preview Deployment
Preview will be automatically removed when this PR is closed. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
5 issues found across 14 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="apps/frontend/src/components/tool-calls/display-chart-edit-dialog.tsx">
<violation number="1" location="apps/frontend/src/components/tool-calls/display-chart-edit-dialog.tsx:102">
P1: `addSeries()` can add a series with `data_key: ''`, which leads to a Radix Select runtime error when rendering `<SelectItem value="">`.</violation>
</file>
<file name="apps/backend/src/queries/chart-image.ts">
<violation number="1" location="apps/backend/src/queries/chart-image.ts:84">
P2: Wrap the config update and chart-image cache invalidation in a single DB transaction to avoid partial writes when one statement fails.</violation>
</file>
<file name="apps/frontend/src/routes/_sidebar-layout.stories.preview.$chatId.$storySlug.tsx">
<violation number="1" location="apps/frontend/src/routes/_sidebar-layout.stories.preview.$chatId.$storySlug.tsx:133">
P2: Chart edit context is enabled even for archived stories, so archived story previews can still show an edit action.</violation>
</file>
<file name="apps/frontend/src/contexts/story-chart-edit.tsx">
<violation number="1" location="apps/frontend/src/contexts/story-chart-edit.tsx:77">
P2: Replacing chart markup by raw string value can update the wrong chart when identical tags exist; the update should target a unique occurrence (or fail when ambiguous).</violation>
</file>
<file name="apps/backend/src/trpc/chart.routes.ts">
<violation number="1" location="apps/backend/src/trpc/chart.routes.ts:49">
P1: Validate that `toolCallId` belongs to a `display_chart` tool call before writing, otherwise this mutation can overwrite non-chart tool inputs for owned chats.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const addSeries = () => { | ||
| const used = new Set(draft.series.map((s) => s.data_key)); | ||
| const fallback = | ||
| availableColumns.find((c) => c !== draft.x_axis_key && !used.has(c)) ?? availableColumns[0] ?? ''; |
Contributor
There was a problem hiding this comment.
P1: addSeries() can add a series with data_key: '', which leads to a Radix Select runtime error when rendering <SelectItem value="">.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/frontend/src/components/tool-calls/display-chart-edit-dialog.tsx, line 102:
<comment>`addSeries()` can add a series with `data_key: ''`, which leads to a Radix Select runtime error when rendering `<SelectItem value="">`.</comment>
<file context>
@@ -0,0 +1,351 @@
+ const addSeries = () => {
+ const used = new Set(draft.series.map((s) => s.data_key));
+ const fallback =
+ availableColumns.find((c) => c !== draft.x_axis_key && !used.has(c)) ?? availableColumns[0] ?? '';
+ setDraft((prev) => ({
+ ...prev,
</file context>
| }); | ||
| } | ||
|
|
||
| await updateChartConfig(input.toolCallId, input.config); |
Contributor
There was a problem hiding this comment.
P1: Validate that toolCallId belongs to a display_chart tool call before writing, otherwise this mutation can overwrite non-chart tool inputs for owned chats.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/backend/src/trpc/chart.routes.ts, line 49:
<comment>Validate that `toolCallId` belongs to a `display_chart` tool call before writing, otherwise this mutation can overwrite non-chart tool inputs for owned chats.</comment>
<file context>
@@ -17,4 +24,29 @@ export const chartRoutes = {
+ });
+ }
+
+ await updateChartConfig(input.toolCallId, input.config);
+ return { success: true as const };
+ }),
</file context>
Comment on lines
+84
to
+87
| await db.update(s.messagePart).set({ toolInput: config }).where(eq(s.messagePart.toolCallId, toolCallId)).execute(); | ||
|
|
||
| // Invalidate any cached PNG so externally-served chart images refresh. | ||
| await db.delete(s.message_part_chart_image).where(eq(s.message_part_chart_image.toolCallId, toolCallId)).execute(); |
Contributor
There was a problem hiding this comment.
P2: Wrap the config update and chart-image cache invalidation in a single DB transaction to avoid partial writes when one statement fails.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/backend/src/queries/chart-image.ts, line 84:
<comment>Wrap the config update and chart-image cache invalidation in a single DB transaction to avoid partial writes when one statement fails.</comment>
<file context>
@@ -66,3 +66,23 @@ export const saveChart = async (toolCallId: string, data: string): Promise<strin
+
+/** Persists an updated `display_chart` config for the given tool call. */
+export const updateChartConfig = async (toolCallId: string, config: displayChart.Input): Promise<void> => {
+ await db.update(s.messagePart).set({ toolInput: config }).where(eq(s.messagePart.toolCallId, toolCallId)).execute();
+
+ // Invalidate any cached PNG so externally-served chart images refresh.
</file context>
Suggested change
| await db.update(s.messagePart).set({ toolInput: config }).where(eq(s.messagePart.toolCallId, toolCallId)).execute(); | |
| // Invalidate any cached PNG so externally-served chart images refresh. | |
| await db.delete(s.message_part_chart_image).where(eq(s.message_part_chart_image.toolCallId, toolCallId)).execute(); | |
| await db.transaction(async (tx) => { | |
| await tx.update(s.messagePart).set({ toolInput: config }).where(eq(s.messagePart.toolCallId, toolCallId)).execute(); | |
| // Invalidate any cached PNG so externally-served chart images refresh. | |
| await tx.delete(s.message_part_chart_image).where(eq(s.message_part_chart_image.toolCallId, toolCallId)).execute(); | |
| }); |
| <PreviewContent | ||
| code={story.code} | ||
| queryData={story.queryData as QueryDataMap | null} | ||
| <StoryChartEditProvider |
Contributor
There was a problem hiding this comment.
P2: Chart edit context is enabled even for archived stories, so archived story previews can still show an edit action.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/frontend/src/routes/_sidebar-layout.stories.preview.$chatId.$storySlug.tsx, line 133:
<comment>Chart edit context is enabled even for archived stories, so archived story previews can still show an edit action.</comment>
<file context>
@@ -129,12 +130,19 @@ function StoryPreviewPage() {
- <PreviewContent
- code={story.code}
- queryData={story.queryData as QueryDataMap | null}
+ <StoryChartEditProvider
chatId={chatId}
- cacheSchedule={story.cacheSchedule}
</file context>
| throw new Error('Could not locate the chart in the current story version.'); | ||
| } | ||
| const nextTag = buildChartTag(config); | ||
| const nextCode = storyCode.replace(rawTag, nextTag); |
Contributor
There was a problem hiding this comment.
P2: Replacing chart markup by raw string value can update the wrong chart when identical tags exist; the update should target a unique occurrence (or fail when ambiguous).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/frontend/src/contexts/story-chart-edit.tsx, line 77:
<comment>Replacing chart markup by raw string value can update the wrong chart when identical tags exist; the update should target a unique occurrence (or fail when ambiguous).</comment>
<file context>
@@ -0,0 +1,98 @@
+ throw new Error('Could not locate the chart in the current story version.');
+ }
+ const nextTag = buildChartTag(config);
+ const nextCode = storyCode.replace(rawTag, nextTag);
+ if (nextCode === storyCode) {
+ return;
</file context>
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
2 participants
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.
Adds an initial edit UI for chart tool calls so users can tweak chart params after generation, with the edited config persisted back to the database — both for charts in chats and charts embedded in stories.
Walkthrough
Chat chart edit
edit_chart_params_demo.mp4
End-to-end demo: open the edit dialog, change title/type/series label/color, save, and reload the page to confirm the changes persisted to the DB.
Story chart edit
Story chart with edit pencil icon at top right
Edit chart dialog open inside the story viewer
The same dialog opens from charts embedded in a story. The save path goes through
story.createVersion, creating a new story version with the chart tag swapped out.Changes
Shared
apps/shared/src/story-segments.ts: addsrawTagtoParsedChartBlock(populated bysplitCodeIntoSegments) and a newbuildChartTag()helper so chart config ↔<chart ... />markup can round-trip consistently across chat and story flows.Backend
apps/backend/src/queries/chart-image.ts: addsgetChartOwnerInfo(toolCallId)(project + chat owner lookup) andupdateChartConfig(toolCallId, config)that updatesmessage_part.tool_inputand invalidates the cached PNG inchart_image.apps/backend/src/trpc/chart.routes.ts: adds the ownership-checkedchart.updateConfigmutation that validates withdisplayChart.InputSchemaand writes the new config.Frontend
apps/frontend/src/components/tool-calls/display-chart-edit-dialog.tsx(new): exports two components.ChartConfigEditDialog— presentational form fortitle,chart_type,x_axis_type,x_axis_key, and each series (data_key,label,color). Columns come from the linkedexecute_sqloutput.DisplayChartEditDialog— chat-specific binding that wires the form tochart.updateConfigwith optimisticsetMessagesrollback.apps/frontend/src/components/tool-calls/display-chart.tsx: adds a pencil edit button next to download; uses the new sharedbuildChartTaghelper for the "Add to story" snippet.apps/frontend/src/components/side-panel/story-chart-embed.tsx: exports aStoryChartEmbedShellthat overlays an "Edit chart" pencil on any rendered story chart when the surroundingStoryChartEditProvideris available.apps/frontend/src/components/story-embeds.tsx: wraps the live story chart embed withStoryChartEmbedShell.apps/frontend/src/components/side-panel/story-editor.tsx: threadsrawTaginto the Tiptap chart node view so edits work in the editor's preview.apps/frontend/src/contexts/story-chart-edit.tsx(new): provides asaveChart(rawTag, config)handler that replaces the chart tag in the current story code and saves throughstory.createVersion(with optimistic cache update + rollback).apps/frontend/src/components/side-panel/story-viewer.tsx: mountsStoryChartEditProvideraroundStoryPreview/StoryCodeViewwhen the user owns the latest version and the story isn't archived, readonly, or being streamed. Edit is disabled in the Tiptapeditview to avoid conflicting with in-flight text edits.apps/frontend/src/routes/_sidebar-layout.stories.preview.$chatId.$storySlug.tsx: mounts the provider around the story preview route.apps/frontend/src/contexts/agent.provider.tsx+apps/frontend/src/hooks/use-agent.ts: addsisReadonlyonAgentHelpersso the in-chat edit button is hidden in readonly views (shared chats, chat replay, story side panels).Notes
userIdcheck on server); story edits reuse the existingchatOwnerProcedureownership guard onstory.createVersion.To show artifacts inline, enable in settings.