Repository navigation
fix(metadata-protocol): the default /diff range labels its to side with the active row's own version - #20443
Conversation
… active row's own version With no toVersion, diffMetaItem compared the active sys_metadata row's body but labelled it with the newest sys_metadata_history row, which is a draft save whenever a draft is pending. One read of the active row now supplies both the body and its version; with no active row both labels are null. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N8TPEsoJxPsdSdNKGnNGEN
Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N8TPEsoJxPsdSdNKGnNGEN
📓 Docs Drift CheckThis PR changes 1 package(s): 16 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 6 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 11 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin daf7398d9e43cf663b88283efc2f5378c475860d && git checkout daf7398d9e43cf663b88283efc2f5378c475860d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 0fcb10184ce5bba6d5538b555b3a898e5ecfc5a5 fd767fec0c8c30181337560801937aa54f1979c4 && git checkout -B drift-repro 0fcb10184ce5bba6d5538b555b3a898e5ecfc5a5 && git merge --no-ff fd767fec0c8c30181337560801937aa54f1979c4
node scripts/docs-audit/affected-docs.mjs --json 0fcb10184ce5bba6d5538b555b3a898e5ecfc5a5
|
Contract reviewServed-tier: Inputs read: card #20397 (body and all four comments: triage 5865097374, claim 5868801238, os-dev-report 5870523105, seat answer 5870555349); PR #20443 body, file list and the net diff against the merge base with ① Derived judgments
② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS |
…rlier version whose body differs (objectstack-ai#20451) (objectstack-ai#20518) Fixes objectstack-ai#20451 Clause-②: no ## What changes `diffMetaItem` (`packages/metadata-protocol/src/protocol.ts`), the default `from` side only. With no `fromVersion`, the from side is now the **nearest earlier history row whose body differs from the to side's**, by the diff's own equality: `diffShallow`'s three buckets not all empty, with an absent body compared as `{}` (the same `?? {}` the comparison below it uses). A body-less row (a delete's tombstone) therefore differs from any non-empty to side, and the walk stops on it (triage's answer A, 5875579209). With no earlier row that differs, the from side is absent: `fromVersion: null`, everything added. - The walk reads only the rows the function already has: the one `find` over `sys_metadata_history` (no limit), which the function sorts by `version` in memory. It adds no read and no cap. A unit pin asserts one history read on a walk path. - It reads no `operation_type` and adds no state column (ruling B on objectstack-ai#20378, 5865708652). - An explicit `?from=` / `?to=` names exactly its versions. The to-side default (the active row's own version, PR objectstack-ai#20443) is untouched. An explicit `?to=` with no `?from=` walks back from the named version's body. - The comparison runs on the stored bodies before redaction, as the diff itself does, so a credential-only change still stops the walk and its values are still not served (unit pin). The four statements of the default rule now say the new rule, each in its own words: the `diffMetaItem` docblock, `DiffMetaItemResponseSchema`'s JSDoc (`packages/spec`), the route's OpenAPI summary in `rest-server.ts`, and the SDK's `diffItem` docblock (`packages/client`). Comment and summary text only: no schema, route or signature change. ## Measured on the real REST stack The REST pins (`packages/rest/src/meta-diff-default-range-labels.test.ts`, real routes and real writes over better-sqlite3 `:memory:`) were committed first (`9b308b12b`) and run against the unchanged source with its closure built: **3 failed, 9 passed**. After the change: **12 passed**. | history (fixture-proved by reading `sys_metadata_history`) | before | after | |:--|:--|:--| | v1 `create` A (active), v2 `create` B (draft save), v3 `publish` B | `2 → 3`, empty | `1 → 3`, `label` and `columns` changed, equal to `?from=1&to=3` | | the same, then a v4 draft save | `2 → 3`, empty | `1 → 3` | | v1 `create` A, v2 `delete` (no body), v3 `create` A2 (draft), v4 `publish` A2 | `3 → 4`, empty | `2 → 4`, everything added, equal to `?from=2&to=4` | | v1 `create` A, v2 `delete` (no body), v3 `create` B (active) | `2 → 3`, everything added | the same bytes (green before and after) | | v1 `create` New (draft), v2 `publish` New | `1 → 2`, empty | `null → 2`, everything added | | v1 `create` (active) only | `null → 1`, everything added | the same bytes | | explicit `?from=2&to=3` over the first lineage | `2 → 3`, empty | the same bytes | The unit pins in `protocol.diff-dead-history-read.test.ts` repeat these lineages over seeded rows beside the file's read-counting double. **Ablation**, from the committed state: `node scripts/ablation-replace.mjs` replaced the walk's differ test (`if (d.added.length || d.removed.length || d.changed.length) {` → `if (true) {`, which is the old immediately-previous rule), anchor 1 → 0, blob `a2d2b7686f29` → `dd9cffcbd1f9`; the file ran **6 failed / 14 passed**, exactly the six walk-dependent pins; restored, blob == HEAD and `git diff HEAD` empty. No build is involved: the metadata-protocol suite imports `./index.js` from source. ## A pending release note corrected: needs confirmation (Check Changeset stays red) `.changeset/20397-diff-default-range-labels.md` (PR objectstack-ai#20443, not yet released) said "The default `fromVersion` is still the history version immediately before that label." This PR makes that sentence false in the same release, so it now reads: "The default `fromVersion` rule is not changed by this entry (objectstack-ai#20451, in the same release, then moves it to the nearest earlier version whose body differs from the to side's)." One sentence, nothing else in that file. This is the DELIBERATE CORRECTION class `check-empty-changeset.mjs` names, so that gate exits 1 locally and **Check Changeset will stay red on purpose**. Please confirm the correction on this PR. It was outside the claim's file surface. `skip-changeset` is not applied and must not be. ## Changeset `.changeset/20451-diff-default-from-differs.md`: `@objectstack/metadata-protocol` `patch` and `@objectstack/rest` `patch`, `Clause-②: no`. The rest line is there because the route's OpenAPI summary is a runtime string served in the OpenAPI document. The `packages/spec` JSDoc and the `packages/client` docblock are comment-only, so they get no line, per the repo's rule that comments do not publish. All three packages are in one `fixed` group, so versions do not move differently either way. ## Verification (measured at `1f258bbd5`, after merging `origin/main` `9449512a3` with a true merge commit) - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`: 88 commands, all run, each exit code written to a file before any pipe. **87 exit 0; 1 exit 1**: `node scripts/check-empty-changeset.mjs --base origin/main`, the release-note correction above. Also run, outside the derivation: the four roster gates whose roster sits under a changed path (`check-changeset-fixed`, `check:meta-url-spelling`, `check:spec-changes`, `check:error-code-casing`), all exit 0. - `dispatch-gates --ran`: "88 derived famil(ies) accounted for — 88 run, 0 NOT-MEASURED". - Tests, each through `os-verify-lock`: metadata-protocol 189 files passed, 3 skipped (2759 tests); rest `--project local` 219 files passed (4181 tests); client 50 files passed (641 tests); spec `--project local` 573 files passed (16801 tests). - Typecheck: metadata-protocol, rest (with `check:test-typecheck`), client (with `check:test-typecheck`) and spec all exit 0. Both edited test files are in their tsc programs (`--listFiles`: 1 hit each). - `pnpm --filter @objectstack/spec check:generated`: all 15 generated artifacts up to date, against a spec `dist` built from this tree. - Declared to CI, not run here: the five path-scheduled jobs (Test Core shards, Temporal Conformance, Dogfood Regression, Dogfood Verify CLI, Build Core) and the workspace type-check lanes, which `dispatch-gates` lists as CI's own shell. ## Statements the census named, measured - **Edited:** the four above, plus the header of `meta-diff-default-range-labels.test.ts` (this PR adds to that file), which said the from side is "the history row immediately preceding that label". - **Not false, not edited:** `docs/qa/platform-checklist/areas/studio-authoring.json` lines 346 and 402 ("omit the params for previous-vs-current"). On that probe's lifecycle (draft save, publish, second draft save, second publish) the default range now compares the second published revision with the first (`2 → 4`), which is the comparison those steps describe. Before this PR it compared the second publish with its own draft save and answered "no changes". - **Not false, not edited:** the test title in `rest-server-query-number-reads.test.ts:300` ("from/to still mean previous-vs-current (no version members)"). It asserts only that no version member reaches the verb, which still holds. This PR does not touch that file. ## Acceptance notes - The same checklist steps' explicit range `?from=1&to=2` compares the probe's first draft save (v1) with its own publish (v2). In the draft-then-publish lifecycle those two rows carry the same body, so that range answers "no changes", before and after this PR. This is a checklist wording issue, not a product defect. No card filed; carrier: none. - `DiffMetaItemResponseSchema.fromVersion`'s `.describe()` reads "`null` when that side is absent (e.g. the item had no earlier version)". It is still true, and now `null` also answers "no earlier version differs". It was left as is: a `.describe()` edit regenerates spec docs, and the claim limits `packages/spec` to comment text. - `.changeset/20139-rest-query-number-census.md` says "previous-vs-current on `/diff`" about absent parameters keeping their default. That is still true of the parameter handling. It is somebody else's pending note and is not touched. - Test-side deviation: `seedLineage` in `protocol.diff-dead-history-read.test.ts` moved from inside the objectstack-ai#20397 `describe` to module scope, unchanged, so the objectstack-ai#20451 block shares it rather than copying it. --- _Generated by [Claude Code](https://claude.ai/code/session_01N8TPEsoJxPsdSdNKGnNGEN)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
Fixes #20397
Clause-②: no
What changes
diffMetaItem(packages/metadata-protocol/src/protocol.ts), its default range only. With notoVersionthe to side is the current activesys_metadatarow. Its body was compared, buttoVersioncame from the newestsys_metadata_historyrow. That row is a draft save whenever a draft is pending, because every draft save appends a history row too. So the labels and the bodies named different rows.Now one read of the active row supplies both facts: the body compared, and the row's own
versionas the label.SysMetadataRepository.putstamps that column in the same transaction that appends the history row carrying the same body. ThefromVersiondefault is unchanged: the history version immediately before the label. With no active row, the to side is absent, sotoVersionisnull, asDiffMetaItemResponseSchemadeclares for an absent side.The hunk also deletes the two locals the old read used (
repo,fullRef; nothing else in the function read them), and adds one clause to the method's docblock. Nothing else inprotocol.tsmoves.rest-server.ts,packages/specandsys_metadata_historyare untouched. The response shape is unchanged:@objectstack/metadata-protocolpatchchangeset.Measured before and after, on the real REST stack
Real routes and a real
ObjectStackProtocolImplementationover better-sqlite3:memory:with the realsys_metadata*objects. Each read isGET /meta/:type/:name/diffwith nofrom/to, as an author (manage_metadata). "Before" isorigin/maindbddf02c1(the dist carried the old label line). "After" is53cad078f. The probe was a scratch file, deleted and never committed.2 → 3,labelchanged "Atlas v2 draft" → "Atlas v1": the to side is the v1 bodynull → 1, the v1 body as added1 → 2null → 1, the v1 body as added2 → 3, no changes1 → 2,labelA → B, byte-equal to?from=1&to=22 → 3, no changes1 → 2, byte-equal to?from=1&to=21 → 2, every v1 key under removed (a draft body served as the from side)null → null, emptynull → 1, emptynull → null, empty2 → 3, every v2 key under removednull → null, empty (see Acceptance notes)3 → 4, no changesnull → null, empty2 → 3no changes (3 → 4with v4 pending)2 → 3no changes in both cases (see Acceptance notes)The order's mechanism hypotheses
request.toVersion === undefinedarm read the body throughrepo.get(..., { state: 'active' })and the label fromhistRows[histRows.length - 1].version.sys_metadatarow carriesversion, equal to the history row whose body it is. Measured in every lineage above: active v1 = history v1, active v2 = history v2, the published row v3 = thepublishhistory row v3. TheMetadataItemthatrepo.getreturns does not carry it.rowToItembuildsreffromfullRef(noversion) and exposes only the contenthash. SodiffMetaItemreads the row itself, with the same predicaterepo.getuses (active state, no package scope). The same file has two precedents: the ADR-0067 commit capture inpublishPackageDraftsreads the raw active row'sversionasprevVersion, andresolveOverlayPackageBindingreads the raw row rather than wideningMetadataItem. Nothing looks a version up by body or hash.toVersiondoes not change the from rule. With v1 active, v2 draft, v3 publish, the default answers2 → 3"no changes". The from side is the unpublished draft save v2, whose body is the one v3 published. The answer is the same with a v4 draft pending. The previously published v1 differs and is not the from side. Which history rows count as versions is not changed here. See Acceptance notes.null → null, empty buckets, and no draft body on either side. Before: labelled with the newest draft save, and with two draft saves the first one's body was served as the from side.Tests
packages/metadata-protocol/src/protocol.diff-dead-history-read.test.ts: a new#20397describe block, 5 cases over seeded rows. It sits beside that file's already-pinned engine double, which honours thewhereon both tables, so the engine-double ledger is untouched. The cases: a pending draft forviewand forappequals the explicit range; the card's app reading; draft-only; deleted, with the deletion still readable as2 → 3.packages/rest/src/meta-diff-default-range-labels.test.ts(new, test side only): 6 cases through the real routes and the real writes.toVersionequals the active row'sversion, read past every door, and the answer equals?from=1&to=2for anappand aview. The two card readings, draft-only, and a lit control with no draft pending, which answers1 → 2both before and after.Ablation, from the committed fix (
53cad078f), throughscripts/ablation-replace.mjs. The mutation puts the old label line back (anchor 1 → 0, blob3bb7041da297→7e3ce0508bd8). After a@objectstack/metadata-protocolrebuild,ablation-dist-preflightfound the marker in 2 built files, exit 0.protocol.diff-dead-history-read.test.ts: 5 failed, 6 passed (the 5 new cases red, the 6 older ones green).meta-diff-default-range-labels.test.ts: 5 failed, 1 passed (the lit control). The restore leg brought the blob back to the HEAD blob withgit diff HEADempty. After a rebuild,--absentfound the marker absent from all 24 built files and the tree clean (exit 0). The re-runs gave 11/11 and 6/6. Direction: red, as predicted. (A first run proved the same mutation and the same 5 + 5 reds. Its preflight tree reading refused only because the source spells the marker with a!that the build drops, so it was re-run with--source-marker.)Gates, at the measured head
fd767fec0(after a true merge oforigin/main)pnpm --filter @objectstack/metadata-protocol exec vitest run: 189 files passed, 3 skipped; 2750 tests passed, 19 skipped.typecheck(tsc --noEmit) exit 0. Its program includes the edited test file (--listFilesOnly: 1).pnpm --filter @objectstack/rest exec vitest run --project local: 215 files passed; 3898 tests passed, 26 skipped.typecheck(tsc --noEmit pluscheck:test-typecheck, 0 debt) exit 0.node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackderived 62 commands. All 62 exit 0.--ranreconciliation: 62 derived, 62 run, 0 NOT-MEASURED, a derived zero with every exit code recorded.check:dual-build-cjs-loadsandcheck:type-check-debtfirst answered PREREQUISITE NOT MET (exit 3). They were re-run exit 0 afterturbo run build --filter='./packages/*' --filter='./packages/*/*', and the dist-reading gates were re-run on that full build.--no-inline-config --format jsongave 3 results, 0 errors, 0 warnings. All 3 are in eslint's own population (isPathIgnoredfalse for each). This config enables no type-aware linting (noparserOptions.project), so the diff cannot move an untouched file's verdict. The repo-widepnpm lintis CI's.Acceptance notes
GET /meta/:type/:name/diffserves PENDING draft content to a member with no authoring capability: its history versions include draft saves, and it is the one draft-serving door the #20338 gate leaves open #20378. The default from side is the history row immediately before the label, draft saves included. So after a draft-then-publish, the default range answers "no changes" (N-1 → N) against the draft that was published, not against the previous published version. That is the documented rule ("the immediately previous history row"), and this PR does not change which history rows count as versions. The order names [finding]GET /meta/:type/:name/diffserves PENDING draft content to a member with no authoring capability: its history versions include draft saves, and it is the one draft-serving door the #20338 gate leaves open #20378 as the carrier. Its ruling (5865708652) took B and declined A (a state column onsys_metadata_history), so as the thread stands no card holds that question.N-1 → Nup to its tombstone, every key removed. It is nownull → null, the same rule as a draft-only item: the to side is absent. The deletion is still one explicit range away (?from=N-1&to=N, pinned).SysMetadataRepository.get's docblock (sys-metadata-repository.ts) names "diffMetacompares this body againstsys_metadata_historybodies" as a reasongetstays verbatim.diffMetaItemno longer callsget. It reads the same row verbatim itself, and its comment carries the same no-conversion rule. The bullet's reasoning holds, but it names a caller that is gone. Carrier: none; noted, not filed./diffhas no in-repo caller of its default range (client.meta.diffItemhas one docs example). objectui'sMetadataClient.diffhas zero callers at the pin, as the [finding]GET /meta/:type/:name/diffserves PENDING draft content to a member with no authoring capability: its history versions include draft saves, and it is the one draft-serving door the #20338 gate leaves open #20378 round measured.Generated by Claude Code