Uh oh!
There was an error while loading. Please reload this page.
fix(components,plugin-view): stop reading count and value off find() answers - #6916
Conversation
…d() answers `QueryResult` declares one rows member (`data`) and one count member (`total`). Two arms reading other keys survived objectui#6726, which measured `records` and said so. This card measures them. - `related-count-store.ts`: drop `typeof res?.count === 'number' ? res.count`, tried second and ahead of the contract's `data` — a precedence inversion. - `ObjectView.tsx`: drop the unwrap ladder's last branch, `results.value`. A pure fallback here, not an inversion; `data` was already read first. Both are raw-payload spellings that `ObjectStackAdapter.normalizeQueryResult` and `ApiDataSource.normalizeQueryResult` fold into `total`/`data` BELOW this seam. Producer sweep over all 452 `find()` definition bodies (331 files): `count` emitted 0 times, controls `total` 85 and `data` 135 from the same cells. Narrowed to the 25 bodies reachable by `ObjectView`: `value` 0, controls 6 and 6. `QueryResult` is NOT widened — published-type change, maintainer's call. One refusal pin per module, each keeping the live arms green alongside the deleted one. The `value` reading is seam-local: at `extractRecords` the same key is still live. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-sam
commented
Aug 30, 2026
PM: CI is terminal green at |
| count | |
|---|---|
| total check runs | 30 |
| success | 27 |
| skipped (never ran) | 3 |
| failure / cancelled / timed out | 0 |
| still in progress | 0 |
The three skipped are Test (coverage), Test (coverage shard …) and dependabot — conditional
jobs, NOT MEASURED rather than green, and none is a required gate. All four Test (shard N/4)
succeeded.
Why this is under clause ② at all — routed by content, over the grading
Triage graded card #6840priority:p3 with no contract-review label. I routed it here anyway, on
the diff's content rather than its label:
This PR deletes two tolerant arms that read undeclared envelope keys off a
DataSource.find()
answer. Before it, a data source emitting{ count: 7 }yielded 7; after, it yields 0.DataSourceis a published interface third parties implement, so a working — if off-contract —
integration can become a silently wrong number rather than a loud failure.
argues the opposite case well: both normalizeQueryResult implementations consume value and count
on the raw payload and return a normalized envelope, so if that fold is total, nothing off-contract
ever reaches these two read sites and the blast radius is far smaller than my routing assumed. If the
reviewer establishes that, the correct outcome is that I was over-cautious — which costs a cycle, and
I would rather pay it and be corrected than land a silent-wrong-number change on my own reading.
⇒ Not marking ready, not arming. The verdict decides, not the green.
What I have already checked myself, so the review is not re-deriving it
- ⛔ Human floor intact:
packages/types/src/data.tsis absent from the diff.QueryResultwas not
widened. - No governed surface (
docs/adr/**,.claude/**,skills/**,AGENTS.md,CLAUDE.md), and
nothing undercontent/docs/releases/. - ⭐ The consistency duty to
extractRecordsreadsrecordsAHEAD ofdataonDataSource.find()answers — the shared-helper copy of the #5945/#6726 inversion, live in 9 renderers #6839 is discharged the right way round: this PR lands first onvalue
and hands over evidence rather than a verdict — 0 producers atObjectView's seam, 5 live atextractRecords' seam — and writes the seam-local warning into the source comment at the deletion
site so the next reader cannot pick the zero up and carry it. ⛔extractRecordsreadsrecordsAHEAD ofdataonDataSource.find()answers — the shared-helper copy of the #5945/#6726 inversion, live in 9 renderers #6839 must run its own sweep.
Generated by Claude Code
os-sam
commented
Aug 30, 2026
CONTRACT_REVIEW_TIER review — head |
os-sam
commented
Aug 30, 2026
PM: clause-② verdict received — ACCEPT WITH FOLLOW-UP. One blocking item, dispatched.
⭐ I asked the reviewer to attack my own routing. It held — and now for a measured reason.I routed this to clause ② over triage's It is not total, and the reviewer showed why rather than asserting it:
⇒ The two ⛔ Routing stands. Not withdrawn. Blocking follow-up — dispatched, not waived
Two lines. A dev seat is applying it now, and I have told it to verify the precedent itself and stop What the review reproduced to the byte, so it is not taken on report
NOT MEASURED, declared: the PR's exact scanner numbers bit-for-bit (the scanner is not committed — One finding handed onward rather than acted on here
Landing once the changeset commit lands and CI is terminal on the new head. Generated by Claude Code |
Both package grades in `.changeset/6840-queryresult-count-value-arms.md` go from `patch` to `minor`. No prose change: the body asserts nothing about patch semantics, and the two frontmatter lines are the whole delta. The predecessor `.changeset/6726-find-envelope-records-arms.md` (on `main`) ships the identical change class — a tolerant arm reading an undeclared envelope key deleted from `DataSource.find()` consumers, in two of the very same modules — and grades its three packages `minor`. Same class, same grade. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
Changeset grade: |
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#6840
QueryResult(@object-ui/types) declares exactly one rows member —data— andexactly one count member —
total. #6726 removed therecordsarm from sevenDataSource.find()consumers and deliberately left two arms reading otherundeclared keys standing, because it had measured
recordsand not them. Its ownpin says so in as many words:
This PR is that deferred measurement, and only then the deletion.
1. The sweep came first, and it is this card's own — not #6726's numbers
The card's most valuable line is its self-restraint, which triage promoted to a binding
order:
So
recordsnumbers were used for nothing here. Two fresh sweeps were run, one per key.Method. The cell is the body of a
finddefinition, delimited by a realbracket scanner over comment- and string-blanked source, so a body cannot leak into
the sibling properties of its enclosing object literal. That precision is
load-bearing: a first, line-window pass reported 4
counthits, and all four turnedout to be
count: async () => 0— the fake adapter's own siblingcount()method,never an envelope key. The scanner was tightened until the number was mechanical
rather than a manual subtraction.
Subject terms and control terms are extracted from the same cells in the same
pass, so the control sits on the join — the cell the zero lives in — not merely
on the terms.
Sweep 1 —
count, repo-wide.related-count-store's probe is bound to anarbitrary
ds.find(containers.tsxhands it(object, query) => ds.find(object, query)), so the repo-wide producer set is this seam's producer set.counttotaldata452
finddefinition bodies across 331 files.Sweep 2 —
value, narrowed to the seam. The 25 bodies in the 24 files that mountplugin-view's
ObjectView:valuedatatotal2. The two named controls, and why a hot control is only half the bar
Triage named the controls rather than leaving "use a control" as a slogan. Both fired.
count→ controltotal. Hot at 85 hits / 75 files in the same pass as the zero.value→ control is the twonormalizeQueryResultimplementations below theadapter, which read
valueon the raw payload. Both hit, verbatim:packages/data-objectstack/src/index.ts:3381—const records = resultObj.records || resultObj.value || [];packages/core/src/adapters/ApiDataSource.ts:398—for (const key of ['data', 'items', 'results', 'records', 'value'])Those two reads are not merely a control — they are the mechanism. Both adapters
consume
valueandcounton the raw payload and return{ data, total, page, pageSize, hasMore }/{ data, total, hasMore, cursor }. Thekeys are folded below the seam, which is exactly why nothing above it emits them.
countis folded the same way, one line down in each file(
index.ts:3382,ApiDataSource.ts:402).Both halves, stated separately as required. A hot control proves the query ran.
It does not prove the question was right. The second half is that the control was
extracted by the same pass, from the same cells, in the same units as the zero —
totalanddataare counted as emitted envelope keys insidefindbodies, which isprecisely what
countandvaluewere counted as. Had the question been wrong (say,scanning read positions instead of emit positions), the controls would have moved
together with the subjects rather than separating 85-to-0.
3. Human floor: respected, untouched
packages/types/src/data.tsis not touched by this PR. Both sweeps returned zeroat their seam, so the "stop and report" branch was not reached; had it been, the
report would have gone back rather than a type change going in.
4.⚠️ Cross-card consistency with #6839 — and it is NOT the answer you would guess
Per 谁先动谁负责对齐, this PR lands first on
valueand therefore owes #6839 anexplicit reading:
The reading this card gives
valueis that its status is SEAM-LOCAL, and #6839 mustNOT delete on this card's zero.
The same repo-wide pass that returned 0 for
ObjectViewfindsvalueemitted 5times across all 452 bodies — and every one of them is a live test double feeding
extractRecords, which is #6839's seam:plugin-calendar/src/ObjectCalendar.markedRefusalToast.test.tsx:113ObjectCalendar→extractRecordsplugin-calendar/src/__tests__/ObjectCalendar.expandGate-6453.test.tsx:180ObjectCalendar→extractRecordsplugin-kanban/src/ObjectKanban.markedRefusalToast.test.tsx:164ObjectKanban→extractRecordsplugin-kanban/src/ObjectKanban.rejectedMoveRollback.test.tsx:128ObjectKanban→extractRecordsplugin-kanban/src/ObjectKanban.requiredWhenPrompt.test.tsx:139ObjectKanban→extractRecordsNone of the five reaches
ObjectVieworRelatedCountStore. So the two cards givingvaluedifferent treatments is not an inconsistency — it is the correct result,and the consistency duty is discharged by saying so out loud instead of letting #6839
inherit a zero that was never measured at its seam. Deleting
extractRecords'valuearm on this PR's numbers would break five tests; that is #6839's call to makeon its own sweep, and this is the evidence handed over, not a verdict.
This reading is recorded in the source comment at the deletion site too, so the next
reader of
ObjectView.tsxcannot pick the zero up and carry it.5. ⭐ Pin form, set here for #6837 to copy
This card and #6837 are one family (tolerant arm = silent absorption point), and
whoever lands first sets the refusal-pin form. This is the form, extending #6726's:
subject count, and the control counts side by side, so the pin carries its own
evidence and a later reader can tell a measured zero from an assumed one.
whole distinction.
related-count-store.contractEnvelope-6840.test.tskeepstotal,dataand the bare array green next to the two refusals; without them astore that simply stopped counting would pass the refusals too.
does NOT count 'count',does NOT read 'value'— asserting the honest zero, not an exception.countwas tried ahead of the contract'sdata, so{ count: 7, data: [a, b] }is pinned to 2.valueinObjectViewwas theladder's last branch, so no inversion exists — and the pin says so in prose
rather than fabricating a case that would pass before and after and measure
nothing. Copying the honesty matters more than copying the case list.
would try it.
6. Verification
vitest runboth new pinsplugin-viewfull suitevitest run packages/plugin-view/srccomponentsfull suitevitest run packages/components/src--filterboth packages,type-checkScope: 2 of 47 workspace projects,tsc --noEmit && tsc -p tsconfig.test.jsontsc -p tsconfig.test.json --listFilesturbo run lint)--filterboth packages,lintScope: 2 of 47, 0 errors in the diffcheck:control-bytes,check:vi-mock-specifiers,check:vi-mock-inherit,check:self-import,check:esm-specifiers,check:phantom-deps,check:element-data-source-declaration,check:side-effects-arrayThe RED-first entry is a real reading, not a formality: this change is behavioural
(a
{ count: 7 }envelope used to yield 7 and now yields 0), so the pins can be redand were.
Ablation — the fact was mutated, not the assertion. Both deleted arms were put
back on the committed tree and the pins re-run:
the mutated module by relative source path (
../hooks/related-count-store,../ObjectView) — no packageexportshop, so nodist/is involved and there isno rebuild leg that could silently leave the ablation measuring stale output.
counts (
res?.count0 → 1;(results as any).value0 → 2) andgit hash-objectdiverging from the HEAD blob on both files.
controls still green, so the pins measure this deletion and not a broken module.
git diff HEADemptyfor those two paths, and
git hash-objectequal to the HEAD blob for each(
b114e864…,ae4d44cd…).trap ... EXIT INT TERMwith absolute paths resolved fromgit rev-parse --show-toplevel, and restore pinned togit checkout HEAD -- pathrather than the bare form that would restore from a mutated index.
NOT MEASURED, reported as such rather than as a verdict:
check:sdui-registration-pinsexits 2 withPREREQUISITE NOT MET— its own message is"a run with nothing to read has measured nothing. Build the console first". It needs a
built console bundle, which this diff does not affect. Left to CI rather than reported
as either colour.
Scope of the local run, declared. The repo-wide flat
eslint .was also run andreports 4013 files in scope, 90 errors, 11644 warnings — all pre-existing; 0 of the
90 fall in this diff's four files (that flat form is a superset of the real gate,
which is
turbo run lint, per package, and passes). Type-aware linting is notenabled (no
projectService/project:ineslint.config.js), so this diff cannotmove the verdict of any file it does not touch. The remaining farm belongs to CI.
All test and gate results above were produced on a tree byte-identical to commit
6163cf9— established by the ablation's own restore proof (git diff HEADempty andhash-objectequal to the HEAD blob), and re-confirmed after the ablation with a4-file / 18-test green run on that commit.
7. Out of scope, filed rather than fixed
The sweep surfaced the same two keys surviving in six more read sites across four
packages this card does not fence. They are not touched here — filed unassigned as
#6917. Notably
packages/fields/src/index.tsx:250readsresult?.value || result?.data, i.e.valueahead of the contract's member — anactive precedence inversion, not a dead arm like the two removed here, so it is graded
separately in that issue.
Two smaller observations went into #6917 as well:
ViewDataProvider.ts:128declares asecond, private
extractRecordswith a different tolerance set from the exported oneof the same name, and two
plugin-gridfixtures return{ value: [], '@odata.count': 0 }to a component that reads
result.dataonly — inert today because the arrays are empty.Authored by Claude Code in session
https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB(recorded in prose because anedited PR body drops the session form from the footer).
Generated by Claude Code