From 189560eaaa19c69738b2057ccf90e35634948b45 Mon Sep 17 00:00:00 2001 From: Carlos Scheidegger Date: Thu, 2 Jul 2026 08:20:28 -0500 Subject: [PATCH] fix(jupyter): emit Quarto-canonical ::: {.cell} cells so captures splice into previews (bd-gthycd33) Jupyter's post-engine markdown lacked the Div.cell wrapper that CaptureSpliceStage and the Bootstrap cell CSS key on, so executed output never spliced into hub-client/q2-preview (knitr worked). Emit the knitr/Q1-parity shape, retire the divergent test-only AST path (JupyterTransform/outputs_to_blocks), and pin the contract with a splice regression pair + a 5-case cross-engine output-parity suite. Co-Authored-By: Claude Fable 5 --- ...-07-01-bd-gthycd33-jupyter-cell-wrapper.md | 436 ++++++++++++++++ crates/quarto-core/src/engine/jupyter/mod.rs | 4 - .../quarto-core/src/engine/jupyter/output.rs | 356 ------------- .../src/engine/jupyter/text_execute.rs | 467 +++++++++++++---- .../src/engine/jupyter/transform.rs | 492 ------------------ .../integration/capture_splice_engines.rs | 163 ++++++ .../tests/integration/engine_output_parity.rs | 237 +++++++++ .../tests/integration/jupyter_integration.rs | 336 +----------- crates/quarto-core/tests/integration/main.rs | 2 + 9 files changed, 1225 insertions(+), 1268 deletions(-) create mode 100644 claude-notes/plans/2026-07-01-bd-gthycd33-jupyter-cell-wrapper.md delete mode 100644 crates/quarto-core/src/engine/jupyter/output.rs delete mode 100644 crates/quarto-core/src/engine/jupyter/transform.rs create mode 100644 crates/quarto-core/tests/integration/capture_splice_engines.rs create mode 100644 crates/quarto-core/tests/integration/engine_output_parity.rs diff --git a/claude-notes/plans/2026-07-01-bd-gthycd33-jupyter-cell-wrapper.md b/claude-notes/plans/2026-07-01-bd-gthycd33-jupyter-cell-wrapper.md new file mode 100644 index 000000000..9d532064e --- /dev/null +++ b/claude-notes/plans/2026-07-01-bd-gthycd33-jupyter-cell-wrapper.md @@ -0,0 +1,436 @@ +# bd-gthycd33: Jupyter engine output not spliced into preview (knitr works) + +**Strand:** bd-gthycd33 (bug, P2, discovered-from bd-sfet3264) +**Branch:** `braid/bd-gthycd33-jupyter-engine-output-not` (off `main`) +**Status:** implemented + verified end-to-end (2026-07-02): Phases 1–4 +complete (unit + engine-gated integration + parity suites green; full +workspace suite 10181 passed; full `cargo xtask verify` passed; CLI +`q2 render` and browser `q2 preview` e2e inspected). Staged, awaiting +commit approval; the feature-branch hub-harness cross-check happens +post-merge. + +## Overview + +Clicking **Run** on an `engine: jupyter` document in hub-client produces a +capture (the `CaptureRef` sidecar arrives, the status bar reads "Showing +executed output"), but the computed output is not spliced into the preview — +the `{python}` cell still renders as source. The identical flow with +`engine: knitr` splices correctly. + +The bug is **independent of the hub execution-provider feature** and exists on +`main`: it affects any consumer of the capture-splice path, including `q2 +preview`'s own server-side capture recording (bd-lucp). Reproduced +mechanically on `main` in this worktree (2026-07-01, see below). + +## Root cause (confirmed by reproduction) + +`derive_cell_outputs` (`crates/quarto-core/src/engine/capture_splice.rs`) +walks the capture pair `(A1 = parse(input_qmd), B1 = parse(result.markdown))` +and requires each engine cell in A1 to map to a **`Div` with class `cell`** +(`is_cell_wrapper`) in B1. + +- **knitr** satisfies this: its vendored Quarto-1 knitr hooks + (`crates/quarto-core/src/engine/knitr/resources/rmd/hooks.R:403`, + `classes <- c("cell", ...)`) wrap every executed chunk in a `::: {.cell}` + pandoc div containing the `{.r .cell-code}` echo fence and + `::: {.cell-output .cell-output-stdout}` output divs. +- **jupyter** does not: `execute_blocks_inner` + `format_outputs` + (`crates/quarto-core/src/engine/jupyter/text_execute.rs`) emit a **bare** + echoed source fence (` ```python `) followed by bare output fences + (` ```{.cell-output} `, ` ```{.cell-output-stdout} `, …) — **no `::: {.cell}` + wrapper at all**. + +So the splice walk, on jupyter's B1, finds a plain `CodeBlock` where it +requires a `Div.cell`, records no entry for the cell, then diverges on the +next lockstep prose comparison and stops. The cell-output map comes out +**empty**, the splice is a fail-soft no-op, and the cell renders as raw +source — exactly the browser symptom. + +### Mechanical reproduction (2026-07-01, this worktree, off `main`) + +Test: `crates/quarto-core/tests/integration/repro_gthycd33.rs` (in this +branch's working tree). It runs `record_capture` with the **real** engine +(mirroring `quarto-hub-provider`'s `pollster::block_on` calling convention — +the jupyter engine builds its own current-thread tokio runtime, so the test +must not be `#[tokio::test]`), then replays exactly what `CaptureSpliceStage` +does: parse `input_qmd` / `result.markdown` with `pampa::readers::qmd::read` +and call `derive_cell_outputs`. + +``` +cargo nextest run -p quarto-core -E 'binary(integration) & test(repro_gthycd33)' --no-capture +``` + +Observed (both engines available on this machine): + +- `jupyter_capture_splice_map_is_nonempty` — **FAIL**: map has 0 entries. + Captured `result.markdown` for `2 + 3`: + + ````markdown + Some prose. + + ```python + 2 + 3 + ``` + + ```{.cell-output} + 5 + ``` + ```` + +- `knitr_capture_splice_map_is_nonempty` — **PASS**: map has 1 entry. + Captured `result.markdown` for `1 + 1`: + + ````markdown + Some prose. + + ::: {.cell} + + ```{.r .cell-code} + 1 + 1 + ``` + + ::: {.cell-output .cell-output-stdout} + + ``` + [1] 2 + ``` + + ::: + ::: + ```` + +The same pair of tests was first run on `feature/hub-execution-provider` +(warm checkout) with identical results, matching the browser-observed +behavior recorded in +`claude-notes/plans/2026-07-01-merge-preview-status-line.md` ("Orthogonal bug +found"). + +### Secondary defect, same root + +`resources/scss/bootstrap/_bootstrap-rules.scss:1470` styles outputs with the +selector `.cell .cell-output-stdout pre code`. Jupyter's wrapper-less emission +never matches the `.cell` ancestor, so jupyter outputs also miss intended +styling in **every** render path, not just the splice. Fixing the emission +shape fixes this for free; loosening the matcher would not. + +## Proposed fix + +**Fix the jupyter engine's emission to produce the Quarto-canonical cell +shape** (what knitr's hooks and Quarto 1's jupyter engine both emit), rather +than teaching `derive_cell_outputs` to tolerate wrapper-less output. + +Rationale: + +- The `::: {.cell}` wrapper is the established Quarto contract for executed + cells; knitr (via the vendored Q1 hooks) and Quarto 1's jupyter engine both + honor it. The Rust jupyter engine is the outlier. +- The splice's `(content-hash, occurrence-index)` design is sound and shared; + special-casing wrapper-less engines there would add a second matching mode + that has to guess where a cell's output run ends (ambiguous when outputs are + markdown/prose). That is the hacky path. +- The wrapper also fixes the CSS-selector mismatch above and gives jupyter + cells the same downstream affordances as knitr cells (cell-level transforms, + code-fold, copy button targeting, etc.). + +### Target emission shape + +For each executed cell, `execute_blocks_inner` emits: + +````markdown +::: {.cell} + +```{.python .cell-code} +2 + 3 +``` + +::: {.cell-output .cell-output-display} + +``` +5 +``` + +::: +::: +```` + +Concretely, in `text_execute.rs`: + +1. Wrap each cell's emission (echoed source + outputs) in `::: {.cell}` … + `:::`. +2. Echoed source fence becomes ` ```{.python .cell-code} ` (attribute-syntax + class list; the code-highlight stage resolves the language from the first + class, same as knitr's `{.r .cell-code}` — highlighting keeps working). +3. Outputs move from bare classed fences to Q1/knitr-parity divs wrapping + plain fences (classes per decision 2): + - stream → `::: {.cell-output .cell-output-stdout}` (or `-stderr`) around + a plain ` ``` ` fence; + - execute_result / display_data `text/plain` → + `::: {.cell-output .cell-output-display}` around a plain fence; + - `text/html` → `::: {.cell-output .cell-output-display}` + ` ```{=html} `, + nested inside the `.cell` wrapper; + - error → `::: {.cell-output .cell-output-error}` around a plain fence. + +Cells that produce no output still get the `.cell` wrapper (source-only cell, +like knitr's `echo`-only chunks) — the splice then correctly replaces the +live cell with the wrapper, and "no output" renders as just the echoed code. + +### Scope notes + +- `JupyterTransform` / `output.rs::outputs_to_blocks` (the AST-path variant) + is **only used by tests** (`jupyter_integration.rs`); production goes + through `ExecutionEngine::execute` → `text_execute.rs`. v1 of this fix + touches only the text path. Whether to align or retire the AST path is + flagged as an open question (decision 3), not silently changed. +- No `.snap` snapshot in the workspace contains `cell-output`, and no test + fixture `.qmd` declares `engine: jupyter` — jupyter execution is only + exercised by kernel-gated tests. Expected fallout is limited to the shape + assertions in `text_execute.rs` unit tests and `jupyter_integration.rs`. +- `capture_splice.rs` itself needs **no change**. + +## Decisions (locked 2026-07-02 with Carlos) + +1. **Full knitr-parity shape** — not just the minimal `::: {.cell}` wrapper; + the `.cell-code` echo fence and `.cell-output` divs too. Ratified + rationale: **cross-engine congruence of post-engine markdown is a + correctness requirement in q2**, not cosmetics — any structural divergence + between engines' text output is precisely the class of bug bd-gthycd33 is + (the splice, CSS selectors, and future cell-level transforms all key on + one shared shape). Corollary: where quarto-cli's jupyter emission differs + *structurally* from knitr's, treat that as a quarto-cli bug and do **not** + replicate it — q2 emits one canonical structure for both engines. + Discrepancies found during implementation are resolved toward the common + structure and **flagged in this plan for Carlos's review when the right + resolution is in doubt** (one such flag already recorded below: knitr + autoprint→`-stdout` vs jupyter execute_result→`-display`). +2. **Adopt Q1's output-class scheme** (verified in + `external-sources/quarto-cli/src/core/jupyter/jupyter.ts`, + `outputTypeCssClass` + the output-div emitter around line 1578): every + output div carries `.cell-output` plus a specific class — + `.cell-output-stdout` / `.cell-output-stderr` for streams, + `.cell-output-display` for `execute_result` and `display_data`, + `.cell-output-error` for errors; the echoed source fence carries + `.cell-code`; the wrapper is `::: {.cell}`. Matches knitr's hooks. Keep + using `external-sources/quarto-cli` as the read-only shape reference + throughout this task. +3. **`execution_count` attributes: deferred.** Nothing in q2 consumes them + yet; add when a consumer appears (file a strand at close-out). +4. **Structural anti-regression instead of an API refactor.** The engine API + stays text-in/text-out (future user-extensible engines will be too), so + AST-emission uniformity cannot be enforced at the API level. Enforce it + with tests instead: a **cross-engine output-parity suite** that runs + equivalent minimal inputs through knitr and jupyter and asserts the parsed + post-engine ASTs have the same structure (design below; runtime-gated on + both engines being installed). Additionally, the test-only AST path + (`JupyterTransform` / `outputs_to_blocks`) is a latent divergence source: + retire it, or make it delegate to the single text-path emitter, whichever + is the modest diff; if both turn out large, record findings and file a + follow-up strand rather than forcing it into this fix. Full engine-API + refactoring is explicitly out of scope. + +## Cross-engine parity suite (decision 4) — design + +New integration module (e.g. +`crates/quarto-core/tests/integration/engine_output_parity.rs`), runtime- +skipped unless **both** engines are available (same gating idiom as +`jupyter_integration.rs`), since it needs R+knitr and jupyter+ipykernel on +the machine. For each pair of equivalent minimal documents: + +| case | knitr input | jupyter input | +|---|---|---| +| stream output | `cat("hi\n")` | `print("hi")` | +| expression result | `1 + 1` | `2 + 3` | +| error | `stop("boom")` | `raise Exception("boom")` | +| source-only (no output) | assignment `x <- 1` | assignment `x = 1` | + +run `record_capture` with the real engine, parse `result.markdown`, and +assert the two block trees have equal **shape signatures**. Signature = +recursive tree of `(node kind, semantic classes)` where *semantic classes* +is the intersection with `{cell, cell-code, cell-output}` — i.e. we pin the +structure the splice/CSS/transforms rely on, while allowing the language +class (`r` vs `python`) and the output-subtype class to differ. Signature +computation is a small pure helper in the test module; content bytes are +ignored. + +**Flagged for review (decision 1 corollary):** the same logical "expression +evaluates to a value" case produces `.cell-output-stdout` under knitr (R +autoprints to stdout) but `.cell-output-display` under jupyter +(execute_result). This is inherited from Q1 semantics; the block *structure* +is identical (a `.cell-output` div wrapping a plain fence) and CSS treats +both, so the parity signature above deliberately tolerates it. If we instead +want subtype-class parity too, say so and the suite tightens to full class +equality with a normalization table. + +## Discrepancy log (decision 1 corollary) + +Divergences between the engines (or inherited from quarto-cli) found while +working this strand. Each is either resolved toward the common structure +here, tolerated deliberately, or filed as a follow-up. + +1. **knitr autoprint→`-stdout` vs jupyter execute_result→`-display`** + (flagged pre-execution, see the parity-suite design above). Same logical + "expression evaluates to a value"; identical block structure; only the + output-subtype class differs. Inherited from Q1 semantics; CSS treats + both. **Tolerated** — the parity signature compares semantic classes + only. Carlos reviewed the recommendation 2026-07-02 (decision round 1). +2. **Error policy: jupyter embeds cell errors and continues; knitr fails + the render** (found 2026-07-02 while writing `parity_error_output`). + With a plain failing cell, knitr's pipeline errors out with no capture — + Q1's default `execute.error: false` policy — while q2's jupyter + unconditionally embeds `.cell-output-error` and reports success. This is + a *behavioral* divergence, out of scope for this shape fix (it needs + `#|` directive awareness in the jupyter text path). **Filed as + bd-ohvl879u** (discovered-from bd-gthycd33). The parity suite pins the + error-output *shape* under `#| error: true`, which both engines execute + today. + +## Work items + +### Phase 1 — tests first (TDD) + +- [x] Keep the two repro tests as the engine-gated regression pair — + renamed to `capture_splice_engines.rs` + (`jupyter_capture_splices_into_preview_ast` / + `knitr_capture_splices_into_preview_ast`), extended to assert the + spliced AST replaces the cell with a `Div.cell` via `splice_cells`, + and gated with a runtime skip (not `#[ignore]` — CI runs without + `--run-ignored`, so an ignored fence would never fire anywhere; with + runtime gating it runs wherever engines are installed and skips + elsewhere). ✅ jupyter FAILS / knitr PASSES (2026-07-02, this + worktree off main). +- [x] Add **pure unit tests** (no kernel needed) in `text_execute.rs`: + 10 exact-string shape tests (echo fence with `.cell-code`, output + divs per class, `render_cell` wrapper with/without outputs, fence + sizing per Q1's `ticksForCode` — max(3, longest leading backtick + run + 1); the current fixed ``` is a latent corruption bug for + outputs containing backticks). The three old shape tests + (`test_echoed_source_fence_strips_braces`, + `test_format_outputs_stream`, `test_format_outputs_error`) replaced + by exact-equality versions. ✅ red confirmed 2026-07-02: E0425 for + the not-yet-existing `render_cell` (format_outputs assert-reds are + masked by the compile error until it exists). +- [x] Add the **cross-engine parity suite** — + `crates/quarto-core/tests/integration/engine_output_parity.rs`, 4 + input pairs (stream, expression value, error, source-only), recursive + shape-parity walker (block kinds + semantic classes ∩ {cell, + cell-code, cell-output}), runtime-skip unless both engines available. + ✅ all 4 FAIL with the expected mismatch (knitr `[Paragraph, Div, + Paragraph]` vs jupyter `[Paragraph, CodeBlock, CodeBlock, Paragraph]`), + 2026-07-02. The error pair uses `#| error: true` in both cells — see + discrepancy log entry 2. +- [x] Run: new/updated unit tests fail ✅ (red = E0425 compile error for + the not-yet-existing `render_cell`); repro jupyter test fails ✅ + (map 0 entries); parity suite fails ✅ (all 4 pairs, expected + mismatch). knitr splice test passes ✅ (control). 2026-07-02. + +### Phase 2 — implementation + +- [x] Rework the emission in `text_execute.rs`: new `render_cell` wraps + echoed source + outputs in `::: {.cell}`; `echoed_source_fence` emits + `{. .cell-code}`; `format_outputs` emits + `::: {.cell-output .cell-output-{stdout,stderr,display,error}}` divs + around plain fences (`fenced_output_div`); all fences sized via + `ticks_for_code` (Q1 rule). Also: a guard in `execute_blocks_inner` + ensures the `::: {.cell}` opener starts its own block when the source + lacked a blank line before the cell (fenced divs can't interrupt a + paragraph), and `text/plain`/`text/html` mime values now go through + `extract_text_content` (handles nbformat's array-of-lines form the + old `as_str()` silently dropped — unit-tested). +- [x] **AST-path disposition (decision 4): retired.** `JupyterTransform` + + `outputs_to_blocks` had **no production consumer** (grep: only + `jupyter_integration.rs` tests) — production execution is + `ExecutionEngine::execute` → `text_execute.rs`. Deleted + `jupyter/transform.rs` + `jupyter/output.rs`; moved the still-used + helpers (`strip_ansi_codes`, `extract_text_content`) + their tests + into `text_execute.rs`; pruned the transform/inline-expr tests from + `jupyter_integration.rs` (their durable coverage lives on: kernel + persistence → `parity_dependent_cells` + `test_full_pipeline_multiple_cells`; + inline `{python} expr` was a prototype only the retired path had → + filed **bd-u996g8g2**). One emission path remains, by construction. +- [x] Added a 5th parity case, `parity_dependent_cells` (two cells, second + reads state from the first) — pins kernel-state persistence through + the production path *and* multi-cell shape, for both engines. +- [ ] While consulting `external-sources/quarto-cli`, log any further + jupyter-vs-knitr structural discrepancies in this plan (decision 1 + corollary) — resolve toward the common structure, flag for review when + in doubt. (Ongoing through Phase 3/4.) +- [x] All Phase 1 tests pass with real kernels: 19 `text_execute` unit + tests, both `capture_splice_engines` tests, all 5 parity cases + (2026-07-02). Also ran the `#[ignore]`d kernel suite: 25 pass; the 3 + failures are **pre-existing on main** (verified by stashing this + work and re-running the baseline): `test_full_pipeline_*` panic on a + nested tokio runtime in `execute_blocks_async` (filed + **bd-2me3cslx**), `test_kernel_execute_matplotlib` needs matplotlib + which this venv lacks (environmental). + +### Phase 3 — regression sweep + +- [x] `cargo build --workspace` ✅ clean (2026-07-02). +- [x] `cargo nextest run --workspace` ✅ **10181 passed, 0 failed** (1 + "leaky" warning — the knitr R child-process handle, passes; also seen + intermittently on the standalone run). +- [x] `cargo xtask verify` (full, WASM leg included) ✅ "All verification + steps passed!" (2026-07-02). Also, separately: `cargo clippy -p + quarto-core --all-targets` clean, `cargo xtask lint` clean, + `cargo fmt --check` clean. +- [x] Grep for consumers assuming jupyter's old bare-fence shape: **none**. + All `cell-output` consumers found expect the *div* form and start + matching better with this fix: `resources/scss/bootstrap/ + _bootstrap-rules.scss` (`.cell .cell-output-stdout pre code`), + `quarto-core/src/project/listing/post_render_upgrade/reader.rs` + (`div.preview-image div.cell-output-display img`), plus an inert + fixture string in `quarto-test/src/assertions/html_elements.rs`. + hub-client TS / ts-packages: no `cell-output` references at all. + +### Phase 4 — end-to-end verification (per CLAUDE.md) + +- [x] **CLI e2e through the real binary** (2026-07-02): rendered a + two-cell jupyter doc (expression `2 + 3` + `print("streamed + output")`) with `./target/debug/q2 render hello.qmd` (the + workspace-built binary; scratch fixture). Inspected `hello.html`: + two `
` wrappers; `
5`; `
...streamed output`; echoed source + carries `class="sourceCode cell-code code-with-copy"` with highlight + spans present. Output inspected directly, not inferred. +- [x] `q2 preview` browser e2e ✅ (2026-07-02). Chain: `cargo xtask verify` + (rebuilt WASM + q2-preview-spa dist) → `cargo build --bin q2` + (re-embed) → `./target/debug/q2 preview /hello.qmd` → + opened `http://127.0.0.1:59606` in a real Chrome tab. Inspected the + preview iframe DOM: **2 `div.cell` wrappers; `.cell-output + .cell-output-display` containing `5`; `.cell-output + .cell-output-stdout` containing `streamed output`** — i.e. the + server-recorded capture spliced into the WASM-rendered preview, + which is exactly the surface that failed in the original report + (the WASM markdown engine cannot compute `5` itself). Screenshot + captured in the session transcript; the rendered page shows both + highlighted cells with their outputs below. +- [ ] Optional cross-check on the feature branch: re-run the Option-B hub + harness (`claude-notes/hub-execution-e2e/`, feature branch only) after + this fix merges — `hello.qmd` Run should splice like `r-demo.qmd`. + +### Phase 5 — close out + +- [ ] Commit (pre-commit review checklist done; awaiting Carlos's + approval), then `braid close bd-gthycd33`. +- [x] File deferred follow-ups as strands (all `discovered-from: + bd-gthycd33`): **bd-ohvl879u** (error-policy divergence: jupyter + embeds cell errors and continues where knitr fails the render), + **bd-2me3cslx** (pre-existing broken `#[ignore]`d full-pipeline + tests: nested tokio runtime), **bd-u996g8g2** (inline `{python} + expr` in the text path; the retired prototype is the reference), + **bd-5t6wvu7m** (real image outputs instead of the placeholder; + TODO in code now carries this id), **bd-vs7sa0qx** + (`execution_count` attributes when a consumer appears — decision 3). + The AST path did NOT need a follow-up: it was retired in Phase 2. + +## References + +- Strand: bd-gthycd33 (comment c-c3qmbpa3 has the original browser repro). +- Discovery context: `claude-notes/plans/2026-07-01-merge-preview-status-line.md` + ("End-to-end evidence" → "Orthogonal bug found"), on + `feature/hub-execution-provider`. +- Splice design: `crates/quarto-core/src/engine/capture_splice.rs` module + docs; `claude-notes/plans/2026-05-18-q2-preview-project-replay-engine.md`. +- knitr's wrapper emission: `crates/quarto-core/src/engine/knitr/resources/rmd/hooks.R` + (vendored Q1 hooks). +- Quarto 1 jupyter emission (shape reference only): `external-sources/quarto-cli`. diff --git a/crates/quarto-core/src/engine/jupyter/mod.rs b/crates/quarto-core/src/engine/jupyter/mod.rs index 885bda2bf..a8221c8d8 100644 --- a/crates/quarto-core/src/engine/jupyter/mod.rs +++ b/crates/quarto-core/src/engine/jupyter/mod.rs @@ -41,18 +41,14 @@ mod daemon; mod error; mod execute; mod kernelspec; -mod output; mod session; mod text_execute; -mod transform; pub use daemon::{JupyterDaemon, daemon}; pub use error::{JupyterError, Result}; pub use execute::{CellOutput, ExecuteResult, ExecuteStatus, MimeBundle}; pub use kernelspec::{ResolvedKernel, find_kernelspec, is_jupyter_language, list_kernelspecs}; -pub use output::{OutputOptions, outputs_to_blocks}; pub use session::{KernelInfo, KernelSession, SessionKey}; -pub use transform::JupyterTransform; use std::path::{Path, PathBuf}; use std::sync::OnceLock; diff --git a/crates/quarto-core/src/engine/jupyter/output.rs b/crates/quarto-core/src/engine/jupyter/output.rs deleted file mode 100644 index e5040a569..000000000 --- a/crates/quarto-core/src/engine/jupyter/output.rs +++ /dev/null @@ -1,356 +0,0 @@ -/* - * engine/jupyter/output.rs - * Copyright (c) 2025 Posit, PBC - * - * Convert Jupyter outputs to Pandoc AST blocks. - */ - -//! Convert Jupyter kernel outputs to Pandoc AST blocks. -//! -//! This module handles converting the various output types from Jupyter -//! (stream, display_data, execute_result, error) into Pandoc AST -//! elements that can be inserted into the document. - -use hashlink::LinkedHashMap; - -use quarto_pandoc_types::{ - AttrSourceInfo, Block, CodeBlock as CodeBlockStruct, Div, Inline, Paragraph, RawBlock, Str, -}; -use quarto_source_map::{By, SourceInfo}; - -use super::execute::{CellOutput, MimeBundle}; - -/// Create an Attr tuple with the given id and classes. -fn make_attr(id: impl Into, classes: Vec) -> quarto_pandoc_types::Attr { - (id.into(), classes, LinkedHashMap::new()) -} - -/// Options for output conversion. -pub struct OutputOptions { - /// Preferred image format for figures. - pub image_format: ImageFormat, - /// Whether to include raw HTML output. - pub include_html: bool, - /// Whether to include raw LaTeX output. - pub include_latex: bool, - /// Directory for writing figure files. - pub figure_dir: Option, -} - -impl Default for OutputOptions { - fn default() -> Self { - Self { - image_format: ImageFormat::Png, - include_html: true, - include_latex: true, - figure_dir: None, - } - } -} - -/// Preferred image format. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum ImageFormat { - Png, - Svg, - Pdf, -} - -/// Convert cell outputs to AST blocks. -/// -/// Returns a vector of blocks representing the outputs. -pub fn outputs_to_blocks(outputs: &[CellOutput], _options: &OutputOptions) -> Vec { - let mut blocks = Vec::new(); - - for output in outputs { - match output { - CellOutput::Stream { name, text } => { - // Stream output → CodeBlock with output class - let class = format!("cell-output-{}", name); - let block = Block::CodeBlock(CodeBlockStruct { - attr: make_attr("", vec![class]), - text: text.clone(), - source_info: SourceInfo::generated(By::jupyter_output()), - attr_source: AttrSourceInfo::empty(), - }); - blocks.push(block); - } - CellOutput::DisplayData { data, .. } | CellOutput::ExecuteResult { data, .. } => { - // Rich output → best representation - if let Some(block) = mime_bundle_to_block(data) { - blocks.push(block); - } - } - CellOutput::Error { - ename, - evalue, - traceback, - } => { - // Error output → CodeBlock with error styling - let error_text = format_error(ename, evalue, traceback); - let block = Block::CodeBlock(CodeBlockStruct { - attr: make_attr("", vec!["cell-output-error".to_string()]), - text: error_text, - source_info: SourceInfo::generated(By::jupyter_output()), - attr_source: AttrSourceInfo::empty(), - }); - blocks.push(block); - } - } - } - - blocks -} - -/// Convert a MIME bundle to the best AST representation. -fn mime_bundle_to_block(data: &MimeBundle) -> Option { - // Priority order for output - let priorities = [ - "text/html", - "image/svg+xml", - "image/png", - "image/jpeg", - "text/markdown", - "text/latex", - "text/plain", - ]; - - for mime_type in priorities { - if let Some(content) = data.get(mime_type) { - return convert_mime_content(mime_type, content); - } - } - - None -} - -/// Convert a single MIME type content to a block. -fn convert_mime_content(mime_type: &str, content: &serde_json::Value) -> Option { - match mime_type { - "text/plain" => { - let text = content.as_str().unwrap_or(""); - Some(Block::CodeBlock(CodeBlockStruct { - attr: make_attr("", vec!["cell-output".to_string()]), - text: text.to_string(), - source_info: SourceInfo::generated(By::jupyter_output()), - attr_source: AttrSourceInfo::empty(), - })) - } - "text/html" => { - let html = extract_text_content(content); - Some(Block::RawBlock(RawBlock { - format: "html".to_string(), - text: html, - source_info: SourceInfo::generated(By::jupyter_output()), - })) - } - "text/markdown" => { - // For now, wrap markdown in a Div - // Full implementation would parse the markdown - let md = extract_text_content(content); - Some(Block::Div(Div { - attr: make_attr("", vec!["cell-output-markdown".to_string()]), - content: vec![Block::Paragraph(Paragraph { - content: vec![Inline::Str(Str { - text: md, - source_info: SourceInfo::generated(By::jupyter_output()), - })], - source_info: SourceInfo::generated(By::jupyter_output()), - })], - source_info: SourceInfo::generated(By::jupyter_output()), - attr_source: AttrSourceInfo::empty(), - })) - } - "text/latex" => { - let latex = extract_text_content(content); - Some(Block::RawBlock(RawBlock { - format: "latex".to_string(), - text: latex, - source_info: SourceInfo::generated(By::jupyter_output()), - })) - } - "image/png" | "image/jpeg" | "image/svg+xml" => { - // For images, we'd normally save to a file and reference it - // For now, create a placeholder - let ext = match mime_type { - "image/png" => "png", - "image/jpeg" => "jpg", - "image/svg+xml" => "svg", - _ => "bin", - }; - - // TODO: Save image data to file and reference it - // For now, create a placeholder paragraph - let placeholder = format!("[Image output: {}]", ext); - Some(Block::Div(Div { - attr: make_attr("", vec!["cell-output-display".to_string()]), - content: vec![Block::Paragraph(Paragraph { - content: vec![Inline::Str(Str { - text: placeholder, - source_info: SourceInfo::generated(By::jupyter_output()), - })], - source_info: SourceInfo::generated(By::jupyter_output()), - })], - source_info: SourceInfo::generated(By::jupyter_output()), - attr_source: AttrSourceInfo::empty(), - })) - } - _ => None, - } -} - -/// Extract text content from a JSON value. -/// -/// Jupyter can send text as either a string or an array of strings. -fn extract_text_content(value: &serde_json::Value) -> String { - match value { - serde_json::Value::String(s) => s.clone(), - serde_json::Value::Array(arr) => arr - .iter() - .filter_map(|v| v.as_str()) - .collect::>() - .join(""), - _ => String::new(), - } -} - -/// Format error output for display. -fn format_error(ename: &str, evalue: &str, traceback: &[String]) -> String { - let mut output = String::new(); - - // Add error name and value - output.push_str(&format!("{}: {}\n", ename, evalue)); - - // Add traceback if present - if !traceback.is_empty() { - output.push('\n'); - for line in traceback { - // Strip ANSI escape codes - let clean_line = strip_ansi_codes(line); - output.push_str(&clean_line); - output.push('\n'); - } - } - - output -} - -/// Strip ANSI escape codes from a string. -pub fn strip_ansi_codes(s: &str) -> String { - // Simple pattern to remove ANSI escape sequences - let mut result = String::with_capacity(s.len()); - let mut chars = s.chars().peekable(); - - while let Some(c) = chars.next() { - if c == '\x1b' { - // Start of escape sequence - if chars.peek() == Some(&'[') { - chars.next(); // consume '[' - // Skip until we hit a letter (end of sequence) - while let Some(&next) = chars.peek() { - chars.next(); - if next.is_ascii_alphabetic() { - break; - } - } - } - } else { - result.push(c); - } - } - - result -} - -/// Determine the MIME type priority for a target format. -/// -/// Will be used when format-specific output conversion is implemented. -/// Has unit test in this module. -#[allow(dead_code)] -pub fn mime_priority_for_format(format: &str) -> &'static [&'static str] { - match format { - "latex" | "pdf" | "beamer" => &[ - "text/latex", - "application/pdf", - "image/pdf", - "image/png", - "image/jpeg", - "text/plain", - ], - "html" | "html5" | "revealjs" => &[ - "text/html", - "image/svg+xml", - "image/png", - "image/jpeg", - "text/markdown", - "text/plain", - ], - _ => &["text/plain", "image/png", "image/jpeg", "text/html"], - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_extract_text_content_string() { - let value = serde_json::json!("Hello, World!"); - assert_eq!(extract_text_content(&value), "Hello, World!"); - } - - #[test] - fn test_extract_text_content_array() { - let value = serde_json::json!(["Hello, ", "World!"]); - assert_eq!(extract_text_content(&value), "Hello, World!"); - } - - #[test] - fn test_extract_text_content_other() { - let value = serde_json::json!(42); - assert_eq!(extract_text_content(&value), ""); - } - - #[test] - fn test_strip_ansi_codes() { - let input = "\x1b[31mRed\x1b[0m Normal"; - assert_eq!(strip_ansi_codes(input), "Red Normal"); - - let input = "No escape codes"; - assert_eq!(strip_ansi_codes(input), "No escape codes"); - } - - #[test] - fn test_format_error() { - let output = format_error( - "NameError", - "name 'x' is not defined", - &[" File \"\", line 1".to_string()], - ); - - assert!(output.contains("NameError")); - assert!(output.contains("name 'x' is not defined")); - assert!(output.contains("File \"\"")); - } - - #[test] - fn test_outputs_to_blocks_stream() { - let outputs = vec![CellOutput::Stream { - name: "stdout".to_string(), - text: "Hello\n".to_string(), - }]; - - let blocks = outputs_to_blocks(&outputs, &OutputOptions::default()); - assert_eq!(blocks.len(), 1); - assert!(matches!(blocks[0], Block::CodeBlock(_))); - } - - #[test] - fn test_mime_priority_for_format() { - let html_priority = mime_priority_for_format("html"); - assert_eq!(html_priority[0], "text/html"); - - let latex_priority = mime_priority_for_format("latex"); - assert_eq!(latex_priority[0], "text/latex"); - } -} diff --git a/crates/quarto-core/src/engine/jupyter/text_execute.rs b/crates/quarto-core/src/engine/jupyter/text_execute.rs index a1e3fa1dd..f291861a7 100644 --- a/crates/quarto-core/src/engine/jupyter/text_execute.rs +++ b/crates/quarto-core/src/engine/jupyter/text_execute.rs @@ -18,7 +18,6 @@ use regex::Regex; use super::daemon::daemon; use super::error::JupyterError; use super::execute::{CellOutput, ExecuteResult as KernelExecuteResult, ExecuteStatus}; -use super::output::strip_ansi_codes; use super::session::SessionKey; use crate::engine::context::{ExecuteResult, ExecutionContext}; use crate::engine::error::ExecutionError; @@ -176,12 +175,12 @@ async fn execute_blocks_inner( // Append content before this block output.push_str(&input[last_end..block.start]); - // Echo the cell source as a *non-executable* fenced block. - // The `{lang}` fence carries execution semantics (the engine - // just ran it); after execution the echoed source should - // render as plain code so the highlight stage picks it up. - output.push_str(&echoed_source_fence(block)); - output.push('\n'); + // A fenced div cannot interrupt a paragraph — make sure the + // `::: {.cell}` opener starts its own block even when the + // source had no blank line before the cell. + if output.ends_with('\n') && !output.ends_with("\n\n") { + output.push('\n'); + } // Execute the code let exec_result = daemon @@ -189,11 +188,9 @@ async fn execute_blocks_inner( .await .ok_or(JupyterError::NotConnected)??; - // Format and append outputs - let output_md = format_outputs(&exec_result); - if !output_md.is_empty() { - output.push_str(&output_md); - } + // Emit the whole cell — echoed source plus outputs — in the + // Quarto-canonical `::: {.cell}` shape. + output.push_str(&render_cell(block, &exec_result)); last_end = block.end; } @@ -204,54 +201,122 @@ async fn execute_blocks_inner( Ok(ExecuteResult::new(output)) } +/// Fence for `text`, sized so the fence can never collide with the +/// content: max(3, longest leading backtick run in `text` + 1) +/// backticks. Mirrors Q1's `ticksForCode` +/// (`external-sources/quarto-cli/src/core/jupyter/jupyter.ts`); a +/// fixed ``` corrupts the emitted markdown whenever a cell or its +/// output contains a line starting with three backticks. +fn ticks_for_code(text: &str) -> String { + let longest = text + .lines() + .map(|line| line.trim_start().bytes().take_while(|b| *b == b'`').count()) + .max() + .unwrap_or(0); + "`".repeat(std::cmp::max(3, longest + 1)) +} + +/// Render one executed cell — echoed source plus formatted outputs — +/// in the Quarto-canonical cell shape shared with knitr's hooks and +/// Q1's jupyter engine (bd-gthycd33): +/// +/// ```markdown +/// ::: {.cell} +/// +/// ```{.python .cell-code} +/// 2 + 3 +/// ``` +/// +/// ::: {.cell-output .cell-output-display} +/// +/// ``` +/// 5 +/// ``` +/// +/// ::: +/// ::: +/// ``` +/// +/// The `Div.cell` wrapper is a cross-engine contract: the preview +/// capture splice (`crate::engine::capture_splice`) replaces a live +/// engine cell with exactly one wrapper Div, and the Bootstrap CSS +/// targets `.cell .cell-output-* pre code`. A cell with no outputs +/// still gets the wrapper (knitr wraps output-less chunks too). +fn render_cell(block: &CodeBlock, result: &KernelExecuteResult) -> String { + format!( + "::: {{.cell}}\n\n{}\n{}\n:::\n", + echoed_source_fence(block), + format_outputs(result) + ) +} + /// Reconstruct the echoed source fence for an executed cell. /// /// The parser captures code cells with a `{lang}` fence (the curly /// braces mean "the engine should execute this"). After the engine -/// runs, we emit the source back into the output markdown so readers -/// see it — but at that point the braces are misleading: the block is -/// no longer scheduled for execution, and downstream stages treat -/// `{lang}` as an opaque class (no highlighting, no language awareness). -/// -/// Strip the braces; keep just the language and the code. Per-cell -/// directives like `#| echo: false` travel inside `block.code` and are -/// handled by whatever stage consumes them — this function only -/// rewrites the fence. +/// runs, we emit the source back as an *attribute-form* fence — +/// `{.python .cell-code}` — so the block is no longer scheduled for +/// execution, the highlight stage resolves the language from the +/// first class, and downstream consumers can target `.cell-code` +/// (same classes knitr's hooks emit). Per-cell directives like +/// `#| echo: false` travel inside `block.code` and are handled by +/// whatever stage consumes them — this function only rewrites the +/// fence. fn echoed_source_fence(block: &CodeBlock) -> String { let code = block.code.trim_end_matches('\n'); - format!("```{}\n{}\n```", block.language, code) + let ticks = ticks_for_code(code); + format!( + "{ticks}{{.{} .cell-code}}\n{}\n{ticks}", + block.language, code + ) +} + +/// Wrap already-trimmed output text in a `::: {}` div around +/// a plain fence, with the fence sized to the content. +fn fenced_output_div(classes: &str, text: &str) -> String { + let ticks = ticks_for_code(text); + format!("\n::: {{{classes}}}\n\n{ticks}\n{text}\n{ticks}\n\n:::\n") } /// Format kernel outputs as markdown. +/// +/// Every output becomes a `::: {.cell-output .cell-output-}` +/// div wrapping a plain fence — the Q1/knitr class scheme +/// (`outputTypeCssClass` in quarto-cli's `jupyter.ts`): streams are +/// `-stdout` / `-stderr`, `execute_result` / `display_data` are +/// `-display`, errors are `-error`. fn format_outputs(result: &KernelExecuteResult) -> String { let mut output = String::new(); for cell_output in &result.outputs { match cell_output { CellOutput::Stream { name, text } => { - // Stream output as a code block with output class - output.push_str(&format!( - "\n```{{.cell-output-{}}}\n{}\n```\n", - name, - text.trim_end() + output.push_str(&fenced_output_div( + &format!(".cell-output .cell-output-{}", name), + text.trim_end(), )); } CellOutput::ExecuteResult { data, .. } | CellOutput::DisplayData { data, .. } => { // Rich output - pick best format if let Some(text) = data.get("text/plain") { - if let Some(s) = text.as_str() { - output.push_str(&format!("\n```{{.cell-output}}\n{}\n```\n", s.trim_end())); - } + let s = extract_text_content(text); + output.push_str(&fenced_output_div( + ".cell-output .cell-output-display", + s.trim_end(), + )); } else if let Some(html) = data.get("text/html") { - if let Some(s) = html.as_str() { - output.push_str(&format!( - "\n::: {{.cell-output-display}}\n```{{=html}}\n{}\n```\n:::\n", - s - )); - } + let s = extract_text_content(html); + let ticks = ticks_for_code(&s); + output.push_str(&format!( + "\n::: {{.cell-output .cell-output-display}}\n\n{ticks}{{=html}}\n{}\n{ticks}\n\n:::\n", + s + )); } else if data.contains_key("image/png") || data.contains_key("image/svg+xml") { - // TODO: Save image to file and reference it - output.push_str("\n::: {.cell-output-display}\n[Image output]\n:::\n"); + // TODO(bd-5t6wvu7m): save the image to a supporting + // file and emit a real figure instead of a placeholder. + output.push_str( + "\n::: {.cell-output .cell-output-display}\n\n[Image output]\n\n:::\n", + ); } } CellOutput::Error { @@ -259,15 +324,9 @@ fn format_outputs(result: &KernelExecuteResult) -> String { evalue, traceback, } => { - // Error output - let mut error_text = format!("{}: {}\n", ename, evalue); - for line in traceback { - error_text.push_str(&strip_ansi_codes(line)); - error_text.push('\n'); - } - output.push_str(&format!( - "\n```{{.cell-output-error}}\n{}\n```\n", - error_text.trim_end() + output.push_str(&fenced_output_div( + ".cell-output .cell-output-error", + format_error_text(ename, evalue, traceback).trim_end(), )); } } @@ -282,14 +341,9 @@ fn format_outputs(result: &KernelExecuteResult) -> String { { // Only add if not already in outputs if result.outputs.is_empty() { - let mut error_text = format!("{}: {}\n", ename, evalue); - for line in traceback { - error_text.push_str(&strip_ansi_codes(line)); - error_text.push('\n'); - } - output.push_str(&format!( - "\n```{{.cell-output-error}}\n{}\n```\n", - error_text.trim_end() + output.push_str(&fenced_output_div( + ".cell-output .cell-output-error", + format_error_text(ename, evalue, traceback).trim_end(), )); } } @@ -297,6 +351,59 @@ fn format_outputs(result: &KernelExecuteResult) -> String { output } +/// `": "` plus the ANSI-stripped traceback lines. +fn format_error_text(ename: &str, evalue: &str, traceback: &[String]) -> String { + let mut error_text = format!("{}: {}\n", ename, evalue); + for line in traceback { + error_text.push_str(&strip_ansi_codes(line)); + error_text.push('\n'); + } + error_text +} + +/// Extract text content from a MIME-bundle JSON value. Jupyter can +/// send text as either a single string or an array of line strings +/// (the nbformat multiline convention). +fn extract_text_content(value: &serde_json::Value) -> String { + match value { + serde_json::Value::String(s) => s.clone(), + serde_json::Value::Array(arr) => arr + .iter() + .filter_map(|v| v.as_str()) + .collect::>() + .join(""), + _ => String::new(), + } +} + +/// Strip ANSI escape codes from a string (kernel tracebacks arrive +/// colorized). +fn strip_ansi_codes(s: &str) -> String { + // Simple pattern to remove ANSI escape sequences + let mut result = String::with_capacity(s.len()); + let mut chars = s.chars().peekable(); + + while let Some(c) = chars.next() { + if c == '\x1b' { + // Start of escape sequence + if chars.peek() == Some(&'[') { + chars.next(); // consume '[' + // Skip until we hit a letter (end of sequence) + while let Some(&next) = chars.peek() { + chars.next(); + if next.is_ascii_alphabetic() { + break; + } + } + } + } else { + result.push(c); + } + } + + result +} + #[cfg(test)] mod tests { use super::*; @@ -406,54 +513,230 @@ print("hello") assert!(!is_executable_language("markdown")); } - #[test] - fn test_echoed_source_fence_strips_braces() { - // `{python}` fence means "execute"; after execution the echoed - // source must come back as a plain `python` fence so the - // highlight stage can pick it up. - let block = CodeBlock { + // ── Emission shape (bd-gthycd33): Quarto-canonical cells ────── + // + // The post-engine markdown must match the shape knitr's vendored + // Q1 hooks and Q1's own jupyter engine emit: every executed cell + // wrapped in `::: {.cell}`, echoed source as a + // `{. .cell-code}` fence, outputs as + // `::: {.cell-output .cell-output-*}` divs around plain fences. + // The preview capture splice keys on the `Div.cell` wrapper and + // the Bootstrap CSS keys on `.cell .cell-output-* pre code`; + // wrapper-less emission breaks both. Exact-string assertions on + // purpose — this shape is a cross-engine contract (see the + // engine_output_parity integration suite). + + fn py_block(code: &str) -> CodeBlock { + CodeBlock { start: 0, end: 0, language: "python".to_string(), - code: "print(\"hi\")\n".to_string(), - }; - assert_eq!(echoed_source_fence(&block), "```python\nprint(\"hi\")\n```"); + code: code.to_string(), + } } - #[test] - fn test_format_outputs_stream() { - let result = KernelExecuteResult { + fn ok_result(outputs: Vec) -> KernelExecuteResult { + KernelExecuteResult { status: ExecuteStatus::Ok, - outputs: vec![CellOutput::Stream { - name: "stdout".to_string(), - text: "Hello, World!\n".to_string(), - }], + outputs, execution_count: Some(1), - }; + } + } - let output = format_outputs(&result); - assert!(output.contains("cell-output-stdout")); - assert!(output.contains("Hello, World!")); + fn text_plain(s: &str) -> std::collections::HashMap { + let mut data = std::collections::HashMap::new(); + data.insert("text/plain".to_string(), serde_json::json!(s)); + data } #[test] - fn test_format_outputs_error() { - let result = KernelExecuteResult { - status: ExecuteStatus::Error { - ename: "NameError".to_string(), - evalue: "name 'x' is not defined".to_string(), - traceback: vec!["Traceback...".to_string()], - }, - outputs: vec![CellOutput::Error { - ename: "NameError".to_string(), - evalue: "name 'x' is not defined".to_string(), - traceback: vec!["Traceback...".to_string()], - }], - execution_count: Some(1), - }; + fn test_echoed_source_fence_emits_cell_code_class() { + // `{python}` fence means "execute"; after execution the echoed + // source comes back as an attribute-form fence with the + // language class first (the highlight stage resolves the + // language from the first class) plus `.cell-code`. + let block = py_block("print(\"hi\")\n"); + assert_eq!( + echoed_source_fence(&block), + "```{.python .cell-code}\nprint(\"hi\")\n```" + ); + } + + #[test] + fn test_echoed_source_fence_grows_ticks_for_backtick_content() { + // Q1's ticksForCode rule: max(3, longest leading backtick + // run + 1). Code containing a ``` line must get a 4-tick + // fence or the emitted markdown is corrupt. + let block = py_block("s = \"\"\n```\n\"\"\n"); + let fence = echoed_source_fence(&block); + assert!( + fence.starts_with("````{.python .cell-code}\n"), + "expected a 4-tick fence, got:\n{fence}" + ); + assert!(fence.ends_with("\n````"), "got:\n{fence}"); + } - let output = format_outputs(&result); - assert!(output.contains("cell-output-error")); - assert!(output.contains("NameError")); + #[test] + fn test_format_outputs_stream_stdout_is_cell_output_div() { + let result = ok_result(vec![CellOutput::Stream { + name: "stdout".to_string(), + text: "Hello, World!\n".to_string(), + }]); + assert_eq!( + format_outputs(&result), + "\n::: {.cell-output .cell-output-stdout}\n\n```\nHello, World!\n```\n\n:::\n" + ); + } + + #[test] + fn test_format_outputs_stream_stderr_is_cell_output_div() { + let result = ok_result(vec![CellOutput::Stream { + name: "stderr".to_string(), + text: "warning\n".to_string(), + }]); + assert_eq!( + format_outputs(&result), + "\n::: {.cell-output .cell-output-stderr}\n\n```\nwarning\n```\n\n:::\n" + ); + } + + #[test] + fn test_format_outputs_execute_result_is_display_div() { + // Q1: execute_result / display_data are `.cell-output-display` + // (not `-stdout` — they aren't streams). + let result = ok_result(vec![CellOutput::ExecuteResult { + execution_count: 1, + data: text_plain("5"), + metadata: serde_json::json!({}), + }]); + assert_eq!( + format_outputs(&result), + "\n::: {.cell-output .cell-output-display}\n\n```\n5\n```\n\n:::\n" + ); + } + + #[test] + fn test_format_outputs_html_is_display_div_with_raw_html() { + let mut data = std::collections::HashMap::new(); + data.insert("text/html".to_string(), serde_json::json!("5")); + let result = ok_result(vec![CellOutput::DisplayData { + data, + metadata: serde_json::json!({}), + }]); + assert_eq!( + format_outputs(&result), + "\n::: {.cell-output .cell-output-display}\n\n```{=html}\n5\n```\n\n:::\n" + ); + } + + #[test] + fn test_format_outputs_error_is_error_div() { + let result = ok_result(vec![CellOutput::Error { + ename: "NameError".to_string(), + evalue: "name 'x' is not defined".to_string(), + traceback: vec!["Traceback...".to_string()], + }]); + assert_eq!( + format_outputs(&result), + "\n::: {.cell-output .cell-output-error}\n\n```\nNameError: name 'x' is not defined\nTraceback...\n```\n\n:::\n" + ); + } + + #[test] + fn test_format_outputs_grows_ticks_for_backtick_output() { + // An output whose text contains a ``` line must get a wider + // fence, or the emitted markdown is corrupt. + let result = ok_result(vec![CellOutput::Stream { + name: "stdout".to_string(), + text: "```\n".to_string(), + }]); + assert_eq!( + format_outputs(&result), + "\n::: {.cell-output .cell-output-stdout}\n\n````\n```\n````\n\n:::\n" + ); + } + + #[test] + fn test_render_cell_wraps_source_and_outputs_in_cell_div() { + let block = py_block("2 + 3\n"); + let result = ok_result(vec![CellOutput::ExecuteResult { + execution_count: 1, + data: text_plain("5"), + metadata: serde_json::json!({}), + }]); + assert_eq!( + render_cell(&block, &result), + "::: {.cell}\n\n```{.python .cell-code}\n2 + 3\n```\n\n::: {.cell-output .cell-output-display}\n\n```\n5\n```\n\n:::\n\n:::\n" + ); + } + + #[test] + fn test_format_outputs_image_placeholder_is_display_div() { + // Until bd-5t6wvu7m lands, image outputs render a placeholder — + // but it must already sit in the canonical display div. + let mut data = std::collections::HashMap::new(); + data.insert("image/png".to_string(), serde_json::json!("base64…")); + let result = ok_result(vec![CellOutput::DisplayData { + data, + metadata: serde_json::json!({}), + }]); + assert_eq!( + format_outputs(&result), + "\n::: {.cell-output .cell-output-display}\n\n[Image output]\n\n:::\n" + ); + } + + #[test] + fn test_strip_ansi_codes() { + let input = "\x1b[31mRed\x1b[0m Normal"; + assert_eq!(strip_ansi_codes(input), "Red Normal"); + + let input = "No escape codes"; + assert_eq!(strip_ansi_codes(input), "No escape codes"); + } + + #[test] + fn test_extract_text_content_string_and_array() { + assert_eq!( + extract_text_content(&serde_json::json!("Hello, World!")), + "Hello, World!" + ); + // nbformat multiline convention: array of line strings. + assert_eq!( + extract_text_content(&serde_json::json!(["Hello, ", "World!"])), + "Hello, World!" + ); + assert_eq!(extract_text_content(&serde_json::json!(42)), ""); + } + + #[test] + fn test_format_outputs_multiline_array_text_plain() { + // A kernel sending text/plain as an array of lines must not + // be silently dropped (the old `as_str()` path did that). + let mut data = std::collections::HashMap::new(); + data.insert("text/plain".to_string(), serde_json::json!(["4", "2"])); + let result = ok_result(vec![CellOutput::ExecuteResult { + execution_count: 1, + data, + metadata: serde_json::json!({}), + }]); + assert_eq!( + format_outputs(&result), + "\n::: {.cell-output .cell-output-display}\n\n```\n42\n```\n\n:::\n" + ); + } + + #[test] + fn test_render_cell_without_output_still_wraps() { + // A source-only cell (assignment, `output: false`, …) still + // gets the `.cell` wrapper — the splice must be able to + // replace the live cell, and knitr wraps output-less chunks + // too. + let block = py_block("x = 1\n"); + let result = ok_result(vec![]); + assert_eq!( + render_cell(&block, &result), + "::: {.cell}\n\n```{.python .cell-code}\nx = 1\n```\n\n:::\n" + ); } } diff --git a/crates/quarto-core/src/engine/jupyter/transform.rs b/crates/quarto-core/src/engine/jupyter/transform.rs deleted file mode 100644 index 2c7aac260..000000000 --- a/crates/quarto-core/src/engine/jupyter/transform.rs +++ /dev/null @@ -1,492 +0,0 @@ -/* - * engine/jupyter/transform.rs - * Copyright (c) 2025 Posit, PBC - * - * JupyterEngine AST transform implementation. - */ - -//! AST transform implementation for Jupyter execution. -//! -//! This module implements the `AstTransform` trait for the Jupyter engine, -//! allowing code cells in the Pandoc AST to be executed via Jupyter kernels. - -use std::path::PathBuf; -use std::sync::Arc; - -use quarto_pandoc_types::block::Block; -use quarto_pandoc_types::inline::{Inline, Str}; -use quarto_pandoc_types::pandoc::Pandoc; -use quarto_source_map::{By, SourceInfo}; - -use crate::Result; -use crate::render::RenderContext; -use crate::transform::AstTransform; - -use super::daemon::{JupyterDaemon, daemon}; -use super::execute::CellOutput; -use super::kernelspec::is_jupyter_language; -use super::output::{OutputOptions, outputs_to_blocks}; -use super::session::SessionKey; - -/// Jupyter engine AST transform. -/// -/// Transforms the AST by executing code blocks via Jupyter kernels -/// and replacing them with output blocks. -pub struct JupyterTransform { - /// The daemon managing kernel sessions. - daemon: Arc, -} - -/// Information about an inline expression to evaluate. -#[derive(Debug, Clone)] -struct InlineExpr { - /// Block index - block_idx: usize, - /// Inline index within the block's content - inline_idx: usize, - /// Language (e.g., "python") - language: String, - /// The expression code - code: String, -} - -impl JupyterTransform { - /// Create a new Jupyter transform using the global daemon. - pub fn new() -> Self { - Self { daemon: daemon() } - } - - /// Create a Jupyter transform with a specific daemon. - pub fn with_daemon(daemon: Arc) -> Self { - Self { daemon } - } - - /// Extract executable code blocks from the AST. - /// - /// Returns a list of (index, language, code) tuples. - fn extract_code_cells(ast: &Pandoc) -> Vec<(usize, String, String)> { - let mut cells = Vec::new(); - - for (idx, block) in ast.blocks.iter().enumerate() { - if let Block::CodeBlock(cb) = block { - // Check if this is an executable code block - // by looking at the classes (second element of attr tuple) - let (_, classes, _) = &cb.attr; - - for class in classes { - // Handle both plain language and Quarto syntax {python} - let lang = class.trim_matches(|c| c == '{' || c == '}'); - if is_jupyter_language(lang) { - cells.push((idx, lang.to_lowercase(), cb.text.clone())); - break; // Only take first matching language - } - } - } - } - - cells - } - - /// Determine the kernel to use from the code cells. - fn determine_kernel(cells: &[(usize, String, String)]) -> Option { - // For now, use the first language found and map to kernel name - cells - .first() - .map(|(_, lang, _)| Self::language_to_kernel(lang)) - } - - /// Map a language name to a kernel name. - fn language_to_kernel(lang: &str) -> String { - match lang { - "python" => "python3".to_string(), - "julia" => "julia-1.9".to_string(), // Common default - "r" => "ir".to_string(), - other => other.to_string(), - } - } - - /// Execute all code cells and collect outputs. - async fn execute_cells( - &self, - key: &SessionKey, - cells: &[(usize, String, String)], - ) -> Result)>> { - let mut results = Vec::new(); - - for (idx, _lang, code) in cells { - // Execute the code - let exec_result = self - .daemon - .execute_in_session(key, code) - .await - .ok_or_else(|| crate::error::QuartoError::other("Kernel session not found"))? - .map_err(|e| crate::error::QuartoError::other(format!("Execution error: {}", e)))?; - - // Convert outputs to AST blocks - let options = OutputOptions::default(); - let output_blocks = outputs_to_blocks(&exec_result.outputs, &options); - - results.push((*idx, output_blocks)); - } - - Ok(results) - } - - /// Replace code blocks with their outputs in the AST. - fn replace_with_outputs(ast: &mut Pandoc, outputs: Vec<(usize, Vec)>) { - // Process in reverse order to maintain correct indices - let mut sorted_outputs = outputs; - sorted_outputs.sort_by_key(|a| std::cmp::Reverse(a.0)); - - for (idx, output_blocks) in sorted_outputs { - if idx < ast.blocks.len() { - // Remove the code block - ast.blocks.remove(idx); - - // Insert output blocks at the same position - for (i, block) in output_blocks.into_iter().enumerate() { - ast.blocks.insert(idx + i, block); - } - } - } - } - - /// Extract inline expressions from the AST. - /// - /// Looks for Code inlines like `{python} 1+1` or `{r} x` in Paragraph blocks. - fn extract_inline_expressions(ast: &Pandoc) -> Vec { - let mut exprs = Vec::new(); - - for (block_idx, block) in ast.blocks.iter().enumerate() { - // Only look in paragraphs for now - if let Block::Paragraph(para) = block { - for (inline_idx, inline) in para.content.iter().enumerate() { - if let Inline::Code(code) = inline { - // Check if it starts with {language} - if let Some(expr) = Self::parse_inline_expression(&code.text) - && is_jupyter_language(&expr.0) - { - exprs.push(InlineExpr { - block_idx, - inline_idx, - language: expr.0, - code: expr.1, - }); - } - } - } - } - } - - exprs - } - - /// Parse an inline expression like `{python} 1+1` or `python 1+1`. - /// - /// Returns (language, expression) if successful. - fn parse_inline_expression(text: &str) -> Option<(String, String)> { - let text = text.trim(); - - // Try {language} syntax first - if text.starts_with('{') - && let Some(close_brace) = text.find('}') - { - let lang = text[1..close_brace].trim().to_lowercase(); - let expr = text[close_brace + 1..].trim().to_string(); - if !lang.is_empty() && !expr.is_empty() { - return Some((lang, expr)); - } - } - - // Try plain `python expr` syntax (common in Quarto) - let parts: Vec<&str> = text.splitn(2, char::is_whitespace).collect(); - if parts.len() == 2 { - let lang = parts[0].trim().to_lowercase(); - let expr = parts[1].trim().to_string(); - if !lang.is_empty() && !expr.is_empty() { - return Some((lang, expr)); - } - } - - None - } - - /// Execute inline expressions and return their results. - async fn execute_inline_expressions( - &self, - key: &SessionKey, - exprs: &[InlineExpr], - ) -> Result> { - let mut results = Vec::new(); - - for expr in exprs { - // Execute the expression silently and capture the result - let exec_result = self - .daemon - .execute_in_session(key, &expr.code) - .await - .ok_or_else(|| crate::error::QuartoError::other("Kernel session not found"))? - .map_err(|e| { - crate::error::QuartoError::other(format!("Inline execution error: {}", e)) - })?; - - // Extract text/plain from the result - let result_text = exec_result - .outputs - .iter() - .find_map(|output| match output { - CellOutput::ExecuteResult { data, .. } => data - .get("text/plain") - .and_then(|v| v.as_str()) - .map(|s| s.trim_matches('\'').trim_matches('"').to_string()), - CellOutput::Stream { name, text } if name == "stdout" => { - Some(text.trim().to_string()) - } - _ => None, - }) - .unwrap_or_else(String::new); - - results.push(result_text); - } - - Ok(results) - } - - /// Replace inline expressions with their results in the AST. - fn replace_inline_expressions(ast: &mut Pandoc, exprs: &[InlineExpr], results: &[String]) { - // Group by block index and process in reverse order - let mut replacements: Vec<(usize, usize, String)> = exprs - .iter() - .zip(results.iter()) - .map(|(expr, result)| (expr.block_idx, expr.inline_idx, result.clone())) - .collect(); - - // Sort by block_idx desc, then inline_idx desc to process in reverse order - replacements.sort_by(|a, b| { - if a.0 != b.0 { - b.0.cmp(&a.0) - } else { - b.1.cmp(&a.1) - } - }); - - for (block_idx, inline_idx, result) in replacements { - if let Some(Block::Paragraph(para)) = ast.blocks.get_mut(block_idx) - && inline_idx < para.content.len() - { - // Replace the Code inline with a Str inline - para.content[inline_idx] = Inline::Str(Str { - text: result, - source_info: SourceInfo::generated(By::jupyter_output()), - }); - } - } - } -} - -impl Default for JupyterTransform { - fn default() -> Self { - Self::new() - } -} - -#[async_trait::async_trait(?Send)] -impl AstTransform for JupyterTransform { - fn name(&self) -> &str { - "jupyter" - } - - async fn transform(&self, ast: &mut Pandoc, ctx: &mut RenderContext) -> Result<()> { - // Check if execution is enabled - if !ctx.options.execute { - tracing::debug!("Jupyter execution disabled, skipping"); - return Ok(()); - } - - // Extract code cells and inline expressions - let cells = Self::extract_code_cells(ast); - let inline_exprs = Self::extract_inline_expressions(ast); - - if cells.is_empty() && inline_exprs.is_empty() { - tracing::debug!("No executable Jupyter code cells or inline expressions found"); - return Ok(()); - } - - tracing::info!( - cell_count = cells.len(), - inline_count = inline_exprs.len(), - "Found Jupyter code cells and inline expressions" - ); - - // Determine kernel from cells or inline expressions - let kernel_name = Self::determine_kernel(&cells) - .or_else(|| { - inline_exprs - .first() - .map(|e| Self::language_to_kernel(&e.language)) - }) - .ok_or_else(|| { - crate::error::QuartoError::other("Could not determine Jupyter kernel") - })?; - - // Get working directory from document - let working_dir = ctx - .document - .input - .parent() - .map_or_else(|| ctx.project.dir.clone(), PathBuf::from); - - // Create session key - let key = SessionKey::new(&kernel_name, working_dir.clone()); - - // Run async execution - let daemon = self.daemon.clone(); - let cells_clone = cells.clone(); - let inline_exprs_clone = inline_exprs.clone(); - - let (outputs, inline_results) = tokio::task::block_in_place(|| { - tokio::runtime::Handle::current().block_on(async { - // Start or get kernel session - daemon - .get_or_start_session(&kernel_name, &working_dir) - .await - .map_err(|e| { - crate::error::QuartoError::other(format!("Failed to start kernel: {}", e)) - })?; - - // Execute all code cells - let cell_outputs = if !cells_clone.is_empty() { - self.execute_cells(&key, &cells_clone).await? - } else { - Vec::new() - }; - - // Execute inline expressions - let inline_results = if !inline_exprs_clone.is_empty() { - self.execute_inline_expressions(&key, &inline_exprs_clone) - .await? - } else { - Vec::new() - }; - - Ok::<_, crate::error::QuartoError>((cell_outputs, inline_results)) - }) - })?; - - // Replace code blocks with outputs - if !outputs.is_empty() { - Self::replace_with_outputs(ast, outputs); - } - - // Replace inline expressions with results - if !inline_results.is_empty() { - Self::replace_inline_expressions(ast, &inline_exprs, &inline_results); - } - - tracing::info!("Jupyter execution complete"); - Ok(()) - } -} - -#[cfg(test)] -mod tests { - use super::*; - use quarto_pandoc_types::ConfigValue; - use quarto_pandoc_types::attr::AttrSourceInfo; - use quarto_pandoc_types::block::{CodeBlock, Paragraph}; - use quarto_pandoc_types::inline::{Inline, Str}; - use quarto_source_map::SourceInfo; - - fn make_code_block(lang: &str, code: &str) -> Block { - Block::CodeBlock(CodeBlock { - attr: ( - String::new(), - vec![format!("{{{}}}", lang)], - Default::default(), - ), - text: code.to_string(), - source_info: SourceInfo::for_test(), - attr_source: AttrSourceInfo::empty(), - }) - } - - fn make_paragraph(text: &str) -> Block { - Block::Paragraph(Paragraph { - content: vec![Inline::Str(Str { - text: text.to_string(), - source_info: SourceInfo::for_test(), - })], - source_info: SourceInfo::for_test(), - }) - } - - #[test] - fn test_extract_code_cells_empty() { - let ast = Pandoc { - meta: ConfigValue::new_map(vec![], SourceInfo::for_test()), - blocks: vec![make_paragraph("Hello")], - }; - - let cells = JupyterTransform::extract_code_cells(&ast); - assert!(cells.is_empty()); - } - - #[test] - fn test_extract_code_cells_python() { - let ast = Pandoc { - meta: ConfigValue::new_map(vec![], SourceInfo::for_test()), - blocks: vec![ - make_paragraph("Intro"), - make_code_block("python", "print('hello')"), - make_paragraph("Middle"), - make_code_block("python", "x = 1 + 1"), - ], - }; - - let cells = JupyterTransform::extract_code_cells(&ast); - assert_eq!(cells.len(), 2); - assert_eq!( - cells[0], - (1, "python".to_string(), "print('hello')".to_string()) - ); - assert_eq!(cells[1], (3, "python".to_string(), "x = 1 + 1".to_string())); - } - - #[test] - fn test_extract_code_cells_non_jupyter() { - let ast = Pandoc { - meta: ConfigValue::new_map(vec![], SourceInfo::for_test()), - blocks: vec![ - make_code_block("rust", "fn main() {}"), - make_code_block("javascript", "console.log('hi')"), - ], - }; - - let cells = JupyterTransform::extract_code_cells(&ast); - assert!(cells.is_empty()); - } - - #[test] - fn test_determine_kernel_python() { - let cells = vec![(0, "python".to_string(), "code".to_string())]; - assert_eq!( - JupyterTransform::determine_kernel(&cells), - Some("python3".to_string()) - ); - } - - #[test] - fn test_determine_kernel_julia() { - let cells = vec![(0, "julia".to_string(), "code".to_string())]; - assert_eq!( - JupyterTransform::determine_kernel(&cells), - Some("julia-1.9".to_string()) - ); - } - - #[test] - fn test_transform_name() { - let transform = JupyterTransform::new(); - assert_eq!(transform.name(), "jupyter"); - } -} diff --git a/crates/quarto-core/tests/integration/capture_splice_engines.rs b/crates/quarto-core/tests/integration/capture_splice_engines.rs new file mode 100644 index 000000000..70630f190 --- /dev/null +++ b/crates/quarto-core/tests/integration/capture_splice_engines.rs @@ -0,0 +1,163 @@ +//! bd-gthycd33: engine captures must be consumable by the preview +//! capture splice for BOTH engines. +//! +//! Regression pair for the bug where a jupyter capture arrived in the +//! hub-client preview but its output never spliced in (the `{python}` +//! cell kept rendering as raw source), while the identical knitr flow +//! worked. Root cause: the jupyter engine emitted its executed cells +//! *without* the `::: {.cell}` wrapper that +//! `derive_cell_outputs` (crate::engine::capture_splice) requires, so +//! the cell-output map came out empty and the splice was a fail-soft +//! no-op. +//! +//! Each test drives the real engine through `record_capture` — the +//! exact producer path `q2 preview` and `q2 provide-hub` use — then +//! replays what `CaptureSpliceStage` does on the consumer side: +//! parse `capture.input_qmd` / `capture.result.markdown` with the +//! pampa qmd reader, derive the cell-output map, and splice onto the +//! live AST. Tests skip (with a note) when the engine isn't installed. + +use std::path::PathBuf; +use std::sync::Arc; + +use quarto_core::engine::EngineRegistry; +use quarto_core::engine::capture_splice::{derive_cell_outputs, engine_cell_lang, splice_cells}; +use quarto_core::engine::preview_record::record_capture; +use quarto_core::project::ProjectContext; +use quarto_pandoc_types::{Block, Div, Pandoc}; +use quarto_system_runtime::{NativeRuntime, SystemRuntime}; + +fn engine_available(name: &str) -> bool { + EngineRegistry::default() + .get(name) + .is_some_and(|e| e.is_available()) +} + +fn fixture( + content: &str, +) -> ( + tempfile::TempDir, + PathBuf, + ProjectContext, + Arc, +) { + let dir = tempfile::tempdir().unwrap(); + let qmd_path = dir.path().join("doc.qmd"); + std::fs::write(&qmd_path, content).unwrap(); + let runtime: Arc = Arc::new(NativeRuntime::new()); + let project = ProjectContext::discover(&qmd_path, runtime.as_ref()).unwrap(); + (dir, qmd_path, project, runtime) +} + +/// Parse a QMD string the way `CaptureSpliceStage` does (no source +/// tracking; throwaway file name). +fn parse(qmd: &str, name: &str) -> Pandoc { + let (pandoc, _, _) = pampa::readers::qmd::read( + qmd.as_bytes(), + false, + name, + &mut std::io::sink(), + false, + None, + ) + .expect("parse"); + pandoc +} + +fn is_cell_div(block: &Block) -> bool { + let Block::Div(Div { attr, .. }) = block else { + return false; + }; + attr.1.iter().any(|c| c == "cell") +} + +/// Run `content` through the real engine via `record_capture`, then +/// assert the capture splices: the cell-output map is non-empty and +/// the engine cell in the (unedited) live AST gets replaced by the +/// captured `Div.cell` wrapper. +fn assert_capture_splices(content: &str, engine: &str) { + if !engine_available(engine) { + eprintln!("Skipping test: engine '{engine}' not available on this machine"); + return; + } + + let (_tmp, path, project, runtime) = fixture(content); + // Mirror the provider's calling convention (quarto-hub-provider's + // spawn_blocking + pollster::block_on): the jupyter engine builds + // its own current-thread tokio runtime internally, so this must + // NOT run inside a #[tokio::test] context. + let captures = + pollster::block_on(record_capture(&path, &project, runtime, None)).expect("record_capture"); + assert_eq!(captures.len(), 1, "expected exactly one capture"); + let capture = &captures[0]; + assert_eq!(capture.engine_name, engine); + + let result_markdown = capture + .result + .get("markdown") + .and_then(|v| v.as_str()) + .unwrap_or(""); + + let a1 = parse(&capture.input_qmd, "capture-input.rmarkdown"); + let b1 = parse(result_markdown, "capture-result.md"); + + let map = derive_cell_outputs(&a1, &b1); + assert_eq!( + map.len(), + 1, + "bd-gthycd33: derive_cell_outputs must map the single {engine} cell; \ + capture.result.markdown was:\n{result_markdown}" + ); + + // The live AST for an unedited doc: input_qmd is the canonical + // serialization of the live pre-engine AST, so re-parsing it is + // the same AST the browser-side pipeline would hand the splice. + let a2 = parse(&capture.input_qmd, "live.qmd"); + let out = splice_cells(a2, &map, engine); + + let cell_divs = out.blocks.iter().filter(|b| is_cell_div(b)).count(); + assert_eq!( + cell_divs, + 1, + "splice must replace the {engine} cell with the captured Div.cell wrapper; \ + got blocks: {:?}", + out.blocks.iter().map(block_kind).collect::>() + ); + let remaining_engine_cells = out + .blocks + .iter() + .filter(|b| engine_cell_lang(b).is_some()) + .count(); + assert_eq!( + remaining_engine_cells, 0, + "no raw engine cell may remain after the splice" + ); +} + +fn block_kind(b: &Block) -> &'static str { + match b { + Block::Paragraph(_) => "Paragraph", + Block::Plain(_) => "Plain", + Block::CodeBlock(_) => "CodeBlock", + Block::Div(_) => "Div", + Block::RawBlock(_) => "RawBlock", + Block::Header(_) => "Header", + _ => "Other", + } +} + +#[test] +fn jupyter_capture_splices_into_preview_ast() { + assert_capture_splices( + "---\ntitle: Splice demo\nengine: jupyter\n---\n\nSome prose.\n\n```{python}\n2 + 3\n```\n", + "jupyter", + ); +} + +#[test] +fn knitr_capture_splices_into_preview_ast() { + assert_capture_splices( + "---\ntitle: Splice demo\nengine: knitr\n---\n\nSome prose.\n\n```{r}\n1 + 1\n```\n", + "knitr", + ); +} diff --git a/crates/quarto-core/tests/integration/engine_output_parity.rs b/crates/quarto-core/tests/integration/engine_output_parity.rs new file mode 100644 index 000000000..1c3766578 --- /dev/null +++ b/crates/quarto-core/tests/integration/engine_output_parity.rs @@ -0,0 +1,237 @@ +//! Cross-engine output-parity suite (bd-gthycd33, decision 4). +//! +//! The engine API is text-in/text-out (and will stay that way for +//! future user-extensible engines), so uniformity of the post-engine +//! markdown *structure* cannot be enforced at the API level. These +//! tests enforce it empirically: equivalent minimal inputs run +//! through knitr and jupyter must produce post-engine markdown that +//! parses to the same block structure. Structural divergence between +//! engines is exactly the class of bug bd-gthycd33 was — the preview +//! capture splice, CSS selectors (`.cell .cell-output-* pre code`), +//! and cell-level transforms all key on one shared shape. +//! +//! "Same structure" is deliberately *shape* parity, not byte parity: +//! we compare block kinds recursively plus the semantic classes the +//! downstream consumers rely on (`cell`, `cell-code`, `cell-output`), +//! ignoring content bytes, language classes (`r` vs `python`), and +//! output-subtype classes. The subtype tolerance is intentional and +//! reviewed: the same logical "expression evaluates to a value" is +//! `.cell-output-stdout` under knitr (R autoprints to stdout) but +//! `.cell-output-display` under jupyter (an execute_result) — a +//! Q1-inherited semantic difference with identical block structure. +//! See claude-notes/plans/2026-07-01-bd-gthycd33-jupyter-cell-wrapper.md. +//! +//! The suite needs BOTH engines installed (R + knitr, jupyter + +//! ipykernel); each test skips with a note when either is missing. + +use std::path::PathBuf; +use std::sync::Arc; + +use quarto_core::engine::EngineRegistry; +use quarto_core::engine::preview_record::record_capture; +use quarto_core::project::ProjectContext; +use quarto_pandoc_types::{Block, Pandoc}; +use quarto_system_runtime::{NativeRuntime, SystemRuntime}; + +fn both_engines_available() -> bool { + let registry = EngineRegistry::default(); + ["knitr", "jupyter"] + .iter() + .all(|name| registry.get(name).is_some_and(|e| e.is_available())) +} + +fn fixture( + content: &str, +) -> ( + tempfile::TempDir, + PathBuf, + ProjectContext, + Arc, +) { + let dir = tempfile::tempdir().unwrap(); + let qmd_path = dir.path().join("doc.qmd"); + std::fs::write(&qmd_path, content).unwrap(); + let runtime: Arc = Arc::new(NativeRuntime::new()); + let project = ProjectContext::discover(&qmd_path, runtime.as_ref()).unwrap(); + (dir, qmd_path, project, runtime) +} + +/// Run the real engine over `content` (via `record_capture`, the same +/// producer path `q2 preview` / `q2 provide-hub` use — see +/// capture_splice_engines.rs for why pollster and not #[tokio::test]) +/// and parse the capture's post-engine markdown. +fn post_engine_ast(content: &str, engine: &str) -> Pandoc { + let (_tmp, path, project, runtime) = fixture(content); + let captures = + pollster::block_on(record_capture(&path, &project, runtime, None)).expect("record_capture"); + assert_eq!(captures.len(), 1, "expected one capture for {engine}"); + let capture = &captures[0]; + assert_eq!(capture.engine_name, engine); + let result_markdown = capture + .result + .get("markdown") + .and_then(|v| v.as_str()) + .unwrap_or(""); + let (pandoc, _, _) = pampa::readers::qmd::read( + result_markdown.as_bytes(), + false, + "capture-result.md", + &mut std::io::sink(), + false, + None, + ) + .expect("parse post-engine markdown"); + pandoc +} + +/// The classes downstream consumers key on. Language classes and +/// output-subtype classes are deliberately not compared (see module +/// docs). +fn semantic_classes(classes: &[String]) -> Vec { + classes + .iter() + .filter(|c| matches!(c.as_str(), "cell" | "cell-code" | "cell-output")) + .cloned() + .collect() +} + +fn block_kind(b: &Block) -> &'static str { + match b { + Block::Paragraph(_) => "Paragraph", + Block::Plain(_) => "Plain", + Block::CodeBlock(_) => "CodeBlock", + Block::Div(_) => "Div", + Block::RawBlock(_) => "RawBlock", + Block::Header(_) => "Header", + _ => "Other", + } +} + +/// Recursively assert the two block sequences have the same shape: +/// same length, same kinds, same semantic classes on Divs and +/// CodeBlocks, recursing into Div content. `path` locates a mismatch +/// in the failure message. +fn assert_blocks_parity(knitr: &[Block], jupyter: &[Block], path: &str) { + assert_eq!( + knitr.len(), + jupyter.len(), + "block count mismatch at '{path}': knitr {:?} vs jupyter {:?}", + knitr.iter().map(block_kind).collect::>(), + jupyter.iter().map(block_kind).collect::>() + ); + for (i, (k, j)) in knitr.iter().zip(jupyter.iter()).enumerate() { + let p = format!("{path}[{i}]"); + match (k, j) { + (Block::Div(dk), Block::Div(dj)) => { + assert_eq!( + semantic_classes(&dk.attr.1), + semantic_classes(&dj.attr.1), + "Div semantic classes differ at '{p}' (knitr {:?} vs jupyter {:?})", + dk.attr.1, + dj.attr.1 + ); + assert_blocks_parity(&dk.content, &dj.content, &p); + } + (Block::CodeBlock(ck), Block::CodeBlock(cj)) => { + assert_eq!( + semantic_classes(&ck.attr.1), + semantic_classes(&cj.attr.1), + "CodeBlock semantic classes differ at '{p}' (knitr {:?} vs jupyter {:?})", + ck.attr.1, + cj.attr.1 + ); + } + (a, b) => { + assert_eq!( + std::mem::discriminant(a), + std::mem::discriminant(b), + "block kind mismatch at '{p}': knitr={} jupyter={}", + block_kind(a), + block_kind(b) + ); + } + } + } +} + +/// Equivalent single-cell documents for the two engines. Identical +/// prose so the whole-document shape comparison is meaningful; only +/// the cell language and body differ. +fn knitr_doc(cell_body: &str) -> String { + format!( + "---\ntitle: Parity\nengine: knitr\n---\n\nBefore.\n\n```{{r}}\n{cell_body}\n```\n\nAfter.\n" + ) +} + +fn jupyter_doc(cell_body: &str) -> String { + format!( + "---\ntitle: Parity\nengine: jupyter\n---\n\nBefore.\n\n```{{python}}\n{cell_body}\n```\n\nAfter.\n" + ) +} + +fn assert_engine_parity(knitr_cell: &str, jupyter_cell: &str) { + if !both_engines_available() { + eprintln!("Skipping test: parity suite needs both knitr and jupyter installed"); + return; + } + let k = post_engine_ast(&knitr_doc(knitr_cell), "knitr"); + let j = post_engine_ast(&jupyter_doc(jupyter_cell), "jupyter"); + assert_blocks_parity(&k.blocks, &j.blocks, "blocks"); +} + +#[test] +fn parity_stream_output() { + assert_engine_parity("cat(\"hi\\n\")", "print(\"hi\")"); +} + +#[test] +fn parity_expression_value() { + assert_engine_parity("1 + 1", "2 + 3"); +} + +/// Error *output shape* parity, under the sanctioned "show errors in +/// the output" mode (`#| error: true`). Without it, knitr fails the +/// whole pipeline on a cell error (Q1's default `error: false` +/// policy) and produces no capture at all, so there is no shape to +/// compare — while q2's jupyter currently embeds the error and +/// succeeds regardless. That *policy* divergence is a separate bug +/// (jupyter should fail the render on cell error unless +/// `error: true`), tracked as a follow-up strand; see the plan. +#[test] +fn parity_error_output() { + assert_engine_parity( + "#| error: true\nstop(\"boom\")", + "#| error: true\nraise Exception(\"boom\")", + ); +} + +#[test] +fn parity_source_only_cell() { + assert_engine_parity("x <- 1", "x = 1"); +} + +/// Two cells where the second depends on state from the first — pins +/// both kernel-state persistence within a document (through the +/// production text path) and the multi-cell output shape. +#[test] +fn parity_dependent_cells() { + if !both_engines_available() { + eprintln!("Skipping test: parity suite needs both knitr and jupyter installed"); + return; + } + let knitr = post_engine_ast( + &two_cell_doc("knitr", "r", "x <- 40", "cat(x + 2, \"\\n\")"), + "knitr", + ); + let jupyter = post_engine_ast( + &two_cell_doc("jupyter", "python", "x = 40", "print(x + 2)"), + "jupyter", + ); + assert_blocks_parity(&knitr.blocks, &jupyter.blocks, "blocks"); +} + +fn two_cell_doc(engine: &str, lang: &str, cell1: &str, cell2: &str) -> String { + format!( + "---\ntitle: Parity\nengine: {engine}\n---\n\nBefore.\n\n```{{{lang}}}\n{cell1}\n```\n\nBetween.\n\n```{{{lang}}}\n{cell2}\n```\n\nAfter.\n" + ) +} diff --git a/crates/quarto-core/tests/integration/jupyter_integration.rs b/crates/quarto-core/tests/integration/jupyter_integration.rs index 926089a36..f4f40c321 100644 --- a/crates/quarto-core/tests/integration/jupyter_integration.rs +++ b/crates/quarto-core/tests/integration/jupyter_integration.rs @@ -196,20 +196,23 @@ async fn test_kernel_execute_error() { } // ============================================================================= -// AST Transform Integration Tests +// Shared fixtures for the pipeline tests below // ============================================================================= +// +// Note (bd-gthycd33): the AST-path transform tests that used to live +// here were retired together with `JupyterTransform` / +// `outputs_to_blocks` — production jupyter execution goes through the +// text path (`ExecutionEngine::execute` -> text_execute.rs), and the +// parallel AST emitter was a latent shape-divergence source with no +// production consumer. Kernel-state persistence is covered by +// `test_full_pipeline_multiple_cells` below and the cross-engine +// `engine_output_parity` suite; inline `{python} expr` evaluation +// (which only the retired prototype supported) is tracked as a +// follow-up strand. -use quarto_core::engine::jupyter::JupyterTransform; use quarto_core::format::Format; use quarto_core::project::{DocumentInfo, ProjectConfig, ProjectContext}; use quarto_core::render::{BinaryDependencies, RenderContext}; -use quarto_core::transform::AstTransform; -use quarto_pandoc_types::ConfigValue; -use quarto_pandoc_types::attr::AttrSourceInfo; -use quarto_pandoc_types::block::{Block, CodeBlock, Paragraph}; -use quarto_pandoc_types::inline::{Inline, Str}; -use quarto_pandoc_types::pandoc::Pandoc; -use quarto_source_map::SourceInfo; fn make_test_project() -> ProjectContext { ProjectContext { @@ -223,220 +226,6 @@ fn make_test_project() -> ProjectContext { } } -fn make_python_code_block(code: &str) -> Block { - Block::CodeBlock(CodeBlock { - attr: ( - String::new(), - vec!["{python}".to_string()], - Default::default(), - ), - text: code.to_string(), - source_info: SourceInfo::for_test(), - attr_source: AttrSourceInfo::empty(), - }) -} - -fn make_paragraph(text: &str) -> Block { - Block::Paragraph(Paragraph { - content: vec![Inline::Str(Str { - text: text.to_string(), - source_info: SourceInfo::for_test(), - })], - source_info: SourceInfo::for_test(), - }) -} - -/// Test that JupyterTransform executes code and transforms the AST. -#[tokio::test(flavor = "multi_thread")] -#[ignore = "requires ipykernel"] -async fn test_jupyter_transform_print() { - if !python_kernel_available().await { - eprintln!("Python kernel not available, skipping test"); - return; - } - - // Create an AST with a Python code block - let mut ast = Pandoc { - meta: ConfigValue::new_map(vec![], SourceInfo::for_test()), - blocks: vec![ - make_paragraph("Introduction"), - make_python_code_block("print('Hello from transform!')"), - make_paragraph("Conclusion"), - ], - }; - - // Set up context with execution enabled - let project = make_test_project(); - let doc = DocumentInfo::from_path(std::env::current_dir().unwrap().join("test.qmd")); - let format = Format::html(); - let binaries = BinaryDependencies::new(); - let mut ctx = RenderContext::new(&project, &doc, &format, &binaries); - ctx.options.execute = true; - - // Run the transform - let transform = JupyterTransform::new(); - transform - .transform(&mut ast, &mut ctx) - .await - .expect("Transform failed"); - - // The code block should be replaced with output - // We should still have 3 blocks: intro, output, conclusion - // (or possibly more if the output produces multiple blocks) - assert!( - ast.blocks.len() >= 2, - "Expected at least 2 blocks, got {}", - ast.blocks.len() - ); - - // First block should still be the intro paragraph - assert!( - matches!(&ast.blocks[0], Block::Paragraph(_)), - "First block should be paragraph" - ); - - // The code block should have been replaced - verify it's not a CodeBlock with {python} anymore - let _has_python_codeblock = ast.blocks.iter().any(|b| { - if let Block::CodeBlock(cb) = b { - cb.attr.1.iter().any(|c| c.contains("python")) - } else { - false - } - }); - - // The code block may still exist if we're preserving it with output - // For now, just verify the transform ran without error - println!("Transform completed. Block count: {}", ast.blocks.len()); - for (i, block) in ast.blocks.iter().enumerate() { - match block { - Block::Paragraph(_) => println!(" {}: Paragraph", i), - Block::CodeBlock(cb) => println!(" {}: CodeBlock (classes: {:?})", i, cb.attr.1), - Block::Div(_) => println!(" {}: Div", i), - Block::RawBlock(rb) => println!(" {}: RawBlock ({})", i, rb.format), - _ => println!(" {}: Other block type", i), - } - } -} - -/// Test that JupyterTransform handles expression output. -#[tokio::test(flavor = "multi_thread")] -#[ignore = "requires ipykernel"] -async fn test_jupyter_transform_expression() { - if !python_kernel_available().await { - eprintln!("Python kernel not available, skipping test"); - return; - } - - let mut ast = Pandoc { - meta: ConfigValue::new_map(vec![], SourceInfo::for_test()), - blocks: vec![make_python_code_block("1 + 1")], - }; - - let project = make_test_project(); - let doc = DocumentInfo::from_path(std::env::current_dir().unwrap().join("test.qmd")); - let format = Format::html(); - let binaries = BinaryDependencies::new(); - let mut ctx = RenderContext::new(&project, &doc, &format, &binaries); - ctx.options.execute = true; - - let transform = JupyterTransform::new(); - transform - .transform(&mut ast, &mut ctx) - .await - .expect("Transform failed"); - - // Verify we got some output - assert!(!ast.blocks.is_empty(), "Expected output blocks"); - - println!("Expression transform completed. Blocks:"); - for (i, block) in ast.blocks.iter().enumerate() { - match block { - Block::CodeBlock(cb) => { - println!( - " {}: CodeBlock text='{}'", - i, - cb.text.chars().take(50).collect::() - ); - } - _ => println!(" {}: {:?}", i, std::mem::discriminant(block)), - } - } -} - -/// Test that daemon persists kernel across multiple transforms. -#[tokio::test(flavor = "multi_thread")] -#[ignore = "requires ipykernel"] -async fn test_daemon_persistence() { - if !python_kernel_available().await { - eprintln!("Python kernel not available, skipping test"); - return; - } - - // Create first AST - set a variable - let mut ast1 = Pandoc { - meta: ConfigValue::new_map(vec![], SourceInfo::for_test()), - blocks: vec![make_python_code_block("test_var = 42")], - }; - - let project = make_test_project(); - let doc = DocumentInfo::from_path(std::env::current_dir().unwrap().join("test.qmd")); - let format = Format::html(); - let binaries = BinaryDependencies::new(); - let mut ctx1 = RenderContext::new(&project, &doc, &format, &binaries); - ctx1.options.execute = true; - - // Run first transform - creates kernel and sets variable - let transform = JupyterTransform::new(); - transform - .transform(&mut ast1, &mut ctx1) - .await - .expect("First transform failed"); - - // Create second AST - read the variable - let mut ast2 = Pandoc { - meta: ConfigValue::new_map(vec![], SourceInfo::for_test()), - blocks: vec![make_python_code_block("print(test_var)")], - }; - - let mut ctx2 = RenderContext::new(&project, &doc, &format, &binaries); - ctx2.options.execute = true; - - // Run second transform - should reuse kernel and see the variable - transform - .transform(&mut ast2, &mut ctx2) - .await - .expect("Second transform failed"); - - // Verify the second transform got the variable value - // Look for stdout containing "42" - let has_output_42 = ast2.blocks.iter().any(|b| { - if let Block::CodeBlock(cb) = b { - cb.text.contains("42") - } else { - false - } - }); - - println!("Second transform blocks:"); - for (i, block) in ast2.blocks.iter().enumerate() { - match block { - Block::CodeBlock(cb) => { - println!( - " {}: CodeBlock text='{}'", - i, - cb.text.chars().take(100).collect::() - ); - } - _ => println!(" {}: {:?}", i, std::mem::discriminant(block)), - } - } - - assert!( - has_output_42, - "Expected output containing '42' from persisted kernel state" - ); -} - /// Test that matplotlib figures produce display_data outputs. #[tokio::test] #[ignore = "requires ipykernel and matplotlib"] @@ -497,107 +286,6 @@ plt.show() .expect("Shutdown failed"); } -// ============================================================================= -// Inline Expression Tests -// ============================================================================= - -use quarto_pandoc_types::inline::Code; - -/// Helper to create a paragraph with an inline expression. -fn make_paragraph_with_inline_expr(prefix: &str, expr: &str, suffix: &str) -> Block { - Block::Paragraph(Paragraph { - content: vec![ - Inline::Str(Str { - text: prefix.to_string(), - source_info: SourceInfo::for_test(), - }), - Inline::Code(Code { - attr: (String::new(), vec![], Default::default()), - text: expr.to_string(), - source_info: SourceInfo::for_test(), - attr_source: AttrSourceInfo::empty(), - }), - Inline::Str(Str { - text: suffix.to_string(), - source_info: SourceInfo::for_test(), - }), - ], - source_info: SourceInfo::for_test(), - }) -} - -/// Test that inline expressions like `{python} 1+1` are evaluated. -#[tokio::test(flavor = "multi_thread")] -#[ignore = "requires ipykernel"] -async fn test_jupyter_transform_inline_expression() { - if !python_kernel_available().await { - eprintln!("Python kernel not available, skipping test"); - return; - } - - // Create an AST with an inline expression - // The text is: "The answer is `{python} 2+2`!" - let mut ast = Pandoc { - meta: ConfigValue::new_map(vec![], SourceInfo::for_test()), - blocks: vec![make_paragraph_with_inline_expr( - "The answer is ", - "{python} 2+2", - "!", - )], - }; - - let project = make_test_project(); - let doc = DocumentInfo::from_path(std::env::current_dir().unwrap().join("test.qmd")); - let format = Format::html(); - let binaries = BinaryDependencies::new(); - let mut ctx = RenderContext::new(&project, &doc, &format, &binaries); - ctx.options.execute = true; - - let transform = JupyterTransform::new(); - transform - .transform(&mut ast, &mut ctx) - .await - .expect("Transform failed"); - - // The paragraph should now contain the result "4" instead of the Code inline - println!("Inline expression transform completed. Blocks:"); - for (i, block) in ast.blocks.iter().enumerate() { - match block { - Block::Paragraph(para) => { - println!(" {}: Paragraph with {} inlines:", i, para.content.len()); - for (j, inline) in para.content.iter().enumerate() { - match inline { - Inline::Str(s) => println!(" {}: Str('{}')", j, s.text), - Inline::Code(c) => println!(" {}: Code('{}')", j, c.text), - _ => println!(" {}: {:?}", j, std::mem::discriminant(inline)), - } - } - } - _ => println!(" {}: {:?}", i, std::mem::discriminant(block)), - } - } - - // Verify the inline expression was replaced with result - let has_result_4 = ast.blocks.iter().any(|b| { - if let Block::Paragraph(para) = b { - para.content.iter().any(|inline| { - if let Inline::Str(s) = inline { - s.text.contains('4') - } else { - false - } - }) - } else { - false - } - }); - - assert!( - has_result_4, - "Expected paragraph to contain '4' from inline expression evaluation" - ); -} - // ============================================================================= // Full Pipeline Integration Tests // ============================================================================= diff --git a/crates/quarto-core/tests/integration/main.rs b/crates/quarto-core/tests/integration/main.rs index e8ba0de3d..d2b4c8bba 100644 --- a/crates/quarto-core/tests/integration/main.rs +++ b/crates/quarto-core/tests/integration/main.rs @@ -13,9 +13,11 @@ pub mod attribution_viewer; pub mod attribution_wasm_invariant; pub mod bootstrap_js_pipeline; pub mod brand_render; +pub mod capture_splice_engines; pub mod crossref_fixtures; pub mod document_profile_pipeline; pub mod engine_merge; +pub mod engine_output_parity; pub mod fail_fast; pub mod get_config_merge; pub mod idempotence;