[ritwik-g]

What

• Five v1 chart components on Recharts in frontend/src/v1/components/ui/chart/: LineChart, AreaChart, BarChart, StackedBarChart and Sparkline (P1-T17).
• They share one props model:
  • All five take data, xKey, xLabel, series[{ key, label, tone?, axis?, format? }], title, summary?, state, emptySlot?, emptyMessage?, height, width?, valueFormat and xFormat.
  • The four axis charts also take markers, legend, axisFormat, rightAxisFormat and grid.
• Each chart has:
  • a hover tooltip, and an arrow-key tooltip once the plot has focus;
  • a legend;
  • a loading skeleton and an empty state;
  • a text alternative: a summary sentence the plot points at via aria-describedby, plus a visually hidden data table.
• An ordinal ramp, --chart-1 to --chart-5, is added to the v1 tokens for every mode and mapped through @theme inline. Its values are the LLMWhisperer design's --mode-* colours.
• Usage docs live in components/ui/chart/README.md: props, examples, colour, the marker rule, error handling, the text alternative and testing tips. src/v1/README.md points to them.

Why

• UN-4401. These are the shared charts for P4-T04 Dashboard overview (lane I) and P7-T01 LLMWhisperer dashboard (lane D). The props API is typed and documented so those lanes can build against it now.

How

• Colours. P0-T04 shipped no chart tokens. --chart-1..5 are the LLMWhisperer design's --mode-native-text … --mode-table values, exactly as Figma defines them for each mode (figq tokens llmw; table below). palette.test.ts pins them to the design and checks lightest-to-darkest order. Per Ritwik's decision there is no contrast rule: an earlier 3:1-against-card rule was proposed by Claude, not the spec, and has been dropped. The README rules follow the designs:
  • The ramp is for stack segments, as in LLMW D01 "Pages by mode".
  • Separate series take a tone by meaning, as in DS D01 "Throughput and failures" (primary and destructive).
  • A single series should be given primary, or the tone for its meaning.
  • A chart has at most five untoned series, counting both stacks together.
  • Gaps the designs don't cover are listed below.
• Typed keys. Row can be any object type, including interfaces and API types with extra fields. xKey must name a string or number field. Series and marker keys must name a number | null | undefined field, so a numeric string (a DRF Decimal) is rejected at compile time. Rows with an index signature, such as pivoted dynamic series, accept any key whose field could hold the right type. @ts-expect-error guards pin each of these rules in typecheck:v1.
• Markers come from the series, never from ReferenceLine/ReferenceDot. I reproduced the defect against recharts 3.7.0: on a categorical axis with a repeated label, a reference element silently draws nothing.
  • A marker is { index, series, label }, addressed by row.
  • Its value and label travel in the row itself, in fields named by series key. A marker therefore stays paired with its point even in the frame before Recharts' store updates.
  • On a line or area, the marker is a dot-only series. On a bar, it is drawn from the bar itself, so it sits over its own bar in a group and on top of its own segment in a stack.
  • One rule decides what is drawn and what the summary reads out. Markers on an uncharted series, a row outside data, a gap, or an already-labelled point are dropped. So are all markers when a series key starts with the reserved __marker.
  • Each drop is logged with console.warn, but only once the chart shows data.
• Label placement. A marker label reads from whichever side (centred, rightward or leftward) leaves the least of it outside the plot, with ties going to centred. The plot bounds come from the first and last rows (line and area) or from Recharts' own usePlotArea() (bars).
• Accessibility. The plot uses Recharts' accessibility layer (role="application", focusable, arrow keys) and takes its name from title. The active point is read out through one polite, visually hidden live region (data-slot="chart-live"). ChartFrame mounts it with the chart, outside Recharts' tooltip wrapper, and the tooltip fills it via onAnnounce. The data table is hidden by an sr-only wrapper, not on the table itself. A sparkline without a tooltip is role="img" and stays out of the tab order. A point with no plotted neighbour is drawn as a dot, so a single row stays visible. The tooltip keeps gap rows and names the category, showing "–" for missing values. axe runs on every variant.
• Rendering. The rows passed to Recharts are data itself unless something is marked. They are memoised on what series and markers say, so inline literals do not reset Recharts' store. The tooltip is passed as an element.
• Errors. Charts have no error state. The README shows rendering InlineAlert with onRetry in place of the chart.

Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)

• No. Everything is new under src/v1, which nothing mounts yet. The only edits to existing files add five tokens and their @theme mapping, update the colour-family count assertion and the token-table snapshot, and add one sentence to the tokens.ts header.

Database Migrations

• None

Env Config

• None

Relevant Docs

• Spec: https://zipstack.atlassian.net/wiki/x/LwCZYQ
• Usage: frontend/src/v1/components/ui/chart/README.md

Related Issues or PRs

• UN-4401 (P1-T17). Builds on UN-4299 [FEAT] v1 design tokens, themes and style isolation (P0-T04) #2399 (P0-T04 tokens), UN-4329 [FEAT] Add the v1 unit-test harness and the frontend-v1-unit rig group #2429 (P0-T08 harness) and UN-4361 [FEAT] Add v1 toasts, inline alerts, progress and skeletons behind one notification entry point #2426 (Skeleton). Unblocks P4-T04 (lane I) and P7-T01 (lane D).

Dependencies Versions

• None added. Uses the existing recharts (^3.5.1, locked at 3.7.0). usePlotArea is public across that range.

Notes on Testing

• bun run typecheck:v1, bun run lint:v1 and the v1 Vitest project pass locally (565 tests, after merging main on 10 Nov). The chart suite also passes under LANG=de_DE, because the tests pin en-US.
• The chart tests cover:
  • per-variant colours, legend, name, summary and table, hover and keyboard tooltip, markers, loading and empty;
  • marker placement through real charts (grouped bars, stacked segments, gaps before a marked bar, two series on one row, a right axis);
  • label anchoring, renderer units, type-level guards and a markup snapshot.
• The self-review loop ran 22 rounds (rounds 11–22 were authorised by Ritwik) and mutation-verified each new guard. Round 22 found no High; its Mediums and Lows are follow-ups in row 32.
• After Deepak's review (10 Nov, go-ahead from Ritwik) main was merged in and rounds 23–26 reviewed the fixes in row 33. Each round's review is on this PR.

Gap analysis: spec vs this PR

Origin tags: [Original spec] the P1-T17 page · [Existing v0] current v0 behaviour · [Gap] something neither the spec nor the landed code covers · [Proposed by Claude] a default chosen here.

#	Point	Origin	Status
1	Line, Area, Bar, StackedBar, Sparkline on Recharts	Original spec	Done
2	Single-hue ordinal ramp from tokens	Original spec	Done: --chart-1..5, blue (the design's mode colours)
3	P0-T04 shipped no chart tokens	Gap	Added here, set to the LLMWhisperer design's --mode-* values for every mode
4	Ramp values	Original design (LLMW --mode-*)	Done: exactly the design values, pinned by test
5	"Legible in three themes" read as WCAG 1.4.11, 3:1 against card	Proposed by Claude	Dropped by Ritwik's decision; the design values are used as they are. Light/Unstract --chart-1 (2.1:1) and --chart-2, and dark --chart-5 (2.2:1), sit below 3:1 against card
6	Hue: blue, not brand violet	Proposed by Claude	Done
7	Semantic tone per series	Proposed by Claude (v0 draws failures red: Existing v0)	Done
8	Hover tooltip, legend, empty and loading states	Original spec	Done
9	Keyboard: arrow keys move the tooltip	Original spec	Done
10	Text alternative: both a summary sentence and a hidden table	Original spec; both chosen by Claude	Done
11	Visible "View as table" toggle	Proposed by Claude	Not built; can be added without breaking anything
12	Markers from the series, not a reference line	Original spec	Done; defect reproduced
13	Markers by row index; labels and values carried in rows	Proposed by Claude	Done
14	Second Y axis and rightAxisFormat	Proposed by Claude	Done
15	Sparkline: no axes or legend; tooltip opt-in	Proposed by Claude	Done
16	Loading reuses Skeleton	Proposed by Claude	Done (#2426)
17	Empty uses P1-T14 EmptyState	Gap: P1-T14 not on main	Pages pass it through emptySlot
18	Gallery entry: every variant and state in three modes	Original spec	Not done: P1-T01 gallery not on main. A markup snapshot and per-theme tests stand in for now
19	P1-T01 conventions (testId, ref forwarding)	Gap: P1-T01 not on main	Follows the landed v1 convention (data-slot)
20	Pie / Donut	Existing v0	Not built (not in spec); flag for lanes K, H, J
21	More than five untoned series	Gap	The ramp repeats with no warning; the README says to keep five per chart (both stacks together) and group the tail
22	Area fill is a flat tint, not D01's gradient	Proposed by Claude	Can be added later
23	Marker label is plain text, not D01's pill	Proposed by Claude	Can be added later
24	No error state; pages render InlineAlert	Proposed by Claude	Documented
25	Number formats follow the viewer's locale (numberFormats(locale))	Proposed by Claude	Done
26	Loading skeleton is the plot height only; a ready chart with a legend is about 28px taller (round 10 #76)	Gap	Done: loading and empty lay out the legend row invisibly
27	Categorical donuts (Human Review H29)	Gap (design)	Known gap for that page's owner: no Pie/Donut here, and the design's segment colours are not tokens
28	Free-choice "Compare on this chart" series (DS D12; P4-T04)	Gap (design)	Known gap for lane I: no designed colour for a series with no meaning
29	DS D01 KPI sparklines: LLM spend in warning, Documents in #5f8ad0 (not a token)	Gap (design)	Known gap; README rule is to use primary until the design names a token
31	Live-region fixes from round 21	Gap	Done in round 22: the region clears when the tooltip unmounts (#125), a gap is spoken as "no data" (#126), plus tests (#127, #128) and doc wording (#129–#132)
32	Live-region follow-ups from round 22 (recorded, not fixed, per Ritwik)	Gap	Follow-up: (a) a Sparkline tooltip true→false test that would fail without the cleanup, plus relabelling the loading test and the comment at ChartTooltip.tsx L88 (#133/#134); (b) README: an Escape dismissal keeps the last sentence (#135); (c) "Pages no data" may be announced for an active tooltip with an empty payload while Recharts keeps it hidden; whether that is reachable is unverified (#136); (d) each active-point change re-renders the whole chart (unverified cost)
33	Deepak's review (5822680995)	Review	Fixed, with tests that fail if reverted: (a) mixed-sign stacked bars use stackOffset="sign" (4510266843); (b) a negative bar's marker label reads under its tip, or beside the dot (on the side that leaves the plot less, within the plot's bottom) where there is no room under it (4510266848); (c) a ready chart whose every series value is a gap shows the empty state, in CartesianChart and Sparkline, with one gap rule (numberAt) shared by the tooltip, table, summary and markers, and a console warning naming any series whose values are not numbers (4510266955). Also pinned with tests: gaps break the line/area/sparkline, xFormat reaches the X ticks, className in every state, grid, the empty state's screen-reader title, the sparkline right axis's dataMin..dataMax domain, toHaveLength
34	Follow-ups from Deepak's review and rounds 23–26 (not done)	Gap	Follow-up: the per-theme test checks only the themed root (4510266870); Sonar nits S3358/S3776/S7755 (dropReason and categoryOf helpers, .at(-1), the range-sentence helper) and S6479/S7770 to be marked accepted in Sonar; the --swatch custom-property rule in v1/README S1 (4510266961); the S2 named-colour list in check-v1-styles.mjs (off-diff); the NumericKey JSDoc lists fewer gap readers than the README; the place type admits x without y; label width is a 7px-per-character estimate; a dev warning for a series key missing from every row; pin that the Sparkline unparsed-value warning logs once per change, and the CartesianChart one in the loading state; name the chart in that warning
30	An untoned series takes --chart-1 (the palest step) with no warning; the README's single-series rule is guidance, not a default	Gap	Known gap (recorded per Ritwik's stop rule). Proposed follow-up, not done: default a lone untoned series to primary, and add palette warnings (untoned outside a stack, more than five untoned, tones mixed with the ramp)

Ramp values (LLMWhisperer design --mode-*, from figq tokens llmw)

Step	Design token	Light	Dark	Unstract
chart-1	--mode-native-text	#86b6ef	#9ec5f4	#86b6ef
chart-2	--mode-low-cost	#5598e7	#6da7ec	#5598e7
chart-3	--mode-high-quality	#2a78d6	#3987e5	#2a78d6
chart-4	--mode-form	#1c5cab	#256abf	#1c5cab
chart-5	--mode-table	#104281	#184f95	#104281

Screenshots

• v1 isn't mounted and the P1-T01 gallery isn't on main yet. The markup for each variant is in components/ui/chart/__snapshots__/markup.md. Local harness renders per theme were shared with the Frontend v1 command session.

Checklist

I have read and understood the Contribution Guidelines.

🤖 Generated with Claude Code

[ritwik-g]

Self-review round 1 — tip 5306b7065 (review-deep)

Review chosen: /unstract:review-shallow was tried first and refused on its pattern gate (auth_authz_secrets from "token", attack_surface from a test regex's .exec(, onprem_rollout). Per the loop rules this round ran /unstract:review-deep. Mode: INITIAL. Base ed53cfded (fetched), 25 files / 2,969 lines, count verified against GitHub.

Verdict — REQUEST CHANGES

Summary — Critical: 0 · High: 0 · Medium: 9 · Low: 7 · Lenses run: 19/19 (11 N/A or not covered, with reasons, below)

Signals: lenses 16 and 17 raised (README and comments; colour-token count changed 30→35), plus the React lifecycle slice. Security deep-dive: done (0 findings, filtering done; scope excludes tests and docs). boundary-sweep: not triggered — the diff adds no outbound client, endpoint, log line or error path. rule-storage-check: 0 to verify, 3 dismissed (--chart-1, --chart-5, --page-aspect are CSS custom properties, not script flags). count-enum-check: not covered — the script could not resolve paths (realpath --no-symlinks failed on macOS).

Findings

1. [Medium] [Lens 3 — Correctness] — A marker on a grouped BarChart, or on a right-axis stack, is drawn at the category centre instead of over its bar

   • Location: frontend/src/v1/components/ui/chart/CartesianChart.tsx:L173-L185
   • Failure mode: markers are a Line, which plots at the centre of the band. Side-by-side bars are offset from that centre, so with 2 series the dot floats in the gap, and with 3 it sits on the wrong bar.
   • Evidence: in __snapshots__/markup.md the bar variant draws row 1's bars at x=148.8–173.8 and 177.8–202.8, while the marker is at cx=176.
   • Suggested fix: on bar kinds, draw the marker from the marked Bar itself (a LabelList content at the bar's x + width/2, y). Add position tests.
   • Confidence: High · Verification: VERIFIED — snapshot coordinates.

2. [Medium] [Lens 3/16 — Correctness, docs] — The summary announces markers the plot drops

   • Location: frontend/src/v1/components/ui/chart/describe.ts:L103
   • Failure mode: planMarkers drops a marker when its series is unknown, its index is not an integer, or the value is a gap. describeChart only checks the index range, so screen readers hear "Marked: …" for a dot nobody sees. This also contradicts the comments at markers.ts:L33 and types.ts:L42.
   • Suggested fix: one validity predicate shared by both, plus summary tests for the gap, fractional and unknown-series cases.
   • Confidence: High · Verification: VERIFIED — both filters read side by side (5 agents independently).

3. [Medium] [Lens 7 — Contract] — ChartDatum's index signature rejects interface rows and rows with non-chart fields

   • Location: frontend/src/v1/components/ui/chart/types.ts:L4-L6
   • Failure mode: consumers' API row types won't type-check (TS2322), which forces a cast that erases key checking. This is cheap to change now and breaking once consumers depend on it.
   • Suggested fix: Row extends object, and constrain only the keys that are read.
   • Confidence: High · Verification: VERIFIED — tsc probe.

4. [Medium] [Lens 7 — Contract] — series[].key and markers[].series accept non-numeric fields (even xKey)

   • Location: frontend/src/v1/components/ui/chart/types.ts:L22
   • Failure mode: key: "day" compiles. The series draws nothing, and the table and summary say "no values".
   • Suggested fix: a NumericKey<Row> mapped type.
   • Confidence: High · Verification: VERIFIED — tsc probe.

5. [Medium] [Lens 3 — Correctness] — Numeric strings are plotted by Recharts but treated as gaps by the tooltip, table, summary and markers

   • Location: frontend/src/v1/components/ui/chart/describe.ts:L35-L38
   • Failure mode: a DRF Decimal serialised as "12.5" draws a line, while every accessible channel says there is no data.
   • Suggested fix: restrict series values to number | null | undefined at the type level (folds into #4).
   • Confidence: Medium · Verification: UNVERIFIED — does a string-valued series plot? Render to confirm.

6. [Medium] [Lens 3/16 — Correctness, docs] — No error path: the README's canonical example turns a failed query into "No data for this period"

   • Location: frontend/src/v1/components/ui/chart/README.md:L25
   • Failure mode: state={isPending ? "loading" : "ready"}, with data=[] on error, renders as empty and offers no retry.
   • Suggested fix: the README says to render InlineAlert with onRetry instead of the chart on error.
   • Confidence: High · Verification: VERIFIED — reading.

7. [Medium] [Lens 3 — Correctness] — A lone point (one row, or a value between two gaps) is invisible in Line, Area and Sparkline

   • Location: frontend/src/v1/components/ui/chart/CartesianChart.tsx:L133-L135
   • Failure mode: d3 emits a zero-length M…Z, so nothing paints, while the summary says "1 point".
   • Suggested fix: draw a dot for any point with no defined neighbour.
   • Confidence: Medium · Verification: UNVERIFIED — does a one-row LineChart paint any mark?

8. [Medium] [Lens 13 — Testing] — The stacked marker "on top of its segment" is not pinned

   • Location: frontend/src/v1/components/ui/chart/describe.test.ts:L123-L136
   • Failure mode: the mutant .slice(0) survives (63/63), so a lower-segment marker could float to the top of the bar unnoticed.
   • Suggested fix: test a marker on a lower segment.
   • Confidence: High · Verification: VERIFIED — mutation run.

9. [Medium] [Lens 13 — Testing] — Right-axis series forming their own stack is untested

   • Location: frontend/src/v1/components/ui/chart/CartesianChart.tsx:L163
   • Failure mode: stackId: "stack" survives.
   • Suggested fix: render a right-axis stacked series and assert its baseline.
   • Confidence: Medium · Verification: VERIFIED that the mutant survives; UNVERIFIED whether Recharts renders it differently.

10. [Low] [Lens 2 — Lifecycle/perf] — data={[...rows]} gives Recharts a new array every render, so setChartData churns

    • Location: frontend/src/v1/components/ui/chart/CartesianChart.tsx:L189, Sparkline.tsx:L66
    • Suggested fix: memoise the plan and pass a stable reference.
    • Confidence: Medium · Verification: UNVERIFIED (render count). (Rated Low here: a wasted re-render, with no wrong output.)

11. [Low] [Lens 2] — Tooltip content is an inline arrow, which Recharts mounts as a new component type each render

    • Location: CartesianChart.tsx:L234-L246, Sparkline.tsx:L82-L104
    • Suggested fix: pass an element.

12. [Low] [Lens 3] — Duplicate React keys when labels repeat (legend, table header)

    • Location: frontend/src/v1/components/ui/chart/ChartFrame.tsx:L105, L143
    • Suggested fix: key by index.

13. [Low] [Lens 3] — The tooltip vanishes, heading included, on a category where every series is a gap

    • Location: frontend/src/v1/components/ui/chart/ChartTooltip.tsx:L46-L48
    • Suggested fix: keep the heading and show "–".

14. [Low] [Lens 7] — One axisFormat serves both Y axes

    • Location: frontend/src/v1/components/ui/chart/types.ts:L86-L87
    • Note: an additive gap, not a break.

15. [Low] [Lens 13] — Tooltip exclusion of marker series and the marker dot fill are untested

    • Location: frontend/src/v1/components/ui/chart/charts.test.tsx:L142-L172

16. [Low] [Lens 16/17 — Docs] — Stale comments

    • Locations:
      • tokens.json:L330 and palette.ts:L3 imply dark orders the ramp differently.
      • README.md:L5-L6 says every chart has a tooltip and a legend (Sparkline doesn't).
      • tokens.ts:L11 defines colour as "the Midnight Bloom collection minus its palette entries", which no longer holds now the chart ramp is in it. Made stale by: tokens.json:L285.

Lens checklist

1. Spec & intent — Clean
2. Architectural fit & precedent — See findings #10, #11
3. Correctness & edge cases — See findings #1, #2, #5, #6, #7, #12, #13
4. Security — Clean (security deep-dive done, 0 findings; assessed by orchestrator)
5. Data integrity & migrations — N/A — no persistence, schema or data migration
6. Concurrency — N/A — no concurrency primitives; synchronous rendering only
7. API & contract compatibility — See findings #3, #4, #14 (consumer search: no consumers outside the chart folder yet, orchestrator)
8. Reliability & resilience — N/A — no I/O, network or retries
9. Performance & cost — See findings #10
10. Observability — N/A — UI components with no logging or metrics surface
11. Operational safety — N/A — v1 is not mounted; no flags, infra or rollout config
12. LLM/agent-specific — N/A — no LLM or agent code
13. Testing — See findings #8, #9, #15
14. Dependencies & build — Clean (no dependency change; recharts 3.7.0 already in bun.lock; orchestrator)
15. Code quality — Clean (pinned Biome 2.3.13 check on changed files: clean; orchestrator)
16. Doc & comment accuracy — See findings #2, #6, #16
17. Cross-document rule consistency — See findings #16 (merge-result sweep ran, exit 1, hits classified; orchestrator)
18. On-prem deployment experience — N/A — frontend v1 components only; no chart, values, schema or on-prem build path
19. Scaling & workload lifecycle — N/A — no workloads or scaling config

Unanchored findings

• PR title UN-4401 [FEAT] … matches the workspace convention (CLAUDE.md "PR titles"), so there is nothing to flag.

Open questions for the author — none beyond the UNVERIFIED items above.

Assumptions made — consumers will pass API response row types directly (this drives #3/#4).

cost: agents=8 wall=04:13 lenses-examined=11 of 19 files=25 lines=2969

Unanchored findings

• frontend/src/v1/styles/tokens.ts:11 (outside the diff hunk): [Low] [Lens 16/17] — #16 Stale docs: this header no longer covers the chart ramp; also tokens.json chart-5 note and palette.ts:3 imply dark orders differently; README intro says every chart has tooltip+legend. Made stale by: tokens.json:L285.

[ritwik-g]

Self-review round 2 — tip 8ed588ab5 (review-deep, FOLLOWUP)

Review chosen: review-deep (lite's whole-PR gate fires the same pattern triggers as round 1). Delta under judgement: 5306b7065..8ed588ab5.

Verdict — REQUEST CHANGES

Prior context fetched — followup-fetch.sh exited 0 (6 of 6). Since boundary: 5306b7065.

Scope change — NO

Summary — Critical: 0 · High: 1 · Medium: 5 · Low: 7 · Lenses run: 19/19 (N/A ones with reasons below)

Signals: lenses 3, 7, 13 and 16 raised by the delta (marker placement, row and key types, new tests, docs). Security deep-dive: done (0 findings, after examination; filtering done). boundary-sweep: not triggered (no outbound client, endpoint, log line or error path). Pinned Biome 2.3.13 on the delta's files: clean.

Prior findings status

• #1 [Medium] Bar marker at category centre → RESOLVED in 8ed588a. It is drawn from the bar (marks.tsx:barMarker) and the position is tested in charts.test.tsx. A gap case remains, filed as new #17.
• #2 [Medium] Summary announces dropped markers → RESOLVED in 8ed588a. One rule (markers.ts:keptMarkers) now drives both; the mutant is killed.
• #3 [Medium] ChartDatum rejected interfaces → RESOLVED in 8ed588a (types.ts:8, ChartDatum = object). A residual edge with index-signature rows is filed as new #20.
• #4 [Medium] Series and marker keys accepted non-numeric fields → RESOLVED in 8ed588a (NumericKey, types.ts:22).
• #5 [Medium] Numeric strings → RESOLVED in 8ed588a. They are now rejected by type (NumericKey).
• #6 [Medium] No error path in the docs → RESOLVED in 8ed588a (README InlineAlert section; types.ts state doc).
• #7 [Medium] Lone point invisible → RESOLVED in 8ed588a (marks.tsx:isolatedDot, tested, neighbour mutants killed).
• #8 [Medium] Stacked marker segment not pinned → RESOLVED in 8ed588a. The marker comes from the segment's geometry, and the lower-segment test asserts it.
• #9 [Medium] Right-axis own stack untested → RESOLVED in 8ed588a. The no-overlap and baseline assertions kill the shared-stackId mutant.
• #10 [Low] New data array every render → PARTIALLY RESOLVED in 8ed588a. The rows are memoised, but on dependencies that inline series/markers literals (the README's own example) change on every render, and the spread discards identity. Residual tracked as new #18.
• #11 [Low] Inline tooltip content remounts → RESOLVED in 8ed588a. It is now an element, which Recharts clones.
• #12 [Low] Label keys → RESOLVED in 8ed588a (series key and index).
• #13 [Low] Tooltip vanishes on an all-gap category → RESOLVED in 8ed588a (filterNull={false}, "–"; tested for CartesianChart). Sparkline is untested, new #29.
• #14 [Low] One axis format → RESOLVED in 8ed588a (rightAxisFormat, tested).
• #15 [Low] Tooltip marker exclusion and marker fill untested → RESOLVED in 8ed588a (both tested; mutants killed).
• #16 [Low] Stale docs → RESOLVED in 8ed588a (tokens.json, palette.ts, README intro, tokens.ts header).

New findings

17. [High] [Lens 3] — On a non-stacked BarChart, a gap before a marked row shifts its marker onto a later bar, or drops it from the plot while the summary still reads it
    • Location: frontend/src/v1/components/ui/chart/marks.tsx:L66-L91
    • Failure mode: Recharts' LabelList passes index as the position among the drawn rectangles (Bar.js filters out null bars), not the data row. So [10, null, 30, 40] with a marker on row 2 draws it over row 3, and a marker on the last row is never drawn.
    • Suggested fix: resolve the row from the entry's payload (or carry a row-index field), not the position. Add a gap test.
    • Confidence: High · Verification: VERIFIED — two independent source traces of recharts 3.7.0 Bar.js/LabelList.js plus a d3 scale probe (null maps to undefined).
18. [Medium] [Lens 2/9] — The rows memo loses identity in the documented usage
    • Location: frontend/src/v1/components/ui/chart/CartesianChart.tsx:L75-L84
    • Failure mode: the memo depends on series/markers (inline literals in the README) and always spreads, so Recharts gets a new array every render. The bar path doesn't even read series. The same spread is in Sparkline.tsx:64.
    • Suggested fix: pass data through when nothing is marked; memoise on key and index content; document the stable-reference expectation.
    • Confidence: High · Verification: VERIFIED by reading the deps against the README.
19. [Medium] [Lens 7] — An inline data={[]} infers Row = never, rejecting xKey and every series key with an error that never mentions data
    • Location: frontend/src/v1/components/ui/chart/types.ts:L71-L73
    • Suggested fix: infer Row from data only (NoInfer on keys) and document data={[] as Row[]}.
    • Verification: VERIFIED — tsc probe.
20. [Medium] [Lens 7] — Rows typed Record<string, string | number> (pivoted, dynamic series) make NumericKey never
    • Location: frontend/src/v1/components/ui/chart/types.ts:L22-L25
    • Suggested fix: fall back to string keys when Row has a string index signature.
    • Verification: VERIFIED — tsc probe.
21. [Medium] [Lens 13] — Number-format expectations depend on the runtime locale
    • Location: frontend/src/v1/components/ui/chart/describe.ts:L14-L18
    • Failure mode: Intl.NumberFormat(undefined) with tests hard-coding "2,981" fails on a de_DE runner.
    • Suggested fix: pin the locale in the tests, or derive the expectations from the formatter.
    • Confidence: High · Verification: UNVERIFIED — does LANG=de_DE.UTF-8 fail the suite?
22. [Medium] [Lens 13] — The compile-time key constraints have no guard
    • Location: frontend/src/v1/components/ui/chart/describe.test.ts:L117-L125
    • Failure mode: loosening NumericKey to accept strings passes both tsc and vitest (mutation run). The old @ts-expect-error went with the rework.
    • Suggested fix: @ts-expect-error cases for a numeric-string key and an unknown key, plus a positive interface-row case.
    • Verification: VERIFIED — mutation.
23. [Low] [Lens 3] — keptMarkers drops invalid markers with no dev signal
    • Location: frontend/src/v1/components/ui/chart/markers.ts:L39-L49
    • Suggested fix: a dev-only console.warn, as lib/pdf/highlights.ts does.
24. [Low] [Lens 16] — keptLabels says "series then row order" but follows the order of the markers prop
    • Location: frontend/src/v1/components/ui/chart/markers.ts:L57-L62
    • Suggested fix: sort by series index.
    • Verification: VERIFIED — run.
25. [Low] [Lens 2] — After a data change, the first frame can place an isolated dot or bar marker from the new rows at the old point's position
    • Location: frontend/src/v1/components/ui/chart/marks.tsx:L99-L127
    • Verification: UNVERIFIED — does the first frame show stale positions?
26. [Low] [Lens 2] — barMarker is a fresh component type each render (Label uses createElement)
    • Location: frontend/src/v1/components/ui/chart/CartesianChart.tsx:L160
27. [Low] [Lens 7] — Key errors name the alias (NumericKey<Api>), not the allowed keys
    • Location: frontend/src/v1/components/ui/chart/types.ts:L11-L25
    • Note: the doc comments surface on hover; accepted as is.
28. [Low] [Lens 7] — CategoryKey/NumericKey not exported, so generic wrappers can't name them
    • Location: frontend/src/v1/components/ui/chart/index.ts:L14-L22
29. [Low] [Lens 13] — Sparkline's filterNull={false} is untested
    • Location: frontend/src/v1/components/ui/chart/Sparkline.tsx:L86

Lens checklist

1. Spec & intent — Clean
2. Architectural fit & precedent — See findings #18, #25, #26
3. Correctness & edge cases — See findings #17, #23
4. Security — Clean (security deep-dive done, 0 findings; orchestrator)
5. Data integrity & migrations — N/A — no persistence
6. Concurrency — N/A — synchronous rendering only
7. API & contract compatibility — See findings #19, #20, #27, #28 (consumer search: still no consumers outside the chart folder; orchestrator)
8. Reliability & resilience — N/A — no I/O
9. Performance & cost — See findings #18
10. Observability — N/A — UI components, no logging or metrics surface
11. Operational safety — N/A — v1 not mounted; no flags or infra
12. LLM/agent-specific — N/A — no LLM code
13. Testing — See findings #21, #22, #29
14. Dependencies & build — Clean (no dependency change; orchestrator)
15. Code quality — Clean (pinned Biome on the delta: clean; orchestrator)
16. Doc & comment accuracy — See findings #24
17. Cross-document rule consistency — Clean (merge-result sweep exit 1, all hits consistent; stale terms planMarkers/"running total"/"finite number is a gap" gone; orchestrator)
18. On-prem deployment experience — N/A — frontend v1 components only
19. Scaling & workload lifecycle — N/A — no workloads

Open questions / assumptions — consumers may build pivoted dynamic-key rows (drives #20).

cost: agents=8 wall=05:28 lenses-examined=11 of 19 files=27 lines=3400

[ritwik-g]

Self-review round 3 — tip 4965a6ed8 (review-deep, FOLLOWUP)

Delta under judgement: 8ed588ab5..4965a6ed8.

Verdict — REQUEST CHANGES

Prior context fetched — followup-fetch.sh exited 0. Since boundary: 8ed588ab5.

Scope change — NO

Summary — Critical: 0 · High: 0 · Medium: 6 · Low: 3 · Lenses run: 19/19 (N/A ones with reasons below)

Security deep-dive: done (0 findings, after examination; no candidates, so nothing to filter). boundary-sweep: 1 sink traced (the new console.warn in markers.ts), clean — it carries a marker label and a row index only. Pinned Biome on the delta: clean.

Prior findings status

• #10 (round 1) → RESOLVED in 4965a6e. Rows are data itself unless something is marked, and are memoised on series/marker content.
• #17 [High] Bar marker drifts after a gap → RESOLVED in 4965a6e. Each bar row carries its own row index, and the gap test would fail on the old code.
• #18 [Medium] Memo identity → RESOLVED in 4965a6e. Untested; see new #34.
• #19 [Medium] data={[]} infers never → PARTIALLY RESOLVED. The new index-signature fallback makes CategoryKey<never> = string, so an empty literal now accepts any key. The doc's "rejects every key" is false, see #33.
• #20 [Medium] Record rows → PARTIALLY RESOLVED. The fallback ignores the index's value type, so Record<string, string> is accepted, see #32.
• #21 [Medium] Locale-dependent tests → RESOLVED in 4965a6e. en-US is pinned and the suite passes under LANG=de_DE. The cost to default-formatter coverage is tracked as #35.
• #22 [Medium] Key constraints unguarded → RESOLVED in 4965a6e. The @ts-expect-error guards are mutation-checked with tsc. The CategoryKey branch is still unguarded, see #37.
• #23 [Low] Dropped markers silent → RESOLVED in 4965a6e. They now warn, but see #30 and #31.
• #24 [Low] keptLabels order → RESOLVED in 4965a6e (sorted by series, tested).
• #25 [Low] Stale first frame → PARTIALLY RESOLVED. isolatedDot now reads points, but marker labels still come from the closure, see #36.
• #26 [Low] barMarker component churn → RESOLVED in 4965a6e (it is passed as an element).
• #27 [Low] Key errors name the alias → WAIVED in round 2 by this reviewer (doc comments show on hover).
• #28 [Low] Key types not exported → RESOLVED in 4965a6e.
• #29 [Low] Sparkline filterNull untested → RESOLVED in 4965a6e (tested; mutant killed).

New findings

30. [Medium] [Lens 3] — The drop warning fires during loading and empty states
    • Location: frontend/src/v1/components/ui/chart/markers.ts:L41-L53
    • Failure mode: keptMarkers runs before the state check, so state="loading" with data=[] warns for every marker. It repeats on every new [], and twice under StrictMode, because the side effect sits inside useMemo. The README example triggers it.
    • Suggested fix: warn only when ready, from an effect keyed on the marker spec.
    • Verification: VERIFIED by reading.
31. [Medium] [Lens 3] — A second marker on the same series and row silently replaces the first
    • Location: frontend/src/v1/components/ui/chart/markers.ts:L55-L57
    • Failure mode: the first marker is neither drawn nor read out, and nothing is logged. This contradicts the docstring's warn-on-every-drop promise.
32. [Medium] [Lens 7] — The NumericKey index-signature fallback accepts Record<string, string>
    • Location: frontend/src/v1/components/ui/chart/types.ts:L30-L35
    • Failure mode: this reopens the numeric-string hole the type documents as closed.
    • Suggested fix: make the fallback depend on the index value type (number extends Row[string & keyof Row]).
    • Verification: VERIFIED — tsc probe.
33. [Medium] [Lens 16] — The docs say data={[]} rejects every key; it now accepts any key
    • Location: frontend/src/v1/components/ui/chart/types.ts:L81-L85, README.md:L75
    • Failure mode: keyof never includes string, so the fallback branch fires. Typos compile unchecked.
    • Suggested fix: reject never first ([Row] extends [never] ? never).
    • Verification: VERIFIED — tsc probe.
34. [Medium] [Lens 13] — The content-keyed memo has no test that a changed marker or series redraws
    • Location: frontend/src/v1/components/ui/chart/CartesianChart.tsx:L85-L98
    • Failure mode: deps reduced to [data] and a label dropped from markerSpec both pass 77/77, with the lint rule suppressed.
    • Verification: VERIFIED — mutation.
35. [Medium] [Lens 13] — Pinning the locale removed all coverage of the default formatters
    • Location: frontend/src/v1/components/ui/chart/charts.test.tsx:L53-L54
    • Failure mode: replacing the defaults with String passes 77/77.
    • Verification: VERIFIED — mutation.
36. [Low] [Lens 2/16] — Marker labels come from the render closure, so for a frame they can sit on the old rows' geometry; the marks.tsx header claims otherwise
    • Location: frontend/src/v1/components/ui/chart/marks.tsx:L3-L9
    • Suggested fix: carry the label in the row.
37. [Low] [Lens 13] — The CategoryKey index-signature branch has no type guard
    • Location: frontend/src/v1/components/ui/chart/types.ts:L16-L21
38. [Low] [Lens 3] — Injected __row/__marker_i fields can overwrite a same-named field on index-signature rows
    • Location: frontend/src/v1/components/ui/chart/markers.ts:L89

Lens checklist

1. Spec & intent — Clean
2. Architectural fit & precedent — See findings #36
3. Correctness & edge cases — See findings #30, #31, #38
4. Security — Clean (security deep-dive done; orchestrator)
5. Data integrity & migrations — N/A — no persistence
6. Concurrency — N/A — synchronous rendering only
7. API & contract compatibility — See findings #32 (still no consumers outside the chart folder; orchestrator)
8. Reliability & resilience — N/A — no I/O
9. Performance & cost — Clean (rows keep identity; reviewed by lifecycle agent)
10. Observability — Clean (new console.warn reviewed; see #30)
11. Operational safety — N/A — v1 not mounted
12. LLM/agent-specific — N/A — no LLM code
13. Testing — See findings #34, #35, #37
14. Dependencies & build — Clean (no dependency change; orchestrator)
15. Code quality — Clean (pinned Biome clean; orchestrator)
16. Doc & comment accuracy — See findings #33, #36
17. Cross-document rule consistency — Clean (merge-result sweep exit 1, hits consistent; orchestrator)
18. On-prem deployment experience — N/A — frontend v1 only
19. Scaling & workload lifecycle — N/A — no workloads

cost: agents=8 wall=04:32 lenses-examined=12 of 19 files=27 lines=3700

[ritwik-g]

Self-review round 4 — tip 0d79da9c8 (review-deep, FOLLOWUP)

Delta under judgement: 4965a6ed8..0d79da9c8.

Verdict — REQUEST CHANGES

Prior context fetched — followup-fetch.sh exited 0. Since boundary: 4965a6ed8.

Scope change — NO

Summary — Critical: 0 · High: 0 · Medium: 4 · Low: 5 · Lenses run: 19/19 (N/A ones with reasons below)

Security deep-dive: done (0 findings, after examination). boundary-sweep: 1 sink traced (the console.warn of marker labels and series keys), clean. Pinned Biome on the delta: clean. Code-reviewer core: clean.

Prior findings status

• #30 [Medium] Warnings while loading → RESOLVED in 0d79da9. They are now emitted from an effect, only when the chart is ready. The repeat nuances are tracked as #43.
• #31 [Medium] A duplicate marker overwrote the earlier one silently → RESOLVED in 0d79da9 (the duplicate is dropped with a warning; tested).
• #32 [Medium] String-only index rows → RESOLVED in 0d79da9 (Record<string, string> is rejected; tested).
• #33 [Medium] The data={[]} doc was wrong → RESOLVED in 0d79da9 (never accepts no key; tested).
• #34 [Medium] Memo untested → RESOLVED in 0d79da9 (rerender test; both deps mutants killed).
• #35 [Medium] Default formatters untested → PARTIALLY RESOLVED. The defaults are now asserted, but the test's name claims the viewer's locale, which it can't tell from en-US. See #41.
• #36 [Low] Labels came from the closure → PARTIALLY RESOLVED. Labels now travel in the rows, but the fields are keyed by series index. See #40.
• #37 [Low] CategoryKey branch unguarded → RESOLVED in 0d79da9.
• #38 [Low] Reserved-key collision → PARTIALLY RESOLVED. It now warns, but the plot is still corrupted, and only while ready. See #44 and #45.

New findings

39. [Medium] [Lens 7/16] — CategoryKey<Record<string, unknown>> is never, while NumericKey of the same row accepts any key
    • Location: frontend/src/v1/components/ui/chart/types.ts:L20-L25
    • Failure mode: Extract<unknown, string | number> is never, so loosely typed JSON rows can't take any xKey. The docstring says otherwise. Three agents found this independently.
    • Verification: VERIFIED — tsc probe.
40. [Medium] [Lens 2] — Marker fields are keyed by series index, so dropping or reordering a series can pair a stale row's label with another series for a frame
    • Location: frontend/src/v1/components/ui/chart/markers.ts:L22-L29
    • Suggested fix: key the fields by series key.
    • Verification: UNVERIFIED (traced in the recharts source).
41. [Medium] [Lens 13] — "formats with the viewer's locale by default" passes when the default is hard-coded to en-US
    • Location: frontend/src/v1/components/ui/chart/charts.test.tsx:L625-L639
    • Verification: VERIFIED — mutation.
42. [Medium] [Lens 13] — No test distinguishes the row-carried labels from the old closure labels
    • Location: frontend/src/v1/components/ui/chart/charts.test.tsx:L567-L599
    • Failure mode: the rerender test passes against the round-2 code.
    • Verification: VERIFIED. Can happy-dom observe the intermediate frame at all? UNVERIFIED.
43. [Low] [Lens 2/16] — "Warn once per distinct problem" is false
    • Location: frontend/src/v1/components/ui/chart/CartesianChart.tsx:L100-L108
    • Failure mode: StrictMode doubles the warnings in dev, the whole set is re-emitted when it changes, and a label containing a newline is split.
44. [Low] [Lens 3] — A reserved-prefix series key still corrupts its own plot
    • Location: frontend/src/v1/components/ui/chart/markers.ts:L122-L131
    • Note: only the warning guards it.
45. [Low] [Lens 3] — The reserved-key warning appears only when the chart has data
    • Location: frontend/src/v1/components/ui/chart/CartesianChart.tsx:L102-L103
46. [Low] [Lens 7] — NumericKey rejects numeric-subtype index rows (Record<string, 0 | 1>)
    • Location: frontend/src/v1/components/ui/chart/types.ts:L43
47. [Low] [Lens 13/16] — The [Row] extends [never] guards are redundant, and two doc sentences are wrong
    • Location: frontend/src/v1/components/ui/chart/types.ts:L33-L36
    • Detail: the never assertions pass without the guards. The NumericKey "otherwise checked as above" and the chartRows docstring's field description are both wrong.

Lens checklist

1. Spec & intent — Clean
2. Architectural fit & precedent — See findings #40, #43
3. Correctness & edge cases — See findings #44, #45
4. Security — Clean (security deep-dive done; orchestrator)
5. Data integrity & migrations — N/A — no persistence
6. Concurrency — N/A — synchronous rendering only
7. API & contract compatibility — See findings #39, #46 (no consumers outside the chart folder; orchestrator)
8. Reliability & resilience — N/A — no I/O
9. Performance & cost — Clean
10. Observability — See findings #43
11. Operational safety — N/A — v1 not mounted
12. LLM/agent-specific — N/A — no LLM code
13. Testing — See findings #41, #42, #47
14. Dependencies & build — Clean (no dependency change; orchestrator)
15. Code quality — Clean (pinned Biome clean; orchestrator)
16. Doc & comment accuracy — See findings #39, #47
17. Cross-document rule consistency — Clean (merge-result sweep, hits consistent; orchestrator)
18. On-prem deployment experience — N/A — frontend v1 only
19. Scaling & workload lifecycle — N/A — no workloads

cost: agents=8 wall=04:15 lenses-examined=12 of 19 files=27 lines=3840

[ritwik-g]

Self-review round 5 — tip 45c4e8f17 (review-deep, FOLLOWUP)

Delta under judgement: 0d79da9c8..45c4e8f17.

Verdict — REQUEST CHANGES

Prior context fetched — followup-fetch.sh exited 0. Since boundary: 0d79da9c8.

Scope change — NO

Summary — Critical: 0 · High: 0 · Medium: 2 · Low: 2 · Lenses run: 19/19 (N/A ones with reasons below)

Security deep-dive: done (0 findings, after examination). boundary-sweep: 1 sink traced (console.warn), clean. Pinned Biome: clean. The code-reviewer, silent-failure, lifecycle, type-design and test reports were all clean. Mutation: 12 of 12 mutants killed.

Prior findings status

• #39 [Medium] CategoryKey on unknown → RESOLVED in 8c13e10 (an unknown/any index accepts any key; tested by tsc).
• #40 [Medium] Index-keyed fields → RESOLVED in 8c13e10 (named by series key; literal-name and two-series tests).
• #41 [Medium] Locale test overclaims → RESOLVED in 8c13e10 (renamed to what it checks, with a comment on why).
• #42 [Medium] Row-carried labels untested → RESOLVED in 8c13e10. marks.test.tsx pins that each renderer reads its label from the row or value. The intermediate frame can't be observed in happy-dom, and the test header says so.
• #43 [Low] Warn-once claim → RESOLVED in 8c13e10 (comment reworded; JSON-keyed, so a newline no longer splits a warning).
• #44 [Low] Reserved key corrupts its plot → RESOLVED in 8c13e10 (no markers drawn when a key is reserved).
• #45 [Low] Reserved-key warning only while ready → RESOLVED in 8c13e10 (warns in any state; tested).
• #46 [Low] Numeric-subtype index → RESOLVED in 8c13e10.
• #47 [Low] Redundant guards and docs → RESOLVED in 8c13e10.

New findings

48. [Medium] [Lens 3] — Marker labels on the first or last point of a line or area chart are centred on the plot edge, so they are likely clipped by the SVG or run over the Y axis
    • Location: frontend/src/v1/components/ui/chart/marks.tsx:L30
    • Failure mode: a category axis without bars uses a point scale, so the end points sit on the plot's edges.
    • Suggested fix: anchor the label start or end at the extremes.
    • Verification: UNVERIFIED — would a browser clip a last-row label?
49. [Medium] [Lens 16] — The keptMarkers docstring ("the one rule") and the README markers row omit the reserved-key drop
    • Location: frontend/src/v1/components/ui/chart/markers.ts:L48-L55, README.md:L94
    • Verification: VERIFIED.
50. [Low] [Lens 16] — "would be overwritten" overstates the reserved-key collision (it "could" be), and so does the test title
    • Location: frontend/src/v1/components/ui/chart/markers.ts:L63-L64
51. [Low] [Lens 16] — The marks.test.tsx header claims the tests pin that a renderer "is given nothing else"
    • Location: frontend/src/v1/components/ui/chart/marks.test.tsx:L6-L14

Lens checklist

1. Spec & intent — Clean
2. Architectural fit & precedent — Clean
3. Correctness & edge cases — See findings #48
4. Security — Clean (security deep-dive done; orchestrator)
5. Data integrity & migrations — N/A — no persistence
6. Concurrency — N/A — synchronous rendering only
7. API & contract compatibility — Clean (type probes broad and clean; orchestrator consumer search: none)
8. Reliability & resilience — N/A — no I/O
9. Performance & cost — Clean
10. Observability — Clean
11. Operational safety — N/A — v1 not mounted
12. LLM/agent-specific — N/A — no LLM code
13. Testing — Clean (12/12 mutants killed)
14. Dependencies & build — Clean (orchestrator)
15. Code quality — Clean (pinned Biome clean; orchestrator)
16. Doc & comment accuracy — See findings #49, #50, #51
17. Cross-document rule consistency — See findings #49 (orchestrator)
18. On-prem deployment experience — N/A — frontend v1 only
19. Scaling & workload lifecycle — N/A — no workloads

cost: agents=8 wall=04:47 lenses-examined=13 of 19 files=28 lines=4043

[ritwik-g]

Self-review round 6 — tip a9662c565 (review-deep, FOLLOWUP)

Delta under judgement: 45c4e8f17..a9662c565.

Verdict — REQUEST CHANGES

Prior context fetched — followup-fetch.sh exited 0. Since boundary: 45c4e8f17.

Scope change — NO

Summary — Critical: 0 · High: 0 · Medium: 1 · Low: 4 · Lenses run: 19/19 (N/A ones with reasons below)

Security deep-dive: done (0 findings, after examination). boundary-sweep: not triggered (no new sink in the delta). Pinned Biome: clean. The core, silent-failure and lifecycle reviews were clean.

Prior findings status

• #48 [Medium] Edge labels clipped → PARTIALLY RESOLVED in a9662c5. Row 0 and the last row now anchor inward, but a long label one row in still overflows. See #52.
• #49 [Medium] Drop rule omitted the reserved key → RESOLVED in a9662c5.
• #50 [Low] "would be overwritten" → RESOLVED in a9662c5.
• #51 [Low] marks.test header → RESOLVED in a9662c5.

New findings

52. [Medium] [Lens 3/16] — Anchoring keys on the row index, not the distance to the edge, so a long label one row in from either edge still overflows
    • Location: frontend/src/v1/components/ui/chart/marks.tsx:L86-L94
    • Failure mode: on a 30-day chart, a label on row 1 or row 28 runs into the axis or is clipped. The docstring claims the label stays inside the chart.
    • Suggested fix: anchor by distance, from the estimated half-width of the label to the plot edge.
    • Verification: UNVERIFIED, since happy-dom does not measure text.
53. [Low] [Lens 16] — The MarkerMark docstring says "first and last point", while the code works by row and centres a single-row chart
    • Location: frontend/src/v1/components/ui/chart/marks.tsx:L15-L19
54. [Low] [Lens 7] — The public ChartMarker doc points at a module consumers can't import, and no type doc mentions the reserved __marker prefix
    • Location: frontend/src/v1/components/ui/chart/types.ts:L79-L84
55. [Low] [Lens 13] — The label's 4px x nudge is untested; dropping it or flipping its direction survives
    • Location: frontend/src/v1/components/ui/chart/marks.tsx:L45
56. [Low] [Lens 13] — No real-chart test pins anchoring; it relies on Recharts passing points for every row
    • Location: frontend/src/v1/components/ui/chart/charts.test.tsx:L293-L317

Lens checklist

1. Spec & intent — Clean
2. Architectural fit & precedent — Clean
3. Correctness & edge cases — See findings #52
4. Security — Clean (security deep-dive done; orchestrator)
5. Data integrity & migrations — N/A — no persistence
6. Concurrency — N/A — synchronous rendering only
7. API & contract compatibility — See findings #54
8. Reliability & resilience — N/A — no I/O
9. Performance & cost — Clean
10. Observability — Clean
11. Operational safety — N/A — v1 not mounted
12. LLM/agent-specific — N/A — no LLM code
13. Testing — See findings #55, #56
14. Dependencies & build — Clean (orchestrator)
15. Code quality — Clean (pinned Biome clean; orchestrator)
16. Doc & comment accuracy — See findings #53
17. Cross-document rule consistency — Clean (orchestrator)
18. On-prem deployment experience — N/A — frontend v1 only
19. Scaling & workload lifecycle — N/A — no workloads

cost: agents=8 wall=03:21 lenses-examined=13 of 19 files=28 lines=4091

[ritwik-g]

Self-review round 7 — tip 0d2f1c1c9 (review-deep, FOLLOWUP)

Delta under judgement: a9662c565..0d2f1c1c9.

Verdict — REQUEST CHANGES

Prior context fetched — followup-fetch.sh exited 0. Since boundary: a9662c565.

Scope change — NO

Summary — Critical: 0 · High: 0 · Medium: 2 · Low: 6 · Lenses run: 19/19 (N/A ones with reasons below)

Security deep-dive: done (0 findings, after examination). boundary-sweep: not triggered. Pinned Biome: clean. The core and lifecycle reviews were clean.

Prior findings status

• #52 [Medium] Row-index anchoring → RESOLVED in 0d2f1c1 (anchored by distance to the plot edge; tested).
• #53 [Low] Docstring rows vs points → RESOLVED in 0d2f1c1.
• #54 [Low] Public marker doc → RESOLVED in 0d2f1c1 (rules and reserved prefix in the type docs). The warning clause is inexact, see #60.
• #55 [Low] Nudge untested → RESOLVED in 0d2f1c1.
• #56 [Low] No real-chart anchor test → RESOLVED in 0d2f1c1.

New findings

57. [Medium] [Lens 3] — Bar and stacked-bar marker labels are always centred, so on the first or last bar a long label crosses the plot edge
    • Location: frontend/src/v1/components/ui/chart/marks.tsx:L173-L189
    • Note: LabelList content receives parentViewBox, which supplies the bounds.
58. [Medium] [Lens 13] — The anchor test doesn't pin the half-width threshold
    • Location: frontend/src/v1/components/ui/chart/marks.test.tsx:L72-L97
    • Failure mode: comparing the full width, or a char width of 14, survives.
    • Verification: VERIFIED — mutation.
59. [Low] [Lens 3] — A label wider than the space on both sides of the dot reads start, which overflows more than centring would
    • Location: frontend/src/v1/components/ui/chart/marks.tsx:L92-L99
60. [Low] [Lens 16] — The ChartMarker doc says every drop warns, but warnings wait for the ready state
    • Location: frontend/src/v1/components/ui/chart/types.ts:L83-L90
61. [Low] [Lens 16] — "so it is not cut off" overstates what the anchoring guarantees
    • Location: frontend/src/v1/components/ui/chart/marks.tsx:L15-L20
62. [Low] [Lens 7] — labelAnchor is exported but nothing imports it
    • Location: frontend/src/v1/components/ui/chart/marks.tsx:L77
63. [Low] [Lens 7] — The reserved prefix is only enforced at runtime
    • Location: frontend/src/v1/components/ui/chart/types.ts:L72
    • Note: optional compile-time Exclude.
64. [Low] [Lens 13] — The strict < vs <= boundary is unpinned
    • Location: frontend/src/v1/components/ui/chart/marks.tsx:L94

Lens checklist

1. Spec & intent — Clean
2. Architectural fit & precedent — Clean
3. Correctness & edge cases — See findings #57, #59
4. Security — Clean (security deep-dive done; orchestrator)
5. Data integrity & migrations — N/A — no persistence
6. Concurrency — N/A — synchronous rendering only
7. API & contract compatibility — See findings #62, #63
8. Reliability & resilience — N/A — no I/O
9. Performance & cost — Clean
10. Observability — Clean
11. Operational safety — N/A — v1 not mounted
12. LLM/agent-specific — N/A — no LLM code
13. Testing — See findings #58, #64
14. Dependencies & build — Clean (orchestrator)
15. Code quality — Clean (pinned Biome clean; orchestrator)
16. Doc & comment accuracy — See findings #60, #61
17. Cross-document rule consistency — Clean (orchestrator)
18. On-prem deployment experience — N/A — frontend v1 only
19. Scaling & workload lifecycle — N/A — no workloads

cost: agents=8 wall=04:06 lenses-examined=13 of 19 files=28 lines=4163

[ritwik-g]

Self-review round 8 — tip 8720d2895 (review-deep, FOLLOWUP)

Delta under judgement: 0d2f1c1c9..8720d2895.

Verdict — REQUEST CHANGES

Prior context fetched — followup-fetch.sh exited 0. Since boundary: 0d2f1c1c9.

Scope change — NO

Summary — Critical: 0 · High: 0 · Medium: 2 · Low: 5 · Lenses run: 19/19 (N/A ones with reasons below)

Security deep-dive: done (0 findings, after examination). boundary-sweep: not triggered. Pinned Biome: clean. The core and lifecycle reviews were clean.

Prior findings status

• #57 [Medium] Bar labels not anchored → RESOLVED in 8720d28 (tested through a real bar chart).
• #58 [Medium] Threshold unpinned → RESOLVED in 8720d28 (half-width and the exact left boundary pinned).
• #59 [Low] Wider-than-both-sides case → RESOLVED in 8720d28. A narrower variant remains, see #65.
• #60 [Low] Warning clause → RESOLVED in 8720d28.
• #61 [Low] "Not cut off" overstated → RESOLVED in 8720d28.
• #62 [Low] labelAnchor export → RESOLVED in 8720d28.
• #63 [Low] Reserved prefix only checked at runtime → WAIVED by this reviewer. Runtime check by design, documented at types.ts ChartSeries.key.
• #64 [Low] < vs <= → RESOLVED for the left edge in 8720d28. The right edge is still unpinned, see #71.

New findings

65. [Medium] [Lens 3] — An inward-anchored label can overflow the opposite edge by more than centring would
    • Location: frontend/src/v1/components/ui/chart/marks.tsx:L85-L93
    • Failure mode: the flip checks the half-width but not the full width it then needs.
    • Suggested fix: choose the anchor with the least overflow.
    • Verification: VERIFIED by trace (found by two agents).
66. [Medium] [Lens 13] — The right-axis term of the bar-marker inset is untested
    • Location: frontend/src/v1/components/ui/chart/CartesianChart.tsx:L195
    • Failure mode: removing, inverting or always adding it all survive.
    • Verification: VERIFIED — mutation.
67. [Low] [Lens 7] — The bar plot bounds are rebuilt from layout constants plus Recharts' undocumented parentViewBox meaning
    • Location: frontend/src/v1/components/ui/chart/CartesianChart.tsx:L191-L197
    • Suggested fix: usePlotArea() would read the bounds directly.
68. [Low] [Lens 7] — labelAnchor types its bounds as unknown, wider than either caller needs
    • Location: frontend/src/v1/components/ui/chart/marks.tsx:L75-L82
69. [Low] [Lens 16] — The MarkerMark docstring overstates when the label moves off-centre
    • Location: frontend/src/v1/components/ui/chart/marks.tsx:L16-L17
70. [Low] [Lens 16] — "once the chart shows data" can be read as a count
    • Location: frontend/src/v1/components/ui/chart/types.ts:L90
71. [Low] [Lens 13] — The right-edge boundary is unpinned
    • Location: frontend/src/v1/components/ui/chart/marks.tsx:L85

Lens checklist

1. Spec & intent — Clean
2. Architectural fit & precedent — Clean
3. Correctness & edge cases — See findings #65
4. Security — Clean (security deep-dive done; orchestrator)
5. Data integrity & migrations — N/A — no persistence
6. Concurrency — N/A — synchronous rendering only
7. API & contract compatibility — See findings #67, #68
8. Reliability & resilience — N/A — no I/O
9. Performance & cost — Clean
10. Observability — Clean
11. Operational safety — N/A — v1 not mounted
12. LLM/agent-specific — N/A — no LLM code
13. Testing — See findings #66, #71
14. Dependencies & build — Clean (orchestrator)
15. Code quality — Clean (pinned Biome clean; orchestrator)
16. Doc & comment accuracy — See findings #69, #70
17. Cross-document rule consistency — Clean (orchestrator)
18. On-prem deployment experience — N/A — frontend v1 only
19. Scaling & workload lifecycle — N/A — no workloads

cost: agents=8 wall=03:44 lenses-examined=13 of 19 files=28 lines=4282

[ritwik-g]

Self-review round 9 — tip a30a61b76 (review-deep, FOLLOWUP)

Delta under judgement: 8720d2895..a30a61b76, plus the pre-commit.ci formatting commit 15796d64a (test file only). The review is pinned to that head.

Verdict — REQUEST CHANGES

Prior context fetched — followup-fetch.sh exited 0. Since boundary: 8720d2895.

Scope change — NO

Summary — Critical: 0 · High: 0 · Medium: 1 · Low: 3 · Lenses run: 19/19 (N/A ones with reasons below)

Security deep-dive: done (0 findings, after examination). boundary-sweep: not triggered. Pinned Biome: clean. The core, silent-failure, lifecycle and type-design reviews were clean. Mutation: 15 of 17 mutants killed; the 2 survivors are #74.

Prior findings status (thread replies already posted)

• #64 [Low] < vs <= boundary → RESOLVED in a30a61b (both edges pinned).
• #65 [Medium] Inward flip overflowed the opposite edge → RESOLVED in a30a61b (least-overflow anchoring).
• #66 [Medium] Right-axis inset untested → OBSOLETE. The inset is gone; the plot area comes from usePlotArea(), and a right-axis test covers it.
• #67 [Low] Bounds restated from constants → RESOLVED in a30a61b.
• #68 [Low] unknown bounds → RESOLVED in a30a61b.
• #69 [Low] MarkerMark docstring → RESOLVED in a30a61b.
• #70 [Low] "once" → "when" → RESOLVED in a30a61b. Two other restatements remain, see #73.
• #71 [Low] Right boundary → RESOLVED in a30a61b.

New findings

72. [Medium] [Lens 16] — The labelAnchor docstring says a label too wide for either side "stays centred", but the code and a test pick start near an edge
    • Location: frontend/src/v1/components/ui/chart/marks.tsx:L82-L84
73. [Low] [Lens 17] — "once" remains in markers.ts:55 and README.md:94
    • Location: frontend/src/v1/components/ui/chart/README.md:L94
    • Made stale by: types.ts:L90
74. [Low] [Lens 13] — The end candidate's nudge is unpinned
    • Location: frontend/src/v1/components/ui/chart/marks.tsx:L102
75. [Low] [Lens 13] — The right-axis bar test passes by only 1.3px
    • Location: frontend/src/v1/components/ui/chart/charts.test.tsx:L753-L777

Lens checklist

1. Spec & intent — Clean
2. Architectural fit & precedent — Clean
3. Correctness & edge cases — Clean
4. Security — Clean (security deep-dive done; orchestrator)
5. Data integrity & migrations — N/A — no persistence
6. Concurrency — N/A — synchronous rendering only
7. API & contract compatibility — Clean (usePlotArea public across ^3.5.1; barrel unchanged)
8. Reliability & resilience — N/A — no I/O
9. Performance & cost — Clean
10. Observability — Clean
11. Operational safety — N/A — v1 not mounted
12. LLM/agent-specific — N/A — no LLM code
13. Testing — See findings #74, #75
14. Dependencies & build — Clean (orchestrator)
15. Code quality — Clean (pinned Biome clean; orchestrator)
16. Doc & comment accuracy — See findings #72
17. Cross-document rule consistency — See findings #73 (orchestrator)
18. On-prem deployment experience — N/A — frontend v1 only
19. Scaling & workload lifecycle — N/A — no workloads

cost: agents=8 wall=05:42 lenses-examined=13 of 19 files=28 lines=4293

[ritwik-g]

Self-review round 10 — tip 0749bfe1d (review-deep, FOLLOWUP) — circuit breaker reached

Delta under judgement: 15796d64a..0749bfe1d (a docstring, two doc wordings and tests).

Verdict — REQUEST CHANGES

Prior context fetched — followup-fetch.sh exited 0. Since boundary: 15796d64a.

Scope change — NO

Summary — Critical: 0 · High: 0 · Medium: 1 · Low: 1 · Lenses run: 19/19 (N/A ones with reasons below)

The delta itself is clean. The core, silent-failure, type-design, lifecycle, comment and test reviews all found nothing, and both new tests kill their mutants. Security deep-dive: done (0 findings). Pinned Biome: clean. The whole-diff cold read raised two findings on code from round 1 that no earlier round flagged.

Prior findings status

• #72 [Medium] Docstring summary contradicted the rule → RESOLVED in 0749bfe.
• #73 [Low] "once" in two restatements → RESOLVED in 0749bfe.
• #74 [Low] End nudge unpinned → RESOLVED in 0749bfe (mutants killed).
• #75 [Low] Right-axis test margin was 1.3px → RESOLVED in 0749bfe (about 4–5px).

New findings

76. [Medium] [Lens 3] — Loading and empty states are only the plot's height, but a ready chart adds the legend row and gap, so it grows about 28px when data arrives and shifts the content below
    • Location: frontend/src/v1/components/ui/chart/ChartFrame.tsx:L56-L100
    • Suggested fix: reserve the legend row in the loading and empty branches when a legend is configured.
    • Verification: VERIFIED by reading the branches.
77. [Low] [Lens 3/16] — Sparkline tooltip's labelOf reads the category from props, contrary to the rule marks.tsx states for renderers
    • Location: frontend/src/v1/components/ui/chart/Sparkline.tsx:L88-L92
    • Failure mode: for a frame after a data change, the heading can name another row.

Circuit breaker

This is round 10 of 10, and the ready test is not met: #76 is above the floor. Per the self-review rules this run stops here. The PR stays a draft, and the fix is described rather than made, since a fix landed at the cap would itself be unreviewed. Case: a fix is still owed. #76 is a small, contained change: a placeholder legend row in the ChartFrame loading and empty branches. #77 is a one-line change: read the category from payload[0].payload. A human should decide whether to authorise one more fix-and-review round.

Lens checklist

1. Spec & intent — Clean
2. Architectural fit & precedent — Clean
3. Correctness & edge cases — See findings #76, #77
4. Security — Clean (security deep-dive done; orchestrator)
5. Data integrity & migrations — N/A — no persistence
6. Concurrency — N/A — synchronous rendering only
7. API & contract compatibility — Clean
8. Reliability & resilience — N/A — no I/O
9. Performance & cost — Clean
10. Observability — Clean
11. Operational safety — N/A — v1 not mounted
12. LLM/agent-specific — N/A — no LLM code
13. Testing — Clean (mutants killed)
14. Dependencies & build — Clean (orchestrator)
15. Code quality — Clean (pinned Biome clean; orchestrator)
16. Doc & comment accuracy — See findings #77
17. Cross-document rule consistency — Clean (orchestrator)
18. On-prem deployment experience — N/A — frontend v1 only
19. Scaling & workload lifecycle — N/A — no workloads

cost: agents=8 wall=04:11 lenses-examined=13 of 19 files=28 lines=4305

[ritwik-g]

Self-review round 11 — tip 6dfc936d5 (review-deep, FOLLOWUP; rounds 11–15 authorised by Ritwik)

Delta under judgement: df4e457a0..6dfc936d5 (the fixes for #76 and #77). The merge of origin/main (df4e457a0) brought in no src/v1 changes.

Verdict — REQUEST CHANGES

Prior context fetched — followup-fetch.sh exited 0. Since boundary: 0749bfe1d.

Scope change — NO

Summary — Critical: 0 · High: 0 · Medium: 2 · Low: 3 · Lenses run: 19/19 (N/A ones with reasons below)

Security deep-dive: done (0 findings, after examination). boundary-sweep: not triggered. Pinned Biome: clean. The core, silent-failure, type-design and lifecycle reviews were clean.

Prior findings status

• #76 [Medium] Loading/empty footprint → RESOLVED in 6dfc936. The legend is laid out invisibly and the footprint is tested. The claim is only true for fixed series, see #78.
• #77 [Low] Sparkline heading read from props → RESOLVED in 6dfc936. It now reads from the hovered row; unit-tested.

New findings

78. [Medium] [Lens 16] — "Keeps the ready chart's footprint" holds only when series is known before the data
    • Location: frontend/src/v1/components/ui/chart/README.md:L165-L166, ChartFrame.tsx:L65-L67
    • Failure mode: series built from the response (pivoted rows) reserve nothing while loading.
79. [Medium] [Lens 13] — No test fails on the pre-change Sparkline tooltip
    • Location: frontend/src/v1/components/ui/chart/ChartTooltip.test.tsx:L10-L31
    • Failure mode: the one-frame divergence it closes can't be observed in happy-dom. Either state that, or describe the change as a refactor.
80. [Low] [Lens 13] — The footprint test doesn't pin "one legend"; a duplicated reserved legend survives
    • Location: frontend/src/v1/components/ui/chart/charts.test.tsx:L795-L800
81. [Low] [Lens 13] — The non-object-row guard in ChartTooltip is unexercised
    • Location: frontend/src/v1/components/ui/chart/ChartTooltip.tsx:L57
82. [Low] [Lens 16] — The footprint test's name and order comment claim more than its assertions check
    • Location: frontend/src/v1/components/ui/chart/charts.test.tsx:L780

Lens checklist

1. Spec & intent — Clean
2. Architectural fit & precedent — Clean
3. Correctness & edge cases — Clean
4. Security — Clean (security deep-dive done; orchestrator)
5. Data integrity & migrations — N/A — no persistence
6. Concurrency — N/A — synchronous rendering only
7. API & contract compatibility — Clean (barrel unchanged)
8. Reliability & resilience — N/A — no I/O
9. Performance & cost — Clean
10. Observability — Clean
11. Operational safety — N/A — v1 not mounted
12. LLM/agent-specific — N/A — no LLM code
13. Testing — See findings #79, #80, #81
14. Dependencies & build — Clean (orchestrator; main merged, no dependency change)
15. Code quality — Clean (pinned Biome clean; orchestrator)
16. Doc & comment accuracy — See findings #78, #82
17. Cross-document rule consistency — Clean (orchestrator)
18. On-prem deployment experience — N/A — frontend v1 only
19. Scaling & workload lifecycle — N/A — no workloads

cost: agents=8 wall=04:09 lenses-examined=13 of 19 files=29 lines=4421

[ritwik-g]

Self-review round 12 — tip e04a78761 (review-deep, FOLLOWUP; rounds 11–15 authorised by Ritwik)

Delta under judgement: 6dfc936d5..e04a78761 (the round-11 fixes for #78–#82: docs, comments and tests). Posted against head f7bcfcc60, which carries this round's fixes; those are round 13's delta.

Verdict — REQUEST CHANGES

Prior context fetched — followup-fetch.sh exited 0, with --prev-sha 6dfc936d5.

Scope change — NO

Summary — Critical: 0 · High: 0 · Medium: 3 · Low: 2 · Lenses run: 19/19 (the N/A lenses give reasons below)

Security deep-dive: done (0 findings, after examination). Boundary sweep: not triggered. Pinned Biome: clean. Suite: 95/95, then 96/96 with this round's fixes. The core, silent-failure, type-design and lifecycle reviews were clean.

Prior findings status

• #78 Footprint claim scoped to series known before the data → RESOLVED in e04a787.
• #79 Tooltip test stated as a mechanism test → RESOLVED in e04a787.
• #80 Exactly one legend asserted → RESOLVED in e04a787 (mutation-verified: a duplicated legend is killed).
• #81 Missing-row guard exercised → RESOLVED in e04a787 (mutation-verified).
• #82 Test name and comment narrowed → RESOLVED in e04a787.

New findings

83. [Medium] [Lens 13] — "Never from the label" is untested for a row that lacks the category key
    • Location: frontend/src/v1/components/ui/chart/ChartTooltip.tsx:L58
    • Failure mode: the mutant row[categoryKey] ?? label survives all four tooltip tests, so a row without the key would show the row index as the heading. Fixed in f7bcfcc with a new case, which kills that mutant.
84. [Medium] [Lens 3, 16] — Ramp steps are separable only by lightness, 1.24–1.40:1 between neighbours
    • Location: frontend/src/v1/components/ui/chart/README.md:L125
    • Failure mode: untoned series that are not an order (this week and last week) on a line or a grouped bar are hard to tell apart, and "do not go past five" implies five are separable. Ratios were recomputed from tokens.json with the WCAG luminance formula. Fixed in f7bcfcc (README guidance: give such series tones; stacks stay on the ramp). The ramp values themselves are still pending lane C and designer sign-off.
85. [Medium] [Lens 16] — The README recommends an EmptyState component that does not exist, in a box that does not grow
    • Location: frontend/src/v1/components/ui/chart/README.md:L82
    • Failure mode: the slot is centred in a fixed height box (40px on a sparkline) and replaces the screen-reader title prefix. Fixed in f7bcfcc: the docs now say the slot must fit and must name the chart.
86. [Low] [Lens 16] — The tooltip test header reads as if it covers every test — ChartTooltip.test.tsx:L15. Fixed in f7bcfcc ("the category-key tests below").
87. [Low] [Lens 16] — "No chart-level test can see it" is stated without evidence — ChartTooltip.test.tsx:L14. Kept: rounds 9–10 tried a chart-level data swap under happy-dom and could not observe the frame. That is the same basis as the marks.test.tsx header.

Lens checklist

1. Spec & intent — Clean
2. Architectural fit & precedent — Clean
3. Correctness & edge cases — See findings #84
4. Security — Clean (security deep-dive done; orchestrator)
5. Data integrity & migrations — N/A — no persistence
6. Concurrency — N/A — synchronous rendering only
7. API & contract compatibility — Clean (barrel and types unchanged)
8. Reliability & resilience — N/A — no I/O
9. Performance & cost — Clean
10. Observability — Clean
11. Operational safety — N/A — v1 not mounted
12. LLM/agent-specific — N/A — no LLM code
13. Testing — See findings #83
14. Dependencies & build — Clean (orchestrator; no dependency change)
15. Code quality — Clean (pinned Biome clean; orchestrator)
16. Doc & comment accuracy — See findings #84, #85, #86, #87
17. Cross-document rule consistency — Clean (orchestrator; README matches types.ts and the v1 README)
18. On-prem deployment experience — N/A — frontend v1 only
19. Scaling & workload lifecycle — N/A — no workloads

cost: agents=8 wall=06:22 lenses-examined=13 of 19 files=29 lines=4454

[ritwik-g]

Self-review round 13 — tip f7bcfcc60 (review-deep, FOLLOWUP; rounds 11–15 authorised by Ritwik)

Delta under judgement: e04a78761..f7bcfcc60 (the round-12 fixes for #83–#86). Posted against head 59e21f566, which carries this round's fixes; those are round 14's delta.

Verdict — REQUEST CHANGES

Prior context fetched — followup-fetch.sh exited 0, with --prev-sha e04a78761.

Scope change — NO

Summary — Critical: 0 · High: 0 · Medium: 4 · Low: 2 · Lenses run: 19/19 (the N/A lenses give reasons below)

Security deep-dive: done (0 findings, after examination). Boundary sweep: not triggered. Pinned Biome: clean. Suite: 96/96, then 97/97 with this round's fixes. The lifecycle and silent-failure reviews were clean.

Prior findings status

• #83 → RESOLVED in f7bcfcc. Mutation-verified: the ?? label and key-presence fallbacks are now killed.
• #84 → RESOLVED in f7bcfcc (README); refined by #89.
• #85 → RESOLVED in f7bcfcc (README); the JSDoc half is #88.
• #86 → RESOLVED in f7bcfcc.
• #87 → WAIVED: the claim rests on the round 9–10 attempt, as recorded in round 12.

New findings

88. [Medium] [Lens 16] — The emptySlot JSDoc still recommends the nonexistent EmptyState and omits the fit and naming rules
    • Location: frontend/src/v1/components/ui/chart/types.ts:L134
    • Fixed in 59e21f5.
89. [Medium] [Lens 16, 2] — "A tone each" leaves only meaning-bearing tones past two series
    • Location: frontend/src/v1/components/ui/chart/README.md:L132
    • Fixed in 59e21f5. Two non-ordered series use primary and muted; with more, chart them separately or group. "Or use tones" is dropped from the five-step rule.
90. [Medium] [Lens 3] — In the Unstract theme, primary (#072b4d) is 1.45:1 from --chart-5 and 1.79:1 from --chart-4
    • Location: frontend/src/v1/components/ui/chart/README.md:L140
    • Ratios recomputed with the WCAG formula from tokens.json. Documented in 59e21f5 (keep primary off ramp charts). Changing the token values is for the designer and lane C, and is added to the PR's sign-off list.
91. [Medium] [Lens 16] — The README says inline series and markers are safe but says nothing about data, whose identity matters
    • Location: frontend/src/v1/components/ui/chart/README.md:L104
    • Fixed in 59e21f5 with the mechanism already stated in CartesianChart.tsx: a new array recomputes the rows and resets Recharts' data store. No claim is made about the tooltip state.
92. [Low] [Lens 3] — A lone untoned series gets the faintest step in light mode — palette.ts. Not changed: this is a design choice for the ramp sign-off, and it is added to that list.
93. [Low] [Lens 13] — The row !== null guard was untested — ChartTooltip.tsx:L57. Fixed in 59e21f5 (a payload: null case, which kills the mutant).

Lens checklist

1. Spec & intent — Clean
2. Architectural fit & precedent — See findings #89
3. Correctness & edge cases — See findings #90, #92
4. Security — Clean (security deep-dive done; orchestrator)
5. Data integrity & migrations — N/A — no persistence
6. Concurrency — N/A — synchronous rendering only
7. API & contract compatibility — Clean (no exported change)
8. Reliability & resilience — N/A — no I/O
9. Performance & cost — Clean
10. Observability — Clean
11. Operational safety — N/A — v1 not mounted
12. LLM/agent-specific — N/A — no LLM code
13. Testing — See findings #93
14. Dependencies & build — Clean (orchestrator; no dependency change)
15. Code quality — Clean (pinned Biome clean; orchestrator)
16. Doc & comment accuracy — See findings #88, #89, #91
17. Cross-document rule consistency — See findings #88
18. On-prem deployment experience — N/A — frontend v1 only
19. Scaling & workload lifecycle — N/A — no workloads

cost: agents=8 wall=04:22 lenses-examined=13 of 19 files=29 lines=4475

[greptile-apps]

via Greptile

Confidence Score: 5/5

[Low impact] The PR appears safe to merge, with no new actionable issue found.

Summary

Adds five shared v1 charts with token-based colours, tooltips, loading and empty states, row-based markers, and text alternatives.

• V1 pages can turn numeric series into shared, accessible charts.
• V1 charts use a five-step color ramp across themes.

_{Reviews (7) · Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." · Reviewed by Greptile}

[Deepak-Kesavan]

🤖 Automated review

Automated review by Unstract PR review kit (Claude Code). These are starting points, not verdicts: the maintainer decides what to act on, closing any of them is a legitimate outcome, and nothing here blocks the merge. Each finding was checked by an independent verifier; anything tagged [unverified] had its facts confirmed but is a judgement call.

26 inline comment(s) · 1 finding(s) not on changed lines (below).

Findings off the diff

• frontend/scripts/check-v1-styles.mjs:104
  [minor] CONFIRMED · (pre-existing, file not in this diff) The S2 checker lets a named CSS colour through as a Recharts prop when the name is not on its 31-name list, so the PR's new colour surface (fill/stroke/activeDot props) is only partly guarded.

A later chart edit writes , , activeDot={{ stroke: "darkblue" }} or const TICK = { fill: "steelblue" }. lint:v1 passes and v1 ships a raw colour that ignores light, dark and Unstract modes. This PR's README and v1/README.md say chart colours come only from --chart-1..5 and the semantic tokens.

Suggested fix: List all 148 CSS named colours in NAMED_COLOURS, at least for the whole-literal check (a string that is only a named colour). Or, in chart/, require fill/stroke values to match var(--token) or none/transparent/currentColor.

_{🤖 Unstract PR review kit (Claude Code) · review-pr-bot:f25e16b969cb}

_{review-pr-bot:review}

[ritwik-g]

Self-review round 23 — tip dc35a4084 (review-deep, FOLLOWUP; Ritwik's go-ahead of 10 Nov)

Delta under judgement:

• f6e4d18dd..dc35a4084: the merge of origin/main (a09a3d5, with conflicts resolved in frontend/src/v1/styles/* and frontend/src/v1/README.md) and the fixes for Deepak's review 5822680995.
• The fixes are the mixed-sign stack (4510266843), the negative-bar marker (4510266848) and the all-gap empty state (4510266955), plus test-only gaps.

Posted against head ae87a9f52, which holds this round's fixes; round 24 reviews them.

Verdict — REQUEST CHANGES (two Mediums, fixed in ae87a9f).

Prior context fetched — followup-fetch.sh exited 0, with --prev-sha f6e4d18dd.

Scope change — NO

Summary — Critical: 0 · High: 0 · Medium: 2 · Low: 3 · Lenses run: 19/19 (the N/A lenses give reasons below)

Security deep-dive: done (0 findings, after examination). Boundary sweep: not triggered. Pinned Biome: clean. The v1 suite passes 560/560 (after bun install --frozen-lockfile for main's new @tanstack/react-virtual).

Merge resolution was verified by three reviewers: both sides kept, and 45 colour tokens (30 + 10 code/diff + 5 chart), matching the test. The lifecycle and type reviews were clean. Recharts 3.7.0 confirms that a negative bar's y is its lower tip, for plain bars and for stackOffset="sign".

Every new fix and test is mutation-verified:

• stack offset;
• marker label position and clamp;
• hasValues in each chart;
• connectNulls on line, area and sparkline;
• the xFormat ticks, className, the grid and the right-axis domain.

New findings

137. [Medium] [Lens 3, 16] — At the plot's bottom edge the clamp put a negative bar's label baseline on its own dot, and the test accepted it
     - Location: frontend/src/v1/components/ui/chart/marks.tsx:L223
     - Found by four reviewers.
     - Fixed in ae87a9f: where there is no room under the tip, the label reads beside the dot, on whichever side fits. The tests require the glyphs to clear the dot, check that the fixture's tip really is at the edge, and cover both sides. Mutants for no side branch, always the clamp, a zero gap, either side forced, and an ignored labelX are all killed.
138. [Medium] [Lens 3] — hasValues read unparsed numeric strings (DRF Decimals in pivoted rows, which NumericKey allows) as gaps, so a chart with data showed "No data for this period"
     - Location: frontend/src/v1/components/ui/chart/describe.ts:L57
     - Fixed in ae87a9f: any present value counts. Only null, missing and non-finite series values are gaps.
     - Unit tests in describe.test.ts pin that only the series' own keys are read (closes #139).
     - Follow-up, recorded in the PR body: a dev warning when a series key is missing from every row.
139. [Low] [Lens 13] — Nothing pinned that hasValues reads only series keys. Fixed in ae87a9f.
140. [Low] [Lens 16, 17] — The marker prose said "on top" / "label above", which is wrong for bars below zero — README.md, marks.tsx, markers.ts. Fixed in ae87a9f.
141. [Low] [Lens 16] — The test comment's "40 minus 4 + 4" read as 40. Fixed in ae87a9f.

Lens checklist

1. Spec & intent — Clean
2. Architectural fit & precedent — Clean
3. Correctness & edge cases — See findings #137, #138
4. Security — Clean (security deep-dive done; orchestrator)
5. Data integrity & migrations — N/A — no persistence
6. Concurrency — N/A — synchronous rendering only
7. API & contract compatibility — Clean (hasValues internal; token contract intact after merge)
8. Reliability & resilience — N/A — no I/O
9. Performance & cost — Clean
10. Observability — Clean
11. Operational safety — N/A — v1 not mounted
12. LLM/agent-specific — N/A — no LLM code
13. Testing — See findings #139
14. Dependencies & build — Clean (main's new dependency installs from the lockfile; nothing added here)
15. Code quality — Clean (pinned Biome clean; orchestrator)
16. Doc & comment accuracy — See findings #141
17. Cross-document rule consistency — See findings #140
18. On-prem deployment experience — N/A — frontend v1 only
19. Scaling & workload lifecycle — N/A — no workloads

cost: agents=8 lenses-examined=13 of 19 files=29 lines=4971

[ritwik-g]

Self-review round 24 — tip ae87a9f52 (review-deep, FOLLOWUP)

Delta under judgement: dc35a4084..ae87a9f52 (beside-the-dot label for bottom-edge negative bars; hasValues counting strings; doc wording). Posted against head 05ac6c317, which holds this round's fixes; round 25 reviews them.

Verdict — REQUEST CHANGES (fixed in 05ac6c3).

Prior context fetched — followup-fetch.sh exited 0, with --prev-sha dc35a4084.

Scope change — NO

Summary — Critical: 0 · High: 0 · Medium: 4 · Low: 4 · Lenses run: 19/19 (the N/A lenses give reasons below)

Security deep-dive: done (0 findings, after examination). Boundary sweep: not triggered. Pinned Biome: clean. The v1 suite passes 562/562. The lifecycle review was clean. On the type review: no exported contract change.

Prior findings status

• #137 → RESOLVED in ae87a9f, then completed in 05ac6c3 (#142, #143).
• #138 → OBSOLETE: ae87a9f counted strings as data, which split the gap rule (#144). 05ac6c3 restores one rule and documents "parse first".
• #139, #140, #141 → RESOLVED in ae87a9f.

New findings

142. [Medium] [Lens 3] — The beside label was not clamped: a tip exactly on the bottom edge (a minimum on a nice tick) put its baseline 4px into the X axis gutter
     - Location: frontend/src/v1/components/ui/chart/marks.tsx:L249
     - Fixed in 05ac6c3 (Math.min(y + LABEL_MIDLINE, bottom)). A ±20 fixture puts the tip on the edge.
143. [Medium] [Lens 3] — When the label did not fit on the right it always went left, even where left overflowed more (into the Y ticks)
     - Location: frontend/src/v1/components/ui/chart/marks.tsx:L243
     - Fixed in 05ac6c3: the label takes the side with less overflow, as labelAnchor does.
     - Tests cover a narrow chart (right side), the third of four bars (left side) and the rightmost bar (left side).
144. [Medium] [Lens 3, 16] — Two meanings of "gap": hasValues counted strings as data while the tooltip, table, summary and markers read them as gaps
     - Location: frontend/src/v1/components/ui/chart/describe.ts:L59
     - An all-string chart was "ready" with a summary of "no values".
     - Fixed in 05ac6c3: hasValues uses numberAt again. The README series row says what a gap is and to parse numeric strings first, because Recharts may still draw them. A dev warning for unparsed values is a follow-up.
145. [Medium] [Lens 13] — The beside label's vertical position and the end side's anchor were unpinned (dropping y or the "end" anchor survived)
     - Fixed in 05ac6c3: the tests assert the baseline sits between the dot's centre and 4px below it, and check the anchor on each side.
146. [Low] [Lens 16] — "The lowest bar, whose tip sits on the bottom edge" and "on whichever side it fits" overstated the code. Reworded in 05ac6c3.
147. [Low] [Lens 16] — markers.ts and BarMarker still said "over its own bar". Fixed in 05ac6c3.
148. [Low] [Lens 2] — The place type admits an x without a y. Module-local, with three construction sites; left as is.
149. [Low] [Lens 16] — The test comment's dot extent says "+6" where the ring's outer edge is 5. The assertion is stricter than needed and still sound; left as is.

Lens checklist

1. Spec & intent — Clean
2. Architectural fit & precedent — See findings #148
3. Correctness & edge cases — See findings #142, #143, #144
4. Security — Clean (security deep-dive done; orchestrator)
5. Data integrity & migrations — N/A — no persistence
6. Concurrency — N/A — synchronous rendering only
7. API & contract compatibility — Clean (no exported change)
8. Reliability & resilience — N/A — no I/O
9. Performance & cost — Clean
10. Observability — Clean
11. Operational safety — N/A — v1 not mounted
12. LLM/agent-specific — N/A — no LLM code
13. Testing — See findings #145
14. Dependencies & build — Clean (orchestrator; no dependency change)
15. Code quality — Clean (pinned Biome clean; orchestrator)
16. Doc & comment accuracy — See findings #146, #147, #149
17. Cross-document rule consistency — Clean (orchestrator)
18. On-prem deployment experience — N/A — frontend v1 only
19. Scaling & workload lifecycle — N/A — no workloads

cost: agents=8 lenses-examined=13 of 19 files=29 lines=5020

[ritwik-g]

Self-review round 25 — tip 05ac6c317 (review-deep, FOLLOWUP)

Delta under judgement: ae87a9f52..05ac6c317. It holds three changes: the least-overflow side and the clamp for bottom-edge labels, one gap rule (hasValues on numberAt), and the docs. The review is posted against head 8f5bd9a78, which holds this round's fixes. Round 26 reviews them.

Verdict — REQUEST CHANGES (fixed in 8f5bd9a).

Prior context fetched — followup-fetch.sh exited 0, with --prev-sha ae87a9f52.

Scope change — NO

Summary — Critical: 0 · High: 0 · Medium: 2 · Low: 4 · Lenses run: 19/19 (the N/A lenses give reasons below)

Security deep-dive: done (0 findings, after examination). Boundary sweep: not triggered. Pinned Biome: clean. The v1 suite passes 563/563.

The core, lifecycle and type reviews were clean. The core review confirmed that the side choice, the clamp and the single gap rule hold, and that the docs match. The test runner killed 25 of 28 mutants: one is equivalent, and the two start-branch mutants are #151.

Prior findings status

• #142 → RESOLVED in 05ac6c3. The rightward side was pinned in 8f5bd9a (#151).
• #143 → RESOLVED in 05ac6c3.
• #144 → RESOLVED in 05ac6c3. The silent side of it is #150.
• #145 → RESOLVED in 05ac6c3.
• #146, #147 → RESOLVED in 05ac6c3.

New findings

150. [Medium] [Lens 3] — With one gap rule, a payload the page forgot to parse shows "No data for this period" with nothing in the console
     - Location: frontend/src/v1/components/ui/chart/describe.ts:L60
     - Raised by the cold read and the silent-failure review.
     - Fixed in 8f5bd9a: unparsedWarnings logs one warning per series that holds a value that is not a number. It fires in any state, from CartesianChart and from Sparkline. The single gap rule stays.
     - Tests: a chart-level test covers both charts, and a unit test checks it never warns for gaps. Removing the warning from either chart fails.
151. [Medium] [Lens 13] — The rightward bottom-edge label was not pinned beside the dot or within the plot
     - Location: frontend/src/v1/components/ui/chart/marks.tsx:L253
     - Two mutants survived: y at the dot's centre, and the clamp dropped.
     - Fixed in 8f5bd9a: a strict y > cy on the narrow-chart case, plus a rightward case with the tip exactly on the edge ([-20, 20, 20, 20]). Both mutants are killed.
152. [Low] [Lens 2] — The NumericKey JSDoc lists fewer places that read a string as a gap than the README does. The two do not contradict each other. Follow-up.
153. [Low] [Lens 16] — The BarMarker JSDoc said edge labels read "as on a line", and the paragraph was badly wrapped. Rewritten in 8f5bd9a.
154. [Low] [Lens 16] — A test comment called a tip 13px above the bottom "on the bottom edge". Fixed in 8f5bd9a: "within 16px; the domain bottoms out at -45".
155. [Low] [Lens 3] — The second guard on overRight (Math.max) is an equivalent mutant. No action.

Lens checklist

1. Spec & intent — Clean
2. Architectural fit & precedent — See findings #152
3. Correctness & edge cases — See findings #150, #155
4. Security — Clean (security deep-dive done; orchestrator)
5. Data integrity & migrations — N/A — no persistence
6. Concurrency — N/A — synchronous rendering only
7. API & contract compatibility — Clean (no exported change)
8. Reliability & resilience — N/A — no I/O
9. Performance & cost — Clean
10. Observability — Clean
11. Operational safety — N/A — v1 not mounted
12. LLM/agent-specific — N/A — no LLM code
13. Testing — See findings #151
14. Dependencies & build — Clean (orchestrator; no dependency change)
15. Code quality — Clean (pinned Biome clean; orchestrator)
16. Doc & comment accuracy — See findings #153, #154
17. Cross-document rule consistency — Clean (orchestrator)
18. On-prem deployment experience — N/A — frontend v1 only
19. Scaling & workload lifecycle — N/A — no workloads

cost: agents=8 lenses-examined=13 of 19 files=29 lines=5125

[ritwik-g]

Self-review round 26 — tip 8f5bd9a78 (review-deep, FOLLOWUP)

Delta under judgement: 05ac6c317..8f5bd9a78. It adds an unparsedWarnings console warning in CartesianChart and Sparkline, pins the rightward bottom-edge label, and rewrites the docs.

Posted against head e8f4830af. That commit holds the two wording fixes below: one comment and one README sentence, with no code. They were checked by diff and by lint/Biome.

Verdict — APPROVE WITH NITS. No finding above Low; the loop ends here.

Prior context fetched — followup-fetch.sh exited 0, with --prev-sha 05ac6c317.

Scope change — NO

Summary — Critical: 0 · High: 0 · Medium: 0 · Low: 5 · Lenses run: 19/19 (the N/A lenses give reasons below)

Security deep-dive: done (0 findings, after examination). Boundary sweep: not triggered. Pinned Biome: clean. The v1 suite passes 565/565.

The core, lifecycle, type and silent-failure reviews found nothing above Low. The checks behind that:

• every unparsed value that empties a chart now warns;
• the effects re-fire only when the set of problems changes;
• hook order is intact;
• no contract changed.

The test runner killed 13 of 17 mutants. Of the survivors, one is equivalent for strings, and the other two are Lows below.

Prior findings status

• #150 → RESOLVED in 8f5bd9a. #151 → RESOLVED in 8f5bd9a. #153, #154 → RESOLVED in 8f5bd9a.

New findings

156. [Low] [Lens 16] — The Sparkline comment said the warning appears "in development", but nothing gates it. Raised by six reviewers. Fixed in e8f4830 ("in the console").
157. [Low] [Lens 16] — The README tied the warning to the empty state, but it fires whenever any value is unparsed. Fixed in e8f4830.
158. [Low] [Lens 13] — Nothing pins that the Sparkline warning logs once per change (it is keyed on a JSON string). Follow-up.
159. [Low] [Lens 13] — The CartesianChart warning in the loading state is unpinned. Follow-up.
160. [Low] [Lens 10] — The warning names the series but not which chart it came from. Follow-up.

Lens checklist

1. Spec & intent — Clean
2. Architectural fit & precedent — Clean
3. Correctness & edge cases — Clean
4. Security — Clean (security deep-dive done; orchestrator)
5. Data integrity & migrations — N/A — no persistence
6. Concurrency — N/A — synchronous rendering only
7. API & contract compatibility — Clean (unparsedWarnings internal)
8. Reliability & resilience — N/A — no I/O
9. Performance & cost — Clean
10. Observability — See findings #160
11. Operational safety — N/A — v1 not mounted
12. LLM/agent-specific — N/A — no LLM code
13. Testing — See findings #158, #159
14. Dependencies & build — Clean (orchestrator; no dependency change)
15. Code quality — Clean (pinned Biome clean; orchestrator)
16. Doc & comment accuracy — See findings #156, #157
17. Cross-document rule consistency — Clean (orchestrator)
18. On-prem deployment experience — N/A — frontend v1 only
19. Scaling & workload lifecycle — N/A — no workloads

cost: agents=8 lenses-examined=13 of 19 files=29 lines=5180

[praveen-formido]

Approving at e8f4830. The three points from Deepak's review (mixed-sign stacked bars, negative-bar marker labels, all-empty charts) each have a test, CI is green and the colours follow the LLMWhisperer design as agreed.

Non-blocking follow-ups from a read-through (not run against Recharts, so treat as things to check, not confirmed bugs):

• Hover/arrow-key moves re-render the whole chart body through the live-region setState, which re-runs the describe and table-row work; memoising on data/series would help on large series.
• A series with a constant value gives the sparkline a zero-width domain, so it may draw on the axis edge.
• Charts with only right-axis series still render the left axis and grid.
• Six or more untoned series reuse a ramp colour with no console warning.

[github-actions]

Frontend Lint Report (Biome)

✅ All checks passed! No linting or formatting issues found.

[sonarqubecloud]

Quality Gate passed

Issues
15 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[github-actions]

Unstract test results

Per-group results

Status	Group	Tier	Passed	Failed	Errors	Skipped	Duration (s)
⚪	e2e-agent-kv	e2e	0	0	0	1	0.0
✅	e2e-api-deployment	e2e	3	0	0	0	11.7
✅	e2e-coowners	e2e	1	0	0	0	1.1
✅	e2e-etl	e2e	1	0	0	0	11.8
✅	e2e-login	e2e	2	0	0	0	0.9
✅	e2e-prompt-studio	e2e	1	0	0	0	8.7
✅	e2e-smoke	e2e	2	0	0	0	0.8
✅	e2e-workflow	e2e	1	0	0	0	13.6
❌	ui	e2e	0	1	0	0	0.0
	TOTAL		11	1	0	1	48.8

Critical paths

⚠️ Critical paths not yet covered

• workflow-execution-fan-out — Multi-file workflow execution fans out to file-processing workers and rejoins. (declared coverage: no groups declared)

💤 Covered, but not exercised in this build

• adapter-register-llm — Register and validate an LLM adapter. (covered by integration-backend; no result reported in this build)
• workflow-author — Create a workflow; its source+destination endpoints materialise and are configurable. (covered by integration-backend; no result reported in this build)
• api-deployment-provision — Deploying a workflow as an API mints a usable key and a resolvable endpoint. (covered by integration-backend; no result reported in this build)
• api-deployment-auth — Unauthenticated or mis-scoped API-deployment calls are rejected before dispatch. (covered by integration-backend; no result reported in this build)
• mcp-server-auth — Unauthenticated or mis-scoped hosted-MCP calls are rejected before any tool runs. (covered by integration-backend; no result reported in this build)
• mcp-platform-auth — The org-scoped MCP endpoint stays behind the platform-API-key middleware; unauthenticated or mis-scoped calls reach no tool. (covered by integration-backend; no result reported in this build)
• platform-key-whoami — A platform API key resolves its own organisation over the org-less whoami endpoint; the org comes from the key row, not the URL. (covered by integration-backend; no result reported in this build)
• prompt-studio-author — Create a Prompt Studio project and add a prompt to it. (covered by integration-backend; no result reported in this build)
• connector-register-test — Connector credentials are validated against the live system and stored encrypted. (covered by integration-backend; no result reported in this build)
• usage-aggregate-read — Per-run token usage aggregates correctly and stays scoped to its organization. (covered by integration-backend; no result reported in this build)

✅ Covered critical paths

• auth-login — covered by e2e-login
• co-owner-manage — covered by e2e-coowners
• workflow-create-execute — covered by e2e-workflow
• api-deployment-run — covered by e2e-api-deployment
• prompt-studio-fetch-response — covered by e2e-prompt-studio
• pipeline-etl-execute — covered by e2e-etl
• usage-token-tracking — covered by e2e-api-deployment
• callback-result-delivery — covered by e2e-api-deployment