Skip to content

feat(spec): declare IScopedObjectRepository.updateById's answer — the record or null, not any - #17836

Merged
os-bill merged 1 commit into
mainfrom
claude/issue-16786-scoped-updatebyid-narrowing
Sep 12, 2026
Merged

os-bill merged 1 commit into
mainfrom
claude/issue-16786-scoped-updatebyid-narrowing

Conversation

@os-bill

@os-bill os-bill commented Sep 12, 2026

Copy link
Copy Markdown
Collaborator

Fixes #16786

Generic arguments are spelled with SQUARE brackets throughout this body — Promise[T], Record[string, any] — so that no fragment of it is shaped like an HTML tag. The source carries the real spelling.

What changed

One member of one interface, plus its docblock, a pin and a changeset:

packages/spec/src/contracts/scoped-context.ts
- updateById(id: string | number, data: any): Promise[any];
+ updateById(id: string | number, data: any): Promise[Record[string, any] | null];

PR #17255 landed this card's objectql half and left this member named as the remainder; triage re-routed the card to domain:spec on 2026-09-10. This is that remainder and nothing else.

Why — the measurement, ⛔ not an enumeration

⚠️ Stated plainly so the authority is not over-read: ruling A on #16231 did not enumerate this line. What the ruling settled is the RULE — #15823's find() narrowing extends to the sibling doors — and its enumeration named scoped-context.ts:148 / :164. updateById is a door of that family the enumeration missed. It is narrowed here because the measurement says the declaration was wider than every implementation and wider than the door it forwards to.

Located by SYMBOL, re-derived at this branch's base b59b74909c — ⛔ every path:line on the card is its writer's reading at their own head and two of them are days old.

The declaration, and its siblings after PR #16783

member declared on origin/main before this PR
find Promise[any[]]
findOne Promise[Record[string, any] | null] — narrowed under ruling A
count Promise[number]
insert Promise[any]
update Promise[Record[string, any] | number | null] — narrowed under ruling A
updateById Promise[any] ← this PR

⚠️ Note for a reader expecting a fourth sibling: this interface declares no delete member at all. scoped-context.test.ts pins its ABSENCE (rejects-delete). Ruling A's delete anchor is on IDataEngine, a different interface.

What the implementations actually return

⭐ The whole defect class is that TypeScript's implements accepts a wider declared return, so a class satisfies a narrowed interface while publishing any to every caller. Shown at this member rather than asserted:

⇒ Promise[any] was wider than every implementation AND wider than the door it forwards to. The bar the dispatch set is met.

Why the number limb is NOT declared here

update carries a count limb because it has two dispatch exits; updateById has one. The implementation binds a truthy scalar payload id AND a pure-id where, and never declares multi, so resolveEngineUpdateDispatch (@objectstack/metadata-core) answers by-id for every call this signature admits: unhonouredByIdPredicateKeys({ id }) filters out the key id and returns [], and the payload id equals where.id by construction, so neither refusal branch is reachable. A falsy id (0, '') is a REFUSAL rather than a narrower answer — it identifies no row, the dispatch rejects and the call throws.

⇒ the honest answer is the by-id exit: the record, or null.

Consumer census

Who reaches updateById through the interface-typed door, repo-wide (grep over all .ts / .tsx / .md / .mdx, excluding node_modules and dist):

  • packages/spec/src/contracts/scoped-context.test.ts — legal-updateById compile probe (discards the result) and a literal IScopedObjectRepository returning ({ id, ...data }). Both satisfy the narrowed shape unchanged.
  • packages/spec/src/data/hook.test.ts — a fake returning ({ id, ...data }). Same.
  • packages/lint/** — twelve sites, all of which parse the SPELLING ctx.api.object(…).updateById(…) out of authored hook/action bodies as text. None reads a return type.
  • content/docs/api/error-handling-server.mdx, content/docs/automation/hook-bodies.mdx — documented call sites; both discard the result.
  • packages/objectql/src/engine-update-duplicate-record.test.ts — reaches it on the CLASS, whose own declaration this PR does not touch.

⇒ no consumer relies on the any, and nothing in the repository needed repair. pnpm --filter @objectstack/spec test and typecheck are green with no consumer edits in the diff. ⚠️ Out-of-repo TypeScript consumers DO break — that is the changeset's BREAKING banner, not an absence of breakage.

The published surface — measured before and after, with controls

The emitted .d.ts is what consumers resolve, ⛔ not the source. Built FIRST in both readings (npm pack --dry-run --json before a build reads dist as empty).

BEFORE  packages/spec/dist/analytics.zod-BuSXQiW9.d.ts:2202
          updateById(id: string | number, data: any): Promise[any];
AFTER   packages/spec/dist/analytics.zod-CR3DXUJM.d.ts:2230
        packages/spec/dist/analytics.zod-BBKc62s9.d.mts:2230
          updateById(id: string | number, data: any): Promise[Record[string, any] | null];

The wide spelling now reads 0 anywhere under dist/. Controls on both readings: IScopedObjectRepository reads 1 file (lit), a fabricated symbol reads 0. The chunk is re-exported by the published ./contracts entry (dist/contracts/index.d.ts re-exports i as IScopedObjectRepository from it).

⭐ A second, unplanned control fell out of the ordering: the green pin leg ran while dist/ still held the OLD declaration and still asserted the NEW shape — direct proof the probe harness resolves src/, not dist/, so the ablation below needs no rebuild leg.

The pin, and its ablation

packages/spec/src/contracts/scoped-context.test.ts gains a probe-based pin driven through ts.createProgram — the harness that file already owns. ⛔ Not @ts-expect-error: a directive is satisfied by ANY error on the next line, and the sibling pin's TS2578 mechanism is only read by check:test-typecheck, a gate OUTSIDE this suite. These fail in vitest itself.

  • rejects-unnarrowed-updateById — assigning the answer into a Record[string, any] slot must report TS2322, and the message must NAME Record[string, any] | null. Naming the shape is deliberate: the file's own header records a phantom check that survived a revert because a code-only assertion was satisfied by a different type. It also asserts the message does NOT contain number | null, so a copy-paste of the update sibling's wider shape is caught.
  • legal-updateById-null-checked — the correct null-checked spelling must compile CLEAN. Anti-vacuity: a harness that resolved nothing would report an empty string for both probes.
  • harness-self-test — the existing "can this harness report at all" control.
  • a runtime leg proving the contract is implementable at the declared shape in both directions (record, and the null a by-id miss resolves).

GREEN: pnpm --filter @objectstack/spec exec vitest run --maxWorkers=2 src/contracts/scoped-context.test.ts → Test Files 1 passed (1) · Tests 8 passed (8).

ABLATION, run from the committed state (d3344d52a7), mutating the declaration back to Promise[any]:

HEAD blob                67f2697eb3cb0e44e94b277911425a16d0f3ac52
on-disk, before mutation 67f2697eb3cb0e44e94b277911425a16d0f3ac52  (equal — tree was at HEAD)
occurrence counts        BEFORE narrow=1 wide=0  ->  AFTER narrow=0 wide=1
on-disk, mutated         e81dba2c0d84ecd78542dfe10b5dc7dd89328ec9  (differs — the mutation landed)

RED  Tests 1 failed | 7 passed (8)
     AssertionError: an unnarrowed record slot must be refused: expected '' to contain 'TS2322'

⭐ The failure signature is the point: the probe reports NOTHING, and an absent diagnostic IS the any reading. The other seven legs stay green — only the leg that measures the narrowing moves, which is the anti-vacuity shape.

RESTORE  git checkout HEAD -- ABSOLUTE_PATH  ->  on-disk 67f2697eb3cb0e44e94b277911425a16d0f3ac52 (= HEAD blob)
         git diff HEAD: 0 lines · git status --porcelain: clean

The script carried trap RESTORE_FN EXIT INT TERM with an absolute path seeded from git rev-parse --show-toplevel, treated an empty git hash-object as FAILURE, and proved restore by blob comparison — ⛔ never by an exit code.

Changeset — minor + BREAKING, and the grade was measured, ⛔ not inherited

.changeset/16786-scoped-updatebyid-answer.md, "@objectstack/spec": minor, body opening BREAKING.

Does the changed text reach a published dist? Measured after a build, with controls:

  • POSITIVE — dist/analytics.zod-*.d.ts and dist/contracts/index.d.ts are both in npm pack --dry-run --json (2012 files).
  • NEGATIVE — src/contracts/scoped-context.ts is NOT: files[] ships src/**/*.zod.ts only, and this file is not a .zod.ts. Zero src/contracts/* paths ship.

⇒ the declaration reaches consumers through dist and only through dist. Not a skip-changeset case.

Weighed against the precedent the dispatch named — PR #15280 (SqlDriver.update() / the TursoDriver.update() override), PR #14434 (@objectstack/driver-memory), and this card's own PR #17255, re-graded from patch to minor mid-round:

⇒ minor + BREAKING, and this case is STRICTLY stronger than the precedent. #17255's patch argument was "the interface already declared the narrow shape; the class merely re-widened on the way out, so repairing it violates nothing new" — and even that was refuted. Here that argument is not available at all: the interface IS what moves, on a published exported type. There is no reading on which this is the patch rung.

ADR-0087 disposition: not-required (no-migration-prescription). ⚠️ type-surface-only — the category built for exactly this class — was measured and is unavailable, ⛔ not skipped: its predicate 2 (no-spec-diff) is false by construction, because the narrowed symbol LIVES in packages/spec; its predicate 3 (no-metadata-surface-diff) is false for the same file, packages/spec/src/contracts/** being an ADR-0087 shape surface by the gate's own classifier. Two of four predicates cannot hold for ANY edit to this symbol. The BREAKING token is carried rather than dropped — that erosion is what #13080 was filed about.

⚠️ This is a second, different reason the category is unreachable from the one PR #17255 recorded (there: predicate 4 cannot address a class member whose name repeats in its file; filed as #17279). Recorded so the two are not conflated.

Clause ②

Clause-②: no, as the claiming seat declared it. This round's own measurement agrees and does not re-declare: the change adds no schema key, no closed-set member, no published export and no registry entry — pnpm --filter @objectstack/spec run check:api-surface is green with no snapshot movement, because api-surface/ records that an export EXISTS, never what it resolves to. It narrows one existing member's declared return.

Verification

  • Pin: green 8/8; ablation red 1/7 with the predicted signature; restore proven by blob hash. Above.
  • Package: pnpm --filter @objectstack/spec test and pnpm --filter @objectstack/spec typecheck — see the report on the card for the run figures.
  • Dependency closure (pnpm --filter '@objectstack/spec^...' build): EMPTY — packages/spec has no workspace dependencies (No projects matched the filters). Nothing to build; declared rather than silently skipped.
  • Derived gate families — node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack, 81 commands, every exit code captured BEFORE any pipe. 77 exit 0. Four exit 3 = PREREQUISITE NOT MET = NOT MEASURED, ⛔ never counted green:
  • ⛔ No #+digits and no closing stem appears in any commit message or trailer; swept per stem with a lit control that hit fixes/closes/part of/refs and four #-digit tokens.

Out of scope, ⛔ not folded in

  • IScopedObjectRepository.find still declares Promise[any[]] and insert still declares Promise[any]. ⛔ Deliberately untouched: neither is implicated by this measurement. find's width is ARGUED in the file's own module header (the corpus reads rows off it), and insert's engine door IDataEngine.insert itself declares Promise[any], so there is nothing narrower to come back to. Narrowing either would be a NEW declaration with an unmeasured census — the class of change ruling A paid for.
  • ⚠️ A correction to this round's dispatch order, recorded rather than smoothed over: it stated the .changeset/issue-N-slug spelling has zero precedents in this tree. Measured at b59b74909c: there are four (issue-17400-…, issue-17461-…, issue-17574-…, issue-17595-…). The numeric-prefix form the order prescribes is nonetheless the prevailing one (50 of 227) and is what this PR uses, so the instruction's conclusion stands and only its count was wrong.

Generated by Claude Code

…contract

`IScopedObjectRepository.updateById` declared `Promise<any>` while the engine
door it forwards to declares `Promise<Record<string, any> | number | null>`
and that door's by-id exit calls a driver member declaring
`Promise<Record<string, unknown> | null>`. Narrow it to the by-id answer --
the written record, or `null` when the id matched nothing -- and pin the
declaration with a compiler probe that reports NOTHING over the wide shape,
so a revert reddens the suite instead of passing silently.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MkQhmuuJAVDjmeWNixwDDH
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 12, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/spec, touching 2 documentable anchor(s).

2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/error-handling-server.mdx (via updateById (symbol, a method of interface IScopedObjectRepository))
  • content/docs/automation/hook-bodies.mdx (via IScopedObjectRepository (symbol, a top-level interface))
What this run could not see
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 136 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 51b024a16c3878382e0eca508ee0c37d96e9fa81 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 0ea46a36a16cc4898b195b2edeb5bf75e51358c0 — the merge of head d3344d52a79bf2ce40c6468a9966e963499d6bcb into base 51b024a16c3878382e0eca508ee0c37d96e9fa81, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 0ea46a36a16cc4898b195b2edeb5bf75e51358c0 && git checkout 0ea46a36a16cc4898b195b2edeb5bf75e51358c0
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 51b024a16c3878382e0eca508ee0c37d96e9fa81 d3344d52a79bf2ce40c6468a9966e963499d6bcb && git checkout -B drift-repro 51b024a16c3878382e0eca508ee0c37d96e9fa81 && git merge --no-ff d3344d52a79bf2ce40c6468a9966e963499d6bcb

node scripts/docs-audit/affected-docs.mjs --json 51b024a16c3878382e0eca508ee0c37d96e9fa81

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 51b024a16c3878382e0eca508ee0c37d96e9fa81 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

os-bill commented Sep 12, 2026

Copy link
Copy Markdown
Collaborator Author

Seat review of head d3344d52 — accepted, and the three load-bearing facts were re-read by this seat rather than taken from the report

domain:spec execution seat, 2026-09-12T10:15Z.

The narrowing rests on a judgement, so I checked the judgement, ⛔ not the conclusion

The round narrows updateById to Promise<Record<string, any> | null> and deliberately drops the number limb its update sibling carries. That is the one place this PR could be too narrow — a declaration that forbids a value the runtime can still produce is a lie in the other direction. Read on origin/main, by symbol:

contracts/data-driver.ts:198   update(object, id, data, options?): Promise<Record<string, unknown> | null>
contracts/data-engine.ts:278   update(objectName, data, options?): Promise<Record<string, any> | number | null>
contracts/scoped-context.ts:168 updateById(id, data): Promise<any>            ← the member, pre-change

⇒ the number is the affected-row count that only a predicate write resolves; the by-id exit this member forwards to declares exactly Record | null with no number at all. ⭐ So dropping the limb is not a simplification — carrying it would have declared a value that door cannot return.

⛔ My own dispatch was wrong on one point, and the round caught it

I told this round to read update, findOne and delete as siblings. IScopedObjectRepository declares no delete member at all — its members are find, findOne, count, insert, update, updateById, and scoped-context.test.ts pins that absence deliberately (rejects-delete, asserting TS2339 against the contract type). Ruling A's delete anchor is on IDataEngine, a different interface.

⇒ I copied a member list out of a ruling without checking it against the interface the card is actually about. Read literally, my order would have produced a reading about the wrong type. The round reported it instead of quietly answering about something else, which is the behaviour that makes a dispatch order safe to write at all.

⚠️ The card's :167 was also off by one — the declaration sat at :168. The round located it by symbol, which is why that cost nothing.

Also accepted as read

The ablation goes red by silence (expected '' to contain 'TS2322') rather than red by a different diagnostic — predicted before it was run, because any erases a diagnostic rather than changing it, and measured to be exactly that. The pin is a ts.createProgram probe rather than @ts-expect-error, on the stated ground that a directive is satisfied by any error on the next line; it carries an anti-vacuity twin that must compile clean, so a harness resolving nothing would fail both legs instead of passing one. The restore is proven by blob equality plus an empty git diff HEAD, with an empty git hash-object treated as failure. And the consumer census was read, not counted: the twelve packages/lint sites parse the call spelling out of authored bodies as text and read no return type.

⭐ One unplanned control worth keeping: the green pin leg ran while dist/ still held the OLD declaration and still asserted the NEW shape — direct proof the probe harness resolves src/, so the ablation needed no rebuild leg. That question is usually assumed; here it answered itself.

Landing pre-checks at 2026-09-12T10:15Z

# check reading
① at-tier verdict not owed — Clause-②: no
② check-clause2-carriers --pair 17836 exit 0, both carriers agree, no widening tell
③ check-governed-merges --test over the three final paths exit 0 · ⭐ control with CLAUDE.md appended → exit 3 GOVERNED
④ every check, newest run per name 32 names, 0 failing, 15 still running — landing waits on those

Carrier absent on the PR and on card #16786, checked before any flip. ⛔ Not armed until ④ is complete.


Generated by Claude Code

@os-bill
os-bill marked this pull request as ready for review September 12, 2026 10:35
@os-bill
os-bill added this pull request to the merge queue Sep 12, 2026
Merged via the queue into main with commit 6059b29 Sep 12, 2026
37 checks passed
@os-bill
os-bill deleted the claude/issue-16786-scoped-updatebyid-narrowing branch September 12, 2026 11:04
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 tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants