diff --git a/CLAUDE.md b/CLAUDE.md index 5cb81d332..712968331 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -366,6 +366,10 @@ The render pipeline has a **profile checkpoint** between `MetadataMergeStage` an Q2 emits HTML **directly from the Pandoc AST** and has **no post-Pandoc DOM-mutation stage**. Quarto 1, by contrast, leans heavily on DOM postprocessors (functions that parse the rendered HTML into a `Document` and mutate it — e.g. reveal's `applyStretch`, which hoists a stretched `` to be a direct child of its `
`). **When porting such a feature, re-express it as an AST transform** (operate on `Block`/`Inline` so the writer emits the right HTML the first time) rather than adding a DOM postprocessor. A DOM-postprocessor stage is a large new architectural surface with no current consumer; do not introduce one without an extremely strong, explicitly-discussed reason. Worked example: revealjs auto-stretch unwraps a `Paragraph[Image]` into a `Plain[Image]` in `crates/quarto-core/src/revealjs/auto_stretch.rs` to get `section > img.r-stretch`, instead of mutating the DOM after the fact (bd-zkstclhl). +### Transform pipeline phases — format-agnostic semantics before format-specific presentation + +The single transform pipeline (`build_transform_pipeline` in `crates/quarto-core/src/pipeline.rs`) runs every output format, splicing format-specific transforms in at chosen positions. It must visit transforms in **non-decreasing phase rank**: `Normalization` → `Crossref` → `Navigation` → `Finalization`. The hard rule: **a format-specific *presentation* transform that consumes semantic structure (a float, caption, number, or resolved `@ref`) must run in `Finalization`, after `crossref-render` — never earlier.** Each `AstTransform` declares its phase via `fn phase(&self) -> TransformPhase` (defaults to `Unclassified`; every pipeline member overrides it). The invariant is enforced by `test_build_transform_pipeline_phase_ordering`, which is format-neutral (loops over format strings, no `is_revealjs` branch). When adding a new format-specific late transform, add it via a named seam like `reveal_finalization_transforms`/`footer_render_stage`, not an inline `if is_revealjs { … }` at an arbitrary position. This is why a revealjs presentation transform (auto-stretch) once silently broke crossrefs by running before they were numbered (bd-w0c6d38k). Full contract + author rule + the preview-pipeline shape contract: `claude-notes/designs/transform-pipeline-phases.md`. + ## hub-client Commit Instructions **IMPORTANT**: When making commits that include changes to `hub-client/`, you MUST also update `hub-client/changelog.md`. diff --git a/claude-notes/designs/transform-pipeline-phases.md b/claude-notes/designs/transform-pipeline-phases.md new file mode 100644 index 000000000..346534b6f --- /dev/null +++ b/claude-notes/designs/transform-pipeline-phases.md @@ -0,0 +1,203 @@ +# Transform pipeline phases & the format-agnostic-first invariant + +**Status:** Implemented under bd-w0c6d38k (2026-06-18). `TransformPhase` and the +defaulted `phase()` method live on `AstTransform` (`crates/quarto-core/src/transform.rs`); +all render-pipeline transforms are classified; the invariant is enforced by +`test_build_transform_pipeline_phase_ordering` in `pipeline.rs`. The first +consumer of the seam is the revealjs auto-stretch move (auto-stretch now runs in +`Finalization`, after `crossref-render`). + +**Audience:** anyone adding or reordering an `AstTransform` in +`build_transform_pipeline` (`crates/quarto-core/src/pipeline.rs`), and especially +anyone adding a new output format (`dashboard`, `typst`, `pdf`, …) with its own +format-specific transforms. + +--- + +## Why this exists + +Q2 renders every output format through **one** transform pipeline +(`build_transform_pipeline`, `pipeline.rs:1078`), with format-specific transforms +spliced in at chosen positions (today via inline `if is_revealjs { … }`). The +pipeline mixes two fundamentally different kinds of work: + +1. **Format-agnostic semantic normalization** — establishing *what the document + means*: callouts, theorems, proofs, floats (Figure/Table), equations, and the + crossref index/resolve that numbers them and rewrites `@refs`. +2. **Format-specific presentation** — reshaping the resolved document for *one* + output: revealjs slide construction, single-image auto-stretch, etc. + +These have a hard dependency direction: **presentation depends on semantics, not +the other way around.** A revealjs transform that hoists a `Figure` to a bare +`` is only correct *after* that `Figure` has been recognized, numbered, and +resolved as a crossref float. + +When that direction is violated, the failure is silent and format-specific. The +motivating incident (bd-w0c6d38k): `RevealAutoStretchTransform` (`pipeline.rs:1148`) +ran *before* `FloatRefTargetSugarTransform` (`:1166`) and the crossref phase +(`:1170`). On a single-image slide it hoisted `![cap](x){#fig-1}` (a Pandoc +`Figure`) into a bare `Plain[Image]` before the float was ever registered — so +`@fig-1` rendered unresolved and a sibling figure was mis-numbered. Every test +passed; only `format: revealjs` was affected, and only for that figure syntax. + +--- + +## The phase model + +Every transform belongs to exactly one phase. Phases are **totally ordered**; +their order is the contract. + +| rank | `TransformPhase` | purpose | examples | +|------|------------------|---------|----------| +| 1 | `Normalization` | format-agnostic semantic sugar **and** format-specific *structural scaffolding* that does not consume crossref semantics | `callout`, `theorem-sugar`, `proof-sugar`, `float-ref-target-sugar`, `equation-label`, `metadata-normalize`, `footnotes`, `example-embed`; **`reveal-columns`, `reveal-slides`, `reveal-footnotes`, `reveal-footer-alias`** | +| 2 | `Crossref` | assign section-scoped numbers; rewrite `@refs` to custom nodes | `crossref-index`, `crossref-resolve` | +| 3 | `Navigation` | TOC / navbar / sidebar / page-nav / footer / listings generate + render | `toc-*`, `navbar-*`, `sidebar-*`, `footer-generate`, `listing-*` | +| 4 | `Finalization` | render custom nodes to writer-visible shapes, then format presentation that *consumes* those shapes, then resource/attribution baking | `link-rewrite`, `appendix-structure`, **`crossref-render`**, **`reveal-auto-stretch`** (after the move), `code-block-render`, `resource-collector`, `attribution-render` | + +Two subtleties this model encodes: + +- **Crossref spans two phases.** Index + resolve happen in `Crossref` (rank 2); + the conversion of crossref custom nodes into real `Figure`/`Div`/`Span`/`Link` + shapes happens in `crossref-render` (`pipeline.rs:1259`), which is `Finalization` + (rank 4). This is *why* a presentation transform that needs the final `Figure` + (auto-stretch) belongs in `Finalization`, after `crossref-render` — not in + `Crossref`. +- **Not all format-specific transforms are late.** `reveal-slides`/`reveal-columns` + build the `
` scaffolding from raw blocks and metadata; they do **not** + read or mutate floats/captions/numbers, so they correctly live in + `Normalization`. The distinction is *what a transform consumes*, not merely that + it is format-specific (see the author rule below). + +--- + +## The invariant + +> **Format-agnostic semantic structure is fully established before any transform +> that consumes it.** Concretely, in the built pipeline the transforms' phase +> ranks are **non-decreasing by position** (a transform never precedes one of a +> lower rank). + +Because `crossref-render` is `Finalization` and a presentation transform that +hoists a crossref float is also `Finalization`, the rank order alone forbids the +bug: a `Finalization` transform cannot sit among `Normalization` transforms ahead +of the `Crossref` phase. + +### Author rule (which phase do I pick?) + +When adding a format-specific transform, ask: **does it read or mutate a structure +produced by the Crossref phase — a float, a caption, a number, or a resolved +`@ref`?** + +- **Yes** → phase `Finalization`, and it must run *after* `crossref-render`. (This + is auto-stretch.) +- **No, it only builds format scaffolding from raw blocks/metadata** → phase + `Normalization` is acceptable. (This is `reveal-slides`.) + +If unsure, choose `Finalization`. Running a presentation transform too late is at +worst a missed optimization; running it too early silently corrupts semantics. + +--- + +## Enforcement + +### `phase()` on the `AstTransform` trait + +`AstTransform` (`crates/quarto-core/src/transform.rs:69`) gains a method: + +```rust +fn phase(&self) -> TransformPhase { TransformPhase::Unclassified } +``` + +It **defaults to `Unclassified`** rather than being required-with-no-default. The +reason is honesty: a handful of `AstTransform` impls are *not* part of the ordered +multi-format render pipeline — engine-stage transforms (`JupyterTransform`), the +stage-level `AttributionGenerateTransform`, and in-module test doubles. Forcing +them to fabricate a render-pipeline phase would be a lie. Instead, `Unclassified` +is the honest default for "not a member of the ordered pipeline," and **every +transform that actually appears in `build_transform_pipeline` overrides it** with +a real phase. The anti-drift guarantee is preserved by the test, not the compiler: +the ordering test **fails if any transform in the built pipeline reports +`Unclassified`** (exhaustiveness), so a new pipeline transform that forgets to +classify itself is caught loudly. This mirrors the existing deny-list-name +validator (`pipeline.rs:1344`), which is likewise a test, not a compiler check. + +Cost: one extra vtable slot per transform *type* (a fn pointer in static rodata); +**zero per-instance memory and zero render-time cost** — `phase()` is called only +by the ordering test, never on the render hot path. (`is_format_specific()` may be +added later as a sibling method if a real consumer appears; the ordering invariant +needs only `phase()`.) + +`TransformPhase` derives `PartialOrd`/`Ord` from declaration order; `Unclassified` +is declared **last** so a leaked value sorts high, though the exhaustiveness check +rejects it before the monotonicity comparison runs. + +### The ordering-invariant test + +A test over the **real** `build_transform_pipeline` (the analogue of the existing +*analysis*-pipeline test, `pipeline.rs:1851`) that: + +1. builds the pipeline for **each supported format string** (`html`, `revealjs`, + and `dashboard`/`typst`/`pdf` as they land — a list, not an `is_revealjs` + branch, so new formats are covered without new assertions); +2. reads each transform's `phase()`; +3. asserts ranks are **non-decreasing by position**; +4. on failure, names the offending transform and its neighbours. + +This test **fails on today's code** (auto-stretch is `Finalization`-rank sitting +before the `Crossref` phase) and **passes after auto-stretch moves** past +`crossref-render` — so it is simultaneously the red/green TDD test for the +structural fix and the permanent guardrail. It is format-neutral by construction: +it never mentions `revealjs`, only phases. + +--- + +## The preview-pipeline shape contract (the anti-recurrence rule) + +The bug had a deeper enabler worth stating as its own rule. + +The preview pipeline (`build_q2_preview_transform_pipeline`, `pipeline.rs:1387`) is +**the render pipeline minus a deny-list** (`Q2_PREVIEW_TRANSFORM_EXCLUDED`, +`:1348`), which removes `"crossref-render"` (`:1374`) so crossref custom nodes +survive for React. That subtraction means a crossref float has **different +concrete AST shapes in the two pipelines** after the crossref phase: a real +`Figure` in native render, a `CustomNode("FloatRefTarget")` in preview. Faced with +that divergence, the original author placed auto-stretch *before* the sugar +transforms — the only point where both pipelines still show a plain `Figure` — and +accepted not stretching crossref figures. That early placement is what created the +ordering inversion. + +**Rule:** *removing (or adding) a transform in the preview deny-list must not +create a post-crossref AST-shape divergence that forces a presentation transform +to run earlier in the render pipeline.* If preview and render must differ in +post-crossref shape, the presentation transform must handle **both** shapes (e.g. +recognize the custom-node form), or the preview must be brought back onto the +common shape — never the reverse, where the render pipeline contorts to match +preview's reduced shape. Tracking the preview's own crossref story is bd-zecehtnc. + +--- + +## Worked example: the auto-stretch inversion + +| | before (buggy) | after (fixed) | +|---|---|---| +| `reveal-auto-stretch` declared phase | `Finalization` | `Finalization` | +| its position | `pipeline.rs:1148`, **before** `Crossref` (`:1170`) | after `crossref-render` (`:1259`) | +| `![cap](x){#fig-1}` at that point | a plain `Figure` → hoisted to bare ``; never registered as a float | a fully-rendered `Figure(id=fig-1)` with caption "Figure 1: …" → hoisted to `section > img.r-stretch` + `

` | +| ordering test | **fails** (rank 4 before rank 2) | **passes** (ranks non-decreasing) | +| `@fig-1` in output | unresolved `?fig-1?` | `Figure 1` | + +--- + +## References + +- Pipeline: `crates/quarto-core/src/pipeline.rs` — `build_transform_pipeline` + (`:1078`), phase headers (`NORMALIZATION` `:1090`, `CROSSREF` `:1169`, + `NAVIGATION` `:1173`, `FINALIZATION` `:1249`), `crossref-render` (`:1259`), + `Q2_PREVIEW_TRANSFORM_EXCLUDED` (`:1348`), preview builder (`:1387`), existing + analysis-pipeline ordering test (`:1851`), `footer_render_stage` format seam + (`:1070`). +- Trait: `crates/quarto-core/src/transform.rs:69` (`AstTransform`). +- Auto-stretch transform + its ordering rationale: `crates/quarto-core/src/revealjs/auto_stretch.rs`. +- Plan: `claude-notes/plans/2026-06-18-revealjs-crossref.md` (bd-w0c6d38k); + follow-ups bd-4ly7ne01 (bare-table desugar), bd-zecehtnc (preview crossref). +- Crossref design epic: `claude-notes/plans/2026-04-15-crossref-design.md` (bd-jsbg). diff --git a/claude-notes/plans/2026-06-18-revealjs-crossref.md b/claude-notes/plans/2026-06-18-revealjs-crossref.md new file mode 100644 index 000000000..a83de6978 --- /dev/null +++ b/claude-notes/plans/2026-06-18-revealjs-crossref.md @@ -0,0 +1,490 @@ +# Cross-references in `format: revealjs` + +**Braid strand:** bd-w0c6d38k (related: bd-jsbg crossref epic, bd-zkstclhl reveal auto-stretch) +**Sub-strands:** bd-4ly7ne01 (Bug B: bare-table desugar, format-agnostic), +bd-zecehtnc (WASM/interactive-preview crossref support: `q2 preview` + hub-client) +**Created:** 2026-06-18 +**Status:** Design — decisions taken 2026-06-18 (see "Decisions"), iterating before implementation + +--- + +## Overview + +Audit and fix cross-reference **computation, resolution, and rendering** under +`format: revealjs`, so that all three crossref categories work: + +- **floats** — Figure, Table +- **blocks** — Theorem, Lemma, … (and Proof-likes) +- **inlines** — Equation + +The trigger was `examples/presentations/14-crossrefs/slides.qmd`, where `@fig-1` +renders unresolved (`?fig-1?`) and `@fig-2` is mis-numbered. + +This is partly a *pipeline-ordering / format-composition* problem: a +revealjs-only AST transform (`RevealAutoStretchTransform`) runs **before** the +crossref phase and destroys crossref float targets. The footer-rendering work +(`footer_render_stage`, bd-n2w0sxgd) is the precedent for reasoning about which +transforms run under which format and in what order. + +--- + +## Reproduction (observed 2026-06-18) + +### A. revealjs is broken; HTML is fine + +`cargo run --bin q2 -- render examples/presentations/14-crossrefs/slides.qmd` +emits `Warning: unresolved crossref @fig-1` and produces: + +```html + +See ?fig-1? and + Figure 1. + +A figure + +

…

Figure 1: Another figure.

+``` + +The **identical content rendered as `format: html`** resolves correctly: +`Figure 1` / `Figure 2`, both `
`, caption `Figure 2: …`. So the +crossref machinery is correct; the revealjs path breaks it. + +### B. Per-category audit in revealjs + +Test file `/tmp/xref-test/all.qmd` (one of each category): + +| ref | form | revealjs result | HTML result | verdict | +|-----|------|-----------------|-------------|---------| +| `@fig-1` | `![cap](x){#fig-1}` (implicit Figure) | **unresolved `?fig-1?`** | `Figure 1` ✓ | **revealjs bug (A)** | +| `@fig-plot` | `::: {#fig-plot}` (div) | `Figure 1` ✓ | `Figure 1` ✓ | works | +| `@tbl-data` | `: cap {#tbl-data}` (pipe-table caption) | **unresolved `?tbl-data?`** | **unresolved `?tbl-data?`** | **general gap (B)** | +| `@thm-main` | `::: {#thm-main}` | `Theorem 1` ✓ | `Theorem 1` ✓ | works | +| `@eq-key` | `$$…$$ {#eq-key}` | `Equation 1` ✓ | `Equation 1` ✓ | works | + +So in revealjs today: **blocks (theorem) and inlines (equation) already work**; +**div-form figure floats work**; the breakage is (A) attribute-form figures and +(B) bare-table floats. (B) is not revealjs-specific. + +### C. Div-form floats are the robust canonical path (verified both formats) + +The `::: {#-…}` div form works correctly in **both HTML and revealjs** +(`/tmp/xref-div/*.qmd`, 2026-06-18): + +| ref | div content | HTML | revealjs | +|-----|-------------|------|----------| +| `@tbl-1` | image-of-table (`![](x)` + caption) | `Table 1`, `
` ✓ | `Table 1`, `
` ✓ | +| `@tbl-2` | real pipe-table + caption | `Table 2` ✓ | `Table 2` ✓ | +| `@fig-1` | `![](x)` + caption | `Figure 1`, `
` ✓ | `Figure 1`, `
` ✓ | + +Notably the single-image `::: {#tbl-1}` slide is rendered as `
` +and is correctly **not** stretched — auto-stretch leaves plain Divs alone (it +only targets top-level `Paragraph[Image]`/`Figure`). This confirms the intended +architecture: **the div/float form is canonical and uniform; the other syntaxes +are sugar that should desugar into (or be classified like) it.** Both bugs are +exactly the sugar forms that fail to reach that canonical path: + +- Bug A: `![cap](x){#fig-1}` *is* classified as a float in HTML, but revealjs + auto-stretch destroys the `Figure` **before** classification. +- Bug B: `: cap {#tbl-data}` (bare `Block::Table`) **never desugars** into the + float form at all. + +--- + +## Root-cause analysis + +### Bug A — auto-stretch runs before the crossref phase (revealjs-specific) + +`build_transform_pipeline` (`crates/quarto-core/src/pipeline.rs`) order: + +``` +NORMALIZATION + … + RevealSlidesTransform (is_revealjs) ~1127 + RevealFootnotesTransform (is_revealjs) ~1143 + RevealAutoStretchTransform (is_revealjs) ~1148 ← TOO EARLY + ExampleEmbedTransform ~1159 + TheoremSugarTransform / ProofSugarTransform ~1164 + FloatRefTargetSugarTransform (Div/Figure → float) ~1166 + EquationLabelTransform ~1167 +CROSSREF PHASE + CrossrefIndexTransform (number floats) 1170 + CrossrefResolveTransform (resolve @refs) 1171 +… +FINALIZATION + CrossrefRenderTransform (CustomNode → Figure) 1259 +``` + +`![A figure](logo.svg){#fig-1}` parses to a Pandoc `Block::Figure` (implicit +figure: a captioned image alone in a paragraph). On a single-image slide, +`RevealAutoStretchTransform` (1148) hits its `Block::Figure` branch and +**hoists** it: replaces the `Figure` with `Plain[Image]` (id transferred onto +the ``) + a caption paragraph — see +`crates/quarto-core/src/revealjs/auto_stretch.rs` `hoist_figure`. + +By the time `FloatRefTargetSugarTransform` (1166) runs, there is **no Figure** +with id `fig-1` left to classify as a float — only a bare `Plain[Image]` that +happens to carry the id. So: + +- `fig-1` is never registered in the crossref index → `@fig-1` unresolved. +- Only `fig-2` (the `:::`-div form, which auto-stretch ignores because at this + point it is still a plain `Block::Div`, not a `Figure`) is counted → it + becomes "Figure 1". + +The id-transfer in `hoist_figure` was a band-aid for the *HTML anchor* (`#fig-1` +still points somewhere) but does nothing for crossref *registration/numbering*, +which needs the float node to survive into the crossref phase. + +`auto_stretch.rs` (lines 50–53) already documents this as a known divergence: +> The *cross-referenceable* figure case (`::: {#fig-…}`) is still un-stretched on +> single-image slides … the crossref→`Figure` conversion runs later and is +> excluded from the preview pipeline … a documented divergence … deferred. + +### Bug B — bare `Block::Table` with caption-id is never a float (general) + +`FloatRefTargetSugarTransform` classifies only `Block::Div` and `Block::Figure` +(`float_ref_target.rs` `classify_div` / `classify_fig`). It handles the +`Div(#tbl-…) > Table` form, but a **bare top-level `Block::Table`** whose caption +carries `{#tbl-data}` (the `: caption {#tbl-data}` pipe-table syntax) is never +classified — it renders as `` with no number and `@tbl-data` +unresolved, in **both HTML and revealjs**. Q1 handles this in +`parsefiguredivs.lua` (wraps caption-bearing tables into FloatRefTargets). + +### The format-composition tension (why we can't just reorder blindly) + +Auto-stretch was deliberately placed *early* so it operates on a shape common to +**both** pipelines: + +- **native render** (`build_transform_pipeline`): includes + `CrossrefRenderTransform` → crossref floats become real `Figure`s by + finalization. +- **preview** (`build_q2_preview_transform_pipeline` = full pipeline minus + `Q2_PREVIEW_TRANSFORM_EXCLUDED`): **excludes `"crossref-render"`** (pipeline.rs + line 1374) so the CustomNodes survive for React's type-specific components. + +Therefore a crossref float has **no single concrete shape** across both +pipelines after the crossref phase: it is a `Figure` in native render but a +`CustomNode("FloatRefTarget")` in preview. That is the real reason the original +author put auto-stretch up front (where both pipelines still see a plain +`Figure`/`Paragraph[Image]`), and accepted not stretching crossref figures. + +Note: `CrossrefIndexTransform` **is** included in preview — only *render* is +excluded — so **numbering is correct in preview**; only the final-Figure DOM +shape is absent there. + +--- + +## Proposed direction (to confirm with user) + +The fix has three parts. Parts 1–2 are mechanical; Part 3 is the design +decision the user flagged. + +### Part 1 — Bug B: desugar caption-bearing bare tables into the canonical float Div (format-agnostic) + +**Tracked separately as bd-4ly7ne01** (format-agnostic; affects HTML too). +A bare `Block::Table` whose caption carries a registered ref-type id (`tbl-…`) +should be **wrapped into a `Div(#tbl-…) > Table`** by a small desugar step, so the +*existing* uniform `FloatRefTargetSugarTransform` `Div > [Table]` arm handles it +— matching the "all syntaxes desugar into divs, then process uniformly" model +rather than adding a special top-level `Table` classification arm. Q1 ref: +`parsefiguredivs.lua`. + +- **Tests first:** qmd fixture in `crossref_fixtures.rs` asserting the index + registers `tbl-data` from `: cap {#tbl-data}`; end-to-end HTML render shows + `Table 1` + resolved `@tbl-data`. + +### Part 2 — Bug A: move revealjs float post-processing after the crossref phase + +Move `RevealAutoStretchTransform` so it runs **after `CrossrefRenderTransform`** +(finalization, after line 1259 / after `ExampleEmbedRenderTransform`). Then for +native render: + +1. `![cap](x){#fig-1}` → `Figure` survives to `FloatRefTargetSugarTransform` → + numbered → resolved → rendered by `CrossrefRenderTransform` to a final + `Figure(id=fig-1)` with caption "Figure 1: A figure". +2. Auto-stretch then hoists *that* Figure to `section > img.r-stretch` + a + `

Figure 1: A figure

`, exactly as it already does for + plain captioned figures. + +Side benefit: the `::: {#fig-…}` div form *also* becomes a real `Figure` by then, +so auto-stretch can finally stretch div-form crossref figures too — closing the +"still un-stretched" divergence noted in `auto_stretch.rs` for native render. + +Ordering constraints to preserve: auto-stretch must still run **after** +`RevealFootnotesTransform` (coalesced aside ⇒ >1 block ⇒ skip) — satisfied, since +that is in NORMALIZATION, far earlier. The reveal `
` tree +(`RevealSlidesTransform`) already exists by finalization. + +- **Tests first:** the existing `auto_stretch.rs` unit tests operate on synthetic + ASTs and stay valid. Add an **end-to-end** revealjs render test (via + `render_document_to_file` / the binary) over `14-crossrefs/slides.qmd` + asserting: `@fig-1` resolves to `Figure 1`, `@fig-2` to `Figure 2`, fig-1's + `` is a direct child of its `
`, and + its caption reads "Figure 1: …". This is the regression the unit tests missed. + +### Part 3 — Preview (`q2-slides`) behavior for crossref figures + +**Decision (2026-06-18): option 3a for this strand, with a committed follow-up +(bd-zecehtnc).** After Part 2, in **native render** crossref figures number, +resolve, render, and stretch correctly. In **preview**, `crossref-render` is +excluded, so the float stays a `CustomNode("FloatRefTarget")`; numbering is still +correct (index runs in preview) but the final-Figure shape + auto-stretch do not +apply there. We accept that divergence in *this* strand (it fixes the +user-visible `q2 render` bug and matches the documented status quo), but the +WASM-based interactive previews (`q2 preview` **and** hub-client) must support +crossrefs well — tracked as **bd-zecehtnc**. + +Options considered for the eventual preview fix (decided in bd-zecehtnc): + +- **3b (teach auto-stretch the CustomNode form):** give auto-stretch a path that + also recognizes single-image `CustomNode("FloatRefTarget")` and hoists it, + preserving the custom node for React. +- **3c (include crossref-render in the slides preview):** drop `"crossref-render"` + from `Q2_PREVIEW_TRANSFORM_EXCLUDED` for the `q2-slides` path; React then + receives `Figure` instead of the custom node — revisit component expectations. +- React-side rendering of crossref CustomNodes (incl. reveal stretch) is a third + candidate. bd-zecehtnc will pick among these. + +### Part 4 — formalize "which transforms run under which format, in what order" + +Today reveal transforms are `pipeline.push`-ed inline under `if is_revealjs`, at +fixed positions. The footer work introduced `footer_render_stage(is_revealjs)` as +"the one place that maps format → render stage … the seam where a format → +stage-sequence mapping would grow." Part 2 adds a *second* format-gated +finalization slot (a reveal float-postprocess step after crossref). + +The **named helper** = the direct analogue of `footer_render_stage`: one +documented function owning "which reveal transforms run after crossref, and +where". Sketch: + +```rust +// returns the reveal-specific finalization transforms (today: just auto-stretch) +fn reveal_finalization_transforms(is_revealjs: bool) -> Vec> { + if is_revealjs { vec![Box::new(RevealAutoStretchTransform::new())] } + else { vec![] } +} +// spliced in once, right after CrossrefRenderTransform (finalization): +for t in reveal_finalization_transforms(is_revealjs) { pipeline.push(t); } +``` + +It is **not** a pipeline DSL or a format→stage map — just a named, greppable +function (CLAUDE.md forbids new architectural surface without strong reason). +If it feels like overkill for one transform, "defer the seam" (inline push at the +new position) is acceptable. Decision pending; default to the small helper since +Part 2 does add a second gated slot. + +--- + +## Decisions (2026-06-18) + +1. **Preview (Part 3):** option **3a** for this strand (native render fixed; preview + numbers/resolves but doesn't stretch). WASM interactive previews (`q2 preview` + + hub-client) must support crossrefs well — committed follow-up **bd-zecehtnc**. +2. **Bug B (Part 1):** **split** into format-agnostic sub-strand **bd-4ly7ne01** + (it affects HTML too). Will be done this session or a direct follow-up. +3. **Part 4 seam:** default to the small named helper + (`reveal_finalization_transforms`); finalize during implementation. Still + confirming whether to do it now vs. inline-push + defer. +4. **Structural guardrail (Part 5, NEW):** add a required `phase()` method to the + `AstTransform` trait (`transform.rs:69`) returning a `TransformPhase` enum + (`Normalization` < `Crossref` < `Navigation` < `Finalization`), and a + **format-neutral ordering-invariant test** over the real `build_transform_pipeline` + asserting phase ranks are non-decreasing by position. Trait-method approach + chosen over a test-only table (negligible cost — one vtable slot per type, zero + render-time/per-instance cost; may find other uses). The test is **format-neutral** + (loops over supported format strings, no `is_revealjs` branch) so future formats + (`dashboard`, `typst`, `pdf`) are covered for free. It **fails on today's code** + and **passes after Part 2's reorder** — i.e. it is the red/green TDD test for the + structural fix as well as the permanent guardrail. +5. **Architecture doc:** written to + `claude-notes/designs/transform-pipeline-phases.md` (phase model, invariant, + author rule, and the **preview-pipeline shape contract** — the anti-recurrence + rule). To be linked from `CLAUDE.md` → Architecture Notes **when the fix lands** + (so CLAUDE.md never points at an unenforced invariant). + +## Remaining checks during implementation + +- Are there other reveal-only transforms that mutate float/caption/identity shape + and would also need to move after crossref? Audit: `RevealColumns`, + `RevealSlides`, `RevealFootnotes`, `RevealFooterAlias`, `RevealFooterLogo` — + none appear to touch float identity, but confirm. +- Confirm `CrossrefRenderTransform` renders figure-floats as `Block::Figure` + (so auto-stretch's `Figure` arm catches them) and table-floats as `Div` (left + un-stretched, matching div-form behavior verified above). + +--- + +## Work items (provisional — finalize after design sign-off) + +- [ ] **Tests first (Part 5):** add `TransformPhase` enum + required `phase()` on + `AstTransform`; classify every existing transform; write the format-neutral + ordering-invariant test. Confirm it **FAILS** today (auto-stretch inversion). +- [ ] **Tests first:** end-to-end revealjs render regression over `14-crossrefs` + (fig-1/fig-2 number + resolve + r-stretch + caption); confirm it FAILS today. +- [ ] **Tests first:** caption-bearing bare-table float fixture (HTML), confirm FAILS. +- [ ] Part 2: move `RevealAutoStretchTransform` to after `CrossrefRenderTransform` + (declared phase `Finalization`); confirm the ordering test now **PASSES**. +- [ ] Part 1 (bd-4ly7ne01): desugar bare `Block::Table` w/ caption-id into + `Div(#tbl-…) > Table` so the uniform classifier handles it. +- [ ] Part 3: implement the chosen preview behavior (default 3a). +- [ ] Part 4 (optional): extract `reveal_finalization_transforms` seam. +- [ ] Part 5 doc: link `transform-pipeline-phases.md` from `CLAUDE.md` → + Architecture Notes once the invariant is enforced. +- [ ] Re-render `14-crossrefs/slides.qmd`; inspect HTML; record the end-to-end + output in this plan per CLAUDE.md "End-to-end verification". +- [ ] `cargo nextest run --workspace`; `cargo xtask verify` (WASM leg, since + `quarto-core` changes affect `wasm-quarto-hub-client`). + +--- + +## Part 5 sub-plan — `phase()` trait method + ordering-invariant test (IN PROGRESS) + +Logically first: it's the red/green TDD test for Part 2. Approach: `phase()` +defaults to `Unclassified`; every transform in `build_transform_pipeline` overrides +it; the test rejects `Unclassified` in the pipeline (exhaustiveness) + asserts +non-decreasing rank by position. Design: `claude-notes/designs/transform-pipeline-phases.md`. + +Phase assignments (47 pipeline structs; `JupyterTransform`, +`AttributionGenerateTransform`, test doubles stay `Unclassified`): + +- **Normalization:** Callout, CalloutResolve, ShortcodeResolve, MetadataNormalize, + CodeBlockGenerate, WebsiteTitlePrefix, WebsiteFavicon, WebsiteBootstrapIcons, + WebsiteCanonicalUrl, RevealColumns, RevealSlides, RevealFooterAlias, TitleBlock, + Sectionize, Footnotes, RevealFootnotes, ExampleEmbed, TheoremSugar, ProofSugar, + FloatRefTargetSugar, EquationLabel +- **Crossref:** CrossrefIndex, CrossrefResolve +- **Navigation:** TocGenerate, NavbarGenerate, SidebarGenerate, PageNavGenerate, + FooterGenerate, ListingGenerate, ListingRender, CategoriesSidebar, + ListingFeedStage, ListingFeedLink, TocRender, NavbarRender, SidebarRender, + PageNavRender, FooterRender, RevealFooterLogo +- **Finalization:** LinkRewrite, AppendixStructure, CrossrefRender, + ExampleEmbedRender, CodeBlockRender, ResourceCollector, TableBootstrapClass, + AttributionRender, AttributionViewer, **RevealAutoStretch** (← currently + mis-ordered before Crossref; Part 2 moves it after CrossrefRender) + +### Part 5 checklist + +- [x] 5.1 Add `TransformPhase` enum (`Normalization` < `Crossref` < `Navigation` < + `Finalization` < `Unclassified`) + defaulted `phase()` on `AstTransform` + (`transform.rs`). Done — enum + defaulted method, doc-commented. +- [x] 5.2 Override `phase()` on all 21 Normalization transforms. +- [x] 5.3 Override `phase()` on the 2 Crossref transforms. +- [x] 5.4 Override `phase()` on all 16 Navigation transforms. +- [x] 5.5 Override `phase()` on all 10 Finalization transforms (incl. + RevealAutoStretch = Finalization). 49 total inserted via script keyed by + struct name; `quarto-core` compiles clean. +- [x] 5.6 Wrote `test_build_transform_pipeline_phase_ordering` in `pipeline.rs`: + loops over `["html","revealjs"]`, asserts no `Unclassified` + non-decreasing + rank; failure names offender + full order. +- [x] 5.7 Confirmed **RED**: `[revealjs] phase ordering inversion: + reveal-auto-stretch (Finalization) runs before example-embed (Normalization)`. + HTML pipeline passes (no auto-stretch). Validates all phase assignments. + +### Part 2 checklist (makes 5.7 green) + +- [x] 2.1 Added `reveal_finalization_transforms(is_revealjs)` helper (Part 4 seam, + analogue of `footer_render_stage`). +- [x] 2.2 Removed `RevealAutoStretchTransform` from the early `is_revealjs` block; + left a NOTE pointing to its new home + the bd-w0c6d38k rationale. +- [x] 2.3 Spliced the helper after `ExampleEmbedRenderTransform` (post + `CrossrefRenderTransform`) and before `CodeBlockRenderTransform`/ + `ResourceCollectorTransform`. +- [x] 2.4 Ordering test **GREEN** (was RED). +- [x] 2.5 e2e regression test `revealjs_crossref_attribute_figure_resolves_and_stretches` + added; RED before the reorder, GREEN after. (Caption *number-prefix* + assertion intentionally dropped — see bd-uwv2eec2 below; ref numbering + + stretch + direct-section-child are asserted. Links use U+00A0.) +- [x] 2.6 Re-rendered `14-crossrefs/slides.qmd` via `q2 render` — see results below. +- [x] 2.7 Full `cargo nextest run -p quarto-core` → **2385 passed, 0 failed**. No + auto-stretch/snapshot/crossref regressions from the reorder. + +### Part 2 end-to-end verification (CLAUDE.md) + +Invocation: `cargo run --bin q2 -- render examples/presentations/14-crossrefs/slides.qmd` + +- **Before:** `Warning: unresolved crossref @fig-1 …` + `1 warning`. +- **After:** `Rendered 1 of 1 files … ` — **no warning.** Inspected `slides.html`: + - body refs: `Figure 1` and + `Figure 2` (correctly numbered; was `?fig-1?` + "Figure 1"). + - fig-1 (`![A figure]{#fig-1}`): `` direct + child of `
`, caption `

A figure

`. + - fig-2 (`::: {#fig-2}`): now ALSO `r-stretch` (the predicted side benefit — a + div-form crossref figure on a single-image slide is stretched now that + auto-stretch sees the rendered `Figure`), caption `

Figure 2: + Another figure.

`. + - `
` count = 0 (both hoisted). + +### Discovered (filed, out of scope here) + +- **bd-uwv2eec2** — attribute-form figure `![cap](x){#fig-N}` gets **no "Figure N:" + caption-number prefix** (the div form does). Pre-existing crossref-render + asymmetry; reproduces in plain HTML (`
A figure
`), so it + is **not** caused by the reorder. Fix belongs in `crossref_render.rs`. + +## Part 1 sub-plan — bare-table caption desugar (bd-4ly7ne01) — DONE + +`: caption {#tbl-…}` parses to a **bare `Block::Table`** with the id on the +table's own `attr` (verified via `pampa -t native`: +`Table ("tbl-bare",[],[]) (Caption Nothing [Plain …])`). The uniform +`classify_div`/`convert_div` path never saw it. + +Fix (matches "all syntaxes desugar into divs"): a `maybe_wrap_bare_table_into_div` +step at the top of `FloatRefTargetSugarTransform::transform_block` wraps such a +table into `Div(#tbl-…) > Table` — id (and classes/kvs) move onto the Div, the +Table is left anonymous (no duplicate id) — so the existing `[Block::Table(_)]` +arm of `convert_div` handles it. Integrating it inside the sugar transform means +it runs everywhere the transform runs (full pipeline, analysis pipeline, fixtures) +with no new registration. + +### Part 1 checklist + +- [x] 1.1 Confirm AST shape (`pampa -t native`): id on bare `Block::Table`. +- [x] 1.2 Tests first: `fixture_bare_table_caption_target` in `crossref_fixtures.rs` + — confirmed **RED** (`idx.get("tbl-bare")` = None). +- [x] 1.3 Implement `maybe_wrap_bare_table_into_div` in `float_ref_target.rs`. +- [x] 1.4 Fixture **GREEN**; index entry `("tbl-bare","tbl",vec![],1)`. +- [x] 1.5 End-to-end: rendered two bare-caption tables + `@tbl-data`/`@tbl-two` + in HTML **and** revealjs — both resolve to `Table 1`/`Table 2`, no + unresolved placeholders. +- [x] 1.6 Full `cargo nextest run -p quarto-core` → **2386 passed**. + +## Part 3 outcome — interactive-preview crossref (bd-zecehtnc) — CLOSED (semantics done) + +Investigation (2026-06-18) found the interactive preview already renders crossrefs +correctly, and Part 2 improved it: + +- The q2-preview React renderer (`ts-packages/preview-renderer/src/q2-preview/`) + has dedicated components for every crossref custom node — `custom/FloatRefTarget.tsx`, + `CrossrefResolvedRef.tsx`, `Theorem.tsx`, `Proof.tsx`, `Equation.tsx` — registered + in `registry.ts` and dispatched by `type_name` (`dispatchers.tsx:207`). + `FloatRefTarget` composes "Figure N:" captions; `CrossrefResolvedRef` renders + `{kind} {n}`. +- **Numbering, links, captions, figure-vs-div, theorems/proofs/equations all work** + — 53 `custom-components.integration.test.tsx` tests pass. +- **Part 2 also fixed preview's attribute-form figure crossref**: the + `![cap](x){#fig-1}` figure is no longer destroyed by early auto-stretch, so it + becomes a proper numbered `FloatRefTarget` custom node in preview too. +- **nbsp parity: already correct.** `CrossrefResolvedRef.tsx` already emits a + non-breaking space between kind and number (byte-confirmed: `…7d c2 a0 24…`); + the "regular space" I first reported was a terminal-rendering artifact. No fix. + +Decision (user): **close bd-zecehtnc as semantics-done.** The only remaining gap +is *visual* revealjs auto-stretch parity for crossref figures in preview (a +single-image crossref figure fills the slide in `render` but shows at natural size +in `preview`, because the Rust auto-stretch transform skips `Block::Custom` and +`FloatRefTarget.tsx` adds no `r-stretch`). Deferred to **bd-hbloemff** (recommended +approach there: make `RevealAutoStretchTransform` CustomNode-aware + have +`FloatRefTarget.tsx` honor the mark). + +## Reference: key files + +- Pipeline order: `crates/quarto-core/src/pipeline.rs:1078-1309` + (`build_transform_pipeline`), `:1348-1375` (`Q2_PREVIEW_TRANSFORM_EXCLUDED`), + `:1070` (`footer_render_stage` precedent). +- Auto-stretch: `crates/quarto-core/src/revealjs/auto_stretch.rs` (esp. + `hoist_figure`, doc lines 44–67 on the crossref divergence). +- Float sugar: `crates/quarto-core/src/transforms/float_ref_target.rs` + (`classify_div`/`classify_fig`; Table handling at the `[Block::Table(_)]` arm). +- Crossref transforms: `crates/quarto-core/src/transforms/crossref_{index,resolve,render}.rs`. +- Crossref design epic: `claude-notes/plans/2026-04-15-crossref-design.md` (bd-jsbg). +- Q1 references: `external-sources/quarto-cli/src/resources/filters/quarto-pre/parsefiguredivs.lua` + (table-caption float wrapping), `.../format-reveal.ts` `applyStretch`. diff --git a/crates/quarto-core/src/pipeline.rs b/crates/quarto-core/src/pipeline.rs index 03bc773d1..86a1a4927 100644 --- a/crates/quarto-core/src/pipeline.rs +++ b/crates/quarto-core/src/pipeline.rs @@ -1075,6 +1075,32 @@ fn footer_render_stage(is_revealjs: bool) -> Box img.r-stretch` +/// (bd-w0c6d38k). A new format that needs late, float-aware reshaping (e.g. +/// `dashboard`, `typst`) adds its transforms here rather than inline. +/// +/// These are `TransformPhase::Finalization` transforms; the phase-ordering +/// invariant (`test_build_transform_pipeline_phase_ordering`) keeps them after +/// the `Crossref` phase. +fn reveal_finalization_transforms( + is_revealjs: bool, +) -> Vec> { + if is_revealjs { + vec![Box::new(crate::revealjs::RevealAutoStretchTransform::new())] + } else { + Vec::new() + } +} + pub fn build_transform_pipeline( shortcode_paths: Vec, extensions: Vec, @@ -1139,13 +1165,16 @@ pub fn build_transform_pipeline( // Per-slide footnote/aside coalescing consumes FootnotesTransform's // resolved output (refs = `Span#fnrefN`, defs in the trailing // `Div#footnotes`), so it must run *after* it. Pure AST → benefits - // render and preview alike. See `revealjs::footnotes`. + // render and preview alike. See `revealjs::footnotes`. This is a + // `Normalization`-phase transform: it builds slide scaffolding and does + // not consume crossref semantics. pipeline.push(Box::new(crate::revealjs::RevealFootnotesTransform::new())); - // Auto-stretch single-image slides (add reveal's `.r-stretch`). Runs - // *after* footnote/aside coalescing so a slide that gained a coalesced - // aside has >1 body block and is correctly skipped. Default-on, gated - // by `auto-stretch: false`. See `revealjs::auto_stretch`. - pipeline.push(Box::new(crate::revealjs::RevealAutoStretchTransform::new())); + // NOTE: revealjs auto-stretch is NOT here. It is a `Finalization`-phase + // transform — it consumes the *rendered* `Figure` shape that + // `CrossrefRenderTransform` produces, so running it this early would + // hoist a crossref figure to a bare `` before the float is ever + // numbered (bd-w0c6d38k). It is spliced in after `CrossrefRenderTransform` + // via `reveal_finalization_transforms` below. } // Example-iframe embeds (bd-z1smhvuo / bd-t3cert81). Sugars // `Div.embed-example-iframe[file=…]` into a `CustomNode("ExampleEmbed")`. @@ -1265,6 +1294,13 @@ pub fn build_transform_pipeline( // dispatches on FloatRefTarget/Theorem/Proof), so the node survives to // here untouched. pipeline.push(Box::new(ExampleEmbedRenderTransform::new())); + // Format-specific presentation that consumes rendered crossref shapes: + // revealjs auto-stretch. Runs *after* `CrossrefRenderTransform` (so a + // single-image `![cap]{#fig-…}` figure is numbered/resolved/rendered to a + // real `Figure` first) and *before* `ResourceCollectorTransform` (so the + // hoisted `` is still visible to resource collection). See + // `reveal_finalization_transforms` and bd-w0c6d38k. + pipeline.extend(reveal_finalization_transforms(is_revealjs)); // bd-1tl09 Phase 0: code-block decoration Render. Consumes the // typed payload produced by `code-block-generate` in the // Normalization Phase and emits the outer wrapping markup @@ -2938,6 +2974,67 @@ mod tests { ); } + /// Format-neutral pipeline phase-ordering invariant (bd-w0c6d38k). + /// + /// Every transform in `build_transform_pipeline` must (1) declare a real + /// phase — not the `Unclassified` default — and (2) appear in non-decreasing + /// phase-rank order. Together these forbid a format-specific *presentation* + /// transform (e.g. revealjs auto-stretch, a `Finalization` transform) from + /// running before the format-agnostic *semantic* structure it consumes (the + /// `Crossref` phase) is established. + /// + /// The test loops over every render format string — there is deliberately + /// **no `is_revealjs` branch** — so a new output format (`dashboard`, + /// `typst`, `pdf`, …) is covered the moment its transforms are classified, + /// without editing this test. + /// + /// See `claude-notes/designs/transform-pipeline-phases.md`. + #[test] + fn test_build_transform_pipeline_phase_ordering() { + use crate::transform::TransformPhase; + + // Add new render format strings here as they land; the invariant then + // covers them automatically. + for format in ["html", "revealjs"] { + let runtime = make_test_runtime(); + let pipeline = build_transform_pipeline(vec![], vec![], runtime, format.to_string()); + let steps: Vec<(&str, TransformPhase)> = + pipeline.iter().map(|t| (t.name(), t.phase())).collect(); + + // (1) Exhaustiveness: every pipeline member must be classified. + let unclassified: Vec<&str> = steps + .iter() + .filter(|(_, p)| *p == TransformPhase::Unclassified) + .map(|(n, _)| *n) + .collect(); + assert!( + unclassified.is_empty(), + "[{format}] these pipeline transforms have no phase() override \ + (still TransformPhase::Unclassified) — classify them per \ + claude-notes/designs/transform-pipeline-phases.md: {unclassified:?}", + ); + + // (2) Monotonicity: phase ranks must not decrease by position. + for win in steps.windows(2) { + let (prev_name, prev_phase) = win[0]; + let (next_name, next_phase) = win[1]; + assert!( + prev_phase <= next_phase, + "[{format}] phase ordering inversion: `{prev_name}` ({prev_phase:?}) \ + runs before `{next_name}` ({next_phase:?}), but {prev_phase:?} \ + ranks after {next_phase:?}. A transform that consumes semantic \ + structure must not precede the phase that produces it. \ + See claude-notes/designs/transform-pipeline-phases.md.\n\ + Full order: {:?}", + steps + .iter() + .map(|(n, p)| format!("{n}:{p:?}")) + .collect::>(), + ); + } + } + } + #[test] fn q2_preview_pipeline_includes_code_block_decoration_transforms() { let runtime = make_test_runtime(); diff --git a/crates/quarto-core/src/project/listing/feed/link_inject.rs b/crates/quarto-core/src/project/listing/feed/link_inject.rs index 643389fc4..77ae61771 100644 --- a/crates/quarto-core/src/project/listing/feed/link_inject.rs +++ b/crates/quarto-core/src/project/listing/feed/link_inject.rs @@ -31,7 +31,7 @@ use crate::Result; use crate::project::listing::ResolvedListing; use crate::project::website_config::{website_site_url, website_title}; use crate::render::RenderContext; -use crate::transform::AstTransform; +use crate::transform::{AstTransform, TransformPhase}; use crate::transforms::is_feature_disabled; /// Pass-2 transform: append a `` to @@ -60,6 +60,10 @@ impl AstTransform for ListingFeedLinkTransform { "listing-feed-link" } + fn phase(&self) -> TransformPhase { + TransformPhase::Navigation + } + async fn transform(&self, ast: &mut Pandoc, ctx: &mut RenderContext) -> Result<()> { if is_feature_disabled(&ast.meta, "listing") { return Ok(()); diff --git a/crates/quarto-core/src/project/listing/feed/stage.rs b/crates/quarto-core/src/project/listing/feed/stage.rs index de420c05a..c71abb3db 100644 --- a/crates/quarto-core/src/project/listing/feed/stage.rs +++ b/crates/quarto-core/src/project/listing/feed/stage.rs @@ -33,7 +33,7 @@ use crate::project::listing::config::{FeedType, ListingFeedOptions}; use crate::project::listing::item::ListingItem; use crate::project::website_config::website_site_url; use crate::render::RenderContext; -use crate::transform::AstTransform; +use crate::transform::{AstTransform, TransformPhase}; use crate::transforms::is_feature_disabled; use super::binding::{ @@ -94,6 +94,10 @@ impl AstTransform for ListingFeedStageTransform { "listing-feed-stage" } + fn phase(&self) -> TransformPhase { + TransformPhase::Navigation + } + async fn transform(&self, ast: &mut Pandoc, ctx: &mut RenderContext) -> Result<()> { if is_feature_disabled(&ast.meta, "listing") { return Ok(()); diff --git a/crates/quarto-core/src/revealjs/auto_stretch.rs b/crates/quarto-core/src/revealjs/auto_stretch.rs index 2ad19eaee..7af9d9fb6 100644 --- a/crates/quarto-core/src/revealjs/auto_stretch.rs +++ b/crates/quarto-core/src/revealjs/auto_stretch.rs @@ -75,7 +75,7 @@ use quarto_source_map::{By, SourceInfo}; use crate::Result; use crate::render::RenderContext; -use crate::transform::AstTransform; +use crate::transform::{AstTransform, TransformPhase}; const STRETCH_CLASS: &str = "r-stretch"; @@ -100,6 +100,10 @@ impl AstTransform for RevealAutoStretchTransform { "reveal-auto-stretch" } + fn phase(&self) -> TransformPhase { + TransformPhase::Finalization + } + async fn transform(&self, ast: &mut Pandoc, _ctx: &mut RenderContext) -> Result<()> { let auto_stretch = ast .meta diff --git a/crates/quarto-core/src/revealjs/columns.rs b/crates/quarto-core/src/revealjs/columns.rs index 887fe7ae0..aad2c6b74 100644 --- a/crates/quarto-core/src/revealjs/columns.rs +++ b/crates/quarto-core/src/revealjs/columns.rs @@ -24,7 +24,7 @@ use quarto_pandoc_types::pandoc::Pandoc; use crate::Result; use crate::render::RenderContext; -use crate::transform::AstTransform; +use crate::transform::{AstTransform, TransformPhase}; /// Transform that rewrites `.column` `width` attributes to `flex-basis`. pub struct RevealColumnsTransform; @@ -47,6 +47,10 @@ impl AstTransform for RevealColumnsTransform { "reveal-columns" } + fn phase(&self) -> TransformPhase { + TransformPhase::Normalization + } + async fn transform(&self, ast: &mut Pandoc, _ctx: &mut RenderContext) -> Result<()> { walk_blocks(&mut ast.blocks); Ok(()) diff --git a/crates/quarto-core/src/revealjs/footer_logo.rs b/crates/quarto-core/src/revealjs/footer_logo.rs index 243c708f5..cad2ba4d3 100644 --- a/crates/quarto-core/src/revealjs/footer_logo.rs +++ b/crates/quarto-core/src/revealjs/footer_logo.rs @@ -51,7 +51,7 @@ use quarto_source_map::{By, SourceInfo}; use crate::Result; use crate::render::RenderContext; -use crate::transform::AstTransform; +use crate::transform::{AstTransform, TransformPhase}; // --- config alias: footer: → page-footer: ----------------------------------- @@ -78,6 +78,10 @@ impl AstTransform for RevealFooterAliasTransform { "reveal-footer-alias" } + fn phase(&self) -> TransformPhase { + TransformPhase::Normalization + } + async fn transform(&self, ast: &mut Pandoc, _ctx: &mut RenderContext) -> Result<()> { // `page-footer:` wins if the user set both (it's the canonical key). if ast.meta.contains_path(&["page-footer"]) { @@ -116,6 +120,10 @@ impl AstTransform for RevealFooterLogoTransform { "reveal-footer-logo" } + fn phase(&self) -> TransformPhase { + TransformPhase::Navigation + } + async fn transform(&self, ast: &mut Pandoc, _ctx: &mut RenderContext) -> Result<()> { if !ast.meta.contains_path(&["rendered", "reveal", "footer"]) && let Some(html) = footer_slot_html(&ast.meta) diff --git a/crates/quarto-core/src/revealjs/footnotes.rs b/crates/quarto-core/src/revealjs/footnotes.rs index 34d896fa5..ac848cf90 100644 --- a/crates/quarto-core/src/revealjs/footnotes.rs +++ b/crates/quarto-core/src/revealjs/footnotes.rs @@ -64,7 +64,7 @@ use quarto_source_map::{By, SourceInfo}; use crate::Result; use crate::render::RenderContext; -use crate::transform::AstTransform; +use crate::transform::{AstTransform, TransformPhase}; /// Transform that coalesces per-slide footnotes/asides for `format: revealjs`. pub struct RevealFootnotesTransform; @@ -87,6 +87,10 @@ impl AstTransform for RevealFootnotesTransform { "reveal-footnotes" } + fn phase(&self) -> TransformPhase { + TransformPhase::Normalization + } + async fn transform(&self, ast: &mut Pandoc, _ctx: &mut RenderContext) -> Result<()> { // Q1: `slideFootnotes = referenceLocation !== "document"`. Reveal never // sets `reference-location`, so the default (unset) coalesces; an diff --git a/crates/quarto-core/src/revealjs/transform.rs b/crates/quarto-core/src/revealjs/transform.rs index f87ba303c..382a35b3b 100644 --- a/crates/quarto-core/src/revealjs/transform.rs +++ b/crates/quarto-core/src/revealjs/transform.rs @@ -26,7 +26,7 @@ use quarto_source_map::{By, SourceInfo}; use crate::Result; use crate::render::RenderContext; -use crate::transform::AstTransform; +use crate::transform::{AstTransform, TransformPhase}; use super::slides::{DEFAULT_SLIDE_LEVEL, build_reveal_slides}; @@ -51,6 +51,10 @@ impl AstTransform for RevealSlidesTransform { "reveal-slides" } + fn phase(&self) -> TransformPhase { + TransformPhase::Normalization + } + async fn transform(&self, ast: &mut Pandoc, _ctx: &mut RenderContext) -> Result<()> { let slide_level = ast .meta diff --git a/crates/quarto-core/src/transform.rs b/crates/quarto-core/src/transform.rs index 78242a062..0932601c5 100644 --- a/crates/quarto-core/src/transform.rs +++ b/crates/quarto-core/src/transform.rs @@ -51,6 +51,47 @@ use crate::Result; use crate::render::RenderContext; +/// The phase a transform belongs to within the ordered render pipeline. +/// +/// Phases are **totally ordered**; the declaration order *is* the contract. +/// The render pipeline must visit transforms in non-decreasing phase rank — +/// a presentation transform must never run before the semantic structure it +/// consumes is established. See +/// `claude-notes/designs/transform-pipeline-phases.md` for the full model, +/// the author rule (which phase do I pick?), and the preview-pipeline shape +/// contract. The invariant is enforced by `test_build_transform_pipeline_phase_ordering` +/// in `pipeline.rs`. +/// +/// `Unclassified` is the default for `AstTransform`s that are **not** members +/// of the ordered multi-format render pipeline (engine-stage transforms, +/// stage-level transforms, test doubles). The ordering test rejects +/// `Unclassified` for any transform that actually appears in +/// `build_transform_pipeline`, so every real pipeline member must override +/// `phase()`. It is declared last so a leaked value sorts high. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub enum TransformPhase { + /// Format-agnostic semantic normalization (callout/theorem/proof/float/ + /// equation sugar, metadata normalize, footnotes, …) **and** format-specific + /// *structural scaffolding* that does not consume crossref semantics + /// (revealjs slide/column construction, footnote coalescing, metadata aliases). + Normalization, + /// Crossref numbering and `@ref` resolution (`crossref-index`, + /// `crossref-resolve`). Note: crossref *render* (custom node → `Figure`/ + /// `Div`/`Span`/`Link`) is `Finalization`, not here. + Crossref, + /// Navigation chrome generate + render (TOC, navbar, sidebar, page-nav, + /// footer, listings). + Navigation, + /// Render custom nodes to writer-visible shapes (`crossref-render`, + /// `example-embed-render`, `code-block-render`), format presentation that + /// *consumes* those rendered shapes (revealjs auto-stretch), then + /// resource/attribution baking. + Finalization, + /// Not a member of the ordered render pipeline. Default; the ordering test + /// rejects this value for transforms that appear in `build_transform_pipeline`. + Unclassified, +} + /// Trait for AST transformations. /// /// Transforms modify the Pandoc AST during the render pipeline. @@ -72,6 +113,16 @@ pub trait AstTransform: Send + Sync { /// Used for logging and debugging. fn name(&self) -> &str; + /// The pipeline phase this transform belongs to. + /// + /// Defaults to [`TransformPhase::Unclassified`]. Every transform that is a + /// member of `build_transform_pipeline` **must** override this with a real + /// phase; the ordering invariant test fails otherwise. See + /// [`TransformPhase`] and `claude-notes/designs/transform-pipeline-phases.md`. + fn phase(&self) -> TransformPhase { + TransformPhase::Unclassified + } + /// Apply the transformation to the AST. /// /// # Arguments diff --git a/crates/quarto-core/src/transforms/appendix.rs b/crates/quarto-core/src/transforms/appendix.rs index 2a16c0728..2133d682b 100644 --- a/crates/quarto-core/src/transforms/appendix.rs +++ b/crates/quarto-core/src/transforms/appendix.rs @@ -56,7 +56,7 @@ use quarto_pandoc_types::ConfigValue; use crate::Result; use crate::render::RenderContext; -use crate::transform::AstTransform; +use crate::transform::{AstTransform, TransformPhase}; use crate::transforms::{AppendixStyle, ReferenceLocation}; /// Transform that consolidates appendix content into a single container. @@ -119,6 +119,10 @@ impl AstTransform for AppendixStructureTransform { "appendix-structure" } + fn phase(&self) -> TransformPhase { + TransformPhase::Finalization + } + async fn transform(&self, ast: &mut Pandoc, _ctx: &mut RenderContext) -> Result<()> { let meta = &ast.meta; let appendix_style = Self::get_appendix_style(meta); diff --git a/crates/quarto-core/src/transforms/attribution_render.rs b/crates/quarto-core/src/transforms/attribution_render.rs index c327f2988..5e0e13746 100644 --- a/crates/quarto-core/src/transforms/attribution_render.rs +++ b/crates/quarto-core/src/transforms/attribution_render.rs @@ -70,7 +70,7 @@ use crate::attribution::{ attribution_viewer_enabled_from_meta, resolve_byte_range, }; use crate::render::RenderContext; -use crate::transform::AstTransform; +use crate::transform::{AstTransform, TransformPhase}; pub struct AttributionRenderTransform; @@ -92,6 +92,10 @@ impl AstTransform for AttributionRenderTransform { "attribution-render" } + fn phase(&self) -> TransformPhase { + TransformPhase::Finalization + } + async fn transform(&self, ast: &mut Pandoc, ctx: &mut RenderContext) -> Result<()> { let Some(data) = ctx.attribution_data.clone() else { return Ok(()); diff --git a/crates/quarto-core/src/transforms/attribution_viewer.rs b/crates/quarto-core/src/transforms/attribution_viewer.rs index 0e9203220..4bb79094b 100644 --- a/crates/quarto-core/src/transforms/attribution_viewer.rs +++ b/crates/quarto-core/src/transforms/attribution_viewer.rs @@ -50,7 +50,7 @@ use quarto_pandoc_types::pandoc::Pandoc; use crate::Result; use crate::attribution::{IdentityMap, VIEWER_CSS, VIEWER_JS}; use crate::render::RenderContext; -use crate::transform::AstTransform; +use crate::transform::{AstTransform, TransformPhase}; /// HTML-comment sentinel embedded in the injected `