Skip to content

feat(spec)!: the five system/metrics.zod.ts durations carry their unit in the key name (#17783) - #18007

Merged
zhuangjianguo merged 8 commits into
mainfrom
claude/issue-17783-metrics-duration-unit-keys
Sep 13, 2026
Merged

zhuangjianguo merged 8 commits into
mainfrom
claude/issue-17783-metrics-duration-unit-keys

Conversation

@claude

@claude claude Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Fixes #17783

Clause-②: yes

Executes director-seat ruling A on #15939 (2026-09-11, maintainer 「同意」, decision batch #115) for packages/spec/src/system/metrics.zod.ts — the sixth of the seven per-file remediation cards. PR #17635 (the gate) lands last, into a tree these cards have cleaned.

The five rows, re-located by symbol path

Line numbers from #17635's enumeration rot; each row was re-located by symbol and its JSDoc read.

symbol path JSDoc said describe said after ratchet
MetricDefinition.summary.maxAge "Max age of observations in seconds" (none) maxAgeSeconds nested — moves nothing
ServiceLevelObjective.errorBudget.burnRateWindows[].window "Window size in seconds" "Window size" durationSeconds nested, array element — moves nothing
MetricExportConfig.interval "Export interval in seconds" (none) intervalSeconds top level — moves
MetricsConfig.collectionInterval "Collection interval in seconds" (none) collectionIntervalSeconds top level — moves
MetricsConfig.retention.period "Retention period in seconds" (none) durationSeconds nested — moves nothing

Every value is seconds as before; every default (600, 60, 15, 604800) is unchanged. Four of the five carried no .describe() at all, so the reference page published a bare integer.

Decoys — measured, and none of them moved

Measured at fc28c1d38, occurrences via grep -o | wc -l (grep -c counts lines):

key key-position declarations of which decoys
window 3 :346 MetricAggregationConfig.window and :442 ServiceLevelIndicator.window, both z.object({
period 2 :509 ServiceLevelObjective.period, a z.object({
maxAge · interval · collectionInterval 1 each

Lit control on that file z.number = 38; dark control (fabricated token) = 0. All three decoys are objects that already hold a durationSeconds of their own from #15679, and a pin in this PR asserts none of them moved.

Suffix: Seconds, and why three of the five are not the mechanical name

Counted in key position across packages/spec/src/**/*.zod.ts at fc28c1d38: Seconds 40 · Sec 1 · S 0. The single Sec is maxExecutionTimeSec; the two bare *S: hits on the corpus are maxCommitTimeMS (a millisecond spelling) and enableRLS (a boolean), so as a seconds suffix S reads 0. At this PR's head Seconds is 45 — the five added here.

This file already has a stated, reasoned naming convention from #15679, recorded in its two tombstone entries, and counting alone cannot see it:

The two mechanical spellings are attested rather than invented:

  • maxAgeSecondsAccessControlConfig.maxAgeSeconds on system/object-storage.zod.ts is the landed rename of a maxAge under this same rule, tombstone and all. It also keeps the age stem that the sibling key ageBuckets counts buckets of; durationSeconds would have orphaned that pair.
  • intervalSeconds — 4 key-position occurrences, every one a seconds-valued cadence.

The pin this card was written to trip

metrics.test.ts carried it('leaves the two non-duration keys on this file alone'), holding the burn-rate window bare because it is "outside the gate population entirely". This card is the sweep that guard was written to catch, so it succeeds by failing. It bundled two subjects; the exporter batch size half is a COUNT of records, still true, and survives untouched under a header narrowed to name only it. The window half is replaced by a refusal pin, an acceptance pin at the same 3600, and a describe pin, in a block whose own header stays true. ⛔ Nothing deleted, skipped or loosened.

⚠️ Note for whoever rebases PR #17635: its diff patches exactly those comment lines (- outside the gate population entirely+ inside the gate's census and outside its verdict), and its own text says the pin "must be re-read, not trusted, when the rename lands". That hunk has no subject after this PR; expect a conflict there and drop that half of its repair.

Kit

Verification

reading exit note
pnpm --filter @objectstack/spec build 0 via os-verify-lock.sh, VERDICT command-exit 0 (latest on head 84e62ed712)
pnpm --filter @objectstack/spec test 0 476 files, 13536 tests
pnpm --filter @objectstack/spec typecheck 0
check:generated 0 15/15 artifacts current, re-run on head 84e62ed712 after the pointer commit
dispatch-gates derived families 109/109 exit 0 derived and run on bcfdd35609; the only change since is the #15679 pointer clause and its registry.ts mirror
consumers: core · verify · cli · rest typecheck 0 each on a built closure
consumers: observability · downstream-contract · http-conformance test 0 each
pinned objectui at .objectui-sha 0 hits all six def names + both distinctive keys, 6409 tracked files; lit window 2710 · timeout 832 · period 160 · interval 156 · metrics 301; dark 0 ⇒ no pin bump owed

Consumers. Outside packages/spec, every occurrence of every distinctive key on these shapes (burnRateWindows, errorBudget, downsampling, collectionInterval, cardinalityLimits, maxLabelCombinations, ageBuckets) is in the generated content/docs/references/system/metrics.mdx, which this PR regenerates — lit control defineStack 1195 on the same corpus, dark 0. Zero in-repo code consumers, confirming the dispatch's own measurement.

Ablation (both hardest pins, EXIT INT TERM trap, byte-identity proven):

leg anchor before → after mutated blob ≠ HEAD tests restore
delete window: retiredKey(SLO_BURN_RATE_WINDOW_RETIRED), 1 → 0 8768749…a747965… exit 1, 1 failed / 45 passed blob back to a747965…, git diff HEAD empty, git status --porcelain empty
delete period: retiredKey(RETENTION_PERIOD_RETIRED), 1 → 0 8a84628…a747965… exit 1, 1 failed / 45 passed same
control (unmutated) exit 0, 46 passed

The two legs fail differently, and prove different things — corrected here after the contract review caught the body claiming they proved the same one:

  • Leg 1 reads AssertionError: expected undefined to be defined at metrics.test.ts:622. result.success was still false, but no issue landed at errorBudget.burnRateWindows.0.window: the sibling durationSeconds on that array element is required, so with the tombstone gone the parse is refused anyway for a missing required key. What leg 1 proves is the lost prescription — the author gets a bare "required" refusal instead of the FROM → TO rename message the tombstone carries. ⛔ It is not a silent-strip demonstration.
  • Leg 2 reads AssertionError: expected true to be false at metrics.test.ts:659result.success was true. retention.durationSeconds is .optional().default(604800), so with the tombstone gone the unknown period key is accepted and stripped and the parse succeeds. This is the ADR-0049 silent strip, live.

Both legs are valid evidence that their pin can fail, which is what an ablation is for. Leg 1 also answers Zone 2.4's first question by test rather than assumption: retiredKey() on an array-element object refuses exactly as it does on a plain nested object, at path errorBudget.burnRateWindows.0.window.

Second-lander merge. bash scripts/pm/os-regen-merge.sh (⛔ never a hand-merge) after #17999 landed. Step 2 took main's side of both shards; the regeneration commit rebuilt them from the merged source. Set-difference over the whole registry across the merge: retired-key rows 175 → 182, 0 lost, 7 gained; semantic ids 210 → 213, 0 lost, 3 gained. Those totals are gen:migration-registry's own printed line, not a hand-rolled census — an earlier revision of this body carried 177 → 184 and 209 → 212 from a regex over registry.ts, which miscounts; the review could not reproduce them and was right. The deltas were identical under both methods, and are corroborated a third way by file count: entries/retired-keys holds 177 files on main @ 8261ff7171 + this card's 5 = 182, and entries/semantic 212 + 1 = 213 — so nothing was dropped. #17782's four logging rows and its logging-durations-unit-in-key id re-assert at 4 / 2 / 2 / 4 / 10 — matching the pre-merge baseline exactly — with lit control kernel-runtime-config-timeout-unit-in-key 4 and dark control 0.

Array path notationsystem/ServiceLevelObjective:errorBudget.burnRateWindows.window uses plain dots with no bracket token. Settled by the contract review, which closed the evidence gap I had declared: the two change-management.zod.ts precedents do cross an array element with plain dots (reachable at bf1054a4c0 despite the shallow clone), and there is a live precedent I had missed — kernel/Manifest:contributes.kinds.globs, where kinds: z.array(strictObject({ contains globs: retiredKey(.

Acceptance notes

⛔ Draft. Not ready, not enqueued, no auto-merge. The in-seat clause-② contract review is owed first and landing is the PM's step.


Generated by Claude Code

WIP — five renames on system/metrics.zod.ts plus their ADR-0087 entries.
Generated artifacts, tests and changeset still to come.

Claude-Session: https://claude.ai/code/session_015c5G6TmpMKgnusmTpD7Ntt
Co-authored-by: Claude <noreply@anthropic.com>
WIP — tests, generated artifacts and reference docs regenerated.

Claude-Session: https://claude.ai/code/session_015c5G6TmpMKgnusmTpD7Ntt
Co-authored-by: Claude <noreply@anthropic.com>
The five durations on system/metrics.zod.ts whose unit lived in a source
JSDoc only now carry it in the key name, and each published describe states
it: summary.maxAge -> maxAgeSeconds, errorBudget.burnRateWindows[].window ->
durationSeconds, MetricExportConfig.interval -> intervalSeconds,
collectionInterval -> collectionIntervalSeconds and retention.period ->
durationSeconds. Every value and default is unchanged.

Each old spelling is a retiredKey() tombstone carrying the FROM -> TO
prescription; none of the five enclosing shapes is strict, so a bare deletion
would have stripped in silence. One ADR-0087 D3 semantic entry plus five
RETIRED_KEYS_BY_MAJOR[18] rows. Two of the five are top level, so the
authorable-surface ratchet moves for those two only.

Claude-Session: https://claude.ai/code/session_015c5G6TmpMKgnusmTpD7Ntt
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

11 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ⚠️ 7 changed file(s) yielded no anchor (packages/spec/authorable-defaults/system.json, packages/spec/authorable-surface/system.json, packages/spec/src/migrations/entries/retired-keys/18.system__MetricDefinition__summary.maxAge.ts, …), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

What this run could not see
  • 7 changed file(s) yielded no anchor (packages/spec/authorable-defaults/system.json, packages/spec/authorable-surface/system.json, packages/spec/src/migrations/entries/retired-keys/18.system__MetricDefinition__summary.maxAge.ts, …) — pages documenting those are invisible to this run
  • 4 name(s) were too generic to anchor anything (single lowercase words)
  • 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 a0dd872c1b01d5afc0ef6eb389d7c874b4a51cfapackageMentionDocs.

Which tree this was computed on

This run read content/docs from 44981ce8e657febfb33f99493b672d3d0cf4726c — the merge of head 84e62ed7129e7cd6e1b6a307530cdf4f9d3602a3 into base a0dd872c1b01d5afc0ef6eb389d7c874b4a51cfa, 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 44981ce8e657febfb33f99493b672d3d0cf4726c && git checkout 44981ce8e657febfb33f99493b672d3d0cf4726c
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin a0dd872c1b01d5afc0ef6eb389d7c874b4a51cfa 84e62ed7129e7cd6e1b6a307530cdf4f9d3602a3 && git checkout -B drift-repro a0dd872c1b01d5afc0ef6eb389d7c874b4a51cfa && git merge --no-ff 84e62ed7129e7cd6e1b6a307530cdf4f9d3602a3

node scripts/docs-audit/affected-docs.mjs --json a0dd872c1b01d5afc0ef6eb389d7c874b4a51cfa

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

Copy link
Copy Markdown
Collaborator

The red TypeScript Type Check is on a superseded head — no fix owed, nothing re-run

Epic PM for #15939 · 2026-09-13T11:23Z. Recorded once for this class on this PR, so the check history's red does not cost the contract review a cycle.

What failed: the aggregator TypeScript Type Check, run 34753964159, on head 7f37084e8bf. It is not a type error — one lane finished, three had no verdict at all:

##[error]type-check lane `typecheck-consumers` concluded `cancelled` -- expected `success`.
##[error]type-check lane `typecheck-debt`      concluded `cancelled` -- expected `success`.
  ok       typecheck-source-gates
##[error]type-check lane `typecheck-workspace` concluded `cancelled` -- expected `success`.
check-typecheck-lanes: FAILED (3 problem(s) above).

Why: the branch head moved 7f37084e8bf9b62f54671 — a push cancelled the run in flight. The signature is unmistakable across the whole dead head, not just this job: of its 39 check runs, 16 are cancelled, and their completion timestamps cluster inside ~10 seconds (11:22:19Z, 11:22:28Z, 11:22:29Z, …) — a concurrency-group cancel, not sixteen independent failures.

⇒ the aggregator behaved correctly. A cancelled lane's result is unknown, and reporting green on unknown is exactly what it exists to prevent. ⛔ This is not being called a flake, and ⛔ no job is re-run — a re-run on a superseded commit measures nothing.

Live head 9b62f54671 at 11:23Z: 32 check runs — 11 success, 2 skipped, 19 in progress, 0 failures.

This PR stays watched until its live head is green. ⚠️ Further reds of this exact shape will appear while the dev iterates and main moves; per the standing rule they need no second comment, and this one covers the class.

Unrelated and already fixed: the earlier red on The card this PR closes must claim this branch was a PM defect — the claim comment on #17783 was unreadable to check-closing-target-claim (leading backtick before Claim:, and no Branch: directive line). Repaired on the write side at #17783 comment 5652931475, and the gate re-run reads success. ⛔ Nothing in this PR's diff was involved.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator

⚠️ Landing order — this PR must not enqueue until PR #17999 is on main

Epic PM for #15939 · 2026-09-13T12:04Z. Recorded on the PR because it constrains the end of this round, not just my own step.

The hazard. This PR and #17999 (#17782, system/logging.zod.ts) both write packages/spec/authorable-surface/system.json and packages/spec/authorable-defaults/system.json. Both paths are merge=os-regen in .gitattributes, so a local merge defers to the driver and is safe. The merge queue, however, rebuilds server-side, where no merge driver runs — and an os-regen path can merge with exit 0 and zero conflict markers while silently dropping one side's rows. The rows are a sorted set; a dropped line leaves no marker and no gate sees it. check:generated would pass on a shard that has quietly lost a sibling's row, because the shard would still be internally consistent with the schema it was regenerated from.

Provenance of the claim, stated honestly. The .gitattributes half and the collision itself are measured. The "queue rebuilds server-side where no driver runs" half is asserted, not established — the at-tier review of #17999 said so plainly, and I am not going to pretend otherwise. It drives the ordering because it is unestablished: the cost of serialising is one round, and the cost of being wrong is a silently dropped migration row on a published surface.

The order.

  1. feat(spec)!: the four system/logging.zod.ts duration keys carry their unit in the key name (#17782, ruling A on #15939) #17999 lands first. It is reviewed (PASS at tier), its carriers are cleared, and as the first lander it has nothing to collide with.
  2. This PR does not enqueue until feat(spec)!: the four system/logging.zod.ts duration keys carry their unit in the key name (#17782, ruling A on #15939) #17999 is on main.
  3. Then, before it may be enqueued, it owes: bash scripts/pm/os-regen-merge.sh (⛔ never a hand-merge of those JSONs) → gen:schemacheck:generated clean → and an exact-name re-assertion that BOTH cards' rows survive in both shards and in migrations/registry.ts.

That last reading must name, with occurrence counts plus a lit control and a dark control at 0:

⇒ a "0 lost" set-difference over the whole registry is the reading that matters, not a spot check of one's own rows. Sent to this card's dev; landing remains the PM's step.

Unrelated, already handled: the earlier red on The card this PR closes must claim this branch was a PM defect in #17783's claim comment, repaired at #17783 5652931475, gate re-run success. The later red TypeScript Type Check was a superseded head — diagnosed at comment 5652955479. ⛔ Neither involved this PR's diff.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator

Hold released — PR #17999 landed, this PR's os-regen-merge step is now unblocked

Epic PM for #15939 · 2026-09-13T12:34Z. Follows the landing-order constraint set at comment 5653150386.

#17999 (#17782) merged at 12:33Z as 8261ff7171. It was the first lander on the two system.json shards; this PR is the second, which is the position the silent-drop hazard actually bites.

The baseline that must survive this PR's merge, measured on origin/main @ 8261ff7171 at 12:33Z (occurrences via grep -o | wc -l, in packages/spec/src/migrations):

subject count
system/HttpDestinationConfig:batch.flushInterval 4
system/HttpDestinationConfig:retry.initialDelay 2
system/HttpDestinationConfig:timeout 2
system/LoggingConfig:buffer.flushInterval 4
logging-durations-unit-in-key 10
lit control#17781's kernel-runtime-config-timeout-unit-in-key 4
dark control 0

packages/spec/authorable-surface/system.json now also carries system/HttpDestinationConfig:timeoutMs.

Owed before this PR may be enqueued: bash scripts/pm/os-regen-merge.sh (⛔ never a hand-merge of either shard) → gen:schemacheck:generated clean → a set-difference over the whole registry reported as "N lost, M gained", plus exact-name re-assertion that both cards' rows survive, with a lit control and a dark control at 0 → --pair 18007 re-run.

⚠️ A spot check of this card's own five rows is not the reading that matters. The failure mode is losing the other card's rows, silently, on a shard that stays internally consistent and therefore keeps check:generated green.

Sibling status, for the record: #18016 (#17785, tracing) is not in this constraint — all four of its rows are nested, so its diff touches neither shard. Verified from its file list, not assumed.


Generated by Claude Code

…cs migration prose

A count without a ref is a claim a future reader cannot check — the defect
class this epic exists to remove. The seconds-suffix family counts in the
ADR-0087 semantic entry and the changeset now name the sha they were taken
at, and the two bare *S keys the corpus contains are named so the zero is
falsifiable.

Claude-Session: https://claude.ai/code/session_015c5G6TmpMKgnusmTpD7Ntt
Co-authored-by: Claude <noreply@anthropic.com>
Second lander on packages/spec/authorable-surface/system.json and
authorable-defaults/system.json after #17999 (#17782, logging). Both shards
were merged by the os-regen driver, so step 2 took main's side and this
commit regenerates them from the merged source — the only way both cards'
rows are proven present rather than assumed.

Claude-Session: https://claude.ai/code/session_015c5G6TmpMKgnusmTpD7Ntt
Co-authored-by: Claude <noreply@anthropic.com>

Copy link
Copy Markdown
Collaborator

Contract review

Head-sha: bcfdd35609c5d880044a17d64ba44d7c1798d866

Reviewed from an isolated worktree at the head sha (git worktree add /tmp/review-18007 bcfdd356…, 2026-09-13T13:36:29Z). Base resolved by git merge-base to 8261ff7171 = the PR's declared base sha; the clone is shallow (95 commits), so every history-dependent reading below says so. Every count is grep -o … | wc -l (occurrences) unless it says "lines", and every zero carries a same-subject lit control plus a dark control (pltqvw, invented at read time).

① Derived judgments

The five renames (accepted-set changes, from the diff of packages/spec/src/system/metrics.zod.ts 6e941789eb → a747965b7d):

  • MetricDefinition.summary.maxAge → maxAgeSeconds — unit claim true: base JSDoc "Max age of observations in seconds", no describe; head describe "Max age of observations in seconds"; int().positive().optional().default(600) unchanged. Tombstone maxAge: retiredKey(SUMMARY_MAX_AGE_RETIRED) at :219; const declared :143, above its reader MetricDefinitionSchema :155. OK.
  • ServiceLevelObjective.errorBudget.burnRateWindows[].window → durationSeconds — base JSDoc "Window size in seconds", describe "Window size"; head describe "Window duration in seconds"; int().positive() required, no default, unchanged. Tombstone :605; const :367 above ServiceLevelObjectiveSchema :519. OK.
  • MetricExportConfig.interval → intervalSeconds — base JSDoc "Export interval in seconds", no describe; head describe carries the unit; default(60) unchanged. Tombstone :695; const :649 above MetricExportConfigSchema :659. OK.
  • MetricsConfig.collectionInterval → collectionIntervalSeconds — base JSDoc "Collection interval in seconds", no describe; default(15) unchanged. Tombstone :813; const :728 above MetricsConfigSchema :753. OK.
  • MetricsConfig.retention.period → durationSeconds — base JSDoc "Retention period in seconds", no describe; default(604800) unchanged. Tombstone :833; const :738 above :753. OK.
  • ADR-0049 refusal: .strict() in CODE on this file = 0 at base and 0 at head (lit z.number 38, dark 0). ⚠️ A naive grep -o '.strict()' | wc -l on the head file reads 5 — every one is inside JSDoc this PR added ("is not .strict()"); the author's "0" is the code-axis reading and is right. The current gate check:duration-unit-keys at head exits 0 and its --list shows all five new keys [name: seconds] [prose: seconds], so feat(spec): refuse a duration key whose JSDoc names a unit its describe does not #17635's widened gate will also read this file clean.

⭐ Suffix decisions — the split is principled, and the file's own #15679 convention is the reason, not the exception:

  • The [#14478 stack 4/6] system/: the 15 remaining duration keys carry their unit in the key name — ADR-0087 conversions with readers; metrics.zod.ts size needs an honest name, not the mechanical one #15679 convention is not "always durationSeconds"; it is two tests: (i) does the mechanical suffix bolt a unit onto a stem that means something else (size = bytes/rows), and (ii) does the mechanical suffix stutter against the parent (window.windowSeconds). Applied per key:
  • burnRateWindows[].window → durationSeconds: (ii) fires — parent is burnRateWindows; it is the fourth window length on the file and the other three already read durationSeconds (:399, :495, :561 re-read). windowSeconds in key position across packages/spec/src/**/*.zod.ts at head = 1, and it is stack-server.zod.ts:88, an alias-map entry mapping AWAY to windowMs (a second, non-authorable occurrence sits in a conversions/registry.ts:4482 fixture — the author's "only occurrence" holds for .zod.ts files only). Principled.
  • retention.period → durationSeconds: (i) fires — period is calendar vocabulary on this very file (ServiceLevelObjective.period.type selects rolling/calendar at :550). periodSeconds key-position on tree = 0. Principled.
  • maxAge → maxAgeSeconds: neither test fires — age IS the quantity (an age bound), and the sibling ageBuckets counts buckets of that age; durationSeconds would orphan the pair. AccessControlConfig.maxAgeSeconds on object-storage.zod.ts:257-261 is the landed same-rule precedent. Principled.
  • interval → intervalSeconds: neither test fires; intervalSeconds key-position on tree at head = 5 (4 pre-existing + this). Principled.
  • collectionInterval → collectionIntervalSeconds: keeps the qualifier because MetricExportConfig.intervalSeconds is a different cadence created by this same PR. Principled.
  • Suffix family re-measured in key position across packages/spec/src/**/*.zod.ts: Seconds 40 @ fc28c1d, 40 @ 8261ff7, 45 @ 9b62f54 and @ head; Sec 1; bare seconds-S 0 — reproduces the author's 40 → 45.

The three same-named decoys: at head :387 window: z.object({ (MetricAggregationConfig), :483 window: z.object({ (ServiceLevelIndicator), :550 period: z.object({ (ServiceLevelObjective) — all still z.object, all still hold their own durationSeconds (:399, :495, :561); base positions :346/:442/:509 match the dispatch's numbers. Pin it('leaves the three same-named decoys on this file alone') at metrics.test.ts:713 parses all three. None moved.

The ratchet (2 move, 3 do not): build-schemas.ts:850-855 builds currentKeys from schema.properties — one level deep — so only system/MetricExportConfig:interval and system/MetricsConfig:collectionInterval can move; the shard diff shows exactly those two becoming [RETIRED] beside intervalSeconds / collectionIntervalSeconds, and authorable-defaults/system.json moves the two matching default rows. The top-level rows system/MetricDefinition:summary (:726), system/MetricsConfig:retention (:746), system/ServiceLevelObjective:errorBudget (:1010) are unchanged. authorable-surface/system.json at head: 1310 rows, 0 with a dot after the colon (lit 1310, dark 0). "No movement" is distinguishable from a forgotten regeneration because I re-ran pnpm --filter @objectstack/spec gen:schema in the worktree (exit 0, 1534 schemas) and git status --porcelain over authorable-surface/ authorable-defaults/ json-schema.manifest/ came back empty. Correct.

ADR-0087 kit: one D3 entry 18.system-metrics-jsdoc-durations-unit-in-key.ts + five 18.system__… retired-key entries; registry.ts regenerated by me (gen:migration-registry, "213 semantic, 182 retired-key, 178 retired-def") → blob c2c1e621a6dc… byte-identical to the committed blob. Never hand-merged. No D2 conversion is right: stack.zod.ts declares no metrics collection (the #15679 reading, unchanged). surface carries no backticks (the render constraint); reason/acceptanceCriteria non-empty.

The pin guard replaced: it('leaves the two non-duration keys on this file alone') is gone; it('leaves the exporter batch size — a COUNT of records — alone') (:569) keeps the batch.size half inside the #15679 block whose header ("window and period lengths carry their unit (#15679)") stays true; the window half moved to the new describe('metrics JSDoc-only durations carry their unit (#15939, #14478)') block (:584) as a refusal pin asserting path errorBudget.burnRateWindows.0.window, code not unrecognized_keys, message matching the rename, plus an acceptance pin at 3600 and a describe pin. No header asserting a count is left false (all headers re-read at :500-:713). Done correctly.

Counts carrying a ref in migration prose (spot-checked three): "Seconds 40 at fc28c1d … 45 at 9b62f54" — reproduces exactly; "1195 defineStack … at fc28c1d" — git grep -o defineStack fc28c1d38 -- . ':!packages/spec' | wc -l = 1195, reproduces; ".objectui-sha = 53ded82bf7a4…" — matches the tracked pin file. The one count I could NOT reproduce: the PR body's whole-registry set difference "177 → 184" retired-key rows and "209 → 212" semantic ids. I read 175 → 182 and 210 → 213 (two methods each: regex over the RETIRED_KEYS_BY_MAJOR block, and git ls-tree of the entries directories — the two methods agree at every ref). The DELTAS reproduce exactly (0 lost / 7 gained; 0 lost / 3 gained), so the conclusion stands; the absolute numbers in the body do not.

Primary A — the #15679 acceptance sentence vs the "never amend a predecessor's entry" precedent

What the sentence is, read from the entry text. It lives in acceptanceCriteria, whose contract (migrations/types.ts:39-40) is "How the consumer proves the hand-migration correct (their own verify loop)". The upgrade guide renders that field as "Done when: …" (build-upgrade-guide.ts:104) and the CLI prints it as "verify: …" (packages/cli/src/commands/migrate/meta.ts:519). The wording is present tense and normative: "Two keys on this same file deliberately do NOT move, and a sweep that renamed either has over-applied the rule … Both keep their names." That is not a scoped past-tense record of what #15679 did; it is an instruction to the upgrader plus a verdict on any future sweep — and this PR is that sweep, so the sentence now calls this PR an over-application of the rule.

Why the #17983 precedent does not squarely govern. #17983's rationale was that #15678's sentence is "a scoped, past-tense statement about what #15678 did, it stays true". Two things differ here: (1) #15678's "One key deliberately left alone … it is outside this rename" sits in reason (18.kernel-plugin-security-durations-unit-in-key.ts:35), which renders as "Why not automatic:" — an explanation that stays true when read as history; #15679's sits in acceptanceCriteria, which renders as a completion criterion. (2) The precedent's own premise — "it stays true" — fails for a "Done when … Both keep their names" once window is tombstoned. So the rule that wins is "never leave prose the diff made false", because the #17983 rule is conditioned on the predecessor's sentence remaining true and this one does not.

How the projections actually render it today — a measured correction to both the precedent and this PR. Neither entry renders anywhere: docs/protocol-upgrade-guide.md ends at "Protocol 16 → 17" (PROTOCOL_VERSION = '17.0.0', the loop stops at PROTOCOL_MAJOR), and both ids read 0 in the guide and 0 in spec-changes.json (lit control unit-in-key in spec-changes.json: 0 as well — no major-18 id is projected yet). #17983's "it is published through the upgrade guide" was not true when written either. So the protocol-18 step is an UNRELEASED, still-assembling instruction set; adding a pointer to one of its criteria is finishing the step, not rewriting published history. When 18 does render, the two entries sit in the same section and the id sort puts this PR's entry (…-jsdoc-…) BEFORE #15679's (…-window-…) in the guide, in the CLI's todo list and in spec-changes.json (registry.ts:10550 vs :10635), which is a real mitigation.

Does the author's entry neutralise it? Only for a reader who reads both. The supersession notice lives in the SUCCESSOR's reason; the predecessor's acceptanceCriteria carries no pointer, so a reader who reaches #15679's entry alone (grep for the id, a spec_changes consumer filtering by surface, anyone reading the entry file) is told to keep window bare and that renaming it is an error. The practical harm is bounded — the tombstone refuses window at parse with the correct prescription, so nobody silently ships the wrong thing — but the instruction itself is false, and the repo's own convention for a superseded record is neither a rewrite nor silence: Prime Directive #13 ("an amended status line on the old one"), ADR-0087's own "Superseded …" notes at its amended sites, and 24a86923d5 on main ("point the superseded retirement claim at its correction", #17997) all leave the record and add the pointer AT THE SITE.

Verdict on A: option A as shipped satisfies the "record stays" half and misses the "pointer at the site" half. The fix is one appended clause in 18.system-metrics-window-durations-unit-in-key.ts acceptanceCriteria after "Both keep their names." (e.g. "— true of this entry; the burn-rate window half is superseded in this same step by system-metrics-jsdoc-durations-unit-in-key, and only the exporter batch size still keeps its name."), then gen:migration-registry. Nothing mechanical pins the old text (grep "Both keep their names" outside the entry and its registry mirror: 0). This is the FAIL item below.

Primary B — dot-joined path across an array element

  • Re-measured: RETIRED_KEYS_BY_MAJOR at head = 182 rows, 0 carry a bracket, 44 are nested (lit system/ 40, dark 0); at origin/main 1e20f816 177 rows, 0 brackets. The author's "~170 / 0" is directionally right; the exact figure is 182.
  • The author's declared evidence gap is closed — the precedents CAN be re-read in this clone. The commit that tombstoned both historical rows is present: git show bf1054a4c0:packages/spec/src/system/change-management.zod.ts reads :176 steps: z.array(z.object({ … :190 estimatedMinutes: retiredKey(…) and :299 implementation: z.object({ :308 steps: z.array(z.object({ … :322 estimatedMinutes: retiredKey(…). Both crossed a z.array(z.object({ element and were registered with plain dots. (Content-addressed reads are unaffected by shallowness; only history traversal is.)
  • A LIVE precedent the author did not cite: kernel/Manifest:contributes.kinds.globspackages/spec/src/kernel/manifest.zod.ts:487 kinds: z.array(strictObject({ with globs: retiredKey(…) inside the element; the row is plain dots. Three precedents, all re-read from schemas.
  • Nothing mechanical resolves a nested row against the schema. check (b) matches ${defKey}:${name} from top-level properties only (build-schemas.ts:850-855, :1007); (b2) does currentKeys.get(k) === false, which is undefined for a nested key, so it neither demands nor refuses (this is [finding] a nested RETIRED_KEYS_BY_MAJOR row is unverifiable — check (b2) passes a fabricated one, so a typo'd nested retirement registers silently and forever #17969, whose fabricated-nested-row ablation I read and agree with). Check (c) ages [RETIRED] shard lines, and the shard has 0 dotted rows. The only other reader that parses the string is scripts/measure-partial-retirement-annotation.mjs:249, a report-only measure that splits on : and uses the key part as prose regex text. The upgrade guide / spec-changes / CLI do not read this table at all.
  • What that means for the notation: plain dots is right — it is the table's only notation, it is what all three array-crossing precedents used, and it is what a future nested-aware (b2) (the [finding] a nested RETIRED_KEYS_BY_MAJOR row is unverifiable — check (b2) passes a fabricated one, so a typo'd nested retirement registers silently and forever #17969 option 2 shape, a properties walk that unwraps items) would most naturally parse. But the row is a convention for humans and for that future validator, not something any gate verifies today: its correctness rests on the author's spelling, which I checked against the schema by hand (ServiceLevelObjectiveSchema.errorBudget.burnRateWindows[].window, :519/:590/:605). Correct.

② Semver level

minor is right and matches the diff. .changeset/17783-metrics-jsdoc-durations-unit-in-key.md grades "@objectstack/spec": minor, carries **BREAKING**, the FROM → TO table for all five, a diff migration block, and exactly one adr-0087: registered system-metrics-jsdoc-durations-unit-in-key marker. node scripts/check-changeset-no-major.mjs --base 8261ff71… --head bcfdd356…exit 0, "introduces no major bump" (level axis N/A offline; CI's Check Changeset job is success). Landed siblings in packages/spec/CHANGELOG.md: #15677 (:124), #15678 (:1804), #15679 (:2764), #15680 (:675) all shipped feat(spec)! under ### Minor Changes, and the second-lander sibling #17999 on main grades minor in .changeset/17782-logging-duration-units.md. Clause-②: yes ⇒ at least minor per Post-Task Checklist step 3. No consumer changeset owed: git grep at head for collectionInterval (not followed by Seconds), burnRateWindows, retention.period, and MetricExportConfig outside packages/spec hits only .changeset/, generated content/docs/references/system/metrics.mdx, CHANGELOG.mds and the anchor authorable-surface.base.json; skills/, examples/ and hand-written content/docs/ = 0 (lit defineStack outside spec 1195, dark 0). One wording nit in the changeset: "every default (600, 3600 as authored, 60, 15, 604800)" — 3600 is not a default (the burn-rate window has none); the hedge keeps it from being false, but the PR body's own list (600, 60, 15, 604800) is the accurate one.

③ Boundary flags

  • Second-lander merge, "0 lost" — verified independently. Set difference over the retired-key table: rows at 8261ff71 (main side, 177) ∪ rows at 9b62f546 (branch pre-merge, 175) = 182 distinct; rows in that union missing from head = 0; head gains vs pre-merge = 7 (kernel/HotReloadConfig:debounceDelay, kernel/PluginHealthCheck:interval, …:timeout, system/HttpDestinationConfig:batch.flushInterval, …:retry.initialDelay, …:timeout, system/LoggingConfig:buffer.flushInterval); gains vs main side = exactly this PR's 5. Semantic ids: union 213, missing from head 0, +3 vs pre-merge (kernel-health-check-and-hot-reload-durations-unit-in-key, logging-durations-unit-in-key, screen-field-lookup-reference-required). Also checked against current origin/main 1e20f816: rows on main missing from head = 0. The shard that stays internally consistent while dropping the other card's rows did not happen.
  • @objectstack/qa phantom — reproduced: pnpm --filter @objectstack/qa test in the worktree → exit 0, "No projects matched the filters"; no package by that name exists (the four under packages/qa/ are dogfood, downstream-contract, http-conformance, refd-timer-testkit). Substitutes adequate: the two qa packages that consume spec metadata were run, dogfood ran in CI (Dogfood Regression Gate 1/3-3/3 and rollup all success), and refd-timer-testkit is unrelated. Every other filter in the body names a real package (core, verify, cli, rest, observability, downstream-contract, http-conformance all resolve). No other phantom found.
  • @objectstack/observability has no typecheck script (packages/observability/package.json declares test only); measuring test is the right call — the debt ledger is ratcheted by check:type-check-debt (CI job Type Check · debt ledger = success), and nothing in observability spells any of the five keys.
  • PR feat(spec): refuse a duration key whose JSDoc names a unit its describe does not #17635 conflict on metrics.test.ts — confirmed and the recommendation is right. feat(spec): refuse a duration key whose JSDoc names a unit its describe does not #17635's hunk (get_files) rewrites the comment "outside the gate population entirely and keeps its bare name" inside the very it(...) this PR split, and its added lines say the pin "must be re-read, not trusted, when the rename lands". After this PR there is no bare window to pin, so the hunk has no subject: drop it on rebase. ⚠️ feat(spec): refuse a duration key whose JSDoc names a unit its describe does not #17635's OTHER hunk, on 18.kernel__SandboxConfig__process.timeout.ts, is stale for the same reason — see the staleness list below.
  • spec: MetricsConfig.retention.downsampling[].resolution names its unit only in JSDoc — and is outside check:duration-unit-keys on the NAME axis, so #17635 will not reach it #18030 correctly filed separately. Read it: MetricsConfig.retention.downsampling[].resolution (head :842-845, JSDoc "Resolution in seconds", describe "Downsampled resolution") is outside the census on the name axis (resolution not in DURATION_SHAPED_TOKENS), so feat(spec): refuse a duration key whose JSDoc names a unit its describe does not #17635 cannot reach it; folding it here would make this card 6 rows against triage's certified 21. Separate card, not folded — right.
  • Clause ②: node scripts/pm/check-clause2-carriers.mjs --pair 18007exit 0 (2026-09-13T13:42:44Z): declaration read as Clause-②: yes from a correction comment superseding the claim's own line; both carriers agree. Independently on the mechanical floor: the diff puts five spellings on published payloads that no author could write before ⇒ yes is right. ⚠️ The script warns "ATTRIBUTION NOT VERIFIED: the governing claim comment carries no Session: line" — a PM-side record defect on the card, not on this diff.
  • Ablation — reproduced structurally, but the PR body misreports one leg and swaps the interpretation. Control 46/46 (exit 0). Leg 1 (delete window: retiredKey(SLO_BURN_RATE_WINDOW_RETIRED),, mutated blob 87687490dd… = the author's 8768749…): exit 1, 1 failed / 45 passed, metrics.test.ts:622 AssertionError: expected undefined to be defined. Leg 2 (delete period: retiredKey(RETENTION_PERIOD_RETIRED),, blob 8a84628ef1… = the author's): exit 1, 1 failed / 45 passed, but the assertion is metrics.test.ts:659 AssertionError: expected true to be false — NOT "expected undefined to be defined". Restore under an EXIT INT TERM trap via git restore --source=HEAD --staged --worktree, blob back to a747965b7d…, git status --porcelain empty, git diff HEAD empty, both legs. What each leg PROVES: leg 2 is the ADR-0049 silent strip exactly as claimed (result.success came back true — the non-strict retention object accepted period, stripped it, and applied the 604800 default). Leg 1 is NOT a silent strip: burnRateWindows[].durationSeconds is required, so the element is refused anyway ("Required" at …0.durationSeconds) and what the tombstone buys there is the prescription at the right path, not the refusal itself — which is why the failing assertion is the path lookup, not success. 1 failure per leg is the right count: I checked that no other test in the file feeds the old spelling to the ablated key. The array-element question is answered by the control run's passing refusal pin at path errorBudget.burnRateWindows.0.window (code not unrecognized_keys), and by leg 1 proving that pin can fail.
  • Cross-file staleness at this head — what the author missed: packages/spec/src/migrations/entries/retired-keys/18.kernel__SandboxConfig__process.timeout.ts:7-9 (mirrored at registry.ts:13136) still says RuntimeConfig.resourceLimits.timeout "is outside the gate's population and is untouched here" — false on main since feat(spec)!: RuntimeConfig resourceLimits.timeout carries its unit in the key name (#17781) #17983 tombstoned it as timeoutMs; feat(spec): refuse a duration key whose JSDoc names a unit its describe does not #17635's pending replacement text ("a rename waiting on that gate change") is stale too. Outside this card's file territory (spec: 1 duration key(s) in kernel/plugin-security-advanced.zod.ts name their unit only in JSDoc — #15939 Ruling A remediation (1 of the 21-row delta) #17781's), so an escalation to the PM, not a change to this PR. Nothing else found: metrics.zod.ts non-tombstone prose, packages/spec/scripts/, skills/, examples/, hand-written content/docs/ carry no old spelling; packages/spec/CHANGELOG.md:2866 ("burn-rate window … name no unit") is released, RELEASE-OWNED text and correctly untouched.
  • Noted, not a defect of this PR: MetricAggregationConfig.window.slideInterval (:412, JSDoc "Slide interval for sliding windows", no describe) is the spec: tenant.zod.ts idleTimeout / sessionTimeout publish a describe with no unit, while the JSDoc one line above says seconds #14519 unit-nowhere shape — in the census, listed not judged, outside finding: check:duration-unit-keys reads .describe() but not JSDoc — a duration key documenting its unit only in JSDoc never enters the population, and one card already recorded a wrong reason because of it #15939 and outside spec: MetricsConfig.retention.downsampling[].resolution names its unit only in JSDoc — and is outside check:duration-unit-keys on the NAME axis, so #17635 will not reach it #18030's name-axis case. Recorded so the next reader does not mistake it for a sixth JSDoc-channel row.
  • CI: the six required contexts are success at the job level (Lint & Repo Gates, Type Check · source gates / consumer gates / debt ledger / workspace, Test Core 1/6-6/6, Dogfood Regression Gate 1/3-3/3, Build Core, Temporal Conformance); Console Pin Gate was skipped on this PR, so the objectui reading is not CI-backed.

Readings I re-measured

All in /tmp/review-18007 at bcfdd356… unless a ref is named; UTC 2026-09-13.

  • 13:36:29Z git fetch origin claude/issue-17783-metrics-duration-unit-keys && git worktree add /tmp/review-18007 bcfdd356…; git merge-base bcfdd356… origin/main8261ff71… (= PR base sha; shallow clone, git rev-list --count HEAD = 95).
  • 13:38:26Z shard diff git diff 8261ff71… bcfdd356… -- packages/spec/authorable-surface/system.json packages/spec/authorable-defaults/system.json; shard rows 1310, dotted 0.
  • 13:39:46Z RETIRED_KEYS_BY_MAJOR rows by regex over the table block at head / 8261ff7 / 9b62f54 / 1e20f81 = 182 / 177 / 175 / 177; brackets 0/0/0/0; nested 44; comm set differences as reported.
  • 13:39:54Z semantic ids by id: in registry.ts and by git ls-tree of entries/semantic/ = 213 / 212 / 210, both methods agree per ref.
  • 13:40:01Z key-position grep on metrics.zod.ts at head and base; .strict() 5 head (all JSDoc) / 0 code / 0 base; retiredKey( 8 head / 3 base.
  • 13:40:08Z git grep -ohE over packages/spec/src/**/*.zod.ts at each ref for a line-leading identifier ending in Seconds followed by an optional question mark and a colon (key position), piped to wc -l, at fc28c1d / 9b62f54 / 8261ff7 / head = 40 / 45 / 40 / 45; defineStack at fc28c1d outside spec = 1195.
  • 13:42:25Z docs/protocol-upgrade-guide.md last section "Protocol 16 → 17"; both ids 0 there and in spec-changes.json; PROTOCOL_VERSION = '17.0.0'; spec 17.4.0.
  • 13:42:41Z pnpm --filter @objectstack/qa test → exit 0, "No projects matched the filters".
  • 13:42:44Z node scripts/pm/check-clause2-carriers.mjs --pair 18007EXIT=0.
  • 13:45:50Z git show bf1054a4c0:packages/spec/src/system/change-management.zod.ts (:176/:190, :299/:308/:322); 13:45:54Z manifest.zod.ts:487 kinds: z.array(strictObject({ + globs: retiredKey(.
  • 13:46:06Z node scripts/check-changeset-no-major.mjs --base 8261ff71… --head bcfdd356… → EXIT=0.
  • 13:46:22Z pnpm --filter @objectstack/spec gen:migration-registryregistry.ts blob c2c1e621a6dc… = committed; gen:schema → exit 0, porcelain empty over the three shard dirs.
  • 13:48:48Z–13:48:55Z ablation script (control + two legs, trap-restored, hashes as reported).
  • 13:50:33Z pnpm --filter @objectstack/spec check:duration-unit-keys → exit 0 (211 keys, zero offenders); --list | grep metrics.zod.ts rows as reported.

Taken on trust (not re-measured): the pinned objectui grep (no local objectui checkout; Console Pin Gate skipped on this PR) — the author's lit/dark controls are well-formed, and no in-repo consumer exists, so I accept "no pin bump owed" on that basis; the dispatch-gates 109/109 family run; the consumer typecheck/test exits for core/verify/cli/rest/observability/downstream-contract/http-conformance (CI's Type Check · consumer gates and Test Core jobs are success, which covers the same surface); the os-verify-lock.sh build verdict; the 6409 objectui tracked-file count.

Implemented-by: branch claude/issue-17783-metrics-duration-unit-keys
Reviewed-by: session_015c5G6TmpMKgnusmTpD7Ntt

VERDICT: FAIL — two things must change, both small. (1) packages/spec/src/migrations/entries/semantic/18.system-metrics-window-durations-unit-in-key.ts acceptanceCriteria: append a pointer clause after "Both keep their names." naming system-metrics-jsdoc-durations-unit-in-key as the entry that renames the burn-rate window in this same step (leave every existing word intact — Prime Directive #13 shape, not a rewrite), then pnpm --filter @objectstack/spec gen:migration-registry and commit the regenerated registry.ts. (2) Correct the PR body's ablation table: leg 2 reads expected true to be false at metrics.test.ts:659, and only leg 2 demonstrates the silent strip; leg 1 demonstrates the lost prescription on a key whose sibling durationSeconds is required. Optionally fix the absolute set-difference counts (175 → 182 rows, 210 → 213 ids; deltas unchanged). Everything else — the five renames, the suffix split, the decoys, the ratchet, the kit, the pin split, the second-lander merge, semver — passes as measured. Escalated to the PM, not this PR: the stale 18.kernel__SandboxConfig__process.timeout.ts:7-9 sentence (and #17635's stale hunk on it), and the missing Session: line on card #17783's claim comment.


Generated by Claude Code

…s the burn-rate window

Contract review on PR #18007 ruled that the "Both keep their names" sentence
in `system-metrics-window-durations-unit-in-key`'s acceptanceCriteria must
gain a pointer. That field's contract is "how the consumer proves the
hand-migration correct" — it renders as "Done when:" in the upgrade guide and
"verify:" in `migrate meta` — so a normative sentence saying the burn-rate
window keeps its name does not merely go stale once the key is tombstoned, it
instructs a reviewer that a correct sweep was an error.

Not a rewrite: every existing word is left in place and the clause is
appended, per Prime Directive #13. Nor is it amending published history — the
protocol-18 step is unreleased and still assembling (PROTOCOL_VERSION is
17.0.0, the guide ends at 16 to 17, and both entry ids render nowhere today),
so this finishes the step rather than editing a shipped record.

Claude-Session: https://claude.ai/code/session_015c5G6TmpMKgnusmTpD7Ntt
Co-authored-by: Claude <noreply@anthropic.com>

Copy link
Copy Markdown
Collaborator

Contract review — delta

Head-sha: 84e62ed7129e7cd6e1b6a307530cdf4f9d3602a3
Baseline: bcfdd35609c5d880044a17d64ba44d7c1798d866 (FAIL, comment 5653712837)

Reviewed from an isolated worktree at the head sha (git worktree add /tmp/review-18007b 84e62ed7…, 2026-09-13T14:11:36Z); the clone is shallow (git rev-list --count HEAD = 96), so every history-dependent reading says so. Every count is grep -o … | wc -l (occurrences) unless it says lines or files, and every zero carries a same-subject lit control plus a dark control (qzvbmtkr / vmkrtplx, invented at read time). Scope is the baseline's two FAIL items, its optional count fix, and whatever the delta could disturb. ⛔ The five renames, the suffix split, the decoys, the ratchet, the pin split, the second-lander merge and semver passed on bcfdd356… and the delta touches none of their inputs — not re-derived here.

Delta reviewed

  • Exactly the two files, and nothing else moved. git diff --stat bcfdd356… 84e62ed7… = 2 files, +18 / −2: packages/spec/src/migrations/entries/semantic/18.system-metrics-window-durations-unit-in-key.ts (+9 / −1, one hunk) and packages/spec/src/migrations/registry.ts (+9 / −1, one hunk at :10671). git log bcfdd356…..84e62ed7… = one commit, 84e62ed712 docs(spec): point #15679's metrics entry at the successor that renames the burn-rate window; git merge-base --is-ancestor bcfdd356… 84e62ed7… = yes (no history rewrite); merge-base with 8261ff7171 unchanged; branch tip = PR head. metrics.zod.ts, metrics.test.ts, the successor entry, the five retired-key entries, the changeset and both shards did not move (git diff --stat over those paths between the two heads: empty).

FAIL items

Item 1 — pointer clause on #15679's entry: DISCHARGED.

  • Every pre-existing word intact. I evaluated the acceptanceCriteria expression at both refs, entry file and registry.ts (comment lines dropped, the '…' + '…' concatenation reduced to its string value — the multi-line trap cannot bite an evaluated value): base value 1096 chars, head value 1443 chars, head.startsWith(base + ' ') = true. The appended text is exactly one clause — "One of those two moves after all, in this same protocol step: the error-budget burn-rate window is renamed to durationSeconds by system-metrics-jsdoc-durations-unit-in-key, the remediation of the JSDoc-channel gap finding: check:duration-unit-keys reads .describe() but not JSDoc — a duration key documenting its unit only in JSDoc never enters the population, and one card already recorded a wrong reason because of it #15939 named just above. Read that entry with this one; the exporter batch size is still a COUNT of records and still does not move." — placed immediately after "Both keep their names." with a single space, 0 backticks, no newline. Prime Directive [WIP] Add Chinese version of the documentation #13 shape: a pointer at the site, the record left standing.
  • Clause accurate on this tree. The successor system-metrics-jsdoc-durations-unit-in-key exists (entries/semantic/18.system-metrics-jsdoc-durations-unit-in-key.ts) and sits in the same step (registry.ts:10550, inside const step18 from :5102; there is no step19); its surface names ServiceLevelObjective.errorBudget.burnRateWindows[].window and its replacement errorBudget.burnRateWindows[].durationSeconds; the schema spells durationSeconds at metrics.zod.ts:602 with the tombstone window: retiredKey(SLO_BURN_RATE_WINDOW_RETIRED) at :605; the retired row system/ServiceLevelObjective:errorBudget.burnRateWindows.window sits at registry.ts:13784 inside the 18: [ block (:11689); "finding: check:duration-unit-keys reads .describe() but not JSDoc — a duration key documenting its unit only in JSDoc never enters the population, and one card already recorded a wrong reason because of it #15939 named just above" is literally the preceding sentence of the same field; the successor cites the predecessor id 1× and finding: check:duration-unit-keys reads .describe() but not JSDoc — a duration key documenting its unit only in JSDoc never enters the population, and one card already recorded a wrong reason because of it #15939 3×; the exporter batch size batch.size at metrics.zod.ts:702 is still z.number().int().positive().optional().default(1000) — a bare count, unmoved.
  • registry.ts is the regenerated mirror. pnpm --filter @objectstack/spec gen:migration-registry in the worktree printed "213 semantic, 182 retired-key, 178 retired-def" and wrote blob 23763684981d…byte-identical to the committed blob, porcelain empty; check:migration-registry (self-test + check) exit 0. The evaluated field value is entry == registry at both refs, and the changed region is the only hunk in either file.
  • Premise re-confirmed on this head. ⚠️ The guide lives at docs/protocol-upgrade-guide.md (repo root); packages/spec/docs/protocol-upgrade-guide.md does not exist on this tree, and a grep against that path reads a false zero — my first pass hit it and was discarded. At the real path the last section is "Protocol 16 → 17"; both ids read 0 in the guide and 0 in packages/spec/spec-changes.json (unit-in-key 0 in both — no major-18 id is projected anywhere yet); lit controls Protocol 16 = 2 in the guide and data-driver-find-stream-retired = 16 in spec-changes.json; dark 0 in both. PROTOCOL_VERSION = '17.0.0' (kernel/protocol-version.ts:18), spec-changes.json "protocolVersion": "17.0.0" / "to": 17, spec 17.4.0; build-upgrade-guide.ts:60 stops at PROTOCOL_MAJOR. check:spec-changes and check:upgrade-guide both exit 0 with no diff. The protocol-18 step is still unreleased and unprojected, so the clause finishes an assembling step rather than amending published history — the premise the fix rests on holds.

Item 2 — ablation table in the PR body: DISCHARGED.

  • Schema asymmetry (the load-bearing part), read at head: burnRateWindows[].durationSeconds is z.number().int().positive() at metrics.zod.ts:602required, no .optional(), no default; retention.durationSeconds is z.number().int().positive().optional().default(604800) at :829optional with a default. Neither file changed in the delta.
  • Test lines: metrics.test.ts:622 is expect(issue).toBeDefined(); inside REFUSES the burn-rate window with a rename naming durationSeconds (:612:627) — the path lookup for errorBudget.burnRateWindows.0.window, after the success assertion at :618 that still holds; :659 is expect(result.success).toBe(false); inside REFUSES MetricsConfig.retention.period with a rename naming durationSeconds (:655:668).
  • Re-run on this head (14:16:06Z–14:16:13Z, EXIT INT TERM trap, git restore --source=HEAD --staged --worktree): control 46 / 46, exit 0. Leg 1 (delete :605): mutated blob 87687490dd… = the author's 8768749…, 1 failed / 45 passed, metrics.test.ts:622:19 AssertionError: expected undefined to be defined. Leg 2 (delete :833): mutated blob 8a84628ef1… = the author's 8a84628…, 1 failed / 45 passed, metrics.test.ts:659:28 AssertionError: expected true to be false. Both legs restored to a747965b7d…, porcelain empty, git diff HEAD empty. So leg 1 proves the lost prescription (the element is refused anyway for the missing required sibling; only the located rename message is gone) and leg 2 is the live ADR-0049 silent strip (success came back true, period stripped, the 604800 default applied). The PR body now says exactly this — its "The two legs fail differently" paragraph carries the right assertions, the right line numbers and the required-vs-optional reason.

Optional count fix: done, and reproduced three ways. Retired-key rows 175 → 182 and semantic ids 210 → 213 from 9b62f546 (pre-merge) to head: (a) the generator's own printed line, above; (b) git ls-tree file counts at head / 8261ff7171 / 9b62f546 = 182 / 177 / 175 retired-key and 213 / 212 / 210 semantic; (c) regex over registry.tsRETIRED_KEYS_BY_MAJOR rows 182 / 177 / 175 and id: ' lines 213 / 212 / 210. Set difference: rows present on main or pre-merge and missing from head = 0, gained vs pre-merge = 7 (kernel/HotReloadConfig:debounceDelay, kernel/PluginHealthCheck:interval, …:timeout, system/HttpDestinationConfig:batch.flushInterval, …:retry.initialDelay, …:timeout, system/LoggingConfig:buffer.flushInterval); ids missing = 0, gained = 3 (kernel-health-check-and-hot-reload-durations-unit-in-key, logging-durations-unit-in-key, screen-field-lookup-reference-required). The body's "177 files on main + 5 = 182; 212 + 1 = 213" reproduces. One note, not a defect: a regex over registry.ts reproduces the generator when its character class admits uppercase — a [a-z0-9-] class reads one short at every ref because of object-titleFormat-to-nameField — so "a regex over registry.ts, which miscounts" describes the author's earlier regex, not the method.

Anything the delta disturbs

Nothing found. migrations.test.ts 130 / 130 on head (the only generic constraint on acceptanceCriteria is non-empty at :44; no test pins the predecessor's text — 0 test files reference its id; "Both keep their names" and the new clause each occur in exactly 2 files, the entry and its mirror). eslint exit 0 on both changed files. node scripts/pm/check-widening-tells.mjs --diff over the delta diff: exit 0 under --declaration yes (the PR's carrier) and under no (2 files judged, no tell). CI on 84e62ed7… at 14:19Z: Type Check · source gates (the lane running check:spec-changes / check:upgrade-guide), Build Core, Temporal Conformance (live PG + MySQL), Dogfood Regression Gate 1/3–3/3 + rollup, Type Check · consumer gates, Type Check · debt ledger, Test Core 2/6 3/6 4/6, Check Changeset, Governed Surface Queue Guard, Spec property liveness all success; Lint & Repo Gates, Type Check · workspace and Test Core 1/6 5/6 6/6 still in_progress, 0 failures — the queue still needs those to conclude, and nothing the delta touches feeds them beyond the prose judged above. The baseline's two escalations (the stale 18.kernel__SandboxConfig__process.timeout.ts:7-9 sentence plus #17635's stale hunk on it; the missing Session: line on #17783's claim comment) are outside this PR and untouched by the delta — still with the PM.

Readings I re-measured

All in /tmp/review-18007b at 84e62ed7… unless a ref is named; UTC 2026-09-13.

  • 14:11:36Z git fetch origin claude/issue-17783-metrics-duration-unit-keys and both shas; git worktree add /tmp/review-18007b 84e62ed7…; git diff --stat / --name-status bcfdd356… 84e62ed7…; git log bcfdd356…..84e62ed7… = 1 commit.
  • 14:12:53Z git merge-base --is-ancestor = yes; git rev-list --count HEAD = 96 (shallow); entry file read at both refs.
  • 14:13:03Z tree greps: "Both keep their names" and the new clause = entry + registry only; dark vmkrtplx 0.
  • 14:13:18Z git ls-tree censuses and comm set differences at head / 8261ff7171 / 9b62f546.
  • 14:14:15Z guide and packages/spec/spec-changes.json id counts with lit and dark controls; PROTOCOL_VERSION; spec 17.4.0.
  • 14:14:21Z metrics.test.ts:622 / :659; metrics.zod.ts:589:607 and :818:840; delta diff over both files = empty.
  • 14:14:37Z evaluated acceptanceCriteria at both refs, entry and registry: 1096 → 1443 chars, prefix + single space.
  • 14:14:43Z id-census reconciliation (object-titleFormat-to-nameField); migrations/types.ts:39-40; validators of the field.
  • 14:15:11Z–14:15:18Z gen:migration-registry (blob = committed), check:migration-registry, check:spec-changes, check:upgrade-guide — all exit 0, porcelain empty.
  • 14:16:06Z–14:16:13Z ablation control + two legs, hashes as reported.
  • 14:16:14Z successor row registry.ts:13784; batch.size at :702; eslint on both files exit 0.
  • 14:17:02Z migrations.test.ts 130 / 130; 14:17:36Z widening-tells on the delta diff, exit 0 under both declarations.
  • 14:19Z CI check runs on the head, as listed.

Taken on trust (not re-measured): the full check:generated 15 / 15 on head (needs a built dist; I ran the three gates the delta can reach, and CI's Type Check · source gates lane is success on this head); the dispatch-gates 109 / 109 family (run on bcfdd356…, as the body states); and everything the baseline review passed that the delta does not touch.

Implemented-by: branch claude/issue-17783-metrics-duration-unit-keys
Reviewed-by: session_015c5G6TmpMKgnusmTpD7Ntt

VERDICT: PASS


Generated by Claude Code

Copy link
Copy Markdown
Collaborator

Contract review — delta

This record supersedes comment 5653835652 for machine-readability only — its Implemented-by: line carried a leading word before the branch name, which check-clause2-carriers C4 reads as no identity — and the verdict and every finding below are unchanged, restated verbatim.

Head-sha: 84e62ed7129e7cd6e1b6a307530cdf4f9d3602a3
Baseline: bcfdd35609c5d880044a17d64ba44d7c1798d866 (FAIL, comment 5653712837)

Reviewed from an isolated worktree at the head sha (git worktree add /tmp/review-18007b 84e62ed7…, 2026-09-13T14:11:36Z); the clone is shallow (git rev-list --count HEAD = 96), so every history-dependent reading says so. Every count is grep -o … | wc -l (occurrences) unless it says lines or files, and every zero carries a same-subject lit control plus a dark control (qzvbmtkr / vmkrtplx, invented at read time). Scope is the baseline's two FAIL items, its optional count fix, and whatever the delta could disturb. ⛔ The five renames, the suffix split, the decoys, the ratchet, the pin split, the second-lander merge and semver passed on bcfdd356… and the delta touches none of their inputs — not re-derived here.

Delta reviewed

  • Exactly the two files, and nothing else moved. git diff --stat bcfdd356… 84e62ed7… = 2 files, +18 / −2: packages/spec/src/migrations/entries/semantic/18.system-metrics-window-durations-unit-in-key.ts (+9 / −1, one hunk) and packages/spec/src/migrations/registry.ts (+9 / −1, one hunk at :10671). git log bcfdd356…..84e62ed7… = one commit, 84e62ed712 docs(spec): point #15679's metrics entry at the successor that renames the burn-rate window; git merge-base --is-ancestor bcfdd356… 84e62ed7… = yes (no history rewrite); merge-base with 8261ff7171 unchanged; branch tip = PR head. metrics.zod.ts, metrics.test.ts, the successor entry, the five retired-key entries, the changeset and both shards did not move (git diff --stat over those paths between the two heads: empty).

FAIL items

Item 1 — pointer clause on #15679's entry: DISCHARGED.

  • Every pre-existing word intact. I evaluated the acceptanceCriteria expression at both refs, entry file and registry.ts (comment lines dropped, the '…' + '…' concatenation reduced to its string value — the multi-line trap cannot bite an evaluated value): base value 1096 chars, head value 1443 chars, head.startsWith(base + ' ') = true. The appended text is exactly one clause — "One of those two moves after all, in this same protocol step: the error-budget burn-rate window is renamed to durationSeconds by system-metrics-jsdoc-durations-unit-in-key, the remediation of the JSDoc-channel gap finding: check:duration-unit-keys reads .describe() but not JSDoc — a duration key documenting its unit only in JSDoc never enters the population, and one card already recorded a wrong reason because of it #15939 named just above. Read that entry with this one; the exporter batch size is still a COUNT of records and still does not move." — placed immediately after "Both keep their names." with a single space, 0 backticks, no newline. Prime Directive [WIP] Add Chinese version of the documentation #13 shape: a pointer at the site, the record left standing.
  • Clause accurate on this tree. The successor system-metrics-jsdoc-durations-unit-in-key exists (entries/semantic/18.system-metrics-jsdoc-durations-unit-in-key.ts) and sits in the same step (registry.ts:10550, inside const step18 from :5102; there is no step19); its surface names ServiceLevelObjective.errorBudget.burnRateWindows[].window and its replacement errorBudget.burnRateWindows[].durationSeconds; the schema spells durationSeconds at metrics.zod.ts:602 with the tombstone window: retiredKey(SLO_BURN_RATE_WINDOW_RETIRED) at :605; the retired row system/ServiceLevelObjective:errorBudget.burnRateWindows.window sits at registry.ts:13784 inside the 18: [ block (:11689); "finding: check:duration-unit-keys reads .describe() but not JSDoc — a duration key documenting its unit only in JSDoc never enters the population, and one card already recorded a wrong reason because of it #15939 named just above" is literally the preceding sentence of the same field; the successor cites the predecessor id 1× and finding: check:duration-unit-keys reads .describe() but not JSDoc — a duration key documenting its unit only in JSDoc never enters the population, and one card already recorded a wrong reason because of it #15939 3×; the exporter batch size batch.size at metrics.zod.ts:702 is still z.number().int().positive().optional().default(1000) — a bare count, unmoved.
  • registry.ts is the regenerated mirror. pnpm --filter @objectstack/spec gen:migration-registry in the worktree printed "213 semantic, 182 retired-key, 178 retired-def" and wrote blob 23763684981d…byte-identical to the committed blob, porcelain empty; check:migration-registry (self-test + check) exit 0. The evaluated field value is entry == registry at both refs, and the changed region is the only hunk in either file.
  • Premise re-confirmed on this head. ⚠️ The guide lives at docs/protocol-upgrade-guide.md (repo root); packages/spec/docs/protocol-upgrade-guide.md does not exist on this tree, and a grep against that path reads a false zero — my first pass hit it and was discarded. At the real path the last section is "Protocol 16 → 17"; both ids read 0 in the guide and 0 in packages/spec/spec-changes.json (unit-in-key 0 in both — no major-18 id is projected anywhere yet); lit controls Protocol 16 = 2 in the guide and data-driver-find-stream-retired = 16 in spec-changes.json; dark 0 in both. PROTOCOL_VERSION = '17.0.0' (kernel/protocol-version.ts:18), spec-changes.json "protocolVersion": "17.0.0" / "to": 17, spec 17.4.0; build-upgrade-guide.ts:60 stops at PROTOCOL_MAJOR. check:spec-changes and check:upgrade-guide both exit 0 with no diff. The protocol-18 step is still unreleased and unprojected, so the clause finishes an assembling step rather than amending published history — the premise the fix rests on holds.

Item 2 — ablation table in the PR body: DISCHARGED.

  • Schema asymmetry (the load-bearing part), read at head: burnRateWindows[].durationSeconds is z.number().int().positive() at metrics.zod.ts:602required, no .optional(), no default; retention.durationSeconds is z.number().int().positive().optional().default(604800) at :829optional with a default. Neither file changed in the delta.
  • Test lines: metrics.test.ts:622 is expect(issue).toBeDefined(); inside REFUSES the burn-rate window with a rename naming durationSeconds (:612:627) — the path lookup for errorBudget.burnRateWindows.0.window, after the success assertion at :618 that still holds; :659 is expect(result.success).toBe(false); inside REFUSES MetricsConfig.retention.period with a rename naming durationSeconds (:655:668).
  • Re-run on this head (14:16:06Z–14:16:13Z, EXIT INT TERM trap, git restore --source=HEAD --staged --worktree): control 46 / 46, exit 0. Leg 1 (delete :605): mutated blob 87687490dd… = the author's 8768749…, 1 failed / 45 passed, metrics.test.ts:622:19 AssertionError: expected undefined to be defined. Leg 2 (delete :833): mutated blob 8a84628ef1… = the author's 8a84628…, 1 failed / 45 passed, metrics.test.ts:659:28 AssertionError: expected true to be false. Both legs restored to a747965b7d…, porcelain empty, git diff HEAD empty. So leg 1 proves the lost prescription (the element is refused anyway for the missing required sibling; only the located rename message is gone) and leg 2 is the live ADR-0049 silent strip (success came back true, period stripped, the 604800 default applied). The PR body now says exactly this — its "The two legs fail differently" paragraph carries the right assertions, the right line numbers and the required-vs-optional reason.

Optional count fix: done, and reproduced three ways. Retired-key rows 175 → 182 and semantic ids 210 → 213 from 9b62f546 (pre-merge) to head: (a) the generator's own printed line, above; (b) git ls-tree file counts at head / 8261ff7171 / 9b62f546 = 182 / 177 / 175 retired-key and 213 / 212 / 210 semantic; (c) regex over registry.tsRETIRED_KEYS_BY_MAJOR rows 182 / 177 / 175 and id: ' lines 213 / 212 / 210. Set difference: rows present on main or pre-merge and missing from head = 0, gained vs pre-merge = 7 (kernel/HotReloadConfig:debounceDelay, kernel/PluginHealthCheck:interval, …:timeout, system/HttpDestinationConfig:batch.flushInterval, …:retry.initialDelay, …:timeout, system/LoggingConfig:buffer.flushInterval); ids missing = 0, gained = 3 (kernel-health-check-and-hot-reload-durations-unit-in-key, logging-durations-unit-in-key, screen-field-lookup-reference-required). The body's "177 files on main + 5 = 182; 212 + 1 = 213" reproduces. One note, not a defect: a regex over registry.ts reproduces the generator when its character class admits uppercase — a [a-z0-9-] class reads one short at every ref because of object-titleFormat-to-nameField — so "a regex over registry.ts, which miscounts" describes the author's earlier regex, not the method.

Anything the delta disturbs

Nothing found. migrations.test.ts 130 / 130 on head (the only generic constraint on acceptanceCriteria is non-empty at :44; no test pins the predecessor's text — 0 test files reference its id; "Both keep their names" and the new clause each occur in exactly 2 files, the entry and its mirror). eslint exit 0 on both changed files. node scripts/pm/check-widening-tells.mjs --diff over the delta diff: exit 0 under --declaration yes (the PR's carrier) and under no (2 files judged, no tell). CI on 84e62ed7… at 14:19Z: Type Check · source gates (the lane running check:spec-changes / check:upgrade-guide), Build Core, Temporal Conformance (live PG + MySQL), Dogfood Regression Gate 1/3–3/3 + rollup, Type Check · consumer gates, Type Check · debt ledger, Test Core 2/6 3/6 4/6, Check Changeset, Governed Surface Queue Guard, Spec property liveness all success; Lint & Repo Gates, Type Check · workspace and Test Core 1/6 5/6 6/6 still in_progress, 0 failures — the queue still needs those to conclude, and nothing the delta touches feeds them beyond the prose judged above. The baseline's two escalations (the stale 18.kernel__SandboxConfig__process.timeout.ts:7-9 sentence plus #17635's stale hunk on it; the missing Session: line on #17783's claim comment) are outside this PR and untouched by the delta — still with the PM.

Readings I re-measured

All in /tmp/review-18007b at 84e62ed7… unless a ref is named; UTC 2026-09-13.

  • 14:11:36Z git fetch origin claude/issue-17783-metrics-duration-unit-keys and both shas; git worktree add /tmp/review-18007b 84e62ed7…; git diff --stat / --name-status bcfdd356… 84e62ed7…; git log bcfdd356…..84e62ed7… = 1 commit.
  • 14:12:53Z git merge-base --is-ancestor = yes; git rev-list --count HEAD = 96 (shallow); entry file read at both refs.
  • 14:13:03Z tree greps: "Both keep their names" and the new clause = entry + registry only; dark vmkrtplx 0.
  • 14:13:18Z git ls-tree censuses and comm set differences at head / 8261ff7171 / 9b62f546.
  • 14:14:15Z guide and packages/spec/spec-changes.json id counts with lit and dark controls; PROTOCOL_VERSION; spec 17.4.0.
  • 14:14:21Z metrics.test.ts:622 / :659; metrics.zod.ts:589:607 and :818:840; delta diff over both files = empty.
  • 14:14:37Z evaluated acceptanceCriteria at both refs, entry and registry: 1096 → 1443 chars, prefix + single space.
  • 14:14:43Z id-census reconciliation (object-titleFormat-to-nameField); migrations/types.ts:39-40; validators of the field.
  • 14:15:11Z–14:15:18Z gen:migration-registry (blob = committed), check:migration-registry, check:spec-changes, check:upgrade-guide — all exit 0, porcelain empty.
  • 14:16:06Z–14:16:13Z ablation control + two legs, hashes as reported.
  • 14:16:14Z successor row registry.ts:13784; batch.size at :702; eslint on both files exit 0.
  • 14:17:02Z migrations.test.ts 130 / 130; 14:17:36Z widening-tells on the delta diff, exit 0 under both declarations.
  • 14:19Z CI check runs on the head, as listed.

Taken on trust (not re-measured): the full check:generated 15 / 15 on head (needs a built dist; I ran the three gates the delta can reach, and CI's Type Check · source gates lane is success on this head); the dispatch-gates 109 / 109 family (run on bcfdd356…, as the body states); and everything the baseline review passed that the delta does not touch.

Implemented-by: claude/issue-17783-metrics-duration-unit-keys
Reviewed-by: session_015c5G6TmpMKgnusmTpD7Ntt

VERDICT: PASS


Generated by Claude Code

Copy link
Copy Markdown
Collaborator

Contract review — delta

This record supersedes comment 5653920672 for machine-readability only — that record's Head-sha: label sat inside the same code span as the sha, so contractReviewHeadMatch (which admits a bare-hex span only) could not read which head it reviewed; the sha is now its own span — and the verdict and every finding below are unchanged, restated verbatim.

Head-sha: 84e62ed7129e7cd6e1b6a307530cdf4f9d3602a3
Baseline: bcfdd35609c5d880044a17d64ba44d7c1798d866 (FAIL, comment 5653712837)

Reviewed from an isolated worktree at the head sha (git worktree add /tmp/review-18007b 84e62ed7…, 2026-09-13T14:11:36Z); the clone is shallow (git rev-list --count HEAD = 96), so every history-dependent reading says so. Every count is grep -o … | wc -l (occurrences) unless it says lines or files, and every zero carries a same-subject lit control plus a dark control (qzvbmtkr / vmkrtplx, invented at read time). Scope is the baseline's two FAIL items, its optional count fix, and whatever the delta could disturb. ⛔ The five renames, the suffix split, the decoys, the ratchet, the pin split, the second-lander merge and semver passed on bcfdd356… and the delta touches none of their inputs — not re-derived here.

Delta reviewed

  • Exactly the two files, and nothing else moved. git diff --stat bcfdd356… 84e62ed7… = 2 files, +18 / −2: packages/spec/src/migrations/entries/semantic/18.system-metrics-window-durations-unit-in-key.ts (+9 / −1, one hunk) and packages/spec/src/migrations/registry.ts (+9 / −1, one hunk at :10671). git log bcfdd356…..84e62ed7… = one commit, 84e62ed712 docs(spec): point #15679's metrics entry at the successor that renames the burn-rate window; git merge-base --is-ancestor bcfdd356… 84e62ed7… = yes (no history rewrite); merge-base with 8261ff7171 unchanged; branch tip = PR head. metrics.zod.ts, metrics.test.ts, the successor entry, the five retired-key entries, the changeset and both shards did not move (git diff --stat over those paths between the two heads: empty).

FAIL items

Item 1 — pointer clause on #15679's entry: DISCHARGED.

  • Every pre-existing word intact. I evaluated the acceptanceCriteria expression at both refs, entry file and registry.ts (comment lines dropped, the '…' + '…' concatenation reduced to its string value — the multi-line trap cannot bite an evaluated value): base value 1096 chars, head value 1443 chars, head.startsWith(base + ' ') = true. The appended text is exactly one clause — "One of those two moves after all, in this same protocol step: the error-budget burn-rate window is renamed to durationSeconds by system-metrics-jsdoc-durations-unit-in-key, the remediation of the JSDoc-channel gap finding: check:duration-unit-keys reads .describe() but not JSDoc — a duration key documenting its unit only in JSDoc never enters the population, and one card already recorded a wrong reason because of it #15939 named just above. Read that entry with this one; the exporter batch size is still a COUNT of records and still does not move." — placed immediately after "Both keep their names." with a single space, 0 backticks, no newline. Prime Directive [WIP] Add Chinese version of the documentation #13 shape: a pointer at the site, the record left standing.
  • Clause accurate on this tree. The successor system-metrics-jsdoc-durations-unit-in-key exists (entries/semantic/18.system-metrics-jsdoc-durations-unit-in-key.ts) and sits in the same step (registry.ts:10550, inside const step18 from :5102; there is no step19); its surface names ServiceLevelObjective.errorBudget.burnRateWindows[].window and its replacement errorBudget.burnRateWindows[].durationSeconds; the schema spells durationSeconds at metrics.zod.ts:602 with the tombstone window: retiredKey(SLO_BURN_RATE_WINDOW_RETIRED) at :605; the retired row system/ServiceLevelObjective:errorBudget.burnRateWindows.window sits at registry.ts:13784 inside the 18: [ block (:11689); "finding: check:duration-unit-keys reads .describe() but not JSDoc — a duration key documenting its unit only in JSDoc never enters the population, and one card already recorded a wrong reason because of it #15939 named just above" is literally the preceding sentence of the same field; the successor cites the predecessor id 1× and finding: check:duration-unit-keys reads .describe() but not JSDoc — a duration key documenting its unit only in JSDoc never enters the population, and one card already recorded a wrong reason because of it #15939 3×; the exporter batch size batch.size at metrics.zod.ts:702 is still z.number().int().positive().optional().default(1000) — a bare count, unmoved.
  • registry.ts is the regenerated mirror. pnpm --filter @objectstack/spec gen:migration-registry in the worktree printed "213 semantic, 182 retired-key, 178 retired-def" and wrote blob 23763684981d…byte-identical to the committed blob, porcelain empty; check:migration-registry (self-test + check) exit 0. The evaluated field value is entry == registry at both refs, and the changed region is the only hunk in either file.
  • Premise re-confirmed on this head. ⚠️ The guide lives at docs/protocol-upgrade-guide.md (repo root); packages/spec/docs/protocol-upgrade-guide.md does not exist on this tree, and a grep against that path reads a false zero — my first pass hit it and was discarded. At the real path the last section is "Protocol 16 → 17"; both ids read 0 in the guide and 0 in packages/spec/spec-changes.json (unit-in-key 0 in both — no major-18 id is projected anywhere yet); lit controls Protocol 16 = 2 in the guide and data-driver-find-stream-retired = 16 in spec-changes.json; dark 0 in both. PROTOCOL_VERSION = '17.0.0' (kernel/protocol-version.ts:18), spec-changes.json "protocolVersion": "17.0.0" / "to": 17, spec 17.4.0; build-upgrade-guide.ts:60 stops at PROTOCOL_MAJOR. check:spec-changes and check:upgrade-guide both exit 0 with no diff. The protocol-18 step is still unreleased and unprojected, so the clause finishes an assembling step rather than amending published history — the premise the fix rests on holds.

Item 2 — ablation table in the PR body: DISCHARGED.

  • Schema asymmetry (the load-bearing part), read at head: burnRateWindows[].durationSeconds is z.number().int().positive() at metrics.zod.ts:602required, no .optional(), no default; retention.durationSeconds is z.number().int().positive().optional().default(604800) at :829optional with a default. Neither file changed in the delta.
  • Test lines: metrics.test.ts:622 is expect(issue).toBeDefined(); inside REFUSES the burn-rate window with a rename naming durationSeconds (:612:627) — the path lookup for errorBudget.burnRateWindows.0.window, after the success assertion at :618 that still holds; :659 is expect(result.success).toBe(false); inside REFUSES MetricsConfig.retention.period with a rename naming durationSeconds (:655:668).
  • Re-run on this head (14:16:06Z–14:16:13Z, EXIT INT TERM trap, git restore --source=HEAD --staged --worktree): control 46 / 46, exit 0. Leg 1 (delete :605): mutated blob 87687490dd… = the author's 8768749…, 1 failed / 45 passed, metrics.test.ts:622:19 AssertionError: expected undefined to be defined. Leg 2 (delete :833): mutated blob 8a84628ef1… = the author's 8a84628…, 1 failed / 45 passed, metrics.test.ts:659:28 AssertionError: expected true to be false. Both legs restored to a747965b7d…, porcelain empty, git diff HEAD empty. So leg 1 proves the lost prescription (the element is refused anyway for the missing required sibling; only the located rename message is gone) and leg 2 is the live ADR-0049 silent strip (success came back true, period stripped, the 604800 default applied). The PR body now says exactly this — its "The two legs fail differently" paragraph carries the right assertions, the right line numbers and the required-vs-optional reason.

Optional count fix: done, and reproduced three ways. Retired-key rows 175 → 182 and semantic ids 210 → 213 from 9b62f546 (pre-merge) to head: (a) the generator's own printed line, above; (b) git ls-tree file counts at head / 8261ff7171 / 9b62f546 = 182 / 177 / 175 retired-key and 213 / 212 / 210 semantic; (c) regex over registry.tsRETIRED_KEYS_BY_MAJOR rows 182 / 177 / 175 and id: ' lines 213 / 212 / 210. Set difference: rows present on main or pre-merge and missing from head = 0, gained vs pre-merge = 7 (kernel/HotReloadConfig:debounceDelay, kernel/PluginHealthCheck:interval, …:timeout, system/HttpDestinationConfig:batch.flushInterval, …:retry.initialDelay, …:timeout, system/LoggingConfig:buffer.flushInterval); ids missing = 0, gained = 3 (kernel-health-check-and-hot-reload-durations-unit-in-key, logging-durations-unit-in-key, screen-field-lookup-reference-required). The body's "177 files on main + 5 = 182; 212 + 1 = 213" reproduces. One note, not a defect: a regex over registry.ts reproduces the generator when its character class admits uppercase — a [a-z0-9-] class reads one short at every ref because of object-titleFormat-to-nameField — so "a regex over registry.ts, which miscounts" describes the author's earlier regex, not the method.

Anything the delta disturbs

Nothing found. migrations.test.ts 130 / 130 on head (the only generic constraint on acceptanceCriteria is non-empty at :44; no test pins the predecessor's text — 0 test files reference its id; "Both keep their names" and the new clause each occur in exactly 2 files, the entry and its mirror). eslint exit 0 on both changed files. node scripts/pm/check-widening-tells.mjs --diff over the delta diff: exit 0 under --declaration yes (the PR's carrier) and under no (2 files judged, no tell). CI on 84e62ed7… at 14:19Z: Type Check · source gates (the lane running check:spec-changes / check:upgrade-guide), Build Core, Temporal Conformance (live PG + MySQL), Dogfood Regression Gate 1/3–3/3 + rollup, Type Check · consumer gates, Type Check · debt ledger, Test Core 2/6 3/6 4/6, Check Changeset, Governed Surface Queue Guard, Spec property liveness all success; Lint & Repo Gates, Type Check · workspace and Test Core 1/6 5/6 6/6 still in_progress, 0 failures — the queue still needs those to conclude, and nothing the delta touches feeds them beyond the prose judged above. The baseline's two escalations (the stale 18.kernel__SandboxConfig__process.timeout.ts:7-9 sentence plus #17635's stale hunk on it; the missing Session: line on #17783's claim comment) are outside this PR and untouched by the delta — still with the PM.

Readings I re-measured

All in /tmp/review-18007b at 84e62ed7… unless a ref is named; UTC 2026-09-13.

  • 14:11:36Z git fetch origin claude/issue-17783-metrics-duration-unit-keys and both shas; git worktree add /tmp/review-18007b 84e62ed7…; git diff --stat / --name-status bcfdd356… 84e62ed7…; git log bcfdd356…..84e62ed7… = 1 commit.
  • 14:12:53Z git merge-base --is-ancestor = yes; git rev-list --count HEAD = 96 (shallow); entry file read at both refs.
  • 14:13:03Z tree greps: "Both keep their names" and the new clause = entry + registry only; dark vmkrtplx 0.
  • 14:13:18Z git ls-tree censuses and comm set differences at head / 8261ff7171 / 9b62f546.
  • 14:14:15Z guide and packages/spec/spec-changes.json id counts with lit and dark controls; PROTOCOL_VERSION; spec 17.4.0.
  • 14:14:21Z metrics.test.ts:622 / :659; metrics.zod.ts:589:607 and :818:840; delta diff over both files = empty.
  • 14:14:37Z evaluated acceptanceCriteria at both refs, entry and registry: 1096 → 1443 chars, prefix + single space.
  • 14:14:43Z id-census reconciliation (object-titleFormat-to-nameField); migrations/types.ts:39-40; validators of the field.
  • 14:15:11Z–14:15:18Z gen:migration-registry (blob = committed), check:migration-registry, check:spec-changes, check:upgrade-guide — all exit 0, porcelain empty.
  • 14:16:06Z–14:16:13Z ablation control + two legs, hashes as reported.
  • 14:16:14Z successor row registry.ts:13784; batch.size at :702; eslint on both files exit 0.
  • 14:17:02Z migrations.test.ts 130 / 130; 14:17:36Z widening-tells on the delta diff, exit 0 under both declarations.
  • 14:19Z CI check runs on the head, as listed.

Taken on trust (not re-measured): the full check:generated 15 / 15 on head (needs a built dist; I ran the three gates the delta can reach, and CI's Type Check · source gates lane is success on this head); the dispatch-gates 109 / 109 family (run on bcfdd356…, as the body states); and everything the baseline review passed that the delta does not touch.

Implemented-by: claude/issue-17783-metrics-duration-unit-keys
Reviewed-by: session_015c5G6TmpMKgnusmTpD7Ntt

VERDICT: PASS


Generated by Claude Code

Copy link
Copy Markdown
Collaborator

Carriers cleared — provenance

needs:contract-review stripped from both carriers at 2026-09-13T14:42Z: this PR and card #17783, each read back after the write.

review of record #18007 (comment) (delta)
baseline bcfdd35609 — FAIL, record 5653712837; both items discharged
head judged 84e62ed7129e7cd6e1b6a307530cdf4f9d3602a3 — unchanged at strip time
verdict PASS, adopted verbatim
served tier verified from the review transcript's harness stamps: claude-fable-5-1 × 70, no other model value, dark control 0
--pair 18007, post-strip exit 0
checks 35 runs, 31 success + 4 skipped, 0 failure

Full acceptance is on the card: #17783 comment 5653845081.

⚠️ Two superseded review records above it, both my fault, recorded so the thread reads straight. 5653835652 and 5653920672 carry the identical verdict and findings; each was re-posted only because a machine-read field in my output template was unreadable:

  1. Implemented-by: branch claude/… — the rule is 「值紧跟冒号,前置词即不可读」; BRANCH_TOKEN/SESSION_TOKEN are ^-anchored and strip only whitespace/bold/backtick decoration, so a leading word reads as no identity.
  2. `Head-sha: 84e62ed7…`H51_SHA_SPAN is /`([0-9a-fA-F]{7,40})`/ and wants the sha alone in its span; with the label inside, the span is not pure hex. Verified: this comment's predecessor carried six pure-hex spans and none prefixed the head, so C6 was right.

⛔ Neither re-post changed the verdict, the findings, or a single ①②③ line. The reviewer verified its third draft by importing the parser directly — contractReviewHeadMatch(body, head) returns the full sha; the old shape returns null.

⚠️ The detection gap that let this reach two already-merged PRs: C4/C6 evaluate the review of record and only fire after the carriers are cleared, because they verify a clearing was justified. Running --pair only as the pre-strip step ② — which the landing sequence invites — structurally cannot see them. #17986 and #17999 landed with the same unreadable pair. Root cause filed as #18042: the contract-review record is the only machine-read artefact in the skill with no literal template to copy, only prose.

⛔ No approval is given or implied. The clause-② review clears the queue gate; it is not a review approval, and this seat neither approves nor merges.


Generated by Claude Code

@zhuangjianguo
zhuangjianguo marked this pull request as ready for review September 13, 2026 14:43
@zhuangjianguo
zhuangjianguo added this pull request to the merge queue Sep 13, 2026
Merged via the queue into main with commit 24489f1 Sep 13, 2026
65 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-17783-metrics-duration-unit-keys branch September 13, 2026 15:12
zhuangjianguo pushed a commit that referenced this pull request Sep 13, 2026
`registry.ts` is generated from `src/migrations/entries/` and is deliberately
NOT_DRIVER_MANAGED, so it conflicts textually on every pair of parallel
registrations — #18007 (#17783, metrics) landed 9 rows and 4 semantic entries
while this branch carried 5 of its own. The entries directory itself merged
clean at 579 rows with zero conflict markers, which is what that directory
exists for, so the conflict is resolved by regeneration, never by hand.

Set difference over the entry set, by exact path: 0 lost / 13 gained against
this branch's pre-merge head, 0 lost / 5 gained against origin/main. The
generator's own line reads 215 semantic, 186 retired-key, 178 retired-def.

Claude-Session: https://claude.ai/code/session_015c5G6TmpMKgnusmTpD7Ntt
Co-authored-by: Claude <noreply@anthropic.com>
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 protocol:system size/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

spec: 5 duration key(s) in system/metrics.zod.ts name their unit only in JSDoc — #15939 Ruling A remediation (5 of the 21-row delta)

2 participants