Bug fix - / makes charts invisible in stories - #1164
Merged
Conversation
A single `\` in a series label made the embedded `<chart>` block's JSON unparseable, so the chart showed "no series configured" instead of rendering. Hand-authored chart blocks routinely write a literal `\` (e.g. `Disc\Rebate`) instead of a JSON-escaped `\\`, producing invalid JSON that both the render and validation paths rejected. Parse the series JSON through a shared `parseSeriesJsonArray` helper that retries after escaping stray backslashes, and route story validation through the same helper (dropping the string-unaware bracket scanner that also false-flagged labels containing `]`). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
🧹 Preview RemovedThe preview deployment for this PR has been cleaned up. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
The backslash-repair regex exempted any `\u` from being escaped, but a JSON `\u` escape is only valid when followed by exactly 4 hex digits. Labels like `A\uZZZZ` or `A\u00` slipped through unrepaired and still failed JSON.parse, hiding the chart. Only exempt `\u` when followed by 4 hex digits; otherwise double the stray backslash like any other. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The tag-extraction regexes used `[^/>]*` for the attribute body, so a `/` inside a quoted value — e.g. `title="13/07 update"` or a `label` like `rev/cost` — truncated the tag and the chart never rendered. A `>` inside a quoted value hit the same class of bug. Centralize a quote-aware attribute-body fragment (`TAG_ATTRS`) in `@nao/shared/story-segments`, plus `chartTagRegex`/`tableTagRegex`/`storyBlockRegex` builders, and route every chart/table tag matcher through them: story-segments, story-validation, backend story-summary and live-story, and the frontend story-editor. Quoted values (with backslash escapes) are now consumed atomically so `/` and `>` inside them no longer terminate the tag, while self-closing `/>` and the `series='[...]'` JSON still parse. Composes with the existing backslash/`\u` series-label repair. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ClaireGz
added a commit
that referenced
this pull request
Jul 13, 2026
- P1 quote-aware block-tag regex: `<table>`/`<chart>` extraction now treats
quoted attribute values as opaque, so `>`/`/` inside a threshold rule's
`formatting='{"...":">="}'` no longer truncates the tag and drops the
formatting. Introduced a shared `TAG_ATTRS` constant in story-segments and
reused it in story-validation, story-summary, and live-story (converges
with the helpers added in #1164).
- Added a threshold (`>=`/`>`) round-trip test through splitCodeIntoSegments
plus a hand-written raw-tag test that would have caught the truncation.
- display_table tool now verifies the query_id result exists (via
getQueryResult) instead of always reporting success.
- Add-to-story button disabled while its mutation is pending (race).
- Serialized formatting edits with an in-flight ref guard + gated the header
menu while pending, and the failure path re-applies this edit's prior
config rather than clobbering newer state.
- Color picker now converts rgba/hex to hex for display via a new shared
`colorToHex`, only falling back to the default when truly unparseable.
- Restricted the format dialog's columns to numeric ones (both display_table
and execute_sql), matching the header menu.
- Removed the genuinely-unused `getTableConfigByToolCallId` helper (no table
download route uses it).
Part of #1085
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ClaireGz
changed the title
ClaireGz
changed the title
Bl3f
approved these changes
Jul 15, 2026
Bl3f
left a comment
Bl3f
left a comment
Contributor
There was a problem hiding this comment.
Can you just remove the comments pls.
Bl3f
added a commit
that referenced
this pull request
Jul 16, 2026
* feat(frontend): add conditional formatting for table columns Introduces a shared, extensible conditional-formatting model for table columns and renders it in the shared TableDisplay so all surfaces (chat SQL output, data table cards, and the story panel) benefit. - Add `@nao/shared/conditional-formatting` with a discriminated-union rule model (color-scale gradient + threshold/bucket) and pure color-scale computation, kept extensible for formula-based rules later. - Render per-column cell backgrounds in TableDisplay; tables without rules render exactly as before. - Add a minimal manual entry point: a per-column header menu (numeric columns) to apply a color scale, highlight above-average values, or clear formatting. State is held in DataTableCard. - Accept an optional `formatting` attribute on story `<table>` blocks and thread it through to both story-panel table embeds (enables LLM auto-application via the existing tool payload). - Unit tests for range computation, color-scale interpolation, and threshold matching. Part of #1085 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(frontend): harden conditional-formatting parsing and color handling Addresses PR review (cubic) findings: - Validate LLM-supplied formatting rule shape before use. Add `isConditionalFormatRule` guard and `sanitizeConditionalFormats`; the story `<table formatting=...>` parser now drops malformed entries, and the table renderer guards each rule so bad JSON can no longer crash the render. - Support 8-digit `#RRGGBBAA` hex endpoint colors (alpha from the last byte), parsing channels by substring to avoid 32-bit overflow. - Reject partially-invalid hex via a strict hex-character check so malformed colors yield no background instead of a misleading one. - Cover the remaining threshold operators (`>`, `<=`, `=`), 3/8-digit and invalid hex, and the sanitizer in tests. Part of #1085 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(shared): validate color-scale rule shape in conditional-formatting guard Addresses PR review (cubic) P1: the color-scale branch of `isConditionalFormatRule` accepted non-string `minColor`/`maxColor` and non-numeric `min`/`max` bounds, so a malformed LLM-supplied rule could pass validation and crash rendering when the color is later parsed. The guard now requires `minColor`/`maxColor` to be strings when present and `min`/`max` to be finite numbers when present, consistent with the threshold branch. Adds tests covering non-string color, non-finite bound, and a fully-specified valid color-scale rule. Part of #1085 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat: add display_table tool + edit UI for table conditional formatting Makes conditional formatting usable end-to-end, like charts. Part A — the agent can apply real conditional formatting: - New `display_table` tool (shared zod schema, backend tool, tool-output, registration) mirroring `display_chart`: it references an execute_sql `query_id` and carries a `conditional_formats` map (color-scale + threshold rules) reusing the shared model. No DB migration — the config is stored in the generic messagePart.toolInput like charts. - System-prompt guidance describing the schema and instructing the agent to use this tool instead of faking formatting with emoji or extra status/label columns. Part B — UI edit affordance (like charts): - `DisplayTableToolCall` renders the formatted table via DataTableCard, with an Edit (pencil) button + per-column header menu. Edits persist to the chat message through a new `table.updateConfig` tRPC route (optimistic message update, mirrors chart.updateConfig). - `TableFormatEditDialog`: pick a rule per column (color scale, or threshold with operator/value/color), reusing Shadcn primitives. - The raw `execute_sql` result table is now editable too via a formatting affordance (session-local; persistence there is deferred). - "Add to story" serializes formatting into a `<table formatting=.../>` block (buildStoryTableBlock), so it renders formatted in the story panel. DataTableCard now supports controlled formatting + a headerActions slot. Tables without rules render exactly as before. Tests: story-table-block round-trip through the parser and display_table schema validation. Shared suite 100 passing. Part of #1085 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: address PR review for table conditional formatting - P1 quote-aware block-tag regex: `<table>`/`<chart>` extraction now treats quoted attribute values as opaque, so `>`/`/` inside a threshold rule's `formatting='{"...":">="}'` no longer truncates the tag and drops the formatting. Introduced a shared `TAG_ATTRS` constant in story-segments and reused it in story-validation, story-summary, and live-story (converges with the helpers added in #1164). - Added a threshold (`>=`/`>`) round-trip test through splitCodeIntoSegments plus a hand-written raw-tag test that would have caught the truncation. - display_table tool now verifies the query_id result exists (via getQueryResult) instead of always reporting success. - Add-to-story button disabled while its mutation is pending (race). - Serialized formatting edits with an in-flight ref guard + gated the header menu while pending, and the failure path re-applies this edit's prior config rather than clobbering newer state. - Color picker now converts rgba/hex to hex for display via a new shared `colorToHex`, only falling back to the default when truly unparseable. - Restricted the format dialog's columns to numeric ones (both display_table and execute_sql), matching the header menu. - Removed the genuinely-unused `getTableConfigByToolCallId` helper (no table download route uses it). Part of #1085 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(frontend): quote-aware tag regexes in the story editor The story editor still used the vulnerable `[^/>]*` attribute pattern in four places, so a threshold-formatted table (operator `>=`/`>` inside `formatting='{...}'`) would truncate when a story was opened or edited. Convert all four `<chart>`/`<table>` regexes in story-editor.tsx to the shared quote-aware `TAG_ATTRS` (via new RegExp), keeping capture-group positions so `.match(...)[1]` still yields the attribute string. `<grid>` regexes are left untouched (their attrs never contain quoted `>`). Regex correctness is covered by the shared story-table-block test that exercises the same `TAG_ATTRS` constant; a dedicated editor test is impractical here (story-editor.tsx pulls in the full @tiptap/React graph). Part of #1085 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore: re-trigger cubic review Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(frontend): polish table conditional-formatting edit UX Three UX changes from user feedback: 1. Remove per-column-header formatting menus. All editing now goes through the single top Edit/Palette toolbar button that opens TableFormatEditDialog. TableDisplay is now display-only for formatting (renders `conditionalFormats`, no `onConditionalFormatsChange`); DataTableCard drops its controlled-format state; execute_sql keeps its Palette dialog (session-local). The header dropdown menus are gone on all editable surfaces. 2. Color scale: pick a main color. `ColorScaleRule` gains an optional `color` field; `resolveCellBackground` derives the gradient from it (a light tint of the color at low alpha → the color at high alpha). Explicit min/max colors still win; rules without `color`/min/max keep the default blue scale, so existing rules render unchanged. The dialog exposes a color picker for the scale, reusing the same swatch component as the threshold color. 3. Dialog layout: the type picker (None / Color scale / Threshold) now sits on its own row in a fixed position; the type-specific controls (scale color, or threshold operator/value/color) render on a new row beneath it, so the picker no longer shifts horizontally when switching types. Tests: added color-scale main-color derivation cases (endpoints, midpoint, rgb input, explicit override, unparseable fallback) and sanitize coverage for the new `color` field. Shared suite 112 passing. Part of #1085 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(frontend): preserve conditional formatting on add-to-story and add story table edit UI Carry conditional_formats into the story <table> block on all add-to-story paths, and add an edit dialog to story-embedded tables that persists formatting. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: reconcile color-scale main color with explicit endpoints Addresses PR review (cubic) on the color-scale `color` field: 1. Advertised color formats now match what renders. `parseColor` gains hsl()/hsla() support, and the display_table schema descriptions state that hex / rgb() / rgba() / hsl() / hsla() are supported and named CSS colors are not (they were silently ignored → fell back to blue). 2. `scaleEndpoints` resolves each end independently: an explicit minColor/maxColor wins for that end; otherwise the end is derived from the main `color`; only when neither is set does it fall back to the default scale. So `color` + a single explicit endpoint no longer reverts the other end to blue. 3. The edit dialog now makes the single main-color picker authoritative: changing it clears minColor/maxColor (preserving min/max domain) so the color takes effect, and the swatch initializes from color ?? maxColor ?? minColor instead of always showing the fallback. Tests: color + one endpoint derives the other from color; hsl/hsla parse. Shared suite 124 passing. Part of #1085 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: address story-formatting + color review findings Backend (story formatting carry-over): - getDisplayTableFormatsForChat now restricts to the ACTIVE message branch (isNull(supersededAt)) so stories don't inherit formatting from edited-away/superseded branches, and orders rows oldest→newest so the latest display_table per query_id wins deterministically. - Extracted the pure reducer to table-config.utils.ts (db-free) and unit tested the deterministic selection. Shared (color parsing): - Clamp HSL saturation/lightness to [0,1] (hue already mod 360) so out-of-range AI colors like hsl(0,200%,50%) yield valid CSS. - Added a common CSS named-color map to parseColor so "red"/"green"/… actually render; schema descriptions now advertise names too (advertised == rendered). Frontend (story embed editing): - Gate the preview-route edit providers on !isChatRunning so a save can't submit captured code mid-generation (matches the side-panel gating). - Extracted the shared save/optimistic pipeline into useStoryBlockEdit, used by both chart and table providers; saves are serialized and build on a code buffer so two quick embed saves no longer discard each other. - Made the tag-replacement util (replaceUniqueStoryBlockTag) and its error messages block-neutral instead of chart-specific. Tests: deterministic reducer (backend), HSL clamping + named colors (shared), neutral tag replacement (frontend). Shared suite 126 passing. Part of #1085 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(frontend): match display_table width to the execute_sql result table The display_table card rendered ~12px narrower on each side than the execute_sql result table. execute_sql's bordered ToolCallWrapper uses `-mx-3` to pull out to the `px-3` message-parts container edge (assistant-message.tsx), while display_table's DataTableCard sat at the plain content width. Apply `className='-mx-3'` to the display_table DataTableCard — the same pattern markdown-table.tsx already uses — so the two tables line up edge-to-edge in the conversation. Scoped to the chat display_table only; story embeds and the fullscreen dialog (which use DataTableCard without this class) are unaffected. Part of #1085 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat: conditional formatting for boolean and string columns Extends the shared conditional-formatting model beyond numeric columns: - New `boolean` rule ({ type: 'boolean', trueColor?, falseColor? }) — a color per truthy/falsy cell; either optional (unset = no background). resolveCellBackground coerces real booleans plus common string/number representations ("true"/"false", 1/0). - New `string` rule ({ type: 'string', operator: 'equals'|'in'|'like', value: string | string[], color }) — equals (exact), in (value in list), like (case-insensitive substring). Non-string cells are stringified. Wired through the existing choke points: - isConditionalFormatRule / sanitizeConditionalFormats validate the new shapes (colors are strings; `in` requires a string array; etc.). - zod schema (display_table tool) adds both rule types to the union. - Agent system prompt documents the boolean/string rules with examples. - Edit dialog now offers formatting on boolean and string columns too: it detects each column's data type (new getFormattableColumnType / isBooleanColumn / isStringColumn helpers) and shows the applicable rule kinds — numeric → color-scale/threshold, boolean → true/false colors, string → operator + value + color. Callers pass all columns + data. Rendering flows through TableDisplay and story <table formatting=…> blocks unchanged. Backward compatible — existing numeric rules are untouched. Tests: boolean resolver (true/false, coercion, unset, non-bool), string resolver (equals/in/like/no-match/stringify/nullish), plus guard/sanitize coverage for both. Shared suite 137 passing. Part of #1085 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Small changes, merge display chart and display table --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Christophe Blefari <christophe.blefari@gmail.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-backslash-parsing # Conflicts: # apps/backend/src/services/live-story.ts # apps/backend/src/utils/story-summary.ts # apps/frontend/src/components/side-panel/story-editor.tsx # apps/shared/src/story-segments.ts # apps/shared/src/story-validation.ts
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.
Issue
Closes #1084
Implemented
Disc\Rebate) so the chart still renders\uescapes to be followed by 4 hex digits<chart>/<table>tag parsing so/or>inside a title or label (e.g.title="13/07 update") no longer truncates the tag; centralized into shared helpersTests done
\uescapes, and slashes in titles/labels🤖 Generated with Claude Code