Skip to content

Delete the unreachable Array.isArray limb at all three adapter.find() sites - #13969

Merged
os-project-manager merged 2 commits into
mainfrom
claude/issue-13706-array-isarray-limb
Aug 31, 2026
Merged

Delete the unreachable Array.isArray limb at all three adapter.find() sites#13969
os-project-manager merged 2 commits into
mainfrom
claude/issue-13706-array-isarray-limb

Conversation

@os-project-manager

Copy link
Copy Markdown
Collaborator

Fixes #13706

PR #13705 deleted the tolerant ?? result?.records alias at the two sites its ruling
named. The Array.isArray(...) limb standing beside it is dead for the same reason and
was left behind. This deletes it, at every site that carries it.

The unreachability argument, re-derived here

Enumerated on objectui rather than quoted from the card — at the sha this repo pins
(.objectui-sha = 9602dc820450dda956843c6cfe5b329bcf88c757) and again at objectui
origin/main (592acaf). The two agree line for line.

ObjectStackAdapter.find() (objectui packages/data-objectstack/src/index.ts) returns
from five points:

  1. return { data: [], total: 0 } — the resource is memoized in missingResources from
    an earlier 404.
  2. return existing — the in-flight dedupe. It hands back a promise built by the same
    IIFE, so it resolves through the other four points and contributes no shape of its own.
  3. return this.normalizeQueryResult(result, params) — the raw-GET path, taken when
    $expand or $search is set.
  4. return this.normalizeQueryResult(result, params) — the client-SDK path.
  5. return { data: [], total: 0 } — a fresh 404 that is not an enable-block denial.

normalizeQueryResult() has exactly two branches, and both return an object literal
carrying data, total, page, pageSize, hasMore. Its first branch is the one that
makes the limb dead: it tests Array.isArray on the transport response and wraps a
bare array into that envelope.

// objectui packages/data-objectstack/src/index.ts — normalizeQueryResult()
if (Array.isArray(result)) {
  return { data: result, total: result.length, page: 1, pageSize: result.length, hasMore: false };
}

The array case is therefore folded before any caller sees it. No page can be handed an
array by find(), so an Array.isArray test on a find() result is false on every
path. The declared return type says the same thing from the other side: find() resolves
a QueryResult, an object type, never a union with an array.

Corroborated by this tree, without relying on the derivation:
examples/app-showcase/test/react-page-adapter-query-contract.test.ts executes the
renewals-pipeline rollup against a contract-faithful adapter double whose find() resolves
{ data, total, page, pageSize, hasMore } and nothing else. It passes unchanged with the
limb removed (5 tests, 5 passed), because the limb was never taken there either.

Sites — re-derived: three, not two

Line numbers on the merge base d7e8f3ed6, which is origin/main as of this branch:

file line before after
content/docs/ui/react-pages.mdx 179 const records = result?.data ?? (Array.isArray(result) ? result : []); then setRows(records) if (alive) setRows(result.data);
examples/app-showcase/src/ui/pages/crm-workbench.page.ts 51 const rows = Array.isArray(all) ? all : (all && all.data) || []; const rows = all?.data ?? [];
examples/app-showcase/src/ui/pages/renewals-pipeline.page.ts 85 const rows = (res) => (Array.isArray(res) ? res : (res && res.data) || []); const rows = (res) => res?.data ?? [];

The card expected two sites. renewals-pipeline is the third: same defect class, same
adapter.find() contract, and already a census anchor of
scripts/check-react-page-adapter-contract.mjs — it is repaired here rather than filed,
because a repair that leaves one of the guard's own three anchors carrying the shape is
the population gap that let this defect survive its first two fixes.

Behaviour-preserving at all three: .data was read first and always won. What goes is a
shape the producer cannot emit. The comment blocks in both page modules already explain
the neighbouring .records trap; each now records this derivation beside it, so the limb
does not come back as a defensive edit.

What PR #13955 changed around the sample

#13955 (merged today, d7e8f3ed6, +62/−9) scoped the react-only half of the page to the
react tier. Around the live-data sample it added exactly one thing: a bold lead-in
immediately under ## Live data

On the react tier. useAdapter and React's hooks exist only where the source is
executed, and the sample below is refused on an html page before any of that matters …

It did not touch the sample's code, so the card's line reference was stale only by the
offset that marker introduces (the fence moved down by 22 lines). The marker does change
what the surrounding sentence should say, and this PR answers it: the sample now sits
under a paragraph that states the result contract, so a reader who has just been told this
section is react-only is also told what find() resolves to on that tier.

The sample is the copy a customer — and a coding agent — starts from, so it must be
correct to copy rather than merely shorter. The prose added under the fence:

find() always resolves to the same envelope — a QueryResult carrying the rows under
data. A backend that answers with a bare array is folded into that envelope before
your code sees it, so result.data is the only row shape a page is ever handed: read it
directly, with no fallback for a shape the adapter cannot produce.

That also retires the related nit the card names: the sample's local was called records
while holding result.data. The local is gone rather than renamed.

Changeset

One, naming @objectstack/docs: patch, following #13955 — the most recent content/docs/**
diff, which named that package. @objectstack/example-showcase is deliberately not named:
it is private and appears in 0 of this repo's changesets, and PR #13705 — the immediately
preceding repair at two of these same three sites, with the same
content/docs + examples/app-showcase + scripts diff shape — carried no changeset at
all. Naming the published docs package is the stricter of the two precedents.

Verification

All at 02086ebb1, the final commit.

  • Gate family derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack
    (46 commands once the changeset existed). comm -23 of derived against run is empty.
  • 45 of 46 green. Five needed a build first and were re-run green after
    turbo run build: the two @objectstack/lint doc gates, @objectstack/spec check:docs,
    check:skill-examples, check:dual-build-cjs-loads.
  • The one remainder is node scripts/check-test-completeness.mjs, exit 3, which is the
    gate's own NOT-MEASURED branch and not a red — it prints: "Arrived here from the gate
    family scripts/pm/dispatch-gates.mjs derives? That list names this script with NO
    argument, which is this branch. There is no local log to hand it, so the local reading
    for this gate is NOT MEASURED."
  • The mandatory non-derivable gate ran: pnpm check:ratchet-remedy-authority
    OK check-ratchet-remedy-authority: 180 scripts swept ….
  • pnpm check:react-page-adapter-contract
    ✓ check-react-page-adapter-contract: 21 app-showcase page module(s) + 1 content/docs react-page sample(s) (from 396 doc file(s), 1946 fenced block(s)) — every adapter query option is $-prefixed and every row read is off data.
  • pnpm lint (repo-wide eslint . --no-inline-config, no narrowing) — exit 0.
  • pnpm check:nul-bytes — OK, 7661 text files, no raw control bytes.
  • pnpm --filter './examples/*' run typecheck — all four example apps Done.
  • pnpm --filter @objectstack/example-showcase exec vitest run --maxWorkers=2
    26 files, 364 tests, all passed, including the executing adapter-contract oracle.

Note for the reviewer, not repaired here

scripts/check-react-page-adapter-contract.mjs has two detectors — recordsReads and
unprefixedQueryKeys. Neither sees an Array.isArray limb, so the guard that stops the
.records shape returning does not stop this one. That guard is out of scope for this
card by its dispatch, and nothing here touches it; the gap is reported so it can be
triaged on its own.

Generated by Claude Code


Generated by Claude Code

claude added 2 commits August 31, 2026 19:45
…ites

`ObjectStackAdapter.find()` cannot resolve to an array. Re-derived on the
pinned objectui sha (9602dc82) and on objectui `origin/main`: find() has two
object-literal returns (`{ data: [], total: 0 }` for a memoized 404 and for a
fresh non-denial 404), two `normalizeQueryResult(...)` returns, and an inflight
`return existing` that hands back a promise from that same set. Both of
`normalizeQueryResult`'s branches return an object literal with exactly
`data, total, page, pageSize, hasMore` -- the first one WRAPS a bare array
response into it. So no `Array.isArray(<find result>)` limb can ever be taken.

Behaviour-preserving, like the `?? records` deletion beside it: `.data` was
already read first and always won. What goes is a shape the producer cannot
emit, in the sample a customer (and a coding agent) copies from.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pk26oZ12t5N1hwGW1m1MgC
`@objectstack/docs` patch, following #13955 (the most recent `content/docs/**`
diff, which named that package). `@objectstack/example-showcase` is deliberately
not named: it is private and appears in 0 of the repo's changesets, and #13705 —
the immediately preceding repair at two of these same three sites — carried no
changeset at all.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pk26oZ12t5N1hwGW1m1MgC
@claude

claude Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

ACCEPTdomain:devx execution PM seat (#6023), session session_01Pk26oZ12t5N1hwGW1m1MgC. Verified against origin/main, ⛔ not the shared checkout.

⛔ First, the correction: the justification I gave this dispatch was false

My Zone 1 rule 3 said: "a delivery which is only 'remove N more instances' is judged on whether it answers why this class will not reappear under a different name — and this card's answer is #13705's guard, which covers only this shape."

Backwards. I read scripts/check-react-page-adapter-contract.mjs after the dev reported it. It has exactly two detectors — unprefixedQueryKeys (:197), recordsReads (:296) — and all three of its Array.isArray appearances are self-test fixtures. The third is decisive:

:594  recordsReads(`const records = result?.data ?? (Array.isArray(result) ? result : []);`).length === 0,

⇒ ⭐ The guard asserts zero findings for a line carrying Array.isArray once the ?? result?.records alias is absent — which is exactly the tree's shape after #13705, and exactly what this PR deletes. The detector fires on the alias, ⛔ not on the limb. The guard is not incidentally blind here; it is pinned blind.

The class question this PR was supposed to answer has no answer, and I asserted one that does not exist. Filed as #13970, which opens by correcting me. ⛔ Not fixable here: the remedy edits :594, a deliberately-pinned case, and the dispatch order forbade touching the guard — the dev stopped and reported, which was right.

The premise, derived by the dev rather than quoted

On objectui at the pinned sha 9602dc82 and again at its origin/main 592acaf (identical), ObjectStackAdapter.find() returns from five points — ⛔ not the card's three: two { data: [], total: 0 } literals (memoized-404 short-circuit; fresh non-denial 404), two normalizeQueryResult() calls, and return existing (in-flight dedupe, handing back a promise from the same IIFE, adding no shape). normalizeQueryResult() has exactly two branches, both returning an object literal with exactly data/total/page/pageSize/hasMore — ⭐ and the first branch is itself an Array.isArray test on the transport response, wrapping a bare array into that envelope.

⇒ No find() result can be an array. The limb is unreachable, established on the producer, ⛔ not inherited.

⛔ My Zone 2 C was wrong again — three sites, and the third one matters most

I expected two. There are three: the card's two plus examples/app-showcase/src/ui/pages/renewals-pipeline.page.ts:85.

⭐ And that third site is a census anchor of the very guard #13705 landed. Leaving it would have rebuilt the population gap that let this defect survive its first two fixes (#11585 → PR #13705 → this card). ⇒ My count would have shipped the class's own escape route intact.

The oracle for a deletion, which is the hard part

A deletion has no new guard to ablate, and the dev said so plainly rather than inventing one. What stands in its place:

  • examples/app-showcase/test/react-page-adapter-query-contract.test.ts, 5/5 — it executes the edited renewals-pipeline rollup effect, lifted verbatim from the page source with an extraction control asserting the lift landed, against a contract-faithful adapter double whose find() resolves { data, total, page, pageSize, hasMore } and nothing else. ⭐ That is an executable form of the unreachability claim, and it stays green with the limb gone.
  • pnpm check:react-page-adapter-contract — green, 21 app-showcase page modules + 1 docs sample from 396 doc files / 1946 fenced blocks.

Zone 2 B — confirmed and refined

#13955 touched no code in the sample; it added one bold On the react tier. lead-in under ## Live data, which is what moved the line numbers from ~157 to 179. ⇒ my "re-derive, the file moved" warning was right for the reason I did not know.

Zone 2 D — the docs sample is correct to copy, not merely shorter

The named nit is discharged the right way: the local called records while holding result.data is gone, not renamed, and one paragraph under the docs fence now states the envelope contract. ⇒ a sample an AI author copies now teaches the shape the producer actually emits.

Gates

46 derived, 46 run, comm empty in both directions — and the derivation was taken after the changeset was committed, so the 8 changeset-triggered families are inside the 46 rather than pending. 45 green; five were PREREQUISITE NOT MET on the first pass and went green after the builds they named. One remainder with its reason: check-test-completeness exit 3, the gate's own NOT MEASURED branch, quoted. check:ratchet-remedy-authority run explicitly per #13813 — green. pnpm lint repo-wide, ⛔ no narrowing declared or needed. Full @objectstack/example-showcase suite: 26 files, 364 tests, all pass.

⚠️ Recorded, ⛔ not actioned: an adjacent unreachable class

By the same derivation, find() never resolves nullish either, so typeof (all && all.total) === 'number' ? … at crm-workbench.page.ts:55 and the same (res && res.total) form in renewals-pipeline are also unreachable. ⭐ The dev correctly did not widen into them — a nullish guard is a different class from a competing-shape alias that teaches a spelling the producer cannot emit. Carried into #13970 for whoever owns enforce-or-remove. ⛔ This PR does not touch them.

Governed-surface check

Diff: content/docs/ui/react-pages.mdx · two examples/app-showcase page modules · one changeset. ⛔ No hit on docs/adr/** · .claude/** · skills/** · AGENTS.md · CLAUDE.md ⇒ this seat may arm it.


Generated by Claude Code

@os-project-manager
os-project-manager marked this pull request as ready for review August 31, 2026 20:29
@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 aee1fd9 Aug 31, 2026
34 checks passed
@os-project-manager
os-project-manager deleted the claude/issue-13706-array-isarray-limb branch August 31, 2026 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/s tooling

Projects

None yet

2 participants