You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.
Context
ChartTooltipContent renders each series' value behind a truthiness check:
item.value is recharts' TooltipValueType — a number, a string, or an array. When it is the number 0, 0 && <span/> evaluates to 0, and React renders the number0 as a raw text node in place of the
element. Two things go wrong at once:
The value loses font-mono font-medium tabular-nums text-foreground. In a tooltip whose whole point is tabular-nums alignment, one row renders in the surrounding body font while its siblings render monospaced.
It bypasses toLocaleString(), so it is not the formatted value the component promises — the same code path
that turns 1200 into 1,200 is skipped.
An empty-string value ("") hits the same branch and renders nothing at all, which is at least silent; 0 is
the visible one. Zero is an ordinary data point for both live consumers — apps/loopover-miner-ui/src/routes/ledgers.tsx and apps/loopover-miner-ui/src/routes/portfolio.tsx both
render ChartTooltipContent over count/earnings series where a zero bucket is normal.
packages/loopover-ui-kit/src/components/chart.test.tsx was added by #9937 precisely because chart.tsx
"shipped with no tests at all", but its payload fixtures use 120 and 80 only — the zero case is untested.
Requirements
A numeric item.value of 0 must render inside the same <span className="font-mono font-medium tabular-nums text-foreground"> as any other number, with its text produced by toLocaleString() (so the
rendered text is "0", inside that span).
The value element must be omitted only when there is genuinely no value — item.value === undefined or item.value === null. Every other value, including 0 and "", must render through the span.
The guard must be an explicit nullish check, not a truthiness check, so no falsy-but-real value can ever
again fall through as a raw React child.
The custom-formatter branch at packages/loopover-ui-kit/src/components/chart.tsx:212 must NOT change: it
already guards on item?.value !== undefined && item.name, and a formatter supplied by the caller still
takes precedence over the default rendering.
No other part of the row (the indicator, the icon, the itemConfig?.label || item.name series label, the nestLabel layout) may change.
⚠️ Required pattern: replace the && guard at packages/loopover-ui-kit/src/components/chart.tsx:252 with
a nullish-aware conditional, mirroring the item?.value !== undefined check the sibling formatter branch
at line 212 already uses. What does NOT satisfy this issue: (a) {item.value != null && (...)} written as a
loose != without a test pinning both 0 and undefined, which is the same one-character-away mistake;
(b) coercing with String(item.value ?? "") and always rendering the span, which makes an
absent value render an empty monospaced box; (c) changing toLocaleString() to a hand-rolled formatter;
(d) a repo-wide sweep of every {x && <jsx/>} in the ui-kit — this issue is chart.tsx's tooltip value
only; (e) a test-only PR.
Deliverables
Rendering <ChartTooltipContent active payload={[{ dataKey: "revenue", name: "revenue", value: 0, color: "#0ea5e9", payload: { month: "Jan", revenue: 0 } }]} /> inside a ChartContainer produces a <span> whose className contains tabular-nums and whose text is "0", asserted in packages/loopover-ui-kit/src/components/chart.test.tsx.
Rendering the same payload entry with value: undefined produces no value <span> at all (the series
label still renders), asserted in the same file.
Rendering the same payload entry with value: "" produces the value <span> with empty text, asserted
in the same file.
The existing "renders one row per payload entry, labelled from the chart config" test (values 120 / 80) still passes unmodified.
A regression test at packages/loopover-ui-kit/src/components/chart.test.tsx named for this bug that
asserts the zero value is NOT a direct text child of the row container — i.e. that the rendered 0 sits
inside the formatted span, not beside it.
All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example one
that changes the guard and asserts only that the text "0" appears somewhere (which the buggy code also
satisfies) — does not resolve this issue.
Test Coverage Requirements
This repo enforces 99%+ Codecov patch coverage, branch-counted. vitest.config.ts's coverage.include
covers src/**/*.ts, packages/loopover-engine/src/**/*.ts, packages/loopover-{miner,mcp}/{lib,bin}/**/*.ts, packages/loopover-contract/src/**/*.ts and packages/discovery-index/src/**/*.ts. It does not cover packages/loopover-ui-kit/**, and that package's own packages/loopover-ui-kit/vitest.config.ts deliberately
declares no coverage block. Codecov does not gate the patch on this change.
The tests are still mandatory and are named above: they go in the existing packages/loopover-ui-kit/src/components/chart.test.tsx and must be run with npm --workspace @loopover/ui-kit run test before pushing (that suite is not currently reached by npm run test:ci, so a local run is the only way to see it pass).
Both arms of the changed guard need a test in that file: value present (0, "", and a non-zero number all
render the span) and value absent (undefined renders nothing). The sibling formatter branch's two arms must
also stay covered — the existing "routes the value through a custom formatter" test covers the formatter arm;
add or keep an assertion for the no-formatter arm with a zero value so the two branches cannot be conflated.
Expected Outcome
A chart data point of 0 shows as a properly formatted, monospaced, tabular-aligned 0 in the tooltip like
every other value, instead of an unstyled bare digit that breaks column alignment and skips toLocaleString().
Links & Resources
packages/loopover-ui-kit/src/components/chart.tsx:252-256 — the truthiness guard
packages/loopover-ui-kit/src/components/chart.tsx:212 — the sibling formatter branch's nullish check
packages/loopover-ui-kit/src/components/chart.test.tsx — the existing suite (fixtures 120/80 only)
apps/loopover-miner-ui/src/routes/ledgers.tsx, apps/loopover-miner-ui/src/routes/portfolio.tsx — the
two live consumers
Context
ChartTooltipContentrenders each series' value behind a truthiness check:item.valueis recharts'TooltipValueType— anumber, astring, or an array. When it is the number0,0 && <span/>evaluates to0, and React renders the number0as a raw text node in place of theelement. Two things go wrong at once:
font-mono font-medium tabular-nums text-foreground. In a tooltip whose whole point istabular-numsalignment, one row renders in the surrounding body font while its siblings render monospaced.toLocaleString(), so it is not the formatted value the component promises — the same code paththat turns
1200into1,200is skipped.An empty-string value (
"") hits the same branch and renders nothing at all, which is at least silent;0isthe visible one. Zero is an ordinary data point for both live consumers —
apps/loopover-miner-ui/src/routes/ledgers.tsxandapps/loopover-miner-ui/src/routes/portfolio.tsxbothrender
ChartTooltipContentover count/earnings series where a zero bucket is normal.packages/loopover-ui-kit/src/components/chart.test.tsxwas added by #9937 precisely becausechart.tsx"shipped with no tests at all", but its payload fixtures use
120and80only — the zero case is untested.Requirements
item.valueof0must render inside the same<span className="font-mono font-medium tabular-nums text-foreground">as any other number, with its text produced bytoLocaleString()(so therendered text is
"0", inside that span).item.value === undefinedoritem.value === null. Every other value, including0and"", must render through the span.again fall through as a raw React child.
formatterbranch atpackages/loopover-ui-kit/src/components/chart.tsx:212must NOT change: italready guards on
item?.value !== undefined && item.name, and aformattersupplied by the caller stilltakes precedence over the default rendering.
itemConfig?.label || item.nameseries label, thenestLabellayout) may change.Deliverables
<ChartTooltipContent active payload={[{ dataKey: "revenue", name: "revenue", value: 0, color: "#0ea5e9", payload: { month: "Jan", revenue: 0 } }]} />inside aChartContainerproduces a<span>whoseclassNamecontainstabular-numsand whose text is"0", asserted inpackages/loopover-ui-kit/src/components/chart.test.tsx.value: undefinedproduces no value<span>at all (the serieslabel still renders), asserted in the same file.
value: ""produces the value<span>with empty text, assertedin the same file.
120/80) still passes unmodified.packages/loopover-ui-kit/src/components/chart.test.tsxnamed for this bug thatasserts the zero value is NOT a direct text child of the row container — i.e. that the rendered
0sitsinside the formatted span, not beside it.
All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example one
that changes the guard and asserts only that the text
"0"appears somewhere (which the buggy code alsosatisfies) — does not resolve this issue.
Test Coverage Requirements
This repo enforces 99%+ Codecov patch coverage, branch-counted.
vitest.config.ts'scoverage.includecovers
src/**/*.ts,packages/loopover-engine/src/**/*.ts,packages/loopover-{miner,mcp}/{lib,bin}/**/*.ts,packages/loopover-contract/src/**/*.tsandpackages/discovery-index/src/**/*.ts. It does not coverpackages/loopover-ui-kit/**, and that package's ownpackages/loopover-ui-kit/vitest.config.tsdeliberatelydeclares no
coverageblock. Codecov does not gate the patch on this change.The tests are still mandatory and are named above: they go in the existing
packages/loopover-ui-kit/src/components/chart.test.tsxand must be run withnpm --workspace @loopover/ui-kit run testbefore pushing (that suite is not currently reached bynpm run test:ci, so a local run is the only way to see it pass).Both arms of the changed guard need a test in that file: value present (
0,"", and a non-zero number allrender the span) and value absent (
undefinedrenders nothing). The siblingformatterbranch's two arms mustalso stay covered — the existing "routes the value through a custom formatter" test covers the formatter arm;
add or keep an assertion for the no-formatter arm with a zero value so the two branches cannot be conflated.
Expected Outcome
A chart data point of
0shows as a properly formatted, monospaced, tabular-aligned0in the tooltip likeevery other value, instead of an unstyled bare digit that breaks column alignment and skips
toLocaleString().Links & Resources
packages/loopover-ui-kit/src/components/chart.tsx:252-256— the truthiness guardpackages/loopover-ui-kit/src/components/chart.tsx:212— the siblingformatterbranch's nullish checkpackages/loopover-ui-kit/src/components/chart.test.tsx— the existing suite (fixtures120/80only)apps/loopover-miner-ui/src/routes/ledgers.tsx,apps/loopover-miner-ui/src/routes/portfolio.tsx— thetwo live consumers