Repository navigation
fix(spec): the pure-annotation build plugin marks real calls only, so the published text equals its source (#20686) - #20992
Conversation
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>
Claude-Session: https://claude.ai/code/session_018fxqvRJW12TaHC7DUQ89Y6 Co-authored-by: Claude <noreply@anthropic.com>
…re-plugin-skips-strings
📓 Docs Drift CheckThis PR changes 1 package(s): ⛔ 1 release-owned page(s) name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 137 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 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
|
|
Queue ejection triage: PR #20992 (card #20686), seat 1 of Signature.
Initial judgment: a runner-level stall, not this PR. Three readings, all from the API:
Disposition: re-queued once. The head is unchanged at |
Fixes #20686
Clause-②: no
The
pure-schema-constructionbuild plugin (packages/spec/tsup.config.ts) now marks real calls oflazySchema/strictObject/defineFormonly, and never touches text. The published@objectstack/spec/migrationsbundle 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. EveryCallExpressionwhose 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.Plugintype is now read off tsup'sOptions['esbuildPlugins']. This package has no directesbuilddependency, so a bare'esbuild'type import resolves for no type checker.ts.resolveModuleNamefrom this file answers UNRESOLVED. The new test puts this file in front oftsconfig.scripts.json.pureSchemaConstructionandmainConfigare named exports, for the test. tsup reads only the default export.packages/spec/scripts/pure-schema-construction.test.ts, in thelocalproject (6 cases). It adds no gate..changeset/20686-pure-plugin-skips-strings.md:@objectstack/specpatch.Why a parse and not a bare
ts.createScannerpassWhether
/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 overpackages/spec/srcat 3693a1b (1774 files, test files excluded): acreateScannerloop 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 thedefineFormJSDoc@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 ofsrctook 1.2 s.Annotation census
Source level (insertion positions over
packages/spec/srcnon-test files, old rule vs new):The old rule's extra or misplaced insertions, all in non-call positions:
migrations/registry.ts:8608: inside a string (theacceptanceCriteriaofdashboard-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 thez.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/.mjsbundles, base plugin vs fixed plugin, same source tree):migrations/index.js/.mjsz.strictObject(DashboardWidgetSchema.shape)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
constinitialised fromz.+ PURE +strictObject({…})is kept, and its PURE annotation is gone from the output. An unusedconstinitialised from PURE +z.strictObject({…})is dropped. So the old rule's output atturso.zod.ts:469marked 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,zodexternal. Sizes are bytes against the before tree, the after tree, and the before tree with every annotation stripped (what the plugin buys):FieldSchemafrom./dataTursoConfigSchemafrom./datadefineStackfrom the rootMIGRATION_SUPPORT_FLOORfrom./migrationsTree-shaking is unchanged. The −16 bytes on
./migrationsis 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 inlint.yml. turbo's@objectstack/spec#testdepends on^buildonly, and spec has no workspace dependency. So in CI,distis 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 ownbuild()with the exportedmainConfig(the plugin, target and treeshake included) oversrc/migrations/index.ts. It writes into a temp directory under the package'snode_modules/.cache(so the bundle'szodimport resolves), imports the bundle and compares:MIGRATIONS_BY_MAJOR,MIGRATION_MAJORS,MIGRATION_SUPPORT_FLOOR,RETIRED_KEYS_BY_MAJORandRETIRED_DEFS_BY_MAJORmust 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.build(). Each string survives byte-identical, and the real calls carry the annotation at the call start.annotatePureCallscases 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.mjsreplaced the plugin's call toannotatePureCallswith the old line-prefix rule. The anchor went from x1 to x0, and the blob from7adc1d65404dtoe73d50e2a72e. The run, under the verify lock:MIGRATIONS_BY_MAJORdiffers from the source at entrydashboard-widget-stage-order-non-funnel-refused, fieldacceptanceCriteria: the live entry.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.annotatePureCallscases 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, andgit diff HEADis empty. Nodistis involved. The test imports the plugin from source and builds in-test, so there was no rebuild leg.Verification (at
7272847e14: the branch merged withorigin/main5f6b63a6fd)pnpm --filter @objectstack/spec build: green.disthas 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 --listFilesincludes the new test andtsup.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-loadsneeds thedistof every workspace package (86 named). Spec's own half was measured directly: all 24requireexport conditions of@objectstack/specload.check:type-check-debtneeds 30 workspace dependencies built.@objectstack/specis graduated from that ledger, and its owntypecheckis green above.check:lean-entry-closureran against an objectql closure rebuilt at this head.check:query-options-erasureran in 307 s.origin/mainmoved 4 commits (to7fa67dada3). None touchespackages/spec. The derivation files that changed (scripts/pm/ensure-pm-labels.shand twoscripts/pm/fleet-write/modules) are unrelated to these paths.Acceptance notes
reason(printed aswhy:). It is the entry'sacceptanceCriteria, whichos migrate metaprints as theverify:line (packages/cli/src/commands/migrate/meta.ts:380). The premise holds. Only the field name differs.src/**/*.test.tsfiles. No entry's graph reaches a test file, so none of them shipped.Generated by Claude Code