Uh oh!
There was an error while loading. Please reload this page.
Keep machine constants off the skill-reference Exports: fallback - #12391
Merged
Conversation
…fallback The `Exports: …` line a pointer row falls back to when its module has no doc block ranked by SOURCE ORDER and had no notion of authorable surface, so `slice(0, 5)` kept whichever five exports were declared first. Three of the eleven modules reaching that fallback declare machine constants near the top, so three published rows headlined them — `DEPRECATED_APPROVER_TYPES` and `NON_AUTHORABLE_APPROVER_TYPES` among them, on a surface loaded whole into a customer agent's context window to teach it what it may author. SCREAMING_SNAKE exports are now dropped and source order kept for the rest, with the cap of five applied AFTER filtering so the authorable names behind the constants are promoted rather than the row merely shortened. A module whose whole export surface is machine constants falls through to no description rather than a bare `Exports:`. Schema-first sorting was considered and NOT taken: on the row that motivated this it demotes `ApproverType`, the enum an author actually writes. The rule moves to `scripts/lib/export-list.ts` so it can be pinned without running the generator (`main()` runs on import), and `export-list.test.ts` enforces it as unit cases plus a corpus gate over the checked-in artifacts. Closes#12201 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RMTpSRF5CjMmQBFfPtPCwJ
Contributor
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs. What this run could not see
Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
os-zhuang
marked this pull request as ready for review
August 26, 2026 00:13
os-zhuang
enabled auto-merge
August 26, 2026 00:13
…-> 14, by name) The pin is judged over the live declaration table by design; the skills/** declaration this PR adds moves the rollback count, so it is recorded by name per the pin's own convention. Both legs green: self-test 40 assertions, main check 105/84 globs covered.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes#12201
Implements the recorded adjudication (comment 5412452726, re-dispatched at 5418442483): ranking A — skip
SCREAMING_SNAKE, keep source order for what remains. ⛔ Schema-first reordering is NOT taken; ⛔ the loud-refusal option (require module doc blocks, drop the fallback) is NOT taken, and no module doc blocks were authored.The defect
When a
.zod.tshas no module doc block,extractDescription()falls back to an export list. The line is true — an accurate list of what the module exports, which is why #12094 kept it. What is wrong is the ranking: rank was source order, and the extraction had no notion of authorable surface, soslice(0, 5)kept whichever five exports were declared first. Three of the eleven modules that reach the fallback declare machine constants near the top and therefore headlined them, on a surface loaded whole into a customer agent's context window to teach it what it may author.check:skill-refscould not see it: that gate compares the artifact against the generator, and the generator ranked faithfully.Before / after — all three rows, verbatim
skills/objectstack-automation/references/_index.mdskills/objectstack-i18n/references/_index.mdskills/objectstack-platform/references/_index.mdApproverTypestays first on the headline row — the property the adjudication turned on, and the reason Schema-first was rejected.The cap of five is applied after filtering, not before. Slicing first would let a module's constants consume the row's five slots and then be deleted from it, shortening the hazardous rows instead of promoting the authorable names behind them (translation would publish four names where five were available).
Acceptance scan, with a positive control shown able to fire
The scan is not a throwaway grep — it is the corpus gate at the end of
packages/spec/scripts/export-list.test.ts, so it ships and cannot rot. It enumerates everyExports:row in the checked-inskills/**artifacts and fails on anySCREAMING_SNAKEname.Positive control, run pre-fix (new test file against the unmodified generator and unmodified artifacts) — it fires, naming all 7 offenders:
(The card named two constants on the approval row; the row actually carried four — four of its five slots were unusable by an author.)
After the fix, the same gate is green, and
grep -rn '— Exports:' skills/returns 14 rows, none naming a machine constant.Token delta — two readings, because the ratchet cannot see this change
node scripts/check-skills-token-ratchet.mjs→✓ check-skills-token-ratchet: 11 published SKILL.md within their ceilings.withbundle total 117851 / 117857 (-6)— identical before and after this diff, because that gate's population isskills/<name>/SKILL.mdonly and this change touches noSKILL.md. Reporting only that number would be reporting a metric blind to the change.Whole-tree
skills/**, same convention (ceil(utf8 bytes / 4)), 49 files both sides:Per changed file (lines unchanged at 44 / 35 / 62):
objectstack-automation/references/_index.mdobjectstack-i18n/references/_index.mdobjectstack-platform/references/_index.mdFiltering shrinks the catalog, as expected — no expansion budget is consumed.
Test, and its reverse verification
packages/spec/scripts/export-list.test.tsextends the pattern #12202 used: that PR carried no test of its own — it inherited coverage by importing the sharedfindModuleDocBlock(), whose pin isfile-description.test.ts, which asserts on an extracted pure module because the generator runsmain()on import. The same move is made here: the ranking rule moves topackages/spec/scripts/lib/export-list.tsand is pinned without running the generator.Six unit cases plus a two-case corpus gate; 8 passed at the final commit. The two halves fail differently, which is why both exist:
MACHINE_CONSTANTtest fromexportListDescription, everything else kept: 4 of the 6 unit cases go red (the three about which names survive, plus the fall-through case, whosenullexists only because filtering can empty a list), while the corpus gate stays green — it reads checked-in artifacts, which only move when someone regenerates. What reddens the corpus gate is regenerating with the rule dropped, i.e. the pre-fix measurement above.dist/is involved and none was needed: the test imports./lib/export-listby relative specifier inside the same package'sscripts/directory, so vitest resolves the TypeScript source — noexportsmap, no build step between the edit and the reading.grep -c1 → 0, unguarded-push 0 → 1,git diff --stat1 insertion(+), 1 deletion(-). The script carriedtrap '<restore>' EXIT INT TERM; restoration was confirmed byte-identical (git diff HEAD -- <lib>empty) before the amend.Three-layer input declaration (found by the gates, not by me)
The corpus gate reads outside its own package, so
check:cross-package-test-inputsreddened — correctly. Repaired by declaring the input, never by moving the fixture, across all three layers the mechanism has:scripts/cross-package-test-inputs.mjs—'skills/**'on@objectstack/specturbo.json— the mirrored$TURBO_ROOT$/skills/**input on@objectstack/spec#test.github/workflows/ci.yml— the same glob verbatim in thecrosspkg:filter, whichcheck-ci-filter-paritythen required (layer 3 is the scheduler, and neither of the other two can see it)The subtree rather than
skills/*/references/_index.md: the test reads the directory too, and a file-only glob does not cover a directory listing (coversDirectoryis the check). The narrower spelling was tried first and rejected by the gate.Gates
Derived with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, re-derived after the edits — the second derivation added five families (check:bash32-floor,check:cli-command-ids,check:entry-guard,check:parse-guard,check:pnpm-filter-targets) once the diff grewscripts/**, and those were run too. All run at final commit74667b86d, exit codes captured before any pipe.pnpm --filter @objectstack/spec run check:skill-refs→✅ 9 generated files in sync with packages/specpnpm --filter @objectstack/spec test scripts/export-list.test.ts→Test Files 1 passed (1) · Tests 8 passed (8)pnpm check:cross-package-test-inputs→OK: 18 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob.node scripts/check-ci-filter-parity.mjs→ exit 0 (was the layer-3 failure above)node scripts/check-skills-token-ratchet.mjs→✓ check-skills-token-ratchet: 11 published SKILL.md within their ceilings.pnpm check:nul-bytes→✓ check-nul-bytes --self-test: 75 assertions over a temp git repo (real scan() path)pnpm --filter @objectstack/spec typecheck→check:test-typecheck: OK — @objectstack/spec's test layer compiles under packages/spec/tsconfig.test.jsonpnpm check:doc-authoring→✓ doc authoring guard: 48 published skill files clean — no internal issue-id references.pnpm check:skill-frame-sync→✓ check-skill-frame-sync: 4 copies of the decision frame are structurally isomorphic across 3 filespnpm check:type-check-coverage→✓ check:type-check-coverage --self-test — 47 semantic case(s) + 65 observation case(s) + 29 re-measure case(s) + 28 built-closure case(s) + 19 auto-lowering case(s) hold.pnpm check:where-matcher→✓ where-matcher conformance holds: 302 matcher(s) discovered, 302 answer the combinator battery correctly or refuse it loudly (189 refuse).pnpm --filter @objectstack/lint run check:doc-formula-expressions→ exit 0, after building its@objectstack/formula+@objectstack/lintclosure (it refuses outright on an unbuilt tree and says so)check:agent-test-spelling,check:role-word,check:pm-governed-merges,check:skill-compatibility,check:changeset-gate-self-tests,check:objectui-changeset,check:merge-driver,check:published-files,check:page-declaration-shape,check:slot-lookup,check:test-source-alias,check:type-source-resolution,check:engine-double-contract,check:query-options-erasure,check:bash32-floor,check:cli-command-ids,check:entry-guard,check:parse-guard,check:pnpm-filter-targets,check:empty-state,check:liveness,check:strictness-ledger,check:variant-docs,check-adr-0087-registration,check-changeset-no-major,check-empty-changeset,check-plugin-teardown-shape,release-rehearsal-clone --self-test,docs-audit/check-affected-docs,docs-audit/check-drift-comment.Declared narrowing — two families not run:
node scripts/check-dev-prereqs.mjsexits 1 in this fresh worktree because 67 of 67 workspace packages have nodist/on disk. Its own remedy ispnpm build— a whole-workspace build CI owns. It measured nothing about this diff; the count being all packages is what shows it is an environment fact, not a consequence of this change.pnpm check:type-check-debt, the convention-triggered ratchet for adding a test file, refuses on an unbuilt workspace closure for the same reason. Its spec-level equivalent,check:test-typecheck, ran green above and is the half that judges this diff's new test file; the repo-wide--re-measureis CI's.Repo-wide
pnpm lintis CI's run, not narrowed here — no claim is made about it either way.Merge regime — governed, measured not assumed
PR #12274 (the #11705 generated-artifact exemption) is still open, not merged, so the pre-exemption regime applies. Measured rather than asserted:
Worth noting for whoever reviews after #12274 lands: this PR would stay governed even then. Its own table puts a diff that edits the generator alongside its artifacts under the #11084 co-edit fence — "the diff edits the instrument that would certify it" — and that is exactly this diff.
Draft regardless; PM reviews. No seat flips it ready, enqueues it, or arms auto-merge.
Changeset present (
@objectstack/spec: patch), matching what #12202 did on its merged diff — this is a user-visible change to published artifacts, soskip-changesetdoes not apply.Generated by Claude Code
Generated by Claude Code