Delete the unreachable Array.isArray limb at all three adapter.find() sites - #13969
Conversation
…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
|
ACCEPT — ⛔ First, the correction: the justification I gave this dispatch was falseMy 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 :594 recordsReads(`const records = result?.data ?? (Array.isArray(result) ? result : []);`).length === 0,⇒ ⭐ The guard asserts zero findings for a line carrying ⇒ 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 The premise, derived by the dev rather than quotedOn objectui at the pinned sha ⇒ No ⛔ My Zone 2 C was wrong again — three sites, and the third one matters mostI expected two. There are three: the card's two plus ⭐ 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 partA deletion has no new guard to ablate, and the dev said so plainly rather than inventing one. What stands in its place:
Zone 2 B — confirmed and refined#13955 touched no code in the sample; it added one bold Zone 2 D — the docs sample is correct to copy, not merely shorterThe named nit is discharged the right way: the local called Gates46 derived, 46 run,
|
Fixes #13706
PR #13705 deleted the tolerant
?? result?.recordsalias at the two sites its rulingnamed. The
Array.isArray(...)limb standing beside it is dead for the same reason andwas 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 objectuiorigin/main(592acaf). The two agree line for line.ObjectStackAdapter.find()(objectuipackages/data-objectstack/src/index.ts) returnsfrom five points:
return { data: [], total: 0 }— the resource is memoized inmissingResourcesfroman earlier 404.
return existing— the in-flight dedupe. It hands back a promise built by the sameIIFE, so it resolves through the other four points and contributes no shape of its own.
return this.normalizeQueryResult(result, params)— the raw-GET path, taken when$expandor$searchis set.return this.normalizeQueryResult(result, params)— the client-SDK path.return { data: [], total: 0 }— a fresh 404 that is not anenable-block denial.normalizeQueryResult()has exactly two branches, and both return an object literalcarrying
data,total,page,pageSize,hasMore. Its first branch is the one thatmakes the limb dead: it tests
Array.isArrayon the transport response and wraps abare array into that envelope.
The array case is therefore folded before any caller sees it. No page can be handed an
array by
find(), so anArray.isArraytest on afind()result isfalseon everypath. The declared return type says the same thing from the other side:
find()resolvesa
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.tsexecutes therenewals-pipeline rollup against a contract-faithful adapter double whose
find()resolves{ data, total, page, pageSize, hasMore }and nothing else. It passes unchanged with thelimb 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 isorigin/mainas of this branch:content/docs/ui/react-pages.mdxconst records = result?.data ?? (Array.isArray(result) ? result : []);thensetRows(records)if (alive) setRows(result.data);examples/app-showcase/src/ui/pages/crm-workbench.page.tsconst rows = Array.isArray(all) ? all : (all && all.data) || [];const rows = all?.data ?? [];examples/app-showcase/src/ui/pages/renewals-pipeline.page.tsconst rows = (res) => (Array.isArray(res) ? res : (res && res.data) || []);const rows = (res) => res?.data ?? [];The card expected two sites.
renewals-pipelineis the third: same defect class, sameadapter.find()contract, and already a census anchor ofscripts/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:
.datawas read first and always won. What goes is ashape the producer cannot emit. The comment blocks in both page modules already explain
the neighbouring
.recordstrap; each now records this derivation beside it, so the limbdoes 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 thereacttier. Around the live-data sample it added exactly one thing: a bold lead-inimmediately under
## Live data—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:
That also retires the related nit the card names: the sample's local was called
recordswhile holding
result.data. The local is gone rather than renamed.Changeset
One, naming
@objectstack/docs: patch, following #13955 — the most recentcontent/docs/**diff, which named that package.
@objectstack/example-showcaseis 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+scriptsdiff shape — carried no changeset atall. Naming the published docs package is the stricter of the two precedents.
Verification
All at
02086ebb1, the final commit.node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack(46 commands once the changeset existed).
comm -23of derived against run is empty.turbo run build: the two@objectstack/lintdoc gates,@objectstack/spec check:docs,check:skill-examples,check:dual-build-cjs-loads.node scripts/check-test-completeness.mjs, exit 3, which is thegate'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."
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-wideeslint . --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.mjshas two detectors —recordsReadsandunprefixedQueryKeys. Neither sees anArray.isArraylimb, so the guard that stops the.recordsshape returning does not stop this one. That guard is out of scope for thiscard 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