diff --git a/.claude/commands/analyze-chart-design.md b/.claude/commands/analyze-chart-design.md index 99d3da78f..6f30083ef 100644 --- a/.claude/commands/analyze-chart-design.md +++ b/.claude/commands/analyze-chart-design.md @@ -45,6 +45,11 @@ node scripts/ai/extract-structure.mjs ./tmp/ai/reference.svg ``` This writes `./tmp/ai/reference.structure.json`. +**Before computing `chartWidth`/`chartHeight` below, derive `referencePlotBounds` from +`reference.structure.json` using the gridline method in the "Derive plot area bounds" section +below.** The sizing calculation depends on `referencePlotBounds.width` and +`referencePlotBounds.height`, so it must be done first. + Also fetch Figma node metadata: `mcp__figma__get_figma_node` with the fileKey and nodeId. Extract: - **Frame name** → derive the story export name (PascalCase, no spaces) @@ -52,20 +57,51 @@ Extract: - **RSC Chart target dimensions** — determine which dimensions to pass to the RSC `Chart` component using this heuristic: - **If the node's direct children include a title text node, a legend instance, and a marks/chart - area as siblings** → the node itself IS the card. Use `frameWidth` × `frameHeight` directly. - The RSC `Chart` component renders title, legend, axes, and marks all within its given size, - so the outer frame maps 1:1. + **If the node is a padded card** (has explicit non-zero padding on all sides AND its direct + children include both a title group and a chart content group as siblings): + + **Important:** Do NOT use the content group dimensions directly. In Figma padded cards the + Y-axis labels/title live in the card's left padding and the X-axis lives in the bottom + padding — outside the content group. RSC allocates those same elements inside its `width` + and `height` budget. Using content group dimensions gives RSC less space than needed. + + Instead, use `referencePlotBounds` (derived from SVG gridlines below) as the anchor, then + add RSC axis/title overhead: + + ``` + chartWidth = referencePlotBounds.width + left_axis_overhead + right_margin_overhead + chartHeight = referencePlotBounds.height + title_overhead + bottom_axis_overhead + ``` + + **RSC overhead estimates — use these defaults unless the chart structure suggests otherwise:** + + | Component | Default estimate | When to adjust | + |---|---|---| + | Left Y-axis (labels + rotated title) | 80px | 65px for short labels (0–100); 90px for wide labels ("$300M") | + | Right margin | 20px | 30px if there is a right-side axis | + | Vega title overhead | 40px | 0px if no Title component | + | Bottom X-axis (labels + title) | 60px | 50px if no axis title; 70px if axis title is long | + + Record `frameWidth`/`frameHeight` from the outer node for reference only. + + **Rationale:** `referencePlotBounds` is derived from the precise pixel positions of horizontal + gridlines in the Figma SVG — it is the most reliable measurement of the actual marks area. + RSC includes axis and title overhead in its `width`/`height` budget, so those must be added + back on top of the Figma marks area. + + **If the node contains only marks and axes (no title/legend siblings, no card padding)** → + this is a bare chart area. Look up the nearest ancestor frame that has title/legend siblings + and apply the padded-card rule there, or use the node's own dimensions if no such ancestor + exists. - **If the node contains only marks and axes (no title/legend siblings)** → this is an inner - content area. Look up the nearest ancestor frame that has title/legend siblings and use its - dimensions instead. + **If the node has no explicit padding or `referencePlotBounds` cannot be computed** → + fall back to `frameWidth × frameHeight` and add an ambiguity note. - Record the chosen dimensions as `chartWidth` and `chartHeight` in the observation. Do not - subtract padding to derive inner content dimensions — that produces the wrong target size. + Record the chosen dimensions as `chartWidth` and `chartHeight` in the observation. -- **Title text node** → find the TEXT node for the chart title; fetch its text style: - `fontSize`, `fontWeight`, `fontFamily`. +- **Title text node** → find the TEXT node for the chart title; record its `fontSize`, + `fontWeight`, and `fontFamily`. This is required — `generate-chart-story` uses `titleFontSize` + to set the correct `fontSize` prop on the `Title` component. #### Derive plot area bounds from the SVG structure @@ -81,6 +117,29 @@ plotH = y-coordinate of bottommost line-horizontal gridline - plotY Coordinates are in the `reference.png` frame's coordinate space (scale=1, so 1:1 with SVG). +#### Derive data values from the SVG line path + +If `reference.structure.json` contains a `line-open` path (the data line), convert its +path-point y-coordinates to data values using the axis gridline positions as a ruler: + +``` +For each path point y-coordinate: + dataValue = axisMin + (axisMaxY - pointY) / (axisMaxY - axisMinY) * (axisMax - axisMin) +``` + +Where: +- `axisMinY` = y-coordinate of the bottom gridline (baseline, value = 0 or axis minimum) +- `axisMaxY` = y-coordinate of the top gridline (value = axis maximum) +- `axisMin` / `axisMax` = the corresponding data values from axis tick labels + +Map each path x-coordinate to a date/category using the x-axis tick positions. +Record the resulting data series in `dataPoints` in `design-observation.json`. This replaces +the qualitative `dataShapeHypothesis` for charts where SVG path data is available. + +**Important:** If the axis gridline pixel-spacing does not correspond to the value-spacing +(i.e. the scale is non-linear in the Figma design), note this explicitly and use the +closest linear interpolation. Do not silently invent data — flag non-linear scale as an ambiguity. + ### If input is a local file path Copy the file to `./tmp/ai/reference.png`. Note that SVG structure is not available — set diff --git a/.claude/commands/figma-example-story.md b/.claude/commands/figma-example-story.md index 490b7bb71..f3d054e36 100644 --- a/.claude/commands/figma-example-story.md +++ b/.claude/commands/figma-example-story.md @@ -17,6 +17,75 @@ All intermediate artifacts are written to `./tmp/ai/`. --- +## Phase 0 — Environment setup + +Run these steps once before Phase 1. They are prerequisites for Phase 3. + +### Read the Storybook port + +The port is set per-developer in `.env` as `STORYBOOK_AI_PORT`. This keeps multiple +worktrees from colliding — each developer assigns a unique port in their local `.env`. + +**Important:** Write the port file to `./tmp/storybook-port.txt` (NOT inside `./tmp/ai/`). +The `analyze-chart-design` skill deletes `./tmp/ai/` in its Step 0 — anything inside it +will be lost before Phase 3 runs. + +```bash +mkdir -p ./tmp +source .env 2>/dev/null || true +STORYBOOK_PORT=${STORYBOOK_AI_PORT:-6100} +echo $STORYBOOK_PORT > ./tmp/storybook-port.txt +echo "Storybook port: $STORYBOOK_PORT" +``` + +If `.env` is missing, fall back to 6100 and continue — do not abort. + +### Ensure Playwright and Chromium are available + +```bash +node -e "require('playwright')" 2>/dev/null || yarn add -DW playwright --ignore-engines +yarn playwright install chromium +``` + +`yarn playwright install chromium` is idempotent — exits immediately if already downloaded. + +### Start Storybook in the background + +Start the S2 Storybook server on the derived port and keep it running for the entire workflow. +The screenshot script attaches to it rather than starting/stopping its own server each iteration. + +**If Storybook is already running on the port, attach to it — do not start a second instance.** +Multiple instances on competing ports are the root cause of port drift between sessions. + +**Important:** Write the PID file to `./tmp/storybook-pid.txt` (NOT inside `./tmp/ai/`) for +the same reason as the port file. + +```bash +STORYBOOK_PORT=$(cat ./tmp/storybook-port.txt) +if curl -s http://localhost:$STORYBOOK_PORT > /dev/null 2>&1; then + echo "Storybook already running on port $STORYBOOK_PORT — attaching." +else + yarn storybook dev -p $STORYBOOK_PORT --config-dir .storybook-s2 --ci > ./tmp/ai/storybook.log 2>&1 & + echo $! > ./tmp/storybook-pid.txt + echo "Storybook PID: $!" + # Wait for it to be ready (poll every second, 120 s timeout) + for i in $(seq 1 120); do + curl -s http://localhost:$STORYBOOK_PORT > /dev/null 2>&1 && echo "Storybook ready on port $STORYBOOK_PORT" && break + sleep 1 + [ $i -eq 120 ] && echo "ERROR: Storybook failed to start. Check ./tmp/ai/storybook.log" && exit 1 + done + # Verify it actually started on the expected port (not auto-incremented) + ACTUAL_PORT=$(grep -o "localhost:[0-9]*" ./tmp/ai/storybook.log | head -1 | cut -d: -f2) + if [ "$ACTUAL_PORT" != "$STORYBOOK_PORT" ]; then + echo "ERROR: Storybook started on port $ACTUAL_PORT instead of $STORYBOOK_PORT." + echo "Another process may still be holding $STORYBOOK_PORT. Kill it and retry." + exit 1 + fi +fi +``` + +--- + ## Phase 1 — Analyze Invoke the `analyze-chart-design` skill with `$ARGUMENTS` (the Figma URL). @@ -48,7 +117,12 @@ Wait for it to complete and confirm that: 3. Read `./tmp/ai/gap-classification.json`. Apply the stop conditions below. -4. If continuing: apply all Category 1 (retryable) fixes directly to the story, then loop. +4. If continuing: apply all Category 1 (retryable) fixes directly to the story. Then: + - **Story/data changes only** (props, data values, axis config): Storybook HMR picks these + up automatically. Wait ~3 seconds before screenshotting. + - **Spec builder changes** (any edit to `vega-spec-builder-s2/` or `vega-spec-builder/`): + Run `yarn build:s2` before screenshotting — HMR does not rebuild library code. + Then loop. ### Stop conditions (check in order — stop on first match) @@ -63,12 +137,25 @@ Wait for it to complete and confirm that: ## Phase 4 — Final gap report +### Tear down Storybook + +Only kill the process if we started it (PID file exists and was written this session). +Do not kill a Storybook that was already running when Phase 0 attached to it. + +```bash +if [ -f ./tmp/storybook-pid.txt ]; then + kill $(cat ./tmp/storybook-pid.txt) 2>/dev/null && echo "Storybook stopped" || echo "Storybook already stopped" +fi +``` + Read the final `./tmp/ai/gap-classification.json` and `./tmp/ai/verification-report.json` and present: ``` ## Story: +**View in Storybook:** http://localhost:/?path=/story/ + ### What was implemented [bullet list of capturedElements from implementation-hypothesis.json] @@ -84,4 +171,7 @@ and present: One row per non-retryable discrepancy. Use em-dashes for empty cells. +The `` comes from `./tmp/storybook-port.txt`. The `` is the kebab-case story ID +used throughout Phase 3. + Present this report directly — do not spawn another agent for this step. diff --git a/.claude/commands/generate-chart-story.md b/.claude/commands/generate-chart-story.md index a4534219f..9cf3ec3b3 100644 --- a/.claude/commands/generate-chart-story.md +++ b/.claude/commands/generate-chart-story.md @@ -49,6 +49,23 @@ For each feature, record one of: `explicitlyNotCaptured` in the hypothesis instead of attempting them and letting `verify-chart-story` discover the gap. +**Decompose compound problems before classifying.** A design feature that cannot be matched +exactly may still have sub-properties that are independently controllable. Always assess each +dimension separately: + +- **Tick count vs tick values**: if the design shows N gridlines and the tick values cannot + match (e.g. non-linear scale), that does not mean the count cannot match. Always check + `tickCountLimit` on `Axis` as a first-class option. Set `tickCountLimit={N-1}` (excluding + the baseline) to approximate the visual density even when exact values differ. Classify + "tick count" and "tick values" as separate feasibility items. + +- **Axis position vs axis formatting**: the side an axis appears on is independent of its + label format. Assess each separately. + +- **Title text vs title style**: title text is always supported; font size (`fontSize` prop on + `Title`) may differ from the design default. Always read `titleFontSize` from + `design-observation.json` and apply it explicitly — do not rely on the RSC default. + --- ## Step 3 — Write `implementation-hypothesis.json` @@ -122,10 +139,11 @@ MyStoryName.args = { }; ``` -**Chart sizing:** Use `chartWidth` × `chartHeight` from `design-observation.json` (these are -the RSC `Chart` target dimensions determined during analysis — the full frame size when the -Figma node contains title/legend siblings). A numeric `width` bypasses ResizeObserver entirely -— no hover-shrink, no layout reflow. Do not use `minWidth`/`maxWidth`. +**Chart sizing:** Use `chartWidth` × `chartHeight` from `design-observation.json`. These are +computed by `analyze-chart-design` from `referencePlotBounds` (the Figma SVG gridline-derived +plot area) plus RSC axis/title overhead — not from the outer Figma frame or inner content group. +A numeric `width` bypasses ResizeObserver entirely — no hover-shrink, no layout reflow. Do not +use `minWidth`/`maxWidth`. ```ts const defaultChartProps: ChartProps = { @@ -160,6 +178,13 @@ minimum values needed to render the chart. Concretely: The goal is that a developer reading the story understands both what props to use *and* what their data might look like in practice. +**Curve smoothness near the origin.** For diminishing-returns or power-law curves where the +axis starts at zero, check whether the first data point should be `(0, 0)`. If the Figma-derived +first point has a very small non-zero value (e.g. spend < 1% of axis max) AND the slope from +that first point to the second is much steeper than subsequent slopes, monotone cubic interpolation +will generate an S-curve bump at the start. Replace the first point with `(dimensionField: 0, +metricField: 0)` to give the interpolation a clean zero-crossing and eliminate the bump. + **Do not work around gaps by changing chart semantics** (e.g. converting linear → categorical just to match specific tick labels). If the only path corrupts the data model, record it in `uncertainElements` instead. diff --git a/.claude/commands/verify-chart-story.md b/.claude/commands/verify-chart-story.md index 9f59551a9..580f1745f 100644 --- a/.claude/commands/verify-chart-story.md +++ b/.claude/commands/verify-chart-story.md @@ -18,63 +18,93 @@ Writes: ## Step 1 — Screenshot the story -Kill any stale Storybook process on port 6008 first: -```bash -lsof -ti:6008 | xargs kill -9 2>/dev/null -``` +Read `chartWidth` and `chartHeight` from `design-observation.json` and the port from +`./tmp/storybook-port.txt` (note: outside `./tmp/ai/` — that directory is wiped by the analyze +step), then screenshot: -Ensure `playwright` and its Chromium browser are available before proceeding: ```bash -node -e "require('playwright')" 2>/dev/null || yarn add -DW playwright --ignore-engines -yarn playwright install chromium +STORYBOOK_PORT=$(cat ./tmp/storybook-port.txt) +node scripts/ai/playwright-screenshot.mjs ./tmp/ai/result.png --port $STORYBOOK_PORT ``` -`yarn playwright install chromium` is idempotent — it exits immediately if Chromium is already downloaded. -Read `chartWidth` and `chartHeight` from `design-observation.json`, then screenshot: +The script attaches to the already-running Storybook server started in Phase 0, loads the story, +and captures the screenshot. It also writes: +- `./tmp/ai/plot-bounds.json` — the Vega plot area bounding box +- `./tmp/ai/result.svg` — the raw Vega `svg.marks` element HTML + +After the screenshot, extract structural data from the result SVG: + ```bash -node scripts/ai/playwright-screenshot.mjs ./tmp/ai/result.png +node scripts/ai/extract-structure.mjs ./tmp/ai/result.svg +# reads result.svg (unchanged), writes result.structure.json alongside it ``` -The script starts its own Storybook dev server on port 6008, takes the screenshot, and shuts down. -It also writes `./tmp/ai/plot-bounds.json` (the Vega plot area bounding box). - **If the screenshot fails** and the error suggests stale build output (e.g. component not rendering, `TypeError: X is not a function`, missing props on types), run the following **once** as a recovery step, then retry the screenshot: ```bash yarn build:parallel && yarn build:s2 ``` -Do not run this preemptively or for unrelated failures (wrong story ID, timeout, Storybook startup error). +Do not run this preemptively or for unrelated failures (wrong story ID, timeout). + +--- + +## Step 2 — Structural comparison + +**Read `./tmp/ai/reference.structure.json` and `./tmp/ai/result.structure.json` in the same +response.** These are the extracted SVG structures for the reference design and the RSC render. + +The two SVGs are produced by different tools (Figma vs Vega) so their schemas differ — do not +diff them mechanically. Instead, reason about what each element represents in context: + +| What to look for | Reference field | Result field | +|---|---|---| +| **Title font size** | `shape-curved` text paths near top or explicit text nodes with large fontSize | same area in result — compare computed font size | +| **Axis label font size** | text nodes near axis positions | same in result | +| **Gridline count** | `line-horizontal` array length | `line-horizontal` array length | +| **Gridline y-positions** | `start[1]` of each `line-horizontal` entry | same | +| **Data line shape** | `line-open` path `start` and `end` y-coordinates at each x-tick | same — compare relative vertical positions | +| **Mark colors** | `strokes` and `fills` arrays | same | +| **Axis tick count** | `line-vertical` array length | same | + +For the data line shape specifically: map the `line-open` path points to x-axis ticks using +x-coordinates, then compare the y-position (as a fraction of plot height) at each tick between +reference and result. A large fraction difference at a specific tick = data shape mismatch there. + +State what you find for each row. This structural pass catches things the pixel comparison misses: +exact font sizes, exact gridline counts, and per-tick data shape fidelity. --- -## Step 2 — Direct visual comparison +## Step 3 — Direct visual comparison **Read `./tmp/ai/reference.png` and `./tmp/ai/result.png` in the same response.** -Compare every visible property against the items listed in `implementation-hypothesis.json`: +Compare every visible property against the items listed in `implementation-hypothesis.json`. +Use the structural findings from Step 2 to anchor your observations — if the structure says +title font-size differs, confirm it visually here. | Property | What to check | |---|---| -| **Title** | Present? Text matches? Font size appropriate? | +| **Title** | Present? Text matches? Font size — use structural finding from Step 2 | +| **Title-to-chart spacing** | Gap between the bottom of the title text and the top of the plot/axis area — measure visually in pixels and compare | +| **Internal chart spacing** | Left margin (axis labels to plot edge), right margin, top padding above the first gridline, bottom padding below the x-axis — compare all four sides | | **Legend position** | Top / bottom / left / right? Labels visible, not truncated? | | **Axis label values** | Tick values match exactly? | | **Axis tick spacing** | Tick intervals match? | | **X-axis baseline/ticks** | Present? Match reference? | -| **Grid lines** | Count and orientation match? | +| **Grid lines** | Count and orientation match? Use structural count from Step 2 | | **Series count** | Same number of lines/bars/etc.? | -| **Curve shapes** | Do relative positions of all series match at multiple x-positions? | +| **Curve shapes** | Do peaks and valleys align at the same x-axis ticks? Use per-tick structural comparison from Step 2. **If the shape still doesn't match after the first retryable fix attempt, classify as a library/interpolation gap rather than continuing to tweak data.** | +| **Curve smoothness** | Is there a visible S-curve bump or inflection near the start of a diminishing-returns curve? This is a monotone interpolation artifact caused by a sharp slope change at the first data point. Check if the first point should be anchored to `(0, 0)` — Category 1 if so. If the bump is at an interior point, classify as Category 3. | | **Colors** | Accepted gap — S2 defaults vs design brand colors always differ. | | **Chart dimensions** | Overall aspect ratio match? | **For every item in `capturedElements` and `explicitlyNotCaptured` from the hypothesis, explicitly -state what you observe in each image before drawing any conclusion.** Do not infer a feature is -working from how the code is written — confirm it from the pixels. If an item in -`explicitlyNotCaptured` appears to work visually, re-examine carefully before reversing the -classification. +state what you observe before drawing any conclusion.** -State differences in plain language. This is the primary assessment — the pixel diff below is -a quantitative confirmation, not a substitute for direct observation. +State differences in plain language. The pixel diff below is a quantitative confirmation, not a +substitute for direct observation. --- diff --git a/.env.example b/.env.example new file mode 100644 index 000000000..791cf7947 --- /dev/null +++ b/.env.example @@ -0,0 +1,6 @@ +# Copy this file to .env (which is gitignored) and set values for your local environment. +# .env is never committed — each developer maintains their own copy. + +# Port for the AI workflow's Storybook instance (used by /figma-example-story). +# Set this to a unique port per worktree so concurrent runs never collide. +STORYBOOK_AI_PORT=6100 diff --git a/.gitignore b/.gitignore index 03a5e8505..a93693496 100644 --- a/.gitignore +++ b/.gitignore @@ -29,6 +29,9 @@ cursor .claude/commands/private .claude/settings.local.json +# Per-developer environment config (port assignments, etc.) +.env + # docusaurus .docusaurus diff --git a/CLAUDE.md b/CLAUDE.md index eb3ab2a4b..35b2d225f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -285,6 +285,23 @@ When adding a new page under `packages/docs/docs/spectrum2/`: When fixing a bug or refactoring behavior in an s1 package file, always check whether the corresponding s2 file needs the same change. The packages mirror each other structurally but s2 has no Venn support, no `s2` prop, and uses s2-specific imports (`vega-spec-builder-s2`, `react-spectrum-charts-s2`). A fix in one without the other leaves the packages inconsistent. +## Building After Spec Builder Changes + +Storybook's dev server rebuilds story files via HMR but does **not** rebuild library packages. +After modifying any file in `vega-spec-builder-s2/`, `vega-spec-builder/`, +`react-spectrum-charts-s2/`, or `react-spectrum-charts/`, run `yarn build:s2` before +screenshotting or verifying behavior in Storybook. Story-only changes (props, data values, +JSX structure) are picked up automatically after ~3 seconds without a rebuild. + +## S2 Spec Builder Defaults + +When a Vega configuration value differs from the S2 design spec (e.g. title offset, font +weight, font size, axis label spacing), encode it as a named default in `s2DefaultOptions` +inside the relevant `vega-spec-builder-s2` spec builder file. Do not expose it as a +user-facing React prop unless the S2 design spec intentionally allows per-chart variation. +Check `applyTitleOptionsDefaults` in `vega-spec-builder-s2/src/title/titleSpecBuilder.ts` +as the canonical pattern. + --- ## Before Implementing Any Feature or Bug Fix diff --git a/packages/react-spectrum-charts-s2/src/stories/Line/LineExamples.story.tsx b/packages/react-spectrum-charts-s2/src/stories/Line/LineExamples.story.tsx index 1c063f800..3cfb9775b 100644 --- a/packages/react-spectrum-charts-s2/src/stories/Line/LineExamples.story.tsx +++ b/packages/react-spectrum-charts-s2/src/stories/Line/LineExamples.story.tsx @@ -174,3 +174,47 @@ TotalVisitsL.args = { metric: 'percentChange', scaleType: 'time', }; + +// Paid media impact — single series showing how revenue grows as spend increases. +// Spend and revenue are stored as actual dollar amounts so shortCurrency formatting +// renders $100M, $200M, etc. Data points sampled from the Figma bezier path. +const paidMediaData = [ + { spend: 0, revenue: 0, series: 'Revenue' }, + { spend: 6_000_000, revenue: 68_000_000, series: 'Revenue' }, + { spend: 17_000_000, revenue: 137_000_000, series: 'Revenue' }, + { spend: 34_000_000, revenue: 211_000_000, series: 'Revenue' }, + { spend: 58_000_000, revenue: 285_000_000, series: 'Revenue' }, + { spend: 90_000_000, revenue: 358_000_000, series: 'Revenue' }, + { spend: 131_000_000, revenue: 425_000_000, series: 'Revenue' }, + { spend: 182_000_000, revenue: 483_000_000, series: 'Revenue' }, + { spend: 244_000_000, revenue: 528_000_000, series: 'Revenue' }, + { spend: 316_000_000, revenue: 558_000_000, series: 'Revenue' }, + { spend: 401_000_000, revenue: 568_000_000, series: 'Revenue' }, +]; + +const paidMediaChartProps: ChartProps = { + data: paidMediaData, + width: 615, + height: 370, +}; + +const PaidMediaImpactStory: StoryFn = (args): ReactElement => { + const props = useChartProps(paidMediaChartProps); + return ( + + + <Axis position="left" grid title="Revenue" range={[0, 600_000_000]} labels={[{ value: 0, label: '0' }, { value: 300_000_000, label: '$300M' }, { value: 600_000_000, label: '$600M' }]} /> + <Axis position="bottom" title="Spend" baseline ticks range={[0, 400_000_000]} tickCountLimit={4} tickMinStep={100_000_000} numberFormat="shortCurrency" /> + <Line {...args} /> + </Chart> + ); +}; + +export const PaidMediaImpact = bindWithProps(PaidMediaImpactStory); +PaidMediaImpact.args = { + color: 'series', + dimension: 'spend', + metric: 'revenue', + scaleType: 'linear', + interpolate: 'monotone', +}; diff --git a/packages/vega-spec-builder-s2/src/title/titleSpecBuilder.test.ts b/packages/vega-spec-builder-s2/src/title/titleSpecBuilder.test.ts index 36f999f0e..f58ddc04c 100644 --- a/packages/vega-spec-builder-s2/src/title/titleSpecBuilder.test.ts +++ b/packages/vega-spec-builder-s2/src/title/titleSpecBuilder.test.ts @@ -18,6 +18,7 @@ describe('applyTitleOptionsDefaults()', () => { expect(titleOptions).toHaveProperty('fontSize', 24); expect(titleOptions).toHaveProperty('position', 'start'); expect(titleOptions).toHaveProperty('orient', 'top'); + expect(titleOptions).toHaveProperty('offset', 44); }); test('should respect custom options', () => { diff --git a/packages/vega-spec-builder-s2/src/title/titleSpecBuilder.ts b/packages/vega-spec-builder-s2/src/title/titleSpecBuilder.ts index d5d05a7b9..066d4338c 100644 --- a/packages/vega-spec-builder-s2/src/title/titleSpecBuilder.ts +++ b/packages/vega-spec-builder-s2/src/title/titleSpecBuilder.ts @@ -24,12 +24,13 @@ const baseDefaults: Required<Pick<TitleOptions, TitleOptionsWithDefaults>> = { orient: 'top', }; -type S2OptionDefaults = 'fontWeight' | 'fontSize' | 'position'; +type S2OptionDefaults = 'fontWeight' | 'fontSize' | 'position' | 'offset'; const s2DefaultOptions: Required<Pick<TitleOptions, S2OptionDefaults>> = { fontWeight: 700, fontSize: 24, position: 'start', + offset: 44, }; export const applyTitleOptionsDefaults = (titleOptions: TitleOptions): TitleOptions => { @@ -41,7 +42,7 @@ export const applyTitleOptionsDefaults = (titleOptions: TitleOptions): TitleOpti }; export const addTitle = produce<ScSpec, [TitleOptions]>((spec, titleOptions) => { - const { text, fontWeight, fontSize, position, orient } = applyTitleOptionsDefaults(titleOptions); + const { text, fontWeight, fontSize, position, orient, offset } = applyTitleOptionsDefaults(titleOptions); spec.title = { text, @@ -50,6 +51,7 @@ export const addTitle = produce<ScSpec, [TitleOptions]>((spec, titleOptions) => anchor: position, frame: 'group', orient, + offset, }; return spec; diff --git a/packages/vega-spec-builder-s2/src/types/titleSpec.types.ts b/packages/vega-spec-builder-s2/src/types/titleSpec.types.ts index 0fa440602..509a757e7 100644 --- a/packages/vega-spec-builder-s2/src/types/titleSpec.types.ts +++ b/packages/vega-spec-builder-s2/src/types/titleSpec.types.ts @@ -25,4 +25,6 @@ export interface TitleOptions { fontSize?: number; /** The location of the title relative to the chart */ orient?: TitleOrient; + /** Pixel offset between the title and the chart plot area */ + offset?: number; } diff --git a/planning/ai/chart-sizing-from-figma.md b/planning/ai/chart-sizing-from-figma.md new file mode 100644 index 000000000..68ae29bbd --- /dev/null +++ b/planning/ai/chart-sizing-from-figma.md @@ -0,0 +1,115 @@ +# Issue: Chart sizing from Figma outer frame produces oversized RSC charts + +## Problem + +When the Figma node is a card-style component (title + chart content inside a padded frame), +`analyze-chart-design` reads the **outer frame dimensions** as `chartWidth`/`chartHeight` and +passes them directly to the RSC `Chart` component. This produces an RSC chart that is visually +much larger than the design. + +### Root cause + +Figma card components have decorative padding (e.g. 56px top / 64px bottom / 148px left / 96px +right) and a title text block that together consume a significant fraction of the outer frame. +RSC does not replicate this padding — the Vega engine fills all of `width × height` with the +chart visualization. As a result, the RSC plot area is far larger than the Figma plot area even +though both are nominally at the same outer dimensions. + +### Concrete example (Line chart L, 4615-28409) + +| | Figma | RSC at 752×480 | +|---|---|---| +| Outer frame | 752×480 | 752×480 | +| Card padding | 56 T / 64 B / 148 L / 96 R | none | +| Title area height | 66px | ~35px (Vega title) | +| Plot area dimensions | 506×244 px | 643×385 px | +| Plot area as % of frame | 67% W × 51% H | 85% W × 80% H | + +The RSC chart is approximately **2.4× larger** in area than the Figma chart content. + +--- + +## Fix 1: Use inner content group dimensions (implemented) + +The first fix (now in `analyze-chart-design`) changed from outer frame to inner content group: + +``` +chartWidth = chart content group width (508) +chartHeight = chart content group height + title group height (294 + 66 = 360) +``` + +This reduced RSC plot area to 399×265 — much closer but still not matching the 506×244 reference. + +--- + +## Fix 2 (known gap): Axis area lives in card padding, not content group + +### Problem + +The Figma Y-axis labels and axis title live in the **card padding** (the 148px left padding), +not inside the "chart content group." RSC allocates these elements within its `width` budget. + +As a result, setting `chartWidth = chart content group width (508)` leaves RSC with only +399px for the actual plot (85px consumed by Y-axis labels/title + 24px right margin). + +Similarly, the Figma bottom axis labels and title live in the 64px **bottom card padding**, +outside the content group. RSC allocates these within its `height` budget. + +### Measurements (4615-28409 at 508×360) + +| Element | Figma | RSC at 508×360 | +|---|---|---| +| Left axis area (Y labels + title) | 148px card padding | 85px from chart width | +| Right margin | 96px card padding | 24px from chart width | +| Plot width | 506px | 399px | +| Title area | 66px title box | 35px from chart height | +| Bottom axis area | 64px card padding | 60px from chart height | +| Plot height | 244px | 265px | + +### Correct heuristic + +Anchor sizing on `referencePlotBounds` (gridline-derived plot area in the SVG), then add +back estimated RSC axis/title overhead: + +``` +chartWidth = referencePlotBounds.width + RSC_left_axis_overhead + RSC_right_margin +chartHeight = referencePlotBounds.height + RSC_title_overhead + RSC_bottom_axis_overhead +``` + +**RSC overhead estimates:** + +| Component | Typical range | Notes | +|---|---|---| +| Left Y-axis (labels + title) | 65–95px | 65px for short labels (0–100), 85–95px for "$300M"-style | +| Right margin | 20–30px | minimal, usually 20px if no right axis | +| Vega title | 35–45px | compact; 0 if no title | +| Bottom X-axis (labels + title) | 50–70px | 50px labels only, +20px if axis title present | + +For the concrete case (4615-28409): +``` +chartWidth = 506 + 85 + 24 = 615px +chartHeight = 244 + 35 + 60 = 339px +``` + +### Why `referencePlotBounds` is the right anchor + +The `referencePlotBounds` is derived from horizontal gridline x/y coordinates in the SVG — it +is a precise pixel measurement of the actual marks area, independent of card padding or group +hierarchy ambiguities. It is more reliable than inferring dimensions from the Figma node tree. + +--- + +## Impact on existing stories + +Any story generated from a Figma padded-card node using inner content group dimensions will +have a plot area narrower than the design. The most visible symptom is axis labels crowding +the plot, or the overall chart appearing compressed horizontally relative to the design. + +## Acceptance criteria + +- [ ] `analyze-chart-design` detects padded card nodes and uses `referencePlotBounds` + overhead +- [ ] `design-observation.json` records `referencePlotBounds`, `chartWidth`, `chartHeight`, and + the RSC overhead estimates used +- [ ] The heuristic rationale in `analyze-chart-design.md` is updated to use this rule +- [ ] At least one round-trip test confirms the corrected size produces a plot area matching the + reference plot dimensions within ±10% diff --git a/planning/ai/interpolation-gap-detection.md b/planning/ai/interpolation-gap-detection.md new file mode 100644 index 000000000..9cc1105cf --- /dev/null +++ b/planning/ai/interpolation-gap-detection.md @@ -0,0 +1,62 @@ +# Feature: Interpolation gap detection in verify-chart-story + +## Problem + +When a line shape doesn't match the reference, the `verify-chart-story` skill currently +treats all shape mismatches as potentially fixable via data changes (Category 1). In practice, +many shape mismatches are caused by differences in curve interpolation algorithm — the +reference may use linear segments while RSC defaults to monotone cubic, or vice versa. + +These are not fixable by tweaking data values. They require either changing the `interpolate` +prop on the Line component (Category 1, if the right algorithm is available in RSC) or +accepting that RSC's interpolation options don't match the design (Category 2/3). Without +detecting this root cause, the agent wastes iterations on data tweaks that can never converge. + +--- + +## Proposed solution + +Add an interpolation detection step to `verify-chart-story` that distinguishes interpolation +mismatches from data mismatches before deciding whether to retry. + +### Detection heuristic + +After computing `shape-comparison.json` (see quantitative-shape-comparison feature), examine +the error distribution: + +- **Data mismatch signature**: errors are concentrated at specific x-ticks (e.g. high error + in Aug, low elsewhere) — the shape is wrong at specific points, not systematically. +- **Interpolation mismatch signature**: errors are distributed evenly across x-ticks, or the + result curve is consistently smoother/flatter than the reference — the shape is + systematically different everywhere. + +A standard deviation of per-tick errors that is low relative to the MAE (i.e. errors are +uniformly distributed) is a strong signal of an interpolation mismatch. + +### Decision logic + +1. If the shape MAE is above threshold AND the error distribution is uniform (stddev/MAE < 0.5): + - Check the current `interpolate` prop on the Line component in the story. + - If `interpolate` is `'monotone'` and the reference SVG shows straight segments between + points (linear), suggest switching to `interpolate: 'linear'` as a one-time Category 1 fix. + - If already `'linear'` or the suggestion was already tried: classify as Category 3 + ("interpolation algorithm mismatch — RSC curve type doesn't match design") and stop retrying. +2. If the error distribution is non-uniform (errors concentrated at specific ticks): treat as + a data shape issue and apply the normal retry logic. + +### Integration with verify-chart-story + +- Run interpolation detection after `shape-comparison.json` is available. +- Add `interpolationMismatch: true/false` to `verification-report.json`. +- When `interpolationMismatch` is true, the gap classification label should be + "Curve interpolation type" with category 1 (if a prop switch hasn't been tried) or + category 3 (if it has). + +## Acceptance criteria + +- [ ] `verification-report.json` includes `interpolationMismatch` boolean and `errorStddev` +- [ ] When interpolation mismatch is detected, the suggested action is to change the + `interpolate` prop before any data tweaking +- [ ] If the interpolate prop change was already tried and shape still doesn't match, + the gap is classified as Category 3 with no further retries +- [ ] Detection works for both line and area chart types diff --git a/planning/ai/quantitative-shape-comparison.md b/planning/ai/quantitative-shape-comparison.md new file mode 100644 index 000000000..8265bd90a --- /dev/null +++ b/planning/ai/quantitative-shape-comparison.md @@ -0,0 +1,62 @@ +# Feature: Quantitative shape comparison for verify-chart-story + +## Problem + +When a line/curve shape doesn't match the reference, the `verify-chart-story` skill has no +objective basis for deciding whether to keep iterating on data or stop and classify the gap. +The agent makes a subjective visual call ("close enough"), which has proven unreliable — it +repeatedly declares partial matches as good and continues tweaking data past the point of +diminishing returns. + +There is no stopping condition tied to actual fidelity. The loop cap of 3 iterations is the +only guard. + +--- + +## Proposed solution + +Extend `extract-structure.mjs` (or add a new `compare-shapes.mjs` script) to produce a +quantitative per-tick shape fidelity score. + +### Algorithm + +1. Extract `line-open` path y-coordinates from both the reference SVG and the result SVG. +2. Normalize both paths to the same coordinate space: x and y as fractions of their + respective plot bounding boxes (0–1 each), so different chart sizes don't inflate the + error. +3. Sample both normalized paths at N evenly-spaced x positions (N=20 recommended). +4. Compute the mean absolute error (MAE) between the two sampled y-value sequences. +5. Write results to `./tmp/ai/shape-comparison.json`: + +```json +{ + "mae": 0.08, + "samples": [ + { "x": 0.0, "referenceY": 0.12, "resultY": 0.15, "error": 0.03 }, + { "x": 0.05, "referenceY": 0.14, "resultY": 0.19, "error": 0.05 } + ], + "worstTick": { "x": 0.3, "error": 0.18, "xLabel": "Aug" } +} +``` + +### Thresholds (to be calibrated with real examples) + +| MAE | Classification | +|-----|---------------| +| < 0.05 | Shape match — color/style gaps only | +| 0.05–0.15 | Partial match — investigate specific ticks | +| > 0.15 | Poor shape match — classify as gap, stop data tweaking | + +### Integration with verify-chart-story + +- `verify-chart-story` reads `shape-comparison.json` after the pixel diff step. +- If MAE > 0.15 after a data fix has already been applied, classify the shape mismatch as + a non-retryable gap rather than looping again. +- Report the MAE and worst-tick in `verification-report.json`. + +## Acceptance criteria + +- [ ] `shape-comparison.json` is written on every verify run +- [ ] `verification-report.json` includes `shapeMae` and `worstShapeTick` +- [ ] The verify skill uses MAE > 0.15 as a hard stop for data-tweak retries +- [ ] The worst-tick label (e.g. "Aug") appears in the gap description when shape is poor diff --git a/planning/ai/spacing-measurement.md b/planning/ai/spacing-measurement.md new file mode 100644 index 000000000..fb8772669 --- /dev/null +++ b/planning/ai/spacing-measurement.md @@ -0,0 +1,84 @@ +# Feature: Spacing measurement in verify-chart-story + +## Problem + +The `verify-chart-story` visual comparison checklist covers series colors, tick values, +gridline count, curve shape, and legend position — but not layout spacing. Spacing differences +between the RSC output and the Figma design go unreported. + +In practice this means gaps like the following are never classified or surfaced: +- Gap between the title and the top of the plot area (title-to-chart spacing) +- Left margin between axis labels and the plot left edge +- Right margin between the plot right edge and axis labels +- Top padding above the first gridline +- Bottom padding below the x-axis tick labels + +These are real library gaps between RSC's default padding and the S2 design spec, and they +belong in the gap classification report. + +### Observed measurements (4615-28409, Line chart at 508×360) + +| Margin | Figma | RSC at 508×360 | Delta | +|---|---|---|---| +| Top (title-to-plot) | 66px | 35px | −31px | +| Left (axis labels to plot) | 0px (Y-axis is in card padding) | 85px | +85px | +| Right | 2px (within content group) | 24px | +22px | +| Bottom (plot to x-axis bottom) | 49px | 60px | +11px | + +The title-to-plot gap (−31px) is the most visible issue: RSC's Vega title takes ~35px while +the Figma title box allocates 66px. This cannot be fixed by resizing alone — it would require +a `titlePadding` or `titleOffset` prop on the RSC `Title` component to match the S2 spec. + +The left-axis margin discrepancy (+85px) is the root of the "chart width too narrow" problem: +the Y-axis area is in the Figma card padding but inside RSC's `width` budget. See +`chart-sizing-from-figma.md` for the corrected `chartWidth` heuristic. + +--- + +## Proposed solution + +Add a `measure-spacing.mjs` script that computes margin measurements from available bounding +box data and writes a diff. + +### Inputs + +- `./tmp/ai/plot-bounds.json` — Vega plot area bounding box (x, y, w, h) within the result screenshot +- `chartWidth`, `chartHeight` from `design-observation.json` — full result frame dimensions +- `referencePlotBounds` from `design-observation.json` — reference plot area bounding box +- `frameWidth`, `frameHeight` from `design-observation.json` — reference frame dimensions + +### Computed margins + +For both reference and result: +``` +topMargin = plotY +leftMargin = plotX +rightMargin = frameWidth - (plotX + plotWidth) +bottomMargin = frameHeight - (plotY + plotHeight) +``` + +Output written to `./tmp/ai/spacing-comparison.json`: + +```json +{ + "reference": { "top": 96, "left": 40, "right": 32, "bottom": 48 }, + "result": { "top": 56, "left": 17, "right": 35, "bottom": 50 }, + "deltas": { "top": -40, "left": -23, "right": 3, "bottom": 2 } +} +``` + +### Integration with verify-chart-story + +- Run `measure-spacing.mjs` after the screenshot step. +- Include a `spacing` section in `verification-report.json`. +- Any delta > 4px on any side surfaces as a discrepancy in `gap-classification.json`. +- Classify spacing gaps as Category 2 or 3 depending on whether they could be addressed + via a new prop (e.g. `titlePadding`) or require spec builder changes. + +## Acceptance criteria + +- [ ] `spacing-comparison.json` is written on every verify run +- [ ] `verification-report.json` includes a `spacing` section with all four margins and deltas +- [ ] Spacing deltas > 4px appear as discrepancies in `gap-classification.json` +- [ ] The checklist in Step 3 of `verify-chart-story` explicitly lists title-to-chart gap + and internal margins as required checks diff --git a/planning/features/axis-tick-values.md b/planning/features/axis-tick-values.md new file mode 100644 index 000000000..9bb1b26b9 --- /dev/null +++ b/planning/features/axis-tick-values.md @@ -0,0 +1,39 @@ +# Axis `tickValues` Prop (S2) + +## What it enables + +Explicit control over which tick values appear on an axis, overriding Vega's automatic tick generation. Without this, Vega picks tick positions algorithmically from the scale domain — useful in most cases, but it can't be relied on to produce clean round numbers when the data range doesn't align with the desired interval. For example, a scale that runs to 11.5M with `tickCountLimit={4}` might produce 0 / 3M / 6M / 9M instead of the intended 0 / 4M / 8M / 12M. + +`tickValues` lets you specify the exact values you want, making the axis predictable regardless of the data range. + +## Scope + +S2 only (`vega-spec-builder-s2`). The s1 package already has this prop; this change brings S2 to parity. + +## Files + +### `packages/vega-spec-builder-s2/src/types/axis/axisSpec.types.ts` + +Add `tickValues` to `AxisOptions`: + +```ts +/** + * Explicitly sets the tick values to display on the axis. + * When provided, overrides automatic tick generation entirely. + * Values should be in scale domain units (e.g. `[0, 4000000, 8000000, 12000000]`). + */ +tickValues?: number[]; +``` + +### `packages/vega-spec-builder-s2/src/axis/axisUtils.ts` + +In `getDefaultAxis`, destructure `tickValues` and wire it through to Vega. When `tickValues` is present, suppress `tickCount` — Vega ignores `values` if `tickCount` is also set: + +```ts +// destructure alongside existing fields +tickValues, + +// in the returned axis object +tickCount: tickValues ? undefined : getTickCount(position, tickCountMinimum, tickCountLimit, grid), +values: tickValues, +``` diff --git a/scripts/ai/playwright-screenshot.mjs b/scripts/ai/playwright-screenshot.mjs index f97cbc4c8..95e021ade 100644 --- a/scripts/ai/playwright-screenshot.mjs +++ b/scripts/ai/playwright-screenshot.mjs @@ -10,21 +10,23 @@ * governing permissions and limitations under the License. */ /** - * Starts a temporary Storybook instance, screenshots the chart area of a story, then shuts it down. + * Screenshots the chart area of a Storybook story. * * Usage: - * node scripts/ai/playwright-screenshot.mjs <story-id> [output-path] [width] [height] + * node scripts/ai/playwright-screenshot.mjs <story-id> [output-path] [width] [height] [--port <n>] * * Arguments: * story-id Storybook story ID. Derived from title + export name, e.g.: * title: 'React Spectrum Charts 2/Line/Examples' → react-spectrum-charts-2-line-examples * export: UserRetentionByCohort → user-retention-by-cohort * id: react-spectrum-charts-2-line-examples--user-retention-by-cohort - * output-path Where to write the PNG. Defaults to scripts/ai/tmp/imgCompare/result.png + * output-path Where to write the PNG. Defaults to tmp/ai/result.png * width Viewport width in px. Defaults to 800. * height Viewport height in px. Defaults to 700. - * - * Storybook is started on port 6008 using the S2 config (.storybook-s2) and shut down automatically. + * --port <n> Port Storybook is (or will be) running on. Defaults to 6008. + * If a server is already listening on this port the script attaches to it + * and does NOT start or stop Storybook. Otherwise it starts Storybook, + * takes the screenshot, and shuts it down. * * Outputs: * <output-path> — PNG screenshot clipped to the Vega chart SVG @@ -37,7 +39,6 @@ import { resolve, dirname } from 'path'; import { fileURLToPath } from 'url'; import { spawn } from 'child_process'; -const PORT = 6008; const STORYBOOK_CONFIG_DIR = '.storybook-s2'; const STORYBOOK_READY_POLL_MS = 500; const STORYBOOK_READY_TIMEOUT_MS = 120_000; @@ -51,10 +52,19 @@ const RENDER_TIMEOUT_MS = 10000; const __dirname = dirname(fileURLToPath(import.meta.url)); const repoRoot = resolve(__dirname, '../..'); -const [, , storyId, outputArg, widthArg, heightArg] = process.argv; +// Parse --port <n> out of argv before assigning positional args +const rawArgs = process.argv.slice(2); +const portFlagIdx = rawArgs.indexOf('--port'); +let PORT = 6008; +if (portFlagIdx !== -1 && rawArgs[portFlagIdx + 1]) { + PORT = parseInt(rawArgs[portFlagIdx + 1], 10); + rawArgs.splice(portFlagIdx, 2); +} + +const [storyId, outputArg, widthArg, heightArg] = rawArgs; if (!storyId) { - console.error('Usage: node playwright-screenshot.mjs <story-id> [output-path] [width] [height]'); + console.error('Usage: node playwright-screenshot.mjs <story-id> [output-path] [width] [height] [--port <n>]'); console.error(''); console.error('story-id example: react-spectrum-charts-2-line-examples--user-retention-by-cohort'); process.exit(1); @@ -70,50 +80,65 @@ const defaultOutputPath = resolve(process.cwd(), 'tmp/ai/result.png'); const outputPath = outputArg ? resolve(process.cwd(), outputArg) : defaultOutputPath; const outputDir = dirname(outputPath); const plotBoundsPath = resolve(outputDir, 'plot-bounds.json'); +const resultSvgPath = resolve(outputDir, 'result.svg'); mkdirSync(outputDir, { recursive: true }); -// --- Start Storybook --- +// --- Start Storybook (only if not already running) --- + +async function isStorybookRunning() { + try { + await fetch(`http://localhost:${PORT}`, { signal: AbortSignal.timeout(1000) }); + return true; + } catch { + return false; + } +} -/** - * Polls http://localhost:PORT until the server responds or the timeout elapses. - */ async function waitForStorybook() { const start = Date.now(); while (Date.now() - start < STORYBOOK_READY_TIMEOUT_MS) { - try { - await fetch(`http://localhost:${PORT}`); - return; // server is up - } catch { - await new Promise((r) => setTimeout(r, STORYBOOK_READY_POLL_MS)); - } + if (await isStorybookRunning()) return; + await new Promise((r) => setTimeout(r, STORYBOOK_READY_POLL_MS)); } throw new Error(`Storybook did not become ready on port ${PORT} within ${STORYBOOK_READY_TIMEOUT_MS / 1000}s`); } -console.error(`Starting Storybook on port ${PORT}...`); +const alreadyRunning = await isStorybookRunning(); +let storybookProcess = null; -const storybookProcess = spawn( - 'yarn', - ['storybook', 'dev', '-p', String(PORT), '--config-dir', STORYBOOK_CONFIG_DIR, '--ci'], - { - cwd: repoRoot, - env: { ...process.env, NODE_OPTIONS: '--openssl-legacy-provider' }, - stdio: ['ignore', 'ignore', 'pipe'], // suppress stdout/stdin; pipe stderr so errors surface if needed - } -); +if (alreadyRunning) { + console.error(`Storybook already running on port ${PORT} — attaching.`); +} else { + console.error(`Starting Storybook on port ${PORT}...`); + storybookProcess = spawn( + 'yarn', + ['storybook', 'dev', '-p', String(PORT), '--config-dir', STORYBOOK_CONFIG_DIR, '--ci'], + { + cwd: repoRoot, + env: { ...process.env, NODE_OPTIONS: '--openssl-legacy-provider' }, + stdio: ['ignore', 'ignore', 'pipe'], + } + ); + + storybookProcess.stderr.on('data', (chunk) => { + const line = chunk.toString(); + if (line.toLowerCase().includes('error')) { + process.stderr.write(`[storybook] ${line}`); + } + }); -storybookProcess.stderr.on('data', (chunk) => { - // Only forward lines that look like errors, not the normal startup noise - const line = chunk.toString(); - if (line.toLowerCase().includes('error')) { - process.stderr.write(`[storybook] ${line}`); + try { + await waitForStorybook(); + } catch (e) { + console.error(e.message); + storybookProcess.kill('SIGTERM'); + process.exit(1); } -}); +} -// Ensure storybook is killed when this process exits, even on error function shutdown() { - if (!storybookProcess.killed) { + if (storybookProcess && !storybookProcess.killed) { storybookProcess.kill('SIGTERM'); } } @@ -121,14 +146,6 @@ process.on('exit', shutdown); process.on('SIGINT', () => { shutdown(); process.exit(130); }); process.on('SIGTERM', () => { shutdown(); process.exit(143); }); -try { - await waitForStorybook(); -} catch (e) { - console.error(e.message); - shutdown(); - process.exit(1); -} - console.error(`Storybook ready at http://localhost:${PORT}`); // --- Launch browser --- @@ -217,6 +234,20 @@ if (plotBounds) { console.error('Warning: could not find plot background element — plot-bounds.json not written'); } +// --- Extract result SVG --- + +const resultSvgContent = await page.evaluate((svgSelector) => { + const svg = document.querySelector(svgSelector); + return svg ? svg.outerHTML : null; +}, VEGA_SELECTOR); + +if (resultSvgContent) { + writeFileSync(resultSvgPath, resultSvgContent); + console.error(`Result SVG: ${resultSvgPath}`); +} else { + console.error('Warning: could not extract result SVG — result.svg not written'); +} + // --- Cleanup --- await browser.close();