Skip to content

fix(spec): the pure-annotation build plugin marks real calls only, so the published text equals its source (#20686) - #20992

Merged
objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-20686-pure-plugin-skips-strings
Oct 1, 2026
Merged

objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-20686-pure-plugin-skips-strings

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #20686
Clause-②: no

The pure-schema-construction build plugin (packages/spec/tsup.config.ts) now marks real calls of lazySchema / strictObject / defineForm only, and never touches text. The published @objectstack/spec/migrations bundle carries the source's text again. The one affected D3 entry, 18.dashboard-widget-stage-order-non-funnel-refused, has no text change.

What changed

  • annotatePureCalls(source, fileName) replaces the line-prefix rule. The TypeScript parser drives the scanner. Every CallExpression whose callee is one of the three names gets the PURE annotation at the start of the call, whether the callee is bare (lazySchema(…)) or a property-access member (z.strictObject(…)). String and template text, comments, JSDoc and declarations are not call expressions, so they are never touched. A call inside a template substitution is code, and it is marked (that behaviour is decided here and pinned). The output is the source plus inserted annotations, and nothing else.
  • The esbuild Plugin type is now read off tsup's Options['esbuildPlugins']. This package has no direct esbuild dependency, so a bare 'esbuild' type import resolves for no type checker. ts.resolveModuleName from this file answers UNRESOLVED. The new test puts this file in front of tsconfig.scripts.json.
  • pureSchemaConstruction and mainConfig are named exports, for the test. tsup reads only the default export.
  • New test packages/spec/scripts/pure-schema-construction.test.ts, in the local project (6 cases). It adds no gate.
  • .changeset/20686-pure-plugin-skips-strings.md: @objectstack/spec patch.

Why a parse and not a bare ts.createScanner pass

Whether / starts a regex literal and whether } resumes a template are grammar questions. The scanner answers them only when the parser asks it to rescan. Measured over packages/spec/src at 3693a1b (1774 files, test files excluded): a createScanner loop without those rescans reports 1538 calls where the syntax tree has 1727. It loses 191 real calls, because every regex literal that contains a quote is mis-lexed. It also marks 2 mentions inside comments: one in the defineForm JSDoc @example (ui/view.zod.ts:7013) and one in a line comment (:7042). So the positions are read off the syntax tree that the parser builds with that same scanner. Parse cost: the new rule parses the 249 files that name a marked constructor (the old rule rewrote 212). The census that ran both rules over all of src took 1.2 s.

Annotation census

Source level (insertion positions over packages/spec/src non-test files, old rule vs new):

old rule new
insertions 1730 1727

The old rule's extra or misplaced insertions, all in non-call positions:

  • migrations/registry.ts:8608: inside a string (the acceptanceCriteria of dashboard-widget-stage-order-non-funnel-refused). This one shipped.
  • migrations/entries/semantic/18.dashboard-widget-stage-order-non-funnel-refused.ts:59: the same string in the entry source. No bundle imports the entry file. Only the generated registry copy is bundled.
  • ui/view.zod.ts:7032: export function defineForm(, a declaration. esbuild drops a comment there, so it has no effect on the bundles.
  • data/driver/turso.zod.ts:469: z.strictObject({. The annotation moved from after the z. to the start of the call. See the note below.

No new position exists beyond that move. Real calls with a type argument or a space before the paren: 0 in src. So including them changes nothing today.

Bundle level (tsup JS pass, all 48 .js / .mjs bundles, base plugin vs fixed plugin, same source tree):

before after
PURE annotations, all bundles 17202 17220 (+18)
migrations/index.js / .mjs 82 / 82 81 / 81 (the in-string one removed)
the 20 bundles that reach the Turso schema (10 entries, ESM and CJS) n n + 1 each (the moved member-call annotation)
the marker inside z.strictObject(DashboardWidgetSchema.shape) 2 0
bundles differing once every annotation is stripped 0 of 48

So the published code is byte-identical apart from annotations: −2 in the string and +20 at the one member call.

The member-call annotation (bounded in-place fix). esbuild 0.28.1 (the version tsup resolves) drops an annotation written after a member access's dot. Measured on an input carrying both spellings: an unused const initialised from z. + PURE + strictObject({…}) is kept, and its PURE annotation is gone from the output. An unused const initialised from PURE + z.strictObject({…}) is dropped. So the old rule's output at turso.zod.ts:469 marked nothing. The new rule puts the annotation where esbuild honours it, because it marks call expressions. It is the same defect class (the plugin wrote an annotation at a token that is not a call start). The fix is mechanical, pinned by the fixture test, in the file this claim holds, and adds no verification surface.

Tree-shake control

Measured with a consumer bundle per row: esbuild, bundle, minify, treeShaking, zod external. Sizes are bytes against the before tree, the after tree, and the before tree with every annotation stripped (what the plugin buys):

consumer imports before after no annotations
FieldSchema from ./data 231,683 231,683 551,411
TursoConfigSchema from ./data 173,044 173,044 551,411
defineStack from the root 954,689 954,689 968,107
MIGRATION_SUPPORT_FLOOR from ./migrations 1,548,424 1,548,408 1,548,521

Tree-shaking is unchanged. The −16 bytes on ./migrations is the 16-character marker that left the string.

The test, and how it reads the build

No spec test reads the real packages/spec/dist. Only gates do, and they run after the build step in lint.yml. turbo's @objectstack/spec#test depends on ^build only, and spec has no workspace dependency. So in CI, dist is either absent or built in parallel when the suite runs. A test that read it would be skipped or racy there. This test therefore runs the real build itself. It calls tsup's own build() with the exported mainConfig (the plugin, target and treeshake included) over src/migrations/index.ts. It writes into a temp directory under the package's node_modules/.cache (so the bundle's zod import resolves), imports the bundle and compares:

  • MIGRATIONS_BY_MAJOR, MIGRATION_MAJORS, MIGRATION_SUPPORT_FLOOR, RETIRED_KEYS_BY_MAJOR and RETIRED_DEFS_BY_MAJOR must equal the source values exactly. Markers are not stripped before comparing: stripped, the buggy build compares equal too. A control requires at least one annotation in that bundle, so the equality cannot hold vacuously for a build that ran no plugin.
  • A fixture quotes every marked name in a single-quoted and a double-quoted string, template text, a template substitution, a regex literal, JSDoc, and trailing block and line comments, next to a declaration and the three real call shapes. It is built through the same build(). Each string survives byte-identical, and the real calls carry the annotation at the call start.
  • Four annotatePureCalls cases pin positions directly. They cover the unparsed hand-back, untouched non-calls, every call shape, and that removing the insertions gives the source back.

Ablation

The fix was committed first (7189a7c485). node scripts/ablation-replace.mjs replaced the plugin's call to annotatePureCalls with the old line-prefix rule. The anchor went from x1 to x0, and the blob from 7adc1d65404d to e73d50e2a72e. The run, under the verify lock:

  • 2 failed | 4 passed. The built MIGRATIONS_BY_MAJOR differs from the source at entry dashboard-widget-stage-order-non-funnel-refused, field acceptanceCriteria: the live entry.
  • The fixture build fails with esbuild Unexpected "export". The old rule wrote an annotation inside a trailing block comment and closed it early. That is the comment-in-comment hazard the old docblock cites.
  • The four annotatePureCalls cases stay green, as expected: they call the function, not the plugin.

The restore was proven by the tool: blob after restore = blob at HEAD = 7adc1d65404d, and git diff HEAD is empty. No dist is involved. The test imports the plugin from source and builds in-test, so there was no rebuild leg.

Verification (at 7272847e14: the branch merged with origin/main 5f6b63a6fd)

  • pnpm --filter @objectstack/spec build: green. dist has 0 in-string markers and 17220 annotations.
  • vitest run scripts/pure-schema-construction.test.ts: 6 passed.
  • pnpm --filter @objectstack/spec test: 585 files passed, 17211 tests passed, 1 todo.
  • pnpm --filter @objectstack/spec typecheck: green. tsc -p tsconfig.scripts.json --listFiles includes the new test and tsup.config.ts.
  • dispatch-gates --commands --repo objectstack-ai/objectstack: 67 derived. --ran: 65 run, all exit 0, and 2 NOT MEASURED (exit 3, PREREQUISITE NOT MET):
    • check:dual-build-cjs-loads needs the dist of every workspace package (86 named). Spec's own half was measured directly: all 24 require export conditions of @objectstack/spec load.
    • check:type-check-debt needs 30 workspace dependencies built. @objectstack/spec is graduated from that ledger, and its own typecheck is green above.
    • Both are declared narrowings, and CI runs them.
  • check:lean-entry-closure ran against an objectql closure rebuilt at this head. check:query-options-erasure ran in 307 s.
  • After this head, origin/main moved 4 commits (to 7fa67dada3). None touches packages/spec. The derivation files that changed (scripts/pm/ensure-pm-labels.sh and two scripts/pm/fleet-write/ modules) are unrelated to these paths.

Acceptance notes

  • The card attributes the altered text to the entry's reason (printed as why:). It is the entry's acceptanceCriteria, which os migrate meta prints as the verify: line (packages/cli/src/commands/migrate/meta.ts:380). The premise holds. Only the field name differs.
  • The old rule also rewrote 56 positions in src/**/*.test.ts files. No entry's graph reaches a test file, so none of them shipped.

Generated by Claude Code

The build plugin's line-prefix rule rewrote a marked constructor name inside
a string on a code line, so the published ./migrations bundle carried a PURE
marker inside a D3 migration reason. The TypeScript parser now drives the
scanner and the annotation goes at the start of each real call expression.

Claude-Session: https://claude.ai/code/session_018fxqvRJW12TaHC7DUQ89Y6
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 1, 2026
@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/spec, touching 10 documentable anchor(s).

⛔ 1 release-owned page(s) name something this change touched. These are read-only:

  • content/docs/releases/v17/17-0.mdx (via defineForm (literal, a string literal in PURE_CONSTRUCTORS))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 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; 97 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 137 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 7fa67dada3a59fa185af0f09f89585c912908aa0 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 7fa67dada3a59fa185af0f09f89585c912908aa0

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

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 7fa67dada3a59fa185af0f09f89585c912908aa0 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 1, 2026 01:54
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 1, 2026 01:54
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 1, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 1, 2026
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Queue ejection triage: PR #20992 (card #20686), seat 1 of domain:spec. Recorded 2026-10-01T02:55Z.

Signature.

  • The merge-group run is 36803445860, on queue commit e6ee66e86e. That commit is main 2f2fa11d75 plus this PR's head 7272847e14.
  • Job Test Core (1/6) (110182684343) was cancelled at its wall: it started 01:56:33Z and was cancelled 02:51:33Z.
  • Step 11, "Run this shard's tests", never finished. The aggregate Test Core then failed at "Verify test shard results".
  • The job log's last line is at 02:08:03Z, inside @objectstack/rest:build warnings (empty-import-meta). After that there is no output for 43 minutes. There is no FAIL line and no assertion, and the stall-guard step never ran.
  • The run's conclusion is cancelled, so Merge Queue Triage skipped it by design (it fires on failure only). No triage comment exists, and this note is the record.

Initial judgment: a runner-level stall, not this PR. Three readings, all from the API:

  1. The queue base is green on the same shard. main 2f2fa11d75 ran Test Core (1/6) three times today, and each passed in 10–14 min.

  2. The first error is a timeout, not an assertion. No test reported a failure, and the output went silent mid-build.

  3. The failing-file disjointness test has nothing to bite on, because no file failed. The stronger reading is the positive control: seven later queue commits that CONTAIN this PR's change all passed the same shard. They are:

    • 724144794f (18 min)
    • 74ba79ffa0 (18 min)
    • 13bed63ee1 (13 min)
    • 9254ece2d0 (2 min)
    • 083374d9a0 (13 min)
    • 1ceb895dde (14 min)
    • e0e6d6ec33 (11 min)

    The PR's own head also passed Test Core (1/6) in 15 min.

Disposition: re-queued once. The head is unchanged at 7272847e14. A driverless merge-tree is clean onto main 2f2fa11d75 and onto the true queue tail 2a4254a608. If this signature (a Test Core shard silent mid-step to its wall, with no FAIL line) ejects this PR again, the seat stops and hands the card to the next seat for re-diagnosis. It does not re-queue a second time.

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 size/m tests tooling

Projects

None yet

2 participants