S2 vega-spec-builder Title offset and ai workflow tweaks

.claude/commands/analyze-chart-design.md

@@ -45,27 +45,63 @@ 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)
 - **Frame dimensions** → top-level node's `width` × `height` → `frameWidth`, `frameHeight`
 - **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

.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: <storyExportName>
 
+**View in Storybook:** http://localhost:<port>/?path=/story/<storyId>
+
 ### 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 `<port>` comes from `./tmp/storybook-port.txt`. The `<storyId>` is the kebab-case story ID
+used throughout Phase 3.
+
 Present this report directly — do not spawn another agent for this step.

.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.