Conversation
…rect testset imports Replace barrel import from `../testset` with direct imports from specific testset files (`revisionEntity`, `revisionSchema`, `store`) to break circular dependency that caused "Cannot access before initialization" errors in standalone consumers. Update pnpm workspace and package.json to include `apps/*` for design-mockups app support.
…mockup app references Update JSON-string UX design docs to reflect production component audit (2026-05-04). Add references to new Next.js mockup app at `web/apps/design-mockups/` which supersedes static HTML mockups. Clarify unique contributions of gaps 02, 03, 05, and 06 after discovering production already ships capabilities originally claimed as missing (CellContentPopover, collapsedFields machinery, groupColumns, ChatMessageList).
…-to-string editing Introduce gap-09 (variable provenance + usage state in playground execution item) to overview, variants README, and TypeChip. Add `unused`, `draft`, and `chain` chip variants for four-state variable classification (used/chain/draft/unused). Update PlaygroundExecutionItemCompact to unify string and null editing into one controlled-input branch so typing into a null field initializes as string without losing focus
…reamline mockup shell Add root-level serialized view (JSON/YAML) to ProposedDrillIn via Segmented toggle in row header. Users can paste a full testcase payload in one shot instead of typing field-by-field; edits auto-apply on keystroke with inline parse-error feedback. Remove `competitiveNotes` prop and styling from MockupPageShell to simplify page headers. Update already-shipped, alt-tree-pane, gap-01, and gap-02 page copy to focus
…pages Update gap-01, gap-03, gap-04, gap-05, and gap-06 solution link descriptions to explicitly reference the "kitchen-sink Vanuatu row" fixture instead of generic "pick a fixture" instructions. Specify which columns/fields demonstrate each gap's unique contribution (e.g., nested `geo.coordinates` for auto-expand, literal `"geo.region"` collision, 5-turn messages array with tool calls). Clarify that Tuvalu and Kiribati rows in the testset demonstrate missing-
…s and add kitchen-sink schema Add `FIXTURE_KITCHEN_SINK_INFERRED_SCHEMA` covering every column in the testset's known-union (country, population, nested geo/inputs/outputs, literal-dotted keys, messages array, stringified metadata). Update solutions-drill-in to always render the schema-aware form section regardless of active fixture—switch between Tuvalu schema and kitchen-sink schema based on selection. Update gap-07 page copy
…able validation from index Remove gap-07 (schema-aware edit form) and gap-08 (playground variable validation) entries from design mockups index page. These gaps were surfaced by competitive analysis but are being deprioritized or consolidated into other work.
…nt axes Separate type classification into two orthogonal axes per 2026-05-05 feedback: type primitive (what the value IS—string/number/boolean/null/object/array) and optional render hint (how it renders—markdown/stringified/messages/tool-calls). Remove `long-str` variant; long-form strings now show `[string][markdown]` chips. Update all classify functions across PlaygroundExecutionItem, PlaygroundExecutionItemCompact, Play
…rings in table cells Introduce `markdown` render hint alongside existing `stringified`, `messages`, and `tool-calls` hints. Cells containing strings >100 chars or with newlines now display `[string][markdown]` chip pair plus a one-line preview (first non-empty line, stripped of markdown syntax, truncated to 60 chars) with char/line count metadata. Column type detection promotes string columns to `markdown` hint when all observed strings match the heuristic; `stringified` takes precedence when
…own render-hint chip Relocate the short/long editor-mode toggle from the `[string]` type chip to the `[markdown]` render-hint chip per 2026-05-05 feedback. When a markdown render hint is visible, its popover owns the mode-switching action (plain inline input ↔ Lexical editor with markdown preview); the type chip's popover retains only type conversions. When no render hint exists (plain string), the type chip keeps the mode toggle
… use concrete collision values
Add `flexWrap: wrap` to ProposedDrillIn field header left section so type/render-hint/name chips (dotted-key, collision, shadowed) wrap to second line when narrow instead of being clipped by parent overflow:hidden. Replace placeholder AI-meta labels in dot-key collision fixtures with concrete values: literal `"geo.region"` = "Melanesia" (what `{{geo.region}}` resolves to under literal-key-first), nested `geo.region` = "Pacific Islands" (what `{{$.geo.region}}` JSONPath resolves to). Makes
…on items and prompt config view Add collapsible footer to PlaygroundExecutionItem and PlaygroundExecutionItemCompact showing testcase columns the prompt chain doesn't reference. Defaults closed; click to reveal column names as inline chips. Filter kitchen-sink execution-item inputs to only variables actually used/chain/draft (via IN_USE_VARIABLE_NAMES set) instead of dumping all 12 testcase columns—unused columns stay reachable via
…tionPlaygroundShell and add prompt config pairing Move PromptVariableValidation component logic into ProductionPlaygroundShell.tsx and delete the standalone file. Update solutions-playground to render prompt config alongside execution items in three-way compare grid, making the variable contract visible (prompt defines variables → testcase fills them). Add production playground shell section showing today's live surface before
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR adds comprehensive JSON↔String UX design documentation, archived notes, test fixtures and backend-response samples, static variant mock pages and shared CSS, plus a new Next.js design-mockups app with providers, pages, interactive components, fixtures, and utilities to prototype the proposed UX across table, drill-in, playground, and variables surfaces. ChangesDesign Documentation & Fixtures for JSON↔String UX
Design Mockups Next.js Application
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
| if (value === null) return `${pad}null` | ||
| if (typeof value === "string") { | ||
| // Quote strings that contain special chars to keep parseability hint | ||
| return /[:#\n]/.test(value) ? `${pad}"${value.replace(/"/g, '\\"')}"` : `${pad}${value}` |
There was a problem hiding this comment.
Actionable comments posted: 18
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (17)
web/apps/design-mockups/src/components/proposed/ProductionPlaygroundShell.tsx-138-140 (1)
138-140:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd accessible names to the icon-only buttons.
These controls are rendered with glyphs only, so assistive tech gets ambiguous button names. Please add
aria-labels and mark the decorative icon content asaria-hidden.Also applies to: 235-243
docs/designs/json-string-ux/test-fixtures/README.md-20-20 (1)
20-20:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix fixture-count inconsistencies in the README.
The doc currently conflicts on counts ("4 testcases each" and "seven separate files"), while this fixture set is clearly variable-sized and contains eight files.
Suggested fix
-1. **Upload each file as a separate testset** in Agenta (4 testcases each ≈ enough variety). +1. **Upload each file as a separate testset** in Agenta (testcase counts vary by fixture). -## Why seven separate files instead of one big one +## Why eight separate files instead of one big oneAlso applies to: 32-32
docs/designs/json-string-ux/test-fixtures/03-arrays.json-22-26 (1)
22-26:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlign Kiribati answer text with structured capital value.
inputs.correct_answersays "Tarawa" whileoutputs.capitalis "South Tarawa". Keeping one canonical value in the same fixture row avoids conflicting expected results.Suggested fix
- "correct_answer": "The capital of Kiribati is Tarawa." + "correct_answer": "The capital of Kiribati is South Tarawa."docs/designs/json-string-ux/variants/README.md-13-13 (1)
13-13:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix broken relative links to
web/paths.From this directory, links to repo-root
web/...need four..segments, not three. Line 13 and Lines 126–128 currently point to non-existent paths.🛠️ Proposed fix
-**Interactive mockup app (added 2026-05-04):** the static `.html` mockups in this folder are now superseded by an interactive Next.js app at [`../../../web/apps/design-mockups/`](../../../web/apps/design-mockups/). Run `pnpm --filter design-mockups dev` (port 3030) and open the **solution** pages — they mount production components alongside the proposal so the diff is visible: +**Interactive mockup app (added 2026-05-04):** the static `.html` mockups in this folder are now superseded by an interactive Next.js app at [`../../../../web/apps/design-mockups/`](../../../../web/apps/design-mockups/). Run `pnpm --filter design-mockups dev` (port 3030) and open the **solution** pages — they mount production components alongside the proposal so the diff is visible: -| `DrillInContent` ([DrillInView/DrillInContent.tsx:178](../../../web/oss/src/components/DrillInView/DrillInContent.tsx)) | read or edit | The workhorse. Detects type, picks widget, renders ChatMessageEditor for messages, owns breadcrumb + raw toggle + column-mapping popover. Wrapped by every other drill-in. | gap-01, gap-03, gap-04, gap-05, gap-06 | -| `DrillInFieldHeader` ([DrillInView/DrillInFieldHeader.tsx:209](../../../web/oss/src/components/DrillInView/DrillInFieldHeader.tsx)) | read or edit | Per-field row inside `DrillInContent`. Renders field name + item count + view-mode `<Select>` (`Text`/`JSON`/`YAML`/`Markdown`/`Raw`) + action buttons. **No type chip today** — that's what gap-01 proposes adding alongside the existing chrome. | gap-01 (chip surface) | -| `EntityDualViewEditor` ([DrillInView/EntityDualViewEditor.tsx:71](../../../web/oss/src/components/DrillInView/EntityDualViewEditor.tsx)) | edit | Adds the Fields ↔ JSON toggle on top of `EntityDrillInView`. Used by testcase drawer, "Add to Testset" preview. | gap-03, gap-04 (the JSON view `?? ""` projection lives at line 144–155) | +| `DrillInContent` ([DrillInView/DrillInContent.tsx:178](../../../../web/oss/src/components/DrillInView/DrillInContent.tsx)) | read or edit | The workhorse. Detects type, picks widget, renders ChatMessageEditor for messages, owns breadcrumb + raw toggle + column-mapping popover. Wrapped by every other drill-in. | gap-01, gap-03, gap-04, gap-05, gap-06 | +| `DrillInFieldHeader` ([DrillInView/DrillInFieldHeader.tsx:209](../../../../web/oss/src/components/DrillInView/DrillInFieldHeader.tsx)) | read or edit | Per-field row inside `DrillInContent`. Renders field name + item count + view-mode `<Select>` (`Text`/`JSON`/`YAML`/`Markdown`/`Raw`) + action buttons. **No type chip today** — that's what gap-01 proposes adding alongside the existing chrome. | gap-01 (chip surface) | +| `EntityDualViewEditor` ([DrillInView/EntityDualViewEditor.tsx:71](../../../../web/oss/src/components/DrillInView/EntityDualViewEditor.tsx)) | edit | Adds the Fields ↔ JSON toggle on top of `EntityDrillInView`. Used by testcase drawer, "Add to Testset" preview. | gap-03, gap-04 (the JSON view `?? ""` projection lives at line 144–155) |Also applies to: 126-128
docs/designs/json-string-ux/archive/05-playground-variables.md-9-9 (1)
9-9:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd language identifiers to fenced code blocks.
Lines 9, 58, and 82 use unlabeled fenced blocks, which triggers MD040. Use
textfor these ASCII wireframes/snippets.🛠️ Proposed fix
-``` +```text ┌─────────────────────────────────────┐ ... -``` +``` -``` +```text ⌄ propertyName [type-chip] -``` +``` -``` +```text Prompt editor: "Capital of {{co│}}" ... -``` +```Also applies to: 58-58, 82-82
docs/designs/json-string-ux/variants/gap-04-fe-shape-preservation.md-130-130 (1)
130-130:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix typo in fixture name (
Stringified).Line 130 says
04 Stringfied Nested; this should be04 Stringified Nested.🛠️ Proposed fix
-- **Stringified-JSON fault line** — Braintrust's variable validator (gap-08 territory) **false-warns** on `04 Stringfied Nested` because their schema treats `metadata` as a string, not a parsed object. Same fault line as the union projection: once a column is a stringified JSON, every downstream tool that depends on the schema silently degrades. Our gap-02 "parse stringified JSON on detect" affordance feeds this gap's fix — without parsing, even Braintrust gets it wrong. +- **Stringified-JSON fault line** — Braintrust's variable validator (gap-08 territory) **false-warns** on `04 Stringified Nested` because their schema treats `metadata` as a string, not a parsed object. Same fault line as the union projection: once a column is a stringified JSON, every downstream tool that depends on the schema silently degrades. Our gap-02 "parse stringified JSON on detect" affordance feeds this gap's fix — without parsing, even Braintrust gets it wrong.docs/designs/json-string-ux/archive/06-edge-cases.md-50-70 (1)
50-70:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd language identifiers to fenced blocks.
Line 50 and Line 97 start fenced code blocks without a language, which can trip markdown lint in docs pipelines.
✏️ Proposed fix
-``` +```text Column header (user can opt in): ┌─────────────────────────────────────────────────┐ │ outputs [str] [convert column]│ └─────────────────────────────────────────────────┘ @@ -``` +```text ⌄ profile [str] [→ obj] [→ arr] ┌────────────────────────────────────────┐ │ name: Ada │ ← not valid JSON └────────────────────────────────────────┘ ⚠ Cannot convert: not valid JSON. Try wrapping in {}. [Dismiss]Also applies to: 97-103
docs/designs/json-string-ux/variants/gap-04-shape-preservation.html-25-25 (1)
25-25:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix broken relative links to source anchors.
Line 25 and Line 33 use
../../../web/..., which is one level short from this file’s location and likely resolves incorrectly.🔗 Proposed fix
- <p><strong>State stays clean</strong> unless the user explicitly edits the JSON view. The "pollution" in the screenshots is a render-time projection at <a href="../../../web/oss/src/components/DrillInView/EntityDualViewEditor.tsx"><code>EntityDualViewEditor.tsx:144–155</code></a> (the <code>?? ""</code> fallback). The column union is a <em>convenience feature</em> — if testcase A has columns X+Z and testcase B has only X, the user can author Z in B without manually adding the column.</p> + <p><strong>State stays clean</strong> unless the user explicitly edits the JSON view. The "pollution" in the screenshots is a render-time projection at <a href="../../../../web/oss/src/components/DrillInView/EntityDualViewEditor.tsx"><code>EntityDualViewEditor.tsx:144–155</code></a> (the <code>?? ""</code> fallback). The column union is a <em>convenience feature</em> — if testcase A has columns X+Z and testcase B has only X, the user can author Z in B without manually adding the column.</p> @@ - <p>The drill-in's JSON view (<code>EntityDualViewEditor</code>'s "JSON" mode) materializes the column union with empty fallbacks via <a href="../../../web/oss/src/components/DrillInView/EntityDualViewEditor.tsx"><code>EntityDualViewEditor.tsx:144–155</code></a> — <code>values[col.key] = entityData[col.key] ?? ""</code>. The underlying <code>data</code> stays clean. Opening the drill-in creates no draft.</p> + <p>The drill-in's JSON view (<code>EntityDualViewEditor</code>'s "JSON" mode) materializes the column union with empty fallbacks via <a href="../../../../web/oss/src/components/DrillInView/EntityDualViewEditor.tsx"><code>EntityDualViewEditor.tsx:144–155</code></a> — <code>values[col.key] = entityData[col.key] ?? ""</code>. The underlying <code>data</code> stays clean. Opening the drill-in creates no draft.</p>Also applies to: 33-33
docs/designs/json-string-ux/archive/06-edge-cases.md-88-89 (1)
88-89:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix stale section reference.
Line 88 references “§ Legacy above,” but there is no “Legacy” section in this document; this makes the guidance ambiguous.
✏️ Proposed fix
-**RFC contract:** strings stay strings. Type metadata determines this. If the cell is marked `string`, treat as string. Migration is opt-in (see § Legacy above). +**RFC contract:** strings stay strings. Type metadata determines this. If the cell is marked `string`, treat as string. Migration is opt-in (see § User-uploaded stringified JSON above).docs/designs/json-string-ux/variants/gap-06-messages-renderer.html-147-235 (1)
147-235:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClarify tool-calls scope between the “After” mock and the recommendation.
The “After” walkthrough currently includes inline
tool_callsrecognition, but the recommendation says to defertool_callsin v1. Please explicitly label that part as optional/future (or align the sections) to avoid implementation drift.Also applies to: 315-316
docs/designs/json-string-ux/archive/02-testset-table.md-11-27 (1)
11-27:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd language identifiers to fenced code blocks (markdownlint MD040).
Please annotate these fences with a language (e.g.,
text) so markdownlint passes cleanly and rendering stays consistent across viewers.Also applies to: 79-81
docs/designs/json-string-ux/archive/01-display-and-indicators.md-30-39 (1)
30-39:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSpecify fenced-code languages for markdownlint compliance.
These code fences are missing language tags; please add one (
textis fine) to satisfy MD040 and keep docs tooling clean.Also applies to: 43-51, 57-66
docs/designs/json-string-ux/archive/03-testcase-drawer.md-9-38 (1)
9-38:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd language tags to fenced blocks to clear MD040 warnings.
Please mark these fenced blocks with a language (
textworks) to align with markdownlint requirements.Also applies to: 42-44, 71-89, 99-106
docs/designs/json-string-ux/competitive-analysis.md-471-649 (1)
471-649:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNormalize fixture naming:
Stringified, notStringfiedThe doc repeatedly uses
Stringfied(including in fixture filename references). Please standardize toStringifiedto match expected fixture naming and keep search/linking reliable.docs/designs/json-string-ux/test-fixtures/02-nested-native.json-15-20 (1)
15-20:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlign Kiribati answer text with the expected output value
inputs.correct_answersays Tarawa whileoutputs.capitalis South Tarawa. This mismatch can create avoidable fixture-level assertion noise.Suggested fix
- "correct_answer": "The capital of Kiribati is Tarawa." + "correct_answer": "The capital of Kiribati is South Tarawa."docs/designs/json-string-ux/variants/gap-01-type-chips.html-6-7 (1)
6-7:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDefine the new layout selectors in
shared.css.This page relies on classes like
nav-jump,page,today-section,grid-2,shot, anddrawer-panel, but the shared stylesheet shown fordocs/designs/json-string-ux/variants/shared.cssonly defines chip/table rules. As-is, the new doc will render mostly as unstyled HTML.Also applies to: 10-15, 34-75
web/apps/design-mockups/src/components/proposed/TreeDrillIn.tsx-16-17 (1)
16-17:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winImplement the promised Enter shortcut.
The component header says Enter should focus the right-pane editor, but
onKeyonly handles arrow keys. Right now Enter is a no-op.Also applies to: 568-585
🧹 Nitpick comments (4)
web/apps/design-mockups/src/components/proposed/ProposedTableCell.tsx (1)
132-134: ⚡ Quick win
parse?is rendered as an interactive button but has no actionA clickable control with no handler reads as broken UX. Either wire an
onClickcallback (e.g., via prop) or render this as non-interactive hint text.web/apps/design-mockups/src/components/proposed/testsetTableHelpers.ts (1)
550-552: 💤 Low valueMarkdown detection heuristic is very broad.
The
looksLikeMarkdownStringfunction considers any string over 100 characters OR containing a newline as "markdown". This will flag many non-markdown strings (e.g., long sentences, multi-line plain text). Since this is mockup code for demonstrating the UX, the heuristic is likely intentional for showcasing the markdown renderer, but worth noting if the logic is ever promoted to production.,
web/apps/design-mockups/src/pages/gap-03-drill-in-root-view.tsx (1)
36-37: 💤 Low valueHardcoded line number reference may become stale.
The reference to "line 1284 of
DrillInContent.tsx" will become inaccurate as the production code evolves. Consider referencing the component/function name instead of line numbers, or noting that line numbers are approximate.,
web/apps/design-mockups/src/components/proposed/TypeChip.tsx (1)
100-181: 🏗️ Heavy liftPrefer declarative classes over inline style objects here.
This new frontend component hard-codes most visuals in
styleprops and mutates hover state via inline mouse handlers. That makes the chip harder to theme, harder to reuse, and fights the repo’s Tailwind-first conventions. Moving the stable variant styles and hover/focus states into classes would also let you drop the per-render handler allocations.As per coding guidelines, "Always prefer Tailwind utility classes over CSS-in-JS or separate CSS files for styling" and "Avoid inline functions and objects in render; use
useCallbackto create stable callbacks outside of render logic".Also applies to: 263-326
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2b3fdffa-18fb-4464-9a82-668da59013b2
⛔ Files ignored due to path filters (97)
docs/designs/json-string-ux/competitive-screenshots/braintrust-01-datasets-list.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.17.55.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.17.59.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.18.05.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.18.16.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.18.23.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.18.27.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.18.33.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.18.38.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.18.48.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.18.54.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.18.58.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.19.05.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.19.10.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.19.14.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.19.21.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.19.26.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.19.36.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.19.42.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.19.59.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.20.06.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.20.13.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.20.18.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.20.22.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.20.26.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.20.27.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.20.35.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.20.43.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.20.50.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.20.56.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.21.10.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.28.08.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.28.14.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.28.21.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.28.30.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.28.40.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.28.46.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/Screenshot 2026-05-01 at 11.28.51.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.23.35.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.23.41.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.23.47.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.23.54.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.24.14.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.24.24.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.24.36.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.24.42.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.24.53.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.24.58.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.25.07.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.25.11.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.25.17.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.25.21.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.25.28.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.25.33.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.25.39.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.25.43.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.25.56.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.26.05.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.26.13.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.26.18.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.27.25.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.27.33.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.27.48.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.28.00.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.28.12.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.28.18.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.28.27.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.28.33.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.28.41.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.28.48.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.29.05.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.29.10.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.29.20.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.29.25.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.29.36.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/Screenshot 2026-05-04 at 01.29.44.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/new/Screenshot 2026-05-04 at 01.36.12.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/new/Screenshot 2026-05-04 at 01.37.06.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/new/Screenshot 2026-05-04 at 01.38.03.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/new/Screenshot 2026-05-04 at 01.38.32.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/new/Screenshot 2026-05-04 at 01.38.50.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/new/Screenshot 2026-05-04 at 01.41.05.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/new/Screenshot 2026-05-04 at 01.46.26.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/new/Screenshot 2026-05-04 at 01.46.33.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/new/Screenshot 2026-05-04 at 01.46.54.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/new/Screenshot 2026-05-04 at 01.47.06.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/new/Screenshot 2026-05-04 at 01.58.31.pngis excluded by!**/*.pngdocs/designs/json-string-ux/screenshots/competitors/new/Screenshot 2026-05-04 at 01.58.38.pngis excluded by!**/*.pngdocs/designs/json-string-ux/test-fixtures/01-flat-strings.csvis excluded by!**/*.csvdocs/designs/json-string-ux/test-fixtures/02-nested-native.csvis excluded by!**/*.csvdocs/designs/json-string-ux/test-fixtures/03-arrays.csvis excluded by!**/*.csvdocs/designs/json-string-ux/test-fixtures/04-stringified-nested.csvis excluded by!**/*.csvdocs/designs/json-string-ux/test-fixtures/05-mixed-per-column.csvis excluded by!**/*.csvdocs/designs/json-string-ux/test-fixtures/06-deeply-nested.csvis excluded by!**/*.csvdocs/designs/json-string-ux/test-fixtures/07-messages-and-tools.csvis excluded by!**/*.csvdocs/designs/json-string-ux/test-fixtures/08-dot-key-collision.csvis excluded by!**/*.csvweb/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (100)
docs/designs/json-string-ux/00-overview.mddocs/designs/json-string-ux/archive/01-display-and-indicators.mddocs/designs/json-string-ux/archive/02-testset-table.mddocs/designs/json-string-ux/archive/03-testcase-drawer.mddocs/designs/json-string-ux/archive/04-conversion-toggle.mddocs/designs/json-string-ux/archive/05-playground-variables.mddocs/designs/json-string-ux/archive/06-edge-cases.mddocs/designs/json-string-ux/archive/07-decisions.mddocs/designs/json-string-ux/archive/README.mddocs/designs/json-string-ux/archive/old-comparison-board.htmldocs/designs/json-string-ux/backend-response data/01-response.jsondocs/designs/json-string-ux/backend-response data/02-response.jsondocs/designs/json-string-ux/backend-response data/03-response.jsondocs/designs/json-string-ux/backend-response data/04-response.jsondocs/designs/json-string-ux/backend-response data/05-response.jsondocs/designs/json-string-ux/backend-response data/06-response.jsondocs/designs/json-string-ux/backend-response data/07-response.jsondocs/designs/json-string-ux/backend-response data/08-response.jsondocs/designs/json-string-ux/competitive-analysis.mddocs/designs/json-string-ux/test-fixtures/01-flat-strings.jsondocs/designs/json-string-ux/test-fixtures/02-nested-native.jsondocs/designs/json-string-ux/test-fixtures/03-arrays.jsondocs/designs/json-string-ux/test-fixtures/04-stringified-nested.jsondocs/designs/json-string-ux/test-fixtures/05-mixed-per-column.jsondocs/designs/json-string-ux/test-fixtures/06-deeply-nested.jsondocs/designs/json-string-ux/test-fixtures/07-messages-and-tools.jsondocs/designs/json-string-ux/test-fixtures/08-dot-key-collision.jsondocs/designs/json-string-ux/test-fixtures/README.mddocs/designs/json-string-ux/variants/README.mddocs/designs/json-string-ux/variants/gap-01-type-chips.htmldocs/designs/json-string-ux/variants/gap-01-type-chips.mddocs/designs/json-string-ux/variants/gap-02-table-cells.htmldocs/designs/json-string-ux/variants/gap-02-table-object-array-cells.mddocs/designs/json-string-ux/variants/gap-03-drill-in-root-view-bailout.mddocs/designs/json-string-ux/variants/gap-03-drill-in-root-view.htmldocs/designs/json-string-ux/variants/gap-04-fe-shape-preservation.mddocs/designs/json-string-ux/variants/gap-04-shape-preservation.htmldocs/designs/json-string-ux/variants/gap-05-dot-key-disambiguation.htmldocs/designs/json-string-ux/variants/gap-05-dot-key-vs-nested-disambiguation.mddocs/designs/json-string-ux/variants/gap-06-messages-renderer-coverage.mddocs/designs/json-string-ux/variants/gap-06-messages-renderer.htmldocs/designs/json-string-ux/variants/gap-09-variable-provenance.mddocs/designs/json-string-ux/variants/index.htmldocs/designs/json-string-ux/variants/shared.cssweb/apps/design-mockups/next-env.d.tsweb/apps/design-mockups/next.config.tsweb/apps/design-mockups/package.jsonweb/apps/design-mockups/postcss.config.mjsweb/apps/design-mockups/src/components/DrawerSurface.tsxweb/apps/design-mockups/src/components/MockupDrillInUIProvider.tsxweb/apps/design-mockups/src/components/MockupPageShell.tsxweb/apps/design-mockups/src/components/MockupShell.tsxweb/apps/design-mockups/src/components/SideBySide.tsxweb/apps/design-mockups/src/components/StubDrillIn.tsxweb/apps/design-mockups/src/components/StubDualView.tsxweb/apps/design-mockups/src/components/StubMoleculeDrillIn.tsxweb/apps/design-mockups/src/components/proposed/ChipConversionPopover.tsxweb/apps/design-mockups/src/components/proposed/PlaygroundExecutionItem.tsxweb/apps/design-mockups/src/components/proposed/PlaygroundExecutionItemCompact.tsxweb/apps/design-mockups/src/components/proposed/PlaygroundExecutionItemToday.tsxweb/apps/design-mockups/src/components/proposed/PlaygroundVariableMap.tsxweb/apps/design-mockups/src/components/proposed/ProductionPlaygroundShell.tsxweb/apps/design-mockups/src/components/proposed/PromptConfigView.tsxweb/apps/design-mockups/src/components/proposed/ProposedDrillIn.tsxweb/apps/design-mockups/src/components/proposed/ProposedTableCell.tsxweb/apps/design-mockups/src/components/proposed/RowDetailPopover.tsxweb/apps/design-mockups/src/components/proposed/SchemaForm.tsxweb/apps/design-mockups/src/components/proposed/TreeDrillIn.tsxweb/apps/design-mockups/src/components/proposed/TypeChip.tsxweb/apps/design-mockups/src/components/proposed/testsetTableHelpers.tsweb/apps/design-mockups/src/data/stubTestcases.tsweb/apps/design-mockups/src/pages/_app.tsxweb/apps/design-mockups/src/pages/_document.tsxweb/apps/design-mockups/src/pages/already-shipped.tsxweb/apps/design-mockups/src/pages/alt-tree-pane.tsxweb/apps/design-mockups/src/pages/gap-01-type-chips.tsxweb/apps/design-mockups/src/pages/gap-02-table-cells.tsxweb/apps/design-mockups/src/pages/gap-03-drill-in-root-view.tsxweb/apps/design-mockups/src/pages/gap-04-shape-preservation.tsxweb/apps/design-mockups/src/pages/gap-05-dot-key-disambiguation.tsxweb/apps/design-mockups/src/pages/gap-06-messages-renderer.tsxweb/apps/design-mockups/src/pages/gap-07-schema-aware-form.tsxweb/apps/design-mockups/src/pages/gap-08-playground-variable-validation.tsxweb/apps/design-mockups/src/pages/gap-09-variable-provenance.tsxweb/apps/design-mockups/src/pages/index.tsxweb/apps/design-mockups/src/pages/molecule-drill-in.tsxweb/apps/design-mockups/src/pages/solutions-drill-in.tsxweb/apps/design-mockups/src/pages/solutions-playground.tsxweb/apps/design-mockups/src/pages/solutions-tables.tsxweb/apps/design-mockups/src/styles/animations.cssweb/apps/design-mockups/src/styles/code-editor-styles.cssweb/apps/design-mockups/src/styles/editor-theme.cssweb/apps/design-mockups/src/styles/globals.cssweb/apps/design-mockups/src/styles/tokens/antd-themeConfig.jsonweb/apps/design-mockups/tailwind.config.tsweb/apps/design-mockups/tsconfig.jsonweb/apps/design-mockups/tsconfig.tsbuildinfoweb/oss/src/state/entities/testcase/queries.tsweb/package.jsonweb/pnpm-workspace.yaml
| theme={{ | ||
| algorithm: antdTheme.defaultAlgorithm, | ||
| token: { | ||
| fontFamily: inter.style.fontFamily, | ||
| fontFamilyCode: inter.style.fontFamily, | ||
| ...antdTokens.token, | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| ...((antdTokens as any).components ?? {}), | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find web -name "MockupShell.tsx" -o -name "*antdToken*" -o -name "*antd*token*" | head -20Repository: Agenta-AI/agenta
Length of output: 115
🏁 Script executed:
# Find the file and examine it
cat -n web/apps/design-mockups/src/components/MockupShell.tsx | head -80Repository: Agenta-AI/agenta
Length of output: 2834
🏁 Script executed:
# Search for antdTokens definition
rg "antdTokens" web/apps/design-mockups/ -A 5 -B 5Repository: Agenta-AI/agenta
Length of output: 2365
🏁 Script executed:
cat web/apps/design-mockups/src/styles/tokens/antd-themeConfig.json | head -100Repository: Agenta-AI/agenta
Length of output: 3167
🏁 Script executed:
# Check if antdTokens has components field
rg "components" web/apps/design-mockups/src/styles/tokens/antd-themeConfig.json -A 5 -B 5Repository: Agenta-AI/agenta
Length of output: 361
🏁 Script executed:
# Check other usages of ConfigProvider theme in the codebase to see the correct pattern
rg "ConfigProvider" web --type tsx --type ts -A 10 | grep -A 10 "components:" | head -40Repository: Agenta-AI/agenta
Length of output: 88
🏁 Script executed:
# Look for other theme configurations with components to confirm the structure
rg "theme\s*=\s*\{" web --type tsx --type ts -A 15 | grep -B 5 -A 10 "components" | head -60Repository: Agenta-AI/agenta
Length of output: 88
🏁 Script executed:
# Check other usages of ConfigProvider theme in the codebase
rg "ConfigProvider" web -A 10 | grep -A 10 "components:" | head -40Repository: Agenta-AI/agenta
Length of output: 42
🏁 Script executed:
# Look for theme configurations with components field
rg "theme\s*=\s*\{" web -A 15 | grep -B 5 -A 10 "components" | head -80Repository: Agenta-AI/agenta
Length of output: 7285
🏁 Script executed:
# Search for ConfigProvider with theme in the OSS app to see the correct pattern
find web/apps/oss -name "*.tsx" -o -name "*.ts" | xargs grep -l "ConfigProvider" | head -5Repository: Agenta-AI/agenta
Length of output: 108
🏁 Script executed:
# Check the actual ThemeContextProvider file in OSS to see the full context
cat web/oss/src/components/Layout/ThemeContextProvider.tsxRepository: Agenta-AI/agenta
Length of output: 2917
🏁 Script executed:
# Verify the antd ConfigProvider type signature to understand the correct structure
rg "ConfigProvider" web -A 20 | grep -B 5 -A 15 "theme\s*\{" | head -100Repository: Agenta-AI/agenta
Length of output: 79
🏁 Script executed:
# Check Ant Design version and look at ConfigProvider type definition
rg "ConfigProvider" web -B 5 | grep "from.*antd" | head -5Repository: Agenta-AI/agenta
Length of output: 375
🏁 Script executed:
# Let's check what the actual structure of ConfigProvider's theme prop should be
# Look at package.json to see antd version
cat web/package.json | grep -A 2 -B 2 "antd"Repository: Agenta-AI/agenta
Length of output: 42
🏁 Script executed:
# Check if there's any TypeScript type definition or docs in the repo about theme structure
find web -name "*.d.ts" -o -name "*types*" | xargs grep -l "theme" 2>/dev/null | head -5Repository: Agenta-AI/agenta
Length of output: 42
🏁 Script executed:
# Find package.json files and check antd version
find web -name "package.json" -exec grep -H "antd" {} \; | head -20Repository: Agenta-AI/agenta
Length of output: 566
🏁 Script executed:
# Look at the root package.json
cat package.json | grep -A 5 -B 5 "antd"Repository: Agenta-AI/agenta
Length of output: 105
🌐 Web query:
Ant Design v6 ConfigProvider theme components field correct structure
💡 Result:
Ant Design v6: correct ConfigProvider theme.components structure Use theme.components as a map keyed by the antd component name (e.g. Button). Each value is the component token override object (optionally including algorithm). Example: const themeConfig = { components: { Button: { colorPrimary: '#00b96b', algorithm: true, // optional }, Input: { colorPrimary: '#eb2f96', algorithm: true, // optional }, }, }; ... Key points 1) theme.components is a ComponentsConfig map: “Component name like Button” → “ComponentToken & AliasToken …” [1]. 2) The supported shape is theme={{ components: { Button: {... } } }} (not theme={{ Button: {... } }}) [1]. 3) If you were using v5-style Button keys like defaultBg/defaultColor/defaultHoverColor, they may not work in v6 because the Button token/semantic theming has changed; issues report these v5 keys as no longer supported in v6 for Button theming [2]. If you share the exact token keys you’re trying to set (e.g. defaultBg vs defaultHoverBg, etc.), I can tell you which v6 token names/fields correspond.
Citations:
- 1: https://ant.design/docs/react/customize-theme/
- 2: ConfigProvider - defaultBg, defaultColor and defaultColor for Button not working in v6.1.1 ant-design/ant-design#56232
🏁 Script executed:
# Let's check the structure of antdTokens.components to confirm it matches the expected structure
cat web/apps/design-mockups/src/styles/tokens/antd-themeConfig.json | jq '.components' | head -50Repository: Agenta-AI/agenta
Length of output: 1610
Move component overrides to correct theme field
Ant Design v6's ConfigProvider expects component overrides in theme.components, not in theme.token. Spreading antdTokens.components into the token object can silently break per-component theming.
Suggested fix
<ConfigProvider
theme={{
algorithm: antdTheme.defaultAlgorithm,
token: {
fontFamily: inter.style.fontFamily,
fontFamilyCode: inter.style.fontFamily,
...antdTokens.token,
- // eslint-disable-next-line `@typescript-eslint/no-explicit-any`
- ...((antdTokens as any).components ?? {}),
},
+ // eslint-disable-next-line `@typescript-eslint/no-explicit-any`
+ components: (antdTokens as any).components ?? {},
}}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| theme={{ | |
| algorithm: antdTheme.defaultAlgorithm, | |
| token: { | |
| fontFamily: inter.style.fontFamily, | |
| fontFamilyCode: inter.style.fontFamily, | |
| ...antdTokens.token, | |
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | |
| ...((antdTokens as any).components ?? {}), | |
| }, | |
| theme={{ | |
| algorithm: antdTheme.defaultAlgorithm, | |
| token: { | |
| fontFamily: inter.style.fontFamily, | |
| fontFamilyCode: inter.style.fontFamily, | |
| ...antdTokens.token, | |
| }, | |
| // eslint-disable-next-line `@typescript-eslint/no-explicit-any` | |
| components: (antdTokens as any).components ?? {}, | |
| }} |
| case "messages": { | ||
| opts.push({ | ||
| label: "Stringify", | ||
| target: "string", | ||
| apply: () => JSON.stringify(value), | ||
| }) | ||
| opts.push({ | ||
| label: "Convert to plain array", | ||
| target: "json-array", | ||
| apply: () => value, | ||
| warning: "Drops role/content semantics — chat rendering will stop", | ||
| }) |
There was a problem hiding this comment.
“Convert to plain array” is a no-op for messages.
This action returns the same array value, and the classifiers in this PR infer messages from that shape alone. On the next render the value is still treated as messages, so the conversion never actually sticks.
| if (typeof value === "string") { | ||
| if (value[0] === "{" || value[0] === "[") { | ||
| try { | ||
| const p = JSON.parse(value) | ||
| if (Array.isArray(p) || (p && typeof p === "object")) { | ||
| return {type: "string", hint: "stringified"} | ||
| } | ||
| } catch { | ||
| // not stringified JSON | ||
| } | ||
| } | ||
| if (value.length > 100 || value.includes("\n")) { | ||
| return {type: "string", hint: "markdown"} | ||
| } |
There was a problem hiding this comment.
Multiline outputs are flattened here.
classify() marks newline-containing strings as long-form/markdown, but this branch still renders them inside a plain <span>. HTML collapses newlines there, so multi-line responses become a single paragraph.
Also applies to: 173-175, 343-347
| const [editing, setEditing] = useState(false) | ||
| const [expanded, setExpanded] = useState(false) | ||
| const [draft, setDraft] = useState(value) | ||
| // Classify off the live draft so the chip variant + value styling react | ||
| // to type conversions (RFC WP-F1 round-trip — the chip IS the storage | ||
| // signal). On prop changes, useState above re-initializes draft. | ||
| const classified = useMemo(() => classifyKind(draft), [draft]) | ||
| // Editor-mode preference (short inline vs long-form Lexical). Initialized | ||
| // ONCE at mount: if the hydrated value is already long-form (>100 chars or | ||
| // contains newlines) we assume the user is dealing with markdown / a | ||
| // multi-paragraph string, so default to "long". After that, the user's | ||
| // explicit choice (chip popover) is the only thing that changes mode — | ||
| // typing past the threshold does NOT auto-flip the editor and break focus. | ||
| // Length isn't the signal at edit-time; it's just a sensible mount-time | ||
| // assumption. | ||
| const [forcedMode, setForcedMode] = useState<"short" | "long">(() => | ||
| typeof value === "string" && isLongFormString(value) ? "long" : "short", | ||
| ) |
There was a problem hiding this comment.
Row-local state survives prop changes.
draft and forcedMode are initialized from value once, but rows are keyed only by f.name. Switching to another testcase with the same field names will keep the previous row's edits/mode instead of reflecting the new props.
Also applies to: 582-589
| import {detectDataType, type DataType} from "@agenta/ui/drill-in" | ||
|
|
||
| /** | ||
| * Type primitive — what the value IS. Always one of these per chip | ||
| * emission. Mirrors JSON's primitive types. | ||
| */ | ||
| export type TypePrimitive = DataType // string|number|boolean|null|json-object|json-array |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the DataType definition in `@agenta/ui/drill-in`
find . -type f -name "coreTypes.ts" | grep "drill-in"Repository: Agenta-AI/agenta
Length of output: 111
🏁 Script executed:
# Also locate the TypeChip.tsx file
find . -type f -name "TypeChip.tsx"Repository: Agenta-AI/agenta
Length of output: 123
🏁 Script executed:
# Get the full content of TypeChip.tsx to verify the claims
cat -n web/apps/design-mockups/src/components/proposed/TypeChip.tsx | head -100Repository: Agenta-AI/agenta
Length of output: 4618
🏁 Script executed:
# Continue reading TypeChip.tsx
cat -n web/apps/design-mockups/src/components/proposed/TypeChip.tsx | sed -n '100,200p'Repository: Agenta-AI/agenta
Length of output: 4249
🏁 Script executed:
# Get the rest of the file
cat -n web/apps/design-mockups/src/components/proposed/TypeChip.tsx | tail -200Repository: Agenta-AI/agenta
Length of output: 8040
🏁 Script executed:
# Check the DataType definition
cat -n web/packages/agenta-ui/src/drill-in/coreTypes.ts | head -40Repository: Agenta-AI/agenta
Length of output: 1347
Decouple TypePrimitive from DataType to align the type contract with semantics.
Line 42 aliases TypePrimitive to DataType, which includes "messages" per web/packages/agenta-ui/src/drill-in/coreTypes.ts:23-30. However, the component's own documentation (lines 10–11) and inferVariant (lines 191–197) make clear that primitives never include "messages". This mismatch means the public type contract drifts from the component's actual behavior. Define TypePrimitive as an explicit union of the six actual primitives, then normalize detectDataType(value) output to that union so the type stays aligned with implementation.
Suggested fix
-import {detectDataType, type DataType} from "@agenta/ui/drill-in"
+import {detectDataType} from "@agenta/ui/drill-in"
@@
-export type TypePrimitive = DataType // string|number|boolean|null|json-object|json-array
+export type TypePrimitive =
+ | "string"
+ | "number"
+ | "boolean"
+ | "null"
+ | "json-object"
+ | "json-array"
@@
function inferVariant(value: unknown): TypePrimitive {
- if (value === null) return "null"
- if (Array.isArray(value)) return "json-array"
- if (typeof value === "object") return "json-object"
- if (typeof value === "boolean") return "boolean"
- if (typeof value === "number") return "number"
- return "string"
+ const detected = detectDataType(value)
+ return detected === "messages" ? "json-array" : detected
}Also applies to: 191–197
| <> | ||
| <BadgeKeyframes /> | ||
| <button | ||
| type="button" | ||
| onClick={onClick} | ||
| aria-label={ariaLabel ?? `Convert type from ${text}`} | ||
| title={badgeTooltip} | ||
| style={{ | ||
| ...baseStyle, | ||
| cursor: "pointer", | ||
| border: style.border ?? "1px solid transparent", | ||
| transition: "transform 0.08s ease, box-shadow 0.08s ease", | ||
| }} | ||
| onMouseEnter={(e) => { | ||
| e.currentTarget.style.transform = "translateY(-1px)" | ||
| e.currentTarget.style.boxShadow = | ||
| "0 1px 3px rgba(5, 23, 41, 0.12)" | ||
| }} | ||
| onMouseLeave={(e) => { | ||
| e.currentTarget.style.transform = "translateY(0)" | ||
| e.currentTarget.style.boxShadow = "none" | ||
| }} | ||
| > | ||
| {text} | ||
| {badgeNode} | ||
| </button> | ||
| </> | ||
| ) | ||
| } | ||
|
|
||
| return ( | ||
| <> | ||
| <BadgeKeyframes /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "TypeChip.tsx" | head -20Repository: Agenta-AI/agenta
Length of output: 123
🏁 Script executed:
fd -t f "TypeChip.tsx" --exec wc -l {} \;Repository: Agenta-AI/agenta
Length of output: 127
🏁 Script executed:
fd -t f "TypeChip.tsx" --exec cat -n {} \;Repository: Agenta-AI/agenta
Length of output: 15610
Move keyframe injection to a hook to eliminate render-phase side effects.
BadgeKeyframes performs DOM mutations (document.createElement, appendChild) during render, violating React's purity principle. In Strict Mode and concurrent rendering, this can cause side effects to execute on abandoned renders. Additionally, keyframes are injected unconditionally even when notificationBadge is false, wasting work.
Convert to a useBadgeKeyframes hook that injects the stylesheet only in useEffect, conditioned on notificationBadge:
Suggested fix
+import {useEffect} from "react"
import {detectDataType} from "@agenta/ui/drill-in"
@@
export function TypeChip({
@@
}: TypeChipProps) {
+ useBadgeKeyframes(Boolean(notificationBadge))
+
const resolved: ChipVariant =
variant ??
(value !== undefined ? inferVariant(value) : "string")
@@
return (
<>
- <BadgeKeyframes />
<button
@@
return (
<>
- <BadgeKeyframes />
<span style={baseStyle}>
{text}
{badgeNode}
</span>
</>
)
}
@@
-function BadgeKeyframes() {
- if (typeof document === "undefined") return null
- if (!document.getElementById("type-chip-badge-keyframes")) {
+function useBadgeKeyframes(enabled: boolean) {
+ useEffect(() => {
+ if (!enabled || typeof document === "undefined") return
+ if (document.getElementById("type-chip-badge-keyframes")) return
+
const style = document.createElement("style")
style.id = "type-chip-badge-keyframes"
style.textContent = `
`@keyframes` chipBadgePulse {
0% { box-shadow: 0 0 0 0 rgba(114, 46, 209, 0.6); }
70% { box-shadow: 0 0 0 5px rgba(114, 46, 209, 0); }
100% { box-shadow: 0 0 0 0 rgba(114, 46, 209, 0); }
}
`
document.head.appendChild(style)
- }
- return null
+ }, [enabled])
}| columnOptions, | ||
| onChange, | ||
| }: StubDrillInProps) { | ||
| const [data, setData] = useState<Record<string, unknown>>(initialData) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether StubDrillIn is reused with changing initialData (without remount key)
rg -n --type=tsx -C3 '\bStubDrillIn\b|initialData=' webRepository: Agenta-AI/agenta
Length of output: 88
🏁 Script executed:
#!/bin/bash
# Search for StubDrillIn component usage and initialData references
rg -n 'StubDrillIn' web --type ts --type tsx -C 3Repository: Agenta-AI/agenta
Length of output: 88
🏁 Script executed:
#!/bin/bash
# Also search for initialData usage more broadly
rg -n 'initialData' web --type ts --type tsx -C 2Repository: Agenta-AI/agenta
Length of output: 88
🏁 Script executed:
#!/bin/bash
# Read the actual StubDrillIn component file to understand full context
wc -l web/apps/design-mockups/src/components/StubDrillIn.tsxRepository: Agenta-AI/agenta
Length of output: 119
🏁 Script executed:
#!/bin/bash
# Read the complete StubDrillIn component
cat -n web/apps/design-mockups/src/components/StubDrillIn.tsxRepository: Agenta-AI/agenta
Length of output: 5634
🏁 Script executed:
#!/bin/bash
# Search for StubDrillIn usage across the web folder using glob pattern
rg 'StubDrillIn' web -g '*.tsx' -g '*.ts' -C 3Repository: Agenta-AI/agenta
Length of output: 6734
🏁 Script executed:
#!/bin/bash
# Search for initialData usage in tsx/ts files
rg 'initialData' web -g '*.tsx' -g '*.ts' -C 2Repository: Agenta-AI/agenta
Length of output: 20521
Add useEffect to sync initialData prop changes to local state.
Line 90 seeds local state once; if initialData prop changes (e.g., switching between fixtures in a shared parent), the component keeps stale data without resync. While current usages protect against this via strategic key changes or static props, the component should follow standard React patterns for dependent state.
Proposed fix
-import {useCallback, useMemo, useState} from "react"
+import {useCallback, useEffect, useMemo, useState} from "react"
@@
const [data, setData] = useState<Record<string, unknown>>(initialData)
+
+ useEffect(() => {
+ setData(initialData)
+ }, [initialData])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [data, setData] = useState<Record<string, unknown>>(initialData) | |
| import {useCallback, useEffect, useMemo, useState} from "react" | |
| const [data, setData] = useState<Record<string, unknown>>(initialData) | |
| useEffect(() => { | |
| setData(initialData) | |
| }, [initialData]) |
| const styles = { | ||
| section: { | ||
| background: "white", | ||
| border: "1px solid rgba(5, 23, 41, 0.08)", | ||
| borderRadius: 8, | ||
| padding: 16, | ||
| marginBottom: 16, | ||
| }, | ||
| sectionHeader: { | ||
| display: "flex", | ||
| alignItems: "center", | ||
| gap: 10, | ||
| marginBottom: 6, | ||
| }, | ||
| sectionTag: { | ||
| fontSize: 10, | ||
| fontWeight: 600, | ||
| padding: "2px 8px", | ||
| borderRadius: 4, | ||
| background: "#f6ffed", | ||
| color: "#389e0d", | ||
| textTransform: "uppercase" as const, | ||
| letterSpacing: "0.04em", | ||
| fontFamily: "ui-monospace, SFMono-Regular, Menlo, Consolas, monospace", | ||
| }, | ||
| sectionTitle: { | ||
| fontSize: 15, | ||
| fontWeight: 700, | ||
| margin: 0, | ||
| color: "#051729", | ||
| }, | ||
| sectionSummary: { | ||
| fontSize: 12, | ||
| color: "rgba(5, 23, 41, 0.65)", | ||
| lineHeight: 1.6, | ||
| margin: "0 0 12px", | ||
| }, | ||
| itemList: { | ||
| listStyle: "none", | ||
| margin: 0, | ||
| padding: 0, | ||
| display: "flex", | ||
| flexDirection: "column" as const, | ||
| gap: 10, | ||
| }, | ||
| item: { | ||
| display: "grid", | ||
| gridTemplateColumns: "180px 1fr", | ||
| gap: 12, | ||
| paddingTop: 8, | ||
| borderTop: "1px solid rgba(5, 23, 41, 0.06)", | ||
| }, | ||
| itemLabel: { | ||
| fontSize: 12, | ||
| fontWeight: 600, | ||
| color: "rgba(5, 23, 41, 0.85)", | ||
| }, | ||
| itemDetail: { | ||
| fontSize: 12, | ||
| color: "#051729", | ||
| lineHeight: 1.7, | ||
| }, | ||
| fileList: { | ||
| listStyle: "disc" as const, | ||
| paddingLeft: 18, | ||
| margin: "6px 0", | ||
| fontFamily: "ui-monospace, SFMono-Regular, Menlo, Consolas, monospace", | ||
| fontSize: 11, | ||
| lineHeight: 1.7, | ||
| }, | ||
| list: { | ||
| listStyle: "disc" as const, | ||
| paddingLeft: 18, | ||
| margin: "6px 0", | ||
| lineHeight: 1.7, | ||
| }, | ||
| pre: { | ||
| background: "#fafafa", | ||
| border: "1px solid rgba(5, 23, 41, 0.06)", | ||
| borderRadius: 4, | ||
| padding: "8px 10px", | ||
| fontSize: 11, | ||
| lineHeight: 1.6, | ||
| fontFamily: "ui-monospace, SFMono-Regular, Menlo, Consolas, monospace", | ||
| margin: "6px 0", | ||
| overflowX: "auto" as const, | ||
| }, | ||
| link: {color: "#1677ff", fontWeight: 500}, | ||
| crossLinks: { | ||
| marginTop: 16, | ||
| padding: "10px 14px", | ||
| background: "#fafafa", | ||
| border: "1px solid rgba(5, 23, 41, 0.06)", | ||
| borderRadius: 8, | ||
| fontSize: 12, | ||
| color: "#051729", | ||
| lineHeight: 1.8, | ||
| }, | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Use Tailwind classes instead of inline style objects for new frontend UI
This page’s styling is implemented as CSS-in-JS style objects. Please migrate to Tailwind utility classes to align with repo standards for new frontend code.
As per coding guidelines, Always prefer Tailwind utility classes over CSS-in-JS or separate CSS files for styling and Avoid CSS-in-JS solutions like react-jss and styled-components for new code; use Tailwind instead.
| <Link href="/solutions-drill-in" style={styles.cta}> | ||
| <span style={styles.ctaTag}>Solution</span> | ||
| <span style={styles.ctaTitle}> | ||
| Solutions · Drill-in — see [not authored] chip on | ||
| union-projected keys → | ||
| </span> | ||
| <span style={styles.ctaBlurb}> | ||
| The kitchen-sink testset's known-column union | ||
| includes literal <code>"geo.subregion"</code>{" "} | ||
| (Tuvalu authors it). Vanuatu's row doesn't, so a | ||
| muted <code>[not authored]</code> ghost row appears | ||
| at the bottom of the drill-in. Save-side filter is | ||
| conceptual (no save loop in the mockup). | ||
| </span> | ||
| </Link> | ||
| <Link href="/solutions-tables" style={styles.cta}> | ||
| <span style={styles.ctaTag}>Solution</span> | ||
| <span style={styles.ctaTitle}> | ||
| Solutions · Tables — see em-dash for missing keys → | ||
| </span> | ||
| <span style={styles.ctaBlurb}> | ||
| The kitchen-sink testset has 3 rows: Vanuatu authors | ||
| every column, Tuvalu and Kiribati each miss several. | ||
| Their cells render as <code>—</code> on the proposed | ||
| side (production renders the same). Distinct from | ||
| null (which has its own chip). | ||
| </span> | ||
| </Link> | ||
|
|
||
| <div style={styles.crossLinks}> | ||
| <strong>Related concept pages:</strong>{" "} | ||
| <Link href="/gap-01-type-chips" style={styles.link}> | ||
| gap-01 ([not authored] chip in vocabulary) | ||
| </Link>{" "} | ||
| ·{" "} | ||
| <Link href="/gap-03-drill-in-root-view" style={styles.link}> | ||
| gap-03 (auto-expand surfaces the union) | ||
| </Link>{" "} | ||
| ·{" "} | ||
| <Link href="/gap-05-dot-key-disambiguation" style={styles.link}> | ||
| gap-05 (dot-key collision compounds the issue) | ||
| </Link>{" "} | ||
| ·{" "} | ||
| <Link href="/gap-09-variable-provenance" style={styles.link}> | ||
| gap-09 (same shape applied at the execution-item surface) | ||
| </Link> | ||
| </div> | ||
| </MockupPageShell> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Migrate new page styling from inline style objects to Tailwind utility classes.
This page is implemented with inline styles for new UI. Please convert these to Tailwind classes to match project styling conventions and keep consistency with the rest of the frontend codebase.
As per coding guidelines, “Always prefer Tailwind utility classes over CSS-in-JS or separate CSS files for styling” and “Avoid CSS-in-JS solutions like react-jss and styled-components for new code; use Tailwind instead”.
Also applies to: 126-165
Railway Preview Environment
|
… solutions-drill-in
Solutions · Drill-in is now three stacked side-by-side comparisons (status quo
vs proposal) at production drawer width:
- Original proposal — ProposedDrillIn (chips + auto-expand + inline messages
+ dotted-key handling).
- New proposal — ProposalV2DrillIn: per-field "View as ▾" dropdown
(Text / Markdown / Chat / Form / JSON / YAML) replacing the old row of
icons; recursive form view with always-on vertical rail; production
ChatMessageList for chat fields; SharedEditor for strings/code with a
scoped white-gutter override.
- Schema-aware form alternative — kept as the third row for comparison.
Other supporting changes:
- ProdTestcaseDrawerSurface: production-faithful drawer chrome (close arrow,
nav, "Testcase N" title, Add to queue, Fields/JSON Segmented, footer).
- SideBySide: optional third column for ad-hoc 3-way comparisons.
- StubDrillIn: forwards showAddControls / showDeleteControls /
getDefaultValueForType so mockups can match the production testcase drawer.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (8)
web/apps/design-mockups/src/components/ProdTestcaseDrawerSurface.tsx (1)
57-60: 🏗️ Heavy liftReplace inline style objects with Tailwind utility classes for this new component.
This component is fully styled via inline objects (
style={styles.*}), which diverges from the frontend styling standard in this repo. Please migrate this surface to Tailwind classes (or a minimal class-based approach) so it stays consistent with the rest of the web app.Proposed direction (partial diff)
- return ( - <div style={styles.frame}> + return ( + <div className="w-full max-w-[736px] flex flex-col overflow-hidden rounded-lg border border-[rgba(5,23,41,0.12)] bg-white shadow-[0_6px_16px_0_rgba(0,0,0,0.08),0_3px_6px_-4px_rgba(0,0,0,0.12),0_9px_28px_8px_rgba(0,0,0,0.05)]"> ... - <header style={styles.header}> + <header className="flex shrink-0 items-center justify-between gap-3 border-b border-[rgba(5,23,41,0.06)] bg-white px-4 py-3"> ... - <div style={styles.body}>{children}</div> + <div className="min-h-[480px] flex-1 overflow-x-auto overflow-y-auto bg-white p-0">{children}</div> ... - <footer style={styles.footer}> + <footer className="flex shrink-0 justify-end gap-3 border-t border-[rgba(5,23,41,0.06)] bg-white px-6 py-3">As per coding guidelines,
Always prefer Tailwind utility classes over CSS-in-JS or separate CSS files for stylingandAvoid CSS-in-JS solutions like react-jss and styled-components for new code; use Tailwind instead.Also applies to: 90-96, 117-193
web/apps/design-mockups/src/components/proposed/ProposalV2FormView.tsx (1)
97-119: 💤 Low valueConsider memoizing
updateKeyto prevent unnecessary child re-renders.
updateKeyis defined inline and passed to eachFormFieldviaonChange. Each render creates a new function reference, potentially causing all child fields to re-render. While acceptable for a mockup, usinguseCallbackwould improve performance for larger forms.Proposed fix
+import {useCallback, useMemo, useState, type ReactNode} from "react" -import {useMemo, useState, type ReactNode} from "react" function ObjectRows({obj, depth, editable, onChange}: ObjectRowsProps) { const entries = Object.entries(obj) if (entries.length === 0) { return <span style={styles.emptyHint}>(empty object)</span> } - const updateKey = (key: string, next: unknown) => { - onChange({...obj, [key]: next}) - } + const updateKey = useCallback( + (key: string, next: unknown) => { + onChange({...obj, [key]: next}) + }, + [obj, onChange], + ) return (web/apps/design-mockups/src/components/StubDualView.tsx (1)
141-172: 💤 Low valueConsider using Tailwind utility classes instead of inline styles.
Per coding guidelines, Tailwind utilities are preferred over CSS-in-JS for styling. These are straightforward flexbox/spacing styles that translate directly to Tailwind classes. However, for a design-mockups app this is lower priority.
Example Tailwind conversion
return ( <div className="flex flex-col gap-3"> <header className="flex items-center gap-3 flex-wrap"> {/* Segmented controls */} {mode === "json" ? ( <div className="flex items-center gap-2 ml-auto"> {/* ... */} <span className="text-[11px] text-[rgba(5,23,41,0.55)] max-w-[280px] leading-relaxed"> {/* hint text */} </span> </div> ) : null} </header> <div className="bg-[`#fafafa`] border border-[rgba(5,23,41,0.08)] rounded-md p-3 min-h-[320px]"> {/* body content */} </div> </div> )As per coding guidelines: "Always prefer Tailwind utility classes over CSS-in-JS or separate CSS files for styling".
web/apps/design-mockups/src/components/SideBySide.tsx (1)
36-57: ⚡ Quick winUse Tailwind classes instead of inline style objects in this new TSX component.
This file is new frontend code and currently relies on
style={...}+ a localstylesobject.♻️ Refactor direction (example)
- <div style={hasThird ? styles.gridThree : styles.grid}> + <div className={hasThird ? "grid grid-cols-3 gap-4" : "grid grid-cols-2 gap-4"}> - <section style={styles.column}> + <section className="min-w-0 overflow-hidden rounded-lg border border-[rgba(5,23,41,0.08)] bg-[`#fafafa`] flex flex-col"> - <header style={styles.colHeader}> + <header className="flex items-center gap-2 border-b border-[rgba(5,23,41,0.08)] bg-white px-[14px] py-[10px]">As per coding guidelines, "Always prefer Tailwind utility classes over CSS-in-JS or separate CSS files for styling" and "Avoid CSS-in-JS solutions like react-jss and styled-components for new code; use Tailwind instead."
Also applies to: 64-125
web/apps/design-mockups/src/components/proposed/ProductionPlaygroundShell.tsx (1)
558-563: 💤 Low valueInitial format may drift from
inferredTypewheninput.formatis provided but invalid.When
input.formatis explicitly provided but doesn't exist inoptions(e.g., someone passesformat: "form"for a string type),safeFormatcorrectly falls back, but the underlyingformatstate remains out of sync. The next re-render will keep correcting it, but this creates a subtle inconsistency between the displayed dropdown value and the state.This is minor for a mockup component, but worth noting.
Proposed fix
function VariableInputRow({input}: {input: VariableInput}) { const inferredType = useMemo<VariableType>( () => input.type ?? inferType(input.value), [input.type, input.value], ) + const initialFormat = useMemo(() => { + const opts = getFormatOptions(inferredType) + const preferred = input.format ?? defaultFormatFor(inferredType) + return opts.includes(preferred) ? preferred : defaultFormatFor(inferredType) + }, [inferredType, input.format]) const [format, setFormat] = useState<VariableFormat>( - input.format ?? defaultFormatFor(inferredType), + initialFormat, )web/apps/design-mockups/src/components/proposed/PlaygroundVariableMap.tsx (1)
59-106: 💤 Low valueType inference logic is duplicated across multiple mockup components.
inferTypeChip()here mirrorsclassify()inPlaygroundExecutionItem.tsx,classifyVariant()inTreeDrillIn.tsx, and similar patterns elsewhere. For a mockup app this is acceptable, but if these components evolve toward production, consider extracting a sharedinferValueType()utility.web/apps/design-mockups/src/components/proposed/TreeDrillIn.tsx (1)
478-486: ⚡ Quick winState reset via ref comparison during render can cause render-phase side effects.
The pattern of comparing
dataKeytolastKeyRef.currentand callingsetDraft(data)during render works but is considered an anti-pattern in React. It triggers an additional render cycle and can lead to subtle timing issues. The recommended approach isuseEffectfor synchronizing state with props.Proposed fix using useEffect
export function TreeDrillIn({ data, rootTitle = "Testcase", detectDotKeyCollisions = false, autoExpand = true, editable = true, }: TreeDrillInProps) { const [draft, setDraft] = useState<Record<string, unknown>>(data) - // Reset draft when source data identity changes - const dataKey = useMemo(() => JSON.stringify(data), [data]) - const lastKeyRef = useRef(dataKey) - if (lastKeyRef.current !== dataKey) { - lastKeyRef.current = dataKey - setDraft(data) - } + // Reset draft when source data identity changes + useEffect(() => { + setDraft(data) + }, [data])If deep comparison is truly needed (to avoid resets when data is a new reference but same content), you can use a stringified key:
+ const dataKey = useMemo(() => JSON.stringify(data), [data]) + useEffect(() => { + setDraft(data) + }, [dataKey]) // eslint-disable-line react-hooks/exhaustive-depsweb/apps/design-mockups/src/components/proposed/TypeChip.tsx (1)
261-276: 🏗️ Heavy liftReplace new inline style/CSS-in-JS usage with Tailwind utility classes.
This new component is built around inline style objects for core rendering paths, which conflicts with the frontend styling standard and makes consistency/theming harder to enforce.
As per coding guidelines, "Always prefer Tailwind utility classes over CSS-in-JS or separate CSS files for styling" and "Avoid CSS-in-JS solutions like react-jss and styled-components for new code; use Tailwind instead".
Also applies to: 282-294, 310-315, 335-335
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4eb4c50f-0c9a-425a-be77-eb4379846a2c
📒 Files selected for processing (43)
web/apps/design-mockups/src/components/MockupPageShell.tsxweb/apps/design-mockups/src/components/ProdTestcaseDrawerSurface.tsxweb/apps/design-mockups/src/components/SideBySide.tsxweb/apps/design-mockups/src/components/StubDrillIn.tsxweb/apps/design-mockups/src/components/StubDualView.tsxweb/apps/design-mockups/src/components/StubMoleculeDrillIn.tsxweb/apps/design-mockups/src/components/proposed/ChipConversionPopover.tsxweb/apps/design-mockups/src/components/proposed/PlaygroundExecutionItem.tsxweb/apps/design-mockups/src/components/proposed/PlaygroundExecutionItemCompact.tsxweb/apps/design-mockups/src/components/proposed/PlaygroundVariableMap.tsxweb/apps/design-mockups/src/components/proposed/ProductionPlaygroundShell.tsxweb/apps/design-mockups/src/components/proposed/PromptConfigView.tsxweb/apps/design-mockups/src/components/proposed/ProposalV2DrillIn.tsxweb/apps/design-mockups/src/components/proposed/ProposalV2FormView.tsxweb/apps/design-mockups/src/components/proposed/ProposalV2ViewTypeSelect.tsxweb/apps/design-mockups/src/components/proposed/ProposedDrillIn.tsxweb/apps/design-mockups/src/components/proposed/ProposedTableCell.tsxweb/apps/design-mockups/src/components/proposed/RowDetailPopover.tsxweb/apps/design-mockups/src/components/proposed/SchemaForm.tsxweb/apps/design-mockups/src/components/proposed/TreeDrillIn.tsxweb/apps/design-mockups/src/components/proposed/TypeChip.tsxweb/apps/design-mockups/src/components/proposed/proposalV2Views.tsweb/apps/design-mockups/src/components/proposed/testsetTableHelpers.tsweb/apps/design-mockups/src/data/stubTestcases.tsweb/apps/design-mockups/src/pages/already-shipped.tsxweb/apps/design-mockups/src/pages/alt-tree-pane.tsxweb/apps/design-mockups/src/pages/gap-01-type-chips.tsxweb/apps/design-mockups/src/pages/gap-02-table-cells.tsxweb/apps/design-mockups/src/pages/gap-03-drill-in-root-view.tsxweb/apps/design-mockups/src/pages/gap-04-shape-preservation.tsxweb/apps/design-mockups/src/pages/gap-05-dot-key-disambiguation.tsxweb/apps/design-mockups/src/pages/gap-06-messages-renderer.tsxweb/apps/design-mockups/src/pages/gap-07-schema-aware-form.tsxweb/apps/design-mockups/src/pages/gap-08-playground-variable-validation.tsxweb/apps/design-mockups/src/pages/gap-09-variable-provenance.tsxweb/apps/design-mockups/src/pages/index.tsxweb/apps/design-mockups/src/pages/molecule-drill-in.tsxweb/apps/design-mockups/src/pages/solutions-drill-in.tsxweb/apps/design-mockups/src/pages/solutions-playground.tsxweb/apps/design-mockups/src/pages/solutions-tables.tsxweb/apps/design-mockups/src/styles/code-editor-styles.cssweb/apps/design-mockups/src/styles/globals.cssweb/apps/design-mockups/tsconfig.tsbuildinfo
✅ Files skipped from review due to trivial changes (3)
- web/apps/design-mockups/src/components/MockupPageShell.tsx
- web/apps/design-mockups/src/components/proposed/RowDetailPopover.tsx
- web/apps/design-mockups/src/components/StubMoleculeDrillIn.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- web/apps/design-mockups/src/components/proposed/PlaygroundExecutionItemCompact.tsx
- web/apps/design-mockups/src/components/proposed/SchemaForm.tsx
- web/apps/design-mockups/src/components/proposed/ProposedDrillIn.tsx
| <Button type="text" size="small" icon={<CaretDoubleRight size={14} />} /> | ||
| <div style={styles.navGroup}> | ||
| <Button type="text" size="small" icon={<CaretUp size={14} />} /> | ||
| <Button type="text" size="small" icon={<CaretDown size={14} />} /> |
There was a problem hiding this comment.
Add accessible labels to icon-only buttons.
At Line 61, Line 63, Line 64, Line 69, and Line 107, the icon-only buttons do not expose an accessible name. Add aria-label (and optional title) so screen reader users can identify each action.
Accessibility fix (localized diff)
-<Button type="text" size="small" icon={<CaretDoubleRight size={14} />} />
+<Button aria-label="Close drawer" title="Close drawer" type="text" size="small" icon={<CaretDoubleRight size={14} />} />
-<Button type="text" size="small" icon={<CaretUp size={14} />} />
+<Button aria-label="Previous testcase" title="Previous testcase" type="text" size="small" icon={<CaretUp size={14} />} />
-<Button type="text" size="small" icon={<CaretDown size={14} />} />
+<Button aria-label="Next testcase" title="Next testcase" type="text" size="small" icon={<CaretDown size={14} />} />
-<Button type="text" size="small" icon={<Copy size={14} />} />
+<Button aria-label="Copy testcase ID" title="Copy testcase ID" type="text" size="small" icon={<Copy size={14} />} />
-<Button type="primary" icon={<CaretUp size={14} />} disabled={!edited} />
+<Button aria-label="More apply actions" title="More apply actions" type="primary" icon={<CaretUp size={14} />} disabled={!edited} />Also applies to: 68-70, 101-108
| export function ProposalV2DrillIn({ | ||
| data: initialData, | ||
| editable = true, | ||
| onChange, | ||
| }: ProposalV2DrillInProps) { | ||
| const [data, setData] = useState<Record<string, unknown>>(initialData) |
There was a problem hiding this comment.
State won't sync when data prop changes after mount.
If the parent component changes the data prop (e.g., opening the drawer with a different testcase), the internal state will retain the previous value since useState only uses its initializer on first render.
For a reusable drawer, consider syncing with prop changes:
Proposed fix
+import {useCallback, useEffect, useMemo, useState} from "react"
-import {useCallback, useMemo, useState} from "react"
export function ProposalV2DrillIn({
data: initialData,
editable = true,
onChange,
}: ProposalV2DrillInProps) {
const [data, setData] = useState<Record<string, unknown>>(initialData)
+
+ useEffect(() => {
+ setData(initialData)
+ }, [initialData])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function ProposalV2DrillIn({ | |
| data: initialData, | |
| editable = true, | |
| onChange, | |
| }: ProposalV2DrillInProps) { | |
| const [data, setData] = useState<Record<string, unknown>>(initialData) | |
| import {useCallback, useEffect, useMemo, useState} from "react" | |
| export function ProposalV2DrillIn({ | |
| data: initialData, | |
| editable = true, | |
| onChange, | |
| }: ProposalV2DrillInProps) { | |
| const [data, setData] = useState<Record<string, unknown>>(initialData) | |
| useEffect(() => { | |
| setData(initialData) | |
| }, [initialData]) |
| if (headValue !== null && typeof headValue === "object" && !Array.isArray(headValue)) { | ||
| // The first segment is also an object key — collision possible. | ||
| collisions.add(literal) | ||
| // Also flag any expanded sub-column under that head. | ||
| for (const col of columns) { | ||
| if (col.parentKey === head) { | ||
| collisions.add(col.key) | ||
| } |
There was a problem hiding this comment.
Collision detection over-flags unrelated columns under the same head.
At Line 397–Line 400, once a literal dotted key is found, every expanded column with the same parentKey is marked as colliding. This produces false collision chips (e.g., literal geo.region can mark geo.coordinates.lat).
Suggested fix
export function detectCollisionColumns(rows: StubRow[], columns: ExpandedColumn[]): Set<string> {
const collisions = new Set<string>()
for (const row of rows) {
// Top-level keys with a dot in the name (literal-dotted).
const dotted = Object.keys(row.data).filter((k) => k.includes("."))
for (const literal of dotted) {
const head = literal.split(".")[0]
const headValue = row.data[head]
if (headValue !== null && typeof headValue === "object" && !Array.isArray(headValue)) {
- // The first segment is also an object key — collision possible.
- collisions.add(literal)
- // Also flag any expanded sub-column under that head.
+ // Flag the literal key, then only expanded keys that overlap
+ // with the same literal path (not every child under `head`).
+ collisions.add(literal)
for (const col of columns) {
- if (col.parentKey === head) {
+ if (
+ col.parentKey === head &&
+ (col.key === literal || col.key.startsWith(`${literal}.`))
+ ) {
collisions.add(col.key)
}
}
}
}
}
return collisions
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (headValue !== null && typeof headValue === "object" && !Array.isArray(headValue)) { | |
| // The first segment is also an object key — collision possible. | |
| collisions.add(literal) | |
| // Also flag any expanded sub-column under that head. | |
| for (const col of columns) { | |
| if (col.parentKey === head) { | |
| collisions.add(col.key) | |
| } | |
| if (headValue !== null && typeof headValue === "object" && !Array.isArray(headValue)) { | |
| // Flag the literal key, then only expanded keys that overlap | |
| // with the same literal path (not every child under `head`). | |
| collisions.add(literal) | |
| for (const col of columns) { | |
| if ( | |
| col.parentKey === head && | |
| (col.key === literal || col.key.startsWith(`${literal}.`)) | |
| ) { | |
| collisions.add(col.key) | |
| } | |
| } |
| const hasThird = third !== undefined | ||
| return ( |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify SideBySide callsites that may pass false/null to `third`
rg -nP --type=tsx -C2 'SideBySide|third=\{[^}]*&&[^}]*\}|third=\{null\}|third=\{false\}'Repository: Agenta-AI/agenta
Length of output: 88
🏁 Script executed:
#!/bin/bash
# First, examine the SideBySide component file
echo "=== SideBySide.tsx content ==="
head -80 web/apps/design-mockups/src/components/SideBySide.tsxRepository: Agenta-AI/agenta
Length of output: 2742
🏁 Script executed:
#!/bin/bash
# Search for usages of SideBySide component with proper ripgrep syntax
# Try without file type first to auto-detect
rg -n "SideBySide" web/apps/design-mockups/src --context=3 -A 5Repository: Agenta-AI/agenta
Length of output: 4182
🏁 Script executed:
#!/bin/bash
# Search for third prop usage patterns in the file
rg -n "third=" web/apps/design-mockups/src -A 2 -B 2Repository: Agenta-AI/agenta
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Get full context of SideBySide usage in alt-tree-pane.tsx
sed -n '125,165p' web/apps/design-mockups/src/pages/alt-tree-pane.tsxRepository: Agenta-AI/agenta
Length of output: 1807
Fix third-column presence check to exclude null and false values.
Line 34 checks third !== undefined, but callers using third={condition && <Component />} will pass false (not undefined) when the condition is falsy. This incorrectly activates the 3-column grid layout with an empty third column.
Proposed fix
- const hasThird = third !== undefined
+ const hasThird = third != null && third !== falseAlternatively, use a truthiness check:
- const hasThird = third !== undefined
+ const hasThird = !!third📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const hasThird = third !== undefined | |
| return ( | |
| const hasThird = third != null && third !== false | |
| return ( |
| <div style={styles.body}> | ||
| {mode === "fields" ? ( | ||
| <StubDrillIn | ||
| initialData={data} | ||
| rootTitle={rootTitle} | ||
| editable={editable} | ||
| showFieldDrillIn | ||
| showFieldCollapse | ||
| enableFieldViewModes | ||
| onChange={setData} | ||
| /> |
There was a problem hiding this comment.
Add key prop to StubDrillIn to force remount when switching modes.
When the user edits JSON and switches back to Fields mode, StubDrillIn receives updated data but won't reflect it due to the useState(initialData) pattern (no sync effect). Adding a key forces a remount to pick up the latest state.
Proposed fix
{mode === "fields" ? (
<StubDrillIn
+ key={JSON.stringify(data)}
initialData={data}
rootTitle={rootTitle}
editable={editable}Alternatively, a simpler key if the sync fix is applied to StubDrillIn:
- key={JSON.stringify(data)}
+ key={mode}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div style={styles.body}> | |
| {mode === "fields" ? ( | |
| <StubDrillIn | |
| initialData={data} | |
| rootTitle={rootTitle} | |
| editable={editable} | |
| showFieldDrillIn | |
| showFieldCollapse | |
| enableFieldViewModes | |
| onChange={setData} | |
| /> | |
| <div style={styles.body}> | |
| {mode === "fields" ? ( | |
| <StubDrillIn | |
| key={JSON.stringify(data)} | |
| initialData={data} | |
| rootTitle={rootTitle} | |
| editable={editable} | |
| showFieldDrillIn | |
| showFieldCollapse | |
| enableFieldViewModes | |
| onChange={setData} | |
| /> |
Summary
Testing
Verified locally
Added or updated tests
QA follow-up
Demo
Checklist
Contributor Resources