fix(lint): validateChartBindings resolves a report's dataset and rows/columns whether or not it has a chart - #16397
Conversation
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8
…/columns whether or not it has a chart `validateChartBindings` reached a report through one closure whose first line was `if (!isRec(chart)) return`, and that closure was the only site that ever received `report.dataset`. A report authored without a chart was therefore not checked at all, and `rows` / `columns` were never resolved against the dataset on any report — on one and the same report object the measure selection was validated and the dimension selection beside it was not. Dataset resolution is lifted out of the chart closure into `resolveDataset`, called once per report and once per block before the chart question is asked. The resolved dataset is then fed to two groups of positions: the report's own selection (`rows` / `columns` -> `chart-dimension-unknown`, `values` -> `chart-measure-unknown`) and, when a chart is present, its axis refs exactly as before. One path entered unconditionally, not a second pass after the early return, so an unresolvable dataset is still exactly one finding. No new rule id and no severity moved. The report dataset finding now points at `reports[i].dataset` rather than the position `reports[i].chart.dataset`, which a report does not have, and its sentence no longer names a chart the report may not draw. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8
📓 Docs Drift Check3 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. What this run could not see
Coarse fallback — 5 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin d79370c03cf15d1fb66f5165d58433a048ea3a01 && git checkout d79370c03cf15d1fb66f5165d58433a048ea3a01
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 4998efa71773154561c471075f4ef12566ecc455 081938a3c2f079b517ec96b2ef5ac48a70a3f771 && git checkout -B drift-repro 4998efa71773154561c471075f4ef12566ecc455 && git merge --no-ff 081938a3c2f079b517ec96b2ef5ac48a70a3f771
node scripts/docs-audit/affected-docs.mjs --json 4998efa71773154561c471075f4ef12566ecc455 |
Fixes #16105
What was wrong
validateChartBindingsreached a report through one closure,checkReportChart, whose first line wasif (!isRec(chart)) return;. That closure was the only site that ever receivedreport.dataset, and it was called withdataset,values,xAxis,yAxis,ownSelectionandseries— never with the report's dimension selection. Two gaps followed, both reproduced here:chartwas not checked at all. Itsdatasetwas never resolved, so a binding to a dataset that does not exist published clean. Itsvaluesmeasures were invisible for the same reason.rows/columnswere resolved on no report, charted or not. On one and the same charted report object the measure selection was resolved against the dataset and the dimension selection beside it was not.Premise check, before the first edit
grep -n 'rows\|columns' packages/lint/src/validate-chart-bindings.tsonorigin/mainreturns 9 lines — 12, 18, 31, 230, 278, 291, 305, 345 and 371 — every one of them prose in the module docblock or in a message string about ADR-0021 result rows. No code path forwardsreport.rows/report.columns. Control on the same file:valuesoccurs 33 times, and those are code.:510has no defect of the same shape, and here is why.checkListCharttakes the dataset FROM the chart (chart.dataset), not from the container, so a container without a chart has no dataset binding to resolve. That is a fact about the schema, not a reading of the walk: inpackages/spec/src/ui/view.zod.tsthe stringdatasetoccurs at exactly one declaration site, line 1275, insideListChartConfigSchema, where it is REQUIRED.ListViewShapeSchema(line 1805, the container that carrieschart: ListChartConfigSchema.optional()at 1923) declares nodatasetof its own; a grep fordatasetover lines 1600 to 2050 returns zero. The dataset-bound page-component surface is the same story —properties.datasetis what marks a component checkable at all. So the:510return skips a container that binds nothing, and no lift applies to it. Not touched.ReportSchemadeclaresrowsas "Dimension names (from the dataset) to group rows by (down axis)" andcolumnsas "Dimension names across (ADR-0021 D2)", bothz.array(z.string());valuesis "Measure names (from the dataset) to display".checkReportOrderin the same file already treats the three exactly that way — anorderkey must name "arows/columnsdimension or avaluesmeasure". There is no schema-permitted derived or bucketed key at these positions, sochart-dimension-unknownis not being widened onto a shape the spec allows, and there is no ambiguity left to record.validate-chart-bindings.*;git ls-remote --heads origin 'claude/issue-16105*'named only this branch.The shape
Dataset resolution is lifted out of the chart closure into
resolveDataset, called once per report and once per block BEFORE the chart question is asked. The resolved dataset is then handed to two groups of positions:rowsandcolumnstochart-dimension-unknown,valuestochart-measure-unknown;xAxis,yAxis,ownSelection,series;chart-axis-not-selectedstays awarningand still resolves against the chart's ownchart.yAxis).One path entered unconditionally, with the chart as the branch it always was — not a second "chartless reports too" pass after the early return. That is also what keeps an unresolvable dataset ONE finding:
resolveDatasetruns once per report surface, not once per group. Blocks of ajoinedreport carry the same keys and go through the same helper, one shape applied twice.ChartBinding.dimensionsbecomes a LIST of selections, because a report has two and the list-view and page surfaces have one. Each entry keeps its own path, so a finding namesreports[i].columns[j]rather than a position in a merged list nobody wrote.No new rule id, no severity moved. Two message corrections ride along, both stated in the changeset:
reports[i].dataset, the key the author wrote — it used to sayreports[i].chart.dataset, a position a report does not have and a chartless report cannot have;Tests
Head sha for every reading below:
081938a3c.Four pins mirroring the card's four injections, each reading the findings ARRAY (rule id, path, severity) rather than an exit code —
chart-axis-not-selectedis awarningand changes no exit code, so a pass/fail assertion could not tell the tiers apart. Two of the four are the card's working controls, pinned so a later refactor cannot break the charted path while the new chartless assertions stay green.pnpm --filter @objectstack/lint exec vitest run --maxWorkers=2 src/validate-chart-bindings.test.ts— 45 passed (35 before this change, all still green).datasetrenamed: exactly onechart-dataset-unknownaterror, pathreports[0].dataset, hint offers the real name.rowson a charted report,columnson a chartless matrix,rowson a chartless report:chart-dimension-unknownaterror, pathsreports[0].rows[0]/reports[0].columns[0].values(the same entrance, one collection over) and ajoinedblock'sdataset+rowswith no chart on the block.datasetrenamed: still gates, and exactly ONE finding — no double report after the lift.valuesrenamed:chart-measure-unknownaterrorpluschart-axis-not-selectedatwarning, in that order, with exactly oneerrorin the array.joinedcontainer, which binds no dataset of its own.Fixture re-scope.
it('ignores a report with no chart')asserted a zero that held only because chartless reports were invisible. Its fixture is in fact CLEAN — dataset resolves,statusis a declared dimension,task_counta declared measure — so the zero survives for a real reason. The test is renamed to say what it now reads ("says nothing about a chartless report whose every binding resolves") with a comment recording the re-scope. No assertion was weakened and nothing was deleted.Ablation (implementation committed first; mutation and restore both proven on disk by blob hash, restore leg proven by an empty
git diff HEAD):git checkout 0374bcba9 -- packages/lint/src/validate-chart-bindings.ts, keeping the new tests. On-disk anchors before/after:checkReportSurface3 to 0,resolveDataset4 to 0,checkReportChart0 to 3. Disk blob equals the base blob, so the mutation landed.expected 'reports[0].chart.dataset' to be 'reports[0].dataset'. Its control substance (one finding,chart-dataset-unknown,error) asserted BEFORE the path and held on the old shape too. Reported as measured rather than as a discrimination the lift produced.git diff HEADempty for the target. No rebuild was needed for either leg and none was claimed: the test imports the subject by relative path within its own package, so vitest resolvessrc, not a dependency'sexportstodist;packages/lint/vitest.config.tssets one unrelated key and no alias.Consumers.
@objectstack/lintwas rebuilt (packages/lint/dist/index.jscarriescheckReportSurface) and every DIRECT dependent was run:@objectstack/example-showcase28 files,@objectstack/metadata-protocol166 files, plus@objectstack/mcp,@objectstack/platform-objects,@objectstack/cloud-connection— all green through the shared verify lock (VERDICT command-exit 0).@objectstack/cliunit tier: 181 files, 2453 passed, 6 expected fail (integration tier declared to CI). Full@objectstack/lintsuite: 100 files, 3410 passed, 5 skipped.pnpm --filter @objectstack/lint typecheck:VERDICT command-exit 0.The examples emit no new findings, and that zero has a control.
pnpm --filter @objectstack/example-showcase --filter @objectstack/example-todo validatepasses on both — everyrows/columns/valuesname in both apps' reports is declared by the bound dataset. Positive control, injected intoexamples/app-showcase/src/ui/reports/index.tsand proven on disk, then restored by blob hash: renaming the matrix report'srowsandcolumnsmakesobjectstack validateexit 1 withThat report declares no chart, so this is the card's Fact 2 measured end to end through
os validateon a real app.Gates
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackderived 54 families; all 54 were run and all exit 0. Reconciliation:54 derived, 54 run, 0 NOT-MEASURED, 0 UNRUN. Re-derived aftergit fetch origin mainmovedorigin/mainto4998efa71— the family list is byte-identical, so no family was missed by the older tree.Two needed a second run and neither was a finding:
pnpm check:dual-build-cjs-loadsfirst exited 3,PREREQUISITE NOT MET(seven packages had nodist/). Afterpnpm build, exit 0 — 103 require entry points across 66 packages load, 619 emitted CJS files parse.pnpm check:type-check-debtexited 124 under a 300s harness timeout, then 3 twice (PREREQUISITE NOT MET, tsc OOM). The gate prints the cause: it re-measures under the caller'sNODE_OPTIONS, this machine's default is--max-old-space-size=2096and the gate's own pinned CI-shaped ceiling is 6144. At 6144 it exits 0 — 5 ledger entries re-measured in 90.8s, 55 raw tsc errors, none above its recorded number.Repo-wide
pnpm lint(eslint . --no-inline-config) exits 0, so no narrowing is claimed or needed. Control-character self-scan over the three changed files: no match.Scope
Three files, no new rule id, no doc transcript moved (
pnpm check:docs-transcript-driftgreen). No out-of-scope findings were filed — nothing outside this card's defect class turned up.Card 15734 (which set
chart-axis-not-selectedresolves against on the report surface) and card 15462 (the widget-side empty-selection gap) both presume the chart entrance is taken; neither is addressed here. hotcrm card 1621'stest/analytics-integrity.test.tsassertion stays where it is — retiring it is a decision for that repo once this lands.Generated by Claude Code