Delete the tolerant ?? records aliases and narrow the guard carve-out that blessed them - #13705
Merged
os-project-manager merged 1 commit intoAug 31, 2026
Merged
Conversation
…essed them `ObjectStackAdapter.find()` resolves a normalized `QueryResult` whose sole non-empty return path (`normalizeQueryResult`) builds an object literal with exactly `data`, `total`, `page`, `pageSize`, `hasMore`. `records` is a key it READS off the transport envelope, never one it writes -- so the `?? .records` limb in both surviving repairs is unreachable by contract. Behaviour-preserving: `.data` is read first and always wins today. What goes is a spelling the producer cannot emit, sitting in the page a customer copies from. The third piece is what stops it returning: `recordsOnlyReads()` skipped any line carrying `.data`, which made `data ?? records` the one shape it could not see. That carve-out is replaced by comment/string stripping, and the detector renamed `recordsReads` since "only" no longer describes it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pk26oZ12t5N1hwGW1m1MgC
This was referenced Aug 31, 2026
os-project-manager
marked this pull request as ready for review
August 31, 2026 07:45
os-project-manager
deleted the
claude/issue-11585-records-tolerant-aliases
branch
August 31, 2026 08:18
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #11585
ObjectStackAdapter.find()resolves a normalizedQueryResultthat declaresdataand neverrecords, so the?? .recordslimb in both surviving repairs is unreachable by contract. This is behaviour-preserving —.datais read first and always wins today, and both pages render correctly either way. What goes is a spelling the producer cannot emit, sitting in the page a customer copies from.Three things ship, because the first two alone let the pattern come back green.
1 + 2 — the two tolerant aliases
examples/app-showcase/src/ui/pages/crm-workbench.page.ts:49—(all && (all.data || all.records)) || []becomes(all && all.data) || [].content/docs/ui/react-pages.mdx:145—result?.data ?? result?.records ?? ...loses the.recordslimb. This is the one that actively TEACHES the fallback.The prose three lines above the app-showcase call said "Read .data first, with .records/array fallbacks for robustness". That sentence was the instruction the alias implemented, so it is rewritten to state the contract instead. The historical explanation of why
.recordswas wrong is kept.3 — the carve-out that blessed them
recordsOnlyReads()skipped any line carrying.data:That made
data ?? recordsthe one shape the detector could not see — which is precisely the shape both surviving repairs had landed as. The carve-out is replaced by comment and string-literal stripping (codeOnly()), so.recordsis judged as a property READ rather than as text that merely spells it. The detector is renamedrecordsReads, since "only" no longer describes it.Blanking string bodies is what lets the carve-out go safely: the webhook shape
emit({ type: 'data.records.updated' })is kept out of the sweep by the SELECTOR today, and a detector that is quiet only because of the selector is one population change away from firing.Measured, not assumed
A
.recordsPRODUCER search — none exists on this path.useAdapter()is typedObjectStackAdapter | null(objectuipackages/react/src/context/AppShellContext.tsx:10), so no otherDataSourcereaches a react page through it.ObjectStackAdapter.find()has exactly three return paths (objectuipackages/data-objectstack/src/index.ts): two{ data: [], total: 0 }literals, andnormalizeQueryResult()at line 3461, which CONSUMES the transport envelope'srecords/valueand returns an object literal with exactlydata,total,page,pageSize,hasMore.recordsis a key it reads, never one it writes. The app-showcase's own contract-faithful adapter double returns the same five keys. The other.recordsshapes in the tree are different contracts and unreachable from these two sites: the@objectstack/clientPaginatedResultthe adapter normalizes, the ObjectQL engine list shape inexamples/app-showcase/src/automation/jobs/sweep-project-health.ts:107,ApiDataSource(which also extractsrecordsINTOdata), and the seed-authoringrecords:key insrc/data/seed/index.ts. So the removal is behaviour-preserving at runtime, not just by declaration.Guard population, before and after — zero bystanders. The sweep's population is unchanged: 21 app-showcase page modules + 1
content/docsreact-page sample, from 395 doc files and 1946 fenced blocks, both before and after. Measuring the carve-out's load-bearing set over the guard's OWN population (importingcollectSources, not re-guessing it) found it suppressing exactly two lines — the two deleted here — and nothing else.Reverse-verified from the committed state: with the guard narrowed and both alias lines restored from the merge base, the gate exits 1 with exactly 2 findings, naming
crm-workbench.page.ts:49andreact-pages.mdx:145and no third file. The restore leg was proven byte-for-byte against the HEAD blobs. So the narrowing bites on the pattern and reds no unrelated existing code.The self-test grows from 30 to 37 assertions: both aliases pinned as findings, the repaired docs sample pinned silent, and the over-fire directions pinned too — a string-literal
data.records.updated, a trailing comment naming.recordsbeside a canonical read, and.recordsCount.Verification
Local gate union at
69e868cf9, the final commit. All 43 derived families green plus both convention-triggered gate-script obligations (bare-root-worklist --self-test;check:pm-dispatch-gates, 1017 cases).pnpm lint— the full-repoeslint . --no-inline-config— clean, run whole rather than narrowed.@objectstack/example-showcase: 26 files / 364 tests pass, typecheck clean, andtest/react-page-adapter-query-contract.test.tsconfirmed to have actually executed (5 tests) rather than inferred from a green suite.Two families read NOT MEASURED rather than green:
check-test-completenessandcheck:dual-build-cjs-loadsboth exit 3PREREQUISITE NOT MET(a saved turbo test log; a fullpnpm build). Neither is a finding.One red was found and fixed during this run, by the gate family rather than by review: the rewritten comment first used backtick-quoted text, and the page body lives inside a
source:template literal, so the backticks terminated it early.check:logger-receiver-detachrefused to scan the unparseable file. The comment now uses the quoting convention the rest of the file already used, and every gate was re-run afterwards on the final tree.Notes for review
skip-changesetapplied. Nothing here publishes:@objectstack/example-showcaseisprivate: true, and the other two paths are docs content and a root gate script.scripts/pr-labels.mjs:44states the criterion as "the exemption for a PR that publishes nothing". Precedent on these same files: the gate-creating PR feat(tooling): sweep the react-page useAdapter() contracts over the docs corpus too #11584 and the previous repair of this exact docs line,243218a36, both landed changeset-free. Happy to add one if this seat reads it differently.Array.isArraylimb is deliberately left in both sites. It is also unreachable against the current contract —normalizeQueryResultwraps a bare array response — and the card raises it as an open question that the triage ruling did not answer. Deciding it here would have pre-empted that, so it is reported instead. See the report comment on the issue.<list-view>) — PascalCase table reads as if it applies to both tiers #12650's html-tier / kebab-case work incontent/docs/ui/react-pages.mdxis untouched. This diff changes one line in that file.Generated by Claude Code
Generated by Claude Code