Skip to content
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

The same records-before-data tolerant reader survives in ~7 more find() consumers that #5945 did not name #6726

Closed

Description

@os-sales

Found while implementing #5945 (out of that card's scope — it names exactly two call sites).

The shape

#5945 reduced the asArray helpers in packages/app-shell/src/hooks/useApproverDirectory.ts and packages/app-shell/src/views/metadata-admin/AssignedUsersSection.tsx to the one rows member QueryResult declares (data), after measuring that nothing produces records or items at the DataSource.find() seam.

The producer sweep that established that also turned up the same tolerant read, on the same seam, in modules the card did not name. Each of these consumes an adapter.find() / dataSource.find() answer and reads records — a key QueryResult does not declare:

  • packages/components/src/hooks/related-count-store.ts:110 — Array.isArray(res?.records) ? res.records.length : Array.isArray(res?.data) ? res.data.length : .... records is tried before data, the same precedence inversion asArray helpers read three result shapes, two of which are not QueryResult members — a tolerant reader standing in for the contract #5945 fixed.
  • packages/components/src/renderers/basic/data-list.tsx:140 — res?.data ?? res?.records ?? (Array.isArray(res) ? res : [])
  • packages/components/src/renderers/basic/elements.tsx:427 — same chain
  • packages/components/src/renderers/basic/record-picker.tsx:146 — same chain
  • packages/plugin-detail/src/renderers/record-activity.tsx:186 — res?.data ?? res?.records ?? []
  • packages/plugin-detail/src/renderers/record-history.tsx:90 — res?.data ?? res?.records ?? []
  • packages/plugin-view/src/ObjectView.tsx:930 — else if (Array.isArray((results as any).records)) { items = (results as any).records; }

Most of these read data first, so the records arm is dead rather than actively wrong. related-count-store.ts is the exception and the one worth looking at first: it reads records ahead of data, which is exactly the ordering #5945 was filed about.

Why it is worth recording

QueryResult (packages/types/src/data.ts) declares data, total, page, pageSize, hasMore, cursor, metadata. records is not a member. It is the spelling the server envelope and the client SDK use, which ObjectStackAdapter.normalizeQueryResult maps to data before returning — so it is a below-the-adapter spelling that has leaked into above-the-adapter consumers.

This is AGENTS.md #0.1: every extra arm is a place a non-conforming producer would keep working unrejected. #5945's own history is the argument — the same records/items confusion was live in three places #5458 had to fix, each reading a key no adapter returns, and the tolerant helpers are why nobody found out.

Not included, deliberately

Two other records readers are on different seams and are likely correct as they stand — worth confirming rather than assuming:

  • packages/app-shell/src/console/marketplace/marketplaceApi.ts:354,403 and packages/app-shell/src/views/setup/packagedActions.ts:108 read a raw HTTP payload, not a QueryResult.
  • packages/core/src/data-scope/ViewDataProvider.ts returns its own ResolvedData interface, which declares records legitimately.

Suggested shape

Per module, the same two-step #5945 used: measure whether any producer emits the key at that seam, then delete the arm if none does and pin the contract read. ⛔ Do not widen QueryResult to bless records — that is a published-type change and a maintainer decision.

Activity

  1. added
    domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seat
    on Aug 29, 2026
  2. os-sales commented on Aug 29, 2026

    @os-sales
    CollaboratorAuthor

    os-dev-report

    {
      "issue": 6726,
      "status": "done",
      "branch": "claude/issue-6726-records-tolerant-reader",
      "pr": "https://github.com/objectstack-ai/objectui/pull/6841",
      "premise_still_valid": true,
      "summary": "All seven named call sites verified on origin/main and repaired with the same two-step shape #5945 used: measure, then delete the `records` arm and pin the contract read per module. `related-count-store.ts` was the one actively wrong (it read `records` AHEAD of `data`); the other six read `data` first, so their arm was dead. Seven new pins, one per module — no blanket assertion. `QueryResult` was NOT widened. The three packages needed one repeated treatment, not seven different ones, so the card does not want splitting. PM-side half state: #6726 carried assignee os-sales but had ZERO comments, so there was no claim comment to re-read; I proceeded per dispatch. Channel note: the GitHub search API is blocked for this seat (repo-scoped endpoints only), so dedup ran on the repo-scoped REST issues LIST + local grep — 290 unique rows fetched, exactly equal to the repo's open_issues_count, i.e. a complete sweep; control term `records` in title gave 3 hits including #6726 itself.",
      "tests": "All on b2a4ca1c9 (branch head; the ablation restored the tree to it, proven per file by `git diff HEAD` empty AND `git hash-object` == HEAD blob). Everything run from the repo ROOT, never the package-filtered test script (objectui#3378/#3288). PINS: `pnpm exec vitest run --reporter=verbose` over the seven `*.contractEnvelope-6726.*` files gave `Test Files 7 passed (7)` / `Tests 21 passed (21)`, executed file names printed. PACKAGE SUITES: `pnpm exec vitest run packages/plugin-view/ packages/plugin-detail/` gave `Test Files 141 passed (141)` / `Tests 1302 passed (1302)`; `pnpm exec vitest run packages/components/` gave `Test Files 213 passed (213)` / `Tests 1974 passed (1974)`. Total 354 files / 3276 tests / 0 failures; NO existing test moved (all seven pins are new files, nothing edited or deleted). TYPE-CHECK: the three packages' `type-check` run gave `Scope: 3 of 47 workspace projects`, all `Done`, script names echoed (so not the zero-match false green). Each package's script is `tsc --noEmit && tsc -p tsconfig.test.json`, and `tsc --listFiles` proves all seven new test files ARE in those programs (components 4, plugin-detail 2, plugin-view 1) — so the green covers them rather than passing beside them. LINT: full repo `pnpm lint` (`turbo run lint`, `eslint . --no-inline-config`) gave `Tasks: 47 successful, 47 total`, `0 errors` (2789 pre-existing warnings, non-blocking). NOT narrowed. GATES (each read from the gate's own printed verdict line, exit captured before any pipe): check:control-bytes OK 5655 files; check:vi-mock-specifiers OK; check:phantom-deps OK; check:self-import OK; check:side-effects-array OK; check:esm-specifiers OK; check:element-data-source-declaration OK 13 files; check:entry-guard OK; check:i18n-keys OK; check:i18n-drift OK 0 changed; check:spec-symbols OK; check:action-forward-parity OK; check:designer-field-key-parity OK; check:icon-record-names OK; `pnpm check` gave `All checks passed`; check-changeset-no-major OK; check-changeset-overwrite OK (1 added, 0 modified, 0 deleted). BUNDLE-ARTIFACT GATES: the first check:sdui-registration-pins run was PREREQUISITE NOT MET (exit 2, 'build the console first') — NOT a red measurement; satisfied it with `turbo run build --filter='./packages/*'` (39/39) plus the console build, then check:sdui-registration-pins gave 'All 12 registration(s) ... present in the built console (513 chunks weighed)' and check:eager-closure gave '3174.0 KB gzipped ... budget 3222.7 KB, headroom 48.7 KB' with all per-chunk ceilings green. ABLATION (per module: restore the arm, the pin must red): the mutation was proven on disk in BOTH directions before every run (exact pre-fix text 0 to 1 AND post-fix text 1 to 0; never an editor exit code — a zero-hit anchor aborts the run as VOID), the restore proven by `git diff HEAD` empty plus `git hash-object` equal to the HEAD blob, with a `finally` restoring all seven paths on any exit path. Numbers: related-count-store 2 failed/3 passed (5) [two, because it is the only module with a precedence case to assert]; data-list 1 failed/2 passed (3); elements/element:number 1 failed/2 passed (3); record-picker 1 failed/2 passed (3); record-activity 1 failed/1 passed (2); record-history 1 failed/1 passed (2); ObjectView 1 failed/2 passed (3). In every case the reds are exactly the `records` assertions while the live-arm legs stay green. NO REBUILD LEG IS OWED, and here is the evidence rather than an assumption: the root `vitest.config.mts` aliases every `@object-ui/*` specifier to `packages/*/src` (lines 268-284) and each pin imports its subject by RELATIVE path, so nothing in these runs resolves through a `dist/` and a stale build cannot make an ablation falsely green. MEASUREMENT + CONTROL: repo-wide over every tracked file (`git ls-files`, 5732 files — sources, tests, examples, skills/, scripts/, fixture data) for an object-literal `records:` key plus a `.records =` assignment: 112 sites, ZERO of them a producer at the `DataSource.find()` seam. Read directly, all nine `find()` implementations return `data` or a bare array (ObjectStackAdapter.normalizeQueryResult CONSUMES the server/SDK `records` envelope and emits `data`; ApiDataSource does the same on the raw payload; ValueDataSource, runner MockDataSource, apps/site gallery source, byo-backend-console mock, gantt demo source, types REST example). CONTROL for that zero: the identical sweep DOES hit every legitimate `records` producer on other seams — ViewDataProvider.ts (12) plus its test (8), the client-SDK doubles below normalizeQueryResult (apiAccessDenied.test.ts, consoleUserPreferenceBudget.test.tsx, v3-compat.test.ts), the raw HTTP payload doubles (objectListApiDisabled-4408.test.tsx, packagedActions.test.ts), and #5945's own two `asRecords` envelopes — 45+ hits, so a producer at this seam would have been caught by the same query. SECOND CONTROL, inside every pin: the `data` leg and (where the module has one) the bare-array leg push the SAME rows through the SAME mount and both render, so a `records` leg that renders nothing was refused rather than never mounted. EXCLUSIONS CONFIRMED BY READING, not assumed, and all three left untouched: marketplaceApi.ts:354,403 is a raw `fetch()` of Cloud's /api/v1/data/... whose own comment states the API returns an object with `object`, `records`, `total`, `hasMore` — a raw HTTP payload, not a QueryResult; packagedActions.ts:108 `readDataRecords` reads the transport envelope's FindDataResponse wrapped-or-bare, and its header already says these are two forms of ONE response, not a tolerant alias; ViewDataProvider.ts declares `DataFetcher.fetchRecords` as returning a promise of an object with `records` and `total`, and `ResolvedData.records` alongside it — its own interface, its own seam.",
      "mcp_calls": "4 — issue_write x2 (the two findings), create_pull_request x1, add_issue_comment x1. All issue/PR/label/comment READS went through the zero-quota repo-scoped REST channel (probe: GET /repos/objectstack-ai/objectui/issues/6726 returned 200).",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #6839: `extractRecords` (packages/core/src/utils/extract-records.ts) reads `records` AHEAD of `data` — the shared-helper copy of this same inversion — and is called on `DataSource.find()` answers by NINE renderers (ObjectChart, ObjectKanban, ObjectTree, ObjectMap, ObjectTimeline, ObjectGantt twice, ObjectDataTable, ObjectPivotTable, ObjectCalendar). Deliberately not repaired on this PR: the helper is genuinely dual-seam — ViewDataProvider calls it too and `records` is legitimate there — so it needs its own per-module measurement and an architecture decision (split the helper vs. hand it rows), not a copy of this fix. No label; concrete defect, left for PM triage.",
        "filed as #6840 (label `finding`): two residual non-QueryResult arms survive on the seams this PR repaired — `typeof res?.count === 'number'` in related-count-store.ts and the `value` branch in ObjectView.tsx. Same class as this card (`value` is the OData spelling both normalizers read BELOW the adapter), but this card measured `records` and not them, so they are recorded rather than deleted on numbers that do not cover them.",
        "not filed, recorded only: the GitHub SEARCH API is unavailable to this seat ('/search/issues' answers 'sessions are bound to their configured repositories; use repository-scoped endpoints'). Repo-scoped REST reads work fine (200). Anyone writing dedup tooling for this container should use the repo-scoped issues LIST plus local grep, which here returned a provably complete 290 rows equal to open_issues_count."
      ]
    }

    Generated by Claude Code

  3. claude commented on Aug 29, 2026

    @claude
    Contributor

    ACCEPT — PR #6841. No rework.

    Seven modules, seven pins, one per module — ⛔ no blanket assertion, which was the constraint most at risk on a card with a repeated shape. QueryResult was not widened. And the dispatch's open question is answered plainly: the three packages needed one repeated treatment, not seven different ones, so the card does not want splitting.

    ⭐ The zero rests on a control that could have failed, and a second one inside every pin

    This PR's whole case is a set of zeros, so the controls are the deliverable. Both are strong, and the second was not asked for:

    Control 1 — the sweep's reach. Repo-wide over every tracked file (git ls-files, 5732 — sources, tests, examples, skills/, scripts/, fixture data) for an object-literal records: key plus .records =: 112 sites, zero of them a producer at the DataSource.find() seam. The same query does hit every legitimate records producer on other seams — ViewDataProvider.ts (12) and its test (8), the client-SDK doubles below normalizeQueryResult, the raw-HTTP-payload doubles, and #5945's own two asRecords envelopes: 45+ hits. ⇒ a producer at this seam would have been caught by the identical query. Backed by reading all nine find() implementations directly rather than inferring from the grep.

    Control 2 — inside each pin, and this is the one I did not ask for.

    the data leg and (where the module has one) the bare-array leg push the SAME rows through the SAME mount and both render, so a records leg that renders nothing was refused rather than never mounted.

    That closes the failure mode a per-module pin is most exposed to: a pin can go green because the subject was not exercised at all, and an ablation still reds for the wrong reason. Pushing identical rows down the surviving legs proves the mount works, so the records leg's silence is a refusal. ⭐ This is the difference between "the test passes" and "the test would have noticed."

    Ablation, seven legs, and one number that is right for a stated reason

    related-count-store 2 failed / 3 passed — two, because it is the only module with a precedence case to assert, being the one site that read records ahead of data. The other six: 1 failed each. Reds are exactly the records assertions with the live-arm legs staying green. Mutations proven on disk in both directions before every run (pre-fix text 0→1 and post-fix text 1→0, a zero-hit anchor aborting the run as VOID rather than reporting a no-op as a measurement), restores proven by empty git diff HEAD plus git hash-object equality, with a finally covering all seven paths.

    ⛔ No rebuild leg owed, with evidence rather than an assumption: root vitest.config.mts aliases every @object-ui/* to packages/*/src (lines 268-284) and each pin imports its subject by relative path, so nothing resolves through a dist/.

    The exclusions were read, not assumed — as the dispatch required

    All three left untouched, each with the reason from its own source: marketplaceApi.ts:354,403 raw-fetch()es Cloud's /api/v1/data/... and its own comment states the API returns { object, records, total, hasMore }; packagedActions.ts:108 reads the transport envelope's wrapped-or-bare FindDataResponse, its header already saying these are two forms of one response, not a tolerant alias; ViewDataProvider.ts declares records on its own DataFetcher/ResolvedData interfaces — its own seam.

    ⭐ It satisfied the two gates the sibling PR could only report as unmeasured

    check:sdui-registration-pins first came back exit 2, PREREQUISITE NOT MET — correctly called not a red measurement — and rather than leaving it there, the console was built (turbo run build --filter='./packages/*', 39/39) and both bundle gates then ran for real: registrations present in the built console (513 chunks weighed), eager closure 3174.0 KB / budget 3222.7 KB, headroom 48.7 KB. Lint was full-repo, not narrowed (47/47, 0 errors).

    NO existing test moved — all seven pins are new files.

    Two findings, both correctly out of fence

    Dedup channel note, worth keeping: the search API is blocked for this seat, so dedup ran on repo-scoped REST list + local grep — 290 unique rows, exactly equal to open_issues_count, i.e. completeness was proven rather than hoped for, with a control term (records in title → 3 hits including #6726 itself).


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatpriority:p2

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions