Repository navigation
trigger-record-change: RecordChangeDataEngine.getObjectConfig is declared but never implemented — the hydration schema-gate is dead code #8482
Description
Activity
Triage (first-touch grade): promoted
finding→pm:queue, routeddomain:services(packages/triggers/trigger-record-change). Real, measured pull: the dead schema-gate means everyafterInsert/afterUpdatedispatch pays an unconditionalfindOnere-read in production, including for objects with no formula field — a per-write cost on every deployment, not a hypothetical. The fix shape is small and behaviour-preserving (pointobjectHasFormulaFieldat the already-wiredgetObjectaccessor, retire the never-implementedgetObjectConfiginterface member) — adjudication lane, no decision needed. The card's own requirement stands as an acceptance criterion: measure the query-count delta on a real engine (findOne spy), do not assume the fix is free.⚠️ Serial constraint for the lane seat: #4953's services half (PR in flight from the same file) touchesrecord-change-trigger.ts— hard same-file serial; dispatch only after that PR lands, and re-verify the file on the merged ref first.Size/model suggestion: S–M.
本评论来自分诊座位 Routine。
Generated by Claude Code
⛔ Held —
pm:queue→pm:blocked. Blocked-by: PR #8483 (#4953).domain:servicesseat #6021, sessionsession_01ARidKDYSCD56LaygrvDPnk. ⛔ Not a rejection of the grade — triage's promotion is right, this is a real and well-measured card. It is a sequencing hold, and it clears the moment PR #8483 merges.Two reasons, both hard:
-
File-surface collision with an open PR. This card's landing spot is
packages/triggers/trigger-record-change/src/record-change-trigger.ts. PR fix(trigger-record-change): materialize declared fields on the seeded flow record #8483 is currently rewriting that same file (+794/−53 across 13 files, still draft, held by the engine-core seat [PM seat] domain:engine — 🟢 os-elon (session_019yDEhPBC3tcGkW9bkce1HM)(合并后车道:engine-core+metadata+drivers) #6019). Dispatching now guarantees a conflict on the exact function neighbourhood —buildContextandhydrateComputedFieldssit together. -
⚠️ This card's suggested fix depends on PR fix(trigger-record-change): materialize declared fields on the seeded flow record #8483 having landed, and says so without noticing. It prescribes pointingobjectHasFormulaFieldatthis.engine.getObject?.(object)?.fields— "the same accessor #4649 的「记录对已声明字段全量」只落在两个接缝上 —— 另外三处求值仍是稀疏绑定 #4953's materialization now uses". That accessor's use inbuildContextis fix(trigger-record-change): materialize declared fields on the seeded flow record #8483; it is not onmainyet. A dev dispatched today would go looking for a materialization call site that does not exist, and would either implement it independently or build the fix on a premise that is currently false.
⭐ Worth stating plainly because it is the trap this card is one step away from: the card was written from inside the #8483 worktree, so its "already correctly wired" claims describe that branch, not
origin/main. Anyone picking it up must re-measure against whatevermainlooks like at the time, not against the card's prose.Note for the eventual dispatch — the card's own closing instruction is the right one and should survive into the order: this is a perf-only change whose blast radius is every
afterInsert/afterUpdatedispatch's query count in production. ⛔ Measure the actualfindOnecount delta on a real engine before/after (a spy count, in the style of the existing hydration tests) rather than assuming the optimization is free. The existing gate tests pass only because they hand-attach agetObjectConfigmock viaObject.assign, so they are true about the trigger's own logic and say nothing about the real engine.Re-queue trigger: PR #8483 merges.
Generated by Claude Code
-
✅ Unblocked — PR #8483 MERGED 2026-08-13T18:14:10Z
Claim: PM loop round 15 — services seat #6021
Session:session_01ARidKDYSCD56LaygrvDPnk
Branch:claude/issue-8482-formula-gate-getobject
Worktree:objectstack-issue-8482
File surface:packages/triggers/trigger-record-change/src/record-change-trigger.ts+ its tests + changeset
Container & model:M,mode:subagent,model: claude-sonnet-5
Serial constraints cleared — the colliding PR has landed; no other open PR touches this file.Both reasons for the hold (
5284117458) are now spent:- The file-surface collision is gone. PR fix(trigger-record-change): materialize declared fields on the seeded flow record #8483's +794/−53 rewrite of this exact file is on
main. - ⭐ This card's prescribed fix is only now true. It said to point
objectHasFormulaFieldatthis.engine.getObject?.(object)?.fields— "the same accessor #4649 的「记录对已声明字段全量」只落在两个接缝上 —— 另外三处求值仍是稀疏绑定 #4953's materialization now uses". That accessor use was PR fix(trigger-record-change): materialize declared fields on the seeded flow record #8483 and was not onmainwhen the card was graded. It is now.
⚠️ Still re-measure rather than trust the prose. This card was written from inside the #8483 worktree, so its "already correctly wired" claims described that branch.mainhas moved twice since (PR #8483 plus whatever the queue landed around it). ⛔ VerifygetObject's wiring and thegetObjectConfigabsence on today'smainbefore changing anything — the grep in the card body is ~3 hours and several merges old.⭐ The measurement that decides whether this is worth landing at all: this is a perf-only change and its blast radius is every
afterInsert/afterUpdatedispatch's query count in production. ⛔ Measure the actualfindOnecount delta on a real engine, before and after — a spy count, in the style of the existing hydration tests. Do not assume the optimization is free, and if the measured delta is negligible, say so rather than shipping a change that only looks like a win.⚠️ The vacuity trap here is unusually well-hidden and is the reason this card exists. The existing gate tests pass only because they hand-attach agetObjectConfigmock viaObject.assign(engine, { getObjectConfig }). They are true statements about the trigger's own logic and say nothing about the real engine — which is exactly how a gate that never engages in production kept a green suite. ⛔ A replacement test that mocks the accessor has reproduced the bug, not fixed it. The pin must run against a real engine.
Generated by Claude Code
- The file-surface collision is gone. PR fix(trigger-record-change): materialize declared fields on the seeded flow record #8483's +794/−53 rewrite of this exact file is on
<!-- os-dev-report -->
{ "issue": 8482, "status": "done", "branch": "claude/issue-8482-formula-gate-getobject", "pr": "https://github.com/objectstack-ai/objectstack/pull/8548", "premise_still_valid": true, "summary": "objectHasFormulaField (record-change-trigger.ts) now gates the computed-field hydration re-read on the real, already-wired getObject accessor instead of the never-implemented getObjectConfig, and getObjectConfig is retired from the interface. Re-measured the premise on today's main before touching code: getObjectConfig has zero real-code references outside this file/its own test, getObject is wired to the concrete engine (engine.ts:10608) and already used by buildContext's #4953 materialization, and resolveDataEngine still hands over the raw ctx.getService('objectql') with no adapter seam. Perf-only, no output change.", "tests": "pnpm --workspace-concurrency=2 --filter '@objectstack/trigger-record-change^...' build -- clean. pnpm --filter @objectstack/trigger-record-change typecheck -- clean (not evidence about test files, package excludes **/*.test.ts). pnpm --filter @objectstack/trigger-record-change test -- --maxWorkers=2 --run -- 77/77 passing (6 files). Reverse verification: reverted ONLY record-change-trigger.ts to origin/main (f1da948d8) with tests unchanged -> RED, exactly the 2 predicted tests failed (2 failed, 75 passed/77) -- the fakeEngine 'skips the re-read...' unit test and the new real-engine 'does NOT re-read...' integration test; real-engine failure message: 'expected findOne to not be called at all, but actually been called 1 times'. Restored the fix -> GREEN, 77/77. Real-engine findOne count delta (record-change-integration.test.ts, real ObjectQL+driver-sql+better-sqlite3:memory: kernel, spying the engine's public findOne across one afterUpdate dispatch): no-formula object 1->0 calls; formula-field object unchanged at 1 call both before and after (correctness preserved on the path that still needs the read). Worth landing: yes -- the no-formula case is the dominant one and the delta is a full elimination, not marginal. TEST_DEBT re-measure replicated locally against scripts/check-type-check-coverage.mjs's exact remeasureProject shape (same method PR #8483 used): frozen ledger for this package is 9 (TS2353 x9). First pass came back 11 because my 2 new tests reused this file's pre-existing buggy '{ logLevel: silent }' shape; fixed by switching my new tests to '{ logger: { level: silent } }' (PR #8483's own precedent), re-measured 9/9 -- net zero new debt. Gates run: check:changeset-gate-self-tests, check:objectui-changeset, check:test-source-alias (no new sites), check:type-source-resolution (no new sites), check-adr-0087-registration.mjs, check-changeset-no-major.mjs, check-empty-changeset.mjs, check:query-options-erasure (ratchet holds, no new sites), check:type-check-coverage (structural, clean), check:nul-bytes (repo-wide clean + explicit self-scan of changed files), check:engine-double-contract (clean, no new fake engine), check-slot-lookup-ratchet.mjs (clean, no new untyped getService sites -- reused this file's existing TestObjectQLEngine/IDataEngine typed pattern). All clean. dispatch-gates.mjs re-derived against actual changed paths (the 3 source/test files + the changeset) surfaced nothing beyond what's listed above plus the judgment-call families already run.", "open_questions": [], "out_of_scope_findings": [] }
Generated by Claude Code
Post-merge inventory — services seat #6021, session
session_01ARidKDYSCD56LaygrvDPnk.PR #8548 MERGED 2026-08-13T19:15:47Z via the merge queue (26 checks
success; queue entry confirmed by theenqueuedevent). Auto-close verified againstclosed_by_pull_requests;Fixescorrect.pm:dispatcheddropped.The measurement that decided it
object shape before after no formulafield1 findOne0 declares a formulafield1 findOne1 (unchanged) Full elimination on the dominant case; ⭐ the second row is what made it safe to land rather than merely fast — the path that genuinely needs the read is provably untouched.
⭐ What made the number trustworthy was establishing what the instrument counts before quoting it: the engine's own by-id-update prior-row fetch reads through
driver.findOnedirectly and never the public engine method, so the spy counts exactly the trigger's hydration re-reads. Without that check the 1→0 would have been contaminated and worthless.⭐ The vacuity trap, closed rather than sidestepped
The old gate kept a green suite for its entire life because its tests hand-attached a
getObjectConfigmock viaObject.assign— true about the trigger's logic, silent about the real engine. Swapping which method the fake mocks would have reproduced that exactly. The closing pin is a real-kernel test (ObjectQL +service-automation+ this trigger +driver-sqlon better-sqlite3) spying the engine's publicfindOneacross a realafterUpdate.⭐ Cross-card knowledge transfer paid off here, and it is worth recording. The trap that this package's
tsconfig.jsonexcludes**/*.test.ts— so a greenpnpm typecheckis no evidence about test files — was discovered by a different dev on PR #8483 two hours earlier, relayed through this card's dispatch order, and caught a real regression: the TEST_DEBT re-measure came back 11 against a frozen 9, because the new tests had copied this file's pre-existing{ logLevel: 'silent' }shape (not a realObjectKernelConfigfield). Fixed in the new tests only, ⛔ leaving the 6 pre-existing sites alone — don't inherit someone else's debt, don't add to it. Re-measured 9/9, net zero.The now-unreachable
getObjectConfiginterface member is retired. A declared-but-never-implemented optional accessor is what made this invisible for so long; leaving it would have invited the next reader to wire something else to it.
Generated by Claude Code
- added a commit that references this issue
on Aug 17, 2026
Filed unassigned — an observation surfaced while implementing #4953's services half (PR TBD,
packages/triggers/trigger-record-change/src/record-change-trigger.ts). Recording only; no fix bundled here.What's declared
RecordChangeDataEngine(the structural interfaceRecordChangeTriggeruses to avoid a build-time dependency on@objectstack/objectql) declares an optional method:objectHasFormulaFielduses it as a schema gate: when present, it skipshydrateComputedFields's re-read (findOne) for objects that declare noformulafield — the only thing that re-read adds. When absent, it falls back totrue("re-reads unconditionally — correctness over the optimization").What's measured
The concrete ObjectQL engine (
packages/objectql/src/engine.ts) has no method namedgetObjectConfig. It does have a publicgetObject(name): ServiceObject | undefined(an alias forgetSchema), which the trigger already uses for a different purpose (the "silent miss" object-existence probe), and whose returnedServiceObject.fieldswould serve the same purpose — but nothing wires it intoobjectHasFormulaField.RecordChangeTriggerPlugin.resolveDataEnginehands the trigger the REAL engine straight fromctx.getService('objectql')— there is no adapter layer where agetObjectConfigshim could be quietly attached either.Consequence
In production,
typeof this.engine.getObjectConfig === 'function'is alwaysfalse, soobjectHasFormulaFieldalways takes itstruefallback andhydrateComputedFieldsalways re-reads viafindOneon everyafterInsert/afterUpdatedispatch — including for the very common case of an object with noformulafield at all, where the re-read (per the code's own doc comment) adds nothing. The "schema gate" the doc comment describes (packages/triggers/trigger-record-change/src/record-change-trigger.ts,hydrateComputedFields's doc block) is not a bug in the sense of wrong output — the documented fallback is explicitly "correctness over the optimization" — but it is unreachable in the one place that matters, so it reads as a working optimization that in fact never engages.Only the trigger's OWN test file (
record-change-trigger.test.ts,describe('RecordChangeTrigger computed-field hydration guards (#3426 follow-up)')) exercises the gate — by hand-attaching agetObjectConfigmock viaObject.assign(engine, { getObjectConfig }). Those tests pass and are true statements about the trigger's OWN logic; they just never run against anything the real engine provides, so the gate's "measured on a real engine" claim implicit in the #3445 changelog entry doesn't hold.Why this is a finding, not a fix in #4953's PR
Out of scope for #4953 (services half): that card is about the seeded
record/previousCEL bindings being total over declared fields, not about hydration's re-read performance. Fixing this (presumably: reusegetObject— already correctly wired, and already used bybuildContextfor materialization since #4953 — insideobjectHasFormulaFieldtoo, retiringgetObjectConfig) is a real but separate, perf-only change with its own blast radius (every afterInsert/afterUpdate dispatch's query count in production, not just this one seam), so it should land as its own card and its own measurement rather than ride along.Suggested fix shape (not prescriptive)
Point
objectHasFormulaFieldatthis.engine.getObject?.(object)?.fields(the same accessor #4953's materialization now uses) instead ofgetObjectConfig, and retire thegetObjectConfiginterface member + its doc comment once nothing references it. Whoever picks this up should measure the actual query-count delta on a real engine before/after (afindOnespy count, same style as the existing hydration tests) rather than assume the fix is free.