diff --git a/Cargo.lock b/Cargo.lock index 23ffc9020..bd19d7448 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3758,9 +3758,9 @@ dependencies = [ [[package]] name = "quarto-error-reporting" -version = "0.1.0" +version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2aa95153b93fea9754e121137d5a6e38e30dd1184c73d2266958f698b8cb9503" +checksum = "9ccd85f4df70f08134ac6b3997c76660e679b01d9c0bfd0f11a68e0cf32442f7" dependencies = [ "ariadne", "quarto-source-map", diff --git a/Cargo.toml b/Cargo.toml index a41aa29a1..9c39b805d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -115,7 +115,7 @@ path = "./crates/quarto-util" version = "0.1.0" [workspace.dependencies.quarto-error-reporting] -version = "0.1.0" +version = "0.2.1" [workspace.dependencies.quarto-error-catalog] path = "./crates/quarto-error-catalog" diff --git a/claude-notes/plans/2026-07-31-repeated-diagnostics-coalescing.md b/claude-notes/plans/2026-07-31-repeated-diagnostics-coalescing.md new file mode 100644 index 000000000..5463301eb --- /dev/null +++ b/claude-notes/plans/2026-07-31-repeated-diagnostics-coalescing.md @@ -0,0 +1,235 @@ +# Coalesce repeated per-page diagnostics in project renders + +**Strand:** bd-mg3ckvp7 +**Related:** bd-9hlja (closed) — built `coalesce_by_source` and wired it for +`pass2_failures` only, explicitly deferring the successful-render case. + +## Overview + +Rendering `external-sources/connect-docs/docs-quarto-2` emits the same +warning once per rendered page when the underlying problem lives in a +*shared* file. Measured on 2026-07-31 (`cargo run --bin q2 -- render …`, +2519 lines of stderr): + +- **186×** `Warning [Q-13-2]: Navbar references missing document + 'api/index.qmd'` — one identical copy per page, all anchored at the + same `_quarto.yml` span. +- Related shapes at smaller counts: `Q-12-7` (template/type fallback, + 15×), listing `sort:` warnings (~10×), and unknown-shortcode warnings + re-reported per includer when the shortcode lives in a shared include + file (`{{< include ../include/_common.qmd >}}`). +- (The bulk of the ~75 unknown-shortcode and ~88 `Q-2-9` warnings are + *distinct* source locations — legitimately separate diagnostics, out + of scope.) + +## Diagnosis + +1. **Each per-document pipeline independently re-diagnoses shared + inputs.** The navbar render transform + (`crates/quarto-core/src/transforms/navbar_render.rs`) runs for every + page and calls `resolve_href_for_html` + (`crates/quarto-core/src/transforms/navigation_href.rs:152`), which + pushes a fresh `Q-13-2` into that page's diagnostics on every miss. + Same story for any diagnostic derived from `_quarto.yml` config or + from included fragments. + +2. **The CLI prints successful-render page diagnostics verbatim, with no + coalescing.** `print_render_diagnostics_text` + (`crates/quarto/src/commands/render.rs:946-954`) loops over + `summary.outputs` and prints each diagnostic. bd-9hlja routed only + `pass2_failures` through `coalesce_by_source`; the comment at + `render.rs:908-914` records the exact blocker: *"`RenderToFileResult` + does not currently carry the input path."* + +3. **The coalescing key already works for this case — verified + empirically.** A temporary probe on a 3-page fixture (broken navbar + href in `_quarto.yml`) showed every page's `Q-13-2` carries an + identical location: + + ``` + location=Some(Substring { parent: Original { file_id: FileId(6053980132863075055), + start_offset: 0, end_offset: 122 }, start_offset: 88, end_offset: 99 }) + resolve_byte_range=Some((6053980132863075055, 88, 99)) + ``` + + `_quarto.yml` values get their `FileId` from + `quarto_yaml::file_id_for_filename` (a hash of the path), so the id + is stable across all per-document `SourceContext`s. + `coalesce_by_source`'s `LocationKey` = `(file_id, start, end)` would + collapse all 186 into one group. + +4. **Hazard: raw `file_id` in the key is unsafe across documents.** + Pampa's per-document contexts use *sequential* FileIds (primary file + = `FileId(0)`). Two page-local diagnostics in *different* documents + at the same byte offsets produce the same `(0, start, end)` key and + would falsely merge. bd-9hlja never hit this because `pass2_failures` + theme errors anchor at hash-based ids. Routing *all* per-page + diagnostics through the upstream coalescer as-is would introduce + false merges. The key must become **(resolved file path when the + entry's own `SourceContext` can name the file, else raw file_id, + start, end)**. Hash-based ids (config files, not registered in + per-doc contexts) fall back to the raw id, which is path-derived and + therefore stable and collision-safe; sequential ids resolve to + distinct paths per document. + +5. **Secondary UX gap: the warning names no source at all.** The + `Q-13-2` text block renders without any file/line/snippet, because + the per-doc `SourceContext` handed to `to_text` doesn't contain + `_quarto.yml` (hash id, never registered). Even a fully coalesced + single warning would still not tell the user *where* the bad href + is. `theme_diagnostic.rs:69` already demonstrates the fix pattern: + read the file, `add_file_with_id(file_id_for_filename(path), …)`. + +## Design decisions (proposed — iterate here) + +- **D1: Fix the key upstream first, then consume it here.** + `coalesce_by_source` lives in `quarto-error-reporting` + (`posit-dev/quarto-error-reporting`, published to crates.io; q2 pins + `0.1.0`). We own the crate, and the path-aware key is a small, + self-contained bug fix: `coalesce_by_source` already receives + `Option` per entry, so `LocationKey` can resolve + `FileId → file path` through the entry's own context (falling back + to the raw id for unregistered hash-based ids) **without any + signature change**. Sequence: file the upstream issue → land the fix + + collision-guard unit tests there → publish `0.1.1` → bump the + version dep here and route the successful-render pool through the + upstream coalescer directly. This avoids writing throwaway local + grouping code and a later migration; the q2-side plumbing (Phases + 1–2) is independent and can proceed in parallel while the release is + in flight. (Rejected alternative, kept for the record: local + grouping in q2 reusing the public `CoalescedDiagnostic` renderer — + only worth it if the upstream release were expensive, which it + isn't for a crate we control.) +- **D2: Print-only change.** `diagnostic_counts()`, `--strict` + promotion, and exit codes keep operating on the un-coalesced + per-page diagnostics. Coalescing affects only the text emission. +- **D3: Sections stay separate in v1.** `pass2_failures` (already + coalesced), `project_diagnostics`, and the new coalesced + successful-render pool print as today's three sections, in that + order. Merging failures + successes into one pool is a possible + follow-up, not v1. +- **D4: `--json-errors` unchanged in v1.** Programmatic consumers get + one `JsonDiagnostic` per page occurrence today; keeping that + preserves per-page attribution (the hub-client overlay depends on + per-page delivery). A future `affected_files` field is a schema + change → separate strand if wanted. +- **D5: Snippet restoration for config-anchored groups** (fixes + §Diagnosis 5): at print time, when a group's representative location + doesn't resolve against its carried `SourceContext`, attempt to + register the project config file(s) (`_quarto.yml`, and the profile + variants in play) under `quarto_yaml::file_id_for_filename` with + content read from disk — mirroring `theme_diagnostic.rs`. Best-effort; + on any failure, render span-less exactly as today. + +## Work items + +### Phase 0 — upstream fix in posit-dev/quarto-error-reporting + +- [x] File the upstream issue: `LocationKey` keys on raw `file_id`; + sequential per-document FileIds falsely merge diagnostics from + different files at identical offsets (see §Diagnosis 4 for the q2 + reproduction context). + Filed: +- [x] Land the fix there (TDD in that repo): path-aware key — + resolve `FileId → path` via the entry's own `SourceContext` when + registered, fall back to raw `file_id` otherwise; no signature + change to `coalesce_by_source`. (Done by another agent; shipped as + **0.2.1**, not 0.1.1 — `FileKey::Path` / `FileKey::Raw` enum, + `LocationKey::from(info, ctx)`.) Unit tests: + - same hash-file-id + span across N entries → one group, N affected + files in encounter order; + - sequential-file-id collision (two contexts, each `FileId(0)`, same + offsets, different registered paths) → **two** groups; + - `location: None` → singleton pass-through; + - mixed pool preserves encounter order. +- [x] Publish to crates.io. (Shipped as `0.2.1`.) + +### Phase 1 — q2-side tests first (TDD; can start in parallel with Phase 0) + +- [x] Integration test (`crates/quarto/tests/integration/` per the + integration-test layout rule) driving the real binary or + `print_render_diagnostics_text`'s input path: 3-page website fixture + with a broken navbar href → exactly **one** `Q-13-2` block on + stderr, with an `Affected files:` tail naming 3 pages. +- [x] Run new tests, verify they fail (red) before implementing. + (Red confirmed: 5 copies / no tail; singleton test passed as expected.) + +### Phase 2 — plumbing (independent of Phase 0) + +- [x] Add the input path to `RenderToFileResult` + (`crates/quarto-core/src/render_to_file.rs:127`), populated where the + orchestrator/render_document_to_file constructs it. (This is the + blocker bd-9hlja recorded.) + +### Phase 3 — coalesced emission (needs Phase 0 published) + +- [x] Bump `quarto-error-reporting` to `0.2.1` in the workspace + `Cargo.toml` (and in `crates/wasm-quarto-hub-client/Cargo.toml`, + which pins it independently — it is outside the workspace). +- [x] Route `summary.outputs[*].render_output.diagnostics` through + `coalesce_by_source` in `print_render_diagnostics_text` (respecting + `--quiet` as today). +- [x] Tests from Phase 1 go green (3/3). + +### Phase 4 — config-file snippet restoration (D5) + +- [x] Best-effort registration of project config sources at print time + so the coalesced `Q-13-2` renders the `_quarto.yml` snippet with the + offending span. (`attach_config_source` in + `crates/quarto/src/commands/render.rs`: matches the group's FileId + against `quarto_yaml::file_id_for_filename(project.config.config_path)` + and registers the file's content under that id; scope is `_quarto.yml` + only for now — open question 3 stands for `_metadata.yml`/profiles.) +- [x] Test: fixture render shows file/line for the navbar href + (`config_anchored_warning_shows_config_snippet`; red confirmed with + the attach call neutralized, green with it active). + +### Phase 5 — verification + +- [x] `cargo build --workspace`, `cargo nextest run --workspace` + (10810 passed), full `cargo xtask verify` (exit 0, covering the + 0.2.1 bump through the WASM/hub legs), and a final + `cargo xtask verify --skip-hub-build` (exit 0) after Phase 4. + One unrelated flake seen once (`collect_reverification_…`, + case-only id mismatch) — filed as bd-gypflveh; 5/5 green in + isolation. +- [x] End-to-end on the testbed: re-rendered + `external-sources/connect-docs/docs-quarto-2` + (`cargo run --bin q2 -- render …`, output inspected). Q-13-2 went + **186 → 1**; stderr 2519 → ~1050 lines; exit code unchanged (1, from + pre-existing Q-5-3 errors). All other repeated classes verified to be + genuinely distinct locations (Q-12-7's 15 hits = 15 distinct files). + Observed emission: + + ``` + Warning: [Q-13-2] Navbar references missing document + ╭─[ …/docs-quarto-2/_quarto.yml:44:15 ] + 44 │ file: api/index.qmd + │ ──────┬────── + │ ╰──────── 'api/index.qmd' is not in the project index. + ────╯ + ℹ Check the spelling, or confirm the target file is included in the render set. + Affected files: …/admin/access-controls/index.qmd, … (and 183 others) + ``` + + Note the snippet now names `_quarto.yml:44` — before this work the + warning carried no source pointer at all. +- [ ] File follow-up strands: optional `--json-errors` affected-files + field; optional single-pool merge (D3). + +## Open questions for review + +1. The `Affected files:` tail lists the *pages* that re-reported the + diagnostic. For a config-anchored warning like Q-13-2, "affected + files" is arguably every page — is the tail even useful there once + the `_quarto.yml` snippet renders (Phase 4)? Alternative: suppress + the tail when the anchor file is a project config file, or reword to + `Reported while rendering: …`. +2. Is `AFFECTED_FILES_CAP = 3` the right display cap (upstream const)? + Local grouping means we could choose our own. +3. Should Phase 4 cover only `_quarto.yml`, or also `_metadata.yml` / + profile configs / `_variables.yml`? (Same mechanism; just a list of + candidate paths.) +4. Priority call: is `q2 preview`'s diagnostic surface in scope? (It + consumes per-page diagnostics through a different path; coalescing + there is a UI concern, likely fine to leave per-page.) diff --git a/crates/quarto-core/src/render_to_file.rs b/crates/quarto-core/src/render_to_file.rs index a3e316777..59cd69526 100644 --- a/crates/quarto-core/src/render_to_file.rs +++ b/crates/quarto-core/src/render_to_file.rs @@ -125,6 +125,11 @@ pub struct RenderToFileOptions { /// Result of rendering a document to a file. #[derive(Debug)] pub struct RenderToFileResult { + /// Path to the input file this result was rendered from, as the + /// caller passed it. Carried so the render summary can attribute + /// per-page diagnostics to their page when coalescing repeated + /// emissions (bd-mg3ckvp7; the missing piece bd-9hlja recorded). + pub input_path: PathBuf, /// Path to the output file. pub output_path: PathBuf, /// Path to the resources directory (e.g., `document_files/`). @@ -416,6 +421,7 @@ pub fn render_document_to_file( let resource_report = std::mem::take(&mut ctx.resource_report); Ok(RenderToFileResult { + input_path: input_path.to_path_buf(), output_path, resources_dir: resource_paths.resource_dir, render_output, diff --git a/crates/quarto/src/commands/render.rs b/crates/quarto/src/commands/render.rs index 9d7c19c23..afc86289f 100644 --- a/crates/quarto/src/commands/render.rs +++ b/crates/quarto/src/commands/render.rs @@ -37,8 +37,8 @@ use quarto_core::project::orchestrator::{ }; use quarto_core::{Format, ProjectContext, QuartoError, RenderToFileOptions}; use quarto_error_reporting::{ - DiagnosticMessage, DiagnosticMessageBuilder, JsonDiagnostic, JsonPass1Failure, - diagnostic_to_json, with_source_file, + CoalescedDiagnostic, DiagnosticMessage, DiagnosticMessageBuilder, JsonDiagnostic, + JsonPass1Failure, diagnostic_to_json, with_source_file, }; use quarto_source_map::SourceContext; use quarto_system_runtime::{NativeRuntime, SystemRuntime}; @@ -694,6 +694,9 @@ fn execute_single_doc( let runtime_arc: Arc = Arc::new(NativeRuntime::new()); let mut project = ProjectContext::discover(&input, runtime_arc.as_ref()) .context("Failed to discover project context")?; + // Captured before the pipeline mutably borrows `project`; used to + // restore config-anchored source snippets at print time. + let config_path = project.config.config_path.clone(); quarto_util::user_status!(args.quiet, "Rendering single file: {}", input.display()); @@ -727,7 +730,7 @@ fn execute_single_doc( summary.promote_warnings_to_errors(); } - print_render_diagnostics(&summary, args); + print_render_diagnostics(&summary, args, config_path.as_deref()); // bd-ooleh: a single-file render has no "Rendered N of M" line to // augment, so the error/warning counts get their own line — printed @@ -765,6 +768,9 @@ fn execute_project( let mut project = ProjectContext::discover(&project_dir, runtime_arc.as_ref()) .context("Failed to discover project context")?; + // Captured before the pipeline mutably borrows `project`; used to + // restore config-anchored source snippets at print time. + let config_path = project.config.config_path.clone(); quarto_util::user_status!( args.quiet, @@ -809,7 +815,7 @@ fn execute_project( summary.promote_warnings_to_errors(); } - print_render_diagnostics(&summary, args); + print_render_diagnostics(&summary, args, config_path.as_deref()); let rendered = summary.outputs.len(); if let Some(mut line) = render_summary_line(false, total_files, rendered, &project.output_dir) { @@ -868,11 +874,12 @@ fn should_exit_nonzero fn print_render_diagnostics( summary: &quarto_core::project::orchestrator::ProjectRenderSummary, args: &RenderArgs, + config_path: Option<&Path>, ) { if args.json_errors { print_render_diagnostics_json(summary); } else { - print_render_diagnostics_text(summary, args.quiet); + print_render_diagnostics_text(summary, args.quiet, config_path); } // bd-c5u2g: emit per-process engine-discovery counters when @@ -888,9 +895,54 @@ fn print_render_diagnostics( /// Text path: the existing ariadne-formatted output. Kept verbatim /// from before bd-iey8o; the only change is that the perf-stats /// emission moved one level up into `print_render_diagnostics`. +/// bd-mg3ckvp7 Phase 4: a config-anchored diagnostic (e.g. a Q-13-2 +/// navbar miss) carries a location whose FileId is quarto_yaml's hash +/// of the config path, but per-document `SourceContext`s never +/// register that file — so the ariadne snippet silently dropped and +/// the warning named no source at all. Best-effort repair at print +/// time: when the group's location doesn't resolve in its carried +/// context and the FileId matches the project config file, register +/// the config's content under that id so the snippet renders. Any +/// failure (no config, hash mismatch, unreadable file) leaves the +/// group unchanged — span-less render, exactly as before. +fn attach_config_source(group: &mut CoalescedDiagnostic, config_path: Option<&Path>) { + use quarto_source_map::FileId; + + let Some(config_path) = config_path else { + return; + }; + let Some(loc) = group.representative.location.as_ref() else { + return; + }; + let Some((fid, _, _)) = loc.resolve_byte_range() else { + return; + }; + if group + .source_context + .as_ref() + .is_some_and(|c| c.get_file(FileId(fid)).is_some()) + { + return; + } + // `parse_config` hashed `config_path.to_string_lossy()`; only a + // diagnostic actually anchored in this config file matches. + let config_str = config_path.to_string_lossy(); + if quarto_yaml::file_id_for_filename(&config_str) != FileId(fid) { + return; + } + let Ok(content) = std::fs::read_to_string(config_path) else { + return; + }; + group + .source_context + .get_or_insert_with(SourceContext::new) + .add_file_with_id(FileId(fid), config_str.into_owned(), Some(content)); +} + fn print_render_diagnostics_text( summary: &quarto_core::project::orchestrator::ProjectRenderSummary, quiet: bool, + config_path: Option<&Path>, ) { for failure in &summary.pass1_failures { eprintln!( @@ -904,14 +956,6 @@ fn print_render_diagnostics_text( // emits one report listing affected pages rather than N copies. // Non-structured failures (no diagnostics) take the legacy // single-line form as before. - // - // Per-page warning diagnostics from successful renders - // (`outputs[i].render_output.diagnostics`) are not coalesced - // here in v1 because `RenderToFileResult` does not currently - // carry the input path; the theme-error use case lives entirely - // in `pass2_failures`. A follow-up could add `input_path` to - // `RenderToFileResult` and route the successful-render - // diagnostics through the coalescer too. { use quarto_error_reporting::coalesce_by_source; @@ -943,20 +987,34 @@ fn print_render_diagnostics_text( eprintln!("{}", diagnostic.to_text(None)); } - for result in &summary.outputs { - if !quiet && !result.render_output.diagnostics.is_empty() { - for diagnostic in &result.render_output.diagnostics { - eprintln!( - "{}", - diagnostic.to_text(Some(&result.render_output.source_context)) - ); - } + // bd-mg3ckvp7: per-page diagnostics from *successful* renders go + // through the same source-location coalescer as pass2_failures, so + // a problem anchored in a shared file (a broken navbar href in + // `_quarto.yml`, a bad shortcode in a shared include) emits once + // with an "Affected files:" tail instead of once per page. + // Coalescing is print-only: `diagnostic_counts()` / `--strict` + // promotion ran on the un-coalesced per-page diagnostics above. + if !quiet { + use quarto_error_reporting::coalesce_by_source; + + let entries = summary.outputs.iter().flat_map(|result| { + result.render_output.diagnostics.iter().map(|d| { + ( + result.input_path.clone(), + d.clone(), + Some(result.render_output.source_context.clone()), + ) + }) + }); + for mut group in coalesce_by_source(entries) { + attach_config_source(&mut group, config_path); + eprintln!("{}", group.to_text()); } // Per-file output line stays on `tracing::info!` (opt-in via // `-v`); enumerating every file is too noisy for a large // site, and the post-render summary covers the common case. - if !quiet { + for result in &summary.outputs { info!("Output: {}", result.output_path.display()); } } diff --git a/crates/quarto/tests/integration/coalesced_diagnostics.rs b/crates/quarto/tests/integration/coalesced_diagnostics.rs new file mode 100644 index 000000000..ff572f3ae --- /dev/null +++ b/crates/quarto/tests/integration/coalesced_diagnostics.rs @@ -0,0 +1,214 @@ +//! End-to-end CLI tests for coalescing repeated per-page diagnostics +//! (bd-mg3ckvp7). +//! +//! When a single underlying problem lives in a file shared by every +//! page — the motivating case is a broken navbar `href:` in +//! `_quarto.yml` — each per-document pipeline re-diagnoses it and the +//! render summary used to print one identical warning per page (186 +//! copies on the connect-docs testbed). These tests pin the coalesced +//! behavior: one emission per distinct source span, with an +//! `Affected files:` tail listing the pages that re-reported it. +//! +//! Contract under verification (per +//! `claude-notes/plans/2026-07-31-repeated-diagnostics-coalescing.md`): +//! - A config-anchored warning (same `_quarto.yml` span reported by N +//! pages) prints exactly once, with all N pages in the tail. +//! - A warning reported by a single page prints without any +//! `Affected files:` tail (legacy shape preserved). +//! - Coalescing is print-only: exit codes are unchanged (warnings +//! still exit 0 without `--strict`). +//! +//! TDD note: written before the implementation; the first test must +//! fail (3 copies, no tail) before Phase 2/3 of the plan start. + +use std::path::Path; +use std::process::Command; + +use tempfile::TempDir; + +const Q2_BIN: &str = env!("CARGO_BIN_EXE_q2"); + +fn write_file(path: &Path, contents: &str) { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).unwrap(); + } + std::fs::write(path, contents).unwrap(); +} + +/// Strip ANSI escape sequences (CSI color codes and OSC-8 hyperlinks) +/// so assertions can match the plain text of ariadne-rendered +/// snippets, which interleave color codes inside highlighted spans. +fn strip_ansi(s: &str) -> String { + let mut out = String::new(); + let mut chars = s.chars(); + while let Some(c) = chars.next() { + if c != '\u{1b}' { + out.push(c); + continue; + } + match chars.next() { + // CSI: `ESC [ ... ` + Some('[') => { + for n in chars.by_ref() { + if n.is_ascii_alphabetic() { + break; + } + } + } + // OSC: `ESC ] ... (BEL | ESC \)` + Some(']') => { + while let Some(n) = chars.next() { + if n == '\u{7}' { + break; + } + if n == '\u{1b}' { + chars.next(); + break; + } + } + } + _ => {} + } + } + out +} + +/// Run `q2 render .` from `cwd` and return (exit-success, stderr with +/// ANSI escapes stripped). +fn run_q2_render(cwd: &Path) -> (bool, String) { + let output = Command::new(Q2_BIN) + .current_dir(cwd) + .args(["render", "."]) + .output() + .expect("spawn q2 binary"); + ( + output.status.success(), + strip_ansi(&String::from_utf8_lossy(&output.stderr)), + ) +} + +/// A website project whose navbar references a document that is not +/// in the render set. Every page's navbar-render transform reports +/// the miss at the same `_quarto.yml` span. +fn write_broken_navbar_project(dir: &Path, pages: &[&str]) { + write_file( + &dir.join("_quarto.yml"), + "project:\n type: website\n output-dir: _site\nwebsite:\n title: Coalesce\n navbar:\n left:\n - href: missing.qmd\n text: Missing\n", + ); + for page in pages { + write_file( + &dir.join(format!("{page}.qmd")), + &format!("---\ntitle: {page}\n---\n\nBody of {page}.\n"), + ); + } +} + +/// Three pages re-reporting the same `_quarto.yml` navbar miss must +/// produce exactly one Q-13-2 emission, tailed by all three pages. +#[test] +fn config_anchored_warning_coalesces_across_pages() { + let dir = TempDir::new().unwrap(); + write_broken_navbar_project(dir.path(), &["index", "a", "b"]); + + let (success, stderr) = run_q2_render(dir.path()); + assert!( + success, + "warnings alone must not fail the render:\n{stderr}" + ); + + let q13_count = stderr.matches("Q-13-2").count(); + assert_eq!( + q13_count, 1, + "expected exactly one coalesced Q-13-2 emission, got {q13_count}:\n{stderr}" + ); + + let tail = stderr + .lines() + .find(|l| l.starts_with("Affected files:")) + .unwrap_or_else(|| panic!("expected an `Affected files:` tail:\n{stderr}")); + for page in ["index.qmd", "a.qmd", "b.qmd"] { + assert!(tail.contains(page), "tail should name {page}; got: {tail}"); + } +} + +/// More affected pages than the display cap: the tail lists the cap's +/// worth of names and summarizes the rest as `(and N other…)`. +#[test] +fn affected_files_tail_caps_long_lists() { + let dir = TempDir::new().unwrap(); + write_broken_navbar_project(dir.path(), &["index", "a", "b", "c", "d"]); + + let (success, stderr) = run_q2_render(dir.path()); + assert!( + success, + "warnings alone must not fail the render:\n{stderr}" + ); + + assert_eq!( + stderr.matches("Q-13-2").count(), + 1, + "expected exactly one coalesced Q-13-2 emission:\n{stderr}" + ); + let tail = stderr + .lines() + .find(|l| l.starts_with("Affected files:")) + .unwrap_or_else(|| panic!("expected an `Affected files:` tail:\n{stderr}")); + assert!( + tail.contains("(and 2 others)"), + "5 pages with a 3-name cap should summarize 2 more; got: {tail}" + ); +} + +/// The coalesced config-anchored warning renders the `_quarto.yml` +/// source snippet (bd-mg3ckvp7 Phase 4 / plan D5). The warning's +/// location FileId is quarto_yaml's hash of the config path, which no +/// per-document `SourceContext` registers — so before Phase 4 the +/// block printed with no file, line, or snippet at all, leaving the +/// user with no pointer to the offending line. +#[test] +fn config_anchored_warning_shows_config_snippet() { + let dir = TempDir::new().unwrap(); + write_broken_navbar_project(dir.path(), &["index", "a", "b"]); + + let (success, stderr) = run_q2_render(dir.path()); + assert!( + success, + "warnings alone must not fail the render:\n{stderr}" + ); + + assert!( + stderr.contains("_quarto.yml"), + "coalesced Q-13-2 should name _quarto.yml as its source:\n{stderr}" + ); + // The YAML source line itself, which only appears if the ariadne + // snippet rendered (the problem text says 'missing.qmd' but never + // `href: missing.qmd`). + assert!( + stderr.contains("href: missing.qmd"), + "coalesced Q-13-2 should render the offending YAML line:\n{stderr}" + ); +} + +/// A single-page project still emits the warning once, with no +/// `Affected files:` tail — the legacy single-page shape. +#[test] +fn singleton_warning_keeps_legacy_shape() { + let dir = TempDir::new().unwrap(); + write_broken_navbar_project(dir.path(), &["index"]); + + let (success, stderr) = run_q2_render(dir.path()); + assert!( + success, + "warnings alone must not fail the render:\n{stderr}" + ); + + assert_eq!( + stderr.matches("Q-13-2").count(), + 1, + "single page emits the warning exactly once:\n{stderr}" + ); + assert!( + !stderr.contains("Affected files:"), + "singleton group must not grow a tail:\n{stderr}" + ); +} diff --git a/crates/quarto/tests/integration/main.rs b/crates/quarto/tests/integration/main.rs index e31fbb924..f26244de5 100644 --- a/crates/quarto/tests/integration/main.rs +++ b/crates/quarto/tests/integration/main.rs @@ -3,6 +3,7 @@ pub mod attribution_cli_e2e; pub mod bootstrap_sh; +pub mod coalesced_diagnostics; pub mod create; pub mod get_config_cli; pub mod json_errors; diff --git a/crates/wasm-quarto-hub-client/Cargo.lock b/crates/wasm-quarto-hub-client/Cargo.lock index 2f1dd1bf7..c68a00b45 100644 --- a/crates/wasm-quarto-hub-client/Cargo.lock +++ b/crates/wasm-quarto-hub-client/Cargo.lock @@ -2226,9 +2226,9 @@ dependencies = [ [[package]] name = "quarto-error-reporting" -version = "0.1.0" +version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2aa95153b93fea9754e121137d5a6e38e30dd1184c73d2266958f698b8cb9503" +checksum = "9ccd85f4df70f08134ac6b3997c76660e679b01d9c0bfd0f11a68e0cf32442f7" dependencies = [ "ariadne", "quarto-source-map", diff --git a/crates/wasm-quarto-hub-client/Cargo.toml b/crates/wasm-quarto-hub-client/Cargo.toml index d1fda9ee5..1178c5d91 100644 --- a/crates/wasm-quarto-hub-client/Cargo.toml +++ b/crates/wasm-quarto-hub-client/Cargo.toml @@ -17,7 +17,7 @@ quarto-core = { path = "../quarto-core" } # NOTE: `quarto-error-catalog` is intentionally NOT a dependency here — the WASM # bridge never surfaces docs URLs, so installing the catalog would only bloat the # bundle past the PWA precache limit. See the comment in `init()` in src/lib.rs. -quarto-error-reporting = { version = "0.1.0", features = ["json"] } +quarto-error-reporting = { version = "0.2.1", features = ["json"] } # Direct dep so the test-only `quarto_highlight_for_test` export can # call the Registry-backed highlight path. Same crate quarto-core uses # via CodeHighlightStage — ensuring WASM tests exercise the same code