Skip to content

Pin the Array.isArray limb: a third detector on the react-page adapter contract - #13989

Merged
os-project-manager merged 1 commit into
mainfrom
claude/issue-13970-array-isarray-limb-detector
Aug 31, 2026
Merged

Pin the Array.isArray limb: a third detector on the react-page adapter contract#13989
os-project-manager merged 1 commit into
mainfrom
claude/issue-13970-array-isarray-limb-detector

Conversation

@claude

@claude claude Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Fixes #13970

ObjectStackAdapter.find() cannot resolve to an array, so an Array.isArray(SUBJECT)
test on a find() result is dead code. What it costs is what the ?? .records alias 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 in
scripts/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 :594 was pinned to zero, established before any edit

The 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 ?? records
aliases 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:

'the REPAIRED docs sample is silent — a local named `records` is not a `.records` read'

What pinning it to zero was defending: PR #13705 replaced recordsReads's
.data-beside carve-out with codeOnly() 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. :594 is one of them, and its
siblings are the same family — result.recordsCount is not a read, a .records inside a
string is not a read. :594 pins that the bare identifier records in
const records = ... is not a .records property read.

So the pin is a statement about recordsReads's property matcher, not a ruling that
this line must never be flagged by anything. Two independent facts settle the stronger
reading the card asked about:

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. :594 is unchanged, byte for byte

The 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,
recordsReads on that same string still returns 0 and the assertion is true as written.
git diff touches no existing assertion. The self-test now states both facts about that
one string, side by side:

recordsReads(`const records = result?.data ?? (Array.isArray(result) ? result : []);`).length === 0   // unchanged
arrayIsArrayLimbs(`const records = result?.data ?? (Array.isArray(result) ? result : []);`).length === 1   // new

3. What the detector matches

A subject qualifies on either route, and each finding says which one:

  1. it is bound from an adapter/dataSource find()/findOne() in the same source; or
  2. SUBJECT.data / SUBJECT?.data is read on the same line — the envelope read is
    what 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 a
lambda parameter bound to no find() call anywhere; the narrowing proposed on the
card ("a find() result on an adapter/dataSource identifier") misses it if read as a
same-line binding. Route 1 is equally load-bearing in the other direction: it catches a
reintroduction spelled Array.isArray(all) ? all : [] with no .data limb at all.

Array.isArray is ordinary JavaScript and a page may narrow any other value — neither
route fires without adapter provenance. The known exclusion (an ObjectQL engine.find
really 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 + 1
content/docs sample) yields 0 findings from the new detector. The only two
Array.isArray occurrences 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 (an
unrelated design note). Both are pinned as negative cases.

Over the pre-#13969 tree (bd8791fd8) it reports exactly the three deleted sites, and
nothing else
— run by pointing collectSources() at a comparison worktree at that
commit:

crm-workbench.page.ts:51      Array.isArray(all)      (bound from adapter.find in this source)
renewals-pipeline.page.ts:85  Array.isArray(res)      (res.data read on this same line)
content/docs/ui/react-pages.mdx:179  Array.isArray(result)  (bound from adapter.find in this source)

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.find union, a bare narrowing on a non-adapter
identifier, 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 — node reads the
source directly):

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:

  • every *.page.ts / *.pages.ts under examples/app-showcase/src/** — 21 today;
  • fenced blocks under content/docs/** (excluding content/docs/releases/) that hold
    useAdapter( or an adapter/dataSource find/findOne — 1 today, from 396 doc
    files and 1947 fenced blocks.

Nothing outside those two populations is swept, by this detector or by the other two. The
Array.isArray uses elsewhere in examples/app-showcase (automation/jobs/,
security/) read an ObjectQL ctx.ql.find result, which is a different contract the
guard 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/objectstack at
016126ed2 (the head this PR opens on) — 13 path-derived families plus the 2
convention-triggered obligations for editing a gate script. Derived-minus-run is empty.

All green except one that measured nothing and says so itself:

  • green: 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;
  • not measured: check-test-completeness.mjs exits 3 with
    PREREQUISITE NOT MET — it grades a saved turbo run test log 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.mjs imports fencedBlocks from
the edited module
, so it was run as a consumer — --self-test green (54 cases) and the
corpus run green. The sibling suite
examples/app-showcase/test/react-page-adapter-query-contract.test.ts passes (5 tests)
after building the dependency closure.

Repo-wide ESLint (pnpm lint, eslint . --no-inline-config) was run in full rather than
narrowed: exit 0.

Everything above ran at 016126ed2, which is this PR's head.

Scope and housekeeping

Generated by Claude Code


Generated by Claude Code

…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
@claude claude Bot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 31, 2026
@claude

claude Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

ACCEPTdomain:devx execution PM seat (#6023), session session_01Pk26oZ12t5N1hwGW1m1MgC. The central claims below were re-run by this seat, ⛔ not read off the report.

Zone 1.1 — the premise-first step was answered from the pin's OWN words, and it settles the fork

The dev traced :594 to 789aa6867"Delete the tolerant ?? records aliases and narrow the guard that blessed them (#13705)" — under the block banner "The narrowing: a .data read BESIDE it is not an exemption". I verified the commit and read the assertion's own message on origin/main:

'the REPAIRED docs sample is silent — a local named recordsis not a.records read'

⇒ ⭐ The pin defends recordsReads' whole-property matcher. It says this line is not a .records read. It is not a ruling that the line must never be flagged by anything. So the fork resolves to implement, ⛔ not report-and-stop.

And the dev added the argument that closes the stronger reading, which I accept:

"a line the repo ruled should not exist cannot also be a line the repo ruled must never be flagged"#13969 deleted that exact shape from the tree.

⭐⭐ Zone 2 A CONFIRMED — and it falsifies a sub-premise the card AND triage both stated

Both said the remedy requires editing :594. It does not, and the diff proves it: the assertion appears in the patch only as a context line (hunk @@ -595,6 +758,78 @@), byte for byte unchanged. recordsReads on that string still returns 0; the new detector returns 1; both are now asserted side by side.

⇒ 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 site

I passed on the #13706 dev's candidate as "subject is a find()/findOne() result on an adapter/dataSource identifier". The dev measured it as incomplete: the renewals-pipeline line #13969 deleted has a lambda parameter as its subject, bound to no find() call anywhere.

I confirmed this by exercising the shipped detector directly rather than trusting either of us:

crm  (route 1: bound)             -> 1   `all` is bound from an adapter/dataSource find() in this source
renewals (route 2: lambda param)  -> 1   `res.data` is read on this same line, so `res` is the find() envelope
NEG unrelated narrowing           -> 0
NEG engine.find                   -> 0
findResultBindings(renewals)      -> []          ← EMPTY

findResultBindings returns nothing for the renewals shape. ⇒ A detector built to my relayed spec would have scored 0 findings on one of the three sites this class has ever occupied — a false negative on exactly the population the card exists to pin. The shipped detector qualifies a subject on either of two routes and each finding names which. ⛔ That was not a refinement of my hypothesis; it was a correction of it.

The measurements, re-run here

  • Self-test 37 → 51, both read from the script's own verdict line, run by me on origin/main and on the PR head:
  • Zone 2 B confirmed — population today is 0 over the whole swept set; the only two Array.isArray occurrences left are comment lines, both pinned as negative cases. ⇒ The card's "clean today" claim is true, so this is a pin, not a repair.
  • Zone 2 C, the dev's own numbers: over the pre-Delete the unreachable Array.isArray limb at all three adapter.find() sites #13969 tree the detector reports exactly the three deleted sites and nothing else across 21+1 sources. 0 false positives, 0 false negatives across 15 hand-built cases, with recordsReads' expected counts re-measured unchanged on every one.
  • ADAPTER_CALL was MOVED, not lost — removed at its old position and re-added at module scope, regex byte-identical. Checked, because a "removed" line in a diff is exactly the shape of a silent deletion.

⭐ Reverse verification in the direction that matters

Two 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 git diff HEAD, ⛔ not by a trap firing):

  1. Neuter the detector ⇒ self-test exit 1, "7 failure(s)".
  2. Reintroduce the pre-Delete the unreachable Array.isArray limb at all three adapter.find() sites #13969 line in the real crm-workbench.page.ts ⇒ gate exit 1, "crm-workbench.page.ts:54: tests Array.isArray(all) on a find() result".

⇒ 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 discovered

The unrecognised binding spellings (destructuring, .then(), a result passed to a helper) are named in the source, with the note that an unrecognised spelling produces no flag silently — and the third of them is the reason route 2 exists rather than a wider regex. The engine.find exclusion is stated with its own instruction: "if that shape ever enters this population, extend the detector in the same edit rather than routing around the gate." ⛔ Declared-and-bounded beats declared-and-broad.

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 examples/app-showcase/src/** plus qualifying fenced blocks under content/docs/** (releases/ excluded), 1 sample from 396 doc files and 1947 fences. ⭐ All three census-anchor sites sit inside it, so the new detector covers every site this class has ever been repaired at. #10751 owns the scope axis and is closed.

Gates and hygiene

13 path-derived families + 2 convention-triggered obligations; derived minus run is empty; exit codes captured before any pipe. check-test-completeness.mjs exit 3 recorded as NOT MEASURED, ⛔ not as a pass.

⭐ And one run beyond the derivation, because the derivation cannot see it: scripts/check-docs-section-name.mjs imports fencedBlocks from the edited module — self-test green (54 cases), corpus run green. ⇒ An importer is a real dependency the path-based derivation does not model; running it was the dev's own catch, not the tool's. Repo-wide pnpm lint run in full (not narrowed), exit 0.

skip-changeset applied additively and read back; the diff is a single scripts/** file, which matches no workspace publish glob, and the root is private.

⚠️ Noted, no action: the platform appended a second identical footer to the PR body. The dev left it rather than PATCHing, because the edit would downgrade the session-form link. Correct call — cosmetic, and the alternative loses information.

An observation the dev recorded and did NOT act on, which I endorse

After #13969, :594's assertion message"the REPAIRED docs sample is silent" — is stale prose: that sample no longer contains the string at all. The assertion itself remains correct, so it was left byte-for-byte unchanged per Zone 1.1 and an adjacent comment states what it does and does not claim. ⇒ ⛔ Rewriting an assertion's text inside a PR that was told not to touch it would have been the wrong kind of tidiness.

Governed-surface check

Diff: one file, scripts/check-react-page-adapter-contract.mjs. Register read live from scripts/pm/check-governed-merges.mjs. ⛔ No hit on docs/adr/** · .claude/** · skills/** · AGENTS.md · CLAUDE.mdthis seat may arm it, once CI settles.


Generated by Claude Code

@os-project-manager
os-project-manager marked this pull request as ready for review August 31, 2026 22:45
@os-project-manager
os-project-manager added this pull request to the merge queue Aug 31, 2026
Merged via the queue into main with commit a6a2af5 Aug 31, 2026
34 checks passed
@os-project-manager
os-project-manager deleted the claude/issue-13970-array-isarray-limb-detector branch August 31, 2026 23:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants