Skip to content

fix(spec): emit one api-surface row per declared kind, so deleting the value half of a dual declaration is breaking (#15919) - #16554

Merged
huangyiirene merged 3 commits into
mainfrom
claude/issue-15919-api-surface-dual-kind-rows
Sep 7, 2026
Merged

fix(spec): emit one api-surface row per declared kind, so deleting the value half of a dual declaration is breaking (#15919)#16554
huangyiirene merged 3 commits into
mainfrom
claude/issue-15919-api-surface-dual-kind-rows

Conversation

@huangyiirene

Copy link
Copy Markdown
Collaborator

Fixes #15919

Clause-②: yes

Read this first — the diff is large and almost entirely mechanical

134 of the changed lines are generated rows under packages/spec/api-surface/. Before reading them:

  • +134 rows across 10 of the 17 entry points./api 48, ./system 39, ./ui 14, ./data 13, ./automation 6, ./security 5, ./kernel 4, ./shared 3, . 1, ./identity 1.
  • It is a PURE INSERTION. git diff --numstat over api-surface/ reads 48/0 · 39/0 · 14/0 · 13/0 · 6/0 · 5/0 · 4/0 · 3/0 · 1/0 · 1/0 — zero rows removed, zero modified, no reordering, because the code-unit sort places Name (const) immediately before its existing Name (type) sibling.
  • ZERO rows represent a real interface change. Every one of the 134 is the (const) half of a name that was already exported and already recorded — recorded as (type) alone. Nothing was added to or removed from the package: api-surface-signatures.json did not move, and the built dist/ is identical (dist/.build-input-hash is e0550924e60ca64e before and after, since no src/ file changed).
  • The row grammar is untouched. Rows are still Name (kind); only completeness moves.

The gate's own diff engine says the same thing in one line — run against the pre-regeneration baseline it printed:

@objectstack/spec public API changed: 0 breaking (removed/narrowed), 134 added.

The two files worth actually reading are packages/spec/scripts/build-api-surface.ts and the new packages/spec/scripts/api-surface-dual-kind-rows.pin.test.ts.

The defect

TypeScript merges an export const X and an export type X into ONE symbol whose flags carry both. build-api-surface.ts mapped that symbol through a first-match-wins kindOf that tested TypeAlias before Variable, so a dual-declared name always printed (type) and the value half was never enumerated. The shard format is name-keyed and recorded it once.

The population is closed at 132 symbols, every one of them the same type SHADOWS const shape — measured on the previous lap from the gate's own TypeScript checker, not by grep. 134 rather than 132 rows because two names are exported from two entry points each.

The repair

kindsOf now returns every kind the symbol's flags declare, and buildSurface emits one row per kind. Ordering is preserved from the old lookup so the shards stay stable, and other is still the answer for a symbol matching no branch.

Not a combined kind (Name (const, type)). That was settled by exercising the real consumer, packages/spec/scripts/lib/docs-import-surface.ts:

encoding consumer behaviour
two rows parse today. loadEntrySurfaces already models a name as a set of kinds, so resolveValueName and resolveTypeName stay bit-identical. Zero consumer changes needed.
combined kind throwscannot parse entry "…", because SURFACE_LINE admits one word-character group. And after widening that regex, resolveTypeName returns null for all 132 names, silently dropping the import type line from every one of those doc pages.

Both halves of that table are now pinned as tests, so the rejected encoding cannot be reintroduced by someone who only sees the throw and widens the regex to make it go away.

check:docs is green on this branch with no regeneration, which is the artifact-level confirmation that the resolvers really did not move: not one reference page changed.

Proof that the gate now SEES the removal

The previous lap ablated origin/main and proved the gate blind: deleting export const RestApiRouteRegistration while keeping its type alias left 17/17 shards byte-identical, the export total still 5277, and check:api-surface printing "public API surface + factory signatures unchanged" at exit 0. This PR re-ran exactly that mutation on the fixed tree.

Positive control first — on the fixed, unmutated tree:

check:api-surface  ->  exit 0
@objectstack/spec public API surface + factory signatures unchanged ✓

A gate that is red for everything is not a fixed gate; this is what makes the red below mean something.

Mutated leg — same deletion, pnpm --filter @objectstack/spec build re-run, mutation proven on disk before anything was read (deleted-anchor grep -c 1 to 0, surviving export type count still 1, blob a6f98505… to b6fd374c…, dist/.build-input-hash e0550924… to 07681a56…) and proven to have reached dist/ (ablation-dist-preflight … --absent printed "marker absent from all 218 built files"):

check:api-surface  ->  exit 1
  ./api
    - RestApiRouteRegistration (const)
@objectstack/spec public API changed: 1 breaking (removed/narrowed), 0 added.
A REMOVED export or a CHANGED factory signature is a BREAKING change for third parties …

Restore leg, proven by state and not by an exit code: blob back to a6f98505…, git diff HEAD empty, dist/.build-input-hash back to the exact leg-0 value e0550924e60ca64e, preflight present-mode marker present in 2 built files + working tree clean against HEAD, and check:api-surface green again. The mutation script carried trap … EXIT INT TERM with absolute paths resolved from git rev-parse --show-toplevel.

The new pin, and proof it can fail

check:api-surface compares the generator against its own committed output, so it cannot notice the generator becoming less complete: revert kindsOf and regenerate, and it is green again on a surface that has silently dropped 132 value exports. api-surface-dual-kind-rows.pin.test.ts reads the committed shards and asserts the property directly.

Reverse-validated by deleting the EpochMs (const) row from the committed shard — the exact state a reverted generator would write:

FAIL  records both halves of `EpochMs`, the worked example from the card
AssertionError: expected Set{ 'type' } to deeply equal Set{ 'const', 'type' }
Test Files  1 failed (1)
     Tests  1 failed | 4 passed (5)

1 failed and 4 passed — including both negative controls, so this is a discriminating pin and not a blanket red. Restored and proven by blob hash plus an empty git diff HEAD.

The negative controls matter as much as the assertion: a generator that stamped every kind onto every name would satisfy "some name carries two kinds" too, so the pin asserts in the same parse that const-only names and type-only names both still exist, and that the aggregate is non-empty.

Changeset: patch, and why

Measured against what a consumer of the published package can observe:

  • No export was added, removed or renamed. dist/ is byte-identical across this change; the TypeScript surface is exactly what it was.
  • The row grammar Name (kind) is unchanged, so anything that parsed api-surface/ before parses it now — including this repo's only in-tree parser, which already modelled a name as a set of kinds.
  • The 134 new rows describe exports that already existed. The record got more complete; no capability arrived.

That is a correction to the accuracy of a shipped record plus a strictness increase in this repo's own CI gate — a fix, not a feature. minor would advertise new API to a consumer reading the changelog, and there is none.

One consequence for the release seat, stated because it is invisible in the diff: build-spec-changes.ts --previous-surface is a release-time join, so a release crossing this change will list those 134 rows as added surface entries in spec-changes.json and the upgrade guide. They are not new API — they are the same exports, newly recorded. The committed spec-changes.json is unaffected here because that flag is opt-in and was not passed.

Verification

what result
pnpm --filter @objectstack/spec build (3 runs, under scripts/pm/os-verify-lock.sh) VERDICT command-exit 0 each time
pnpm --filter @objectstack/spec test 485 files / 13156 tests passed, VERDICT command-exit 0
pnpm --filter @objectstack/spec typecheck VERDICT command-exit 0 (includes check:scripts-typecheck, which compiles the edited scripts/)
pnpm --filter @objectstack/spec check:generated exit 0 — every spec generated artifact up to date
pnpm lint (eslint . --no-inline-config) exit 0 — 6277 files enumerated by eslint from its own config, 0 errors / 0 warnings
the 60 gate commands derived by node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack 58 exit 0
pnpm check:dual-build-cjs-loads · pnpm check:type-check-debt exit 3, PREREQUISITE NOT MET — both read a built monorepo closure and only @objectstack/spec was built here. NOT MEASURED, not a pass and not a finding; declared to CI, which builds the closure first

Review routing


Generated by Claude Code

TypeScript merges `export const X` and `export type X` into one symbol
carrying both flags. `build-api-surface.ts` mapped that symbol through a
first-match-wins `kindOf` that tested TypeAlias before Variable, so the
shard recorded `X (type)` alone and the value half was never enumerated.
Deleting `export const X` therefore left the shard byte-identical and
`check:api-surface` green on a removed public value export.

`kindsOf` now returns every kind the flags declare, and `buildSurface`
emits one row per kind. The row grammar `Name (kind)` is unchanged; only
completeness moves.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T6HeZvT9wdSJD1ZxJb5Eno
…rations

Mechanical `gen:api-surface` output. +134 rows over 10 of 17 entry points,
every one of them the previously-unenumerated `(const)` half of a name that
also declares a type. Pure insertion: 0 rows removed, 0 modified, no
reordering — the code-unit sort places `Name (const)` immediately before its
existing `Name (type)`. Export total 5277 -> 5411;
api-surface-signatures.json unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T6HeZvT9wdSJD1ZxJb5Eno
api-surface-dual-kind-rows.pin.test.ts asserts the committed shards carry
both halves of a dual-declared name, with negative controls so a generator
that stamped every kind onto every name fails here too. check:api-surface
cannot catch that regression on its own: it compares the generator against
its own regenerated output.

docs-import-surface's fixture now models the merged declaration as the two
rows the generator really emits, and two new cases pin why two rows and not
a combined kind: the extra row leaves both resolvers unmoved, while a
combined kind throws and, after widening the parser, silently resolves to
no type at all.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T6HeZvT9wdSJD1ZxJb5Eno
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

⚠️ 10 changed file(s) yielded no anchor (packages/spec/api-surface/api.json, packages/spec/api-surface/automation.json, packages/spec/api-surface/data.json, …), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files. Nothing else in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 1 changed package(s)).

What this run could not see
  • 10 changed file(s) yielded no anchor (packages/spec/api-surface/api.json, packages/spec/api-surface/automation.json, packages/spec/api-surface/data.json, …) — pages documenting those are invisible to this run
  • 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 — 130 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 f48f3f1b21fe207c39d94e49609ac49e19635e3bpackageMentionDocs.

Copy link
Copy Markdown
Collaborator Author

Contract review at CONTRACT_REVIEW_TIER: PASS — no blocking findings. Four advisories; none holds the PR.

Reviewed at claude-fable-5-1 by an isolated reviewer. Base 78e8485d67 (merge-base confirmed), head 5c8fa344bb, from a detached worktree with a filtered install and no build — so no gate's baseline could move under it. This seat runs claude-opus-5, below tier, and ⛔ did not self-review.

It re-derived the whole claim rather than checking it

Not a rubber stamp — the reviewer built its own harness (ts.createProgram over all 17 exports subpaths, getExportsOfModule → unalias → flags, run through both the old kindOf and the new kindsOf) and got:

  • rows 5277 → 5411, delta 134; multi-kind pairs 134, unique names 132; the only combination present is type+const — no class+interface, no function+namespace, no three-flag symbol.
  • src-derived NEW surface vs the committed shards: 0 mismatching entries over 17. The committed artifact is exactly what the checker says.
  • Pure insertion confirmed independently: base row sequence is a subsequence of head in every shard; +134/−0; every added (const) has its (type) twin in the same shard; the two double-entry names are HttpMethod and ExpressionDialect; api-surface-signatures.json unchanged.
  • It re-ran the see-it-now ablation itself, no build: deleting the export const half loses exactly ./api: RestApiRouteRegistration (const) under kindsOf, and the OLD kindOf sees nothing. Landing and restore both proven by blob hash.

⭐ A2 — the reviewer supplied a better justification for the new pin than the PR body gives

The PR body says check:api-surface "cannot notice the generator becoming less complete." Precisely: it does go red on a regression without regeneration (134 - X (const) rows). What it cannot do is distinguish that from a legitimate removal — and its own remedy line tells you to regenerate, which makes it green.

The real case for the pin, which the reviewer measured and the body does not state:

.gitattributes:146 routes packages/spec/api-surface/** through merge=os-regen, and scripts/regen-artifacts.mjs:151 lists it — so a regression riding a driver regen would be green with no human ever reading the deletions.

⇒ The pin's own header already frames it correctly ("revert kindsOf and regenerate"). Recording the stronger mechanism here so the durable record carries it. ⛔ Not worth a repair round on its own.

Changeset — patch confirmed correct, and the reasoning is worth keeping

AGENTS.md Post-Task Checklist §3 ("a bug fix in a released package takes a patch"), with the precedent being the previous fix to this same gate (#16176, also patch). What a consumer observes: dist unchanged (the diff touches zero files under packages/spec/src/), row grammar unchanged, and every added row describes an export that already shipped. minor would advertise API that did not arrive. The one release-visible consequence — 134 added entries in the next --previous-surface join — is disclosed in the changeset body.

The other advisories

  • A3 — the reviewer did not reproduce the author's build-derived numbers (dist/.build-input-hash, the ablation blob) because it did not build. It measured the equivalent instead: zero src/ files in the diff ⇒ dist unchanged by construction. Its own ablation blob differs because its deletion was textually different — correctly stated as not a discrepancy rather than banked as one.
  • A4 — the pin header says "130-odd value exports"; body and changeset say 132. Cosmetic.

⚠️ A1 — a real follow-up, and I am carding it

packages/spec/scripts/build-export-origins.ts:117 and check-dual-source-exports.ts:87 carry verbatim copies of the old first-match lookup. After this PR, api-surface/ records the 132 names as const+type while export-origins/ still records (type) alone — the two artifacts now disagree on kind semantics. And scripts/lib/export-origins-testkit.ts:213-246 (runtimeParityOf) explicitly under-claims on exactly this shape: its RUNTIME_KINDS filter skips those 132 value exports in the missingAtRuntime check.

⇒ Measured no gate conflict — nothing joins the two artifacts by kind (the reviewer grepped every script reading both; only prose and registry mentions). ⛔ So this is not this PR's job and does not hold it. Filed separately.


Flipping ready and arming. The advisories are prose and a follow-up; none makes the shipped artifact wrong.


Generated by Claude Code

@huangyiirene
huangyiirene marked this pull request as ready for review September 7, 2026 09:13
@huangyiirene
huangyiirene added this pull request to the merge queue Sep 7, 2026
Merged via the queue into main with commit 90e7e6d Sep 7, 2026
40 checks passed
@huangyiirene
huangyiirene deleted the claude/issue-15919-api-surface-dual-kind-rows branch September 7, 2026 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

2 participants