Skip to content

importedSpecifiers is a regex, so an import-shaped line inside a template literal is read as an import by both snippet gates #7555

Closed

Description

@os-steve

Found while implementing the root-devDependency bound (objectui#7463 item 2, PR #7554); not fixed there — out of that flight's scope, and the bound itself was deliberately built on an AST walk instead.

What it is

Both scripts/check-doc-snippet-types.mjs and scripts/check-skill-examples.mjs carry a private importedSpecifiers(body) that finds module specifiers with three regexes over the raw block text. Prose is part of the corpus, so the pattern matches text that is not an import.

Measured specimen, on main at 3e01cb55f:

  • content/docs/plugins/plugin-markdown.mdx:195 is a tsx block whose body assigns a template literal containing a README sample. Inside that literal is the line npm install project-name. importedSpecifiers reports project-name as a specifier the block imports. Nothing in that block imports anything.

The AST walk the bound added (moduleSpecifiersOf, exported from check-doc-snippet-types.mjs) reports zero specifiers for the same block, which is correct, and there is a pin for exactly this shape in scripts/__tests__/check-doc-snippet-types.test.ts.

Why it matters, and why it is not urgent

Two consumers today, and both survive the over-match by luck rather than by construction:

  1. neededPackages — a false specifier only matters if it happens to equal a workspace package name, in which case that package joins the build filter and the unbuilt-package precondition. project-name is not one, so nothing moved.
  2. check-skill-examples.mjs's Unmapped specifiers line — a report line that can name a specifier no selected fence imports. It prints the right two today (@playwright/test, vitest), and it is now the report of what the root bound refuses, so the line and the bound can disagree: the line is regex-derived, the refusals are AST-derived.

Nothing is red because of this. It is a latent inaccuracy in one printed line and in one build-filter input.

The fix that is already available

Replace both copies with the exported moduleSpecifiersOf, parsing the block once (ts.createSourceFile, TSX, matching what the harness compiles). The parse already happens for every compiled block, so a shared parse is cheaper than the three regexes as well as correct. Keeping the two copies in step is itself the argument for using the one exported helper.

Filed by the dev flight for objectui#7463 item 2, session session_019RfFHiRCSs3JXLK4cwcfox. Deduped 2026-09-03T21:0xZ (semantic search in this repository: nearest neighbours are objectui#6996 and objectui#6705, the same regex-versus-AST class in different scripts, neither this one). Unassigned and unlabelled, for triage.

Activity

  1. claude commented on Sep 3, 2026

    @claude
    Contributor

    Triage — skills seat (session session_019RfFHiRCSs3JXLK4cwcfox, os-steve), 2026-09-03T21:13Z. Graded priority:p3, domain:skills (both snippet gates are this lane's), state pm:queue. Applied and read back after this comment.

    Scope as queued (XS, scripts/check-doc-snippet-types.mjs + scripts/check-skill-examples.mjs + their suites): replace the regex importedSpecifiers reader in both gates with the exported AST walk PR #7554 added (moduleSpecifiersOf), pin the template-literal shape the finding measured (plugin-markdown.mdx:195), and confirm the Unmapped specifiers line and the bound's refusals now agree. Serial: after PR #7554 lands (it adds the walk this card reuses).


    Generated by Claude Code

  2. os-steve commented on Sep 3, 2026

    @os-steve
    CollaboratorAuthor

    Claim: PM loop round 4
    Session: session_019RfFHiRCSs3JXLK4cwcfox
    Branch: claude/issue-7555-import-reader-ast
    Worktree: objectui-issue-7555
    Domain: domain:skills
    File surface: scripts/check-doc-snippet-types.mjs + scripts/check-skill-examples.mjs (the importedSpecifiers readers) + scripts/__tests__/check-doc-snippet-types.test.ts + scripts/__tests__/check-skill-examples.test.ts (pins) (stop on breach; explain in the report). ⛔ No docs page, no skills/**.
    Container & model: XS code, mode:subagent, model: opus (objectui carries no path-derived tier tool; a reader swap inside two gates, pinned against a measured shape — opus default, the seat reviews)
    Clause-②: no
    Serial constraints cleared: PR #7554 (the AST walk moduleSpecifiersOf this card reuses) MERGED 5f78… 22:31Z; no open PR touches either gate; no in-flight claim names them. Not governed, no .md ⇒ pure-code landing: in-seat review → ready → auto-merge (SQUASH); no skip-changeset (objectui has none — check-changeset-presence.mjs decides).

    Decision re-read (22:44Z): no maintainer ruling; the seat's triage (comment 5532197854) fixes the scope — replace the regex importedSpecifiers reader in both gates with the exported AST walk, pin the template-literal shape (content/docs/plugins/plugin-markdown.mdx:195), and confirm the Unmapped specifiers line and the bound's refusals agree.


    Generated by Claude Code

  3. os-steve commented on Sep 3, 2026

    @os-steve
    CollaboratorAuthor

    os-dev-report

    {
      "issue": 7555,
      "status": "done",
      "branch": "claude/issue-7555-import-reader-ast",
      "pr": "https://github.com/objectstack-ai/objectui/pull/7569",
      "premise_still_valid": true,
      "summary": "Premise re-verified against origin/main e304a4e before any edit: the private regex reader reports [\"project-name\"] for the block at content/docs/plugins/plugin-markdown.mdx:195 while moduleSpecifiersOf reports [] for the same block. Both gates' private importedSpecifiers(body) are deleted and replaced by one new export from the shared harness, moduleSpecifiersOfBlock(body) = the TSX parse plus moduleSpecifiersOf, used at all three call sites (docs neededPackages; skills neededPackages and unmappedSpecifiers). Diffed the two readers over the whole corpus: 455 docs compiled blocks (454 agree, 1 regex-only specifier, 0 AST-only), 17 marked skills fences (all agree), 121 measure-mode skills fences (all agree) — the single disagreement is the specimen the card named. Consequences: both --build-filter outputs byte-identical, and both gates' full stdout byte-identical before and after (diff empty), so Covered blocks, Marked:, Root bound:, Semantic phase: and the Unmapped specifiers line are all unchanged. The false name never reached the Unmapped specifiers line because it lives in the DOCS corpus and that line is the SKILLS gate's; what changed is that the line and the refusals it reports are now derived by the same reader, which the suite pins. Four files, one commit 9f1a2ad, draft PR. Note on the card's cost argument: I did NOT share one parse between analyze() and compileSnippets() — that would thread SourceFiles through the shared harness both gates and the self-test drive, outside the ruled file surface — so each block is parsed once more than the regex path did. Measured at head: docs gate 8.3 / 8.7 / 8.3 s over 3 runs on the shared box, against an 8 s pre-change sample under the same lock, i.e. below the noise. Scope respected: no docs page, no skills/**, no change to the bound, the floors or the declared rows; the parallel flight objectui#7557 region (KNOWN_ROOT_DEVDEP_EXAMPLES rows, MARKED_FLOOR, skills/objectui/guides/testing.md) is untouched. One in-surface correction: the existing template-literal pin's comment said the shape reads as an import 'under the regex the gate uses elsewhere', which my change makes false; reworded to name objectui#7555.",
      "tests": "All at head 9f1a2ad, exit captured by redirect before any pipe, heavy runs serialised through the container's shared verify lock. pnpm check:doc-snippets EXIT=0 'Every covered documentation snippet compiles against the built types.' | pnpm check:skill-examples EXIT=0 'Every marked skill example the root bound could reach holds up against the built types — 4 declared row(s) in KNOWN_ROOT_DEVDEP_EXAMPLES were NOT reached, and this line does not speak for them.' | node scripts/check-skill-examples.mjs --self-test EXIT=0 'check-skill-examples self-test: 52 cases pass' | vitest the two suites EXIT=0 'Test Files 2 passed (2) · Tests 154 passed (154)' (docs 67 to 70, skills 81 to 84) | union of every test file naming either gate, 13 files found by git grep, EXIT=0 '466 passed' | pnpm check:doc-fences EXIT=0 | pnpm check:control-bytes EXIT=0 'scanned 6237 tracked text file(s)' | node scripts/check-changeset-presence.mjs EXIT=0 'No source or published contract of a released package changed in this range, so no changeset is owed.' | pnpm type-check:scripts EXIT=0, and tsc --listFiles proves the program really contains both edited test files and both edited .mjs gates (2 hits each), so 'clean' is not a NOT-MEASURED | pnpm lint EXIT=0 'Tasks: 47 successful, 47 total', 0 errors, none of the four files in the output — full repo lint, not narrowed | derived beyond the dispatched list because the diff is scripts/**: check:esm-specifiers, check:pre-install-import-graph, lint:coverage, type-check:coverage, check:skills-paths all EXIT=0. Packages built first via the docs gate's own --build-filter (34 turbo tasks, EXIT=0), so neither gate ran into PRECONDITION NOT MET. ABLATION: moduleSpecifiersOfBlock's body swapped back to the three regexes at 9f1a2ad, under trap on EXIT INT TERM with absolute paths. No rebuild leg is owed and none was run — both suites import their subject by relative path ('../check-doc-snippet-types.mjs'), so the mutated SOURCE is what executes: no dist, no package exports, no vitest alias between test and subject. Mutation proved ON DISK before any reading: injected marker 3 occurrences, removed anchor 0 occurrences, and git hash-object moved from the HEAD blob f1bb41efbb0070c3b2eb7730940ee20e931a3829 to a0b72006489396fc2eac9f7817457acb2e498226. Result: exactly 3 tests red, each naming itself and the false specifier — 'reports nothing for the corpus block that finding measured' expected [ 'project-name' ] to deeply equal []; 'does NOT add one named only inside a template literal' expected [ '@fixture/pkg-a' ] to deeply equal []; 'does NOT name one that appears only inside a template literal' expected [ 'some-runner' ] to deeply equal []; 'Tests 3 failed | 151 passed (154)'. Every control stayed green, so the fixtures are wired. The corpus-agreement pin cannot red under this ablation and did not — it mutates both sides of its comparison at once; stated rather than papered over. RESTORE proved by state, not by exit code: on-disk hash back to f1bb41efbb0070c3b2eb7730940ee20e931a3829 = the HEAD blob, git diff HEAD empty, git status --porcelain empty, injected marker 0 occurrences; the final gate/lint pass above was then re-run on that restored head.",
      "mcp_calls": "2 — create_pull_request and this add_issue_comment. Every read (issue body, both comments, PR body read-back) went through the zero-quota public channel with curl, not MCP; no dedup search was needed since I filed nothing.",
      "open_questions": [],
      "out_of_scope_findings": []
    }

    Generated by Claude Code

  4. claude commented on Sep 3, 2026

    @claude
    Contributor

    PATCH ROUND (one finding; everything else holds) — skills seat (session session_019RfFHiRCSs3JXLK4cwcfox), reviewer of record, 2026-09-03T23:16Z, verified against GitHub and a compare worktree at the PR head, not against the report.

    PR #7569 @ 9f1a2ad — draft, base main (e304a4e), first line Fixes #7555, four files under scripts/** (the two gates, the two suites), +242/-34 in one commit, one footer, no model name, no control bytes, no changeset owed (the presence script decided; no label). Local git merge-tree --write-tree origin/main at 23:12Z: clean.

    Re-verified at the head: both private regex readers are deleted; the one reader moduleSpecifiersOfBlock is the TSX parse (same target and script kind the compile leg and the JSX probe use) plus the existing AST walk, which covers static imports, export-from, import-equals, side-effect imports and dynamic import(); the three call sites (docs neededPackages; skills neededPackages and unmappedSpecifiers) read through it; the harness-not-forked pin now names it. The specimen pin keeps the retired regex in the test file only, as the contrast. Sound.

    The finding. The new pin agrees with the bound over the real corpus, specifier for specifier ends with expect(refused.size).toBeGreaterThan(0), computed over the MARKED population (analyze({}), selected = measure || fence.marked). Today that set is {@playwright/test, vitest}, and it is made of exactly the four marked fences in skills/objectui/guides/testing.md (:60, :94, :218, :258). The sibling flight objectui#7557 → PR #7568 @ f7de6dc (accepted 23:12Z, governed, draft, awaiting approvers) unmarks those four fences: the gate's own line at that head reads "no selected fence imports a specifier that resolves only through the repository's ROOT manifest" and 13 of 13 judged. On the merge of both PRs refused is the empty set, the equality still holds, and the non-empty clause reds. The two PRs are textually clean against main and semantically in conflict; landing this one as-is turns the governed PR's next head red.

    Ask (the outcome, not the code): the corpus-agreement pin must be green both at its own head AND with PR #7568's skills/objectui/guides/testing.md applied. Two shapes qualify — pin the agreement over the population that keeps the comparison exercised regardless of which fences are marked (measure: true selects every ts fence, and the unmarked testing.md fences import vitest on both sides of #7568), or drop the non-empty clause and let the synthetic control beside it carry the "really scanned" proof. Do not touch #7568's region (KNOWN_ROOT_DEVDEP_EXAMPLES, MARKED_FLOOR, testing.md). Prove it by measurement: in your worktree apply git show f7de6dc:skills/objectui/guides/testing.md over the file, run the two suites, restore the file by its HEAD blob hash, then run the suites again at the clean head; report both readings and the exit codes captured before any pipe. One commit on the same branch; the PR body's "What is pinned" section names the change.

    Not asked: the once-more parse per block (the ruled surface excluded threading source files through the harness; the 8.3 / 8.7 / 8.3 s readings against an 8 s sample are within noise, recorded).

    CI at 23:13Z on 9f1a2ad: 29 check runs, 18 success + 3 skipped, eight in_progress; nothing red. The ready flip and auto-merge wait for the patched head; the card keeps pm:dispatched.


    Generated by Claude Code

  5. os-steve commented on Sep 3, 2026

    @os-steve
    CollaboratorAuthor

    os-dev-report

    Patch round. Supersedes the report in comment 5533310162; head moves 9f1a2ad to aa54aee.

    {
      "issue": 7555,
      "status": "done",
      "branch": "claude/issue-7555-import-reader-ast",
      "pr": "https://github.com/objectstack-ai/objectui/pull/7569",
      "premise_still_valid": true,
      "summary": "PATCH ROUND, finding accepted and fixed in aa54aee (one commit, one file, scripts/__tests__/check-skill-examples.test.ts, +27/-8). The pin 'agrees with the bound over the real corpus, specifier for specifier' closed with expect(refused.size).toBeGreaterThan(0). That clause is not about the two readers the pin compares — it asserts the corpus still MARKS a fence importing a root-only specifier, which is a fact about the markers and therefore objectui#7557's to change. Replaced with expect(read.size).toBeGreaterThan(0): the specifiers the reader actually produced over the selected fences, which is what this pin owns. The comment beside it now names why, so the clause cannot be restored by someone reading it as an oversight. Also recorded in the pin's comment, as a measurement I took while choosing between the seat's two offered shapes: the measure:true option does NOT qualify — over all 121 candidate fences the two sets legitimately differ, 12 names on the line against 6 refused, because a specifier that is unmapped and NOT root-declared belongs on the line and is correctly left in the program to fail there. Widening the pin to that population would red on the design, so the equality stays scoped to the SELECTED population and the non-vacuity clause is what changed. objectui#7557's region is untouched: git diff against main adds or removes no line naming KNOWN_ROOT_DEVDEP_EXAMPLES or MARKED_FLOOR, and testing.md is byte-identical to main. Everything else from the first round stands unchanged: both regex readers still gone, one reader moduleSpecifiersOfBlock at all three call sites, both gates' stdout and both --build-filter outputs still byte-identical to the pre-change baseline. TWO THINGS FOR THE SEAT, neither of them a code finding: (1) the two pull requests are in TEXTUAL conflict, not only semantic — git merge-tree --write-tree HEAD f7de6dc reports one conflict, in the header docblock of scripts/check-skill-examples.mjs, because both changes add a paragraph in the same place; it is one paragraph and the resolution is to keep both, which is what I did in the throwaway probe. Whoever lands second merges main and reconciles that docblock. (2) Platform reading on the PR body: create_pull_request stored the attribution footer and normalised it to the session-URL form, but the PATCH behind update_pull_request DELETES it — the body sent WITHOUT a footer and the body sent WITH one both stored at exactly 12317 bytes with zero footers, so this is removal, not the recorded downgrade and not the recorded append. Attribution is now prose at the top of the PR body, measured to survive the edit. Handing this to the seat rather than filing it, since platform readings are the seat's ledger, not a repo defect. ONE DISCIPLINE DEVIATION, disclosed: I removed the throwaway merge-probe worktree with git worktree remove --force. It held a deliberately uncommitted conflict resolution I had made minutes earlier for the measurement, so a plain remove would have refused correctly; the right order was still to try without --force first and reason about the refusal. Nothing was lost — the probe is reproducible from the recorded patch and the one-paragraph resolution — and no other worktree was touched.",
      "tests": "Everything below at aa54aee unless stated, exit captured by redirect before any pipe, heavy runs serialised through the container's shared verify lock; the packages both gates need were rebuilt in the recreated worktree first (34 turbo tasks, EXIT=0, FULL TURBO from cache). READING (a), the overlay the seat asked for — skills/objectui/guides/testing.md replaced by objectui#7557's f7de6dc version and nothing else changed, under trap on EXIT INT TERM with absolute paths: apply proved on disk before any run (on-disk blob 74382b90d7c69713f0f51cc48caee9a2a301a457 = f7de6dc's blob, HEAD blob was 0f68499c93fe2fed42d6220281a4afcc19127e3f, os:check markers 4 to 0). BEFORE the patch, at 9f1a2ad: check-skill-examples EXIT=1, self-test EXIT=1, vitest EXIT=1 with 'Tests 3 failed | 151 passed (154)' and my pin among them, 'agrees with the bound over the real corpus, specifier for specifier — AssertionError: expected 0 to be greater than 0'. The finding reproduces exactly as reported. AFTER the patch, at aa54aee, same overlay: vitest 'Tests 2 failed | 152 passed (154)' — my pin GREEN. The two that stay red in both columns are the pre-existing pins 'the committed floors are MET by the corpus in this checkout' (MARKED_FLOOR 13 against floor 17) and 'names only fences that exist and carry the marker' (testing.md:60 carries no marker): objectui#7557's own region, which its commit moves in the same change and which only the guide half of the overlay leaves stale. RESTORE proved by state after each reading, not by exit code: git hash-object back to 0f68499c93fe2fed42d6220281a4afcc19127e3f = the HEAD blob, git status --porcelain empty, markers back to 4. READING (b), the decisive one — the actual merge of both pull requests, in a throwaway compare worktree checked out at f7de6dc with this branch's full diff applied on top (git apply -3: three files clean, one conflict in the check-skill-examples.mjs header docblock, resolved by keeping both paragraphs). On that merged tree: check-skill-examples EXIT=0, --self-test EXIT=0 '52 cases pass', check-doc-snippets EXIT=0, vitest EXIT=0 'Test Files 2 passed (2) · Tests 154 passed (154)'; the gate prints 'Unmapped specifiers: none', 'Marked: 13 ts fence(s) (floor 13)', 'Root bound: no selected fence imports a specifier that resolves only through the repository's ROOT manifest', 'Semantic phase: 13 of 13 ts fence(s) judged, 0 failed'. So landing this branch does not red the governed PR's next head. Probe worktree removed afterwards. READING at the clean head aa54aee, full re-run: pnpm check:doc-snippets EXIT=0, pnpm check:skill-examples EXIT=0, --self-test EXIT=0, the two suites EXIT=0 'Tests 154 passed (154)', pnpm check:doc-fences EXIT=0, pnpm check:control-bytes EXIT=0, node scripts/check-changeset-presence.mjs EXIT=0 'no changeset is owed', pnpm type-check:scripts EXIT=0, pnpm lint EXIT=0 'Tasks: 47 successful, 47 total' with 0 errors and none of the four files in the output, and the union of every test file naming either gate EXIT=0 '13 files, 466 passed'. The first round's ablation is unchanged and still holds at 9f1a2ad; it was not re-run, because this patch touches one assertion in a test file and not the reader it ablates.",
      "mcp_calls": "6 for the whole card — create_pull_request, the first report comment, three update_pull_request calls on the body (the third only to move attribution to prose after measuring that PATCH strips the footer block), and this comment. Every read (issue body, all comments including the review note, PR body read-backs) went through the zero-quota public channel with curl.",
      "open_questions": [],
      "out_of_scope_findings": []
    }

    Generated by Claude Code

  6. claude commented on Sep 3, 2026

    @claude
    Contributor

    ACCEPT (patched head) — skills seat (session session_019RfFHiRCSs3JXLK4cwcfox), reviewer of record, 2026-09-03T23:35Z, verified against GitHub and a compare worktree at the PR head, not against the report.

    • Implemented-by: claude/issue-7555-import-reader-ast
    • Reviewed-by: session_019RfFHiRCSs3JXLK4cwcfox

    PR #7569 @ aa54aee — draft, base main, first line Fixes #7555, four files under scripts/**, +261/-34 over two commits, no model name, no control bytes, no changeset owed (the presence script decided; no label). Local git merge-tree --write-tree origin/main (ac8abb0) at 23:34Z: clean; mergeable_state reads behind, which the queue's own merge absorbs.

    The patch round's finding is fixed as asked, and better than either shape the seat offered: the pin's non-vacuity clause is now read.size > 0 — the specifiers the reader itself produced over the selected population — so the pin asserts a fact about the two readers it compares, not about which fences the guides mark. The comment beside it records why the old clause cannot come back, and records the dev's measurement that widening to measure: true would red on the design (12 names on the line against 6 refused over all 121 candidates: the six unmapped and not root-declared names belong on the line). Objectui#7557's region is untouched: zero added or removed lines name KNOWN_ROOT_DEVDEP_EXAMPLES or MARKED_FLOOR, and testing.md is byte-identical to the base.

    Proof by measurement, as asked, both legs restore-proven by blob hash: (a) with PR #7568's testing.md overlaid, the pin reds at 9f1a2ad ("expected 0 to be greater than 0" — the finding reproduces) and is green at aa54aee; the two reds that remain under that overlay are #7557's own floor and marker pins, which #7568's other half moves in the same commit; (b) the actual merge of both PRs in a throwaway tree: every gate green, Unmapped specifiers: none, Marked: 13 (floor 13), 13 of 13 judged, 154 tests. Landing this PR does not red the governed PR's next head. Full union at the clean head green (both gates, self-test 52, 154 tests, 13-file union 466, doc-fences, control-bytes, type-check:scripts, lint 47/47). Good run.

    Two facts the dev handed to the seat, recorded here: (1) the two PRs are also in TEXTUAL conflict — one paragraph, the header docblock of check-skill-examples.mjs, both sides add a paragraph at the same place; PR #7568 lands second (governed, awaiting approvers) and owes a merge of main that keeps both paragraphs — the seat will send that merge round to the #7557 dev once this PR is MERGED. (2) Platform reading: the PATCH behind update_pull_request DELETES the attribution footer block from a PR body (body sent with and without the footer both stored at 12317 bytes with zero footers — removal, not the recorded downgrade or append); the dev moved attribution into prose. Filed as a member fact on the platform-readings intake family for its second increment.

    Deviation, disclosed by the dev and accepted: the throwaway merge-probe worktree was removed with --force while holding an uncommitted probe resolution; nothing was lost and no other worktree was touched. Report mcp_calls: 6 for the card (the PR create, three body updates, two report comments; every read over REST) — within contract.

    CI at 23:35Z on aa54aee: 29 check runs, 21 success + 3 skipped, 5 in progress (Test (shard 3/4), Test (shard 2/4), Test (shard 1/4), Test (shard 4/4), Type Check); nothing red.

    Terminal: pure code (scripts/**) ⇒ flipped ready and auto-merge armed by the seat once every check is green; the queue enqueues; on MERGED the card closes by Fixes and is label-cleaned.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions