Skip to content

expand is a silent no-op when the nested fields omits id — the exact spelling two retirement messages prescribe #7537

Closed

Description

@huangyiirene

Symptom

POST /api/v1/data/showcase_invoice/query with

{"expand":{"account":{"object":"showcase_account","fields":["name"]}}}

answers 200 with account left as the raw FK id — the expansion silently did not happen. Adding "id" to the nested projection ("fields":["id","name"]) expands correctly. Confirmed 2/2 on a second object pair.

Expected: account carries the related record projected to name, or a refusal. Nothing in the response distinguishes "expanded" from "not expanded" — the field just still holds an id, which is a valid-looking value.

The stakes are raised by where the failing spelling comes from: it is the form prescribed verbatim in two spec retirement messages (FIELD_NODE_OBJECT_FORM_REMOVED and QUERY_JOINS_REMOVED) and in this checklist item's own steps. A caller who does exactly what the error message told them to do gets a silent no-op.

Root cause

expandRelatedRecords (packages/objectql/src/engine.ts) forwards nestedAST.fields to the sub-find, then keys recordMap on rec.id. A nested projection that omits id therefore yields an empty map, and the injection falls through to recordMap.get(val) ?? val — i.e. the original FK value is written back and the expansion is indistinguishable from never having been requested.

Fix shape: id must be added to the sub-read's projection unconditionally (it is the join key, not a caller-chosen column) and stripped from the emitted nested record if the caller did not ask for it — or, failing that, the mismatch must be refused rather than degraded.

Reproduction

  1. Boot showcase on a fresh isolated file DB (SqlDriver / better-sqlite3); authenticate as admin@objectos.ai.
  2. POST /api/v1/data/showcase_invoice/query body {"expand":{"account":{"object":"showcase_account","fields":["name"]}}} → 200, account is the raw FK id.
  3. Add "id": {"expand":{"account":{"object":"showcase_account","fields":["id","name"]}}} → expands correctly.
  4. Repeat on a second object pair (showcase_task → project) — same 2/2 result.

Source

Extracted from the QA run #7463 (framework a86db17).

Activity

  1. self-assigned this
    on Aug 11, 2026
  2. os-help commented on Aug 11, 2026

    @os-help
    Collaborator

    Claim: PM loop round 1 (engine-core seat, 2026-08-11 shift)
    Session: session_01PemxJmMZT9JphrsNx886Ce
    Branch: claude/issue-7537-expand-nested-fields-id
    Worktree: objectstack-issue-7537 (mode:cloud — own clone)
    Domain: domain:engine-core
    File surface: packages/objectql/src/engine.ts (expandRelatedRecords, ~:6168) + expand pin tests + changeset (stop on breach; explain in the report)
    Container & model: M (semantic seam: forced join-key sub-projection + strip vs refuse), mode:cloud, model: opus
    Serial constraints cleared: engine.ts freed — last claims on it (#6752/PR #7482, #7413/PR #7476) are MERGED; #4953's pm:dispatched is a finished-claim anchor (its engine-core share landed via PR #6454, per the 2026-08-08 accounting note); round sibling #7561 owns search-companion.ts/registry.ts — disjoint. #7557 (registry.ts + package routes) deliberately held to a later round.


    Generated by Claude Code

  3. os-help commented on Aug 11, 2026

    @os-help
    Collaborator

    Dispatched mode:cloud: session session_01HJVZqLviCkZMJV8Nh3DUao (created 06:18Z, model opus, outcome branch = the claim branch). Report expected as an issue comment here at draft-PR time.


    Generated by Claude Code

  4. os-help commented on Aug 11, 2026

    @os-help
    Collaborator

    os-dev report — #7537 · session session_01HJVZqLviCkZMJV8Nh3DUao · branch claude/issue-7537-expand-nested-fields-id · draft PR #7594

    premise_still_valid: true — verified against origin/main @ 97b6658 (not a working tree), and reproduced empirically before any edit.

    Verification record

    The issue was filed against a86db175; engine.ts has moved. Re-checked on 97b6658: expandRelatedRecords is still at :6168, still forwards nestedAST.fields verbatim at :6292, still keys recordMap on rec.id at :6301, and the injection still falls through recordMap.get(String(val)) ?? val at :6330. The issue's root-cause paragraph is accurate as written.

    Empirical repro (new real-SqlDriver integration test, pre-fix tree) — 3 failures, exactly the reported symptom:

    × expands with `fields: ["name"]`   → expected 'string' to be 'object'
    × second object pair (task→project) → expected 'uHiWgyMU_yePSp2T' to equal { name: 'Apollo' }
    × findOne takes the same path       → expected 'xYvxWjuwNasB1pNK' to equal { name: 'Acme' }
    

    Both controls passed pre-fix (fields: ['id','name'], and no projection) — matching "adding id expands correctly".

    Two measurements beyond the issue

    The defect is driver-conditional, which is why 3157 objectql tests missed it. SqlDriver emits the column list verbatim (builder.select(query.fields), sql-driver.ts:3383) and genuinely drops id; InMemoryDriver.projectFields force-adds the primary key to every projection (memory-driver.ts:1370), so its rows always carry the join key and the map is never empty. Every existing expand pin runs on mocks returning whole rows. Hence the end-to-end pin is wired to a real SqlDriver, not a mock.

    The PM's "is the join key configurable?" fork resolves to "no — literally id." referenceTargetOf (field-value.zod.ts:132) returns a target object name only; no reference type carries target-column metadata anywhere in spec. (relationshipField/foreignKey at field.zod.ts:712 names the child's FK field for related lists, not a target key on the parent.) The batch filter is { id: { $in } } by construction, so forcing id is correct rather than a guess.

    Route taken: add-and-strip (not refuse)

    Refusing would break five documented migration targets, not two: FIELD_NODE_OBJECT_FORM_REMOVED (query.zod.ts:322), QUERY_JOINS_REMOVED (:359), migrations/registry.ts:3062 and :3083, and the two semantic migration entries 17.query-field-node-object-form-retired.ts / 17.query-joins-retired.ts — all prescribing expand: { owner: { object: 'user', fields: ['name'] } } verbatim. Add-and-strip is the answer with no collateral, as the PM leaned.

    id is appended to the sub-read's projection when the nested fields is a non-empty array not already naming it (non-triggering shapes forward byte-identical), and stripped from each related record before injection — after the recursive-expand block, which rebuilds recordMap on rec.id and still needs it. All four entry points (find :6805, findOne :6944, recursion :6307) route through the one helper; confirmed by test, not by reading — the findOne case failed pre-fix and passes now, and there is a dedicated recursive pin.

    Behaviour change flagged

    A nested projection omitting id now emits a nested record without id. Previously driver-dependent: SQL-backed stores gave the no-op (no .id to read — the value was a plain FK string), InMemoryDriver gave an expanded record carrying an unrequested id. Both now emit exactly the projected columns. Recorded in the changeset.

    Tests (substance-asserting)

    packages/runtime/src/expand-nested-fields-join-key.integration.test.ts (new, real engine + real SqlDriver/better-sqlite3 on disk) — 5 cases: the prescribed fields: ['name'] spelling expands to exactly { name: 'Acme' } (unrequested industry absent, id absent); ['id','name'] keeps the key; the issue's second object pair showcase_task → project; findOne; no-projection unchanged.

    packages/objectql/src/engine.test.ts — 4 unit pins under [#7537]: the sub-read is actually called with fields: ['name','id'] while the emitted record is { name: 'Alice' }; a caller ['id','name'] forwarded untouched; array-valued (multiple: true) expansion strips too; recursive nested expand resolves both levels and strips at both.

    No refusal path added → ADR-0112 envelope assertions not applicable.

    Gates — all local gates PASS

    gate status
    pnpm check:adr-anchors pass
    pnpm check:durability-log-level pass
    pnpm check:engine-double-contract pass (148 pinned / 133 DEBT / 2 exempt)
    pnpm check:stack-collection-maps pass
    node scripts/check-engine-split-ratio.mjs pass
    pnpm check:nul-bytes pass (7015 files)
    full pnpm build pass (71/71 tasks)
    objectql suite pass (178 files / 3157 tests)
    runtime suite pass (125 files / 1995 tests)
    rest suite pass (82 files / 1344 tests)
    typecheck (objectql, runtime) pass
    eslint (3 changed files) pass
    CI in_progress — PM owns convergence

    Changeset: .changeset/expand-nested-fields-join-key.md (@objectstack/objectql: patch).

    open_questions: none blocking. One judgement recorded rather than asked: the strip half is a visible change for InMemoryDriver-backed callers (they used to receive an unrequested id). Decided in favour of honouring the projection — no in-repo consumer reads .id off a nested-projected expansion (swept every expand: { … fields: … } site), and on any SQL-backed store there was nothing to read there before.


    Generated by Claude Code

  5. os-help commented on Aug 11, 2026

    @os-help
    Collaborator

    ACCEPT — PR #7594 reviewed and moving to landing (engine-core seat, session_01PemxJmMZT9JphrsNx886Ce). Verified against GitHub, not the report's claims:

    • 5 files, exactly the declared surface (engine.ts + engine unit pins + real-SqlDriver integration test + docs paragraph + changeset). No content/docs/releases/, no docs/adr/**.
    • Mechanism re-verified and the fix read line-by-line: id forced into the sub-read only when a non-empty nested fields omits it; strip runs after the recursive block (which still needs the key) and per-record, not per-parent. Non-triggering shapes forward byte-identical.
    • Route choice (add-and-strip over refuse) is the right one and now evidence-backed: the dev found five in-tree prescriptions of the failing spelling, not the two the issue named — refusal would have broken the platform's own migration guidance.
    • The driver-conditional measurement (SqlDriver honours projections verbatim; InMemoryDriver force-adds id — why 3157 mock-based tests missed this) justifies the real-driver integration pin; direction-predicted reverse verification: 3/3 red pre-fix on exactly the reported symptom.
    • Behaviour change (nested record no longer carries an unrequested id on memory-backed stores) is flagged in the changeset with the caller remedy, and the dev swept in-repo consumers — none read .id off a nested projection.
    • CI: 26/26 checks completed, ESLint and TypeScript Type Check job conclusions both success.
    • Cross-package note: the integration test lands in packages/runtime (cli lane's package family) — test-only, zero collision surface; declared on the cli seat post.

    Flipping ready + arming auto-merge now; the merge queue owns landing.


    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

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions