MySTRA becomes a stock-MyST plugin for ASTRA analyses

[EiffL]

Summary

Reworks MySTRA from a generate-everything content server into a single MyST plugin. Instead of generating and serving the whole document, an author writes a normal MyST Markdown report and imports/cites ASTRA components by reference; the plugin reads astra.yaml at build time and emits standard MyST AST, running on the stock myst CLI and themes — no custom server.

Net diff: +2,506 / −8,879 across 55 files — roughly half the codebase removed.

The governing principle: three single sources of truth

Concern	Single source
Data — what a decision/output/finding is	astra.yaml (+ universes/, results/)
Composition — what appears, where, in what order	the author's index.md
Presentation — how it looks	the MyST theme

The plugin is a pure projector: it renders the elements the author placed (neutral semantic AST any theme shows) and bakes a resolved data store for rich themes. It makes no authoring or styling decisions.

What changed

Removed (the generate-everything stack):

• src/server/ — the bespoke Express content server, routes, watcher, WebSocket
• src/cli.ts, src/theme/launcher.ts — the two-server boot CLI and theme launcher
• src/transform/index.ts — buildAllPages/astraToMystAST whole-document orchestration
• src/doi/ and src/papers/ — custom DOI resolver/cache + paper backlinks (delegated to MyST)
• src/types/astra.ts, src/types/content-server.ts, src/utils/hash.ts — hand-mirrored types + content cache
• Old test surface: page-shape, schema-coverage, server-routes

Kept / refactored into the plugin core:

• src/index.ts — the plugin is the package entry (default export)
• src/loader.ts — one-file project loader via @astra-spec/sdk, deterministic on-demand artifact resolution
• src/transform/* — the per-component renderers actually used by directives/roles
• src/transform/resolved-store.ts — bakes the resolved ASTRA model onto a hidden div.astra-store carrier (per page), keyed by id, for rich themes
• Data-model types now imported directly from @astra-spec/sdk — MySTRA defines none of its own
• New tests/plugin-emission.test.ts + tests/loader-validation.test.ts

Authoring vocabulary — block directives (astra:decision|output|finding|prior-insight|inputs|outputs|subanalysis), inline roles ({astra:decision} …), and live value interpolation ({astra:value} pulls real cells from result products at build time, so no measured number is hand-typed).

Docs & packaging (recent commits)

• README rewritten as a user-oriented MyST-plugin guide; published-package workflow (npm install @astra-spec/mystra, reference by name in myst.yml)
• Package renamed mystra → @astra-spec/mystra, version reset to 0.0.1
• Relicensed Apache-2.0 → BSD 3-Clause to match astra-spec (LICENSE, package.json, lockfile, README)
• SPEC.md rewritten to the implemented design; STRATEGY-A-REFACTOR.md records the full keep/refactor/remove rationale

Out of scope (follow-ups)

• The separate lightcone-astra rich theme (glyphs, hover cards, dependency graphs) — until it ships, book-theme is the clean baseline
• Author–year citations + linked reference list once a project bibliography is wired

Test plan

• npm run build clean (no dead imports after removals)
• npm test — plugin-emission, resolved-store, and parser tests green
• Reviewer: myst start on an ASTRA project renders cleanly on book-theme — figures load, numbers are live, no astra.yaml read by the server/theme

🤖 Generated with Claude Code

[EiffL]

@claude please review this PR carefully

[EiffL]

Code Review — PR #5: "MySTRA becomes a stock-MyST plugin for ASTRA analyses"

I reviewed the diff plus the resulting working tree, built it, and ran the test suite. Build is clean. The test suite is not — and that's the headline finding.

Overview

A large, well-executed architectural pivot: MySTRA stops being a generate-everything Express content server and becomes a single MyST plugin (directives + roles + transforms) that runs on the stock myst CLI. Net −6,100 LOC, roughly half the codebase. The "three sources of truth" framing (data in astra.yaml, composition in index.md, presentation in the theme) is clean and the plugin genuinely behaves as a pure projector: it emits stock mdast and bakes a resolved store onto a hidden carrier for rich themes. Documentation in the source is excellent — most modules explain why, not just what.

🔴 Blocking: the new test surface can't run in a clean checkout

The headline new file tests/plugin-emission.test.ts (404 lines, ~22 tests — the primary validation of this entire refactor) drives the plugin against a prototype/ DESI project that is gitignored and was never committed:

.gitignore (added in this PR):  prototype/   dev/

Result on a fresh clone of this branch:

Test Files  1 failed | 2 passed (3)
Tests      22 failed | 42 passed (64)

Every plugin-emission test fails with ENOENT … /prototype or the cascading expected undefined…. Only loader-validation and prose pass — because they build their own temp fixtures.

This directly contradicts the PR's test plan checkbox [x] npm test … green. It's only green on a machine where prototype/ happens to exist locally. There's also no CI workflow (.github/workflows is empty), so nothing catches this.

Options, roughly in order of preference:

1. Commit a small, committed fixture project under tests/fixtures/ (the old schema-v0.0.7/ fixtures were deleted in this PR) and point the tests at it. The old page-shape/schema-coverage suites used committed fixtures — that pattern was lost here.
2. If prototype/ must stay private, gate the suite (describe.skipIf(!existsSync(PROJECT_ROOT))) so the suite is honestly green rather than silently red, and say so in the test plan.
3. Add a CI workflow so this can't regress again.

As it stands, the core behavior of the new plugin is effectively untested for anyone but the author.

Code quality & correctness

Generally high. Specific notes:

• src/index.ts:105 cache freshness is narrower than it looks. getSource busts the cache only on astra.yaml's mtime. Editing a universes/*.yaml file or regenerating results/ will keep serving the stale parse during a long-lived myst start. The comment acknowledges .md-only watching but not the universe/results gap. Consider stat-ing the active universe file too, or document the limitation explicitly.

• src/transform/resolved-store.ts:36 doc drift. The header comment says the store "is carried on a hidden node's data (see astra-plugin.ts)" — that file no longer exists; it's index.ts now. Minor, but the rename missed this reference.

• Path handling from author-supplied ids. resolveArtifact (loader.ts:143) builds join(base, 'results', universeId, outputId) and valueRole reads arbitrary table files by id. splitPath splits on . so the worst case is mangled, not classic ../ traversal, and this is a trusted local build tool — but a one-line guard rejecting ids containing / or \ would close it cleanly.

• renderOneOutput resultUrl arity (render-evidence.ts:125). The opt is typed (absPath, outputId) => string but the plugin passes resultUrl(scope.root) which is (absPath) => string. Harmless (extra arg ignored), but the two signatures should agree or the second param dropped.

• provenance.ts is solid. The parent-linked frame walk with universe narrowing, seen cycle guard, and "direct beats transitive" dedup (addDecision, line 112) is careful work and reads correctly. This is the most intricate new logic and it's the least tested in a clean checkout (see above) — another reason the fixture matters.

• Loader validation policy (loader.ts:65) is a good call — advisory-only, each validator try/caught so a throwing v0.0.x validator can't take rendering down. Well reasoned and actually covered by loader-validation.test.ts.

Smaller things

• package.json rename to @astra-spec/mystra / 0.0.1 / BSD-3-Clause all look consistent; files: ["dist"] correctly ships only the build.
• The astra-cites hidden-carrier trick (emitting one cite per insight DOI so MyST's own citation pipeline resolves them) is clever and the right delegation — but it's exercised only by a prototype-dependent test.

Verdict

The architecture, the deletions, and the code itself are strong — the design is approvable. But the principal test file is red on a clean checkout while the test plan claims otherwise. Please commit a fixture (or gate the suite) and ideally add CI before merge. Everything else above is non-blocking polish.

🤖 Generated with Claude Code

[EiffL]

Update — blocking issue addressed

Resolved the test-suite problem by removing the prototype/ coupling entirely rather than vendoring the fixture in:

• Dropped prototype/ and dev/ from .gitignore (added in this PR).
• Deleted tests/plugin-emission.test.ts — the whole suite drove the plugin against the uncommitted prototype/ project, so it could never pass in a clean checkout.
• Added a minimal .github/workflows/ci.yml (checkout → npm ci → build → npm test --run on Node 20) so this can't regress silently. @astra-spec/sdk resolves from the public registry, so CI installs cleanly.

git grep -i prototype over tracked files is now empty, build is clean, and npm test --run is 38 passed and exits non-watch.

Tradeoff: this drops emission/role/value/resolved-store coverage — exactly the surface this refactor adds. Follow-up: a self-contained emission suite that builds a tiny astra.yaml + result files in a temp dir per test (same pattern loader-validation.test.ts already uses), to restore that coverage with zero prototype reference.

🤖 Generated with Claude Code

[EiffL]

@claude Can you redo a review of the current state of the project?

[claude]

Claude finished @EiffL's task in 3m 39s —— View job

---

Code Review — PR #5 (current state)

Tasks:

• Load tools
• Read CLAUDE.md and repo structure
• Review diff vs origin/main
• Read key source files
• Post comprehensive review

---

Overview

This is a review of the current working tree on strategy-a-refactor. Since the prior review, two things changed:

1. tests/plugin-emission.test.ts was deleted (rather than re-pointed at a committed fixture), removing the prototype coupling entirely.
2. .github/workflows/ci.yml was added: checkout → npm ci → npm run build → npm test --run on Node 20.

The architecture pivot itself is clean and well-executed. The "three sources of truth" framing (data in astra.yaml, composition in index.md, presentation in the theme) is consistently applied across every module. The plugin genuinely behaves as a pure projector.

---

🟡 Test coverage gap (non-blocking, follow-up needed)

Resolving the prototype coupling by deletion was the right call. But the trade-off is real: the core new behavior has zero direct test coverage.

What is covered today:

• loader-validation.test.ts — advisory validation policy, loadASTRASource load/warn contract
• prose.test.ts — myst-parser round-trip, anchor resolution grammar, all resolveAnchorPath branches

What has no coverage:

• Every directive (astra:decision, astra:output, astra:finding, astra:prior-insight, astra:inputs, astra:outputs, astra:subanalysis)
• Every inline role ({astra:decision}, {astra:output}, …)
• {astra:value} — the most complex role (filtering, uncertainty, err=/pm column logic, sig=)
• buildResolvedStore and every serializer in resolved-store.ts
• traceProvenance / pageFrames in provenance.ts
• renderOneOutput, renderDecision, renderFinding (these are render-pipeline helpers, not exposed directly)

loader-validation.test.ts already demonstrates the right pattern: build a temp dir, write a minimal astra.yaml, call the entry point directly, assert on output. The same approach applied to buildResolvedStore (call with a minimal Analysis object) and traceProvenance (call with a hand-built ProvFrame) would restore coverage without any prototype reference. The value role is the most urgent candidate: its column-filter + pm/err logic has several branching paths that are easy to get wrong silently.

The suggestion from the previous review still stands: a tests/plugin-core.test.ts using the same temp-dir fixture pattern as loader-validation.test.ts would be a solid follow-up.

---

Code quality findings

Stale doc in resolved-store.ts:19

The store is built once per page scope and carried on a hidden node's `data`
(see `astra-plugin.ts`)

astra-plugin.ts was never committed — the file is src/index.ts. Fix this →

Cache freshness: universe/results gap (src/index.ts:114)

mtimeMs = statSync(join(root, 'astra.yaml')).mtimeMs;

The cache is invalidated only when astra.yaml changes. During a live myst start session, editing a universes/*.yaml file or regenerating results/ will silently serve the stale parse until the server is restarted. The comment at line 109 mentions myst watches only .md files — which explains why the mtime check exists — but the limitation is that the universe file isn't included in the check. A low-cost fix: also stat the active universe file (its path is derivable from universeName() and the universes/ convention). Fix this →

resultUrl type/arity mismatch (src/transform/render-evidence.ts:125)

opts?: { resultUrl?: (absPath: string, outputId: string) => string }

But resultUrl(scope.root) in src/index.ts:237 returns (absPath: string) => string — a one-argument function. At line 144 of render-evidence.ts, the call is opts.resultUrl(resultPath, artifactId), passing artifactId as a second argument that is always ignored. TypeScript doesn't flag this because a narrower function is assignable to a wider signature, but the declared outputId parameter is dead code. Either drop it from the type signature, or wire it through the factory so custom URL schemes can use it. Fix this →

Multi-level ../ traversal collapses in provenance.ts:99-103

while (dec?.from?.startsWith('../') && at.parent) {
  const target = dec.from.replace(/^(\.\.\/)+/, '');  // strips ALL ../
  at = at.parent;                                       // but only climbs one level
  dec = at.analysis.decisions?.[target];
  id = target;
}

The regex /^(\.\.\/)+/ greedily strips all leading ../ in one pass, but at = at.parent climbs only one level. So from: ../../x and from: ../x both resolve the same way — ../../x silently loses the outer parent level. This probably isn't exercised in practice (decisions rarely carry multi-level parent escapes), but it's a latent bug worth a comment or a fix. Fix this →

---

Highlights

• loader.ts validation policy (lines 65–107): the advisory-only, each-call-wrapped-in-try/catch pattern is exactly right for v0.0.x SDK validators. Well-reasoned and covered by tests.
• provenance.ts cycle guard and dedup (the seen set + "direct beats transitive" rule in addDecision): careful and correct for the trickiest new logic in this PR.
• prose.test.ts: exhaustive coverage of anchor grammar paths, including parent-escape fallback, sub-analysis cross-page URLs, and image embed resolution. These tests are well-structured and will catch regressions in the prose engine.
• makeTabItem counter (ast-helpers.ts:132-148): the closure-scoped counter (rather than a module-global) is the right call — prevents spurious diff noise in serialized AST across passes.
• rewriteStaticImages + anchorTransform: the two-pass design (directives resolve per-component, the transform covers author-written anchors) cleanly handles both author prose and plugin-rendered output without duplicating logic.
• buildResolvedStore parentInputs flow: the innermost-wins merge for aliased inputs is correct and the logic in serializeInput handles the cross-scope from: scope.out_id case cleanly (left as-is, not confused with a plain alias).

---

CI

The workflow at .github/workflows/ci.yml is minimal and correct: Node 20, npm ci, build, test with --run. The only minor note: on.push.branches: [main] means pushes to feature branches without open PRs don't get CI. Not a problem in practice for this repo's flow, but worth knowing.

---

Verdict

The architecture is sound and the previous blocking issue (prototype-coupled tests, no CI) is resolved. All remaining items are non-blocking polish. The design is approvable. The one strong follow-up recommendation is a tests/plugin-core.test.ts with self-contained temp fixtures for buildResolvedStore, traceProvenance, and the {astra:value} role — that would restore coverage for the code that matters most without any prototype dependency.

[EiffL]

Addressed the re-review — e6d7a1b

Every item from the latest review is now resolved.

Test coverage gap (the main follow-up): added tests/plugin-core.test.ts — a self-contained temp-dir fixture (its own astra.yaml, universe, and result artifacts; no prototype/ dependency, modeled on a real ASTRA project rather than copied). It covers what was previously untested:

• all seven block directives (decision/output×{figure,table,metric,alias}/finding/finding :compact:/prior-insight/subanalysis/inputs/outputs) plus error paths (bare-from decision, unknown id)
• every inline cite role + the |display override + scoped dotted paths
• {astra:value} — pm, explicit err=, sig=, scoped product, and both error branches (missing column, non-matching row)
• buildResolvedStore (including a direct call with a hand-built Analysis and no result files) — outputs/inputs/decisions/findings/insights/subanalyses, inlined table_data + metric, project-relative urls, universe-resolved selection, the hidden carrier, and the astra-cites DOI emission
• traceProvenance/pageFrames — cross-scope inputs_root/decisions_transitive with universe narrowing and dedup, plus a direct unit test for multi-level ../

Suite is 77 passing (was 38), build clean.

Code findings:

• 🐛 provenance.ts multi-level ../ — fixed: now climbs one scope per ../ (the greedy replace(/^(\.\./)+/) collapsed ../../x to a single climb). Added a regression test, and verified it fails against the old code and passes against the fix.
• Cache freshness (universe gap) — fixed: getSource now busts on the newest mtime of astra.yaml and the active universe file (exposed universeFilePath from the loader so both paths agree). Covered by the live-reload test, which now advances the universe file specifically.
• resultUrl arity mismatch — fixed: dropped the dead outputId param from the type and call site.
• Stale doc (resolved-store.ts → astra-plugin.ts) — fixed to point at the astra-resolved-store transform in index.ts.

CI note (on.push.branches: [main]): PRs already get CI via the pull_request trigger, so this branch is covered; left as-is per your "not a problem in practice."

🤖 Generated with Claude Code