diff --git a/src/commands/coverage.rs b/src/commands/coverage.rs index 88cb292..bf1d12e 100644 --- a/src/commands/coverage.rs +++ b/src/commands/coverage.rs @@ -15,7 +15,6 @@ use std::path::{Path, PathBuf}; use anyhow::Context; use clap::Parser; -use quire_cli::io; use quire_cli::safety; use quire_rs::grammar::{GrammarSeverityLevel, GrammarSeverityMap}; use quire_rs::{compute_coverage, Registry, Spec}; @@ -789,11 +788,7 @@ pub(super) fn load_registry_for( if scope.join("manifest.yaml").is_file() { return super::load_module_registry(ctx, scope); } - let registry = Registry::from_env() - .map_err(super::failure::invalid) - .context("loading modules")?; - io::emit_quire_diagnostics(ctx.diagnostics, registry.diagnostics()); - Ok(registry) + super::validate::load_scoped_with_defaults(ctx, scope) } #[cfg(test)] diff --git a/src/commands/trace.rs b/src/commands/trace.rs index 0dd2b8f..92de8e0 100644 --- a/src/commands/trace.rs +++ b/src/commands/trace.rs @@ -56,7 +56,9 @@ use clap::Parser; use serde::Serialize; use quire_cli::safety; -use quire_rs::symbols::trace::{ImplementsRelation, Mention, SymbolGraph, VerifiesRelation}; +use quire_rs::symbols::trace::{ + trace_id_segments, ImplementsRelation, Mention, SymbolGraph, VerifiesRelation, +}; use quire_rs::symbols::trace_search::{self, language_confidence, symbol_language, Query}; use quire_rs::symbols::SymbolExtraction; @@ -140,9 +142,12 @@ pub enum OutputFormat { } /// What was asked, echoed back so a JSON consumer never has to re-derive it -/// from argv. `matched_ids` is populated only under `--prefix`: it is the set -/// of concrete ids the prefix actually matched, present so a caller can see -/// exactly what got unioned into one result. +/// from argv. `matched_ids` is populated only under `--prefix`: every concrete +/// id spelling present in the result rows, so a caller can see exactly what +/// got unioned into one result. It is the ids the prefix matched by segment +/// boundary plus any other spelling the engine's exact `Query::Id` search +/// folded onto them (`FR047` folds onto `FR-047`), so it always covers the +/// payload it describes. #[derive(Debug, Clone, Serialize)] #[serde(tag = "kind", rename_all = "snake_case")] enum QueryDescription { @@ -417,7 +422,7 @@ fn describe_query(query: &Query) -> QueryDescription { } /// `--id --prefix`: union every distinct id in the graph whose -/// [`id_segments`] start with `prefix`'s segments, running the engine's own +/// [`trace_id_segments`] start with `prefix`'s segments, running the engine's own /// exact `Query::Id` search once per **normalized-equivalence class** of /// matched id and concatenating. /// @@ -438,7 +443,7 @@ fn search_by_prefix( ids.extend(graph.verifies.iter().map(|v| v.trace_id.as_str())); ids.extend(graph.implements.iter().map(|i| i.trace_id.as_str())); ids.extend(graph.mentions.iter().map(|m| m.trace_id.as_str())); - let matched_ids: Vec = ids + let boundary_matched: Vec = ids .into_iter() .filter(|id| prefix_matches(id, prefix)) .map(String::from) @@ -448,7 +453,7 @@ fn search_by_prefix( let mut implements = Vec::new(); let mut citations = Vec::new(); let mut searched_normalized: BTreeSet = BTreeSet::new(); - for id in &matched_ids { + for id in &boundary_matched { // The one representative per normalized-equivalence class. A second // raw spelling that normalizes the same is skipped here, not // searched-then-deduplicated after the fact — the engine's search @@ -462,6 +467,19 @@ fn search_by_prefix( implements.extend(r.implements); citations.extend(r.citations); } + // `Query::Id` matches by the engine's fold, which discards separators, so + // it can return rows spelled differently from every boundary match. + // Report those spellings too: `matched_ids` must cover its payload. + let matched_ids: Vec = boundary_matched + .iter() + .map(String::as_str) + .chain(verifies.iter().map(|v| v.trace_id.as_str())) + .chain(implements.iter().map(|i| i.trace_id.as_str())) + .chain(citations.iter().map(|m| m.trace_id.as_str())) + .collect::>() + .into_iter() + .map(String::from) + .collect(); let resolved = !(verifies.is_empty() && implements.is_empty() && citations.is_empty()); let result = trace_search::SearchResult { query: Query::Id(prefix.to_string()), @@ -481,59 +499,24 @@ fn search_by_prefix( ) } -/// Split `value` on runs of non-alphanumeric characters and uppercase each -/// resulting piece, dropping empty pieces (so a leading/trailing separator — -/// `FR-047-` — contributes nothing, and consecutive separators collapse). -/// -/// This is character-for-character the same fold quire-rs's own -/// `normalized_trace_id` applies (`src/symbols/trace.rs:871`: filter -/// `is_ascii_alphanumeric`, uppercase) — that function is `pub(crate)` and -/// unreachable from here, so this duplicates its per-character rule rather -/// than sharing it (asked for during review; requesting the export be made -/// `pub` upstream is the preferred long-term fix and does not, by itself, -/// replace this function even if granted — see below). It is deliberately -/// NOT a drop-in reimplementation of `normalized_trace_id` itself: that -/// function concatenates every alphanumeric character into one string, -/// discarding exactly the segment boundaries `--prefix` needs to tell -/// `FR-047` (must match `FR-047-AC-1`) apart from `FR-0470` (must not). No -/// function in the engine currently preserves those boundaries — boundary- -/// aware prefix matching is CLI-only logic by design (quire-rs -/// `traceability.rs`: the engine knows nothing of FR/AC/TC hierarchy). -/// -/// This duplication is a liability by itself: if quire-rs ever changes what -/// `normalized_trace_id` folds (a new character class, different case -/// handling), this copy keeps the OLD rule and `--prefix` silently diverges -/// from exact `--id` again, with every test in this file staying green -/// because they all check this copy against itself. The integration test -/// `prefix_fold_tracks_the_engines_own_normalization_across_spellings` in -/// `tests/cli_trace.rs` is the guard against that: it measures the engine's -/// fold behaviorally (through exact `--id`, since `normalized_trace_id` -/// itself is `pub(crate)` and unreachable) and this crate's fold -/// behaviorally (through `--prefix`) over the same spelling table, so a -/// drift between the two shows up as a named, failing assertion instead of -/// silence. Do not replace that test with a doc comment restating this -/// property — an assertion the code does not enforce is what this note -/// itself would become. -fn id_segments(value: &str) -> Vec { - value - .split(|c: char| !c.is_ascii_alphanumeric()) - .filter(|s| !s.is_empty()) - .map(str::to_ascii_uppercase) - .collect() -} - -/// The engine's own normalized id, reproduced by concatenating -/// [`id_segments`] — used only to tell whether two RAW spellings are the +/// The engine's normalized id: the concatenation of its own +/// [`trace_id_segments`]. Used only to tell whether two RAW spellings are the /// same id as far as `Query::Id`'s own match is concerned (H2's dedup key), -/// never to decide `--prefix` boundaries (which need the segments, not the -/// concatenation — see [`id_segments`]). +/// never to decide `--prefix` boundaries, which need the segments. fn normalized_id(value: &str) -> String { - id_segments(value).concat() + trace_id_segments(value).concat() } -/// Segment-boundary prefix match, fold-consistent with the engine's own -/// `normalized_trace_id` equality: `prefix` matches `candidate` when -/// `candidate`'s [`id_segments`] start with `prefix`'s. +/// Segment-boundary prefix match: `prefix` matches `candidate` when +/// `candidate`'s [`trace_id_segments`] (the engine's own fold, shared rather +/// than copied) start with `prefix`'s. +/// +/// Known divergence from the engine's exact `--id`, which compares the +/// CONCATENATED fold: this match is sensitive to where alphanumeric runs +/// break. `FR047` is one segment (`["FR047"]`), so the engine treats it as +/// `FR-047` for `--id` but `--prefix FR-047` does not match `FR047-AC-1`. +/// Closing that gap would re-admit the `FR-0470` decoy (`"FR0470"` starts +/// with `"FR047"`), so the boundary rule wins. /// /// `FR-047` (segments `["FR","047"]`) matches `FR-047-AC-1` /// (`["FR","047","AC","1"]`, a genuine extra segment) and `FR_047_AC_9` @@ -544,11 +527,11 @@ fn normalized_id(value: &str) -> String { /// segments, not merely as a string-prefix check within one segment) — the /// property this flag exists to guarantee. fn prefix_matches(candidate: &str, prefix: &str) -> bool { - let prefix_segments = id_segments(prefix); + let prefix_segments = trace_id_segments(prefix); if prefix_segments.is_empty() { return false; } - let candidate_segments = id_segments(candidate); + let candidate_segments = trace_id_segments(candidate); candidate_segments.len() >= prefix_segments.len() && candidate_segments[..prefix_segments.len()] == prefix_segments[..] } @@ -920,7 +903,7 @@ mod tests { // Review finding H1: the previous raw, case-sensitive `strip_prefix` // diverged from the engine's own `normalized_trace_id` fold in three - // reproduced ways. Each of these failed before the `id_segments` rewrite. + // reproduced ways. Each of these failed before the former `id_segments` rewrite. #[test] fn tc_prefix_matches_folds_case_like_the_engine() { assert!(prefix_matches("FR-047", "fr-047")); @@ -946,10 +929,13 @@ mod tests { #[test] fn tc_id_segments_splits_on_non_alphanumeric_runs_and_uppercases() { - assert_eq!(id_segments("FR-047-AC-1"), vec!["FR", "047", "AC", "1"]); - assert_eq!(id_segments("fr_047"), vec!["FR", "047"]); - assert_eq!(id_segments("FR-047-"), vec!["FR", "047"]); - assert_eq!(id_segments(""), Vec::::new()); + assert_eq!( + trace_id_segments("FR-047-AC-1"), + vec!["FR", "047", "AC", "1"] + ); + assert_eq!(trace_id_segments("fr_047"), vec!["FR", "047"]); + assert_eq!(trace_id_segments("FR-047-"), vec!["FR", "047"]); + assert_eq!(trace_id_segments(""), Vec::::new()); } #[test] diff --git a/src/commands/validate.rs b/src/commands/validate.rs index 0242082..6199641 100644 --- a/src/commands/validate.rs +++ b/src/commands/validate.rs @@ -460,6 +460,14 @@ pub(crate) fn load_registry(ctx: &Ctx, args: &Args, scope: &Path) -> anyhow::Res return super::load_module_registry(ctx, scope); } + load_scoped_with_defaults(ctx, scope) +} + +/// Scoped module discovery shared by every command whose `--module` is +/// omitted (`validate`, `coverage`, `symbols`): ``, `/.ix/modules`, +/// the module env vars and the default install root, with a one-shot lazy +/// install of the default set when nothing is found. +pub(crate) fn load_scoped_with_defaults(ctx: &Ctx, scope: &Path) -> anyhow::Result { // Scoped discovery. Load once; if nothing is found, lazy-install the // default module set via quoin and reload exactly once before failing — // so a fresh machine validates without any manual `quoin` step or env var. diff --git a/tests/cli_coverage.rs b/tests/cli_coverage.rs index 5ccaf77..f8725df 100644 --- a/tests/cli_coverage.rs +++ b/tests/cli_coverage.rs @@ -1313,6 +1313,80 @@ fn it108_a_declared_source_exclude_scopes_the_code_walk() { ); } +// PLAT-179 (quire-cli#80): with `--module` omitted, `coverage --scope ` +// discovers modules exactly as `validate` does, which includes +// `/.ix/modules`. It used to read only the ambient env, so a repo-local +// module's `source_exclude` never reached symbol extraction. Ambient discovery +// is emptied here (no env paths, an empty HOME), so the only place the module +// can come from is the repository itself. +#[test] +fn plat179_scoped_coverage_discovers_repo_local_ix_modules() { + let dir = TempDir::new().expect("tempdir"); + let home = TempDir::new().expect("home"); + let local = dir.path().join(".ix/modules/local"); + fs::create_dir_all(&local).expect("mkdir module"); + fs::write( + local.join("manifest.yaml"), + "name: local\nmanifest_version: 1.0.0\nversion: 0.0.1\nartifact_types:\n\ + - name: TestMatrix\n\ + traceability:\n source_exclude: ['vendor/**']\n trace_targets:\n - name: test-case\n\ + \x20 archetype: TestMatrix\n section: Test Case Summary\n\ + \x20 id_column: Test ID\n status:\n column: Status\n\ + \x20 complete: [\"\u{2705}\"]\n pending: [\"\u{1F6A7}\"]\n\ + \x20 failed: [\"\u{274C}\"]\n retired: [\"\u{26D4}\"]\n\ + \x20 trace_tags:\n legacy:\n - name: comment-id\n\ + \x20 pattern: '(?://|#)\\s*((?:TC|FR)-\\d+)'\n", + ) + .expect("write manifest"); + fs::create_dir_all(dir.path().join("spec")).expect("mkdir spec"); + fs::create_dir_all(dir.path().join("vendor")).expect("mkdir vendor"); + fs::create_dir_all(dir.path().join("tests")).expect("mkdir tests"); + fs::write( + dir.path().join("spec/tests.md"), + matrix_doc("TM-001", "TC-001"), + ) + .expect("write matrix"); + fs::write( + dir.path().join("vendor/dep.rs"), + "// TC-999\n#[test]\nfn tc999_vendored() {}\n", + ) + .expect("write vendored"); + fs::write( + dir.path().join("tests/real.rs"), + "// TC-998\n#[test]\nfn tc998_real() {}\n", + ) + .expect("write source"); + + let scope = dir.path().to_string_lossy().into_owned(); + let out = quire() + .args(["coverage", "--scope", &scope, "--json"]) + .env("HOME", home.path()) + .env_remove("IX_FILAMENT_MODULES_PATH") + .env_remove("IX_SCHEMA_PATH") + .output() + .expect("run"); + assert!( + out.status.success(), + "repo-local module was not discovered: {}", + String::from_utf8_lossy(&out.stderr) + ); + let report: serde_json::Value = serde_json::from_slice(&out.stdout).expect("json"); + let ids: Vec = report["untracked_symbols"] + .as_array() + .expect("array") + .iter() + .filter_map(|s| s["trace_id"].as_str().map(str::to_string)) + .collect(); + assert!( + !ids.contains(&"TC-999".to_string()), + "the repo-local source_exclude did not reach the walk: {ids:?}" + ); + assert!( + ids.contains(&"TC-998".to_string()), + "real source lost: {ids:?}" + ); +} + /// The it107 manifest (self-referencing TC ids, declared status vocabulary, /// legacy comment trace tags), reusable by the #51-batch tests below. fn self_ref_manifest() -> &'static str { diff --git a/tests/cli_trace.rs b/tests/cli_trace.rs index 885dda6..02ff003 100644 --- a/tests/cli_trace.rs +++ b/tests/cli_trace.rs @@ -397,29 +397,12 @@ fn prefix_does_not_double_count_ids_that_normalize_the_same() { ); } -// Coordinator follow-up on H1/H2: `id_segments` duplicates the engine's -// `normalized_trace_id` fold rather than sharing it (that function is -// `pub(crate)`, and boundary-aware `--prefix` needs the segment structure it -// discards anyway). A duplicated invariant with nothing comparing the two -// copies is exactly the defect class this campaign exists to kill — if -// quire-rs ever changes its fold, this crate's copy keeps the old rule and -// `--prefix` silently diverges from exact `--id` again, with every other -// test in this file staying green because they all check this crate's fold -// against itself. -// -// This test measures BOTH folds independently and behaviorally, never by -// calling the unreachable `normalized_trace_id`: -// - the ENGINE's fold, observed through exact `--id` (which already -// case/separator-folds internally, per the HIGH 1 repro: `--id fr-047` -// found `FR-047`); -// - THIS CRATE's fold, observed through `--prefix` (`id_segments` / -// `prefix_matches`). -// Over a table of five spellings of `FR-047` — including one that mixes -// separator characters within a single spelling (`FR_047-`: underscore -// internally, dash trailing) — plus the `FR-0470` decoy, which must never -// enter either set. If the engine's rule ever moves, the exact-`--id` -// assertions below change while the `--prefix` assertions do not, and this -// test fails naming the diverging spelling. +// PLAT-898: `--prefix` no longer carries its own copy of the engine's trace-id +// fold; it calls quire-rs's exported `trace_id_segments`, the single +// implementation `normalized_trace_id` is built from. This test still pins the +// observable agreement: exact `--id` (the engine's concatenated fold) and +// `--prefix` (its segment fold) must resolve every separator/case spelling of +// `FR-047` to the same family, and the `FR-0470` decoy must stay out of both. #[test] fn prefix_fold_tracks_the_engines_own_normalization_across_spellings() { let dir = TempDir::new().expect("tempdir"); @@ -483,6 +466,76 @@ fn prefix_fold_tracks_the_engines_own_normalization_across_spellings() { } } +// PLAT-908: the one known place `--prefix` and exact `--id` disagree. The +// engine folds by concatenation, so `FR047` is `FR-047` to `--id`; `--prefix` +// compares alphanumeric-run segments (the only way to keep `FR-0470` out), so +// `FR047` is the single segment `FR047` and `FR047-AC-1` is not under the +// prefix `FR-047`. This pins that documented behaviour so a change to either +// fold shows up here, and checks `matched_ids` still covers every id the +// payload returns (it used to omit `FR047`, which the engine folded in). +#[test] +fn prefix_known_divergence_for_ids_without_a_separator() { + let dir = TempDir::new().expect("tempdir"); + let src = dir.path().join("src"); + fs::create_dir_all(&src).expect("mkdir src"); + fs::write( + src.join("lib.rs"), + "#[trace(\"FR-047\")]\n#[test]\nfn tc_dash() {}\n\n\ + #[trace(\"FR047\")]\n#[test]\nfn tc_no_separator() {}\n\n\ + #[trace(\"FR047-AC-1\")]\n#[test]\nfn tc_no_separator_child() {}\n", + ) + .expect("write lib.rs"); + let scope = dir.path().to_string_lossy().into_owned(); + let m = module(&dir); + let ids_of = |payload: &serde_json::Value| -> Vec { + let mut v: Vec = payload["claims"]["verifies"] + .as_array() + .expect("verifies") + .iter() + .map(|r| r["trace_id"].as_str().unwrap().to_string()) + .collect(); + v.sort(); + v + }; + let run_json = |extra: &[&str]| -> serde_json::Value { + let mut args = vec!["trace", "--scope", &scope, "--module", &m, "--json"]; + args.extend_from_slice(extra); + let out = run(&args); + assert_eq!(out.code, Some(0), "stderr: {}", out.stderr); + serde_json::from_str(&out.stdout).expect("json") + }; + + // Engine fold: `FR047-AC-1` IS `FR-047-AC-1`, and FR047 IS FR-047. + let child = run_json(&["--id", "FR-047-AC-1"]); + assert_eq!(ids_of(&child), vec!["FR047-AC-1".to_string()], "{child}"); + let exact = run_json(&["--id", "FR-047"]); + assert_eq!( + ids_of(&exact), + vec!["FR-047".to_string(), "FR047".to_string()], + "{exact}" + ); + + // Prefix fold: the boundary-differing child is NOT in the family. + let prefixed = run_json(&["--id", "FR-047", "--prefix"]); + assert_eq!( + ids_of(&prefixed), + vec!["FR-047".to_string(), "FR047".to_string()], + "known divergence changed: {prefixed}" + ); + let matched: Vec = prefixed["query"]["matched_ids"] + .as_array() + .expect("matched_ids") + .iter() + .map(|v| v.as_str().unwrap().to_string()) + .collect(); + for id in ids_of(&prefixed) { + assert!( + matched.contains(&id), + "matched_ids {matched:?} omits {id} which the payload returned" + ); + } +} + #[test] fn ambiguous_bare_symbol_name_lists_every_candidate() { // Upstream trace-search criterion 4: never a silent pick.