Pin the Array.isArray limb: a third detector on the react-page adapter contract - #13989
Conversation
…etector (#13970) `ObjectStackAdapter.find()` cannot resolve to an array, so an `Array.isArray(<find result>)` limb is dead code that teaches a row shape the producer cannot emit. The shape was repaired three times (#11585 -> #13705 -> #13969) and each repair left nothing pinning it, so a reintroduction at any of the three sites got a green guard. `arrayIsArrayLimbs` is a THIRD detector, not a widening of `recordsReads`. The self-test case that pins `const records = result?.data ?? (Array.isArray(result) ? result : []);` to zero `recordsReads` findings is a false-positive control on the `.records` PROPERTY matcher — the `records` there is a local — and it is still correct. It is unchanged, byte for byte: the new detector sees the limb on that same line, and both statements are true. A subject qualifies on either route: bound from an `adapter`/`dataSource` find() in the same source, or read as `<subject>.data` on the same line. The second route is load-bearing — the renewals-pipeline repair's subject was a lambda parameter bound to no find() call anywhere. Measured over the pre-#13969 tree (bd8791f): exactly the three deleted sites across the whole population, no false positives. On `origin/main` today: zero. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pk26oZ12t5N1hwGW1m1MgC
|
ACCEPT — Zone 1.1 — the premise-first step was answered from the pin's OWN words, and it settles the forkThe dev traced
⇒ ⭐ The pin defends And the dev added the argument that closes the stronger reading, which I accept:
⭐⭐ Zone 2 A CONFIRMED — and it falsifies a sub-premise the card AND triage both statedBoth said the remedy requires editing ⇒ The thing that looked like a decision requiring a ruling was a measurement, and taking it as given would have flipped a correct false-positive control for no reason. ⛔⛔ Zone 2 C — MY relayed narrowing was WRONG, and it would have missed a real siteI passed on the #13706 dev's candidate as "subject is a I confirmed this by exercising the shipped detector directly rather than trusting either of us: ⭐ The measurements, re-run here
⭐ Reverse verification in the direction that mattersTwo legs, both expected RED, each mutation and each restore proved on disk (injected text count 1, deleted anchor 0, blob hash moved; restore proved by blob hash equal to HEAD's and an empty
⇒ Leg 2 is the card's exact scenario — "someone re-adding it gets a green guard" — and it is now red. That is the class question answered, and it is why a delivery that only counted deletions would not have been acceptable. Honest boundaries the gate publishes rather than leaves to be discoveredThe unrecognised binding spellings (destructuring, Zone 1.3 / 1.4 held⛔ The unreachable nullish guards were not touched — the dev correctly declined to file a card for them too, since the finding is already recorded on #13970 itself and routed to the enforce-or-remove lane. ⛔ The guard's file scope was reported, not changed: 21 page modules under Gates and hygiene13 path-derived families + 2 convention-triggered obligations; derived minus run is empty; exit codes captured before any pipe. ⭐ And one run beyond the derivation, because the derivation cannot see it:
An observation the dev recorded and did NOT act on, which I endorseAfter #13969, Governed-surface checkDiff: one file, Generated by Claude Code |
Fixes #13970
ObjectStackAdapter.find()cannot resolve to an array, so anArray.isArray(SUBJECT)test on a find() result is dead code. What it costs is what the
?? .recordsalias cost:it teaches an author — and a coding assistant reading the page as a sample — a row shape
the producer cannot emit. The shape was repaired three times (#11585, then #13705, then
#13969) and every repair left nothing pinning it, so a reintroduction at any of the
three sites got a green guard.
This adds
arrayIsArrayLimbs, a third detector inscripts/check-react-page-adapter-contract.mjs, and the self-test cases that pin it.The pin is the deliverable; the deletion count is zero on purpose (see B below).
1. Premise-first — why
:594was pinned to zero, established before any editThe card and the triage ruling both required this to be answered before touching the
guard. It was introduced by
789aa6867(PR #13705, "Delete the tolerant?? recordsaliases and narrow the guard that blessed them"), inside a block whose own banner reads
The narrowing: a .data read BESIDE it is not an exemption, and its assertion message is:What pinning it to zero was defending: PR #13705 replaced
recordsReads's.data-beside carve-out withcodeOnly()plus a whole-property matcher(
RECORDS_READ = /\??\.\s*records\b/). That narrowing made the detector strictly wider,so the same edit added its false-positive controls.
:594is one of them, and itssiblings are the same family —
result.recordsCountis not a read, a.recordsinside astring is not a read.
:594pins that the bare identifierrecordsinconst records = ...is not a.recordsproperty read.So the pin is a statement about
recordsReads's property matcher, not a ruling thatthis line must never be flagged by anything. Two independent facts settle the stronger
reading the card asked about:
Array.isArraylimb at all threeadapter.find()sites #13969 subsequently deleted that exact line from the tree as an unreachable limb. Aline the repo has ruled should not exist cannot also be a line the repo has ruled must
never be flagged.
Verdict: the reason does not extend to a third detector, so this is the implement
branch, not the fork branch. And
recordsReads's pin is still correct on its own terms,so it stays.
2.
:594is unchanged, byte for byteThe card, the triage ruling and the dispatch order all assumed the remedy requires
editing that line. It does not. Because the new detector is a separate function,
recordsReadson that same string still returns 0 and the assertion is true as written.git difftouches no existing assertion. The self-test now states both facts about thatone string, side by side:
3. What the detector matches
A subject qualifies on either route, and each finding says which one:
adapter/dataSourcefind()/findOne()in the same source; orSUBJECT.data/SUBJECT?.datais read on the same line — the envelope read iswhat identifies it as a find() result.
Route 2 is load-bearing, not a nicety. The renewals-pipeline repair was
(res) => (Array.isArray(res) ? res : (res && res.data) || []), whose subject is alambda parameter bound to no
find()call anywhere; the narrowing proposed on thecard ("a find() result on an
adapter/dataSourceidentifier") misses it if read as asame-line binding. Route 1 is equally load-bearing in the other direction: it catches a
reintroduction spelled
Array.isArray(all) ? all : []with no.datalimb at all.Array.isArrayis ordinary JavaScript and a page may narrow any other value — neitherroute fires without adapter provenance. The known exclusion (an ObjectQL
engine.findreally does resolve to an array-or-envelope union, and app-showcase reads exactly that
shape in its job runtime) is stated in the function header and pinned as a self-test case,
rather than left to be discovered later.
4. Measurements, including the ones that could have falsified this
Population today is 0 — hypothesis confirmed, the card's "clean today" claim holds.
The whole swept population on
origin/main(21 app-showcase page modules + 1content/docssample) yields 0 findings from the new detector. The only twoArray.isArrayoccurrences in it are comment lines:crm-workbench.page.ts:43(the note#13969 left explaining why the limb is unreachable) and
contact-form.page.ts:120(anunrelated design note). Both are pinned as negative cases.
Over the pre-#13969 tree (
bd8791fd8) it reports exactly the three deleted sites, andnothing else — run by pointing
collectSources()at a comparison worktree at thatcommit:
False positives 0, false negatives 0 over 15 hand-built cases plus the two whole-tree
sweeps above: the three self-test fixture strings, the three real lines #13969 deleted,
the three repaired shapes as they stand today, both comment lines in the tree, a
string-only spelling, an
engine.findunion, a bare narrowing on a non-adapteridentifier, and a binding-route reintroduction.
Reverse verification, both legs, mutation and restore each proven on disk (blob hash
compared against the HEAD blob; no build step exists for this script —
nodereads thesource directly):
Array.isArraylimb at all threeadapter.find()sites #13969 line in the real tree (crm-workbench.page.ts) reds thegate itself, exit 1, one finding naming the line — the exact scenario the card
reports as getting a green guard today.
5. Coverage — what this actually covers, stated rather than implied
The guard's file scope is unchanged by this PR, and it is not this card's axis
(#10751 owns it). Stating it so the new detector's reach is checkable rather than assumed:
*.page.ts/*.pages.tsunderexamples/app-showcase/src/**— 21 today;content/docs/**(excludingcontent/docs/releases/) that holduseAdapter(or anadapter/dataSourcefind/findOne— 1 today, from 396 docfiles and 1947 fenced blocks.
Nothing outside those two populations is swept, by this detector or by the other two. The
Array.isArrayuses elsewhere inexamples/app-showcase(automation/jobs/,security/) read an ObjectQLctx.ql.findresult, which is a different contract theguard deliberately excludes; they are not defects of this class and are untouched.
6. Self-test assertion count
37 before, 51 after (+14). Counts come from the script's own verdict line.
Gates
Derived with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackat016126ed2(the head this PR opens on) — 13 path-derived families plus the 2convention-triggered obligations for editing a gate script. Derived-minus-run is empty.
All green except one that measured nothing and says so itself:
check:agent-test-spelling,check:bash32-floor,check:cli-command-ids,check:cross-package-test-inputs,check:entry-guard,check:parse-guard,check:pnpm-filter-targets,check:react-page-adapter-contract,check:watch-hint-literal,check-ci-filter-parity.mjs,check-cross-package-test-inputs.mjs,check-shard-attestation.mjs,scripts/pm/bare-root-worklist.mjs --self-test,check:pm-dispatch-gates;check-test-completeness.mjsexits 3 withPREREQUISITE NOT MET— it grades a savedturbo run testlog and none exists locally.Its own text says this is not a red and there is nothing to fix.
Beyond the derivation:
scripts/check-docs-section-name.mjsimportsfencedBlocksfromthe edited module, so it was run as a consumer —
--self-testgreen (54 cases) and thecorpus run green. The sibling suite
examples/app-showcase/test/react-page-adapter-query-contract.test.tspasses (5 tests)after building the dependency closure.
Repo-wide ESLint (
pnpm lint,eslint . --no-inline-config) was run in full rather thannarrowed: exit 0.
Everything above ran at
016126ed2, which is this PR's head.Scope and housekeeping
scripts/**matches no workspace publish glob andthe root package is private, so this carries the
skip-changesetlabel instead of achangeset.
docs/adr/** · .claude/** · skills/** · AGENTS.md · CLAUDE.md; this diff touchesscripts/only.unreachable nullish guards at
crm-workbench.page.ts:55and the(res && res.total)form in
renewals-pipeline.page.ts(a different class), and the guard's file scope(that is [finding] the #10288 react-page adapter-contract guard sweeps only app-showcase pages, so the same
.recordsread survived a third time in content/docs #10751's axis).Generated by Claude Code
Generated by Claude Code