diff --git a/.beads/issues.jsonl b/.beads/issues.jsonl index 9340e2de8..dc270902b 100644 --- a/.beads/issues.jsonl +++ b/.beads/issues.jsonl @@ -111,6 +111,7 @@ {"id":"bd-57y4","title":"Vendor and integrate quarto-listing.scss with theme-CSS pipeline","description":"Per L3 D5 (decided 2026-05-06), the listing JS pair (list.min.js + quarto-listing.js) shipped via the artifact store as Project-scoped js: artifacts in the L3 phase 7 commit. The third asset, quarto-listing.scss, was deferred because it requires Bootstrap variable wiring + media-breakpoint mixins from the existing theme-CSS pipeline (CompileThemeCssStage / quarto_sass::SassLayer). That's a larger design task than the static JS bundling.\n\nThis issue: integrate the listing SCSS as a SassLayer that gets composed with the website's theme bundle. The Q1 source file lives at external-sources/quarto-cli/src/resources/projects/website/listing/quarto-listing.scss; copy to resources/listing/quarto-listing.scss per the External Sources Policy and add it to the theme bundle when at least one listing is rendered.\n\nWhile the SCSS isn't compiled, listings render with default browser styling — the markup and JS are unchanged. The list / grid / table layouts work but lack the polished card / table styling Q1 ships.\n\nDiscovered while shipping L3 phase 7 (bd-ml8z); see D5 in claude-notes/plans/2026-05-06-listings-L3-resolve-transform.md.\n\nL5 (bd-5vsr) lands the markup that consumes this SCSS; merging bd-57y4 restores Q1 visual parity for category sidebars and per-item category chips.","status":"open","priority":2,"issue_type":"task","created_at":"2026-05-06T20:52:15.894830Z","created_by":"cscheid","updated_at":"2026-05-07T13:46:18.157025Z","source_repo":".","compaction_level":0,"original_size":0,"dependencies":[{"issue_id":"bd-57y4","depends_on_id":"bd-ml8z","type":"discovered-from","created_at":"2026-05-06T20:52:15.894830Z","created_by":"cscheid","metadata":"{}","thread_id":""}]} {"id":"bd-5qnj","title":"Manage trace size for use as replay/regression-test fixtures","description":"Spun out of bd-45yw (replay engine). Once traces double as replay fixtures (and as user-attached bug-report artifacts), trace size becomes a real constraint — they will be checked into the repo as regression fixtures and posted by users in issues.\n\nCurrent quarto-trace output captures per-stage pipeline state (see crates/quarto-trace/src/lib.rs and crates/quarto-core/src/stage/trace.rs::JsonTraceObserver), which is already on the heavy side for routine diagnostic use.\n\nInvestigate:\n- Where the bulk lives in current traces (per-stage AST snapshots? supporting_files content? something else?)\n- Which content is actually load-bearing for replay vs. diagnostics, and whether the two roles can share one artifact (the design preference noted in bd-45yw's plan)\n- Compression on disk; lazy loading on read\n- Whether per-stage AST snapshots can be elided/diffed when not needed\n- Size budgets — what is reasonable for (a) checked-in CI fixtures and (b) user-attached bug reports\n\nGoal: make 'one trace serves both diagnostic and replay roles' practical. If size cannot be bounded, fall back to two separate artifacts and document why.\n\nReferences:\n- crates/quarto-trace/src/lib.rs (TraceDocument, TraceEntry)\n- crates/quarto-core/src/stage/trace.rs (JsonTraceObserver)\n- claude-notes/plans/2026-05-03-replay-engine.md (open subquestion in Phase 1)","status":"closed","priority":2,"issue_type":"task","created_at":"2026-05-03T21:11:19.859036Z","created_by":"cscheid","updated_at":"2026-05-03T23:29:05.405253Z","closed_at":"2026-05-03T23:29:05.405068Z","close_reason":"Phases 1, 2, 3, and 5 complete. End-to-end verified on bd-5qnj fixtures: big.qmd 16.3 MB pretty -> 62 KB gzipped+deduped (~265x). Unified-artifact promise realized post-merge with bd-45yw. Real-engine supporting_files re-measurement tracked as bd-sr73.","source_repo":".","compaction_level":0,"original_size":0,"dependencies":[{"issue_id":"bd-5qnj","depends_on_id":"bd-45yw","type":"related","created_at":"2026-05-03T21:11:19.859036Z","created_by":"cscheid","metadata":"{}","thread_id":""}]} {"id":"bd-5vsr","title":"L5 — Categories sidebar","description":"Right-margin category list emitted from L3's resolved item set, grouped by category. Three Q1-parity styles: category-default, category-unnumbered, category-cloud. Templates embedded as doctemplate via MemoryResolver. Click-to-filter interactivity scoped out of v1. See claude-notes/plans/2026-05-05-listings-epic.md §L5.","status":"closed","priority":2,"issue_type":"feature","created_at":"2026-05-05T19:53:32.333630Z","created_by":"cscheid","updated_at":"2026-05-07T16:00:41.250061Z","closed_at":"2026-05-07T16:00:41.249924Z","close_reason":"L5 — categories sidebar landed on feature/listings via merge 9e8afa0d (impl 2750546b + follow-on 67a985f4). Per-item chips + right-margin sidebar with three modes (default/unnumbered/cloud); Q-12-11 and Q-12-12 diagnostics; aggregate across multi-listing pages. End-to-end CLI verification recorded in plan §End-to-end CLI verification record. cargo xtask verify clean (Rust + hub-client + WASM). Test count 8570 → 8621 (+51). Discovered-from follow-ups filed: bd-99ru (localize labels), bd-754f (review encoding scheme), bd-ra5j (hub-client browser smoke deferred), bd-nwyp (PandocInlines parser audit).","source_repo":".","compaction_level":0,"original_size":0,"dependencies":[{"issue_id":"bd-5vsr","depends_on_id":"bd-61cd","type":"parent-child","created_at":"2026-05-05T19:53:32.333630Z","created_by":"cscheid","metadata":"{}","thread_id":""},{"issue_id":"bd-5vsr","depends_on_id":"bd-ml8z","type":"blocks","created_at":"2026-05-05T19:53:32.333630Z","created_by":"cscheid","metadata":"{}","thread_id":""}]} +{"id":"bd-5yff4","title":"Sequential multi-engine execution (engine: [a, b, ...])","description":"Investigate and design support for running multiple Quarto 2 execution engines in sequence for a single document.\n\nTwo coupled changes:\n\n1. YAML config: allow 'engine:' to be an ordered array (e.g. engine: [knitr, mermaidjs]) in addition to the current singular string/map forms. Distinct engines only; order is significant (engine N may emit code cells consumed by engine N+1). Array merge across config layers uses the existing default (!concat); no engine-specific tag default for now.\n\n2. Pipeline: thread N engines through EngineExecutionStage. The AST->text->engine->text->AST->reconcile loop already type-checks end-to-end; each subsequent engine starts from the AST after the prior engine's results were reconciled in. Generalize the FileId/intermediate-file slot handling (currently 2 slots: .qmd + one .rmarkdown) to N+1 slots.\n\nTracing/replay/preview redesign: TraceDocument.engine_capture (single Option slot, name-keyed replay) becomes per-engine. Capture one AST snapshot + one EngineCapture per engine invocation within the stage, mirroring the existing transform: sub-entry pattern. Preview record/replay (record_capture, with_replay, preview cache) generalize from one capture to an ordered list.\n\nValidation/testing uses a simple file-backed test engine (reads a results file and splices per-cell outputs in order) so multi-engine sequencing can be exercised deterministically without R/Python/Jupyter, and without committing to a real second engine (e.g. mermaidjs) yet.\n\nPlan: claude-notes/plans/2026-05-27-multi-engine-execution.md\n\nStatus: design/investigation. Implementation gated on user go-ahead after plan review.","status":"in_progress","priority":2,"issue_type":"feature","created_at":"2026-05-27T14:07:37.941942Z","created_by":"cscheid","updated_at":"2026-05-27T14:33:39.118501Z","source_repo":".","compaction_level":0,"original_size":0} {"id":"bd-5ym51","title":"tree-sitter inline-multiline-attrs tests fail with 20 parse errors on main","description":"Running 'tree-sitter test' from crates/tree-sitter-qmd/tree-sitter-markdown/ reports 20 failing test cases in the inline-multiline-attrs corpus group (cases 95-104 plus a heading case), with 'Total parses: 508; successful parses: 488; failed parses: 20; success percentage: 96.06%'. This trips 'cargo xtask verify' at Step 4/12.\n\nConfirmed pre-existing on origin/main HEAD (3e781667 at time of report) by stashing all local changes and re-running the test — same failures. The failing cases overlap with the feature that landed in 1afd5a6ac95dd ('tree-sitter-qmd - allow multi-line attribute lists on inline images/spans (#209)', merged 2026-05-17).\n\nFailing case names include:\n- image with multi-line class-only attrs (quarto-web hero-banner case)\n- image with multi-line mixed class + key=value attrs\n- image with single-line class + leading and trailing whitespace inside braces\n- span with multi-line class attrs\n- span with multi-line key=value attrs\n- heading with leading whitespace inside block attribute_specifier braces\n- bulleted/ordered list item with multi-line span/image attrs\n- nested bulleted list with multi-line span attrs in inner item\n\nLikely causes (need investigation):\n1. The compiled parser.c committed in 1afd5a6a doesn't match the grammar.js changes — needs regeneration.\n2. The test corpus expectations were updated but the parser output diverged.\n3. The 'tree-sitter test' tool version disagrees with what was used at merge time.\n\nDiscovered-from: bd-j73yw (Phase 1 of bd-1tl09, code-block decorations) — surfaced while running cargo xtask verify after Phase 0 / Phase 1 commit 1 work. Not caused by that work; clean stash confirms regression on main.\n\nWorkaround for the dependent epic: run 'cargo xtask verify --skip-treesitter-tests' (or call cargo nextest / cargo build steps directly) until this is fixed.","status":"open","priority":2,"issue_type":"bug","created_at":"2026-05-19T20:41:11.658379Z","created_by":"cscheid","updated_at":"2026-05-19T20:41:11.658379Z","source_repo":".","compaction_level":0,"original_size":0,"dependencies":[{"issue_id":"bd-5ym51","depends_on_id":"bd-j73yw","type":"discovered-from","created_at":"2026-05-19T20:41:11.658379Z","created_by":"cscheid","metadata":"{}","thread_id":""}]} {"id":"bd-61cd","title":"Listings feature epic","description":"Implement Q1-feature-parity listings on Q2's DocumentProfile + 2-pass architecture. Sub-phases L0–L11. Plan: claude-notes/plans/2026-05-05-listings-epic.md. Discussion: claude-notes/plans/2026-05-05-listings-design-discussion.md. Settled decisions: C5 listing_item profile field, doctemplate-not-EJS, generate/render decomposition, L7 bracketed as CLI-only with mandatory L1 fallback.","status":"open","priority":1,"issue_type":"epic","created_at":"2026-05-05T19:52:42.035992Z","created_by":"cscheid","updated_at":"2026-05-05T19:52:42.035992Z","source_repo":".","compaction_level":0,"original_size":0} {"id":"bd-66nd","title":"CI disk-space cleanup: profile.ci + drop redundant cargo build","description":"Test-suite workflow hits 'No space left on device' on ubuntu-latest during cargo nextest run despite the existing free-disk-space step (run 25062065055 on PR #139, freed 18.8 GB but not enough). Add [profile.ci] Cargo profile to strip debuginfo, remove redundant cargo build step before nextest, enable remove_tool_cache: true on free-disk-space, and prune Docker images. Full rationale in claude-notes/2026-04-28-ci-disk-space-and-profile-ci.md. Implemented on branch ci-disk-space-profile-ci (4 commits, not yet pushed).","status":"closed","priority":1,"issue_type":"chore","created_at":"2026-04-28T17:17:54.174452300Z","created_by":"cderv","updated_at":"2026-05-04T14:42:11.368436Z","closed_at":"2026-05-04T14:42:11.368004800Z","close_reason":"Merged in PR #141 (squash commit 7fb348e7)","source_repo":".","compaction_level":0,"original_size":0,"comments":[{"id":17,"issue_id":"bd-66nd","author":"cderv","text":"Branch pushed and PR #141 opened https://github.com/quarto-dev/q2/pull/141 — CI verification pending on next workflow run. Will close when PR merges.","created_at":"2026-04-29T08:06:15Z"}]} @@ -137,6 +138,7 @@ {"id":"bd-87tr","title":"L8 follow-up: WASM/VFS-aware custom listing template loading","description":"Today (L8 D1) custom-template loading uses std::fs::read_to_string directly, so hub-client previews of `type: custom` listings fall back to the default built-in with Q-12-8. Plumb runtime through RenderContext (or pre-load template content during Pass-1) so VFS reads work in WASM. See claude-notes/plans/2026-05-07-listings-L8-custom-templates.md §\"Out of scope\" / §\"Decisions log\" D1.","status":"closed","priority":2,"issue_type":"feature","created_at":"2026-05-08T14:56:49.244272Z","created_by":"cscheid","updated_at":"2026-05-08T14:57:25.638507Z","closed_at":"2026-05-08T14:57:25.638365Z","close_reason":"Duplicate of bd-tmka","source_repo":".","compaction_level":0,"original_size":0,"dependencies":[{"issue_id":"bd-87tr","depends_on_id":"bd-rqgx","type":"discovered-from","created_at":"2026-05-08T14:56:49.244272Z","created_by":"cscheid","metadata":"{}","thread_id":""}]} {"id":"bd-8d6rk","title":"Migrate sidebar/navbar 'missing document information' warnings to structured diagnostics","description":"Today the missing-document and dropped-auto warnings in `crates/quarto-core/src/transforms/navigation_href.rs` (`resolve_href_for_html`, `resolve_doc_relative_href`) and `sidebar_auto.rs` (`strip_entries`, `expand_spec`) are emitted as plain `DiagnosticMessage::warning(format!(...))` strings: no error code, no SourceInfo, no catalog entry, no docs URL. Programmatic consumers (`--json`, hub-client) get a title-string only.\n\nGoal: migrate these warnings to structured `DiagnosticMessage` via the builder API. Allocate a new navigation subsystem in the error catalog (likely Q-13-*) for: missing referenced document (sidebar variant), missing referenced document (navbar variant), missing referenced document (page-footer variant), missing referenced document (body link variant), dropped `auto:` (no project index), unmatched `auto:` glob. Each gets a code, a title, a problem statement, a hint, and a catalog entry with `docs_url`.\n\nNote: this issue does NOT yet attach SourceInfo to the diagnostics — the parsers currently strip it. Initial diagnostics will set `location: None`. The follow-up (path-resolution / source-location plumbing) will fill in the location.\n\nPlan: claude-notes/plans/2026-05-20-bd-PLACEHOLDER-navigation-diagnostics.md (to be created)","notes":"Plan written: claude-notes/plans/2026-05-20-bd-8d6rk-navigation-diagnostics.md (draft, awaiting iteration before implementation)","status":"closed","priority":2,"issue_type":"task","created_at":"2026-05-20T14:56:13.306265Z","created_by":"cscheid","updated_at":"2026-05-20T16:59:04.371379Z","closed_at":"2026-05-20T16:59:04.371196Z","close_reason":"Implementation complete on main. Q-13-1..7 navigation diagnostics live; all 9189 workspace tests pass; cargo xtask verify --skip-hub-build clean. End-to-end verified on docs/guide/index.qmd. Plan: claude-notes/plans/2026-05-20-bd-8d6rk-navigation-diagnostics.md (all phases checked). Changes uncommitted pending user review.","source_repo":".","compaction_level":0,"original_size":0} {"id":"bd-8exa","title":"Implement shareable project URLs for hub-client","description":"Add support for shareable URLs that use automerge index document IDs instead of local IndexedDB UUIDs. This enables users to share project links with others while minimizing exposure of sensitive document IDs.\n\nPlan: claude-notes/plans/2026-02-03-shareable-urls.md","status":"in_progress","priority":1,"issue_type":"feature","created_at":"2026-02-03T15:17:50.527124Z","created_by":"cscheid","updated_at":"2026-02-03T15:24:33.898151Z","source_repo":".","compaction_level":0,"original_size":0} +{"id":"bd-8h3sn","title":"Precise cross-engine source attribution in EngineExecutionStage","description":"EngineExecutionStage threads the document's top-level source_context (constant) to every engine for error attribution. This is exact for engine 1; for engine 2+, offsets into content produced by an earlier engine don't resolve (their FileIds live only in the merged ASTContext), so error mapping is best-effort (None = no precise location). Matches the existing single-engine code's stated tolerance. A future refinement could thread the accumulating merged context. See engine_execution.rs run loop comment.","status":"open","priority":3,"issue_type":"task","created_at":"2026-05-27T16:24:40.252234Z","created_by":"cscheid","updated_at":"2026-05-27T16:24:40.252234Z","source_repo":".","compaction_level":0,"original_size":0,"dependencies":[{"issue_id":"bd-8h3sn","depends_on_id":"bd-5yff4","type":"discovered-from","created_at":"2026-05-27T16:24:40.252234Z","created_by":"cscheid","metadata":"{}","thread_id":""}]} {"id":"bd-8h9o","title":"L1 listings: filter unresolved shortcodes in image-src auto-fill","description":"ListingItemInfoStage (bd-izqh) runs after IncludeExpansionStage but BEFORE PreEngineSugaringStage. An Image node whose original src was a shortcode like '{{< meta thumbnail >}}.png' will still carry that literal text in target.0 at L1 time.\n\nL1 currently does not filter such cases when picking the first body image. When listings consume this field (L3+), unresolved shortcode markup could leak into rendered listings as the .\n\nInvestigate:\n- Real-world cases where image src is set via shortcode (Q1 examples worth grepping).\n- Whether the literal markers are limited to '{{<' / '>}}' or whether other deferred-substitution forms (e.g. '$var$') also reach this point.\n- Whether pre-engine sugaring resolves these in time for L1's data to be useful, or whether shortcode-bearing images need their own pass.\n- Resolution choice: skip such images entirely (use second image, or none); or store the unresolved form and have L3 strip; or wait for sugaring before extracting.\n\nOut of scope: L1 itself. This issue is the dedicated study; no behavior change in bd-izqh.","status":"open","priority":3,"issue_type":"task","created_at":"2026-05-06T13:40:57.410421Z","created_by":"cscheid","updated_at":"2026-05-06T13:40:57.410421Z","source_repo":".","compaction_level":0,"original_size":0,"dependencies":[{"issue_id":"bd-8h9o","depends_on_id":"bd-izqh","type":"discovered-from","created_at":"2026-05-06T13:40:57.410421Z","created_by":"cscheid","metadata":"{}","thread_id":""}]} {"id":"bd-8k4ny","title":"Author error-docs pages for internal subsystem (2 codes)","description":"Author stub-quality pages for all 2 internal subsystem error codes under docs/errors/internal/. Follow template in docs/errors/yaml/Q-1-10.qmd. See claude-notes/plans/2026-05-22-error-docs-content.md.","status":"closed","priority":2,"issue_type":"task","created_at":"2026-05-24T21:07:57.559596Z","created_by":"cscheid","updated_at":"2026-05-24T21:15:22.865203Z","closed_at":"2026-05-24T21:15:22.864846Z","source_repo":".","compaction_level":0,"original_size":0} {"id":"bd-8kp3","title":"Implement include-in-header / include-before-body / include-after-body (HTML)","description":"Implement Q1-compatible document-include slot keys (file-path-based with smart-include {file:..} / {text:..} object form) for HTML output. Reuse the navbar/footer 'generate then render' pipeline pattern: an IncludeResolveTransform reads authored keys + folds engine PandocIncludes + resolves smart-includes, writes flat literal-text lists to a canonical 'rendered.includes.{header,before-body,after-body}' metadata location; the template reads from there. Migrates WebsiteFaviconTransform / WebsiteCanonicalUrlTransform off direct authored-meta writes onto the new contributor surface. Adds the file-include set to DocumentProfile for Phase-8 cache invalidation.\\n\\nPlan: claude-notes/plans/2026-05-04-includes-feature.md (draft for review).\\nRelated: bd-xfwx (IncludeExpansionStage shortcode form), bd-r82e (profile-includes cache invalidation), bd-b9mz (Phase-7 favicon/canonical-url transforms to migrate).","status":"in_progress","priority":2,"issue_type":"feature","created_at":"2026-05-04T19:18:34.560486Z","created_by":"cscheid","updated_at":"2026-05-04T19:40:54.092659Z","source_repo":".","compaction_level":0,"original_size":0,"dependencies":[{"issue_id":"bd-8kp3","depends_on_id":"bd-r82e","type":"related","created_at":"2026-05-04T19:18:37.299526Z","created_by":"cscheid","metadata":"{}","thread_id":""},{"issue_id":"bd-8kp3","depends_on_id":"bd-xfwx","type":"related","created_at":"2026-05-04T19:18:37.412747Z","created_by":"cscheid","metadata":"{}","thread_id":""}]} @@ -265,6 +267,7 @@ {"id":"bd-ij1l","title":"Phase 3: Wire forked runtimelib into Quarto, expose diagnostic error","description":"Add [patch.crates-io] entry pointing runtimelib at cscheid/runtimelib feat/venv-kernelspec-discovery branch (pin to a rev once stable). Switch crates/quarto-core/src/engine/jupyter/kernelspec.rs to list_kernelspecs_with_jupyter_paths(). Replace JupyterError::KernelspecNotFound { name } with { name, searched: Vec, available: Vec } and update Display to render searched paths, available kernels, and a remediation hint pointing at jupyter kernelspec list and JUPYTER_PATH.\n\nEnd-to-end verification per CLAUDE.md: render convert-test-3.qmd from a venv shell and confirm success; record invocation + output snippet.\n\nBlocked by: bd-34wy (fork must exist first).\nPlan: claude-notes/plans/2026-05-04-jupyter-kernelspec-discovery-and-errors.md (Phase 3)","status":"closed","priority":1,"issue_type":"feature","created_at":"2026-05-04T15:49:56.240599Z","created_by":"cscheid","updated_at":"2026-05-04T16:15:19.841987Z","closed_at":"2026-05-04T16:15:19.841852Z","close_reason":"Wired runtimelib fork into Quarto via [patch.crates-io] (path patch — will become git rev before push). list_kernelspecs/find_kernelspec route through *_with_jupyter_paths variants. JupyterError::KernelspecNotFound now carries searched: Vec and available: Vec; Display renders a multi-line diagnostic with searched paths, available kernels, and a remediation hint. From pattern-matches KernelNotFound and forwards fields.\n\nEnd-to-end verified in commit 03e8cab2:\n- Original failing fixture (convert-test-3.qmd) now renders with kernel output, when run from the venv shell.\n- Ruby fixture (no kernel installed) produces the new multi-line diagnostic.\n- cargo build --workspace clean, cargo nextest run --workspace 8360 passed, cargo xtask verify --skip-hub-build all green.\n\nCloses the user-visible bug for bd-fu0l. Phase 4 (bd-875x, upstream PR) is now unblocked.","source_repo":".","compaction_level":0,"original_size":0,"labels":["dx","feature","jupyter"],"dependencies":[{"issue_id":"bd-ij1l","depends_on_id":"bd-34wy","type":"blocks","created_at":"2026-05-04T15:49:56.240599Z","created_by":"cscheid","metadata":"{}","thread_id":""},{"issue_id":"bd-ij1l","depends_on_id":"bd-fu0l","type":"discovered-from","created_at":"2026-05-04T15:49:56.240599Z","created_by":"cscheid","metadata":"{}","thread_id":""}]} {"id":"bd-ilv8p","title":"tree-sitter qmd: allow line breaks inside inline code spans and inline math","description":"Pampa rejects multi-line inline code spans that pandoc accepts as a single Code element. Example: `A simple ``code\\nspan`` test.` errors at the opening backtick. See claude-notes/plans/2026-05-24-multiline-inline-code-spans.md for diagnosis and implementation plan.","status":"closed","priority":2,"issue_type":"bug","created_at":"2026-05-24T21:12:25.161104Z","created_by":"cscheid","updated_at":"2026-05-24T22:27:40.235371Z","closed_at":"2026-05-24T22:27:40.235225Z","close_reason":"Implemented in commit (pending). Scanner+grammar+post-processor changes for code spans and math spans; full E2E parity with pandoc verified on 11 fixtures; cargo xtask verify green (12/12 steps); 9425/9425 workspace tests pass. Plan: claude-notes/plans/2026-05-24-multiline-inline-code-spans.md","source_repo":".","compaction_level":0,"original_size":0} {"id":"bd-imiw","title":"Design and implement YAML-controlled top-level navbars and page footers for Quarto 2 HTML output","description":"Quarto 1 supports website navbars and page footers only as project-level (website/book) features. Quarto 2 already allows user-controlled TOCs via YAML metadata at the document level. This issue extends that pattern to top-level navbars and page footers: HTML documents should be able to declare and customize navbar/footer contents via YAML metadata, mirroring the TOC mechanism. Includes design proposal (YAML schema, familiar to Quarto 1 users), pipeline integration (Generate + Render transforms, mirroring TocGenerateTransform/TocRenderTransform), and template wiring.\n\nPlan: claude-notes/plans/2026-04-18-navbar-footer-design.md","status":"in_progress","priority":1,"issue_type":"feature","created_at":"2026-04-18T17:13:52.866569Z","created_by":"cscheid","updated_at":"2026-04-18T18:17:33.919378Z","source_repo":".","compaction_level":0,"original_size":0} +{"id":"bd-iq0hp","title":"Multi-engine preview: browser E2E + composing test engines","description":"A browser end-to-end test of MULTI-engine q2 preview was not possible during bd-5yff4: the preview server uses the default engine registry (real engines), the test-only FixtureEngine isn't registered there, and knitr/jupyter don't compose cleanly (knitr claims python cells). Single-engine preview is unchanged + unit-tested; the capture-sequence transport round-trips via quarto-preview integration tests. Options: (a) wire a flag-gated fixture engine into the preview server for tests, (b) build a real second engine, (c) hand-construct a two-capture sidecar and drive a browser session. Plan: claude-notes/plans/2026-05-27-multi-engine-execution.md","status":"open","priority":2,"issue_type":"task","created_at":"2026-05-27T16:24:40.064471Z","created_by":"cscheid","updated_at":"2026-05-27T16:24:40.064471Z","source_repo":".","compaction_level":0,"original_size":0,"dependencies":[{"issue_id":"bd-iq0hp","depends_on_id":"bd-5yff4","type":"discovered-from","created_at":"2026-05-27T16:24:40.064471Z","created_by":"cscheid","metadata":"{}","thread_id":""}]} {"id":"bd-ir8n","title":"L9 follow-up: inline-code-style syntax-highlight class maps in full feeds","description":"Port Q1's inline-code-style transform: maps highlight classes (token comment, etc.) to inline style=\"color: ...\" so feeds render with colors when the subscriber's stylesheet doesn't include Quarto's CSS. v1 leaves highlight classes verbatim; readers without the CSS render code blocks in default mono. Reader extension lives in feed/reader_ext.rs; gated behind an RssReaderOptions flag.","status":"open","priority":3,"issue_type":"feature","created_at":"2026-05-08T17:33:16.610890Z","created_by":"cscheid","updated_at":"2026-05-08T17:33:16.610890Z","source_repo":".","compaction_level":0,"original_size":0,"dependencies":[{"issue_id":"bd-ir8n","depends_on_id":"bd-o90m","type":"discovered-from","created_at":"2026-05-08T17:33:16.610890Z","created_by":"cscheid","metadata":"{}","thread_id":""}]} {"id":"bd-itj9","title":"Add WASM test execution to CI","description":"Add real wasm32 smoke tests and CI job for pampa Lua WASM code paths. 6 tests in crates/pampa/tests/wasm_lua.rs verify restricted Lua VM, filter/shortcode execution, error handling, and synthetic io/os on wasm32-unknown-unknown. Includes wasm-c-shim shared crate, dev-dep gating, and wasm-tests CI job. Blocked by JS bridge raw_module paths in quarto-system-runtime needing feature-gate. Cfg proxy fix and cleanup shipped separately in PR #116.","notes":"PR #109 rebased on #116, base branch changed. PR #116 (cfg fix + cleanup) is CI green and ready to merge. PR #109 now focused solely on WASM CI test infrastructure. JS bridge blocker documented, recommended fix is feature-gating raw_module extern blocks in quarto-system-runtime.","status":"in_progress","priority":3,"issue_type":"task","created_at":"2026-04-02T10:43:59.896261300Z","created_by":"cderv","updated_at":"2026-04-24T17:13:49.430217800Z","source_repo":".","compaction_level":0,"original_size":0,"comments":[{"id":1,"issue_id":"bd-itj9","author":"cderv","text":"WASM test coverage analysis: npm run test:wasm (TS side) tests rendering, templates, format detection via compiled WASM module — does NOT cover Lua filter traversal. The test cfg proxy in Rust is the ONLY thing catching WASM-incompatible Lua code in filter tests. So the proxy has value on Linux/macOS — just not on Windows. wasm-pack test would be the proper replacement: compiles Rust to wasm32, runs #[wasm_bindgen_test] in browser/Node on the real WASM target. build-wasm.yml already installs wasm-pack but only runs build, not test. Infrastructure needed: wasm-pack test in CI, test harness crate with wasm_bindgen_test, possibly browser driver. Approach: (1) Remove test from cfg guard in filter.rs/shortcode.rs, (2) Add wasm-pack test to build-wasm.yml or new workflow with #[wasm_bindgen_test] versions of key filter tests, (3) Gate io_wasm/os_wasm unit tests to wasm32 target only.","created_at":"2026-04-02T15:10:34Z"},{"id":3,"issue_id":"bd-itj9","author":"cderv","text":"Important: this project does NOT use wasm-pack. It uses cargo build --target wasm32-unknown-unknown + wasm-bindgen CLI directly because -Zbuild-std=std,panic_unwind is needed for Lua error handling (setjmp/longjmp to panic/catch_unwind). See hub-client/scripts/build-wasm.js. This means wasm-pack test wont work — WASM testing would need cargo test --target wasm32-unknown-unknown -Zbuild-std plus wasm-bindgen-test-runner, mirroring the custom build approach.","created_at":"2026-04-03T09:09:11Z"},{"id":4,"issue_id":"bd-itj9","author":"cderv","text":"wasm-bindgen-test infrastructure already exists: wasm-qmd-parser/tests/web.rs has a placeholder #[wasm_bindgen_test] test with run_in_browser config. The crate already depends on wasm-bindgen-test 0.3.34. Pattern is ready to extend with real filter tests. Note: wasm-qmd-parser uses wasm-pack (simpler build), wasm-quarto-hub-client uses custom cargo build + wasm-bindgen (needs -Zbuild-std). Test runner choice depends on which crate hosts the WASM filter tests.","created_at":"2026-04-03T09:11:25Z"},{"id":6,"issue_id":"bd-itj9","author":"cderv","text":"Branch split decision (2026-04-13): PR #109 grew too large mixing cfg proxy fix with WASM CI test infrastructure. Split into two branches: (1) fix/wasm-cfg-proxy-and-cleanup ships the cfg proxy removal, dead crate cleanup, dtolnay replacement, and doc updates. (2) feature/wasm-testing-and-cleanup retains the WASM smoke tests, wasm-c-shim crate, CI job, and related infrastructure for a future PR. Known blocker for Branch B: quarto-system-runtime/src/wasm.rs has 4 raw_module extern blocks (/src/wasm-js-bridge/{template,sass,cache,fetch}.js) that get baked into any wasm32 binary. wasm-bindgen-test-runner generates require() calls for these absolute paths which fail in Node.js. Recommended fix: feature-gate the JS bridge (add js-bridge feature to quarto-system-runtime, gate the 4 extern blocks, provide stub impls when off, wasm-quarto-hub-client enables it, pampa test builds dont).","created_at":"2026-04-13T20:48:28Z"},{"id":7,"issue_id":"bd-itj9","author":"cderv","text":"PR restructured (2026-04-14): PR #116 (fix/wasm-cfg-proxy-and-cleanup) opened for Branch A, CI green. PR #109 rebased onto #116 and base changed to fix/wasm-cfg-proxy-and-cleanup. PR #109 diff now shows only WASM CI test additions. When #116 merges to main, GitHub auto-updates #109 base to main.","created_at":"2026-04-14T12:34:20Z"},{"id":8,"issue_id":"bd-itj9","author":"cderv","text":"PR #116 merged to main (squash commit 52968801) on 2026-04-23. Ships cfg proxy removal from filter.rs/shortcode.rs, dofile_wasm test skip on native, and updated docs (dev-docs/wasm.md, .claude/rules/wasm.md, testing.md). PR #109 remains open with WASM CI smoke test infrastructure; auto-rebased to main. Remaining blocker: JS bridge raw_module paths in quarto-system-runtime need feature-gating before wasm-bindgen-test-runner can work.","created_at":"2026-04-23T14:05:04Z"},{"id":10,"issue_id":"bd-itj9","author":"cderv","text":"2026-04-24 session: Rebased PR #109 onto latest origin/main (50+ commits since Gordon's prior rebase on Apr 17). 2 conflicts resolved (Cargo.toml + Cargo.lock). Docs audit found 3 drifts, fixed in b2e88ec1. cargo fmt on shim.rs in 1f080db5. Verified wasm-c-shim/src/shim.rs byte-identical to main's c_shim.rs modulo CRLF. Build succeeds after cmake install (bd-n7x2 introduced cmake as a hard requirement via tree-sitter wasm feature). Pending: cargo xtask verify + force-push to update PR. Dev-setup improvements split to bd-tjbr; tree-sitter CRLF Windows failures split to bd-ntnx (pre-existing, not caused by #109).","created_at":"2026-04-24T17:13:49Z"}]} {"id":"bd-iuzmk","title":"q2 preview: set browser-tab title to ' (Quarto Preview)' from the active AST's meta.title","description":"Today the q2-preview SPA's browser tab always reads 'Quarto Preview' regardless of which doc is active. With a real doc title in YAML frontmatter we should surface it as ' (Quarto Preview)' so users with the live site + preview open side-by-side can tell tabs apart. Implementation: useEffect in q2-preview-spa/src/PreviewApp.tsx watching state.astJson, parse meta.title (handle MetaString / MetaInlines / bare Inlines), set document.title. Fall back to 'Quarto Preview' when no title. Acceptance: integration test asserting title updates on AST change; browser verification against ~/Desktop/daily-log/2026/05/15/q2-preview-test-website (should show 'Hello, world (Quarto Preview)').","status":"closed","priority":2,"issue_type":"feature","created_at":"2026-05-18T16:20:13.444289Z","created_by":"cscheid","updated_at":"2026-05-18T16:30:32.982406Z","closed_at":"2026-05-18T16:30:32.982262Z","close_reason":"Implementation complete: AST meta.title surfaces in document.title as '<title> (Quarto Preview)'. Verified end-to-end against the fixture website; live-edit case (retitle on disk → tab updates) also confirmed. 3 new SPA integration tests; cargo xtask verify 12/12 green.","source_repo":".","compaction_level":0,"original_size":0,"dependencies":[{"issue_id":"bd-iuzmk","depends_on_id":"bd-kw93","type":"parent-child","created_at":"2026-05-18T16:20:13.444289Z","created_by":"cscheid","metadata":"{}","thread_id":""}]} @@ -389,6 +392,7 @@ {"id":"bd-qpa2","title":"Display math column-strip uses wrong column source, mishandles inline-wrapped and labeled math (issue #181 follow-up)","description":"Follow-up to bd-q6ed / issue #181, with revised diagnosis after a second-pass investigation (see `claude-notes/issue-reports/181/triage.md` § \"Second-pass investigation\").\n\n## Symptom\n\nTwo reporter-supplied edge cases (rundel, 2026-05-12):\n\n1. **Labeled display math inside a blockquote** — `> $$\\n> p(x)\\n> $$ {#eq-p}` round-trips with a literal `> ` left in `Math.text` after the second pass.\n2. **Labeled display math at top level** — `$$\\na\\n b\\n$$ {#eq-x}` loses one column of leading whitespace from interior lines on each round trip.\n\nSubsequent probing revealed both are instances of a broader brittleness in the column-strip introduced by bd-q6ed:\n\n3. `_$$\\na\\n b\\n$$_` (emph wrapping multi-line displaymath) loses one space on **first** parse — no round trip needed.\n4. `> _$$\\n> a\\n> b\\n> $$_` (same inside a blockquote) leaks `> ` AND loses interior whitespace on first parse.\n\n## Root cause\n\n`strip_continuation_prefix` in `crates/pampa/src/pandoc/treesitter.rs` uses `node.start_position().column` (the column of `$$`) as the strip width. That column equals the cumulative block-continuation prefix width **only when `$$` is the leftmost non-prefix character on its line**. Anything that precedes `$$` on the same line (`_`, `**`, `[`, the writer's own `[` for `quarto-math-with-attribute` Spans, etc.) shifts the column while the body bytes' source layout does not change. The strip then either eats real content (when the prefix is all whitespace/`>`) or refuses to strip real prefix (when the column overshoots into real content).\n\n## Constraint\n\n`DisplayMath` must remain an inline AST node — there are large existing corpora with display math nested inside paragraphs, and the grammar restructure proposed in the original triage would break those documents.\n\n## Fix shape (reader-side, no grammar change, no AST shape change)\n\nChange the strip-width source from `math_node.start_position().column` to the start column of the **enclosing block-leaf ancestor** (`pandoc_paragraph` / `pandoc_plain`). The cumulative block-continuation prefix width equals the paragraph's start column, which is constant across all interior lines of the paragraph regardless of what precedes `$$` on the opening line.\n\nPandoc's markdown reader uses this exact strategy (verified empirically). The fix matches Pandoc's behaviour on all probed cases — including lazy-continuation, nested blockquotes, list-item indent, and inline-wrapped multi-line math. The existing conservative `bytes ∈ {>, space, tab}` guard in the helper continues to handle lazy continuation correctly.\n\nThis single-input change subsumes:\n- bd-q6ed canonical case (paragraph col = math col, identical result)\n- bd-qpa2 edges A and B (paragraph col reflects true bq/top-level width)\n- Inline-wrapped multi-line math at any block context\n\n## Regression coverage to add\n\nFixtures under `crates/pampa/tests/roundtrip_tests/qmd-json-qmd/`:\n\n- `labeled_display_math_in_blockquote.qmd` (bd-qpa2 edge A)\n- `labeled_display_math_top_level_indented.qmd` (bd-qpa2 edge B)\n- `emph_around_multiline_display_math.qmd`\n- `emph_around_multiline_display_math_in_blockquote.qmd`\n\nReference inputs preserved at `claude-notes/issue-reports/181/exp-labeled-math-in-bq.qmd` and `exp-labeled-math-toplevel.qmd`.\n\n## Worktree / branch\n\nImplementation on its own branch with plan file at `claude-notes/plans/2026-05-12-displaymath-column-strip-fix.md`. Upstream: https://github.com/quarto-dev/q2/issues/181.\n\n## Out of scope (separate issue if desired)\n\nThe writer's `[$$…$$]{attr}` emission for `quarto-math-with-attribute` Spans is suboptimal — Pandoc emits the natural form `$$\\n…\\n$$ {attr}` for the same AST. Switching to that form would improve readability and Pandoc compatibility, but with the reader fix above it is no longer required for round-trip correctness.","status":"open","priority":1,"issue_type":"bug","created_at":"2026-05-12T19:32:00.197200Z","created_by":"cscheid","updated_at":"2026-05-12T19:55:56.757118Z","source_repo":".","compaction_level":0,"original_size":0,"dependencies":[{"issue_id":"bd-qpa2","depends_on_id":"bd-q6ed","type":"related","created_at":"2026-05-12T19:32:00.197200Z","created_by":"cscheid","metadata":"{}","thread_id":""}]} {"id":"bd-r7v2","title":"Browser smoke: confirm MathJax/KaTeX typeset in real browser","description":"Phase 4.3 of bd-w5ov could not be completed in-session because chrome-devtools-mcp disconnected after a stale browser process was killed. Implementation is complete and Phase 4.2 (CLI exercise) verified the rendered HTML markup is correct. What remains: load the rendered fixtures in a real Chromium session and confirm the math is actually typeset, not just present as raw \\(x^2\\) text.\n\nAcceptance:\n- Render /tmp/q2-math-cli/inline_math.html, display_math.html, labelled_eq.html via http.server.\n- chrome-devtools-mcp: navigate, evaluate document.querySelectorAll('mjx-container').length > 0 (MathJax fixtures).\n- Render katex.html, evaluate document.querySelectorAll('.katex, .katex-display').length > 0.\n- Confirm zero console errors from the math runtime (404s on jsDelivr would fail this).\n- Record observations in claude-notes/plans/2026-05-04-math-mode.md.","status":"closed","priority":1,"issue_type":"task","created_at":"2026-05-04T23:58:12.775314Z","created_by":"cscheid","updated_at":"2026-05-05T00:04:57.347714Z","closed_at":"2026-05-05T00:04:57.347571Z","close_reason":"Browser smoke completed in same session as bd-w5ov implementation: 5 fixtures verified live in Chromium via chrome-devtools-mcp. MathJax 3.2.2 typesets inline + display + labelled equations from jsDelivr; KaTeX typesets via auto-render; math-free pages stay clean; user URL override honored. Recorded in plan §Browser smoke log.","source_repo":".","compaction_level":0,"original_size":0,"dependencies":[{"issue_id":"bd-r7v2","depends_on_id":"bd-w5ov","type":"discovered-from","created_at":"2026-05-04T23:58:12.775314Z","created_by":"cscheid","metadata":"{}","thread_id":""}]} {"id":"bd-r82e","title":"DocumentProfile: add includes: Vec<PathBuf> for incremental-rebuild invalidation","description":"Surfaced during the main -> feature/websites merge (bd-xfwx) that threaded IncludeExpansionStage before the DocumentProfile checkpoint.\n\nProblem: after the merge, DocumentProfile.outline (and any other AST-derived fields) can be populated from headings / code / crossref targets spliced in via {{< include child.qmd >}}. The profile therefore depends on the contents of every included file, but the profile struct itself records no trail back to those files.\n\nFor incremental rebuilds (Phase 8 of the websites epic, bd-*) and for the future 'freeze' feature, the cache-key computation needs to invalidate a parent document's cached profile when ANY of its (transitive) includes change. Without tracking the include set on the profile, that is impossible.\n\nProposal: add a field roughly of the form\n\n includes: Vec<PathBuf> // or Vec<IncludeEntry { path, hash }>\n\nto DocumentProfile. Populated by IncludeExpansionStage (or by DocumentProfileStage reading a side-channel set that IncludeExpansionStage populated on the DocumentAst). Bump profile_version on the serialized shape.\n\nScope:\n- Decide on the shape (bare PathBuf list vs. (path, content-hash) pairs). For Phase 8 we'll likely want the hash too so nav-state invalidation can compare without re-reading the files.\n- Populate from IncludeExpansionStage.\n- Extend contract doc (claude-notes/designs/document-profile-contract.md).\n- Tests: profile records every included file (direct + transitive); round-trip serialization.\n\nNon-blocking for the website-epic MVP; becomes a hard prerequisite for Phase 8.","status":"closed","priority":2,"issue_type":"feature","created_at":"2026-04-24T20:05:53.778909Z","created_by":"cscheid","updated_at":"2026-04-28T00:43:08.078191Z","closed_at":"2026-04-28T00:43:08.077941Z","close_reason":"Closed as part of Phase 8.0 — DocumentProfile.includes added (v2). Verified by editing_include_invalidates_parent_profile (incremental_rebuild.rs).","source_repo":".","compaction_level":0,"original_size":0,"labels":["websites"],"dependencies":[{"issue_id":"bd-r82e","depends_on_id":"bd-0tr6","type":"related","created_at":"2026-04-24T20:05:53.778909Z","created_by":"cscheid","metadata":"{}","thread_id":""},{"issue_id":"bd-r82e","depends_on_id":"bd-fegm","type":"blocks","created_at":"2026-04-27T22:21:01.025552Z","created_by":"cscheid","metadata":"{}","thread_id":""},{"issue_id":"bd-r82e","depends_on_id":"bd-xfwx","type":"related","created_at":"2026-04-24T20:05:53.778909Z","created_by":"cscheid","metadata":"{}","thread_id":""}]} +{"id":"bd-r8n4r","title":"CaptureSplice fold: handle engine-handoff cells nested in cell wrappers","description":"apply_capture_splices folds capture splices for a multi-engine preview, but splice_cells walks TOP-LEVEL blocks only. If engine A emits engine B's cell inside a Div.cell wrapper, engine B's splice won't reach it and that cell renders as raw source. The common case (each engine's cells at top level) works. v1 limitation documented in capture_splice.rs and the plan.","status":"open","priority":3,"issue_type":"task","created_at":"2026-05-27T16:24:40.170895Z","created_by":"cscheid","updated_at":"2026-05-27T16:24:40.170895Z","source_repo":".","compaction_level":0,"original_size":0,"dependencies":[{"issue_id":"bd-r8n4r","depends_on_id":"bd-5yff4","type":"discovered-from","created_at":"2026-05-27T16:24:40.170895Z","created_by":"cscheid","metadata":"{}","thread_id":""}]} {"id":"bd-r9hs","title":"Cargo: upgrade ureq v2.12.1 → v3.3.0","description":"Major upgrade surfaced by cargo-upgrade survey 2026-05-04. Current 2.12.1 is range-pinned in workspace; latest is 3.3.0. Type: major. Review changelog and bump deliberately. See claude-notes/plans/2026-05-04-cargo-upgrade-survey.md and bd-hb8h.","status":"closed","priority":3,"issue_type":"chore","created_at":"2026-05-04T18:15:55.424872Z","created_by":"cscheid","updated_at":"2026-05-04T20:30:45.584646Z","closed_at":"2026-05-04T20:30:45.584505Z","close_reason":"merged: b0ee0f18","source_repo":".","compaction_level":0,"original_size":0,"labels":["cargo","deps"],"dependencies":[{"issue_id":"bd-r9hs","depends_on_id":"bd-hb8h","type":"discovered-from","created_at":"2026-05-04T18:16:05.861885Z","created_by":"cscheid","metadata":"{}","thread_id":""}]} {"id":"bd-ra5j","title":"L5 hub-client browser smoke for categories sidebar","description":"L5 (bd-5vsr) shipped per-item category chips and the right-margin categories sidebar. Native CLI e2e was verified directly (see plan §'End-to-end CLI verification record'). Hub-client browser smoke was deferred — same call as L3.\n\nTest plan: build hub-client (cd hub-client && npm run build:all && npm run dev), open dev server, load a multi-listing fixture project (or hand-author one in the editor), confirm:\n- Each post block shows clickable category chips.\n- The right margin shows the categories sidebar with the expected pills + counts.\n- Clicking a chip or a sidebar pill filters the listing via list.js.\n\nNote: until bd-57y4 (vendor + integrate quarto-listing.scss) lands, the visuals will look bare (vertical stack of plain divs, no hover states, no cloud sizing). The functionality should still be correct — clicks fire window.quartoListingCategory(...) and list.js filters items. Visual parity is bd-57y4's job; this issue is just the functional smoke that markup + JS still glue correctly in a real browser.","status":"open","priority":3,"issue_type":"task","created_at":"2026-05-07T14:20:13.761736Z","created_by":"cscheid","updated_at":"2026-05-07T14:20:13.761736Z","source_repo":".","compaction_level":0,"original_size":0,"dependencies":[{"issue_id":"bd-ra5j","depends_on_id":"bd-5vsr","type":"discovered-from","created_at":"2026-05-07T14:20:13.761736Z","created_by":"cscheid","metadata":"{}","thread_id":""}]} {"id":"bd-rcdo","title":"InlineSplice silently drops header attribute changes","description":"When changing a Header's attributes (classes, key-value pairs) without changing its inline content, the incremental writer's InlineSplice path preserves the original block text verbatim — including the old attribute suffix. The new attributes are silently lost. Discovered while testing the kanban demo's setCardStatus mutation.","status":"closed","priority":1,"issue_type":"bug","created_at":"2026-02-11T14:11:27.114610Z","created_by":"cscheid","updated_at":"2026-02-11T14:17:36.513428Z","closed_at":"2026-02-11T14:17:36.513409Z","close_reason":"Fixed: block_attrs_eq checks classes, kvs, and explicit IDs. 9 tests, all 6402 workspace tests pass.","source_repo":".","compaction_level":0,"original_size":0} diff --git a/claude-notes/plans/2026-05-27-multi-engine-execution.md b/claude-notes/plans/2026-05-27-multi-engine-execution.md new file mode 100644 index 000000000..d4f50138a --- /dev/null +++ b/claude-notes/plans/2026-05-27-multi-engine-execution.md @@ -0,0 +1,672 @@ +# Sequential multi-engine execution + +**Issue:** bd-5yff4 — feature/design. + +**Status:** design / investigation. Implementation is gated on explicit +user go-ahead after this plan is reviewed and iterated. + +## Overview + +Today a Quarto 2 document declares exactly one execution engine: + +```yaml +engine: knitr +``` + +This plan investigates running **several engines in sequence** for one +document: + +```yaml +engine: + - knitr + - mermaidjs # hypothetical future diagram engine +``` + +The two coupled pieces of work are (1) a YAML-config change to accept an +ordered array of engines, and (2) a pipeline change to thread N engines +through the engine-execution stage. A third concern — tracing, replay, +and preview — must be redesigned because today's model assumes a single +engine per document. + +### Decisions locked with the user (2026-05-27) + +1. **Engines are distinct within the sequence; order is significant.** + The same engine never appears twice. Ordering matters because engine + N may emit code cells that engine N+1 consumes. → Replay captures can + stay **keyed by engine name** (the current registry model), but the + execution order and per-engine input must be preserved. +2. **Array merge uses the existing default (`!concat`).** We do *not* + introduce an engine-specific default merge op (e.g. `!prefer`). + Schema-specific tag defaults are deferred until we have a genuine + schema-driven merging system. Users opt into replacement with an + explicit `!prefer` tag. +3. **Validate with a simple file-backed test engine**, not a real second + engine. The test engine reads a results file (per-cell outputs, in + order) and splices them in — deterministic, dependency-free, and a + small useful design in its own right. A real second engine (e.g. + mermaidjs) is a separate follow-up. +4. **Trace records one snapshot per engine.** One AST snapshot **and** + one `EngineCapture` per engine invocation within the stage, mirroring + the existing `transform:<name>` sub-entry pattern — provided the + trace-format change stays tractable (it should; see §4). + +## Current architecture (what we're changing) + +### Engine detection — `crates/quarto-core/src/engine/detection.rs` + +`detect_engine(metadata) -> DetectedEngine` returns a **single** engine: + +```rust +pub struct DetectedEngine { + pub name: String, // singular + pub config: Option<ConfigValue>, +} +``` + +It handles three input shapes (`detection.rs:150-190`): +- `engine: knitr` (string) → `DetectedEngine::new("knitr")` +- `engine: { jupyter: { kernel: python3 } }` (map) → takes + `entries.first()` — the **first** key only +- top-level `jupyter:`/`knitr:` key (no `engine:` key) +- defaults to `markdown` + +There is **no array branch**. An `engine: [knitr, mermaidjs]` would fall +into the map branch, find no `.first()` map entry on a sequence, and fall +through to `markdown`. + +### Engine execution — `crates/quarto-core/src/stage/stages/engine_execution.rs` + +`EngineExecutionStage::run` (`engine_execution.rs:150-401`) does, once: + +1. `detect_engine(&doc_ast.ast.meta)` (single engine). +2. Markdown engine → early return, passthrough (`:191-198`). +3. `serialize_ast_to_qmd(&doc_ast.ast)` → `(qmd, source_info)` (`:201`). +4. `engine.execute(&qmd, &exec_context)` → `ExecuteResult` (`:230`). +5. Emit `EngineCapture` aux event (`:248-270`). +6. Accumulate `includes` + `supporting_files` onto `ctx` (`:273-297`). +7. Parse engine output back to AST against an intermediate filename + `<stem>.rmarkdown` (`:313-324`). +8. Build a **2-slot** merged `ASTContext`: slot 0 = original `.qmd` + (`FileId(0)`), slot 1 = intermediate `.rmarkdown` (`:326-362`). +9. Remap executed-AST `FileId(0)` → `FileId(1)` (`:368-370`). +10. `quarto_ast_reconcile::reconcile(doc_ast.ast, executed_ast)` (`:376`). + +The serialize → execute → parse → reconcile loop is **already +idempotent in type**: it consumes a `DocumentAst` and produces a +`DocumentAst`. Threading N engines is mechanically a `for` loop over that +body. The non-trivial part is the FileId/slot bookkeeping (step 8–9) and +the trace/replay model (steps 5). + +### Engine registry — `crates/quarto-core/src/engine/registry.rs` + +`EngineRegistry` is a `HashMap<String, Arc<dyn ExecutionEngine>>` keyed by +`engine.name()`. `register` is last-write-wins. `with_replay(capture)` +(`registry.rs:157`) registers a single `ReplayEngine` under the recorded +engine's name. Because names are the key and our sequences are +name-distinct, registering one `ReplayEngine` per engine works — but the +constructor only takes one capture today. + +### Engine trait — `crates/quarto-core/src/engine/traits.rs` + +```rust +pub trait ExecutionEngine: Send + Sync { + fn name(&self) -> &str; + fn execute(&self, input: &str, ctx: &ExecutionContext) + -> Result<ExecuteResult, ExecutionError>; + fn can_freeze(&self) -> bool { false } + fn intermediate_files(&self, _input_path: &Path) -> Vec<PathBuf> { Vec::new() } + fn is_available(&self) -> bool { true } +} +``` + +The trait is **per-invocation text→text** and needs no change to support +sequencing — the *stage* drives the loop, not the engine. + +### Metadata merge + tags — `crates/quarto-config/` + +`!prefer` / `!concat` are parsed in `quarto-config/src/tag.rs:135-181`. +The merge in `quarto-config/src/merged.rs:286-327` walks config layers +lowest→highest; arrays **concatenate by default** (`MergeOp::Concat`), +and `MergeOp::Prefer` clears prior items. `MetadataMergeStage` +(`crates/quarto-core/src/stage/stages/metadata_merge.rs:133`) assembles +the layers (project → extension → directory → document → runtime) and +materializes the merged config into `doc.ast.meta`. + +→ **An `engine:` array Just Works under the existing array-merge +machinery.** No merge-engine change is required; we only need to *read* +the merged array and enforce distinctness (see §1, "Duplicate handling"). + +### Trace / replay / preview — the single-capture assumption + +- `quarto-trace/src/lib.rs:111`: `TraceDocument.engine_capture: + Option<EngineCapture>` — **one** slot. +- `EngineCapture` = `{ engine_name, input_qmd, result }` + (`lib.rs:138-155`). +- Trace observer (`crates/quarto-core/src/stage/trace.rs:209-243`): + `on_auxiliary_data` routes the `EngineCapture` kind to the single + `engine_capture` slot. There is precedent for **intra-stage** + granularity: `on_transform_data` (`trace.rs:185-207`) emits a + `transform:<name>` `TraceEntry` per transform within + `AstTransformsStage`. +- Replay engine (`crates/quarto-core/src/engine/replay.rs`): validates + `input_qmd` byte-equality and returns the recorded `ExecuteResult`; + hard-fails on mismatch. +- Preview record (`crates/quarto-core/src/engine/preview_record.rs`): + `record_capture` runs the pipeline through `EngineExecutionStage`, + collects the **first** `EngineCapture` (`first-write-wins`, + `preview_record.rs:79-82`), returns `Option<EngineCapture>`. +- Preview cache (`crates/quarto-preview/src/cache.rs`): caches one + capture per doc keyed by SHA-256 of the canonical input QMD; the + browser-side WASM replays it. + +All four hinge on "one engine ⇒ one capture." Multi-engine breaks the +assumption. + +## Feasibility verdict + +**Feasible.** The type contract closes end-to-end (each engine consumes +and produces a `DocumentAst`), the merge system already concatenates +arrays, and the trace model has a precedent for intra-stage sub-entries. +The genuine design work is in three places: + +1. Reading + validating the engine array (distinctness, ordering). +2. Generalizing FileId/intermediate-slot bookkeeping from 2 → N+1. +3. Turning the single-capture trace/replay/preview path into an ordered + list, with per-engine snapshots. + +Each is addressed below, with the open design questions called out. + +--- + +## 1. YAML config: `engine:` as an ordered array + +### Accepted shapes (back-compat is mandatory) + +| YAML | Parsed sequence | +|------|-----------------| +| `engine: knitr` | `[knitr]` | +| `engine: { jupyter: { kernel: python3 } }` | `[jupyter+config]` | +| `engine: [knitr, mermaidjs]` | `[knitr, mermaidjs]` | +| `engine:`<br>` - knitr`<br>` - mermaidjs: { theme: dark }` | `[knitr, mermaidjs+config]` | +| top-level `jupyter:` (no `engine:`) | `[jupyter+config]` | +| (none) | `[markdown]` | + +Array elements may be a bare string or a single-key map (engine name → +config), matching the existing scalar/map forms element-wise. + +### New API + +Add `detect_engines(metadata) -> Vec<DetectedEngine>` alongside (or +replacing the internals of) `detect_engine`. Keep `detect_engine` as a +thin wrapper returning the first element for any remaining single-engine +call sites, or migrate all call sites — TBD during implementation. + +### Merge behavior — verified against the code (2026-05-27) + +`MetadataMergeStage` materializes the merged config via +`materialize_cursor` (`crates/quarto-config/src/materialize.rs:85`), +which calls `MergedCursor::as_value` +(`crates/quarto-config/src/merged.rs:238-256`). `as_value` walks layers +**highest-priority-first and returns on the first layer that defines the +key** — so the **topmost layer that sets `engine:` decides the kind**: + +| Project (lower) | Doc (higher) | Materialized `engine` | Detected sequence | +|---|---|---|---| +| `[knitr]` | `jupyter` (scalar) | scalar `jupyter` (array dropped) | `[jupyter]` | +| `jupyter` (scalar) | `[knitr]` (array) | `[knitr]` (scalar dropped) | `[knitr]` | +| `[jupyter]` | `[jupyter]` | `[jupyter, jupyter]` (concat) | dup → see below | +| `[knitr]` | `[mermaidjs]` | `[knitr, mermaidjs]` (concat) | `[knitr, mermaidjs]` | + +Two consequences, both confirmed by reading `as_array` +(`merged.rs:295-320`, which collects items **only** from layers whose +kind is `Array`, line 297): + +1. **Duplicates arise only in the array + array case.** Scalar/array + mismatches never produce a duplicate, because the topmost layer's + kind wins and a mismatched lower layer is dropped wholesale. The only + shape that repeats an engine is two array layers naming the same + engine (e.g. project `engine: [jupyter]` and a doc that restates + `engine: [jupyter]`). +2. **Gotcha to document:** because `as_array` drops lower *scalar* + layers, the "concat accumulates across layers" intuition holds **only + when every contributing layer uses array syntax.** A project-level + `engine: jupyter` (scalar) is silently discarded the instant any + higher layer writes `engine:` as an array. We surface this in user + docs. + +### Duplicate handling (resolved with user 2026-05-27) + +**Dedup keeping the first occurrence**, and emit a diagnostic naming any +dropped duplicate. This fires only for the array + array repeated-engine +case above. Erroring would be hostile to the benign "two array layers +both say jupyter" case. The cost: a cross-layer *reordering* attempt +(project `[knitr, jupyter]` + doc `[jupyter, knitr]` → deduped +`[knitr, jupyter]`) is silently normalized to first-seen order; +reordering requires an explicit `!prefer` to replace the list. Documented +as a v1 limitation. + +### Schema / validation + +There is currently **no Rust-side schema** validating `engine:` +(detection-only; unknown names fall back with a warning at the stage). +We keep that posture: the array branch accepts any element shape and +defers unknown-engine handling to the stage's existing +`get_engine_with_fallback`. No new schema work in this plan. + +--- + +## 2. Pipeline: thread N engines through `EngineExecutionStage` + +### Loop shape + +```text +engines = detect_engines(meta) // ordered, distinct +ast = doc_ast.ast +slots = [".qmd"] // FileId(0) +for (i, engine) in engines.enumerate(): + if engine.name == "markdown": continue // per-engine no-op skip + (qmd, source_info) = serialize_ast_to_qmd(ast) + emit pre-engine capture input (qmd) // for trace/replay + result = engine.execute(qmd, exec_ctx_for(engine, i)) + record EngineCapture { engine_name, input_qmd: qmd, result } + accumulate includes + supporting_files + executed_ast = parse(result.markdown, intermediate_name(i)) + merged_context.add_slot(intermediate_name(i)) // FileId(slots.len()) + remap executed_ast FileId(0) -> FileId(slots.len()-1 .. ) // see below + (ast, plan) = reconcile(ast, executed_ast) + emit per-engine AST snapshot (trace) // engine:<name> +final DocumentAst { ast, ast_context: merged_context, ... } +``` + +### FileId / intermediate-slot bookkeeping (the hard part) + +Today (`engine_execution.rs:326-370`): 2 fixed slots, executed AST's +`FileId(0)` remapped by `+1` to land in slot 1. For N engines we need +**N+1 slots** (original + one intermediate per non-markdown engine) and +the remap offset becomes "current slot count," not a constant `+1`. + +Invariant to preserve: a block's `FileId` identifies its provenance — +the `.qmd` for kept blocks, the relevant intermediate for blocks first +introduced by engine k. We must verify that `quarto_ast_reconcile`'s +keep/replace/recurse decisions remain correct when the "original" side +of the reconcile already carries FileIds from multiple prior slots. + +**Risk:** the second reconcile's "original" AST (output of the first +reconcile) mixes `FileId(0)` and `FileId(1)`. Remapping the second +engine's executed AST to `FileId(2)` must not collide. We add a dedicated +test (Phase 1) that runs two appending engines and asserts three distinct +FileIds land coherently. + +### Markdown skip + +Markdown engines anywhere in the sequence are individually skipped +(passthrough), preserving the current optimization. A sequence of only +markdown engines collapses to today's no-op fast path. + +### Includes / supporting files + +Already additive (`extend` / `add_engine_files`). Accumulating across +engines needs no change beyond running inside the loop. + +### Determinism requirement (load-bearing for replay) + +Engine k+1's `input_qmd` is the serialization of the AST *after* engine +k's reconcile. For replay to validate (byte-equality on `input_qmd`), +serialize→reconcile→serialize must be **deterministic** across runs. This +is already assumed by single-engine replay; we extend the assumption and +add a regression test that records a two-engine trace and replays it +byte-clean. + +--- + +## 3. Test engine: file-backed cell-results splicer + +Per decision 3 — a deterministic, dependency-free engine to exercise +sequencing. Proposed contract: + +- **Name:** `fixture` (working name; bikeshed later). +- **Config:** `engine: { fixture: { results: results.json } }` — a path + (relative to the doc) to an ordered list of cell results. +- **Behavior:** walk the input QMD's executable code cells in document + order; for cell i, splice the i-th entry from the results file as the + cell's output (as Quarto already represents engine output — e.g. an + output block / div following the source). Cells beyond the results + list pass through unchanged; surplus results are a diagnostic. + +To exercise **engine→engine handoff** (engine A emits a cell that engine +B executes), the test fixtures use two registrations of the file-backed +engine under two distinct names (e.g. `fixture-a`, `fixture-b`), where +`fixture-a`'s results include a fenced `{fixture-b}` cell that +`fixture-b` then fills. This proves the "engine N produces cells for +engine N+1" requirement without any real runtime. + +This belongs in `crates/quarto-core/src/engine/` next to `markdown.rs` +and `replay.rs`. **Registration is test-registry-only** (decision +2026-05-27): the engine is constructed in tests via +`EngineRegistry::register`, never wired into the default registry, and +never user-selectable in a real render. + +**Not a freeze mechanism.** The file-backed engine resembles Quarto 1's +freeze, but Quarto 2's freeze will instead reuse the **trace** directly — +roughly "`engine: replay` as freeze": commit a trace file into the repo +and flag Quarto to replay its recorded `ExecuteResult` instead of +running the engine. So the test engine carries no freeze responsibility; +it exists purely to make multi-engine sequencing testable without R / +Python / Jupyter. + +--- + +## 4. Trace / replay / preview redesign + +### Trace schema (one capture + one snapshot per engine) + +- `quarto-trace`: replace `engine_capture: Option<EngineCapture>` with + **`engine_captures: Vec<EngineCapture>`** (ordered by execution). + - Back-compat: readers fold a legacy single `engine_capture` into a + one-element vec; writers emit the vec. Keep `#[serde(default, + skip_serializing_if = "Vec::is_empty")]`. + - Bump `SCHEMA_VERSION` 2 → 3 and document; readers stay + forward/backward tolerant (unknown fields ignored). +- Per-engine AST snapshot: emit an `engine:<name>` `TraceEntry` after + each engine's reconcile, **exactly mirroring** `on_transform_data` → + `transform:<name>` (`trace.rs:185-207`). This is the precedent that + makes "one snapshot per engine" *not* a big change: we add an + `on_engine_data(name, index, ast, ast_context)` observer hook (or + reuse the transform hook with an `engine:` prefix) and call it from the + stage loop. The dedup pass (bd-5qnj) collapses identical snapshots, so + the size cost is bounded. + +### Observer / capture plumbing + +- `engine_execution.rs` emits one `EngineCapture` aux event **per + engine** (already a loop after the change). The `index` field + distinguishes them. +- `JsonTraceObserver::on_auxiliary_data` (`trace.rs:215`) pushes onto the + `engine_captures` vec instead of overwriting the single slot. + +### Replay + +- `EngineRegistry::with_replay(capture)` → `with_replay_many(captures: + Vec<EngineCapture>)`: register one `ReplayEngine` per capture, keyed by + `engine_name` (distinct names ⇒ no collision — decision 1). +- The stage loop drives each engine; each `ReplayEngine` validates its + own `input_qmd`. Order is implied by the engine sequence in `meta`, + which must match the recording. +- A replay miss on **any** engine surfaces as a stage error (extend the + existing single-engine behavior, `replay.rs`). + +### Preview (`q2 preview`) — in scope + +> **Design correction (2026-05-27, discovered during Phase 3→4):** the +> WASM preview path does **not** use `ReplayEngine`/`with_replay`. Per +> bd-lucp (`claude-notes/plans/2026-05-18-q2-preview-project-replay-engine.md`), +> the browser threads the capture into **`CaptureSpliceStage`** +> (`crates/quarto-core/src/engine/capture_splice.rs`), which treats the +> capture as a recipe — `derive_cell_outputs(A1, B1)` then `splice_cells` +> match engine cells by `(content-hash, occurrence)` and replace them +> with the recorded `Div.cell` output. `EngineExecutionStage` is bypassed +> in preview. So the Phase-4 plan below is revised accordingly. + +`q2 preview` support ships **with** this feature, not after it. The +preview flow has two parts; both are in scope: + +1. **Capture the sequence + splice it in the browser** (the part that + makes preview *correct* for multi-engine docs): + - `record_capture` (`preview_record.rs:130`) returns + `Vec<EngineCapture>`; `CaptureCollector` collects **all** captures + in order instead of first-write-wins (`preview_record.rs:79`). + - `CaptureSpliceStage` **folds** the captures in order: + `a = a2; for cap in captures { a = apply_capture_splice(a, + parse(cap.input_qmd), parse(cap.result.markdown), cap.engine_name) }`. + Splice 1 turns engine-A cells into A's output; splice 2 turns + engine-B cells (authored, or produced by A) into B's output. + - **Known edge (follow-up):** `splice_cells` walks *top-level* blocks + only. If engine A emits engine-B's cell *inside* a `Div.cell` + wrapper, the fold won't reach it. The common case (each engine's + cells at top level) works. Document as a v1 limitation. + - Transport: the capture binary doc + (`capture_driver.rs` / `CAPTURE_MIME_TYPE`) payload becomes a JSON + **array** of captures (was a single object); the sidecar still + references one doc per file. WASM `parse_capture_from` deserializes + a `Vec<EngineCapture>`. hub-client TS likely needs no change (it + ships opaque bytes; only the WASM parse shape changes) — confirm. + +2. **Incremental capture cache** (the part that makes preview *fast* + across edits — `quarto-preview/src/cache.rs`'s `record_capture_cached` + keys a capture by SHA-256 of the canonical input QMD so unchanged code + cells don't re-run the engine). For a sequence this becomes: + - store/serve the **ordered vec** of captures per doc; + - staleness: engine 1's input is the doc's canonical QMD (today's + key); engines 2..N consume *derived* inputs (each is a + deterministic function of the prior engine's reconciled output), so + a single key on the doc's canonical QMD invalidates the whole + sequence correctly. We confirm this during implementation; if a + finer per-engine input vector is needed it's a localized change. + + ("Cache sequencing" earlier referred to *this* incremental + optimization — not to preview support itself.) + +### "Overwhelmingly complex?" assessment + +No. The vec-ification of `engine_capture` is additive + a schema bump; +the per-engine snapshot reuses the `transform:<name>` mechanism +verbatim; `with_replay_many` is small; the preview collector change is +collecting a vec instead of first-write-wins. The only piece with any +unknowns is the incremental-cache staleness model for sequences, and +even there the deterministic-derivation argument suggests the existing +doc-keyed invalidation already covers it. + +--- + +## Phased work plan (TDD) + +> Phases are ordered so each ends green. No implementation starts until +> the user approves this plan. + +### Phase 0 — Test scaffolding +- [x] Design + land the **file-backed test engine** (`FixtureEngine`, + `crates/quarto-core/src/engine/fixture.rs`) with 15 unit tests: + splices per-cell results in order; in-memory + JSON-file-backed + results; ignores other engines' cells / display blocks; skips + content inside non-matching fences; engine→engine handoff + (result introduces the next engine's cell); surplus/missing/ + unterminated diagnostics; longer-fence round-trip. Gated to + non-WASM; **never** wired into the default registry. Verified the + cell form against pampa: executable cells serialize as + ```` ```{<name>} ```` (braces kept inside the class name). +- [x] Duplicate-handling policy: **dedup keeping first occurrence + + diagnostic** (resolved with user; only fires for array+array + repeated engine). +- [x] Test-engine registration scope: **test-registry-only** (resolved + with user; Q2 freeze will use trace-replay, not this engine). + +### Phase 1 — Pipeline threading (single → N engines) ✅ +- [x] **Failing test first:** `test_two_engines_run_in_sequence_with_handoff` + (`fixture-a` emits a `{fixture-b}` cell that `fixture-b` fills). + Confirmed it failed pre-implementation (array detected as markdown). +- [x] `detect_engines() -> Vec<DetectedEngine>` + `detect_engine_sequence() + -> EngineSequence` with the array branch (bare names and single-key + maps) + back-compat for string/map/top-level forms. `detect_engine` + is now a first-of-sequence shim. 13 detection unit tests. +- [x] Generalized `EngineExecutionStage::run` to loop over the sequence; + merged `ASTContext` grows one intermediate slot per engine; FileId + remap offset is the running file count (`merged_context.filenames. + len()`), strictly safer than the old hardcoded `+1`. Per-engine + intermediate label `<stem>.<engine>.rmarkdown`. +- [x] **FileId coherence test:** `test_two_engines_assign_distinct_file_ids` + — three distinct FileIds (0=.qmd, 1=engine-a, 2=engine-b), no strays. +- [x] Post-merge duplicate dedup + diagnostic: + `test_duplicate_engine_dedups_and_warns` (and detection-level dedup + tests). +- [x] Markdown-in-sequence skip + all-markdown fast path: + `test_markdown_in_sequence_is_skipped` (+ existing markdown + passthrough tests still green). Full `quarto-core` suite: 2147 pass. +- [x] Updated two existing single-engine tests for the new per-engine + intermediate label (no snapshots pinned `.rmarkdown`). + +### Phase 2 — YAML merge integration ✅ +- [x] Tests (`crates/quarto-core/tests/engine_merge.rs`, 7 cases): + `engine:` array merges via `!concat` default across project/dir/doc + layers (2- and 3-layer); `!prefer` replaces; scalar-over-array and + array-over-scalar kind resolution; cross-layer duplicate dedup + + reported drop; lone-scalar back-compat. Exercises the production + `MergedConfig::materialize()` → `detect_engine_sequence` path — the + executable form of the plan's verified-behavior table. **No + merge-engine code change was needed** (arrays already concat). +- [ ] Full-render E2E with layered config → folded into **Phase 5**. + Note: the `q2` binary cannot use the test-only `FixtureEngine`, so + binary-level E2E uses a single-element array (`engine: [<real>]`) + with an available engine; the multi-engine *threading* is verified + end-to-end through the real HTML pipeline with a fixture registry. + +### Phase 3 — Trace / replay redesign ✅ +- [x] `quarto-trace`: `engine_capture: Option` → `engine_captures: Vec`. + **Kept `SCHEMA_VERSION = 2`** per the project rule (bump only on + pipeline-entry-shape changes; this is an additive top-level field). + Back-compat: a `TraceDocumentDe` deserialization mirror folds a + legacy single `engine_capture` object into the vec on read (covers + every read path, not just `read_trace`). Round-trip + legacy-fold + tests in `roundtrip.rs`. +- [x] Per-engine `engine:<name>` AST snapshot via a new + `PipelineObserver::on_engine_data` hook (mirrors `on_transform_data` + → `transform:<name>`); `JsonTraceObserver` records one entry per + engine. The stage calls it after each reconcile. +- [x] `on_auxiliary_data` pushes captures onto the vec (one per engine, + run_index order). `EngineRegistry::with_replay_many(Vec)`; + `with_replay` kept as a single-capture shim. CLI/`render_to_file` + replay path generalized (`load_replay_captures`, + `replay_captures: Vec`). +- [x] Tests: `test_multi_engine_trace_records_per_engine_snapshots_and_captures` + (two ordered captures + `engine:fixture-a`/`engine:fixture-b` + snapshots, gzipped round-trip) and + `test_multi_engine_record_then_replay_is_byte_clean` (the + determinism invariant: record two-engine run, replay via + `with_replay_many`, output identical). Full workspace: 9482 pass. + +### Phase 4 — Preview integration (`q2 preview`) ✅ +Revised after the CaptureSpliceStage discovery (see §4): +- [x] `CaptureSpliceStage` holds `Vec<EngineCapture>` and **folds** them + in order (`apply_capture_splices`); the stage parses + applies each + capture in sequence, fail-soft per capture. Unit tests: two-engine + fold (`{r}`→R1 then `{python}`→P1), empty-sequence identity, + the + documented top-level-only limitation. +- [x] `record_capture` / `record_capture_cached` → `Vec<EngineCapture>`; + `CaptureCollector` collects all in execution order. +- [x] Capture binary-doc payload → JSON **array** (`capture_driver.rs` + `write_capture_doc` / `read_capture_from_doc`); per-doc cache file + → array (`cache.rs`); both with a lenient single-object fallback + for stale docs. `re_execute.rs` updated. Staleness keys on the + **first** engine's `input_qmd` (deterministic-derivation argument). +- [x] WASM `parse_capture_from` → `Vec<EngineCapture>` (array + single + fallback); `RenderToPreviewAstRenderer::with_captures`; + `build_q2_preview_pipeline_stages` / `render_qmd_to_preview_ast` + thread the vec; all WASM call sites updated. +- [x] hub-client TS / q2-preview SPA: **no change needed** — confirmed + `PreviewApp.tsx` passes `binaryDoc.content` as opaque bytes; the + `CaptureRef` sidecar (one doc per file) is unchanged. +- [x] Full `cargo xtask verify` (WASM rebuild + SPA bundle + hub-client + build + Rust/hub tests) — **green**. Workspace: 9484 tests pass. +- [ ] **Verification gap (explicit):** a *browser* E2E of multi-engine + preview was not run — the preview server uses real engines, the + test-only `FixtureEngine` isn't available there, and knitr/jupyter + don't compose cleanly. The single-engine preview path is unchanged + in behavior (folding a one-element vec ≡ the old single-capture + splice) and the fold is unit-tested; the transport round-trips are + covered by `quarto-preview` integration tests. + +### Phase 5 — End-to-end verification +- [x] **Real-binary E2E of the array config (2026-05-27).** Through the + actual `q2` CLI with R/knitr installed: + - `cargo run --bin q2 -- render mengine-e2e/array-engine.qmd` with + `engine:\n - knitr` and an `{r}` cell `1 + 41` → output HTML + contains `<code>[1] 42</code>`, prose preserved. Proves the + **array** `engine:` form flows through real CLI arg parsing, + metadata merge, detection, and a real engine — inspected the HTML + directly. + - Scalar back-compat: `engine: knitr` with `2 * 21` → `[1] 42`. + - Two real engines (`engine: [knitr, jupyter]`, `{r}` + `{python}`) + is confounded by **knitr claiming `{python}` cells** (errors + without reticulate; with `python.reticulate: false` knitr runs + python itself and jupyter sees no cell). This is engine + cell-ownership — the "non-intuitive multi-engine behavior" we + anticipated — **not** a threading bug. Clean N-engine threading + is proven by the fixture-engine integration tests, which control + cell ownership precisely. +- [x] `cargo xtask verify` (full, incl. WASM + hub-client build) — green + after Phases 1–3. +- [x] Re-run full `cargo xtask verify` after Phase 4 — green. +- [x] Single-engine trace/replay/preview parity preserved: all existing + single-capture tests pass unchanged; the splice/replay paths fold a + one-element vec identically to the prior single-capture behavior. + +### Phase 6 — Commit (await explicit push approval per CLAUDE.md) +- [x] All work committed on `feature/multi-engine` (8 commits). +- [ ] **Push gated on explicit user approval** (CLAUDE.md GIT PUSH POLICY). + +## Follow-ups filed (discovered-from bd-5yff4) +- **bd-iq0hp** — Multi-engine preview: browser E2E + composing test + engines (the verification gap). +- **bd-r8n4r** — CaptureSplice fold: handle engine-handoff cells nested + in cell wrappers (top-level-only splice limitation). +- **bd-8h3sn** — Precise cross-engine source attribution in + EngineExecutionStage (engine 2+ error mapping is best-effort). + +## Status: Phases 0–5 complete + +Implemented and verified: ordered `engine:` array config + merge, +sequential N-engine threading with per-engine FileId provenance, per-engine +trace captures + snapshots, sequence replay, and multi-engine `q2 preview` +(capture-sequence fold). Full `cargo xtask verify` green; real-binary E2E +of the array config confirmed with knitr. Remaining: the browser-E2E gap +(bd-iq0hp) and two minor limitations (bd-r8n4r, bd-8h3sn), all tracked. + +## Resolved with the user (2026-05-27) + +1. **Duplicate handling** (§1): dedup keeping first occurrence + + diagnostic. Verified this only ever fires for the array+array + repeated-engine case; scalar/array mismatches can't produce a + duplicate. +2. **Test-engine registration** (§3): test-registry-only. Q2 freeze will + be trace-replay-based ("`engine: replay` as freeze"), so the test + engine has no freeze role. +3. **Real second engine:** mermaidjs / any concrete engine is **out of + scope** here; separate follow-up. +4. **Preview** (§4): `q2 preview` support ships **with** this feature. + "Cache sequencing" referred only to the incremental capture-cache + optimization, not to preview support; both are in scope, with the + incremental cache the only piece carrying minor unknowns. + +## Remaining open questions + +_None blocking. Surface during implementation if the FileId-coherence +reconcile (§2) or the incremental-cache staleness model (§4) turns out +harder than the analysis suggests._ + +## Out of scope + +- A real second engine implementation (mermaidjs etc.) — follow-up. +- Parallel engine execution (engines run strictly in sequence). +- Schema-driven / key-specific merge-tag defaults — deferred until a + schema-driven merging system exists (user decision 2). +- Cross-document engine sequencing concerns beyond a single doc. + +## Key source references + +- Engine detection: `crates/quarto-core/src/engine/detection.rs:150` +- Engine stage: `crates/quarto-core/src/stage/stages/engine_execution.rs:150` +- Registry / replay seam: `crates/quarto-core/src/engine/registry.rs:157` +- Engine trait: `crates/quarto-core/src/engine/traits.rs` +- Replay engine: `crates/quarto-core/src/engine/replay.rs` +- Preview record: `crates/quarto-core/src/engine/preview_record.rs:130` +- Preview cache: `crates/quarto-preview/src/cache.rs` +- Trace schema: `crates/quarto-trace/src/lib.rs:91` (`TraceDocument`), + `:138` (`EngineCapture`) +- Trace observer + `transform:<name>` precedent: + `crates/quarto-core/src/stage/trace.rs:185` (`on_transform_data`), + `:209` (`on_auxiliary_data`) +- Merge tags: `crates/quarto-config/src/tag.rs:135` +- Array merge: `crates/quarto-config/src/merged.rs:286` +- Metadata merge stage: + `crates/quarto-core/src/stage/stages/metadata_merge.rs:133` +- Pipeline builder + stage order: + `crates/quarto-core/src/pipeline.rs:242` (engine stage at `:288`) diff --git a/crates/quarto-core/src/engine/capture_splice.rs b/crates/quarto-core/src/engine/capture_splice.rs index 4bf393b68..4381acdee 100644 --- a/crates/quarto-core/src/engine/capture_splice.rs +++ b/crates/quarto-core/src/engine/capture_splice.rs @@ -302,6 +302,28 @@ pub fn apply_capture_splice(a2: Pandoc, a1: &Pandoc, b1: &Pandoc, engine_name: & splice_cells(a2, &map, engine_name) } +/// Fold a **sequence** of capture splices onto `a2`, in order (bd-5yff4). +/// +/// Multi-engine preview: engine 1's recorded output is spliced first, +/// then engine 2's splice runs on the *result* of engine 1's splice, and +/// so on — mirroring how the engines ran server-side, each consuming the +/// previous engine's output. Each tuple is `(A1, B1, engine_name)` for +/// one engine (the parsed `capture.input_qmd`, `capture.result.markdown`, +/// and `capture.engine_name`). +/// +/// Like a single splice, this is fail-soft per engine: a capture whose +/// cells don't match leaves those cells as raw source. **Limitation:** +/// `splice_cells` walks top-level blocks only, so if engine 1 emits +/// engine 2's cell *inside* a `Div.cell` wrapper, engine 2's splice +/// won't reach it (the cell renders as raw source). The common case — +/// each engine's cells at top level — folds correctly. +pub fn apply_capture_splices(mut a2: Pandoc, splices: &[(Pandoc, Pandoc, String)]) -> Pandoc { + for (a1, b1, engine_name) in splices { + a2 = apply_capture_splice(a2, a1, b1, engine_name); + } + a2 +} + #[cfg(test)] mod tests { use super::*; @@ -532,6 +554,47 @@ mod tests { assert!(matches!(out.blocks[1], Block::CodeBlock(_))); } + #[test] + fn two_engine_fold_splices_both_engines_cells() { + // bd-5yff4: A2 has an `{r}` cell and a `{python}` cell. Capture 1 + // (knitr) maps the `{r}` cell → R1; capture 2 (jupyter) maps the + // `{python}` cell → P1. Folding both splices must replace both + // cells with their respective wrappers. + let r_cell = code_cell("r", "cat('hi')"); + let py_cell = code_cell("python", "print('yo')"); + let a2 = pandoc_of(vec![r_cell.clone(), py_cell.clone()]); + + // Capture 1: knitr ran first. Its A1 is the original (both cells); + // its B1 turned the `{r}` cell into R1 but left `{python}` as a + // code cell (knitr doesn't own it here). + let cap1_a1 = pandoc_of(vec![r_cell.clone(), py_cell.clone()]); + let cap1_b1 = pandoc_of(vec![cell_wrapper("R1"), py_cell.clone()]); + + // Capture 2: jupyter ran second, on knitr's output. Its A1 has the + // `{python}` cell (R1 wrapper is prose-like to it); its B1 turned + // the `{python}` cell into P1. + let cap2_a1 = pandoc_of(vec![cell_wrapper("R1"), py_cell.clone()]); + let cap2_b1 = pandoc_of(vec![cell_wrapper("R1"), cell_wrapper("P1")]); + + let splices = vec![ + (cap1_a1, cap1_b1, "knitr".to_string()), + (cap2_a1, cap2_b1, "jupyter".to_string()), + ]; + let out = apply_capture_splices(a2, &splices); + + assert_eq!(out.blocks.len(), 2); + assert_eq!(first_div_marker(&out.blocks[0]), Some("R1")); + assert_eq!(first_div_marker(&out.blocks[1]), Some("P1")); + } + + #[test] + fn empty_splice_sequence_is_identity() { + let a2 = pandoc_of(vec![prose("hi"), code_cell("r", "x")]); + let out = apply_capture_splices(a2.clone(), &[]); + assert_eq!(out.blocks.len(), 2); + assert!(matches!(out.blocks[1], Block::CodeBlock(_))); + } + #[test] fn plain_language_tag_without_braces_is_not_an_engine_cell() { // ```r — display-only — has classes ["r"] (no braces) and diff --git a/crates/quarto-core/src/engine/detection.rs b/crates/quarto-core/src/engine/detection.rs index 70f477cbf..b62f21881 100644 --- a/crates/quarto-core/src/engine/detection.rs +++ b/crates/quarto-core/src/engine/detection.rs @@ -148,32 +148,89 @@ fn extract_string_value(value: &ConfigValue) -> Option<&str> { /// assert_eq!(detected.name, "markdown"); /// ``` pub fn detect_engine(metadata: &ConfigValue) -> DetectedEngine { - // Case 1: Look for explicit "engine" key - if let Some(engine_value) = metadata.get("engine") { - // Case 1a: engine: markdown|knitr|jupyter (string value) - if let Some(name) = extract_string_value(engine_value) { - // Return the engine name even if unknown - the pipeline stage - // will handle fallback and warning for unknown engines - return DetectedEngine::new(name); - } + // Back-compat shim: the first engine of the detected sequence (or the + // default markdown engine). Single-engine call sites that predate + // multi-engine support (bd-5yff4) keep their exact behavior because + // the string/map/top-level forms each yield a one-element sequence. + detect_engines(metadata) + .into_iter() + .next() + .unwrap_or_default() +} - // Case 1b: engine: { knitr: ... } or engine: { jupyter: ... } - if let Some(entries) = engine_value.as_map_entries() { - // The first key should be the engine name - if let Some(first_entry) = entries.first() { - let engine_name = &first_entry.key; +/// Interpret a single engine specifier — either the whole `engine:` +/// value or one element of an `engine:` array — as a [`DetectedEngine`]. +/// +/// - A bare string is the engine name (`engine: knitr`, or `- knitr`). +/// - A single-key map is `name: config` +/// (`engine: { jupyter: { kernel: python3 } }`, or +/// `- mermaidjs: { theme: dark }`). The first key wins, matching the +/// long-standing single-engine behavior. +/// +/// Returns `None` for shapes that name no engine (e.g. an empty map), +/// so callers can skip them. +fn detected_from_value(value: &ConfigValue) -> Option<DetectedEngine> { + // String form first (also covers Path/Glob/Expr and single-Str + // PandocInlines via `extract_string_value`). + if let Some(name) = extract_string_value(value) { + return Some(DetectedEngine::new(name)); + } + // Single-key map form: `name: config`. + if let Some(entries) = value.as_map_entries() { + if let Some(first_entry) = entries.first() { + return Some(DetectedEngine::with_config( + first_entry.key.clone(), + first_entry.value.clone(), + )); + } + } + None +} - // Return even if unknown - the pipeline stage will - // handle fallback and warning for unknown engines - return DetectedEngine::with_config(engine_name.clone(), first_entry.value.clone()); +/// Detect the **ordered** execution-engine sequence from document +/// metadata (bd-5yff4). +/// +/// Accepted `engine:` shapes, in addition to the single-engine forms +/// [`detect_engine`] documents: +/// +/// ```yaml +/// engine: [knitr, mermaidjs] # ordered array of names +/// engine: +/// - knitr +/// - mermaidjs: { theme: dark } # array elements may carry config +/// ``` +/// +/// Order is significant — engine N may emit code cells that engine N+1 +/// consumes. The returned sequence may contain **duplicates**; callers +/// that require distinct engines should go through +/// [`detect_engine_sequence`], which de-duplicates and reports the drops. +/// +/// Unknown engine names are returned as-is; the pipeline stage handles +/// fallback and warnings. An `engine:` array that names no engines (empty +/// or all-unparseable) falls through to the same default as no `engine:` +/// key at all. +pub fn detect_engines(metadata: &ConfigValue) -> Vec<DetectedEngine> { + // Explicit `engine:` key. + if let Some(engine_value) = metadata.get("engine") { + // Array form: ordered, may mix bare names and single-key maps. + if let Some(items) = engine_value.as_array() { + let engines: Vec<DetectedEngine> = + items.iter().filter_map(detected_from_value).collect(); + if !engines.is_empty() { + return engines; } + // Array named no engines: fall through to the default below + // (mirrors the pre-array behavior for a non-string, + // non-map `engine:` value). + } else if let Some(detected) = detected_from_value(engine_value) { + // Scalar string or single-key map: a one-element sequence. + return vec![detected]; } } - // Case 2: Look for engine-specific top-level keys - // This handles cases like: - // jupyter: - // kernel: python3 + // Engine-specific top-level keys (e.g. `jupyter:` / `knitr:` with no + // `engine:` key). Single engine only — this shorthand has no array + // form. for engine_name in KNOWN_ENGINES { // Skip "markdown" - it doesn't have a top-level config key if *engine_name == "markdown" { @@ -181,12 +238,56 @@ pub fn detect_engine(metadata: &ConfigValue) -> DetectedEngine { } if let Some(config) = metadata.get(engine_name) { - return DetectedEngine::with_config(engine_name.to_string(), config.clone()); + return vec![DetectedEngine::with_config( + engine_name.to_string(), + config.clone(), + )]; } } // Default: markdown engine (no execution) - DetectedEngine::default() + vec![DetectedEngine::default()] +} + +/// An ordered, de-duplicated engine sequence plus the names of any +/// duplicate engines that were dropped. +/// +/// Engines in a sequence must be **distinct** (decision in the +/// multi-engine plan): order is significant, so a repeated engine is +/// ambiguous. Duplicates arise most often from the `!concat` array-merge +/// default when two config layers each name the same engine (e.g. a +/// project `engine: [jupyter]` and a document that restates +/// `engine: [jupyter]`). We keep the **first** occurrence and record the +/// rest in [`dropped_duplicates`](EngineSequence::dropped_duplicates) so +/// the stage can emit a diagnostic. To intentionally replace a sequence +/// (including reordering) across layers, use the `!prefer` merge tag. +#[derive(Debug, Clone)] +pub struct EngineSequence { + /// The engines to run, in order, de-duplicated (first occurrence + /// kept). + pub engines: Vec<DetectedEngine>, + /// Names of duplicate engines that were dropped, in the order they + /// were encountered. Empty in the common case. + pub dropped_duplicates: Vec<String>, +} + +/// Detect the engine sequence and de-duplicate it (keeping the first +/// occurrence of each engine name). See [`EngineSequence`]. +pub fn detect_engine_sequence(metadata: &ConfigValue) -> EngineSequence { + let mut seen = std::collections::HashSet::new(); + let mut engines = Vec::new(); + let mut dropped_duplicates = Vec::new(); + for detected in detect_engines(metadata) { + if seen.insert(detected.name.clone()) { + engines.push(detected); + } else { + dropped_duplicates.push(detected.name); + } + } + EngineSequence { + engines, + dropped_duplicates, + } } #[cfg(test)] @@ -403,4 +504,122 @@ mod tests { let detected = detect_engine(&meta); assert_eq!(detected.name, "markdown"); } + + // === bd-5yff4: multi-engine detection (detect_engines) === + + fn array_config(items: Vec<ConfigValue>) -> ConfigValue { + ConfigValue::new_array(items, SourceInfo::default()) + } + + #[test] + fn test_detect_engines_single_string_backcompat() { + // engine: knitr → one-element sequence + let meta = map_config(vec![("engine", string_config("knitr"))]); + let engines = detect_engines(&meta); + assert_eq!(engines.len(), 1); + assert_eq!(engines[0].name, "knitr"); + } + + #[test] + fn test_detect_engines_array_of_strings_preserves_order() { + // engine: [knitr, mermaidjs] + let meta = map_config(vec![( + "engine", + array_config(vec![string_config("knitr"), string_config("mermaidjs")]), + )]); + let engines = detect_engines(&meta); + let names: Vec<_> = engines.iter().map(|e| e.name.as_str()).collect(); + assert_eq!(names, vec!["knitr", "mermaidjs"]); + assert!(engines.iter().all(|e| e.config.is_none())); + } + + #[test] + fn test_detect_engines_array_element_with_config() { + // engine: + // - knitr + // - mermaidjs: { theme: dark } + let mermaid_cfg = map_config(vec![("theme", string_config("dark"))]); + let meta = map_config(vec![( + "engine", + array_config(vec![ + string_config("knitr"), + map_config(vec![("mermaidjs", mermaid_cfg)]), + ]), + )]); + let engines = detect_engines(&meta); + let names: Vec<_> = engines.iter().map(|e| e.name.as_str()).collect(); + assert_eq!(names, vec!["knitr", "mermaidjs"]); + assert!(engines[0].config.is_none()); + assert!(engines[1].config.is_some()); + assert!(engines[1].config.as_ref().unwrap().get("theme").is_some()); + } + + #[test] + fn test_detect_engines_empty_array_defaults_to_markdown() { + let meta = map_config(vec![("engine", array_config(vec![]))]); + let engines = detect_engines(&meta); + assert_eq!(engines.len(), 1); + assert_eq!(engines[0].name, "markdown"); + } + + #[test] + fn test_detect_engines_map_form_still_single() { + // engine: { jupyter: { kernel: python3 } } stays one engine + let jupyter_cfg = map_config(vec![("kernel", string_config("python3"))]); + let meta = map_config(vec![("engine", map_config(vec![("jupyter", jupyter_cfg)]))]); + let engines = detect_engines(&meta); + assert_eq!(engines.len(), 1); + assert_eq!(engines[0].name, "jupyter"); + assert!(engines[0].config.is_some()); + } + + #[test] + fn test_detect_engine_backcompat_returns_first_of_array() { + // The singular shim returns the first engine of the sequence. + let meta = map_config(vec![( + "engine", + array_config(vec![string_config("knitr"), string_config("mermaidjs")]), + )]); + assert_eq!(detect_engine(&meta).name, "knitr"); + } + + // === bd-5yff4: de-duplication (detect_engine_sequence) === + + #[test] + fn test_detect_engine_sequence_distinct_passes_through() { + let meta = map_config(vec![( + "engine", + array_config(vec![string_config("knitr"), string_config("mermaidjs")]), + )]); + let seq = detect_engine_sequence(&meta); + let names: Vec<_> = seq.engines.iter().map(|e| e.name.as_str()).collect(); + assert_eq!(names, vec!["knitr", "mermaidjs"]); + assert!(seq.dropped_duplicates.is_empty()); + } + + #[test] + fn test_detect_engine_sequence_dedups_keeping_first() { + // [jupyter, knitr, jupyter] → [jupyter, knitr], dropped [jupyter] + let meta = map_config(vec![( + "engine", + array_config(vec![ + string_config("jupyter"), + string_config("knitr"), + string_config("jupyter"), + ]), + )]); + let seq = detect_engine_sequence(&meta); + let names: Vec<_> = seq.engines.iter().map(|e| e.name.as_str()).collect(); + assert_eq!(names, vec!["jupyter", "knitr"]); + assert_eq!(seq.dropped_duplicates, vec!["jupyter".to_string()]); + } + + #[test] + fn test_detect_engine_sequence_single_engine_no_drops() { + let meta = map_config(vec![("engine", string_config("knitr"))]); + let seq = detect_engine_sequence(&meta); + assert_eq!(seq.engines.len(), 1); + assert_eq!(seq.engines[0].name, "knitr"); + assert!(seq.dropped_duplicates.is_empty()); + } } diff --git a/crates/quarto-core/src/engine/fixture.rs b/crates/quarto-core/src/engine/fixture.rs new file mode 100644 index 000000000..f7013be37 --- /dev/null +++ b/crates/quarto-core/src/engine/fixture.rs @@ -0,0 +1,447 @@ +/* + * engine/fixture.rs + * Copyright (c) 2025 Posit, PBC + * + * File-backed test engine for exercising multi-engine sequencing. + */ + +//! File-backed test engine (test-registry-only). +//! +//! [`FixtureEngine`] is a deterministic, dependency-free +//! [`ExecutionEngine`] used to exercise **sequential multi-engine +//! execution** (bd-5yff4) without R, Python, or Jupyter. It "executes" +//! code cells by splicing pre-recorded results in document order: +//! +//! 1. It scans the input QMD for executable code cells whose language +//! token matches its [`name`](FixtureEngine::name) — i.e. fences of +//! the form ```` ```{<name>} ````. (pampa keeps the braces inside the +//! code block's class name, so an executable cell serializes back to +//! this exact form; a plain ```` ```<name> ```` display block does +//! *not* match.) +//! 2. For the i-th such cell (in document order) it replaces the entire +//! fenced block with the i-th entry of its results list. +//! +//! Results come from either an in-memory list ([`with_results`], a +//! convenience for filesystem-free unit tests) or a JSON file named by +//! the engine config — `engine: { <name>: { results: <path> } }`, a JSON +//! array of strings, one per cell, in order ([`new`]). The file-backed +//! form is the design the multi-engine plan calls for; the in-memory form +//! keeps unit tests simple. +//! +//! Because a spliced result may itself contain a ```` ```{<other>} ```` +//! cell, a `FixtureEngine` named `a` can hand work to a `FixtureEngine` +//! named `b` that runs later in the sequence — exactly the "engine N +//! produces cells for engine N+1" property multi-engine execution must +//! support. +//! +//! # Not for production +//! +//! This engine is **never** registered in the default +//! [`EngineRegistry`](super::EngineRegistry); tests register it +//! explicitly via [`EngineRegistry::register`]. Quarto 2's freeze feature +//! will reuse the execution *trace* (`engine: replay`), not this engine, +//! so it carries no production responsibility. It is gated to non-WASM +//! targets since it is a native test utility. +//! +//! [`new`]: FixtureEngine::new +//! [`with_results`]: FixtureEngine::with_results + +use super::context::{ExecuteResult, ExecutionContext}; +use super::error::ExecutionError; +use super::traits::ExecutionEngine; + +/// A deterministic, file-backed test engine. See module docs. +#[derive(Debug, Clone)] +pub struct FixtureEngine { + /// Engine name. Matches both the `engine:` declaration and the cell + /// language token (```` ```{<name>} ````). + name: String, + /// In-memory results, used when `Some`. When `None`, results are read + /// from the JSON file named by the engine config `results` key. + results: Option<Vec<String>>, +} + +impl FixtureEngine { + /// Create a file-backed engine. Results are read at `execute` time + /// from the JSON file named by the engine config `results` key, + /// resolved relative to the execution `cwd`. + pub fn new(name: impl Into<String>) -> Self { + Self { + name: name.into(), + results: None, + } + } + + /// Create an engine with in-memory results (filesystem-free; for unit + /// tests). Each entry replaces one matching cell, in document order. + pub fn with_results(name: impl Into<String>, results: Vec<String>) -> Self { + Self { + name: name.into(), + results: Some(results), + } + } + + /// Resolve the ordered results list: in-memory if present, else from + /// the JSON file named by the engine config `results` key. Absent + /// config (or no `results` key) yields an empty list, which the + /// cell/result count check in [`splice_cells`] treats as "no cells + /// expected". + fn resolve_results(&self, ctx: &ExecutionContext) -> Result<Vec<String>, ExecutionError> { + if let Some(results) = &self.results { + return Ok(results.clone()); + } + let Some(config) = &ctx.engine_config else { + return Ok(Vec::new()); + }; + let Some(results_val) = config.get("results") else { + return Ok(Vec::new()); + }; + let Some(path_str) = results_val.as_str() else { + return Err(ExecutionError::execution_failed( + &self.name, + "engine config `results` must be a string path to a JSON results file", + )); + }; + let path = ctx.cwd.join(path_str); + let bytes = std::fs::read(&path).map_err(|e| { + ExecutionError::execution_failed( + &self.name, + format!("failed to read results file {}: {e}", path.display()), + ) + })?; + let parsed: Vec<String> = serde_json::from_slice(&bytes).map_err(|e| { + ExecutionError::execution_failed( + &self.name, + format!( + "results file {} must be a JSON array of strings: {e}", + path.display() + ), + ) + })?; + Ok(parsed) + } +} + +impl ExecutionEngine for FixtureEngine { + fn name(&self) -> &str { + &self.name + } + + fn execute( + &self, + input: &str, + ctx: &ExecutionContext, + ) -> Result<ExecuteResult, ExecutionError> { + let results = self.resolve_results(ctx)?; + let output = splice_cells(input, &self.name, &results) + .map_err(|msg| ExecutionError::execution_failed(&self.name, msg))?; + Ok(ExecuteResult::new(output)) + } + + fn is_available(&self) -> bool { + true + } +} + +/// Replace each ```` ```{name} ```` executable cell in `input` with the +/// corresponding entry of `results`, in document order. +/// +/// Errors (returned as a message, wrapped into `ExecutionError` by the +/// caller) on: +/// - an unterminated `{name}` cell, +/// - a count mismatch between matching cells and results (missing or +/// surplus results). +/// +/// Non-matching fenced blocks (other engines' cells, plain display +/// blocks, fenced divs) are passed through untouched, and their contents +/// are *not* scanned for false `{name}` matches — the scanner skips over +/// every fenced block, splicing only the ones whose info string is +/// exactly `{name}`. +fn splice_cells(input: &str, name: &str, results: &[String]) -> Result<String, String> { + // `split('\n')` is the exact inverse of `join("\n")`, so the trailing + // newline (and any blank trailing line) round-trips precisely. + let lines: Vec<&str> = input.split('\n').collect(); + let want_info = format!("{{{name}}}"); + + // Pass 1: locate every matching cell as a (start, end_inclusive) line + // range, skipping the content of all fenced blocks so a `{name}` + // string inside another block is not mistaken for a cell. + let mut cells: Vec<(usize, usize)> = Vec::new(); + let mut i = 0; + while i < lines.len() { + let Some((fence_len, info)) = parse_opening_fence(lines[i]) else { + i += 1; + continue; + }; + let is_match = info.trim() == want_info; + // Find the closing fence (>= fence_len backticks, nothing else). + let mut j = i + 1; + let mut closed = false; + while j < lines.len() { + if is_closing_fence(lines[j], fence_len) { + closed = true; + break; + } + j += 1; + } + if !closed { + if is_match { + return Err(format!( + "fixture engine '{name}': unterminated code cell opened at line {}", + i + 1 + )); + } + // A non-matching unterminated fence is malformed input that is + // not ours to police; treat the remainder as block content. + break; + } + if is_match { + cells.push((i, j)); + } + i = j + 1; + } + + if cells.len() != results.len() { + return Err(format!( + "fixture engine '{name}': document has {} executable cell(s) but {} result(s) were provided", + cells.len(), + results.len() + )); + } + + // Pass 2: rebuild, replacing each cell's line range with its result. + let mut out: Vec<&str> = Vec::with_capacity(lines.len()); + let mut prev_end = 0; // first not-yet-emitted line index + let mut result_lines_storage: Vec<Vec<&str>> = Vec::with_capacity(cells.len()); + for result in results { + result_lines_storage.push(result.split('\n').collect()); + } + for (k, (start, end)) in cells.iter().enumerate() { + out.extend_from_slice(&lines[prev_end..*start]); + out.extend_from_slice(&result_lines_storage[k]); + prev_end = end + 1; + } + out.extend_from_slice(&lines[prev_end..]); + Ok(out.join("\n")) +} + +/// If `line` opens a backtick code fence (3+ leading backticks), return +/// the fence length and the info string following the backticks. +fn parse_opening_fence(line: &str) -> Option<(usize, &str)> { + let n = line.bytes().take_while(|&b| b == b'`').count(); + if n < 3 { + return None; + } + Some((n, &line[n..])) +} + +/// True if `line` is a closing fence for an opening fence of `fence_len` +/// backticks: at least `fence_len` backticks and nothing else (trailing +/// whitespace allowed; leading whitespace is not, since a space is not a +/// backtick). +fn is_closing_fence(line: &str, fence_len: usize) -> bool { + let trimmed = line.trim_end(); + trimmed.len() >= fence_len && !trimmed.is_empty() && trimmed.bytes().all(|b| b == b'`') +} + +#[cfg(test)] +mod tests { + use super::*; + use std::path::PathBuf; + + fn ctx() -> ExecutionContext { + ExecutionContext::new( + PathBuf::from("/tmp"), + PathBuf::from("/project"), + PathBuf::from("/project/doc.qmd"), + "html", + ) + } + + #[test] + fn name_is_configurable() { + assert_eq!(FixtureEngine::new("fixture-a").name(), "fixture-a"); + assert_eq!( + FixtureEngine::with_results("fixture-b", vec![]).name(), + "fixture-b" + ); + } + + #[test] + fn always_available() { + assert!(FixtureEngine::new("fixture-a").is_available()); + } + + #[test] + fn splices_single_cell() { + let engine = FixtureEngine::with_results("fixture-a", vec!["**Result:** 2".to_string()]); + let input = "Intro.\n\n```{fixture-a}\n1 + 1\n```\n\nOutro.\n"; + let out = engine.execute(input, &ctx()).unwrap().markdown; + assert_eq!(out, "Intro.\n\n**Result:** 2\n\nOutro.\n"); + } + + #[test] + fn splices_multiple_cells_in_order() { + let engine = + FixtureEngine::with_results("fixture-a", vec!["one".to_string(), "two".to_string()]); + let input = "```{fixture-a}\na\n```\n\nmid\n\n```{fixture-a}\nb\n```\n"; + let out = engine.execute(input, &ctx()).unwrap().markdown; + assert_eq!(out, "one\n\nmid\n\ntwo\n"); + } + + #[test] + fn ignores_other_engine_cells_and_display_blocks() { + // Only `{fixture-a}` cells are matched. `{fixture-b}` and the + // plain display block `fixture-a` (no braces) are untouched. + let engine = FixtureEngine::with_results("fixture-a", vec!["A!".to_string()]); + let input = "```{fixture-a}\nx\n```\n\n```{fixture-b}\ny\n```\n\n```fixture-a\nz\n```\n"; + let out = engine.execute(input, &ctx()).unwrap().markdown; + assert_eq!( + out, + "A!\n\n```{fixture-b}\ny\n```\n\n```fixture-a\nz\n```\n" + ); + } + + #[test] + fn does_not_match_inside_other_fenced_blocks() { + // A `{fixture-a}` line that is the *content* of a plain ``` block + // must not be treated as an opening cell fence. + let engine = FixtureEngine::with_results("fixture-a", vec![]); + let input = "```\n```{fixture-a}\n```\n"; + // Zero matching cells, zero results → passthrough unchanged. + let out = engine.execute(input, &ctx()).unwrap().markdown; + assert_eq!(out, input); + } + + #[test] + fn handoff_result_can_introduce_next_engines_cell() { + // fixture-a's result contains a `{fixture-b}` cell — proves an + // engine can emit a cell for a later engine in the sequence. + let engine = FixtureEngine::with_results( + "fixture-a", + vec!["```{fixture-b}\nfrom-a\n```".to_string()], + ); + let input = "```{fixture-a}\nseed\n```\n"; + let out = engine.execute(input, &ctx()).unwrap().markdown; + assert_eq!(out, "```{fixture-b}\nfrom-a\n```\n"); + } + + #[test] + fn errors_on_too_few_results() { + let engine = FixtureEngine::with_results("fixture-a", vec!["only-one".to_string()]); + let input = "```{fixture-a}\na\n```\n\n```{fixture-a}\nb\n```\n"; + let err = engine.execute(input, &ctx()).unwrap_err(); + let msg = format!("{err}"); + assert!( + msg.contains("2 executable cell(s) but 1 result(s)"), + "got: {msg}" + ); + } + + #[test] + fn errors_on_surplus_results() { + let engine = + FixtureEngine::with_results("fixture-a", vec!["one".to_string(), "two".to_string()]); + let input = "```{fixture-a}\na\n```\n"; + let err = engine.execute(input, &ctx()).unwrap_err(); + let msg = format!("{err}"); + assert!( + msg.contains("1 executable cell(s) but 2 result(s)"), + "got: {msg}" + ); + } + + #[test] + fn errors_on_unterminated_cell() { + let engine = FixtureEngine::with_results("fixture-a", vec!["x".to_string()]); + let input = "```{fixture-a}\nnever closed\n"; + let err = engine.execute(input, &ctx()).unwrap_err(); + assert!(format!("{err}").contains("unterminated")); + } + + #[test] + fn longer_fences_round_trip() { + // A 4-backtick cell closes only on >= 4 backticks; an interior + // 3-backtick line is content, not a close. + let engine = FixtureEngine::with_results("fixture-a", vec!["spliced".to_string()]); + let input = "````{fixture-a}\n```\ninner\n```\n````\n"; + let out = engine.execute(input, &ctx()).unwrap().markdown; + assert_eq!(out, "spliced\n"); + } + + #[test] + fn no_cells_no_results_is_passthrough() { + let engine = FixtureEngine::with_results("fixture-a", vec![]); + let input = "# Just prose\n\nNo cells here.\n"; + let out = engine.execute(input, &ctx()).unwrap().markdown; + assert_eq!(out, input); + } + + #[test] + fn file_backed_reads_results_from_engine_config() { + use quarto_pandoc_types::ConfigValue; + use quarto_pandoc_types::config_value::ConfigMapEntry; + use quarto_source_map::SourceInfo; + + let dir = tempfile::tempdir().unwrap(); + let results_path = dir.path().join("results.json"); + std::fs::write(&results_path, r#"["**from file**"]"#).unwrap(); + + let engine_config = ConfigValue::new_map( + vec![ConfigMapEntry { + key: "results".to_string(), + key_source: SourceInfo::default(), + value: ConfigValue::new_string("results.json", SourceInfo::default()), + }], + SourceInfo::default(), + ); + + let exec_ctx = ExecutionContext::new( + dir.path().to_path_buf(), + dir.path().to_path_buf(), + dir.path().join("doc.qmd"), + "html", + ) + .with_engine_config(Some(engine_config)); + + let engine = FixtureEngine::new("fixture-a"); + let input = "```{fixture-a}\nseed\n```\n"; + let out = engine.execute(input, &exec_ctx).unwrap().markdown; + assert_eq!(out, "**from file**\n"); + } + + #[test] + fn file_backed_reports_unreadable_file() { + use quarto_pandoc_types::ConfigValue; + use quarto_pandoc_types::config_value::ConfigMapEntry; + use quarto_source_map::SourceInfo; + + let engine_config = ConfigValue::new_map( + vec![ConfigMapEntry { + key: "results".to_string(), + key_source: SourceInfo::default(), + value: ConfigValue::new_string("does-not-exist.json", SourceInfo::default()), + }], + SourceInfo::default(), + ); + let exec_ctx = ExecutionContext::new( + PathBuf::from("/tmp"), + PathBuf::from("/nonexistent-dir-xyz"), + PathBuf::from("/tmp/doc.qmd"), + "html", + ) + .with_engine_config(Some(engine_config)); + + let engine = FixtureEngine::new("fixture-a"); + let err = engine + .execute("```{fixture-a}\nx\n```\n", &exec_ctx) + .unwrap_err(); + assert!(format!("{err}").contains("failed to read results file")); + } + + #[test] + fn is_send_sync() { + fn assert_send_sync<T: Send + Sync>() {} + assert_send_sync::<FixtureEngine>(); + } +} diff --git a/crates/quarto-core/src/engine/mod.rs b/crates/quarto-core/src/engine/mod.rs index a06a5fc3e..c206eb678 100644 --- a/crates/quarto-core/src/engine/mod.rs +++ b/crates/quarto-core/src/engine/mod.rs @@ -63,6 +63,12 @@ mod registry; mod replay; mod traits; +// File-backed test engine for exercising multi-engine sequencing +// (bd-5yff4). Native-only test utility; never registered in the default +// registry. See `fixture.rs` module docs. +#[cfg(not(target_arch = "wasm32"))] +mod fixture; + // Native-only modules #[cfg(not(target_arch = "wasm32"))] pub mod jupyter; @@ -71,8 +77,13 @@ mod knitr; // Re-export public types pub use context::{ExecuteResult, ExecutionContext}; -pub use detection::{DetectedEngine, KNOWN_ENGINES, detect_engine, is_known_engine}; +pub use detection::{ + DetectedEngine, EngineSequence, KNOWN_ENGINES, detect_engine, detect_engine_sequence, + detect_engines, is_known_engine, +}; pub use error::ExecutionError; +#[cfg(not(target_arch = "wasm32"))] +pub use fixture::FixtureEngine; pub use markdown::MarkdownEngine; pub use registry::EngineRegistry; pub use replay::ReplayEngine; diff --git a/crates/quarto-core/src/engine/preview_record.rs b/crates/quarto-core/src/engine/preview_record.rs index 058d7f84d..c94c6763b 100644 --- a/crates/quarto-core/src/engine/preview_record.rs +++ b/crates/quarto-core/src/engine/preview_record.rs @@ -51,17 +51,17 @@ use super::EngineRegistry; /// delegate to [`NoopObserver`] — we don't care about stage events, /// only the capture payload. struct CaptureCollector { - captured: Arc<Mutex<Option<EngineCapture>>>, + captured: Arc<Mutex<Vec<EngineCapture>>>, } impl CaptureCollector { fn new() -> Self { Self { - captured: Arc::new(Mutex::new(None)), + captured: Arc::new(Mutex::new(Vec::new())), } } - fn handle(&self) -> Arc<Mutex<Option<EngineCapture>>> { + fn handle(&self) -> Arc<Mutex<Vec<EngineCapture>>> { Arc::clone(&self.captured) } } @@ -73,13 +73,13 @@ impl PipelineObserver for CaptureCollector { } match serde_json::from_value::<EngineCapture>(data.clone()) { Ok(capture) => { - // First-write-wins. EngineExecutionStage runs once per - // pipeline, so this realistically only fires once; - // guarding anyway keeps the type honest. - let mut slot = self.captured.lock().expect("capture mutex poisoned"); - if slot.is_none() { - *slot = Some(capture); - } + // bd-5yff4: collect one capture per engine, in execution + // order. `EngineExecutionStage` emits these sequentially + // as it threads the engine sequence. + self.captured + .lock() + .expect("capture mutex poisoned") + .push(capture); } Err(e) => { // Should not happen — the producer in @@ -119,10 +119,10 @@ fn build_capture_pipeline_stages( /// Record the engine capture for `path` by running the q2-preview /// pipeline up through engine execution. /// -/// Returns `Ok(Some(capture))` when an engine fired and emitted its -/// `EngineCapture` aux event, `Ok(None)` when no engine ran (default -/// markdown engine short-circuits without emitting). Pipeline errors -/// propagate. +/// Returns the captures in execution order — one per engine that fired +/// and emitted its `EngineCapture` aux event (bd-5yff4). An empty vec +/// means no engine ran (e.g. the default markdown engine short-circuits +/// without emitting). Pipeline errors propagate. /// /// `engine_registry` lets callers substitute a custom registry for /// tests (passthrough engine standing in for jupyter, etc.); production @@ -132,7 +132,7 @@ pub async fn record_capture( project: &ProjectContext, runtime: Arc<dyn SystemRuntime>, engine_registry: Option<EngineRegistry>, -) -> Result<Option<EngineCapture>, PipelineError> { +) -> Result<Vec<EngineCapture>, PipelineError> { let content = runtime .file_read(path) .map_err(|e| PipelineError::other(format!("Failed to read {}: {}", path.display(), e)))?; @@ -157,7 +157,7 @@ pub async fn record_capture( let _final_data = pipeline.run(input, &mut ctx).await?; let mut slot = captured.lock().expect("capture mutex poisoned"); - Ok(slot.take()) + Ok(std::mem::take(&mut *slot)) } /// Build the staleness-check sub-pipeline: everything up to (and @@ -287,8 +287,8 @@ mod tests { .expect("pipeline runs cleanly for prose"); assert!( - result.is_none(), - "expected None for prose-only doc with default (markdown) engine" + result.is_empty(), + "expected no captures for prose-only doc with default (markdown) engine" ); } @@ -306,7 +306,12 @@ mod tests { .await .expect("pipeline runs cleanly with test engine"); - let capture = result.expect("expected Some(capture) for doc with test-passthrough engine"); + assert_eq!( + result.len(), + 1, + "expected one capture for doc with test-passthrough engine" + ); + let capture = &result[0]; assert_eq!(capture.engine_name, "test-passthrough"); assert!( !capture.input_qmd.is_empty(), @@ -349,7 +354,7 @@ mod tests { let result = record_capture(&path, &project, runtime, Some(registry)) .await .expect("pipeline runs"); - let capture = result.expect("capture present"); + let capture = result.first().expect("capture present"); assert!( capture.input_qmd.contains("SENTINEL"), @@ -369,7 +374,7 @@ mod tests { let result = record_capture(&path, &project, runtime, None) .await .expect("pipeline runs"); - assert!(result.is_none()); + assert!(result.is_empty()); } // ────────────────────────────────────────────────────────────── @@ -388,10 +393,10 @@ mod tests { let mut registry = EngineRegistry::new(); registry.register(Arc::new(PassthroughTestEngine)); - let capture = record_capture(&path, &project, runtime.clone(), Some(registry)) + let captures = record_capture(&path, &project, runtime.clone(), Some(registry)) .await - .expect("record_capture") - .expect("capture present"); + .expect("record_capture"); + let capture = captures.first().expect("capture present"); let computed = compute_input_qmd(&path, &project, runtime) .await diff --git a/crates/quarto-core/src/engine/registry.rs b/crates/quarto-core/src/engine/registry.rs index d326303cc..257716989 100644 --- a/crates/quarto-core/src/engine/registry.rs +++ b/crates/quarto-core/src/engine/registry.rs @@ -155,8 +155,28 @@ impl EngineRegistry { /// Activation lives at the orchestrator/CLI layer (`q2 render /// --replay <trace>`); this constructor is the seam. pub fn with_replay(capture: quarto_trace::EngineCapture) -> Self { + Self::with_replay_many(vec![capture]) + } + + /// Build a registry that substitutes a [`super::ReplayEngine`] for + /// each recorded engine in a sequence (bd-5yff4). + /// + /// Starts from a default registry, then registers one replay engine + /// per capture, each keyed by its recorded `engine_name`. Because the + /// engines in a sequence are distinct (the trace records one capture + /// per engine, in order), the name-keyed registry holds them all + /// without collision, and `EngineExecutionStage` drives them in the + /// order the document's `engine:` sequence declares — which must match + /// the recording. Each replay engine validates its own `input_qmd`. + /// + /// Captures whose `engine_name` is not also in the document's engine + /// sequence simply never run; extra real engines (those without a + /// matching capture) keep their default implementations. + pub fn with_replay_many(captures: Vec<quarto_trace::EngineCapture>) -> Self { let mut registry = Self::new(); - registry.register(Arc::new(super::ReplayEngine::new(capture))); + for capture in captures { + registry.register(Arc::new(super::ReplayEngine::new(capture))); + } registry } } diff --git a/crates/quarto-core/src/pipeline.rs b/crates/quarto-core/src/pipeline.rs index 08e2aabfc..67ab0cbd4 100644 --- a/crates/quarto-core/src/pipeline.rs +++ b/crates/quarto-core/src/pipeline.rs @@ -379,21 +379,20 @@ const Q2_PREVIEW_STAGE_EXCLUDED: &[&str] = &["math-js", "render-html-body", "app /// `claude-notes/plans/2026-05-18-q2-preview-project-replay-engine.md`. pub fn build_q2_preview_pipeline_stages( engine_registry: Option<crate::engine::EngineRegistry>, - capture: Option<quarto_trace::EngineCapture>, + captures: Vec<quarto_trace::EngineCapture>, ) -> Vec<Box<dyn PipelineStage>> { let mut stages = build_html_pipeline_stages_with_options(None, engine_registry); stages.retain(|s| !Q2_PREVIEW_STAGE_EXCLUDED.contains(&s.name())); // Insert the splice stage immediately *before* EngineExecutionStage. // bd-lucp: this is the q2-preview-specific consumer of recorded - // captures. The HTML pipeline doesn't include it — `q2 render` - // either runs the real engine natively or uses `--replay` (which - // goes through `EngineRegistry::with_replay`, an entirely - // different code path). - let splice_stage: Box<dyn PipelineStage> = match capture { - Some(c) => Box::new(crate::stage::CaptureSpliceStage::new().with_capture(c)), - None => Box::new(crate::stage::CaptureSpliceStage::new()), - }; + // captures. bd-5yff4: the captures are an ordered sequence (one per + // engine); the splice folds them. The HTML pipeline doesn't include + // it — `q2 render` either runs the real engine natively or uses + // `--replay` (which goes through `EngineRegistry::with_replay_many`, + // an entirely different code path). + let splice_stage: Box<dyn PipelineStage> = + Box::new(crate::stage::CaptureSpliceStage::new().with_captures(captures)); let engine_idx = stages .iter() .position(|s| s.name() == "engine-execution") @@ -867,7 +866,7 @@ pub async fn render_qmd_to_preview_ast( ctx: &mut RenderContext<'_>, runtime: Arc<dyn quarto_system_runtime::SystemRuntime>, engine_registry: Option<crate::engine::EngineRegistry>, - capture: Option<quarto_trace::EngineCapture>, + captures: Vec<quarto_trace::EngineCapture>, ) -> Result<PreviewAstOutput> { // The q2-preview stage list excludes `CodeHighlightStage` / // `RenderHtmlBodyStage` / `ApplyTemplateStage`, so the @@ -883,7 +882,7 @@ pub async fn render_qmd_to_preview_ast( // `EngineExecutionStage` runs (which then no-ops via the WASM // markdown fallback). See // `claude-notes/plans/2026-05-18-q2-preview-project-replay-engine.md`. - let stages = build_q2_preview_pipeline_stages(engine_registry, capture); + let stages = build_q2_preview_pipeline_stages(engine_registry, captures); let (output, diagnostics) = run_pipeline(content, source_name, ctx, runtime, stages).await?; let ast = output.into_document_ast().ok_or_else(|| { @@ -2207,7 +2206,12 @@ mod tests { let runtime = make_test_runtime(); let output = pollster::block_on(render_qmd_to_preview_ast( - content, "test.qmd", &mut ctx, runtime, None, None, + content, + "test.qmd", + &mut ctx, + runtime, + None, + Vec::new(), )) .expect("q2-preview render"); @@ -2261,7 +2265,12 @@ mod tests { let runtime = make_test_runtime(); let output = pollster::block_on(render_qmd_to_preview_ast( - content, "test.qmd", &mut ctx, runtime, None, None, + content, + "test.qmd", + &mut ctx, + runtime, + None, + Vec::new(), )) .expect("q2-preview render"); @@ -2299,7 +2308,12 @@ mod tests { let runtime = make_test_runtime(); let output = pollster::block_on(render_qmd_to_preview_ast( - content, "test.qmd", &mut ctx, runtime, None, None, + content, + "test.qmd", + &mut ctx, + runtime, + None, + Vec::new(), )) .expect("q2-preview render"); @@ -2368,7 +2382,7 @@ mod tests { /// Python / etc. cells; `q2 render` keeps highlighting. #[test] fn q2_preview_pipeline_includes_code_highlight() { - let stages = build_q2_preview_pipeline_stages(None, None); + let stages = build_q2_preview_pipeline_stages(None, Vec::new()); let names: Vec<&str> = stages.iter().map(|s| s.name()).collect(); assert!( names.contains(&"code-highlight"), @@ -2484,7 +2498,12 @@ mod tests { let runtime = make_test_runtime(); pollster::block_on(render_qmd_to_preview_ast( - content, "test.qmd", &mut ctx, runtime, None, None, + content, + "test.qmd", + &mut ctx, + runtime, + None, + Vec::new(), )) .expect("baseline q2-preview render") }; @@ -2522,7 +2541,12 @@ mod tests { let runtime = make_test_runtime(); let output = pollster::block_on(render_qmd_to_preview_ast( - content, "test.qmd", &mut ctx, runtime, None, None, + content, + "test.qmd", + &mut ctx, + runtime, + None, + Vec::new(), )) .expect("attributed q2-preview render"); diff --git a/crates/quarto-core/src/project/pass2_renderer.rs b/crates/quarto-core/src/project/pass2_renderer.rs index 04f88afc5..ed87ba239 100644 --- a/crates/quarto-core/src/project/pass2_renderer.rs +++ b/crates/quarto-core/src/project/pass2_renderer.rs @@ -573,12 +573,13 @@ pub struct RenderToPreviewAstRenderer { /// Synthetic VFS root under which every artifact lives in WASM. /// Same semantics as [`RenderToHtmlRenderer::new`]. vfs_root: std::path::PathBuf, - /// bd-lucp: optional engine-execution capture used to splice - /// recorded engine output into the AST at preview time. Plumbed - /// through to [`crate::pipeline::render_qmd_to_preview_ast`] on - /// every per-doc `render` call. None by default (no splice; engine - /// cells render as raw source — same as the pre-bd-lucp path). - capture: Option<quarto_trace::EngineCapture>, + /// bd-lucp / bd-5yff4: ordered engine-execution captures used to + /// splice recorded engine output into the AST at preview time (one + /// per engine that ran server-side). Plumbed through to + /// [`crate::pipeline::render_qmd_to_preview_ast`] on every per-doc + /// `render` call. Empty by default (no splice; engine cells render as + /// raw source — same as the pre-bd-lucp path). + captures: Vec<quarto_trace::EngineCapture>, /// Optional transport JSON payload (a serialized /// [`crate::attribution::types::TransportAttributionData`]). When /// `Some`, the renderer installs a @@ -598,22 +599,21 @@ impl RenderToPreviewAstRenderer { Self { vfs_root: vfs_root.into(), attribution_json: None, - capture: None, + captures: Vec::new(), } } - /// Attach a recorded [`EngineCapture`](quarto_trace::EngineCapture). - /// The renderer threads the capture into every per-doc - /// `render_qmd_to_preview_ast` call so the - /// [`CaptureSpliceStage`](crate::stage::CaptureSpliceStage) - /// can substitute the captured engine output blocks for the - /// document's engine code cells. Used by the WASM - /// `render_page_for_preview` entry point when the SPA hands in a - /// capture binary doc from the IndexDocument sidecar; native test - /// callers either pass `None` (no splice) or hand in a synthetic - /// capture. - pub fn with_capture(mut self, capture: quarto_trace::EngineCapture) -> Self { - self.capture = Some(capture); + /// Attach the recorded [`EngineCapture`](quarto_trace::EngineCapture) + /// sequence (one per engine that ran, in order). The renderer threads + /// them into every per-doc `render_qmd_to_preview_ast` call so the + /// [`CaptureSpliceStage`](crate::stage::CaptureSpliceStage) can fold + /// the captured engine output blocks onto the document's engine code + /// cells. Used by the WASM `render_page_for_preview` entry point when + /// the SPA hands in a capture binary doc from the IndexDocument + /// sidecar; native test callers either pass an empty vec (no splice) + /// or hand in synthetic captures. + pub fn with_captures(mut self, captures: Vec<quarto_trace::EngineCapture>) -> Self { + self.captures = captures; self } @@ -690,13 +690,13 @@ impl Pass2Renderer for RenderToPreviewAstRenderer { &mut ctx, runtime.clone(), None, - // bd-lucp: forward the renderer-attached capture (if any) - // so `CaptureSpliceStage` can splice recorded engine - // output blocks into the live AST. Cloned per-doc so + // bd-lucp / bd-5yff4: forward the renderer-attached capture + // sequence (if any) so `CaptureSpliceStage` can fold recorded + // engine output blocks onto the live AST. Cloned per-doc so // every page in a project pipeline run sees the same - // capture; future per-page capture maps would replace - // this with an index-lookup. - self.capture.clone(), + // captures; future per-page capture maps would replace this + // with an index-lookup. + self.captures.clone(), ) .await?; diff --git a/crates/quarto-core/src/render_to_file.rs b/crates/quarto-core/src/render_to_file.rs index 64f6d1e5e..aea4ad0bb 100644 --- a/crates/quarto-core/src/render_to_file.rs +++ b/crates/quarto-core/src/render_to_file.rs @@ -88,26 +88,27 @@ pub struct RenderToFileOptions { pub output_dir: Option<PathBuf>, /// Suppress informational messages (logging). pub quiet: bool, - /// Replay engine capture loaded from a trace file (bd-45yw). + /// Replay engine captures loaded from a trace file (bd-45yw, + /// extended to a sequence by bd-5yff4). /// - /// When `Some`, the render substitutes a - /// [`crate::engine::ReplayEngine`] for the engine of the - /// recorded name in the pipeline's registry. Activated - /// out-of-band by the orchestrator/CLI (`--replay <trace>` / - /// `QUARTO_REPLAY=...`); the document under investigation does - /// not need to know. - pub replay_capture: Option<quarto_trace::EngineCapture>, + /// When non-empty, the render substitutes a + /// [`crate::engine::ReplayEngine`] for each recorded engine + /// (one capture per engine, in execution order) in the + /// pipeline's registry. Activated out-of-band by the + /// orchestrator/CLI (`--replay <trace>` / `QUARTO_REPLAY=...`); + /// the document under investigation does not need to know. + pub replay_captures: Vec<quarto_trace::EngineCapture>, /// Direct override for the engine registry the pipeline uses /// (bd-45yw, primarily for tests). /// - /// When `Some`, takes precedence over `replay_capture`: the + /// When `Some`, takes precedence over `replay_captures`: the /// caller hands the pipeline an arbitrary registry. Tests use /// this seam to register probe engines that capture the QMD /// `EngineExecutionStage` hands to `execute()` (so a replay /// trace can be fabricated against the real pipeline rather /// than a synthetic context). Production callers should prefer - /// `replay_capture`. + /// `replay_captures`. pub engine_registry_override: Option<crate::engine::EngineRegistry>, /// Resolved attribution mode (CLI override merged with YAML). @@ -290,14 +291,17 @@ pub fn render_document_to_file( // shared assets. ctx.resource_resolver = Some(resolver.clone()); let mut config = HtmlRenderConfig::with_resolver(resolver.clone()); - // bd-45yw: pick the engine registry the pipeline will use. - // engine_registry_override takes precedence (tests / probe - // engines). Otherwise replay_capture builds the registry. - // Otherwise the pipeline builds its own default registry. + // bd-45yw / bd-5yff4: pick the engine registry the pipeline will + // use. engine_registry_override takes precedence (tests / probe + // engines). Otherwise replay_captures (one per recorded engine) + // build the registry. Otherwise the pipeline builds its own default + // registry. if let Some(reg) = options.engine_registry_override.clone() { config.engine_registry = Some(reg); - } else if let Some(capture) = options.replay_capture.clone() { - config.engine_registry = Some(crate::engine::EngineRegistry::with_replay(capture)); + } else if !options.replay_captures.is_empty() { + config.engine_registry = Some(crate::engine::EngineRegistry::with_replay_many( + options.replay_captures.clone(), + )); } // Run the render pipeline diff --git a/crates/quarto-core/src/stage/observer.rs b/crates/quarto-core/src/stage/observer.rs index aa1d3bfc1..6c9e023c9 100644 --- a/crates/quarto-core/src/stage/observer.rs +++ b/crates/quarto-core/src/stage/observer.rs @@ -157,6 +157,33 @@ pub trait PipelineObserver: Send + Sync { ) { } + /// Called after each engine runs within `EngineExecutionStage` + /// (bd-5yff4), giving finer-grained tracing of a multi-engine + /// sequence: one AST snapshot per engine, in execution order. + /// + /// Mirrors [`Self::on_transform_data`] — trace observers record an + /// `engine:<name>` entry so a debugger can step through the sequence + /// (e.g. `engine:knitr`, then `engine:mermaidjs`). The single-engine + /// case emits exactly one such entry, just before the stage's own + /// final snapshot. + /// + /// # Arguments + /// + /// * `engine_name` - Name of the engine that just ran (e.g. `"knitr"`). + /// * `index` - Zero-based position of the engine in the executed + /// sequence (markdown no-ops are not counted). + /// * `ast` - The reconciled AST after this engine's output was merged. + /// * `ast_context` - The AST context (multi-slot after the first + /// engine), carrying filename attribution and the source-info pool. + fn on_engine_data( + &self, + _engine_name: &str, + _index: usize, + _ast: &quarto_pandoc_types::pandoc::Pandoc, + _ast_context: &pampa::pandoc::ASTContext, + ) { + } + /// Called by transforms that want to publish auxiliary structured data /// alongside the AST trace. /// diff --git a/crates/quarto-core/src/stage/stages/capture_splice.rs b/crates/quarto-core/src/stage/stages/capture_splice.rs index 40207f23f..ddc8d6c8c 100644 --- a/crates/quarto-core/src/stage/stages/capture_splice.rs +++ b/crates/quarto-core/src/stage/stages/capture_splice.rs @@ -55,27 +55,38 @@ use crate::trace_event; /// /// [`with_capture`]: CaptureSpliceStage::with_capture pub struct CaptureSpliceStage { - capture: Option<EngineCapture>, + /// Recorded engine captures, in execution order (bd-5yff4). One per + /// engine that ran server-side. Empty = clean pass-through. + captures: Vec<EngineCapture>, } impl CaptureSpliceStage { - /// Build a splice stage with no capture attached. The stage runs + /// Build a splice stage with no captures attached. The stage runs /// as a clean pass-through. Used when the q2-preview pipeline is /// built without a recorded capture (no server-side execution /// happened yet, or the user has not opened a project with /// engine cells). pub fn new() -> Self { - Self { capture: None } + Self { + captures: Vec::new(), + } } - /// Attach a recorded capture. On every per-doc run, the stage - /// re-parses `capture.input_qmd` and the markdown field of - /// `capture.result`, derives a cell-output map, and splices it - /// onto the current AST. The capture is held by value because - /// the stage may run on multiple pipeline executions (e.g. the - /// q2-preview project pipeline runs once per active page). - pub fn with_capture(mut self, capture: EngineCapture) -> Self { - self.capture = Some(capture); + /// Attach a single recorded capture. Convenience for the + /// single-engine case; equivalent to `with_captures(vec![capture])`. + pub fn with_capture(self, capture: EngineCapture) -> Self { + self.with_captures(vec![capture]) + } + + /// Attach the recorded capture sequence (bd-5yff4). On every per-doc + /// run, the stage folds the captures in order: for each, it re-parses + /// `capture.input_qmd` and the markdown field of `capture.result`, + /// derives a cell-output map, and splices it onto the current AST — + /// so engine N+1's splice runs on engine N's spliced output. Held by + /// value because the stage may run on multiple pipeline executions + /// (e.g. the q2-preview project pipeline runs once per active page). + pub fn with_captures(mut self, captures: Vec<EngineCapture>) -> Self { + self.captures = captures; self } } @@ -113,78 +124,77 @@ impl PipelineStage for CaptureSpliceStage { )); }; - let Some(capture) = self.capture.as_ref() else { - trace_event!(ctx, EventLevel::Debug, "no capture attached; pass-through"); + if self.captures.is_empty() { + trace_event!(ctx, EventLevel::Debug, "no captures attached; pass-through"); return Ok(PipelineData::DocumentAst(doc_ast)); - }; - - // Extract result.markdown from the opaque JSON. We don't need - // the rest of the ExecuteResult shape here (filters, includes, - // supporting_files) — those are engine-side concerns the - // splice doesn't reproduce in the AST. Future work could - // surface them through StageContext if a real splice consumer - // needs them. - let result_markdown = capture - .result - .get("markdown") - .and_then(|v| v.as_str()) - .unwrap_or(""); - - // Parse both QMD strings via the same pampa reader the rest - // of the pipeline uses. Slot in throwaway names — the parsed - // ASTs are immediately consumed by the splice, so source- - // attribution doesn't escape this stage. - let (a1, _, _a1_warnings) = match pampa::readers::qmd::read( - capture.input_qmd.as_bytes(), - false, - "capture-input.rmarkdown", - &mut std::io::sink(), - false, // don't track source locations — splice ignores them - None, - ) { - Ok(t) => t, - Err(_) => { - // Parse failure: degrade to pass-through so a corrupt - // capture can't take the preview down. + } + + // bd-5yff4: fold the captures in order. Engine N+1's splice runs + // on engine N's spliced output, mirroring the server-side + // sequence. Each capture is fail-soft: a parse failure or an + // unmatched cell leaves that engine's cells as raw source without + // taking the preview down. + for capture in &self.captures { + // Extract result.markdown from the opaque JSON. We don't need + // the rest of the ExecuteResult shape here (filters, includes, + // supporting_files) — those are engine-side concerns the + // splice doesn't reproduce in the AST. + let result_markdown = capture + .result + .get("markdown") + .and_then(|v| v.as_str()) + .unwrap_or(""); + + // Parse both QMD strings via the same pampa reader the rest + // of the pipeline uses. Slot in throwaway names — the parsed + // ASTs are immediately consumed by the splice, so source- + // attribution doesn't escape this stage. + let Ok((a1, _, _)) = pampa::readers::qmd::read( + capture.input_qmd.as_bytes(), + false, + "capture-input.rmarkdown", + &mut std::io::sink(), + false, // don't track source locations — splice ignores them + None, + ) else { trace_event!( ctx, EventLevel::Warn, - "failed to parse capture.input_qmd; falling through" + "failed to parse capture.input_qmd for engine={}; skipping this capture", + capture.engine_name ); - return Ok(PipelineData::DocumentAst(doc_ast)); - } - }; - let (b1, _, _b1_warnings) = match pampa::readers::qmd::read( - result_markdown.as_bytes(), - false, - "capture-result.md", - &mut std::io::sink(), - false, - None, - ) { - Ok(t) => t, - Err(_) => { + continue; + }; + let Ok((b1, _, _)) = pampa::readers::qmd::read( + result_markdown.as_bytes(), + false, + "capture-result.md", + &mut std::io::sink(), + false, + None, + ) else { trace_event!( ctx, EventLevel::Warn, - "failed to parse capture.result.markdown; falling through" + "failed to parse capture.result.markdown for engine={}; skipping this capture", + capture.engine_name ); - return Ok(PipelineData::DocumentAst(doc_ast)); - } - }; - - let before_blocks = doc_ast.ast.blocks.len(); - doc_ast.ast = apply_capture_splice(doc_ast.ast, &a1, &b1, &capture.engine_name); - let after_blocks = doc_ast.ast.blocks.len(); - - trace_event!( - ctx, - EventLevel::Debug, - "capture-splice: engine={}, blocks {}→{}", - capture.engine_name, - before_blocks, - after_blocks - ); + continue; + }; + + let before_blocks = doc_ast.ast.blocks.len(); + doc_ast.ast = apply_capture_splice(doc_ast.ast, &a1, &b1, &capture.engine_name); + let after_blocks = doc_ast.ast.blocks.len(); + + trace_event!( + ctx, + EventLevel::Debug, + "capture-splice: engine={}, blocks {}→{}", + capture.engine_name, + before_blocks, + after_blocks + ); + } Ok(PipelineData::DocumentAst(doc_ast)) } diff --git a/crates/quarto-core/src/stage/stages/engine_execution.rs b/crates/quarto-core/src/stage/stages/engine_execution.rs index a93274222..c35bc8cdf 100644 --- a/crates/quarto-core/src/stage/stages/engine_execution.rs +++ b/crates/quarto-core/src/stage/stages/engine_execution.rs @@ -29,7 +29,7 @@ use std::sync::Arc; use quarto_error_reporting::DiagnosticMessage; -use crate::engine::{EngineRegistry, ExecutionContext, ExecutionEngine, detect_engine}; +use crate::engine::{EngineRegistry, ExecutionContext, ExecutionEngine, detect_engine_sequence}; use crate::stage::{ DocumentAst, EventLevel, PipelineData, PipelineDataKind, PipelineError, PipelineStage, StageContext, @@ -39,10 +39,10 @@ use crate::trace_event; /// Stable [`PipelineObserver::on_auxiliary_data`] kind tag for the /// engine capture emitted by this stage (bd-45yw). /// -/// `JsonTraceObserver` recognizes this kind and routes the payload to -/// `TraceDocument.engine_capture` (the typed slot) instead of to the -/// generic pipeline aux channel. The payload's JSON shape matches -/// [`quarto_trace::EngineCapture`]. +/// `JsonTraceObserver` recognizes this kind and appends the payload to +/// `TraceDocument.engine_captures` (the typed slot — one per engine, in +/// execution order) instead of to the generic pipeline aux channel. The +/// payload's JSON shape matches [`quarto_trace::EngineCapture`]. pub const ENGINE_CAPTURE_KIND: &str = "EngineCapture"; /// Pipeline stage that executes code cells via the appropriate engine. @@ -160,255 +160,297 @@ impl PipelineStage for EngineExecutionStage { )); }; - // Step 1: Detect engine from metadata - let detected = detect_engine(&doc_ast.ast.meta); + // Step 1: Detect the (ordered, de-duplicated) engine sequence. + // bd-5yff4: `engine:` may be an array; engines run in declared + // order, each consuming the AST the previous engine produced. + let sequence = detect_engine_sequence(&doc_ast.ast.meta); + + // Distinct engines are required; report any duplicates the merge + // produced (first occurrence kept). See `EngineSequence`. + for dropped in &sequence.dropped_duplicates { + ctx.add_diagnostics(vec![DiagnosticMessage::warning(format!( + "Duplicate engine '{dropped}' in engine sequence ignored \ + (engines must be distinct; the first occurrence is kept — \ + use the !prefer merge tag to replace the list)" + ))]); + } trace_event!( ctx, EventLevel::Debug, - "detected engine: {} (config: {})", - detected.name, - if detected.config.is_some() { - "yes" - } else { - "no" - } + "detected engine sequence: [{}]", + sequence + .engines + .iter() + .map(|e| e.name.as_str()) + .collect::<Vec<_>>() + .join(", ") ); - // Step 2: Get the engine implementation (with fallback) + // Step 2: Resolve each detected engine to an implementation (with + // fallback), dropping markdown — it's a no-op, so a markdown + // engine anywhere in the sequence is skipped (this also covers + // unavailable engines that fall back to markdown). let mut engine_warnings = Vec::new(); - let engine = self.get_engine_with_fallback(&detected.name, &mut engine_warnings); - - // Add any engine lookup diagnostics to context + let mut to_run: Vec<( + Arc<dyn ExecutionEngine>, + Option<quarto_pandoc_types::ConfigValue>, + )> = Vec::new(); + for detected in &sequence.engines { + let engine = self.get_engine_with_fallback(&detected.name, &mut engine_warnings); + if engine.name() == "markdown" { + trace_event!( + ctx, + EventLevel::Debug, + "engine '{}' resolved to markdown (no-op) — skipping", + detected.name + ); + continue; + } + to_run.push((engine, detected.config.clone())); + } if !engine_warnings.is_empty() { ctx.add_diagnostics(engine_warnings); } - trace_event!(ctx, EventLevel::Debug, "using engine: {}", engine.name()); - - // Step 3: For markdown engine, skip execution (optimization) - // The markdown engine is a no-op, so we can avoid the serialize/parse round-trip - if engine.name() == "markdown" { + // Step 3: Fast path — nothing to execute (all markdown / no + // engines). Pass the AST through unchanged, exactly as the single + // markdown-engine optimization did. + if to_run.is_empty() { trace_event!( ctx, EventLevel::Debug, - "markdown engine - passing through unchanged" + "no executable engines — passing through unchanged" ); return Ok(PipelineData::DocumentAst(doc_ast)); } - // Step 4: Serialize AST to QMD for engine execution - let (qmd, qmd_source_info) = serialize_ast_to_qmd(&doc_ast.ast)?; - - trace_event!( - ctx, - EventLevel::Debug, - "serialized AST to {} bytes of QMD", - qmd.len() - ); - - // Step 5: Prepare execution context - // Clone source_context into Arc — it's finalized after include expansion. - let source_context = std::sync::Arc::new(doc_ast.source_context.clone()); - let exec_context = ExecutionContext::new( - ctx.temp_dir.clone(), - ctx.project.dir.clone(), - doc_ast.path.clone(), - &ctx.format.identifier.to_string(), - ) - .with_project_dir(if ctx.project.is_single_file { - None - } else { - Some(ctx.project.dir.clone()) - }) - .with_engine_config(detected.config.clone()) - .with_source_info(qmd_source_info, source_context); - - // Step 6: Execute the engine - trace_event!(ctx, EventLevel::Info, "executing engine: {}", engine.name()); + // Thread the AST, the merged (multi-slot) ASTContext, and warnings + // through the sequence. Slot 0 is the original `.qmd`; each engine + // appends one intermediate slot. `merged_context.filenames.len()` + // is the next FileId (files are added in lock-step with filenames). + let DocumentAst { + path, + ast, + ast_context, + source_context, + warnings, + recorded_includes, + } = doc_ast; + let mut ast = ast; + let mut merged_context = ast_context; + let mut warnings = warnings; + // Source context for engine error attribution. Constant across the + // sequence: it resolves the original document's FileIds exactly + // (as in the single-engine path). Offsets into content produced by + // an *earlier* engine don't resolve here (their FileIds live only + // in `merged_context`); engine error mapping is best-effort and + // treats an unresolved offset as "no precise location", so this is + // adequate. Precise cross-engine attribution is a possible + // follow-up. + let source_context_arc = std::sync::Arc::new(source_context.clone()); + + for (run_index, (engine, engine_config)) in to_run.into_iter().enumerate() { + // Serialize the current AST to QMD for this engine. + let (qmd, qmd_source_info) = serialize_ast_to_qmd(&ast)?; + trace_event!( + ctx, + EventLevel::Debug, + "[{}] serialized AST to {} bytes of QMD", + engine.name(), + qmd.len() + ); - let mut result = engine - .execute(&qmd, &exec_context) - .map_err(|e| PipelineError::stage_error(self.name(), e.to_string()))?; + let exec_context = ExecutionContext::new( + ctx.temp_dir.clone(), + ctx.project.dir.clone(), + path.clone(), + &ctx.format.identifier.to_string(), + ) + .with_project_dir(if ctx.project.is_single_file { + None + } else { + Some(ctx.project.dir.clone()) + }) + .with_engine_config(engine_config) + .with_source_info(qmd_source_info, source_context_arc.clone()); - trace_event!( - ctx, - EventLevel::Debug, - "engine produced {} bytes of markdown", - result.markdown.len() - ); + trace_event!(ctx, EventLevel::Info, "executing engine: {}", engine.name()); + let mut result = engine + .execute(&qmd, &exec_context) + .map_err(|e| PipelineError::stage_error(self.name(), e.to_string()))?; + trace_event!( + ctx, + EventLevel::Debug, + "[{}] engine produced {} bytes of markdown", + engine.name(), + result.markdown.len() + ); - // bd-45yw: emit the engine capture for trace recording. - // Must happen before the rest of the stage mutates `result` - // (draining `includes`, taking `supporting_files`) so the - // capture reflects the engine's full output. JsonTraceObserver - // recognizes ENGINE_CAPTURE_KIND and routes the payload to - // TraceDocument.engine_capture; other observers (CLI, WASM - // callbacks, NoopObserver) ignore the kind and stay quiet. - match serde_json::to_value(&result) { - Ok(result_json) => { - let payload = serde_json::json!({ - "engine_name": engine.name(), - "input_qmd": qmd, - "result": result_json, - }); - ctx.observer - .on_auxiliary_data(self.name(), 0, ENGINE_CAPTURE_KIND, &payload); + // bd-45yw / bd-5yff4: emit one engine capture per engine, with + // `run_index` distinguishing engines within this stage so the + // trace can carry an ordered capture per engine. Emitted before + // `result` is drained so the capture reflects the full output. + // JsonTraceObserver routes ENGINE_CAPTURE_KIND to the trace's + // engine capture(s); other observers ignore the kind. + match serde_json::to_value(&result) { + Ok(result_json) => { + let payload = serde_json::json!({ + "engine_name": engine.name(), + "input_qmd": qmd, + "result": result_json, + }); + ctx.observer.on_auxiliary_data( + self.name(), + run_index, + ENGINE_CAPTURE_KIND, + &payload, + ); + } + Err(e) => { + // Should not happen — ExecuteResult is fully + // serializable — but if it ever does we surface it + // without breaking the render path: the capture is a + // recording-time concern, not a correctness concern. + trace_event!( + ctx, + EventLevel::Warn, + "failed to serialize ExecuteResult for trace capture: {}", + e + ); + } } - Err(e) => { - // Should not happen — ExecuteResult is fully serializable — - // but if it ever does we surface it without breaking the - // render path: the capture is a recording-time concern, - // not a correctness-of-output concern. - trace_event!( - ctx, - EventLevel::Warn, - "failed to serialize ExecuteResult for trace capture: {}", - e + + // Accumulate engine-produced includes and supporting files + // (append-only; later engines add to earlier ones). See + // bd-o8pr for the supporting-files / resource-report contract. + ctx.includes + .header_includes + .extend(result.includes.header_includes); + ctx.includes + .include_before + .extend(result.includes.include_before); + ctx.includes + .include_after + .extend(result.includes.include_after); + if !result.supporting_files.is_empty() { + ctx.resource_report.add_engine_files( + engine.name(), + &path, + std::mem::take(&mut result.supporting_files), ); } - } - - // Save engine-produced includes (e.g., from knitr/jupyter) onto context - ctx.includes - .header_includes - .extend(result.includes.header_includes); - ctx.includes - .include_before - .extend(result.includes.include_before); - ctx.includes - .include_after - .extend(result.includes.include_after); - - // bd-o8pr Phase 2: route engine-emitted supporting files - // into the per-document resource report. The orchestrator - // drains this after Pass-2 and copies the entries into the - // output dir alongside static-channel resources. Engine - // contributions are append-only — there's no - // ResourceReportStage::remove. The report carries the doc - // source on each entry so it stays attributable after the - // per-doc reports are merged into the project-wide list. - if !result.supporting_files.is_empty() { - ctx.resource_report.add_engine_files( - engine.name(), - &doc_ast.path, - std::mem::take(&mut result.supporting_files), - ); - } - - // Step 7: Parse the executed markdown back to AST. - // - // The engine runs against an intermediate `<stem>.rmarkdown` file - // (knitr's convention — see `knitr::postprocess_markdown`). We parse - // with that name so `SourceInfo` attribution on new (engine-produced) - // blocks points at the intermediate file rather than the original. - // Blocks kept from the original AST keep their original attribution - // after the FileId merge below. - // - // Note: `result.markdown` is the engine's *output* buffer, so the - // SourceInfo byte offsets technically index into that buffer rather - // than the on-disk `.rmarkdown`. For the current use (filename - // attribution in the trace), this is adequate; a future refinement - // could expose the on-disk intermediate via the engine interface. - let intermediate_name = intermediate_filename(&doc_ast.path); - let (mut executed_ast, executed_ast_context, parse_warnings) = pampa::readers::qmd::read( - result.markdown.as_bytes(), - false, // loose mode - &intermediate_name, // filename for error messages - &mut std::io::sink(), - true, // track source locations - None, // file_id - ) - .map_err(|diagnostics| { - PipelineError::stage_error_with_diagnostics(self.name(), diagnostics) - })?; - // Step 7a: Build the merged ASTContext that covers BOTH files. - // - // Slot 0 = original `.qmd` (FileId(0) in original AST). Slot 1 = - // intermediate `.rmarkdown` (FileId(0) in executed AST — remapped - // to FileId(1) below). This keeps the reconcile contract intact: - // FileId in the reconciled AST identifies provenance. - let mut merged_ast_context = doc_ast.ast_context.clone(); - // `executed_ast_context` has the intermediate file as FileId(0) with - // the right FileInformation (line breaks, total length) — carry that - // into the merged context at slot 1. - if let Some(intermediate_file) = executed_ast_context - .source_context - .get_file(quarto_source_map::FileId(0)) - .cloned() - { - if let Some(info) = intermediate_file.file_info { - merged_ast_context - .source_context - .add_file_with_info(intermediate_name.clone(), info); + // Parse the executed markdown back to AST against a per-engine + // intermediate filename (`<stem>.<engine>.rmarkdown`) so + // engine-produced blocks attribute to a distinct slot. The + // SourceInfo offsets index into the engine's output buffer; for + // the current use (filename attribution in the trace) this is + // adequate. + let intermediate_name = intermediate_filename(&path, engine.name()); + let (mut executed_ast, executed_ast_context, parse_warnings) = + pampa::readers::qmd::read( + result.markdown.as_bytes(), + false, // loose mode + &intermediate_name, + &mut std::io::sink(), + true, // track source locations + None, // file_id + ) + .map_err(|diagnostics| { + PipelineError::stage_error_with_diagnostics(self.name(), diagnostics) + })?; + + // Register this engine's intermediate as a new file slot. The + // next FileId is the current file count; `filenames` grows in + // lock-step with the source context. + let new_slot = quarto_source_map::FileId(merged_context.filenames.len()); + if let Some(intermediate_file) = executed_ast_context + .source_context + .get_file(quarto_source_map::FileId(0)) + .cloned() + { + if let Some(info) = intermediate_file.file_info { + merged_context + .source_context + .add_file_with_info(intermediate_name.clone(), info); + } else { + merged_context + .source_context + .add_file(intermediate_name.clone(), None); + } } else { - merged_ast_context + merged_context .source_context .add_file(intermediate_name.clone(), None); } - } else { - merged_ast_context - .source_context - .add_file(intermediate_name.clone(), None); - } - merged_ast_context.filenames.push(intermediate_name); - // Example-list counter is cell-ordering state tied to the executed - // AST. Preserve the executed parse's final value so subsequent - // example-list numbering stays coherent. - merged_ast_context - .example_list_counter - .set(executed_ast_context.example_list_counter.get()); - - // Step 7b: Pre-remap the executed AST so its `FileId(0)` references - // become `FileId(1)` (the intermediate file's slot in the merged - // context). After this, kept original blocks still reference - // `FileId(0)` (the `.qmd`) and new executed blocks reference - // `FileId(1)` (the intermediate). - quarto_ast_reconcile::remap_file_ids(&mut executed_ast, &|id| { - quarto_source_map::FileId(id.0 + 1) - }); + merged_context.filenames.push(intermediate_name); + // Carry the executed parse's example-list counter forward; the + // last engine's value wins so downstream numbering is coherent. + merged_context + .example_list_counter + .set(executed_ast_context.example_list_counter.get()); + + // Remap the executed AST's `FileId(0)` to this engine's slot. + // Kept original blocks keep their (lower) FileIds; new blocks + // reference `new_slot`. + quarto_ast_reconcile::remap_file_ids(&mut executed_ast, &|id| { + quarto_source_map::FileId(id.0 + new_slot.0) + }); + + // Reconcile: keep unchanged content's source locations, take + // new content from the executed AST. The "original" side may + // already carry FileIds from earlier engines; reconcile decides + // keep/replace by content, so mixed provenance is fine. + let (reconciled_ast, reconciliation_plan) = + quarto_ast_reconcile::reconcile(ast, executed_ast); + ast = reconciled_ast; + warnings.extend(parse_warnings); - // Step 8: Reconcile source locations - // For content that hasn't changed, preserve original source locations. - // For new content (execution outputs), use locations from executed AST. - // Uses the three-phase reconciliation algorithm from quarto-ast-reconcile. - let (reconciled_ast, reconciliation_plan) = - quarto_ast_reconcile::reconcile(doc_ast.ast, executed_ast); - - trace_event!( - ctx, - EventLevel::Debug, - "reconciliation: {} kept, {} replaced, {} recursed", - reconciliation_plan.stats.blocks_kept, - reconciliation_plan.stats.blocks_replaced, - reconciliation_plan.stats.blocks_recursed - ); + trace_event!( + ctx, + EventLevel::Debug, + "[{}] reconciliation: {} kept, {} replaced, {} recursed", + engine.name(), + reconciliation_plan.stats.blocks_kept, + reconciliation_plan.stats.blocks_replaced, + reconciliation_plan.stats.blocks_recursed + ); - // Step 9: Collect warnings - let mut warnings = doc_ast.warnings; - warnings.extend(parse_warnings); + // bd-5yff4: record one AST snapshot per engine so a trace can + // show the sequence step by step (`engine:<name>`). The + // observer ignores this unless trace recording is active. + ctx.observer + .on_engine_data(engine.name(), run_index, &ast, &merged_context); + } - // Step 10: Return updated DocumentAst Ok(PipelineData::DocumentAst(DocumentAst { - path: doc_ast.path, - ast: reconciled_ast, - ast_context: merged_ast_context, - source_context: doc_ast.source_context, + path, + ast, + ast_context: merged_context, + source_context, warnings, - recorded_includes: doc_ast.recorded_includes, + recorded_includes, })) } } -/// Derive the intermediate filename engines see from the source path. +/// Derive the per-engine intermediate filename from the source path. /// -/// Mirrors knitr's convention: `foo.qmd` → `foo.rmarkdown`. Used only as a -/// filename label for source attribution; no file needs to exist on disk. -fn intermediate_filename(source_path: &std::path::Path) -> String { - let mut with_ext = source_path.to_path_buf(); - with_ext.set_extension("rmarkdown"); - with_ext.display().to_string() +/// `foo.qmd` + engine `knitr` → `foo.knitr.rmarkdown`. Echoes knitr's +/// `.rmarkdown` convention but tags the label with the engine name so a +/// multi-engine sequence (bd-5yff4) gives each engine's produced blocks a +/// distinct provenance slot. Used only as a filename label for source +/// attribution; no file needs to exist on disk. +fn intermediate_filename(source_path: &std::path::Path, engine_name: &str) -> String { + let stem = source_path + .file_stem() + .map(|s| s.to_string_lossy().into_owned()) + .unwrap_or_default(); + source_path + .with_file_name(format!("{stem}.{engine_name}.rmarkdown")) + .display() + .to_string() } /// Serialize a Pandoc AST to QMD text. @@ -904,12 +946,12 @@ mod tests { let output = stage.run(input, &mut ctx).await.unwrap(); let result = output.into_document_ast().expect("Should be DocumentAst"); - // Filenames are the two-slot merged context. + // Filenames are the two-slot merged context (per-engine label). assert_eq!( result.ast_context.filenames, vec![ "/project/test.qmd".to_string(), - "/project/test.rmarkdown".to_string(), + "/project/test.mock-appending.rmarkdown".to_string(), ] ); @@ -936,6 +978,348 @@ mod tests { } } + // ────────────────────────────────────────────────────────────── + // bd-5yff4 Phase 1: sequential multi-engine execution + // ────────────────────────────────────────────────────────────── + + /// Two engines run in declared order, and the first engine's output + /// feeds the second: `fixture-a` fills its cell with a `{fixture-b}` + /// cell, which `fixture-b` (next in the sequence) then fills. The + /// final AST must contain only the second engine's output, with both + /// engines' source cells consumed. + #[tokio::test] + async fn test_two_engines_run_in_sequence_with_handoff() { + use crate::engine::FixtureEngine; + + let mut registry = EngineRegistry::new(); + registry.register(Arc::new(FixtureEngine::with_results( + "fixture-a", + vec!["```{fixture-b}\nhanded-off\n```".to_string()], + ))); + registry.register(Arc::new(FixtureEngine::with_results( + "fixture-b", + vec!["Final output from B.".to_string()], + ))); + + let stage = EngineExecutionStage::with_registry(registry); + let mut ctx = make_test_context(); + + let content = b"---\nengine: [fixture-a, fixture-b]\n---\n\n```{fixture-a}\nseed\n```\n"; + let doc_ast = parse_qmd_to_ast(content, "/project/test.qmd"); + + let input = PipelineData::DocumentAst(doc_ast); + let output = stage.run(input, &mut ctx).await.unwrap(); + let result = output.into_document_ast().expect("DocumentAst"); + + let qmd = serialize_ast_to_qmd(&result.ast).unwrap().0; + assert!( + qmd.contains("Final output from B."), + "second engine's output should be present; got:\n{qmd}" + ); + assert!( + !qmd.contains("{fixture-a}"), + "fixture-a cell should be consumed; got:\n{qmd}" + ); + assert!( + !qmd.contains("{fixture-b}"), + "fixture-b cell should be consumed; got:\n{qmd}" + ); + } + + /// FileId coherence across a two-engine sequence: kept original blocks + /// keep `FileId(0)` (the `.qmd`), blocks produced by the first engine + /// get `FileId(1)` (its intermediate), and blocks produced by the + /// second get `FileId(2)` (its intermediate). No stray FileIds. + #[tokio::test] + async fn test_two_engines_assign_distinct_file_ids() { + use crate::engine::FixtureEngine; + + let mut registry = EngineRegistry::new(); + registry.register(Arc::new(FixtureEngine::with_results( + "fixture-a", + vec!["Para from A.".to_string()], + ))); + registry.register(Arc::new(FixtureEngine::with_results( + "fixture-b", + vec!["Para from B.".to_string()], + ))); + + let stage = EngineExecutionStage::with_registry(registry); + let mut ctx = make_test_context(); + + // One kept prose block + one cell per engine (both present from + // the start; fixture-a leaves fixture-b's cell untouched). + let content = b"---\nengine: [fixture-a, fixture-b]\n---\n\nOriginal prose.\n\n```{fixture-a}\na\n```\n\n```{fixture-b}\nb\n```\n"; + let doc_ast = parse_qmd_to_ast(content, "/project/test.qmd"); + + let input = PipelineData::DocumentAst(doc_ast); + let output = stage.run(input, &mut ctx).await.unwrap(); + let result = output.into_document_ast().expect("DocumentAst"); + + // Three slots: .qmd, fixture-a intermediate, fixture-b intermediate. + assert_eq!( + result.ast_context.filenames, + vec![ + "/project/test.qmd".to_string(), + "/project/test.fixture-a.rmarkdown".to_string(), + "/project/test.fixture-b.rmarkdown".to_string(), + ], + "expected three file slots (original + one per engine)" + ); + + let ids = collect_file_ids(&result.ast); + assert!( + ids.contains(&quarto_source_map::FileId(0)), + "FileId(0): {ids:?}" + ); + assert!( + ids.contains(&quarto_source_map::FileId(1)), + "FileId(1): {ids:?}" + ); + assert!( + ids.contains(&quarto_source_map::FileId(2)), + "FileId(2): {ids:?}" + ); + for id in &ids { + assert!(id.0 < 3, "unexpected FileId {id:?} (3 slots): {ids:?}"); + } + } + + /// A duplicate engine in the sequence is de-duplicated (run once) and + /// produces a "Duplicate engine" diagnostic. + #[tokio::test] + async fn test_duplicate_engine_dedups_and_warns() { + use crate::engine::FixtureEngine; + + let mut registry = EngineRegistry::new(); + registry.register(Arc::new(FixtureEngine::with_results( + "fixture-a", + vec!["spliced once".to_string()], + ))); + + let stage = EngineExecutionStage::with_registry(registry); + let mut ctx = make_test_context(); + + // engine declared twice; one fixture-a cell. If dedup failed, + // fixture-a would run twice and the second run's count check + // (0 cells vs 1 result) would error. + let content = b"---\nengine: [fixture-a, fixture-a]\n---\n\n```{fixture-a}\nx\n```\n"; + let doc_ast = parse_qmd_to_ast(content, "/project/test.qmd"); + + let input = PipelineData::DocumentAst(doc_ast); + let output = stage.run(input, &mut ctx).await.unwrap(); + let result = output.into_document_ast().expect("DocumentAst"); + + let qmd = serialize_ast_to_qmd(&result.ast).unwrap().0; + assert!(qmd.contains("spliced once"), "got:\n{qmd}"); + // Only one intermediate slot — the engine ran exactly once. + assert_eq!(result.ast_context.filenames.len(), 2); + assert!( + ctx.diagnostics + .iter() + .any(|d| d.title.contains("Duplicate engine 'fixture-a'")), + "expected a duplicate-engine diagnostic; got: {:?}", + ctx.diagnostics + ); + } + + /// A markdown engine anywhere in the sequence is skipped (no-op), + /// while the other engines still run. + #[tokio::test] + async fn test_markdown_in_sequence_is_skipped() { + use crate::engine::FixtureEngine; + + let mut registry = EngineRegistry::new(); + registry.register(Arc::new(FixtureEngine::with_results( + "fixture-a", + vec!["from A".to_string()], + ))); + + let stage = EngineExecutionStage::with_registry(registry); + let mut ctx = make_test_context(); + + let content = b"---\nengine: [markdown, fixture-a]\n---\n\n```{fixture-a}\nx\n```\n"; + let doc_ast = parse_qmd_to_ast(content, "/project/test.qmd"); + + let input = PipelineData::DocumentAst(doc_ast); + let output = stage.run(input, &mut ctx).await.unwrap(); + let result = output.into_document_ast().expect("DocumentAst"); + + let qmd = serialize_ast_to_qmd(&result.ast).unwrap().0; + assert!(qmd.contains("from A"), "got:\n{qmd}"); + // Only fixture-a added a slot; markdown was skipped. + assert_eq!( + result.ast_context.filenames, + vec![ + "/project/test.qmd".to_string(), + "/project/test.fixture-a.rmarkdown".to_string(), + ] + ); + } + + /// bd-5yff4 Phase 3: a multi-engine run records one `EngineCapture` + /// per engine (in order) AND one `engine:<name>` AST snapshot per + /// engine in the trace pipeline. Round-trips through a real gzipped + /// trace on disk. + #[tokio::test] + async fn test_multi_engine_trace_records_per_engine_snapshots_and_captures() { + use crate::engine::FixtureEngine; + use crate::stage::JsonTraceObserver; + use quarto_trace::RenderInfo; + + let dir = std::env::temp_dir().join(format!( + "quarto-trace-multi-engine-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_nanos()) + .unwrap_or(0) + )); + let _ = std::fs::remove_dir_all(&dir); + let trace_path = dir.join("latest.json.gz"); + + let observer = Arc::new(JsonTraceObserver::new( + trace_path.clone(), + RenderInfo { + input_path: Some("/project/test.qmd".into()), + format_target: Some("html".into()), + git_hash: Some(quarto_trace::BUILD_GIT_HASH.to_string()), + ..Default::default() + }, + )); + + let mut registry = EngineRegistry::new(); + registry.register(Arc::new(FixtureEngine::with_results( + "fixture-a", + vec!["```{fixture-b}\nfrom-a\n```".to_string()], + ))); + registry.register(Arc::new(FixtureEngine::with_results( + "fixture-b", + vec!["Final B".to_string()], + ))); + let stage = EngineExecutionStage::with_registry(registry); + let mut ctx = make_test_context(); + ctx.observer = observer.clone(); + + let content = b"---\nengine: [fixture-a, fixture-b]\n---\n\n```{fixture-a}\nseed\n```\n"; + let doc_ast = parse_qmd_to_ast(content, "/project/test.qmd"); + stage + .run(PipelineData::DocumentAst(doc_ast), &mut ctx) + .await + .unwrap(); + + observer.write_trace().unwrap(); + let read_back = quarto_trace::read::read_trace(&trace_path).unwrap(); + + // One capture per engine, in execution order. + assert_eq!( + read_back + .engine_captures + .iter() + .map(|c| c.engine_name.as_str()) + .collect::<Vec<_>>(), + vec!["fixture-a", "fixture-b"] + ); + + // One AST snapshot per engine, in order. + let engine_entries: Vec<_> = read_back + .pipeline + .iter() + .filter(|e| e.stage.starts_with("engine:")) + .map(|e| e.stage.as_str()) + .collect(); + assert_eq!(engine_entries, vec!["engine:fixture-a", "engine:fixture-b"]); + + let _ = std::fs::remove_dir_all(&dir); + } + + /// bd-5yff4 Phase 3: record the captures from a two-engine run, then + /// replay them via `with_replay_many` against the same document. The + /// replay must be byte-clean — engine N+1's input is engine N's + /// reconciled-then-reserialized output, so this is the determinism + /// invariant the sequence relies on — and produce identical output. + #[tokio::test] + async fn test_multi_engine_record_then_replay_is_byte_clean() { + use crate::engine::{EngineRegistry, FixtureEngine, ReplayEngine}; + use crate::stage::JsonTraceObserver; + use quarto_trace::RenderInfo; + + let content = b"---\nengine: [fixture-a, fixture-b]\n---\n\n```{fixture-a}\nseed\n```\n"; + + // --- Record pass: real fixture engines + trace observer. --- + let dir = std::env::temp_dir().join(format!( + "quarto-replay-multi-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_nanos()) + .unwrap_or(0) + )); + let _ = std::fs::remove_dir_all(&dir); + let trace_path = dir.join("latest.json.gz"); + let observer = Arc::new(JsonTraceObserver::new( + trace_path.clone(), + RenderInfo::default(), + )); + + let mut registry = EngineRegistry::new(); + registry.register(Arc::new(FixtureEngine::with_results( + "fixture-a", + vec!["```{fixture-b}\nfrom-a\n```".to_string()], + ))); + registry.register(Arc::new(FixtureEngine::with_results( + "fixture-b", + vec!["Final B".to_string()], + ))); + let stage = EngineExecutionStage::with_registry(registry); + let mut ctx = make_test_context(); + ctx.observer = observer.clone(); + + let output_recorded = stage + .run( + PipelineData::DocumentAst(parse_qmd_to_ast(content, "/project/test.qmd")), + &mut ctx, + ) + .await + .unwrap() + .into_document_ast() + .unwrap(); + let recorded_qmd = serialize_ast_to_qmd(&output_recorded.ast).unwrap().0; + + observer.write_trace().unwrap(); + let captures = quarto_trace::read::read_trace(&trace_path) + .unwrap() + .engine_captures; + assert_eq!(captures.len(), 2); + + // --- Replay pass: substitute a ReplayEngine per recorded engine. --- + let mut replay_registry = EngineRegistry::new(); + for capture in captures { + replay_registry.register(Arc::new(ReplayEngine::new(capture))); + } + let replay_stage = EngineExecutionStage::with_registry(replay_registry); + let mut replay_ctx = make_test_context(); + + let output_replayed = replay_stage + .run( + PipelineData::DocumentAst(parse_qmd_to_ast(content, "/project/test.qmd")), + &mut replay_ctx, + ) + .await + .expect("replay of a recorded two-engine sequence must succeed byte-clean") + .into_document_ast() + .unwrap(); + let replayed_qmd = serialize_ast_to_qmd(&output_replayed.ast).unwrap().0; + + assert_eq!( + replayed_qmd, recorded_qmd, + "replay output must match the recorded run exactly" + ); + assert!(replayed_qmd.contains("Final B")); + + let _ = std::fs::remove_dir_all(&dir); + } + /// After engine execution, the merged `ASTContext` must carry BOTH the /// original `.qmd` filename and the intermediate `<stem>.rmarkdown` /// filename the engine saw. Regression test for bd-b0f2 Phase 2. @@ -963,7 +1347,7 @@ mod tests { filenames ); assert_eq!(filenames[0], "/project/test.qmd"); - assert_eq!(filenames[1], "/project/test.rmarkdown"); + assert_eq!(filenames[1], "/project/test.mock-includes.rmarkdown"); } #[tokio::test] @@ -1319,10 +1703,12 @@ mod tests { observer.write_trace().unwrap(); let read_back = quarto_trace::read::read_trace(&trace_path).unwrap(); - let capture = read_back - .engine_capture - .as_ref() - .expect("engine_capture must be populated"); + assert_eq!( + read_back.engine_captures.len(), + 1, + "exactly one engine capture must be recorded" + ); + let capture = &read_back.engine_captures[0]; assert_eq!(capture.engine_name, "mock-includes"); assert!( !capture.input_qmd.is_empty(), @@ -1438,16 +1824,21 @@ mod tests { let asts = raw_json["asts"] .as_object() .expect("v2 unified artifact must have an `asts` map"); + // The three identical pre-populated DocumentAst snapshots collapse + // to one stored entry; the per-engine `on_engine_data` snapshot + // (bd-5yff4) is a distinct post-reconcile AST, so two are stored. + // The key invariant is that dedup happened — without it the three + // identical snapshots alone would store three entries. assert_eq!( asts.len(), - 1, - "three identical AST snapshots must collapse to one stored entry" + 2, + "3 identical snapshots collapse to 1; the per-engine snapshot adds 1" ); - assert!( - !raw_json["engine_capture"].is_null(), - "v2 unified artifact must have an `engine_capture` field for non-markdown engines" + let captures = raw_json["engine_captures"].as_array().expect( + "v2 unified artifact must have an `engine_captures` array for non-markdown engines", ); - assert_eq!(raw_json["engine_capture"]["engine_name"], "mock-includes"); + assert_eq!(captures.len(), 1, "one engine ran → one capture"); + assert_eq!(captures[0]["engine_name"], "mock-includes"); // 2. read_trace round-trip: rehydrated doc has both deduped // ASTs (folded back inline; `asts` empty) and the engine @@ -1457,10 +1848,12 @@ mod tests { read_back.asts.is_empty(), "reader must clear `asts` after rehydration" ); - let capture = read_back - .engine_capture - .as_ref() - .expect("engine_capture must round-trip through gzipped trace"); + assert_eq!( + read_back.engine_captures.len(), + 1, + "engine capture must round-trip through gzipped trace" + ); + let capture = &read_back.engine_captures[0]; assert_eq!(capture.engine_name, "mock-includes"); let parsed: crate::engine::ExecuteResult = serde_json::from_value(capture.result.clone()) .expect("result must round-trip back to ExecuteResult"); diff --git a/crates/quarto-core/src/stage/trace.rs b/crates/quarto-core/src/stage/trace.rs index 17d37b4e9..0aabae130 100644 --- a/crates/quarto-core/src/stage/trace.rs +++ b/crates/quarto-core/src/stage/trace.rs @@ -206,6 +206,34 @@ impl PipelineObserver for JsonTraceObserver { ); } + fn on_engine_data( + &self, + engine_name: &str, + index: usize, + ast: &quarto_pandoc_types::pandoc::Pandoc, + ast_context: &pampa::pandoc::ASTContext, + ) { + // bd-5yff4: one AST snapshot per engine in a multi-engine + // sequence, recorded as an `engine:<name>` entry — mirrors the + // `transform:<name>` sub-entries. The dedup pass collapses + // snapshots that are byte-identical to a neighbor, so the size + // cost is bounded. + let data_json = serialize_pandoc_ast(ast, ast_context); + let mut state = self.state.lock().unwrap(); + Self::push_entry( + &mut state, + TraceEntry { + stage: format!("engine:{}", engine_name), + index, + data_kind: Some(PipelineDataKind::DocumentAst.to_string()), + data: Some(data_json), + duration_ms: None, + status: StageStatus::Ok, + error: None, + }, + ); + } + fn on_auxiliary_data(&self, stage: &str, index: usize, kind: &str, data: &serde_json::Value) { let mut state = self.state.lock().unwrap(); // bd-45yw: route the typed engine capture to the dedicated @@ -215,7 +243,10 @@ impl PipelineObserver for JsonTraceObserver { if kind == ENGINE_CAPTURE_KIND { match serde_json::from_value::<quarto_trace::EngineCapture>(data.clone()) { Ok(capture) => { - state.doc.engine_capture = Some(capture); + // bd-5yff4: one capture per engine, in execution order. + // The stage emits these sequentially (run_index order), + // so push preserves order. + state.doc.engine_captures.push(capture); return; } Err(e) => { @@ -639,7 +670,7 @@ mod tests { } /// bd-45yw: an `on_auxiliary_data` event with kind - /// `"EngineCapture"` must land on `TraceDocument.engine_capture` + /// `"EngineCapture"` must land on `TraceDocument.engine_captures` /// (the typed slot), not as a generic pipeline aux entry. Recording /// the engine output is what makes a trace double as a replay /// fixture. @@ -674,14 +705,15 @@ mod tests { .pipeline .iter() .any(|e| e.stage == "aux:engine-execution"), - "EngineCapture should be routed to engine_capture, not pipeline aux" + "EngineCapture should be routed to engine_captures, not pipeline aux" ); // The typed slot must hold the capture. - let cap = state - .doc - .engine_capture - .as_ref() - .expect("engine_capture should be populated"); + assert_eq!( + state.doc.engine_captures.len(), + 1, + "exactly one capture should be recorded" + ); + let cap = &state.doc.engine_captures[0]; assert_eq!(cap.engine_name, "jupyter"); assert_eq!(cap.input_qmd, "---\nengine: jupyter\n---\n"); assert_eq!(cap.result["markdown"], "out\n"); @@ -702,7 +734,7 @@ mod tests { ); let state = observer.state.lock().unwrap(); - assert!(state.doc.engine_capture.is_none()); + assert!(state.doc.engine_captures.is_empty()); assert_eq!(state.doc.pipeline.len(), 1); assert_eq!(state.doc.pipeline[0].stage, "aux:crossref-index"); assert_eq!( diff --git a/crates/quarto-core/tests/integration/engine_merge.rs b/crates/quarto-core/tests/integration/engine_merge.rs new file mode 100644 index 000000000..d6e1c441f --- /dev/null +++ b/crates/quarto-core/tests/integration/engine_merge.rs @@ -0,0 +1,135 @@ +//! Metadata-merge interaction tests for the `engine:` key (bd-5yff4). +//! +//! These pin down how a multi-engine `engine:` array composes across +//! config layers (project `_quarto.yml` → directory `_metadata.yml` → +//! document front matter) under the general merge rules in +//! `quarto-config`, and how the resolved value is then interpreted by +//! `detect_engine_sequence`. They are the executable form of the +//! "verified merge behavior" table in +//! `claude-notes/plans/2026-05-27-multi-engine-execution.md`. +//! +//! Key facts they lock in: +//! - Arrays merge by `!concat` (default): layers accumulate. +//! - `!prefer` replaces the array wholesale. +//! - The *kind* of the highest-priority layer that sets `engine:` wins: +//! a scalar layer drops a lower array layer, and an array layer drops a +//! lower scalar layer. +//! - Engines must be distinct; duplicates from `!concat` across layers +//! are de-duplicated (first kept) with the drops reported. + +use quarto_config::MergedConfig; +use quarto_core::engine::detect_engine_sequence; +use quarto_pandoc_types::ConfigMapEntry; +use quarto_pandoc_types::config_value::{ConfigValue, MergeOp}; +use quarto_source_map::SourceInfo; + +fn s(x: &str) -> ConfigValue { + ConfigValue::new_string(x, SourceInfo::default()) +} + +fn map(entries: Vec<(&str, ConfigValue)>) -> ConfigValue { + let info = SourceInfo::default(); + let map_entries: Vec<ConfigMapEntry> = entries + .into_iter() + .map(|(k, v)| ConfigMapEntry { + key: k.to_string(), + key_source: info.clone(), + value: v, + }) + .collect(); + ConfigValue::new_map(map_entries, info) +} + +fn arr(items: Vec<ConfigValue>) -> ConfigValue { + ConfigValue::new_array(items, SourceInfo::default()) +} + +/// Merge layers (lowest priority first) via the production merge +/// machinery and return the flat `ConfigValue` that would be assigned to +/// `ast.meta`. +fn merge(layers: Vec<&ConfigValue>) -> ConfigValue { + MergedConfig::new(layers) + .materialize() + .expect("materialize") +} + +/// Resolve the engine names a merged metadata map yields. +fn engine_names(meta: &ConfigValue) -> Vec<String> { + detect_engine_sequence(meta) + .engines + .into_iter() + .map(|e| e.name) + .collect() +} + +#[test] +fn concat_default_accumulates_engine_arrays() { + // project engine: [knitr] + doc engine: [mermaidjs] → [knitr, mermaidjs] + let project = map(vec![("engine", arr(vec![s("knitr")]))]); + let document = map(vec![("engine", arr(vec![s("mermaidjs")]))]); + let merged = merge(vec![&project, &document]); + assert_eq!(engine_names(&merged), vec!["knitr", "mermaidjs"]); +} + +#[test] +fn concat_accumulates_across_three_layers_in_order() { + let project = map(vec![("engine", arr(vec![s("knitr")]))]); + let directory = map(vec![("engine", arr(vec![s("jupyter")]))]); + let document = map(vec![("engine", arr(vec![s("mermaidjs")]))]); + let merged = merge(vec![&project, &directory, &document]); + assert_eq!(engine_names(&merged), vec!["knitr", "jupyter", "mermaidjs"]); +} + +#[test] +fn prefer_replaces_engine_array() { + // doc uses !prefer → its array replaces the project's. + let project = map(vec![("engine", arr(vec![s("knitr"), s("jupyter")]))]); + let document = map(vec![( + "engine", + arr(vec![s("mermaidjs")]).with_merge_op(MergeOp::Prefer), + )]); + let merged = merge(vec![&project, &document]); + assert_eq!(engine_names(&merged), vec!["mermaidjs"]); +} + +#[test] +fn scalar_doc_overrides_lower_array() { + // Highest layer is a scalar → it wins; the lower array is dropped. + let project = map(vec![("engine", arr(vec![s("knitr")]))]); + let document = map(vec![("engine", s("jupyter"))]); + let merged = merge(vec![&project, &document]); + assert_eq!(engine_names(&merged), vec!["jupyter"]); +} + +#[test] +fn array_doc_drops_lower_scalar() { + // Highest layer is an array → array semantics apply, and the lower + // *scalar* layer is not folded in (documented gotcha). + let project = map(vec![("engine", s("jupyter"))]); + let document = map(vec![("engine", arr(vec![s("knitr")]))]); + let merged = merge(vec![&project, &document]); + assert_eq!(engine_names(&merged), vec!["knitr"]); +} + +#[test] +fn duplicate_engine_across_layers_is_deduped() { + // The one shape that yields a duplicate: two array layers naming the + // same engine. Dedup keeps the first; the drop is reported. + let project = map(vec![("engine", arr(vec![s("jupyter")]))]); + let document = map(vec![("engine", arr(vec![s("jupyter")]))]); + let merged = merge(vec![&project, &document]); + + let seq = detect_engine_sequence(&merged); + let names: Vec<_> = seq.engines.iter().map(|e| e.name.as_str()).collect(); + assert_eq!(names, vec!["jupyter"]); + assert_eq!(seq.dropped_duplicates, vec!["jupyter".to_string()]); +} + +#[test] +fn single_scalar_engine_unchanged_by_merge() { + // Back-compat: a lone scalar engine still resolves to one engine. + let project = map(vec![("engine", s("knitr"))]); + let document = map(vec![("title", s("Doc"))]); + let merged = merge(vec![&project, &document]); + assert_eq!(engine_names(&merged), vec!["knitr"]); +} diff --git a/crates/quarto-core/tests/integration/main.rs b/crates/quarto-core/tests/integration/main.rs index b1c1bf3d2..7a66f9331 100644 --- a/crates/quarto-core/tests/integration/main.rs +++ b/crates/quarto-core/tests/integration/main.rs @@ -15,6 +15,7 @@ pub mod bootstrap_js_pipeline; pub mod brand_render; pub mod crossref_fixtures; pub mod document_profile_pipeline; +pub mod engine_merge; pub mod include_resolve_pipeline; pub mod incremental_rebuild; pub mod jupyter_integration; diff --git a/crates/quarto-core/tests/integration/project_resources.rs b/crates/quarto-core/tests/integration/project_resources.rs index a69669495..571ca76a1 100644 --- a/crates/quarto-core/tests/integration/project_resources.rs +++ b/crates/quarto-core/tests/integration/project_resources.rs @@ -671,7 +671,7 @@ mod orchestrator_engine_channel { quarto_core::project::ProjectContext::discover(&project_dir, runtime.as_ref()).unwrap(); let options = RenderToFileOptions { - replay_capture: Some(capture), + replay_captures: vec![capture], ..Default::default() }; diff --git a/crates/quarto-core/tests/integration/replay_engine.rs b/crates/quarto-core/tests/integration/replay_engine.rs index 98ba67ae5..6b6845d9b 100644 --- a/crates/quarto-core/tests/integration/replay_engine.rs +++ b/crates/quarto-core/tests/integration/replay_engine.rs @@ -160,7 +160,7 @@ fn replay_capture_in_options_overrides_engine_through_render_to_file() { }; let options = RenderToFileOptions { - replay_capture: Some(capture), + replay_captures: vec![capture], ..Default::default() }; @@ -220,7 +220,7 @@ fn replay_capture_miss_surfaces_as_render_error() { }; let options = RenderToFileOptions { - replay_capture: Some(capture), + replay_captures: vec![capture], ..Default::default() }; @@ -249,7 +249,7 @@ fn replay_capture_absent_uses_default_registry() { write_file(&qmd_path, "---\ntitle: Test\n---\n\n# Hello\n\nWorld\n"); let options = RenderToFileOptions { - replay_capture: None, + replay_captures: Vec::new(), ..Default::default() }; diff --git a/crates/quarto-preview/src/cache.rs b/crates/quarto-preview/src/cache.rs index 1960fc247..1a74a6ec7 100644 --- a/crates/quarto-preview/src/cache.rs +++ b/crates/quarto-preview/src/cache.rs @@ -65,12 +65,17 @@ pub fn cache_file_path(cache_dir: &Path, key: &str) -> PathBuf { cache_dir.join(format!("{key}.{CACHE_FILE_EXT}")) } -/// Look up a cached capture. Returns `Ok(None)` for a clean miss -/// (no file at that path); `Ok(Some(_))` on hit; `Err` if the file -/// is present but unreadable or unparseable. Callers in the driver -/// downgrade `Err` to a miss + a `tracing::warn!` so a corrupt -/// cache entry doesn't take the preview server down. -pub fn read_cached(cache_dir: &Path, key: &str) -> std::io::Result<Option<EngineCapture>> { +/// Look up a cached capture sequence (bd-5yff4 — one per engine, in +/// order). Returns `Ok(None)` for a clean miss (no file at that path); +/// `Ok(Some(vec))` on hit; `Err` if the file is present but unreadable +/// or unparseable. Callers in the driver downgrade `Err` to a miss + a +/// `tracing::warn!` so a corrupt cache entry doesn't take the preview +/// server down. +/// +/// For robustness against a stale pre-bd-5yff4 entry (a single capture +/// object), a failed array parse falls back to a single-object parse +/// wrapped in a vec. +pub fn read_cached(cache_dir: &Path, key: &str) -> std::io::Result<Option<Vec<EngineCapture>>> { let path = cache_file_path(cache_dir, key); let file = match std::fs::File::open(&path) { Ok(f) => f, @@ -80,20 +85,32 @@ pub fn read_cached(cache_dir: &Path, key: &str) -> std::io::Result<Option<Engine let mut decoder = flate2::read::GzDecoder::new(file); let mut json_bytes = Vec::new(); decoder.read_to_end(&mut json_bytes)?; - let capture: EngineCapture = serde_json::from_slice(&json_bytes) - .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?; - Ok(Some(capture)) + match serde_json::from_slice::<Vec<EngineCapture>>(&json_bytes) { + Ok(captures) => Ok(Some(captures)), + Err(array_err) => match serde_json::from_slice::<EngineCapture>(&json_bytes) { + Ok(single) => Ok(Some(vec![single])), + Err(_) => Err(std::io::Error::new( + std::io::ErrorKind::InvalidData, + array_err, + )), + }, + } } -/// Persist a capture to the cache. Creates `cache_dir` (and parents) -/// if necessary. The write is atomic: temp file → fsync → rename. -pub fn write_cached(cache_dir: &Path, key: &str, capture: &EngineCapture) -> std::io::Result<()> { +/// Persist a capture sequence to the cache. Creates `cache_dir` (and +/// parents) if necessary. The write is atomic: temp file → fsync → +/// rename. The payload is a gzipped JSON **array** of captures. +pub fn write_cached( + cache_dir: &Path, + key: &str, + captures: &[EngineCapture], +) -> std::io::Result<()> { std::fs::create_dir_all(cache_dir)?; let tmp_path = cache_dir.join(format!("{key}.{CACHE_FILE_EXT}.tmp")); let final_path = cache_file_path(cache_dir, key); - let json = serde_json::to_vec(capture) + let json = serde_json::to_vec(captures) .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?; // Wrap the file in a scope so the encoder + file close before @@ -127,8 +144,8 @@ pub fn write_cached(cache_dir: &Path, key: &str, capture: &EngineCapture) -> std /// authoritative capture is still emitted to the caller, and a future /// invocation simply re-runs the engine. /// -/// Returns `Ok(None)` exactly when `record_capture` would (no engine -/// in play for this doc); `Ok(Some(_))` on either path; `Err` on +/// Returns an empty vec exactly when `record_capture` would (no engine +/// in play for this doc); the recorded sequence on either path; `Err` on /// pipeline failures (cache read failures are downgraded to a miss). pub async fn record_capture_cached( cache_dir: &Path, @@ -136,7 +153,7 @@ pub async fn record_capture_cached( project: &ProjectContext, runtime: Arc<dyn SystemRuntime>, engine_registry: Option<EngineRegistry>, -) -> Result<Option<EngineCapture>, PipelineError> { +) -> Result<Vec<EngineCapture>, PipelineError> { // Compute the same input_qmd the engine would receive — this // doubles as the cache key derivation and as the source of truth // for what we hash. compute_input_qmd already runs the pre-engine @@ -146,9 +163,9 @@ pub async fn record_capture_cached( let key = compute_key(&input_qmd); match read_cached(cache_dir, &key) { - Ok(Some(capture)) => { + Ok(Some(captures)) => { tracing::debug!(path = %path.display(), key = %key, "engine capture cache hit"); - return Ok(Some(capture)); + return Ok(captures); } Ok(None) => {} Err(e) => { @@ -161,9 +178,9 @@ pub async fn record_capture_cached( } } - let capture = record_capture(path, project, runtime, engine_registry).await?; - if let Some(ref c) = capture { - if let Err(e) = write_cached(cache_dir, &key, c) { + let captures = record_capture(path, project, runtime, engine_registry).await?; + if !captures.is_empty() { + if let Err(e) = write_cached(cache_dir, &key, &captures) { tracing::warn!( path = %path.display(), key = %key, @@ -172,7 +189,7 @@ pub async fn record_capture_cached( ); } } - Ok(capture) + Ok(captures) } #[cfg(test)] @@ -272,15 +289,16 @@ mod tests { let capture = sample_capture(); let key = compute_key(capture.input_qmd.as_bytes()); - write_cached(&cache_dir, &key, &capture).expect("write succeeds"); + write_cached(&cache_dir, &key, std::slice::from_ref(&capture)).expect("write succeeds"); let read = read_cached(&cache_dir, &key) .expect("read succeeds") .expect("entry exists"); - assert_eq!(read.engine_name, capture.engine_name); - assert_eq!(read.input_qmd, capture.input_qmd); - assert_eq!(read.result, capture.result); + assert_eq!(read.len(), 1); + assert_eq!(read[0].engine_name, capture.engine_name); + assert_eq!(read[0].input_qmd, capture.input_qmd); + assert_eq!(read[0].result, capture.result); } #[test] @@ -310,7 +328,8 @@ mod tests { let cache_dir = tmp.path().join("nested").join("captures"); let capture = sample_capture(); let key = compute_key(capture.input_qmd.as_bytes()); - write_cached(&cache_dir, &key, &capture).expect("write creates parent"); + write_cached(&cache_dir, &key, std::slice::from_ref(&capture)) + .expect("write creates parent"); assert!(cache_dir.exists()); assert!(cache_file_path(&cache_dir, &key).exists()); } @@ -325,15 +344,16 @@ mod tests { let mut first = sample_capture(); first.result = json!({"markdown": "first\n"}); - write_cached(&cache_dir, key, &first).unwrap(); + write_cached(&cache_dir, key, std::slice::from_ref(&first)).unwrap(); let mut second = sample_capture(); second.result = json!({"markdown": "second\n"}); - write_cached(&cache_dir, key, &second).unwrap(); + write_cached(&cache_dir, key, std::slice::from_ref(&second)).unwrap(); let read = read_cached(&cache_dir, key).unwrap().unwrap(); + assert_eq!(read.len(), 1); assert_eq!( - read.result.get("markdown").unwrap().as_str(), + read[0].result.get("markdown").unwrap().as_str(), Some("second\n"), "second write must win" ); @@ -380,8 +400,8 @@ mod tests { let first = record_capture_cached(&cache_dir, &path, &project, runtime.clone(), Some(registry)) .await - .expect("first call records a capture") - .expect("capture present (passthrough engine)"); + .expect("first call records a capture"); + assert_eq!(first.len(), 1, "passthrough engine → one capture"); assert_eq!(calls.load(Ordering::SeqCst), 1, "engine should run once"); // Second call against unchanged content — cache hit, no engine @@ -389,8 +409,8 @@ mod tests { let (registry2, calls2) = counting_registry(); let second = record_capture_cached(&cache_dir, &path, &project, runtime, Some(registry2)) .await - .expect("second call succeeds") - .expect("capture present from cache"); + .expect("second call succeeds"); + assert_eq!(second.len(), 1, "cache hit → one capture"); assert_eq!( calls2.load(Ordering::SeqCst), 0, @@ -398,9 +418,9 @@ mod tests { ); // Sanity: both captures carry the same payload. - assert_eq!(first.engine_name, second.engine_name); - assert_eq!(first.input_qmd, second.input_qmd); - assert_eq!(first.result, second.result); + assert_eq!(first[0].engine_name, second[0].engine_name); + assert_eq!(first[0].input_qmd, second[0].input_qmd); + assert_eq!(first[0].result, second[0].result); } #[tokio::test] @@ -422,8 +442,7 @@ mod tests { let (registry, calls) = counting_registry(); let _ = record_capture_cached(&cache_dir, &path, &project, runtime.clone(), Some(registry)) .await - .expect("first run") - .expect("capture"); + .expect("first run"); assert_eq!(calls.load(Ordering::SeqCst), 1); // Edit on disk. @@ -437,15 +456,14 @@ mod tests { let capture2 = record_capture_cached(&cache_dir, &path, &project2, runtime, Some(registry2)) .await - .expect("second run") - .expect("capture"); + .expect("second run"); assert_eq!( calls2.load(Ordering::SeqCst), 1, "different content must miss the cache and invoke the engine" ); assert!( - capture2.input_qmd.contains("SECOND"), + capture2[0].input_qmd.contains("SECOND"), "second capture should reflect the edit" ); } @@ -474,25 +492,24 @@ mod tests { let (registry, calls) = counting_registry(); let capture = record_capture_cached(&cache_dir, &path, &project, runtime, Some(registry)) .await - .expect("wrapper recovers") - .expect("capture present"); + .expect("wrapper recovers"); assert_eq!( calls.load(Ordering::SeqCst), 1, "corrupt cache should force a real engine run" ); - assert!(capture.input_qmd.contains("FOO")); + assert!(capture[0].input_qmd.contains("FOO")); // And the corrupt entry should now be replaced by a valid one. let re_read = read_cached(&cache_dir, &key) .expect("cache file is now valid") .expect("entry present"); - assert_eq!(re_read.engine_name, "test-passthrough"); + assert_eq!(re_read[0].engine_name, "test-passthrough"); } #[tokio::test] - async fn cached_wrapper_returns_none_for_prose_only_doc() { - // No engine in play → record_capture returns None → + async fn cached_wrapper_returns_empty_for_prose_only_doc() { + // No engine in play → record_capture returns an empty vec → // record_capture_cached must do the same and NOT write a // bogus cache entry. let tmp = TempDir::with_prefix("c7-cached-prose-").unwrap(); @@ -502,14 +519,14 @@ mod tests { let result = record_capture_cached(&cache_dir, &path, &project, runtime, None) .await .expect("wrapper succeeds"); - assert!(result.is_none(), "prose-only doc yields no capture"); + assert!(result.is_empty(), "prose-only doc yields no captures"); // The cache_dir may not even exist; if it does, nothing in it. if cache_dir.exists() { let entries: Vec<_> = std::fs::read_dir(&cache_dir).unwrap().collect(); assert!( entries.is_empty(), - "no cache entry should be written for None captures; got {:?}", + "no cache entry should be written for empty captures; got {:?}", entries .into_iter() .map(|e| e.unwrap().file_name()) diff --git a/crates/quarto-preview/src/capture_driver.rs b/crates/quarto-preview/src/capture_driver.rs index ba8ba301b..a6b1ac12d 100644 --- a/crates/quarto-preview/src/capture_driver.rs +++ b/crates/quarto-preview/src/capture_driver.rs @@ -167,7 +167,7 @@ async fn record_one( // C.7: route through the cache-aware wrapper so identical content // across (re-)opens hits the filesystem cache instead of re-running // the engine. - let capture = record_capture_cached( + let captures = record_capture_cached( cache_dir, abs_path, &project, @@ -177,11 +177,11 @@ async fn record_one( .await .map_err(|e| RecordError::RecordFailed(format!("{}", e)))?; - let Some(capture) = capture else { + if captures.is_empty() { return Ok(false); - }; + } - let capture_doc_id = write_capture_doc(ctx, &capture).await?; + let capture_doc_id = write_capture_doc(ctx, &captures).await?; let capture_ref = CaptureRef { capture_doc_id, @@ -193,10 +193,15 @@ async fn record_one( .set_capture(rel_path, &capture_ref) .map_err(|e| RecordError::SidecarFailed(format!("{}", e)))?; + let engines = captures + .iter() + .map(|c| c.engine_name.as_str()) + .collect::<Vec<_>>() + .join(", "); tracing::info!( rel_path = %rel_path, - engine = %capture.engine_name, - "recorded engine capture", + engines = %engines, + "recorded engine capture(s)", ); Ok(true) } @@ -239,10 +244,24 @@ pub async fn recompute_staleness( return Ok(false); }; - // Load and decode the existing capture so we can compare its - // input_qmd to what the file would produce now. + // Load and decode the existing capture sequence so we can compare + // its input_qmd to what the file would produce now. bd-5yff4: the + // staleness key is the *first* engine's input_qmd — the document's + // canonical pre-engine QMD. Each later engine's input is a + // deterministic function of the prior engine's output, so an + // unchanged first input means the whole recorded sequence is still + // valid. let recorded_input_qmd = match read_capture_from_doc(&ctx, &existing.capture_doc_id).await { - Ok(cap) => cap.input_qmd, + Ok(captures) => match captures.into_iter().next() { + Some(first) => first.input_qmd, + None => { + tracing::warn!( + rel_path = %rel_path, + "recorded capture doc was empty; skipping staleness check", + ); + return Ok(false); + } + }, Err(e) => { tracing::warn!( rel_path = %rel_path, @@ -301,14 +320,15 @@ pub async fn recompute_staleness( Ok(true) } -/// Serialize + gzip + store the EngineCapture as a samod binary doc. -/// Returns the new doc's stringified DocumentId for use in the sidecar -/// `captureDocId` field. +/// Serialize + gzip + store the EngineCapture sequence as a samod binary +/// doc (bd-5yff4: a JSON array, one per engine). Returns the new doc's +/// stringified DocumentId for use in the sidecar `captureDocId` field. async fn write_capture_doc( ctx: &Arc<HubContext>, - capture: &EngineCapture, + captures: &[EngineCapture], ) -> Result<String, RecordError> { - let json = serde_json::to_vec(capture).map_err(|e| RecordError::Serialize(format!("{}", e)))?; + let json = + serde_json::to_vec(captures).map_err(|e| RecordError::Serialize(format!("{}", e)))?; let gzipped = gzip_bytes(&json).map_err(|e| RecordError::Gzip(format!("{}", e)))?; let automerge_doc = create_binary_document(&gzipped, CAPTURE_MIME_TYPE) @@ -353,16 +373,18 @@ pub enum RecordError { SidecarFailed(String), } -/// Resolve the captured EngineCapture out of a binary doc by its -/// document ID. The on-the-wire format (gzipped JSON of -/// [`EngineCapture`] inside a `quarto_hub::resource::create_binary_document` -/// envelope) is shared with Phase C.4's WASM/SPA reader, so keeping -/// the Rust reader public lets test code and any future server-side -/// inspector use the same path. +/// Resolve the captured EngineCapture sequence out of a binary doc by +/// its document ID (bd-5yff4: a JSON array, one per engine). The +/// on-the-wire format (gzipped JSON inside a +/// `quarto_hub::resource::create_binary_document` envelope) is shared +/// with Phase C.4's WASM/SPA reader, so keeping the Rust reader public +/// lets test code and any future server-side inspector use the same path. +/// +/// Tolerates a stale single-object capture doc by wrapping it in a vec. pub async fn read_capture_from_doc( ctx: &Arc<HubContext>, capture_doc_id: &str, -) -> Result<EngineCapture, String> { +) -> Result<Vec<EngineCapture>, String> { use std::io::Read; use std::str::FromStr; @@ -391,7 +413,13 @@ pub async fn read_capture_from_doc( decoder .read_to_end(&mut json_bytes) .map_err(|e| format!("gunzip failed: {}", e))?; - serde_json::from_slice(&json_bytes).map_err(|e| format!("JSON parse failed: {}", e)) + match serde_json::from_slice::<Vec<EngineCapture>>(&json_bytes) { + Ok(captures) => Ok(captures), + Err(array_err) => match serde_json::from_slice::<EngineCapture>(&json_bytes) { + Ok(single) => Ok(vec![single]), + Err(_) => Err(format!("JSON parse failed: {}", array_err)), + }, + } } #[cfg(test)] @@ -617,9 +645,11 @@ mod tests { assert_eq!(count, 1); let entry = ctx.index().get_capture("doc.qmd").unwrap(); - let capture = read_capture_from_doc(&ctx, &entry.capture_doc_id) + let captures = read_capture_from_doc(&ctx, &entry.capture_doc_id) .await .expect("capture doc round-trips"); + assert_eq!(captures.len(), 1); + let capture = &captures[0]; assert_eq!(capture.engine_name, "test-passthrough"); assert!( diff --git a/crates/quarto-preview/src/re_execute.rs b/crates/quarto-preview/src/re_execute.rs index be79907a9..60a1552c6 100644 --- a/crates/quarto-preview/src/re_execute.rs +++ b/crates/quarto-preview/src/re_execute.rs @@ -305,12 +305,14 @@ async fn perform_re_execute( let project = ProjectContext::discover(&abs_path, runtime.as_ref()) .map_err(|e| format!("project discovery failed: {e}"))?; - let capture = record_capture_cached(cache_dir, &abs_path, &project, runtime.clone(), registry) + let captures = record_capture_cached(cache_dir, &abs_path, &project, runtime.clone(), registry) .await - .map_err(|e| format!("engine pipeline failed: {e}"))? - .ok_or_else(|| "engine produced no capture (no code cells?)".to_string())?; + .map_err(|e| format!("engine pipeline failed: {e}"))?; + if captures.is_empty() { + return Err("engine produced no capture (no code cells?)".to_string()); + } - let new_doc_id = write_capture_doc(&ctx, &capture) + let new_doc_id = write_capture_doc(&ctx, &captures) .await .map_err(|e| format!("failed to store capture binary doc: {e}"))?; @@ -333,9 +335,9 @@ async fn perform_re_execute( /// shared `CAPTURE_MIME_TYPE` constant. async fn write_capture_doc( ctx: &Arc<HubContext>, - capture: &EngineCapture, + captures: &[EngineCapture], ) -> Result<String, String> { - let json = serde_json::to_vec(capture).map_err(|e| format!("serialize: {e}"))?; + let json = serde_json::to_vec(captures).map_err(|e| format!("serialize: {e}"))?; let mut enc = flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::default()); enc.write_all(&json) .map_err(|e| format!("gzip write: {e}"))?; diff --git a/crates/quarto-preview/tests/integration/cache_hit.rs b/crates/quarto-preview/tests/integration/cache_hit.rs index 4f60c78b1..8e81916d1 100644 --- a/crates/quarto-preview/tests/integration/cache_hit.rs +++ b/crates/quarto-preview/tests/integration/cache_hit.rs @@ -129,9 +129,11 @@ async fn second_eager_run_with_same_cache_dir_hits_cache_skips_engine() { .get_capture("doc.qmd") .expect("sidecar repopulated"); assert!(!sidecar.capture_doc_id.is_empty()); - let capture = capture_driver::read_capture_from_doc(&ctx, &sidecar.capture_doc_id) + let captures = capture_driver::read_capture_from_doc(&ctx, &sidecar.capture_doc_id) .await .expect("binary doc round-trips"); + assert_eq!(captures.len(), 1); + let capture = &captures[0]; assert_eq!(capture.engine_name, "test-passthrough"); assert!(capture.input_qmd.contains("IDENT")); let markdown = capture diff --git a/crates/quarto-preview/tests/integration/eager_capture.rs b/crates/quarto-preview/tests/integration/eager_capture.rs index e6c827b37..7d95e5def 100644 --- a/crates/quarto-preview/tests/integration/eager_capture.rs +++ b/crates/quarto-preview/tests/integration/eager_capture.rs @@ -123,9 +123,11 @@ async fn eager_capture_populates_index_sidecar() { // EngineCapture with the expected fields. This exercises the // same path Phase C.4 will use in the browser: read the binary // doc, ungzip, parse JSON. - let capture = capture_driver::read_capture_from_doc(&ctx, &entry.capture_doc_id) + let captures = capture_driver::read_capture_from_doc(&ctx, &entry.capture_doc_id) .await .expect("capture doc round-trips"); + assert_eq!(captures.len(), 1); + let capture = &captures[0]; assert_eq!(capture.engine_name, "test-passthrough"); assert!( diff --git a/crates/quarto-trace/src/lib.rs b/crates/quarto-trace/src/lib.rs index cea7edda4..3c3b298ee 100644 --- a/crates/quarto-trace/src/lib.rs +++ b/crates/quarto-trace/src/lib.rs @@ -89,6 +89,7 @@ pub const SCHEMA_VERSION: u32 = 2; /// and ad-hoc serialization, but the on-disk artifact will not have /// the v2 size benefits. #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(from = "TraceDocumentDe")] pub struct TraceDocument { pub schema_version: u32, pub render: RenderInfo, @@ -99,16 +100,64 @@ pub struct TraceDocument { #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] pub asts: BTreeMap<String, serde_json::Value>, pub pipeline: Vec<TraceEntry>, - /// Engine execution capture for replay (bd-45yw). + /// Ordered engine execution captures for replay (bd-45yw, extended to + /// a sequence by bd-5yff4). /// - /// When `trace: true` is set, the pipeline records the - /// `ExecutionEngine`'s output here so the trace can later drive - /// the in-Rust replay engine for deterministic regression tests - /// without R/Python/Jupyter installs. Absent for traces produced - /// before bd-45yw landed and for renders where the markdown + /// When `trace: true` is set, the pipeline records each + /// `ExecutionEngine`'s output here — **one capture per engine that + /// ran, in execution order** — so the trace can later drive the + /// in-Rust replay engine(s) for deterministic regression tests + /// without R/Python/Jupyter installs. Empty for traces produced + /// before bd-45yw landed and for renders where only the markdown /// engine ran (no execution to record). - #[serde(default, skip_serializing_if = "Option::is_none")] - pub engine_capture: Option<EngineCapture>, + /// + /// On-disk back-compat: a legacy single `engine_capture` object + /// (schema written before bd-5yff4) is folded into a one-element + /// vector on read (see `TraceDocumentDe`). Writers always emit the + /// `engine_captures` array. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub engine_captures: Vec<EngineCapture>, +} + +/// Deserialization mirror for [`TraceDocument`]. +/// +/// Exists solely to accept the **legacy** single `engine_capture` field +/// (written before bd-5yff4) and fold it into the `engine_captures` +/// vector, so old on-disk traces keep loading. `TraceDocument` itself +/// deserializes through this via `#[serde(from = "TraceDocumentDe")]`; +/// serialization is unaffected (the writer emits only `engine_captures`). +#[derive(Deserialize)] +struct TraceDocumentDe { + schema_version: u32, + render: RenderInfo, + #[serde(default)] + asts: BTreeMap<String, serde_json::Value>, + pipeline: Vec<TraceEntry>, + #[serde(default)] + engine_captures: Vec<EngineCapture>, + /// Legacy single-capture field (pre-bd-5yff4). Read-only. + #[serde(default)] + engine_capture: Option<EngineCapture>, +} + +impl From<TraceDocumentDe> for TraceDocument { + fn from(de: TraceDocumentDe) -> Self { + let mut engine_captures = de.engine_captures; + // Fold the legacy single capture in when the new field is absent. + // (If both are somehow present, the new field wins.) + if engine_captures.is_empty() { + if let Some(capture) = de.engine_capture { + engine_captures.push(capture); + } + } + TraceDocument { + schema_version: de.schema_version, + render: de.render, + asts: de.asts, + pipeline: de.pipeline, + engine_captures, + } + } } impl TraceDocument { @@ -120,7 +169,7 @@ impl TraceDocument { render, asts: BTreeMap::new(), pipeline: Vec::new(), - engine_capture: None, + engine_captures: Vec::new(), } } } diff --git a/crates/quarto-trace/tests/roundtrip.rs b/crates/quarto-trace/tests/roundtrip.rs index faf635a9b..4f458dede 100644 --- a/crates/quarto-trace/tests/roundtrip.rs +++ b/crates/quarto-trace/tests/roundtrip.rs @@ -353,19 +353,17 @@ fn test_engine_capture_roundtrip_through_disk() { }); let mut doc = TraceDocument::new(RenderInfo::default()); - doc.engine_capture = Some(EngineCapture { + doc.engine_captures = vec![EngineCapture { engine_name: "jupyter".into(), input_qmd: "---\nengine: jupyter\n---\n\n# Hello\n".into(), result: result_json.clone(), - }); + }]; write_trace(&doc, &path).unwrap(); let read_back = read_trace(&path).unwrap(); - let capture = read_back - .engine_capture - .as_ref() - .expect("engine_capture should round-trip"); + assert_eq!(read_back.engine_captures.len(), 1); + let capture = &read_back.engine_captures[0]; assert_eq!(capture.engine_name, "jupyter"); assert_eq!(capture.input_qmd, "---\nengine: jupyter\n---\n\n# Hello\n"); assert_eq!(capture.result, result_json); @@ -373,15 +371,36 @@ fn test_engine_capture_roundtrip_through_disk() { #[test] fn test_engine_capture_absent_by_default() { - // Existing traces without an engine_capture field should still - // deserialize cleanly. + // Existing traces without engine captures should still deserialize + // cleanly (empty vector). let json_text = r#"{ "schema_version": 1, "render": {}, "pipeline": [] }"#; let doc: TraceDocument = serde_json::from_str(json_text).unwrap(); - assert!(doc.engine_capture.is_none()); + assert!(doc.engine_captures.is_empty()); +} + +#[test] +fn test_legacy_single_engine_capture_folds_into_vec() { + // A pre-bd-5yff4 trace with a single `engine_capture` object must + // fold into the one-element `engine_captures` vector on read. + let json_text = r#"{ + "schema_version": 2, + "render": {}, + "pipeline": [], + "engine_capture": { + "engine_name": "knitr", + "input_qmd": "---\nengine: knitr\n---\n", + "result": {"markdown": "out\n", "supporting_files": [], "filters": [], + "includes": {"header_includes": [], "include_before": [], "include_after": []}, + "needs_postprocess": false} + } + }"#; + let doc: TraceDocument = serde_json::from_str(json_text).unwrap(); + assert_eq!(doc.engine_captures.len(), 1); + assert_eq!(doc.engine_captures[0].engine_name, "knitr"); } #[test] diff --git a/crates/quarto/src/commands/render.rs b/crates/quarto/src/commands/render.rs index 165a582f6..a29de7d9f 100644 --- a/crates/quarto/src/commands/render.rs +++ b/crates/quarto/src/commands/render.rs @@ -485,17 +485,20 @@ pub fn render_summary_line( /// Returns a hard error (no fallback) if: /// - The trace file cannot be read (missing path, permission denied, /// malformed JSON). -/// - The trace lacks an `engine_capture` block — replaying a trace -/// that was recorded without engine capture would silently behave -/// like the markdown engine, which is exactly the kind of "silent -/// degradation" the hard-fail policy guards against. -fn load_replay_capture(cli_replay: Option<&str>) -> Result<Option<quarto_trace::EngineCapture>> { +/// - The trace lacks any `engine_captures` — replaying a trace that was +/// recorded without engine capture would silently behave like the +/// markdown engine, which is exactly the kind of "silent degradation" +/// the hard-fail policy guards against. +/// +/// Returns the captures in execution order (one per engine that ran); +/// an empty vec means replay is inactive (no `--replay` / `QUARTO_REPLAY`). +fn load_replay_captures(cli_replay: Option<&str>) -> Result<Vec<quarto_trace::EngineCapture>> { let path_str = match cli_replay { Some(p) => Some(p.to_string()), None => std::env::var("QUARTO_REPLAY").ok(), }; let Some(path_str) = path_str else { - return Ok(None); + return Ok(Vec::new()); }; let path = PathBuf::from(&path_str); @@ -507,15 +510,15 @@ fn load_replay_capture(cli_replay: Option<&str>) -> Result<Option<quarto_trace:: ) })?; - let capture = trace.engine_capture.ok_or_else(|| { - anyhow::anyhow!( - "Trace file {} has no `engine_capture` block. \ + if trace.engine_captures.is_empty() { + return Err(anyhow::anyhow!( + "Trace file {} has no `engine_captures`. \ Re-record the trace from a real engine run with `trace: true` set.", path.display() - ) - })?; + )); + } - Ok(Some(capture)) + Ok(trace.engine_captures) } /// Execute the render command. @@ -558,14 +561,14 @@ pub fn execute(args: RenderArgs) -> Result<()> { // QUARTO_REPLAY=<path> is set. Hard-fail on read errors and on // traces missing engine_capture so investigators don't waste // time on a silently-degraded replay. - let replay_capture = load_replay_capture(args.replay.as_deref())?; + let replay_captures = load_replay_captures(args.replay.as_deref())?; // Set up render options let options = RenderToFileOptions { output_path: args.output.as_ref().map(PathBuf::from), output_dir: args.output_dir.as_ref().map(PathBuf::from), quiet: args.quiet, - replay_capture, + replay_captures, engine_registry_override: None, // Phase 3c: CLI override-only for v1 (YAML reading is deferred // until project YAML schema work; the resolver matrix is still diff --git a/crates/wasm-quarto-hub-client/src/lib.rs b/crates/wasm-quarto-hub-client/src/lib.rs index 7caed5fce..1c5b2a1d6 100644 --- a/crates/wasm-quarto-hub-client/src/lib.rs +++ b/crates/wasm-quarto-hub-client/src/lib.rs @@ -967,7 +967,16 @@ pub async fn render_qmd(path: &str, user_grammars: Option<JsUserGrammars>) -> St Err(e) => return error_response(format!("Failed to discover project context: {}", e)), }; - render_single_doc_to_response(path, &content, &project, user_grammars, false, None, None).await + render_single_doc_to_response( + path, + &content, + &project, + user_grammars, + false, + Vec::new(), + None, + ) + .await } /// Render QMD content directly (without reading from VFS). @@ -996,7 +1005,7 @@ pub async fn render_qmd_content( &project, user_grammars, false, - None, + Vec::new(), None, ) .await @@ -1104,7 +1113,7 @@ pub async fn render_page_in_project_with_attribution( &project, user_grammars, false, - None, + Vec::new(), attribution_json, ) .await; @@ -1128,7 +1137,7 @@ pub async fn render_page_in_project_with_attribution( project, user_grammars, false, - None, + Vec::new(), attribution_json, ) .await @@ -1176,8 +1185,8 @@ pub async fn render_page_for_preview( // `CaptureSpliceStage` rather than constructing a // `ReplayEngine`-bearing registry; `ReplayEngine` remains the // bd-45yw regression-testing tool and is not on this path. - let capture = match parse_capture_from(capture_gz_json) { - Ok(cap) => cap, + let captures = match parse_capture_from(capture_gz_json) { + Ok(caps) => caps, Err(e) => { return error_response(format!("Failed to parse capture: {}", e)); } @@ -1197,7 +1206,7 @@ pub async fn render_page_for_preview( &project, user_grammars, true, - capture, + captures, None, ) .await; @@ -1215,32 +1224,35 @@ pub async fn render_page_for_preview( project, user_grammars, true, - capture, + captures, None, ) .await } -/// Ungzip + JSON-deserialize a capture payload into a typed -/// [`EngineCapture`](quarto_trace::EngineCapture). `None` input → -/// `None` output; an empty `Vec` is treated the same as `None` for -/// symmetry with WASM/JS callers that may pass an empty `Uint8Array` -/// to mean "no capture." +/// Ungzip + JSON-deserialize a capture payload into the ordered +/// sequence of [`EngineCapture`](quarto_trace::EngineCapture)s (one per +/// engine that ran server-side; bd-5yff4). `None`/empty input → empty +/// vec, for symmetry with WASM/JS callers that may pass an empty +/// `Uint8Array` to mean "no capture." +/// +/// The payload is a gzipped JSON **array** of captures. For robustness +/// against a stale pre-bd-5yff4 doc (a single capture object), a failed +/// array parse falls back to a single-object parse wrapped in a vec. /// -/// bd-lucp: this used to construct an -/// `EngineRegistry::with_replay(capture)`, but the preview path no -/// longer routes through `ReplayEngine` — see +/// bd-lucp: the preview path consumes these via `CaptureSpliceStage`, +/// not `ReplayEngine` — see /// `claude-notes/plans/2026-05-18-q2-preview-project-replay-engine.md`. fn parse_capture_from( capture_gz_json: Option<Vec<u8>>, -) -> Result<Option<quarto_trace::EngineCapture>, String> { +) -> Result<Vec<quarto_trace::EngineCapture>, String> { use std::io::Read; let Some(bytes) = capture_gz_json else { - return Ok(None); + return Ok(Vec::new()); }; if bytes.is_empty() { - return Ok(None); + return Ok(Vec::new()); } let mut decoder = flate2::read::GzDecoder::new(&bytes[..]); @@ -1248,9 +1260,17 @@ fn parse_capture_from( decoder .read_to_end(&mut json) .map_err(|e| format!("ungzip failed: {}", e))?; - let capture: quarto_trace::EngineCapture = - serde_json::from_slice(&json).map_err(|e| format!("JSON parse failed: {}", e))?; - Ok(Some(capture)) + + match serde_json::from_slice::<Vec<quarto_trace::EngineCapture>>(&json) { + Ok(captures) => Ok(captures), + Err(array_err) => { + // Lenient fallback: a stale single-object capture doc. + match serde_json::from_slice::<quarto_trace::EngineCapture>(&json) { + Ok(single) => Ok(vec![single]), + Err(_) => Err(format!("JSON parse failed: {}", array_err)), + } + } + } } /// Single-doc render path — used by `render_qmd` directly and by @@ -1275,12 +1295,13 @@ async fn render_single_doc_to_response( project: &ProjectContext, user_grammars: Option<JsUserGrammars>, prefer_preview_format: bool, - // bd-lucp: recorded engine capture for the preview-time AST-splice - // path. Forwarded to `render_qmd_to_preview_ast` (which threads it - // into `CaptureSpliceStage`) on the preview branch. The HTML - // branch ignores it (HTML renders don't consume captures in the - // WASM today — `q2 render` natively runs engines instead). - capture: Option<quarto_trace::EngineCapture>, + // bd-lucp / bd-5yff4: recorded engine capture sequence for the + // preview-time AST-splice path (one per engine, in order). Forwarded + // to `render_qmd_to_preview_ast` (which folds them through + // `CaptureSpliceStage`) on the preview branch. The HTML branch + // ignores it (HTML renders don't consume captures in the WASM today — + // `q2 render` natively runs engines instead). + captures: Vec<quarto_trace::EngineCapture>, attribution_json: Option<String>, ) -> String { let doc = DocumentInfo::from_path(path); @@ -1339,7 +1360,7 @@ async fn render_single_doc_to_response( &mut ctx, runtime_arc, None, - capture, + captures, ) .await { @@ -1440,12 +1461,13 @@ async fn render_project_active_page_to_response( mut project: ProjectContext, user_grammars: Option<JsUserGrammars>, prefer_preview_format: bool, - // bd-lucp: recorded engine capture attached to the q2-preview - // pass-2 renderer. `RenderToPreviewAstRenderer::with_capture` - // threads it into every per-doc `render_qmd_to_preview_ast` call; - // [`CaptureSpliceStage`] inside the q2-preview pipeline applies the - // splice. The non-preview branch ignores it. - capture: Option<quarto_trace::EngineCapture>, + // bd-lucp / bd-5yff4: recorded engine capture sequence attached to + // the q2-preview pass-2 renderer (one per engine, in order). + // `RenderToPreviewAstRenderer::with_captures` threads them into every + // per-doc `render_qmd_to_preview_ast` call; [`CaptureSpliceStage`] + // inside the q2-preview pipeline folds the splices. The non-preview + // branch ignores it. + captures: Vec<quarto_trace::EngineCapture>, attribution_json: Option<String>, ) -> String { use quarto_core::project::orchestrator::{ProjectPipeline, RenderMode, project_type_for}; @@ -1503,12 +1525,12 @@ async fn render_project_active_page_to_response( let summary = match kind { Some("preview") => { let mut renderer = RenderToPreviewAstRenderer::new("/.quarto/project-artifacts"); - // bd-lucp: when a capture is present, attach it to the - // renderer so `CaptureSpliceStage` (inserted by - // `build_q2_preview_pipeline_stages`) can splice recorded + // bd-lucp / bd-5yff4: when captures are present, attach the + // sequence to the renderer so `CaptureSpliceStage` (inserted + // by `build_q2_preview_pipeline_stages`) can fold the recorded // engine output blocks into each per-doc AST. - if let Some(cap) = capture { - renderer = renderer.with_capture(cap); + if !captures.is_empty() { + renderer = renderer.with_captures(captures); } if let Some(json) = attribution_json { renderer = renderer.with_attribution(json);