Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions src/commands/coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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)]
Expand Down
112 changes: 49 additions & 63 deletions src/commands/trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
///
Expand All @@ -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<String> = ids
let boundary_matched: Vec<String> = ids
.into_iter()
.filter(|id| prefix_matches(id, prefix))
.map(String::from)
Expand All @@ -448,7 +453,7 @@ fn search_by_prefix(
let mut implements = Vec::new();
let mut citations = Vec::new();
let mut searched_normalized: BTreeSet<String> = 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
Expand All @@ -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<String> = 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::<BTreeSet<&str>>()
.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()),
Expand All @@ -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<String> {
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`
Expand All @@ -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[..]
}
Expand Down Expand Up @@ -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"));
Expand All @@ -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::<String>::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::<String>::new());
}

#[test]
Expand Down
8 changes: 8 additions & 0 deletions src/commands/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`): `<scope>`, `<scope>/.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<Registry> {
// 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.
Expand Down
74 changes: 74 additions & 0 deletions tests/cli_coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1313,6 +1313,80 @@ fn it108_a_declared_source_exclude_scopes_the_code_walk() {
);
}

// PLAT-179 (quire-cli#80): with `--module` omitted, `coverage --scope <repo>`
// discovers modules exactly as `validate` does, which includes
// `<scope>/.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<String> = 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 {
Expand Down
Loading
Loading