Skip to content

Delete the tolerant ?? records aliases and narrow the guard carve-out that blessed them - #13705

Merged
os-project-manager merged 1 commit into
mainfrom
claude/issue-11585-records-tolerant-aliases
Aug 31, 2026
Merged

Delete the tolerant ?? records aliases and narrow the guard carve-out that blessed them#13705
os-project-manager merged 1 commit into
mainfrom
claude/issue-11585-records-tolerant-aliases

Conversation

@claude

@claude claude Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Fixes #11585

ObjectStackAdapter.find() resolves a normalized QueryResult that declares data and never records, so the ?? .records limb in both surviving repairs is unreachable by contract. This is behaviour-preserving.data is 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:145result?.data ?? result?.records ?? ... loses the .records limb. 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 .records was wrong is kept.

3 — the carve-out that blessed them

recordsOnlyReads() skipped any line carrying .data:

if (!trimmed.includes('.records')) continue;
if (trimmed.includes('.data')) continue;   // THIS ONE

That made data ?? records the 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 .records is judged as a property READ rather than as text that merely spells it. The detector is renamed recordsReads, 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 .records PRODUCER search — none exists on this path. useAdapter() is typed ObjectStackAdapter | null (objectui packages/react/src/context/AppShellContext.tsx:10), so no other DataSource reaches a react page through it. ObjectStackAdapter.find() has exactly three return paths (objectui packages/data-objectstack/src/index.ts): two { data: [], total: 0 } literals, and normalizeQueryResult() at line 3461, which CONSUMES the transport envelope's records/value and returns an object literal with exactly data, total, page, pageSize, hasMore. records is a key it reads, never one it writes. The app-showcase's own contract-faithful adapter double returns the same five keys. The other .records shapes in the tree are different contracts and unreachable from these two sites: the @objectstack/client PaginatedResult the adapter normalizes, the ObjectQL engine list shape in examples/app-showcase/src/automation/jobs/sweep-project-health.ts:107, ApiDataSource (which also extracts records INTO data), and the seed-authoring records: key in src/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/docs react-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 (importing collectSources, 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:49 and react-pages.mdx:145 and 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 .records beside 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-repo eslint . --no-inline-config — clean, run whole rather than narrowed. @objectstack/example-showcase: 26 files / 364 tests pass, typecheck clean, and test/react-page-adapter-query-contract.test.ts confirmed to have actually executed (5 tests) rather than inferred from a green suite.

Two families read NOT MEASURED rather than green: check-test-completeness and check:dual-build-cjs-loads both exit 3 PREREQUISITE NOT MET (a saved turbo test log; a full pnpm 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-detach refused 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

  • No changeset, skip-changeset applied. Nothing here publishes: @objectstack/example-showcase is private: true, and the other two paths are docs content and a root gate script. scripts/pr-labels.mjs:44 states 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.
  • The Array.isArray limb is deliberately left in both sites. It is also unreachable against the current contract — normalizeQueryResult wraps 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.
  • Hot-file boundary respected: docs(react-pages): html tier requires kebab-case registered names (<list-view>) — PascalCase table reads as if it applies to both tiers #12650's html-tier / kebab-case work in content/docs/ui/react-pages.mdx is untouched. This diff changes one line in that file.

Generated by Claude Code


Generated by Claude Code

…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
@claude claude Bot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 31, 2026
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation labels Aug 31, 2026
@os-project-manager
os-project-manager marked this pull request as ready for review August 31, 2026 07: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 789aa68 Aug 31, 2026
37 checks passed
@os-project-manager
os-project-manager deleted the claude/issue-11585-records-tolerant-aliases branch August 31, 2026 08:18
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/m skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[finding] the surviving .records repairs are tolerant ?? records aliases, and the guard's .data-beside carve-out is what blesses them

2 participants