diff --git a/claude-notes/plans/2026-06-19-crossref-duplicate-id-diagnostic.md b/claude-notes/plans/2026-06-19-crossref-duplicate-id-diagnostic.md new file mode 100644 index 000000000..52e076354 --- /dev/null +++ b/claude-notes/plans/2026-06-19-crossref-duplicate-id-diagnostic.md @@ -0,0 +1,227 @@ +# Duplicate crossref identifier — diagnosis & structured diagnostic + +**Strand:** bd-rr6qzcvu (discovered-from bd-bxrkxblx) +**Date:** 2026-06-19 +**Status:** decisions settled 2026-06-22; implementing **Facet B first** (Stage 1). + +## Decisions (settled 2026-06-22) +- **A — relabeling:** approved (`fig-charts-jupyter` / `fig-charts-knitr`), but + **deferred to Stage 2**. The user wants to *see* the improved error message on + a real `docs/` render before the underlying duplicate is removed. +- **B — catalog code:** dedicated `crossref` subsystem **15**, code `Q-15-1`. +- **C — scope/order:** **staggered, two stages.** + - **Stage 1 (now): Facet B** — the structured diagnostic. Leave the duplicate + in `figures.qmd` in place so `q2 render docs/` still fires it and shows the + new boxed `[Q-15-1]` form. + - **Stage 2 (later): Facet A** — relabel the two cells; `docs/` render goes to + 0 errors. + +## Symptom + +`cargo run --bin q2 -- render docs/` finishes with: + +``` +Rendered 168 of 168 files to …/docs/_site — 1 error, 27 warnings +``` + +The lone error is: + +``` +Error: duplicate crossref identifier `fig-charts` +``` + +It is **collected**, not fatal — the render completes (168/168). It prints as a +bare one-line string: no error code, no source span, no ariadne box (contrast +the `Q-5-6`/`Q-13-4` diagnostics in the same run, which render with boxes). + +There are **two independent problems** here. One clears the error; the other is +the durable improvement the strand is really about. + +--- + +## Facet A — the docs content bug (what trips the error) + +`docs/guides/authoring/figures.qmd` has **two** code cells both labeled +`#| label: fig-charts`: + +- line ~428 — a **Jupyter** (`{python}`) cell, inside a `::: panel-tabset` +- line ~448 — a **Knitr** (`{r}`) cell, the sibling tab in the same tabset + +They are the "Subcaptions" example shown in two engines (Jupyter tab / Knitr +tab). Because both panels live in the rendered DOM simultaneously (tabsets are +CSS show/hide), two elements end up with `id="fig-charts"` — invalid HTML and an +ambiguous cross-reference target. The crossref indexer rejects the second one. + +**Safe to fix:** there is **no `@fig-charts` reference anywhere in `docs/`** +(grep is empty), so relabeling breaks nothing. + +### Options (Decision A) +- **(A1, recommended) Relabel both** to engine-specific ids: + `fig-charts-jupyter` and `fig-charts-knitr`. Keeps both figures + cross-referenceable and unambiguous; matches the "same example, two engines" + intent. +- **(A2) Keep one labeled, drop the label from the other.** The unlabeled tab + loses numbering/cross-referenceability. Simpler diff, but asymmetric for no + real reason. + +This is the change that makes `q2 render docs/` report **0 errors**. + +> Note: this is genuinely a per-document authoring fix, *not* a sign the +> crossref engine is wrong to reject it — two live DOM nodes sharing an id is a +> real defect regardless of tabs. + +--- + +## Facet B — the engineering bug (the diagnostic is unstructured) + +The diagnostic is built by `duplicate_id_diagnostic` in +`crates/quarto-core/src/transforms/crossref_index.rs:369`: + +```rust +fn duplicate_id_diagnostic( + id: &str, + _first: &quarto_source_map::SourceInfo, + _second: &quarto_source_map::SourceInfo, +) -> DiagnosticMessage { + // SourceInfo attachment to DiagnosticMessage would let ariadne render + // both occurrences; plumb that through when the broader source-context + // threading in transforms is sorted out (same TODO as the metadata + // diagnostics). + DiagnosticMessage::error(format!("duplicate crossref identifier `{id}`")) +} +``` + +**The fix is unusually clean** — most of the wiring already exists: + +1. **Both spans are already passed in and discarded.** At the call site + (`crossref_index.rs:262-268`) the indexer has `existing_src` (the first + occurrence, stored in the index) and `node.source_info` (the duplicate). It + passes both to `duplicate_id_diagnostic` as `_first`/`_second`. The TODO is + the only thing standing between us and a located diagnostic. +2. **The render path already carries them to output.** `walker.diagnostics` is + pushed onto `ctx.diagnostics` (`crossref_index.rs:89-92`) → + `render_output.diagnostics`, rendered against `render_output.source_context` + — the exact path the new `Q-5-6` warning rides. The doc's source is in that + context, so `Original` spans resolve. +3. **The plain-text rendering is purely a missing-location artifact.** `to_text` + gates the ariadne box on `has_any_location = self.location.is_some() || any + detail has a location` (`diagnostic.rs:357`). Today that's false → plain + string. Attaching a location flips it to the boxed render automatically. + +### The change +Rewrite `duplicate_id_diagnostic` to use `DiagnosticMessageBuilder`: + +```rust +DiagnosticMessageBuilder::error("Duplicate crossref identifier") + .with_code("Q-15-1") + .with_location(second.clone()) // underline the duplicate + .problem(format!( + "The crossref identifier `{id}` is defined more than once. \ + Each crossref id must be unique within a document." + )) + .add_detail_at("first defined here", first.clone()) // the original site + .add_hint("Give one of the targets a different `label:` / `#id`.") + .build() +``` + +(`add_detail_at` / `add_faded_at` already exist on the builder — see +`builder.rs`.) Keep it **error-level**: a duplicate id is genuinely ambiguous. + +### New catalog code (Decision B) +No `crossref` subsystem exists yet. The `CONTRIBUTING-ERRORS.md` table predates +the catalog's real growth (it stops at 11 and marks "12+ Reserved", yet the +catalog already has listing=12, navigation=13, theme=14 — each a dedicated +subsystem with a descriptive string). Following that established practice: + +- **(B1, recommended) Dedicated `crossref` subsystem = number 15, code + `Q-15-1`.** Mirrors listing/navigation/theme; gives `docs/errors/crossref/` a + natural home. Next new subsystem number after theme(14). +- **(B2) Fold under `Q-4` "Rendering and Formats"** (currently unused). Honors + the CONTRIBUTING table's reservation, but crossref isn't really "formats", and + no Q-4 code exists to anchor it. + +If B1: also refresh the stale `CONTRIBUTING-ERRORS.md` subsystem table to record +12/13/14/15 (small, optional, keeps the registry honest). + +### Scope (Decision C) +The strand description floated splitting A and B. Recommendation: **do both in +this one strand/PR.** Facet A is what clears the actual CI/docs error; Facet B +is small and is the reason the strand is interesting. They're tightly related +(B's regression test is most naturally a duplicate-id fixture, and A is the +real-world instance B improves the reporting of). + +--- + +## Test plan (TDD — write tests first) + +1. **Catalog presence test** (`catalog.rs`): `Q-15-1` exists, subsystem + `crossref`, title mentions "duplicate"/"crossref", `docs_url` ends in the + code. (Red → green when the catalog entry lands.) +2. **Transform unit test** (`crossref_index.rs` tests): index two custom + crossref targets sharing an id, each with a *distinct* `SourceInfo`; assert + the single emitted diagnostic has `code == Q-15-1`, a primary + `location.is_some()` (the second), and at least one detail whose + `location.is_some()` (the first). The existing test scaffolding + (`run …` returning `(ast, index, diagnostics)`, the `si()` helper) makes this + direct. +3. **Render smoke (optional)**: assert the rendered diagnostic now satisfies + `has_any_location` (i.e. it would take the ariadne branch) — guards against a + future regression dropping the spans again. +4. **Docs build**: `q2 render docs/errors/crossref/Q-15-1.qmd` (via the project) + builds clean. +5. **End-to-end**: + - After **Facet A**: `q2 render docs/` reports **0 errors** (down from 1). + - For **Facet B**: a tiny fixture qmd with a deliberate duplicate id, rendered + through the binary, shows the boxed `[Q-15-1]` diagnostic underlining both + label sites. Paste the output here. + +## Work items + +### Stage 1 — Facet B (the structured diagnostic) — IN PROGRESS + +- [x] Settle Decisions A (relabel approach), B (subsystem number), C (one PR vs + split) with the user. *(2026-06-22: relabel both but defer to Stage 2; + crossref=15; staggered, B first.)* +- [x] **(B)** Add `Q-15-1` to `error_catalog.json` (subsystem `crossref`); + catalog presence test (`error_catalog_has_q_15_1_crossref`) — red → green. +- [x] **(B)** Transform unit test `duplicate_id_diagnostic_is_coded_and_located` + (code + primary location + located detail) — red → green. +- [x] **(B)** Rewrite `duplicate_id_diagnostic` to the builder form + (`with_code` / `with_location(second)` / `add_detail_at(first)` / hint); + the `first`/`second` spans were already passed in. Stale TODO removed. +- [x] **(B)** Authored `docs/errors/crossref/Q-15-1.qmd` (new `crossref/` + subsystem dir). +- [x] **(B)** Refreshed the `CONTRIBUTING-ERRORS.md` subsystem table + (added 12/13/14/15 + a note on how new subsystems are numbered). +- [x] End-to-end (Facet B): fixture with two `#fig-charts` images → boxed + `[Q-15-1]` underlining **both** sites ("first defined here" + problem), + exit 0. Real `docs/` render: the `figures.qmd` tabset duplicate now renders + the same boxed multi-line diagnostic (cells 427–441 and 447–457). Tally + stays "1 error" by design (duplicate still present — that's Stage 2). +- [ ] `cargo nextest run --workspace` (10,289 passed) ✓; `cargo xtask verify` + (full — `quarto-core` is in the WASM closure) — *running.* + +### Stage 2 — Facet A (relabel the docs cells) — DONE + +- [x] **(A)** Relabeled the two `fig-charts` cells in + `docs/guides/authoring/figures.qmd` to `fig-charts-jupyter` / + `fig-charts-knitr`. *(2026-06-22)* `q2 render docs/` now reports + **`Rendered 169 of 169 files … — 27 warnings`** (was `1 error, 27 + warnings`): no `Error:` lines, no `Q-15-1`. The 27 warnings are the + pre-existing `Q-5-6` (missing images) and `Q-13-4` (body links) — out of + scope here. + +## Key references +- `crates/quarto-core/src/transforms/crossref_index.rs:262-268` — call site + (both spans available) and `:369` — `duplicate_id_diagnostic` (drops them). +- `crates/quarto-core/src/transforms/crossref_index.rs:89-92` — diagnostics → + `ctx.diagnostics`. +- `crates/quarto-error-reporting/src/diagnostic.rs:357` — the `has_any_location` + gate that currently forces plain-text rendering. +- `crates/quarto-error-reporting/src/builder.rs` — `with_code`, `with_location`, + `add_detail_at`, `add_hint`. +- `crates/quarto-error-reporting/CONTRIBUTING-ERRORS.md:83` — subsystem-number + registry (stale past 11). +- `docs/guides/authoring/figures.qmd:428,448` — the duplicate-labeled cells. +- Precedent: bd-bxrkxblx's `resource_copy_diagnostics.rs` + `Q-5-6`/`Q-5-7` + pages — same shape of fix (code + span + docs page). diff --git a/crates/quarto-core/src/transforms/crossref_index.rs b/crates/quarto-core/src/transforms/crossref_index.rs index 60195aba1..74113891a 100644 --- a/crates/quarto-core/src/transforms/crossref_index.rs +++ b/crates/quarto-core/src/transforms/crossref_index.rs @@ -366,16 +366,29 @@ fn extract_caption_inlines(node: &CustomNode) -> Option DiagnosticMessage { - // SourceInfo attachment to DiagnosticMessage would let ariadne render - // both occurrences; plumb that through when the broader source-context - // threading in transforms is sorted out (same TODO as the metadata - // diagnostics). - DiagnosticMessage::error(format!("duplicate crossref identifier `{id}`")) + quarto_error_reporting::DiagnosticMessageBuilder::error("Duplicate crossref identifier") + .with_code("Q-15-1") + .with_location(second.clone()) + .problem(format!( + "The crossref identifier `{id}` is defined more than once. Each \ + crossref id must be unique within a document so a reference like \ + `@{id}` resolves to exactly one target." + )) + .add_detail_at("first defined here", first.clone()) + .add_hint("Give one of the targets a different `label:` (or `#id`).") + .build() } #[cfg(test)] @@ -540,6 +553,40 @@ mod tests { assert_eq!(idx.entries.len(), 1); } + // bd-rr6qzcvu: the duplicate-id diagnostic must be a structured, + // coded, located diagnostic — not a bare string. It carries `Q-15-1`, + // a primary location (the duplicate occurrence), and a detail located + // at the first occurrence, so the renderer underlines both sites. + #[tokio::test] + async fn duplicate_id_diagnostic_is_coded_and_located() { + use quarto_error_reporting::DiagnosticKind; + + let (_, _idx, diags) = run(vec![ + fig_div("fig-dup", "first caption"), + fig_div("fig-dup", "second caption"), + ]) + .await; + assert_eq!(diags.len(), 1); + let diag = &diags[0]; + + assert_eq!(diag.code.as_deref(), Some("Q-15-1"), "must carry the code"); + assert_eq!(diag.kind, DiagnosticKind::Error, "duplicate ids are errors"); + assert!( + diag.location.is_some(), + "must have a primary location (the duplicate occurrence)" + ); + assert!( + diag.details.iter().any(|d| d.location.is_some()), + "must point at the first occurrence via a located detail" + ); + // The offending id is still named, for users reading the bare text. + let text = diag.to_text(None); + assert!( + text.contains("fig-dup"), + "diagnostic text must name the duplicate id; got: {text}" + ); + } + #[test] fn section_stack_algo_basic() { let mut s = Vec::new(); diff --git a/crates/quarto-error-reporting/CONTRIBUTING-ERRORS.md b/crates/quarto-error-reporting/CONTRIBUTING-ERRORS.md index cc8eee0fc..c73cd98f5 100644 --- a/crates/quarto-error-reporting/CONTRIBUTING-ERRORS.md +++ b/crates/quarto-error-reporting/CONTRIBUTING-ERRORS.md @@ -96,7 +96,17 @@ Quarto uses TypeScript-style error codes: **`Q--`** | **9** | XML/CSL | Q-9-1 (XML syntax error) | | **10** | Templates | Q-10-1 (Template parse error) | | **11** | Lua Filters | Q-11-1 (Lua filter diagnostic) | -| **12+** | Reserved | For future subsystems | +| **12** | Listing (`listing`) | Q-12-1 (Listing diagnostics) | +| **13** | Navigation (`navigation`) | Q-13-1 (Sidebar references missing document) | +| **14** | Theme (`theme`) | Q-14-1 (Invalid theme config) | +| **15** | Crossref (`crossref`) | Q-15-1 (Duplicate crossref identifier) | +| **16+** | Reserved | For future subsystems | + +> New subsystems take the next free number with a descriptive +> `subsystem` string (as `listing`/`navigation`/`theme`/`crossref` +> did). The number is what appears in the code; the string is what +> appears in the catalog entry and the `docs/errors//` +> directory. ### Finding the Next Error Code diff --git a/crates/quarto-error-reporting/error_catalog.json b/crates/quarto-error-reporting/error_catalog.json index 7574a4e5c..b7f65e775 100644 --- a/crates/quarto-error-reporting/error_catalog.json +++ b/crates/quarto-error-reporting/error_catalog.json @@ -1035,5 +1035,12 @@ "message_template": "The `theme:` value names a theme Quarto does not recognize. Use one of the built-in Bootswatch theme names (e.g. `cosmo`, `darkly`), a path to a `.scss`/`.css` file, or `theme: none` to suppress Bootstrap.", "docs_url": "https://quarto.org/docs/errors/theme/Q-14-2", "since_version": "99.9.9" + }, + "Q-15-1": { + "subsystem": "crossref", + "title": "Duplicate Crossref Identifier", + "message_template": "A crossref identifier (e.g. a `fig-`/`tbl-` label or an explicit `#id`) is defined more than once in the same document. Each crossref id must be unique within a document, so a reference like `@fig-foo` resolves to exactly one target. Give one of the targets a different label.", + "docs_url": "https://quarto.org/docs/errors/crossref/Q-15-1", + "since_version": "99.9.9" } } diff --git a/crates/quarto-error-reporting/src/catalog.rs b/crates/quarto-error-reporting/src/catalog.rs index 622f19ab0..4aed037e3 100644 --- a/crates/quarto-error-reporting/src/catalog.rs +++ b/crates/quarto-error-reporting/src/catalog.rs @@ -284,4 +284,34 @@ mod tests { q7.docs_url ); } + + // bd-rr6qzcvu: Q-15-1 — the new `crossref` subsystem's first code, + // for a duplicate crossref identifier. + #[test] + fn error_catalog_has_q_15_1_crossref() { + let info = get_error_info("Q-15-1").expect("Q-15-1 must be in the catalog"); + assert_eq!(info.subsystem, "crossref"); + assert!( + info.title.to_lowercase().contains("duplicate") + && info.title.to_lowercase().contains("crossref"), + "Q-15-1 title must mention a duplicate crossref; got: {}", + info.title + ); + assert!( + info.message_template.to_lowercase().contains("unique") + || info + .message_template + .to_lowercase() + .contains("more than once"), + "Q-15-1 message must explain the uniqueness requirement; got: {}", + info.message_template + ); + assert!( + info.docs_url + .as_deref() + .is_some_and(|u| u.ends_with("Q-15-1")), + "Q-15-1 docs_url must end with the code; got: {:?}", + info.docs_url + ); + } } diff --git a/docs/errors/crossref/Q-15-1.qmd b/docs/errors/crossref/Q-15-1.qmd new file mode 100644 index 000000000..db088ddf7 --- /dev/null +++ b/docs/errors/crossref/Q-15-1.qmd @@ -0,0 +1,62 @@ +--- +title: "Duplicate Crossref Identifier" +description: "A crossref identifier (such as a `fig-`/`tbl-` label or an explicit `#id`) is defined more than once in the same document." +code: Q-15-1 +subsystem: crossref +status: stub +since: "99.9.9" +categories: + - crossref +--- + +# `Q-15-1` — Duplicate Crossref Identifier + +> A crossref identifier (such as a `fig-`/`tbl-` label or an +> explicit `#id`) is defined more than once in the same document. + +## What this means + +Cross-references resolve by identifier: when you write `@fig-plot`, +Quarto looks up the one element whose id is `fig-plot` and links to +it with the right number ("Figure 3"). For that to be unambiguous, +each crossref id must be **unique within a document**. + +This error fires when the same id is assigned to two (or more) +elements — two figures both labeled `#| label: fig-plot`, two divs +both carrying `{#fig-plot}`, and so on. The diagnostic underlines +**both** occurrences: the first definition and the duplicate. + +Quarto keeps the **first** occurrence in the crossref index and +skips numbering the duplicate, so the render still completes — but +any `@fig-plot` reference would be ambiguous, and the duplicated +`id` produces invalid HTML (two elements sharing one `id`). + +## Why this happens + +Common causes: + +- **Copy-paste** — a figure or table block was duplicated and its + `label:` / `#id` was not changed. +- **The same example shown twice** — e.g. the same figure presented + in two tabs of a `panel-tabset` (one per engine), both keeping the + original label. +- **A generated label collides** — an auto-derived id happens to + match one you wrote by hand. + +## How to fix + +Give one of the targets a different identifier so each is unique: + +- For computational figures/tables, change the `#| label:` option + (e.g. `fig-plot-jupyter` and `fig-plot-knitr`). +- For explicit ids, change the `{#fig-plot}` attribute. + +Then update any `@`-references to point at the id you intend. If the +two elements really are the same figure shown two ways and you do +not need to cross-reference one of them, you can remove the label +from that one instead. + +## Related + +- `Q-12-4` — a duplicate **listing** id (the same uniqueness + requirement, in the listing subsystem). diff --git a/docs/guides/authoring/figures.qmd b/docs/guides/authoring/figures.qmd index 6c39e954f..068ba7895 100644 --- a/docs/guides/authoring/figures.qmd +++ b/docs/guides/authoring/figures.qmd @@ -425,7 +425,7 @@ You can create subcaptions for computational output by combining the `fig-cap` a #### Jupyter ```{{python}} -#| label: fig-charts +#| label: fig-charts-jupyter #| fig-cap: "Charts" #| fig-subcap: #| - "First" @@ -445,7 +445,7 @@ plt.show() #### Knitr ```{{r}} -#| label: fig-charts +#| label: fig-charts-knitr #| fig-cap: "Charts" #| fig-subcap: #| - "Cars"