From 329103ddd03189764fff4b7db2e5ff6cafb62a47 Mon Sep 17 00:00:00 2001 From: Teakowa <27560638+Teakowa@users.noreply.github.com> Date: Tue, 29 Sep 2026 18:02:26 +0800 Subject: [PATCH 1/2] feat(driver): share finding selection across CLI and agent surfaces Finding selection (severity threshold, rule id, file, max) is implemented once in wright-driver and applied to envelope diagnostics and lint findings only after the verdict and exit code are fixed on the complete set, so a filtered view can never flip a failing project to exit 0. The CLI exposes --severity/--rule-id/--file/--max on check, analyze, and lint; the wright-agent/v1 findings, lint, and costEstimate requests accept the same fields flat in the request. Truncation is reported via selection.total/withheld, unknown rule ids are usage errors, and consecutive identical lint findings collapse into one rendered entry listing all locations. Fixes #430 --- crates/wright-bench/src/main.rs | 5 +- crates/wright-cli/src/cli.rs | 52 +++- crates/wright-cli/src/main.rs | 44 ++- crates/wright-cli/src/present.rs | 133 +++++++-- crates/wright-cli/tests/agent_contract.rs | 48 +++- crates/wright-cli/tests/cli.rs | 186 ++++++++++++ crates/wright-cli/tests/serve.rs | 30 ++ crates/wright-consumer/src/workflow.rs | 12 +- crates/wright-driver/src/config.rs | 4 + crates/wright-driver/src/lib.rs | 2 + crates/wright-driver/src/result.rs | 10 + crates/wright-driver/src/select.rs | 281 +++++++++++++++++++ crates/wright-driver/src/service.rs | 115 ++++++-- crates/wright-driver/src/session.rs | 17 ++ crates/wright-driver/src/session/semantic.rs | 8 +- crates/wright-driver/tests/service.rs | 125 ++++++++- docs/agent-contract.md | 28 +- docs/cli/commands.md | 9 +- docs/cli/lint.md | 25 ++ docs/cli/presentation.md | 10 + schemas/wright-agent-v1.schema.json | 70 ++++- schemas/wright-check-v1.schema.json | 9 + 22 files changed, 1141 insertions(+), 82 deletions(-) create mode 100644 crates/wright-driver/src/select.rs diff --git a/crates/wright-bench/src/main.rs b/crates/wright-bench/src/main.rs index abfc9e23..e4931967 100644 --- a/crates/wright-bench/src/main.rs +++ b/crates/wright-bench/src/main.rs @@ -5,6 +5,7 @@ use std::process::ExitCode; use std::time::{Duration, Instant}; use wright_driver::CompilerSession; +use wright_driver::FindingSelection; use wright_driver::Profile; use wright_driver::config::{InputSpec, SessionConfig, SourceKind}; use wright_driver::service::{ToolRequest, ToolResponse, ToolService}; @@ -283,9 +284,9 @@ fn semantic_query_trial( ToolRequest::References { symbol: 0 }, ToolRequest::Usage { symbol: 0 }, ToolRequest::Cfg { rule: 0 }, - ToolRequest::Findings, + ToolRequest::Findings(FindingSelection::default()), ToolRequest::PersistentObjects, - ToolRequest::Lint, + ToolRequest::Lint(FindingSelection::default()), ToolRequest::LintRules, ]; let start = Instant::now(); diff --git a/crates/wright-cli/src/cli.rs b/crates/wright-cli/src/cli.rs index 4eff5886..d5febba8 100644 --- a/crates/wright-cli/src/cli.rs +++ b/crates/wright-cli/src/cli.rs @@ -54,6 +54,12 @@ LINT OPTIONS: --disable-rule Disable a lint rule (repeatable) --rule-severity : Override a lint rule severity (repeatable) +FINDING SELECTION (check, analyze, lint): + --severity Report findings at or above a severity: error|warning|info + --rule-id Report findings from one lint rule id only + --file Report findings in one source file (resolved span.path) + --max Report at most N findings (withheld counts are shown) + UPDATE OPTIONS: --check Check for an update without modifying the installation --version Install an exact version instead of the latest stable release"; @@ -67,9 +73,9 @@ pub(crate) enum Command { /// currently shipped. Convert(ConvertArgs), /// Check frontend, project, semantic, and validation correctness. - Check(CommonArgs), + Check(ReportArgs), /// Summarize semantic structure, CFG hotspots, and cross-cutting state. - Analyze(CommonArgs), + Analyze(ReportArgs), /// Parse, lower, and report lint findings. Lint(LintArgs), /// Parse, lower, and show exhaustive structural/semantic facts. @@ -117,6 +123,38 @@ pub(crate) struct SemanticCompareArgs { pub(crate) actual: PathBuf, } +/// Arguments of commands that report findings: shared workflow options plus +/// the finding-selection options (#430). +#[derive(Debug, Args)] +pub(crate) struct ReportArgs { + #[command(flatten)] + pub(crate) common: CommonArgs, + #[command(flatten)] + pub(crate) select: SelectArgs, +} + +/// Finding-selection options shared by `check`, `analyze`, and `lint` +/// (`cost` joins with the query surface, #429). Selection narrows reported +/// output only — verdicts and exit codes always reflect the complete set. +#[derive(Debug, Args, Default)] +pub(crate) struct SelectArgs { + /// Report findings at or above this severity only. + #[arg(long, value_enum, value_name = "LEVEL")] + pub(crate) severity: Option, + /// Report findings produced by this lint rule id only; an unknown id is + /// a usage error. + #[arg(long, value_name = "ID")] + pub(crate) rule_id: Option, + /// Report findings located in this source file only (the resolved + /// span.path, e.g. `src/main.ws`). + #[arg(long, value_name = "PATH")] + pub(crate) file: Option, + /// Report at most N findings; withheld findings are reported, never + /// silently dropped. + #[arg(long, value_name = "N")] + pub(crate) max: Option, +} + #[derive(Debug, Args)] pub(crate) struct CommonArgs { /// Input path; `-` reads standard input and an omitted path uses the current directory. @@ -152,6 +190,8 @@ pub(crate) struct CommonArgs { pub(crate) struct LintArgs { #[command(flatten)] pub(crate) common: CommonArgs, + #[command(flatten)] + pub(crate) select: SelectArgs, /// Read project lint configuration YAML. #[arg(long = "lint-config", value_name = "PATH")] pub(crate) lint_config: Option, @@ -299,6 +339,14 @@ pub(crate) enum ColorArg { Never, } +#[derive(Clone, Copy, Debug, Eq, PartialEq, ValueEnum)] +pub(crate) enum SeverityArg { + Error, + #[value(alias = "warn")] + Warning, + Info, +} + #[derive(Clone, Copy, Debug, Eq, PartialEq, ValueEnum)] pub(crate) enum ShellArg { Bash, diff --git a/crates/wright-cli/src/main.rs b/crates/wright-cli/src/main.rs index 50f1e8f3..ab5f219c 100644 --- a/crates/wright-cli/src/main.rs +++ b/crates/wright-cli/src/main.rs @@ -139,18 +139,27 @@ fn run_workflow(command: Command) -> ExitCode { move |session: &mut wright_driver::CompilerSession| session.convert(target), ) } - Command::Check(args) => run_configured( - config_from_common(&args, true), - present::Presentation::from_common(&args), - wright_driver::CompilerSession::check, - ), - Command::Analyze(args) => run_configured( - config_from_common(&args, true), - present::Presentation::from_common(&args), - wright_driver::CompilerSession::analyze, - ), + Command::Check(args) => { + let mut config = config_from_common(&args.common, true); + config.selection = selection_from_args(&args.select); + run_configured( + config, + present::Presentation::from_common(&args.common), + wright_driver::CompilerSession::check, + ) + } + Command::Analyze(args) => { + let mut config = config_from_common(&args.common, true); + config.selection = selection_from_args(&args.select); + run_configured( + config, + present::Presentation::from_common(&args.common), + wright_driver::CompilerSession::analyze, + ) + } Command::Lint(args) => { let mut config = config_from_common(&args.common, true); + config.selection = selection_from_args(&args.select); if let Some(path) = &args.lint_config { config.lint = match LintConfig::from_yaml_path(path) { Ok(config) => config, @@ -301,6 +310,21 @@ fn config_from_common(common: &CommonArgs, provider_workflow: bool) -> SessionCo } } +/// Map the CLI finding-selection flags onto the shared driver selection +/// model; the driver applies it to diagnostics and lint findings alike (#430). +fn selection_from_args(select: &cli::SelectArgs) -> wright_driver::FindingSelection { + wright_driver::FindingSelection { + severity: select.severity.map(|severity| match severity { + cli::SeverityArg::Error => wright_driver::Severity::Error, + cli::SeverityArg::Warning => wright_driver::Severity::Warning, + cli::SeverityArg::Info => wright_driver::Severity::Info, + }), + rule: select.rule_id.clone(), + file: select.file.clone(), + max: select.max, + } +} + fn is_opy_input(common: &CommonArgs) -> bool { match common.kind { cli::SourceKindArg::Opy => true, diff --git a/crates/wright-cli/src/present.rs b/crates/wright-cli/src/present.rs index 5de00ddc..ac3d35a9 100644 --- a/crates/wright-cli/src/present.rs +++ b/crates/wright-cli/src/present.rs @@ -273,6 +273,14 @@ fn render_text(envelope: &Envelope, for diag in &envelope.diagnostics { render_diagnostic(diag, color); } + if let Some(selection) = &envelope.selection { + if selection.withheld > 0 { + eprintln!( + " ... {} diagnostic(s) withheld (--max)", + selection.withheld + ); + } + } if !envelope.ok { if envelope.diagnostics.is_empty() { eprintln!("{}: failed", envelope.command); @@ -296,7 +304,13 @@ fn render_verdict(envelope: &Envelope< }; println!("{label} {}", envelope.command); let metadata = match envelope.command.as_str() { - "check" => format!("{} diagnostic(s)", envelope.diagnostics.len()), + "check" => format!( + "{} diagnostic(s)", + envelope + .selection + .as_ref() + .map_or(envelope.diagnostics.len(), |selection| selection.total) + ), _ => envelope.result.metadata().unwrap_or_default(), }; println!(" {}", dim(&metadata, color)); @@ -470,11 +484,21 @@ impl ResultPresentation for AnalyzeResult { impl ResultPresentation for LintResult { fn metadata(&self) -> Option { - Some(format!( + let total = self + .selection + .as_ref() + .map_or_else(|| array_len(&self.findings), |selection| selection.total); + let mut metadata = format!( "{} finding(s) across {} rule(s)", - array_len(&self.findings), + total, array_len(&self.rules) - )) + ); + if let Some(selection) = &self.selection { + if selection.withheld > 0 { + metadata.push_str(&format!(", {} withheld", selection.withheld)); + } + } + Some(metadata) } fn render_body(&self) { render_lint(self); @@ -487,10 +511,24 @@ impl ResultPresentation for LintResult { } } fn update_summary_status(&self, status: &mut SummaryStatus) { - if let Some(findings) = self.findings.as_array() { - for finding in findings { - if let Some(sev) = finding.get("severity").and_then(serde_json::Value::as_str) { - *status = (*status).max(SummaryStatus::from_finding_severity(sev)); + match &self.selection { + // Selection narrows the reported list only; the verdict keeps the + // full set's highest severity (#430). + Some(selection) => { + if let Some(severity) = selection.max_severity { + *status = + (*status).max(SummaryStatus::from_finding_severity(severity.as_str())); + } + } + None => { + if let Some(findings) = self.findings.as_array() { + for finding in findings { + if let Some(sev) = + finding.get("severity").and_then(serde_json::Value::as_str) + { + *status = (*status).max(SummaryStatus::from_finding_severity(sev)); + } + } } } } @@ -518,12 +556,27 @@ fn summary_status( } else { SummaryStatus::Error }; - for diag in &envelope.diagnostics { - status = status.max(match diag.severity { - Severity::Error => SummaryStatus::Error, - Severity::Warning => SummaryStatus::Warn, - Severity::Info => SummaryStatus::Pass, - }); + match &envelope.selection { + // The verdict reflects the full diagnostic set, not the selected + // remainder (#430). + Some(selection) => { + if let Some(severity) = selection.max_severity { + status = status.max(match severity { + Severity::Error => SummaryStatus::Error, + Severity::Warning => SummaryStatus::Warn, + Severity::Info => SummaryStatus::Pass, + }); + } + } + None => { + for diag in &envelope.diagnostics { + status = status.max(match diag.severity { + Severity::Error => SummaryStatus::Error, + Severity::Warning => SummaryStatus::Warn, + Severity::Info => SummaryStatus::Pass, + }); + } + } } envelope.result.update_summary_status(&mut status); status.as_str() @@ -700,17 +753,42 @@ fn render_lint(result: &LintResult) { if findings.is_empty() { println!(" none"); } - for f in &findings { - let code = f["code"].as_str().unwrap_or("finding"); - let sev = f["severity"].as_str().unwrap_or("info"); - let ev = f["evidence"].as_str().unwrap_or("exact"); - let msg = f["message"].as_str().unwrap_or_default(); - match f.get("boundedness").and_then(serde_json::Value::as_str) { + // Consecutive findings sharing a rule id and message collapse into one + // entry that lists its locations (#430). + let mut index = 0; + while index < findings.len() { + let first = &findings[index]; + let mut end = index + 1; + while end < findings.len() + && findings[end]["code"] == first["code"] + && findings[end]["message"] == first["message"] + { + end += 1; + } + let code = first["code"].as_str().unwrap_or("finding"); + let sev = first["severity"].as_str().unwrap_or("info"); + let ev = first["evidence"].as_str().unwrap_or("exact"); + let msg = first["message"].as_str().unwrap_or_default(); + match first.get("boundedness").and_then(serde_json::Value::as_str) { Some(v) => println!(" {sev}[{code}] (evidence: {ev}) (boundedness: {v}): {msg}"), None => println!(" {sev}[{code}] (evidence: {ev}): {msg}"), } - if let Some(span) = f.get("span") { - print_span(span, " "); + if end == index + 1 { + if let Some(span) = first.get("span") { + print_span(span, " "); + } + } else { + for finding in &findings[index..end] { + if let Some(span) = finding.get("span") { + print_location(span, " "); + } + } + } + index = end; + } + if let Some(selection) = &result.selection { + if selection.withheld > 0 { + println!(" ... {} finding(s) withheld (--max)", selection.withheld); } } } @@ -764,6 +842,16 @@ fn render_diagnostic(diagnostic: &wright_driver::Diagnostic, color: bool) { } } +fn print_location(span: &serde_json::Value, indent: &str) { + let path = span + .get("path") + .and_then(serde_json::Value::as_str) + .unwrap_or(""); + let line = span_position(span, "start", "line").unwrap_or(0); + let col = span_position(span, "start", "col").unwrap_or(0); + println!("{indent}--> {path}:{line}:{col}"); +} + fn print_span(span: &serde_json::Value, indent: &str) { let path = span .get("path") @@ -957,6 +1045,7 @@ mod tests { ok, exit: if ok { 0 } else { 1 }, diagnostics, + selection: None, result: LintResult { findings, ..LintResult::default() diff --git a/crates/wright-cli/tests/agent_contract.rs b/crates/wright-cli/tests/agent_contract.rs index b300429b..f0346911 100644 --- a/crates/wright-cli/tests/agent_contract.rs +++ b/crates/wright-cli/tests/agent_contract.rs @@ -6,7 +6,7 @@ use std::process::{Command, Stdio}; use jsonschema::JSONSchema; use serde_json::{Value, json}; use wright_driver::service::{AGENT_CONTRACT, ToolRequest, ToolResponse, ToolService}; -use wright_driver::{CompilerSession, InputSpec, SessionConfig, SourceKind}; +use wright_driver::{CompilerSession, FindingSelection, InputSpec, SessionConfig, SourceKind}; const EXPECTED_V1_OPERATIONS: &[&str] = &[ "capabilities", @@ -301,3 +301,49 @@ fn agent_v1_schema_covers_every_advertised_request_and_response() { ); } } + +#[test] +fn cli_and_agent_selections_return_the_same_set() { + // #430: the CLI flags and the agent request fields drive one driver-side + // selection, so both surfaces must return the same selected findings. + let input = workspace_root().join("tests/fixtures/workshop/real-world/overpy-cake.ws"); + let mut session = CompilerSession::new(SessionConfig { + input: InputSpec::Path(input.clone()), + kind: SourceKind::Workshop, + ..SessionConfig::default() + }) + .expect("session starts"); + let mut service = ToolService::new(&mut session).expect("service loads"); + let agent = match service.handle(&ToolRequest::Lint(FindingSelection { + rule: Some("repeated-value".to_string()), + max: Some(2), + ..FindingSelection::default() + })) { + ToolResponse::Ok { result } => result, + ToolResponse::Error { error } => panic!("lint selection failed: {error:?}"), + }; + + let output = Command::new(env!("CARGO_BIN_EXE_wright")) + .args([ + "lint", + input.to_str().unwrap(), + "--rule-id", + "repeated-value", + "--max", + "2", + "-f", + "json", + ]) + .stdin(Stdio::null()) + .output() + .expect("wright lint runs"); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + let cli = serde_json::from_slice::(&output.stdout).unwrap()["result"].clone(); + assert_eq!(cli["findings"], agent["findings"], "same selected set"); + assert_eq!(cli["selection"], agent["selection"], "same truncation"); + assert_eq!(agent["selection"], json!({"total": 10, "withheld": 7})); +} diff --git a/crates/wright-cli/tests/cli.rs b/crates/wright-cli/tests/cli.rs index c63b7b97..e106df3c 100644 --- a/crates/wright-cli/tests/cli.rs +++ b/crates/wright-cli/tests/cli.rs @@ -503,6 +503,192 @@ fn lint_flags_are_usage_errors_for_other_commands() { } } +// ── Finding selection (#430) ───────────────────────────────────────────────── +// `real-world/overpy-cake.ws` produces 10 findings: 9 identical +// `repeated-value` warnings and 1 `min-wait-loop` warning. + +#[test] +fn lint_selection_filters_findings_and_reports_withheld() { + let path = temp_file("cake.txt", &corpus_workshop("real-world/overpy-cake")); + let path = path.to_str().unwrap(); + + // --rule-id selects one rule; --max truncates and reports the withheld + // count in the envelope. + let output = run(&[ + "lint", + path, + "--rule-id", + "repeated-value", + "--max", + "2", + "-f", + "json", + ]); + assert!(output.status.success(), "{}", command_result(&output)); + let envelope = parse_json(&output.stdout); + let findings = envelope["result"]["findings"].as_array().unwrap(); + assert_eq!(findings.len(), 2); + assert!(findings.iter().all(|f| f["code"] == "repeated-value")); + assert_eq!(envelope["result"]["selection"]["total"], 10); + assert_eq!(envelope["result"]["selection"]["withheld"], 7); + + // --severity is a threshold: `error` selects none of the warning findings + // while the envelope records the full set. + let output = run(&["lint", path, "--severity", "error", "-f", "json"]); + assert!(output.status.success()); + let envelope = parse_json(&output.stdout); + assert!( + envelope["result"]["findings"] + .as_array() + .unwrap() + .is_empty() + ); + assert_eq!(envelope["result"]["selection"]["total"], 10); + assert_eq!(envelope["result"]["selection"]["withheld"], 0); + + // --file matches the resolved span.path exactly. + let reported_path = parse_json(&run(&["lint", path, "-f", "json"]).stdout) + ["result"]["findings"][0]["span"]["path"] + .as_str() + .unwrap() + .to_string(); + let output = run(&["lint", path, "--file", &reported_path, "-f", "json"]); + let envelope = parse_json(&output.stdout); + assert_eq!(envelope["result"]["findings"].as_array().unwrap().len(), 10); + let output = run(&["lint", path, "--file", "other.ws", "-f", "json"]); + let envelope = parse_json(&output.stdout); + assert!( + envelope["result"]["findings"] + .as_array() + .unwrap() + .is_empty() + ); + assert_eq!(envelope["result"]["selection"]["total"], 10); + + let _ = std::fs::remove_dir_all(Path::new(path).parent().unwrap()); +} + +#[test] +fn omitted_selection_reproduces_the_unfiltered_envelope() { + // Omitting every selection option reproduces the current output: + // complete findings/diagnostics arrays and no `selection` member (#430). + let path = temp_file("cake.txt", &corpus_workshop("real-world/overpy-cake")); + let output = run(&["lint", path.to_str().unwrap(), "-f", "json"]); + assert!(output.status.success()); + let envelope = parse_json(&output.stdout); + assert_eq!(envelope["result"]["findings"].as_array().unwrap().len(), 10); + assert!(envelope["result"].get("selection").is_none()); + assert!(envelope.get("selection").is_none()); + let _ = std::fs::remove_dir_all(path.parent().unwrap()); +} + +#[test] +fn selection_never_changes_the_verdict_or_exit_code() { + // The ablation guard of #430: if exit codes were computed from the + // selected set instead of the full set, filtering an error out of view + // would flip a failing check to exit 0. + let broken = temp_file( + "broken.txt", + "rule (\"x\") { event { Ongoing - Global; } actions { If(True); }", + ); + let baseline = run(&["check", broken.to_str().unwrap(), "-f", "json"]); + assert_eq!(baseline.status.code(), Some(1)); + let full = parse_json(&baseline.stdout)["diagnostics"] + .as_array() + .unwrap() + .len(); + assert!(full > 0); + + let output = run(&[ + "check", + broken.to_str().unwrap(), + "--rule-id", + "min-wait-loop", + "-f", + "json", + ]); + assert_eq!( + output.status.code(), + Some(1), + "selection must not turn a failing project into exit 0" + ); + let envelope = parse_json(&output.stdout); + assert_eq!(envelope["ok"], false); + assert_eq!(envelope["exit"], 1); + assert!( + envelope["diagnostics"].as_array().unwrap().is_empty(), + "the error diagnostic is selected out of the report" + ); + assert_eq!( + envelope["selection"]["total"].as_u64().unwrap() as usize, + full + ); + + // The lint verdict survives an empty selected set on the warning-only + // fixture: severity=error selects nothing, the verdict still says WARN. + let cake = temp_file("cake.txt", &corpus_workshop("real-world/overpy-cake")); + let output = run(&["lint", cake.to_str().unwrap(), "--severity", "error"]); + assert!(output.status.success()); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("WARN lint"), "{stdout}"); + assert!(stdout.contains("10 finding(s)"), "{stdout}"); + + let _ = std::fs::remove_dir_all(broken.parent().unwrap()); + let _ = std::fs::remove_dir_all(cake.parent().unwrap()); +} + +#[test] +fn lint_max_reports_the_withheld_count_in_text_and_json() { + let path = temp_file("cake.txt", &corpus_workshop("real-world/overpy-cake")); + let output = run(&["lint", path.to_str().unwrap(), "--max", "3"]); + assert!(output.status.success()); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("7 finding(s) withheld"), "{stdout}"); + assert!(stdout.contains("10 finding(s)"), "{stdout}"); + let _ = std::fs::remove_dir_all(path.parent().unwrap()); +} + +#[test] +fn lint_text_collapses_identical_findings_into_one_entry() { + let path = temp_file("cake.txt", &corpus_workshop("real-world/overpy-cake")); + let output = run(&["lint", path.to_str().unwrap()]); + assert!(output.status.success()); + let stdout = String::from_utf8_lossy(&output.stdout); + // The nine identical findings render as one entry listing nine locations; + // the verdict still reports the true total. + assert!(stdout.contains("10 finding(s)"), "{stdout}"); + assert_eq!( + stdout.matches("[repeated-value]").count(), + 1, + "repeated findings collapse into one entry: {stdout}" + ); + assert_eq!( + stdout.matches(" --> ").count(), + 10, + "nine grouped locations plus the single min-wait-loop: {stdout}" + ); + let _ = std::fs::remove_dir_all(path.parent().unwrap()); +} + +#[test] +fn an_unknown_rule_id_in_a_selection_is_a_usage_error() { + let path = temp_file("cake.txt", &corpus_workshop("real-world/overpy-cake")); + for command in ["lint", "check", "analyze"] { + let output = run(&[command, path.to_str().unwrap(), "--rule-id", "not-a-rule"]); + assert_eq!( + output.status.code(), + Some(2), + "{command} --rule-id not-a-rule must be a usage error" + ); + assert!(output.stdout.is_empty(), "usage errors write stderr only"); + assert!( + String::from_utf8_lossy(&output.stderr).contains("not-a-rule"), + "the message names the unknown id" + ); + } + let _ = std::fs::remove_dir_all(path.parent().unwrap()); +} + #[test] fn stdin_workshop_works_and_legacy_protocol_is_refused() { // Workshop text on stdin. diff --git a/crates/wright-cli/tests/serve.rs b/crates/wright-cli/tests/serve.rs index 0f7ad17b..506734b8 100644 --- a/crates/wright-cli/tests/serve.rs +++ b/crates/wright-cli/tests/serve.rs @@ -186,6 +186,36 @@ fn serve_reserves_stdin_for_session_requests() { assert!(String::from_utf8_lossy(&output.stderr).contains("reserves stdin for requests")); } +#[test] +fn stdio_transport_applies_finding_selection() { + // #430: selection fields ride on the wire request and the response + // reports the withheld count; an unknown rule id is a structured error. + let input = corpus_workshop("real-world/overpy-cake"); + let responses = run_lines( + "stdio", + &input, + &[ + r#"{"op":"findings","max":3}"#, + r#"{"op":"lint","severity":"error"}"#, + r#"{"op":"lint","rule":"not-a-rule"}"#, + ], + ); + assert_eq!( + responses[0]["result"]["findings"].as_array().unwrap().len(), + 3 + ); + assert_eq!(responses[0]["result"]["selection"]["total"], 10); + assert_eq!(responses[0]["result"]["selection"]["withheld"], 7); + assert!( + responses[1]["result"]["findings"] + .as_array() + .unwrap() + .is_empty() + ); + assert_eq!(responses[1]["result"]["selection"]["total"], 10); + assert_eq!(responses[2]["error"]["code"], "invalid-selection"); +} + #[test] fn transports_match_in_process_semantics() { // The same query through both transports yields equivalent results. diff --git a/crates/wright-consumer/src/workflow.rs b/crates/wright-consumer/src/workflow.rs index 2bb79be8..76edc845 100644 --- a/crates/wright-consumer/src/workflow.rs +++ b/crates/wright-consumer/src/workflow.rs @@ -1,5 +1,7 @@ use wright_driver::service::{ToolRequest, ToolService}; -use wright_driver::{CompilerSession, InputSpec, Profile, SessionConfig, SourceKind}; +use wright_driver::{ + CompilerSession, FindingSelection, InputSpec, Profile, SessionConfig, SourceKind, +}; pub fn run_consumer(input: &str) -> Result<(), String> { let source = std::fs::read_to_string(input).map_err(|e| e.to_string())?; @@ -61,15 +63,15 @@ pub fn run_consumer(input: &str) -> Result<(), String> { for request in [ ToolRequest::Project, ToolRequest::Rules, - ToolRequest::Findings, - ToolRequest::Lint, + ToolRequest::Findings(FindingSelection::default()), + ToolRequest::Lint(FindingSelection::default()), ToolRequest::LintRules, - ToolRequest::CostEstimate, + ToolRequest::CostEstimate(FindingSelection::default()), ToolRequest::TargetMetadata, ] { match service.handle(&request) { wright_driver::service::ToolResponse::Ok { result } => { - if matches!(request, ToolRequest::Lint) { + if matches!(request, ToolRequest::Lint(_)) { for finding in result["findings"].as_array().unwrap() { assert!( finding.get("evidence").is_some(), diff --git a/crates/wright-driver/src/config.rs b/crates/wright-driver/src/config.rs index 18a433fa..29502f5b 100644 --- a/crates/wright-driver/src/config.rs +++ b/crates/wright-driver/src/config.rs @@ -88,6 +88,10 @@ pub struct SessionConfig { pub profile: wright_transform::Profile, pub lint: LintConfig, pub lint_rule_paths: Vec, + /// Finding/diagnostic selection applied to reported output (#430). + /// Selection narrows presentation only; verdicts and exit codes are + /// computed on the full set. + pub selection: crate::select::FindingSelection, pub providers: wright_lpp::ProviderRegistry, pub opy_provider: crate::opy_provider::OpyProviderConfig, } diff --git a/crates/wright-driver/src/lib.rs b/crates/wright-driver/src/lib.rs index 5ccb8907..4e7583e2 100644 --- a/crates/wright-driver/src/lib.rs +++ b/crates/wright-driver/src/lib.rs @@ -12,6 +12,7 @@ pub mod progress; pub mod provider; pub mod provider_edit; pub mod result; +pub mod select; pub mod service; pub mod session; pub mod source_provider; @@ -29,6 +30,7 @@ pub use result::{ AnalyzeResult, CheckResult, CompileResult, CompiledOutput, ConvertResult, ConvertTarget, Envelope, InspectResult, LintResult, RESULT_CONTRACT, }; +pub use select::{FindingSelection, SelectionOutcome}; pub use session::{CompilerSession, Loaded, Provenance}; pub use source_provider::{ SourceBackend, SourceCompilation, SourceLanguage, SourceProvenance, SourceProvider, diff --git a/crates/wright-driver/src/result.rs b/crates/wright-driver/src/result.rs index 227f3262..1cc5674c 100644 --- a/crates/wright-driver/src/result.rs +++ b/crates/wright-driver/src/result.rs @@ -26,7 +26,12 @@ pub struct Envelope { pub command: String, pub ok: bool, pub exit: u8, + /// Diagnostics remaining after selection; `ok`/`exit` always reflect the + /// full set, never the selected remainder. pub diagnostics: Vec, + /// Present when a finding selection reduced `diagnostics` (#430). + #[serde(skip_serializing_if = "Option::is_none")] + pub selection: Option, pub result: T, } @@ -92,8 +97,13 @@ pub struct LintResult { pub program: serde_json::Value, pub rules: serde_json::Value, pub config: serde_json::Value, + /// Lint findings after selection. The verdict reports the true total via + /// `selection`, not this array's length. pub findings: serde_json::Value, pub skipped: serde_json::Value, + /// Present when a finding selection was applied to `findings` (#430). + #[serde(skip_serializing_if = "Option::is_none")] + pub selection: Option, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Default, Serialize)] diff --git a/crates/wright-driver/src/select.rs b/crates/wright-driver/src/select.rs new file mode 100644 index 00000000..f853d850 --- /dev/null +++ b/crates/wright-driver/src/select.rs @@ -0,0 +1,281 @@ +//! Finding selection shared by the CLI workflow flags and the +//! `wright-agent/v1` request fields (#430). One selection model drives every +//! finding-shaped output (envelope diagnostics and `result.findings`), so the +//! CLI and the agent contract return the same selected set. +//! +//! Selection only narrows what is *reported*: verdicts and exit codes are +//! computed on the full set before selection applies, and the returned +//! [`SelectionOutcome`] keeps the true total, the truncation count, and the +//! full set's highest severity so no surface can understate the result. + +use serde::{Deserialize, Serialize}; +use wright_analyzer::registry::{LintConfig, LintRegistry}; + +use crate::diag::{Diagnostic, Severity}; + +/// A finding/diagnostic selection: `severity`/`rule`/`file` filter the set, +/// `max` truncates what remains. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct FindingSelection { + /// Minimum severity reported (`info` selects everything, `error` selects + /// only errors). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub severity: Option, + /// Select only findings produced by this rule id (the finding `code`). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub rule: Option, + /// Select only findings located in this source file, matched exactly + /// against the resolved `span.path`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub file: Option, + /// Report at most this many findings after filtering. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub max: Option, +} + +/// The recorded outcome of applying a [`FindingSelection`]: the true total +/// and the count dropped by `max`. `max_severity` preserves the full set's +/// highest severity for verdict/exit computation and is not part of the wire +/// payload. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct SelectionOutcome { + /// Findings present before selection. + pub total: usize, + /// Filtered findings dropped by `max`. + pub withheld: usize, + /// Highest severity over the full, unselected set. + #[serde(skip)] + pub max_severity: Option, +} + +fn severity_rank(severity: Severity) -> u8 { + match severity { + Severity::Info => 0, + Severity::Warning => 1, + Severity::Error => 2, + } +} + +/// Serialized findings always carry a known severity; a missing or unknown +/// value falls back to `Warning`, the same conservative default the CLI +/// verdict uses (`SummaryStatus::from_finding_severity`), so malformed data +/// can never understate the true maximum severity. +fn parse_severity(value: Option<&str>) -> Severity { + match value { + Some("error") => Severity::Error, + Some("info") | Some("notice") => Severity::Info, + _ => Severity::Warning, + } +} + +impl FindingSelection { + /// Whether any selection dimension is set. + pub fn is_active(&self) -> bool { + self.severity.is_some() || self.rule.is_some() || self.file.is_some() || self.max.is_some() + } + + /// The selection is a usage error when `rule` names no registered lint + /// rule: silently selecting nothing would report a clean result for a + /// typo'd id. + pub fn validate(&self, registry: &LintRegistry) -> Result<(), String> { + if let Some(rule) = &self.rule { + let known = registry + .descriptors(&LintConfig::default()) + .iter() + .any(|descriptor| descriptor.id == *rule); + if !known { + return Err(format!("unknown rule id '{rule}'")); + } + } + Ok(()) + } + + /// Apply the selection to envelope diagnostics. + pub fn apply_diagnostics( + &self, + diagnostics: Vec, + ) -> (Vec, Option) { + self.apply(diagnostics, |diagnostic| { + ( + diagnostic.severity, + diagnostic.code.as_str(), + diagnostic.span.as_ref().map(|span| span.path.as_str()), + ) + }) + } + + /// Apply the selection to serialized findings (`code`, `severity`, + /// `span.path` fields). + pub fn apply_findings( + &self, + findings: Vec, + ) -> (Vec, Option) { + self.apply(findings, |finding| { + ( + parse_severity(finding.get("severity").and_then(|v| v.as_str())), + finding.get("code").and_then(|v| v.as_str()).unwrap_or(""), + finding + .get("span") + .and_then(|span| span.get("path")) + .and_then(|path| path.as_str()), + ) + }) + } + + fn apply( + &self, + items: Vec, + key: impl Fn(&T) -> (Severity, &str, Option<&str>), + ) -> (Vec, Option) { + if !self.is_active() { + return (items, None); + } + let total = items.len(); + let max_severity = items + .iter() + .map(|item| key(item).0) + .max_by_key(|severity| severity_rank(*severity)); + let mut kept: Vec = items + .into_iter() + .filter(|item| { + let (severity, code, path) = key(item); + self.severity + .is_none_or(|min| severity_rank(severity) >= severity_rank(min)) + && self.rule.as_ref().is_none_or(|rule| rule == code) + && self + .file + .as_ref() + .is_none_or(|file| Some(file.as_str()) == path) + }) + .collect(); + let withheld = self.max.map_or(0, |max| kept.len().saturating_sub(max)); + if let Some(max) = self.max { + kept.truncate(max); + } + ( + kept, + Some(SelectionOutcome { + total, + withheld, + max_severity, + }), + ) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + fn finding(code: &str, severity: &str, path: Option<&str>) -> serde_json::Value { + json!({ + "code": code, + "severity": severity, + "message": "msg", + "span": { "path": path }, + }) + } + + fn selection(input: serde_json::Value) -> FindingSelection { + serde_json::from_value(input).unwrap() + } + + #[test] + fn inactive_selection_returns_items_untouched() { + let items = vec![finding("a", "warning", None)]; + let (kept, outcome) = FindingSelection::default().apply_findings(items.clone()); + assert_eq!(kept, items); + assert!(outcome.is_none()); + } + + #[test] + fn severity_threshold_keeps_at_or_above() { + let items = vec![ + finding("a", "info", None), + finding("b", "warning", None), + finding("c", "error", None), + ]; + let (kept, outcome) = selection(json!({"severity": "warning"})).apply_findings(items); + assert_eq!( + kept.iter() + .map(|f| f["code"].as_str().unwrap()) + .collect::>(), + vec!["b", "c"] + ); + let outcome = outcome.unwrap(); + assert_eq!(outcome.total, 3); + assert_eq!(outcome.withheld, 0); + assert_eq!(outcome.max_severity, Some(Severity::Error)); + } + + #[test] + fn rule_and_file_filter() { + let items = vec![ + finding("a", "warning", Some("x.ws")), + finding("a", "warning", Some("y.ws")), + finding("b", "warning", Some("x.ws")), + ]; + let (kept, _) = selection(json!({"rule": "a", "file": "x.ws"})).apply_findings(items); + assert_eq!(kept.len(), 1); + // No span path never matches a file selection. + let (kept, _) = + selection(json!({"file": "x.ws"})).apply_findings(vec![finding("a", "warning", None)]); + assert!(kept.is_empty()); + } + + #[test] + fn max_truncates_and_reports_withheld() { + let items = vec![ + finding("a", "warning", None), + finding("b", "warning", None), + finding("c", "warning", None), + finding("d", "warning", None), + ]; + let (kept, outcome) = selection(json!({"max": 1})).apply_findings(items); + assert_eq!(kept.len(), 1); + let outcome = outcome.unwrap(); + assert_eq!(outcome.total, 4); + assert_eq!(outcome.withheld, 3); + } + + #[test] + fn diagnostics_share_the_same_semantics() { + let mut items = vec![ + Diagnostic::error("parse-error", crate::diag::Stage::Frontend, "a"), + Diagnostic::warning("warn-code", crate::diag::Stage::Analysis, "b"), + ]; + items[0].severity = Severity::Error; + let (kept, outcome) = selection(json!({"severity": "error"})).apply_diagnostics(items); + assert_eq!(kept.len(), 1); + assert_eq!(kept[0].code, "parse-error"); + assert_eq!(outcome.unwrap().max_severity, Some(Severity::Error)); + } + + #[test] + fn unknown_rule_is_an_error() { + let registry = LintRegistry::default(); + assert!( + selection(json!({"rule": "min-wait-loop"})) + .validate(®istry) + .is_ok() + ); + let err = selection(json!({"rule": "not-a-rule"})) + .validate(®istry) + .unwrap_err(); + assert!(err.contains("not-a-rule")); + } + + #[test] + fn outcome_serializes_only_wire_fields() { + let outcome = SelectionOutcome { + total: 4, + withheld: 3, + max_severity: Some(Severity::Warning), + }; + assert_eq!( + serde_json::to_value(&outcome).unwrap(), + json!({"total": 4, "withheld": 3}) + ); + } +} diff --git a/crates/wright-driver/src/service.rs b/crates/wright-driver/src/service.rs index e1514eeb..a566f2cf 100644 --- a/crates/wright-driver/src/service.rs +++ b/crates/wright-driver/src/service.rs @@ -57,18 +57,21 @@ pub enum ToolRequest { Usage { symbol: u32 }, /// The control-flow graph of one rule. Cfg { rule: u32 }, - /// Every static-analysis finding. - Findings, + /// Every static-analysis finding, optionally narrowed by an inline + /// selection (`severity`, `rule`, `file`, `max`; #430). + Findings(crate::select::FindingSelection), /// Persistent Workshop object facts, separate from lint diagnostics. PersistentObjects, - /// Lint findings plus rule metadata and effective configuration (#98). - Lint, + /// Lint findings plus rule metadata and effective configuration (#98), + /// optionally narrowed by an inline selection (#430). + Lint(crate::select::FindingSelection), /// The registered lint rules and the effective lint configuration. LintRules, /// The subroutine call graph (caller rules → callee subroutines). CallGraph, - /// Generated-resource cost estimates (exact counts + findings). - CostEstimate, + /// Generated-resource cost estimates (exact counts + findings), + /// optionally narrowed by an inline selection (#430). + CostEstimate(crate::select::FindingSelection), /// Target/catalog metadata (actions, values, events, enum domains). TargetMetadata, /// Validate and preview a caller-supplied source-edit transaction @@ -268,12 +271,12 @@ impl<'a> ToolService<'a> { self.semantic_query(Request::GetUsage { symbol: *symbol }) } ToolRequest::Cfg { rule } => self.semantic_query(Request::GetCfg { rule: *rule }), - ToolRequest::Findings => self.findings(), + ToolRequest::Findings(selection) => self.findings(selection), ToolRequest::PersistentObjects => self.persistent_objects(), - ToolRequest::Lint => self.lint(), + ToolRequest::Lint(selection) => self.lint(selection), ToolRequest::LintRules => self.configured_semantic().handle(&Request::LintRules), ToolRequest::CallGraph => self.ok(self.call_graph()), - ToolRequest::CostEstimate => self.ok(self.cost_estimate()), + ToolRequest::CostEstimate(selection) => self.cost_estimate(selection), ToolRequest::TargetMetadata => self.ok(self.target_metadata()), ToolRequest::ValidateEdit { sources, @@ -410,9 +413,28 @@ impl<'a> ToolService<'a> { /// /// The tool/agent surface resolves `span.path` exactly like the CLI /// `analyze`/`lint` workflows, so one file identity holds per finding - /// across every surface (#102). - fn findings(&self) -> ToolResponse { - self.semantic_query_with_resolved_span_paths(Request::GetFindings) + /// across every surface (#102). A selection narrows the reported set + /// (#430): without selection fields the result stays the bare finding + /// array; with them it becomes `{"findings": [...], "selection": {...}}` + /// so truncation is always visible. + fn findings(&self, selection: &crate::select::FindingSelection) -> ToolResponse { + if let Some(error) = self.selection_error(selection) { + return error; + } + match self.semantic_query_with_resolved_span_paths(Request::GetFindings) { + ToolResponse::Ok { result } => { + let findings = match result { + serde_json::Value::Array(findings) => findings, + _ => Vec::new(), + }; + let (findings, outcome) = selection.apply_findings(findings); + match outcome { + Some(outcome) => self.ok(json!({ "findings": findings, "selection": outcome })), + None => self.ok(serde_json::Value::Array(findings)), + } + } + other => other, + } } /// `persistentObjects`: persistent-object facts with resolved source paths. @@ -439,10 +461,29 @@ impl<'a> ToolService<'a> { self.lint_semantic.as_ref().unwrap_or(&self.semantic) } + /// A selection naming an unknown rule id is a usage error, never a + /// silent empty result (#430). + fn selection_error(&self, selection: &crate::select::FindingSelection) -> Option { + selection + .validate(self.session.lint_registry()) + .err() + .map(|message| ToolResponse::Error { + error: ToolErrorInfo { + code: "invalid-selection".to_string(), + message, + }, + }) + } + /// `lint`: rule metadata, effective configuration, and findings over the /// loaded program through the same semantic-service path as the CLI - /// `lint` workflow (no duplicated rule execution, #98). - fn lint(&self) -> ToolResponse { + /// `lint` workflow (no duplicated rule execution, #98). A `selection` + /// member records the true total and withheld count when the request + /// selected a subset (#430). + fn lint(&self, selection: &crate::select::FindingSelection) -> ToolResponse { + if let Some(error) = self.selection_error(selection) { + return error; + } let service = self.configured_semantic(); let lint_rules = match service.handle(&Request::LintRules) { Response::Ok { result } => result, @@ -453,13 +494,22 @@ impl<'a> ToolService<'a> { Response::Error { .. } => serde_json::json!([]), }; crate::session::resolve_span_paths(&mut findings, &self.loaded); - self.ok(json!({ + let findings = match findings { + serde_json::Value::Array(findings) => findings, + _ => Vec::new(), + }; + let (findings, outcome) = selection.apply_findings(findings); + let mut result = json!({ "inputIdentity": self.loaded.input.identity, "rules": lint_rules.get("rules").cloned().unwrap_or_else(|| json!([])), "config": lint_rules.get("config").cloned().unwrap_or_else(|| json!({})), "findings": findings, "skipped": lint_rules.get("skipped").cloned().unwrap_or_else(|| json!([])), - })) + }); + if let Some(outcome) = outcome { + result["selection"] = serde_json::to_value(outcome).expect("selection serializes"); + } + self.ok(result) } /// Program summary with origin and source identity. @@ -500,7 +550,13 @@ impl<'a> ToolService<'a> { serde_json::Value::Array(edges) } - fn cost_estimate(&self) -> serde_json::Value { + /// `costEstimate`: exact generated-resource counts plus static findings. + /// The findings subset honors the shared selection (#430); findings carry + /// no span, so a `file` selection matches nothing here. + fn cost_estimate(&self, selection: &crate::select::FindingSelection) -> ToolResponse { + if let Some(error) = self.selection_error(selection) { + return error; + } let locale = CompilerSession::locale_for(&self.loaded); let text = workshop_rs::emitter::emit(&self.loaded.program, self.session.catalog(), &locale) @@ -517,24 +573,35 @@ impl<'a> ToolService<'a> { &self.loaded.program, &wright_analyzer::registry::LintConfig::default(), ); - json!({ + let findings = findings + .iter() + .map(|f| { + json!({ + "code": f.code, + "severity": f.severity.as_str(), + "message": f.message, + }) + }) + .collect::>(); + let (findings, outcome) = selection.apply_findings(findings); + let mut result = json!({ "exact": { "emittedBytes": text.len(), "programActions": self.loaded.program.rules.iter().map(|r| r.actions.len()).sum::(), "programRules": self.loaded.program.rules.len(), "waitActions": waits, }, - "findings": findings.iter().map(|f| json!({ - "code": f.code, - "severity": f.severity.as_str(), - "message": f.message, - })).collect::>(), + "findings": findings, "kind": { "exact": "exact target-resource counts", "findings": "static/heuristic execution indicators", "performance": "compiler-host performance is measured by wright-bench, not in-process", }, - }) + }); + if let Some(outcome) = outcome { + result["selection"] = serde_json::to_value(outcome).expect("selection serializes"); + } + self.ok(result) } fn target_metadata(&self) -> serde_json::Value { diff --git a/crates/wright-driver/src/session.rs b/crates/wright-driver/src/session.rs index b3d1f4d8..40572dff 100644 --- a/crates/wright-driver/src/session.rs +++ b/crates/wright-driver/src/session.rs @@ -109,6 +109,13 @@ impl CompilerSession { Diagnostic::error("lint-rule-error", Stage::Analysis, error.to_string()) })?; } + if let Err(message) = config.selection.validate(&lint_registry) { + return Err(Diagnostic::error( + "invalid-selection", + Stage::Discovery, + message, + )); + } Ok(CompilerSession { config, catalog, @@ -758,15 +765,25 @@ impl CompilerSession { } } + /// The lint registry backing this session, for finding-selection + /// validation (#430). + pub(crate) fn lint_registry(&self) -> &LintRegistry { + &self.lint_registry + } + fn finish(&mut self, command: &str, result: T) -> Envelope { let diagnostics = std::mem::take(&mut self.diagnostics); let exit = exit_code_from(&diagnostics); + // Selection narrows the reported diagnostics after the exit code is + // fixed; filtering must never change the verdict. + let (diagnostics, selection) = self.config.selection.apply_diagnostics(diagnostics); Envelope { wright: version_info(), command: command.to_string(), ok: exit == crate::result::exit::SUCCESS, exit, diagnostics, + selection, result, } } diff --git a/crates/wright-driver/src/session/semantic.rs b/crates/wright-driver/src/session/semantic.rs index 47a01d93..218e7256 100644 --- a/crates/wright-driver/src/session/semantic.rs +++ b/crates/wright-driver/src/session/semantic.rs @@ -239,6 +239,11 @@ impl CompilerSession { )); let mut findings = service_response(&service, &Request::GetFindings); resolve_span_paths(&mut findings, &loaded); + let findings = match findings { + serde_json::Value::Array(findings) => findings, + _ => Vec::new(), + }; + let (findings, selection) = session.config.selection.apply_findings(findings); let (rules, config, skipped) = if let serde_json::Value::Object(mut object) = lint_rules { ( @@ -264,8 +269,9 @@ impl CompilerSession { program, rules, config, - findings, + findings: serde_json::Value::Array(findings), skipped, + selection, } }, ) diff --git a/crates/wright-driver/tests/service.rs b/crates/wright-driver/tests/service.rs index 7881cd79..ef31ebb3 100644 --- a/crates/wright-driver/tests/service.rs +++ b/crates/wright-driver/tests/service.rs @@ -4,7 +4,9 @@ use std::path::{Path, PathBuf}; use wright_driver::service::{ToolRequest, ToolResponse, ToolService}; -use wright_driver::{CompilerSession, InputSpec, SessionConfig, SourceKind}; +use wright_driver::{ + CompilerSession, FindingSelection, InputSpec, SessionConfig, Severity, SourceKind, +}; fn workspace_root() -> PathBuf { Path::new(env!("CARGO_MANIFEST_DIR")).join("..").join("..") @@ -27,8 +29,8 @@ fn tool_service_queries_canonical_workshop() { ToolRequest::Capabilities, ToolRequest::Project, ToolRequest::Rules, - ToolRequest::Findings, - ToolRequest::CostEstimate, + ToolRequest::Findings(FindingSelection::default()), + ToolRequest::CostEstimate(FindingSelection::default()), ] { assert!(matches!(service.handle(&request), ToolResponse::Ok { .. })); } @@ -75,7 +77,7 @@ fn tool_service_lint_queries_keep_the_session_configuration() { "error" ); - let lint = match service.handle(&ToolRequest::Lint) { + let lint = match service.handle(&ToolRequest::Lint(FindingSelection::default())) { ToolResponse::Ok { result } => result, ToolResponse::Error { error } => panic!("lint failed: {error:?}"), }; @@ -88,7 +90,8 @@ fn tool_service_lint_queries_keep_the_session_configuration() { .expect("the fixture triggers min-wait-loop"); assert_eq!(configured_finding["severity"], "error"); - let default_findings = match service.handle(&ToolRequest::Findings) { + let default_findings = match service.handle(&ToolRequest::Findings(FindingSelection::default())) + { ToolResponse::Ok { result } => result, ToolResponse::Error { error } => panic!("findings failed: {error:?}"), }; @@ -127,6 +130,118 @@ fn tool_service_routes_workflows_through_the_agent_request_contract() { } } +#[test] +fn agent_finding_selection_filters_and_truncates_without_touching_defaults() { + // #430: `real-world/overpy-cake.ws` produces 10 findings — 9 identical + // `repeated-value` warnings and one `min-wait-loop` warning. + let input = workspace_root().join("tests/fixtures/workshop/real-world/overpy-cake.ws"); + let mut session = CompilerSession::new(SessionConfig { + input: InputSpec::Path(input), + kind: SourceKind::Workshop, + ..SessionConfig::default() + }) + .unwrap(); + let mut service = ToolService::new(&mut session).unwrap(); + let mut result_of = |request| match service.handle(&request) { + ToolResponse::Ok { result } => result, + ToolResponse::Error { error } => panic!("{error:?}"), + }; + + // No selection fields: the bare array result is unchanged. + let plain = result_of(ToolRequest::Findings(FindingSelection::default())); + assert!(plain.is_array(), "the default result stays a bare array"); + assert_eq!(plain.as_array().unwrap().len(), 10); + + // `severity` is a threshold; `error` selects none of the warning findings + // while the summary still reports the true total. + let errors = result_of(ToolRequest::Findings(FindingSelection { + severity: Some(Severity::Error), + ..FindingSelection::default() + })); + assert!(errors["findings"].as_array().unwrap().is_empty()); + assert_eq!( + errors["selection"], + serde_json::json!({"total": 10, "withheld": 0}) + ); + + // `rule` + `max` on `lint` embed the same summary beside the findings. + let lint = result_of(ToolRequest::Lint(FindingSelection { + rule: Some("repeated-value".to_string()), + max: Some(2), + ..FindingSelection::default() + })); + assert_eq!(lint["findings"].as_array().unwrap().len(), 2); + assert_eq!( + lint["selection"], + serde_json::json!({"total": 10, "withheld": 7}) + ); + + // The same fields apply to `costEstimate` findings. + let cost = result_of(ToolRequest::CostEstimate(FindingSelection { + max: Some(1), + ..FindingSelection::default() + })); + assert_eq!(cost["findings"].as_array().unwrap().len(), 1); + assert!(cost["selection"]["withheld"].as_u64().unwrap() > 0); +} + +#[test] +fn an_unknown_rule_id_is_a_service_error_not_an_empty_result() { + let mut session = CompilerSession::new(SessionConfig { + input: InputSpec::Path(workshop_path()), + kind: SourceKind::Workshop, + ..SessionConfig::default() + }) + .unwrap(); + let mut service = ToolService::new(&mut session).unwrap(); + for request in [ + ToolRequest::Findings(FindingSelection { + rule: Some("not-a-rule".to_string()), + ..FindingSelection::default() + }), + ToolRequest::Lint(FindingSelection { + rule: Some("not-a-rule".to_string()), + ..FindingSelection::default() + }), + ToolRequest::CostEstimate(FindingSelection { + rule: Some("not-a-rule".to_string()), + ..FindingSelection::default() + }), + ] { + match service.handle(&request) { + ToolResponse::Error { error } => { + assert_eq!(error.code, "invalid-selection"); + assert!(error.message.contains("not-a-rule")); + } + ToolResponse::Ok { result } => { + panic!("an unknown rule id must not return a result: {result}") + } + } + } +} + +#[test] +fn an_empty_selection_serializes_identically_to_no_selection() { + // Omitting every selection option reproduces the previous JSON output + // byte-for-byte (#430). + let config = SessionConfig { + input: InputSpec::Path(workshop_path()), + kind: SourceKind::Workshop, + ..SessionConfig::default() + }; + let plain = CompilerSession::new(config.clone()).unwrap().lint(); + let selected = CompilerSession::new(SessionConfig { + selection: FindingSelection::default(), + ..config + }) + .unwrap() + .lint(); + assert_eq!( + serde_json::to_vec(&plain).unwrap(), + serde_json::to_vec(&selected).unwrap(), + ); +} + #[test] fn tool_service_keeps_provider_refusals_structured() { let path = workspace_root().join("tests/fixtures/opy/basic-rule.opy"); diff --git a/docs/agent-contract.md b/docs/agent-contract.md index cf9633e2..b8e20a6a 100644 --- a/docs/agent-contract.md +++ b/docs/agent-contract.md @@ -80,18 +80,40 @@ the successful `result` payload. | `references` | required `symbol` | References for the symbol id | | `usage` | required `symbol` | Usage counts for the symbol id | | `cfg` | required `rule` | Control-flow graph for the rule id | -| `findings` | none | Wright static-analysis findings | +| `findings` | optional selection | Wright static-analysis findings; `{"findings": [...], "selection": {...}}` when a selection is applied | | `persistentObjects` | none | Persistent Workshop object facts | -| `lint` | none | Lint findings, rule metadata, and effective configuration | +| `lint` | optional selection | Lint findings, rule metadata, effective configuration, and `selection` when applied | | `lintRules` | none | Registered lint rules and effective configuration | | `callGraph` | none | Subroutine call graph | -| `costEstimate` | none | Exact generated-resource counts and separate static findings | +| `costEstimate` | optional selection | Exact generated-resource counts, findings, and `selection` when applied | | `targetMetadata` | none | Canonical target/catalog metadata | | `validateEditTransaction` | `sources`, `transaction` | Atomic validation status, diagnostics, and previews when valid | | `semanticRename` | `sources`, `target` | Validated rename transaction or structured refusal | | `providerSemanticRename` | `language_id`, `documents`, `position_document_uri`, `position`, `new_name`, optional `project_root`, `sources` | Provider-resolved rename transaction or structured refusal | | `providerValidateEdit` | `language_id`, `documents`, `transaction`, `sources`, optional `project_root` | Provider-validated transaction or structured refusal | +### Finding selection (#430) + +`findings`, `lint`, and `costEstimate` accept optional selection fields: + +* `severity`: a threshold — `error` reports errors only, `warning` errors and + warnings, `info` everything. +* `rule`: one lint rule id (the finding `code`). An unknown id is a + structured `invalid-selection` error, never a silent empty result. +* `file`: one source file, matched exactly against the resolved `span.path`. + `costEstimate` findings carry no span, so `file` selects nothing there. +* `max`: a bound on the reported count, applied after filtering. + +The CLI options `--severity`, `--rule-id`, `--file`, and `--max` on `lint`, +`check`, and `analyze` drive the same `wright-driver` selection, so both +surfaces return the same selected set for the same input and selection. + +When a request applies any selection field, the result reports +`selection: {"total": , "withheld": }`: +`findings` becomes `{"findings": [...], "selection": {...}}`, while `lint` +and `costEstimate` add a `selection` member to their existing result objects. +Requests without selection fields receive the previous shapes unchanged. + Edit transactions use source identities and half-open, 1-based line/column ranges. Provider positions use 0-based line/character coordinates. Wright proposes and validates edits; a caller remains responsible for applying them. diff --git a/docs/cli/commands.md b/docs/cli/commands.md index 97730e40..30e2a5a4 100644 --- a/docs/cli/commands.md +++ b/docs/cli/commands.md @@ -15,7 +15,8 @@ CompilerSession (wright-driver) ├─ emission (Workshop text) └─ reconstruction (WIR → canonical OPY source, #126) ↓ - Envelope (typed result + diagnostics + exit code) + Envelope (typed result + diagnostics + exit code; finding selection + narrows reported sets without touching the verdict, #430) ↓ `wright` CLI: text rendering | JSON serialization ``` @@ -65,6 +66,12 @@ The rationale for current-directory defaults, directory targets, and explicit ownership ambiguity is recorded in [`ADR-0016`](../adr/0016-current-directory-and-directory-project-targets.md). +Commands that report findings (`check`, `analyze`, `lint`) share the +finding-selection options `--severity`, `--rule-id`, `--file`, and `--max`, +which narrow reported diagnostics/findings without changing verdicts or exit +codes; see [lint configuration and findings](lint.md) and +[presentation](presentation.md). + ## `wright convert` and the reconstruction surface (#126) `wright convert [INPUT] --target opy|ostw` reconstructs **validated Workshop diff --git a/docs/cli/lint.md b/docs/cli/lint.md index 77b660d7..1999695f 100644 --- a/docs/cli/lint.md +++ b/docs/cli/lint.md @@ -20,6 +20,31 @@ The following lint-only flags configure the registry and are repeatable: These flags are usage errors on every other command (exit 2). +`lint`, `check`, and `analyze` additionally share the finding-selection +options (#430). They narrow the *reported* findings/diagnostics through the +shared `wright-driver` selection — the same model the agent `findings`, +`lint`, and `costEstimate` operations expose — and never change the verdict +or the exit code, which always reflect the complete set: + +* `--severity `: report findings at or above a severity + threshold (`error` reports errors only, `info` reports everything). +* `--rule-id `: report findings produced by one rule id. An unknown id + is a usage error (exit 2), never a silent empty result. +* `--file `: report findings located in one source file, matched + exactly against the resolved `span.path`. +* `--max `: report at most N findings. + +When `max` drops findings, the result reports how many were withheld: the +JSON envelope adds `selection` — `{"total": , +"withheld": }` — beside the filtered `findings`/`diagnostics` +arrays, and text output prints a `... N finding(s) withheld` line. The verdict +metadata keeps reporting the true total (for example `10 finding(s) across +6 rule(s)`), so a selected result is never presented as complete. + +In text mode, consecutive lint findings sharing a rule id and message +collapse into one entry that lists its locations instead of repeating the +message and a source-context line per occurrence. + `ongoing-condition-hot-path` is a heuristic about the per-tick evaluation of an `Ongoing - Global` or `Ongoing - Each Player` rule's conditions. Each tick evaluates conditions in source order until one short-circuits the rule, so a diff --git a/docs/cli/presentation.md b/docs/cli/presentation.md index 3836daa5..4da0cfa7 100644 --- a/docs/cli/presentation.md +++ b/docs/cli/presentation.md @@ -76,6 +76,16 @@ Workflow commands accept these CLI-only presentation options: precedence over environment detection; GitHub Actions keeps workflow command lines free of ANSI even when color is explicitly requested. +`check`, `analyze`, and `lint` also accept the finding-selection options +`--severity`, `--rule-id`, `--file`, and `--max` (#430), applied by the +driver to reported diagnostics and lint findings. Selection is a rendering +concern only: it runs after the verdict and exit code are fixed on the +complete set, so it can never turn a failing command into `exit 0` or a +`WARN` verdict into `PASS`. Consecutive lint findings sharing a rule id and +message render as one entry listing all locations; when `--max` withholds +findings, text output states the withheld count and the JSON envelope carries +`selection.total`/`selection.withheld` beside the filtered array. + JSON output is one `wright-result/v1` envelope on stdout with no ANSI, progress, or workflow commands. `compile` and `convert` source artifacts remain the only stdout payload in text mode, including when GitHub Actions presentation is diff --git a/schemas/wright-agent-v1.schema.json b/schemas/wright-agent-v1.schema.json index 04c21496..0d76fc74 100644 --- a/schemas/wright-agent-v1.schema.json +++ b/schemas/wright-agent-v1.schema.json @@ -411,7 +411,30 @@ }, "additionalProperties": true }, - "FindingsResult": { "type": "array", "items": { "$ref": "#/$defs/Finding" } }, + "SelectionSummary": { + "type": "object", + "required": ["total", "withheld"], + "properties": { + "total": { "type": "integer", "minimum": 0 }, + "withheld": { "type": "integer", "minimum": 0 } + }, + "additionalProperties": true + }, + "FindingsResult": { + "anyOf": [ + { "type": "array", "items": { "$ref": "#/$defs/Finding" } }, + { "$ref": "#/$defs/FindingsSelectionResult" } + ] + }, + "FindingsSelectionResult": { + "type": "object", + "required": ["findings", "selection"], + "properties": { + "findings": { "type": "array", "items": { "$ref": "#/$defs/Finding" } }, + "selection": { "$ref": "#/$defs/SelectionSummary" } + }, + "additionalProperties": true + }, "PersistentObjectsResult": { "type": "array", "items": { @@ -439,7 +462,8 @@ "rules": { "type": "array", "items": { "$ref": "#/$defs/LintRule" } }, "config": { "$ref": "#/$defs/LintConfiguration" }, "findings": { "type": "array", "items": { "$ref": "#/$defs/Finding" } }, - "skipped": { "type": "array", "items": { "$ref": "#/$defs/SkippedRule" } } + "skipped": { "type": "array", "items": { "$ref": "#/$defs/SkippedRule" } }, + "selection": { "$ref": "#/$defs/SelectionSummary" } }, "additionalProperties": true }, @@ -473,7 +497,8 @@ "additionalProperties": true }, "findings": { "type": "array", "items": { "type": "object", "required": ["code", "severity", "message"], "properties": { "code": { "type": "string" }, "severity": { "type": "string" }, "message": { "type": "string" } }, "additionalProperties": true } }, - "kind": { "type": "object", "required": ["exact", "findings", "performance"], "properties": { "exact": { "type": "string" }, "findings": { "type": "string" }, "performance": { "type": "string" } }, "additionalProperties": true } + "kind": { "type": "object", "required": ["exact", "findings", "performance"], "properties": { "exact": { "type": "string" }, "findings": { "type": "string" }, "performance": { "type": "string" } }, "additionalProperties": true }, + "selection": { "$ref": "#/$defs/SelectionSummary" } }, "additionalProperties": true }, @@ -698,12 +723,45 @@ "required": ["op", "rule"], "additionalProperties": false }, - "FindingsRequest": { "type": "object", "properties": { "op": { "const": "findings" } }, "required": ["op"], "additionalProperties": false }, + "FindingsRequest": { + "type": "object", + "properties": { + "op": { "const": "findings" }, + "severity": { "enum": ["error", "warning", "info"] }, + "rule": { "type": "string" }, + "file": { "type": "string" }, + "max": { "type": "integer", "minimum": 0 } + }, + "required": ["op"], + "additionalProperties": false + }, "PersistentObjectsRequest": { "type": "object", "properties": { "op": { "const": "persistentObjects" } }, "required": ["op"], "additionalProperties": false }, - "LintRequest": { "type": "object", "properties": { "op": { "const": "lint" } }, "required": ["op"], "additionalProperties": false }, + "LintRequest": { + "type": "object", + "properties": { + "op": { "const": "lint" }, + "severity": { "enum": ["error", "warning", "info"] }, + "rule": { "type": "string" }, + "file": { "type": "string" }, + "max": { "type": "integer", "minimum": 0 } + }, + "required": ["op"], + "additionalProperties": false + }, "LintRulesRequest": { "type": "object", "properties": { "op": { "const": "lintRules" } }, "required": ["op"], "additionalProperties": false }, "CallGraphRequest": { "type": "object", "properties": { "op": { "const": "callGraph" } }, "required": ["op"], "additionalProperties": false }, - "CostEstimateRequest": { "type": "object", "properties": { "op": { "const": "costEstimate" } }, "required": ["op"], "additionalProperties": false }, + "CostEstimateRequest": { + "type": "object", + "properties": { + "op": { "const": "costEstimate" }, + "severity": { "enum": ["error", "warning", "info"] }, + "rule": { "type": "string" }, + "file": { "type": "string" }, + "max": { "type": "integer", "minimum": 0 } + }, + "required": ["op"], + "additionalProperties": false + }, "TargetMetadataRequest": { "type": "object", "properties": { "op": { "const": "targetMetadata" } }, "required": ["op"], "additionalProperties": false }, "ValidateEditTransactionRequest": { "type": "object", diff --git a/schemas/wright-check-v1.schema.json b/schemas/wright-check-v1.schema.json index 8561fd55..a16882ee 100644 --- a/schemas/wright-check-v1.schema.json +++ b/schemas/wright-check-v1.schema.json @@ -23,6 +23,15 @@ "type": "array", "items": { "$ref": "#/$defs/diagnostic" } }, + "selection": { + "type": "object", + "additionalProperties": false, + "required": ["total", "withheld"], + "properties": { + "total": { "type": "integer", "minimum": 0 }, + "withheld": { "type": "integer", "minimum": 0 } + } + }, "result": { "type": "object" } }, "$defs": { From 5bad6ad5939ef492136fde8084ef4fda39d9dddd Mon Sep 17 00:00:00 2001 From: Teakowa <27560638+Teakowa@users.noreply.github.com> Date: Tue, 29 Sep 2026 22:51:39 +0800 Subject: [PATCH 2/2] fix(driver): match finding-selection file by canonical identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reported span.path spelling differs per surface — lint findings resolve root-relative while check/analyze envelope diagnostics carry the cwd-relative display path — so an exact string match silently selected nothing for the input path exactly as passed. Resolve both the file argument and each reported span.path under the session bases (invocation cwd and input root) to a canonical path; pseudo-paths keep exact-match semantics. Also correct the Envelope::selection doc: the member is emitted whenever a selection is applied, not only when it reduces the set. --- crates/wright-cli/src/cli.rs | 7 +- crates/wright-cli/tests/cli.rs | 11 +- crates/wright-driver/src/result.rs | 2 +- crates/wright-driver/src/select.rs | 184 ++++++++++++++++--- crates/wright-driver/src/service.rs | 9 +- crates/wright-driver/src/session.rs | 35 +++- crates/wright-driver/src/session/semantic.rs | 5 +- docs/agent-contract.md | 6 +- docs/cli/lint.md | 6 +- 9 files changed, 220 insertions(+), 45 deletions(-) diff --git a/crates/wright-cli/src/cli.rs b/crates/wright-cli/src/cli.rs index d5febba8..0ff414a9 100644 --- a/crates/wright-cli/src/cli.rs +++ b/crates/wright-cli/src/cli.rs @@ -57,7 +57,7 @@ LINT OPTIONS: FINDING SELECTION (check, analyze, lint): --severity Report findings at or above a severity: error|warning|info --rule-id Report findings from one lint rule id only - --file Report findings in one source file (resolved span.path) + --file Report findings in one source file (any spelling that resolves to it) --max Report at most N findings (withheld counts are shown) UPDATE OPTIONS: @@ -145,8 +145,9 @@ pub(crate) struct SelectArgs { /// a usage error. #[arg(long, value_name = "ID")] pub(crate) rule_id: Option, - /// Report findings located in this source file only (the resolved - /// span.path, e.g. `src/main.ws`). + /// Report findings located in this source file only; any spelling that + /// resolves to the same file (as passed, root-relative, or absolute) + /// selects it. #[arg(long, value_name = "PATH")] pub(crate) file: Option, /// Report at most N findings; withheld findings are reported, never diff --git a/crates/wright-cli/tests/cli.rs b/crates/wright-cli/tests/cli.rs index 4063ceeb..3516ed73 100644 --- a/crates/wright-cli/tests/cli.rs +++ b/crates/wright-cli/tests/cli.rs @@ -583,7 +583,9 @@ fn lint_selection_filters_findings_and_reports_withheld() { assert_eq!(envelope["result"]["selection"]["total"], 10); assert_eq!(envelope["result"]["selection"]["withheld"], 0); - // --file matches the resolved span.path exactly. + // --file selects by file identity: the reported span.path is + // root-relative, and the absolute input path exactly as passed selects + // the same file. let reported_path = parse_json(&run(&["lint", path, "-f", "json"]).stdout) ["result"]["findings"][0]["span"]["path"] .as_str() @@ -592,6 +594,13 @@ fn lint_selection_filters_findings_and_reports_withheld() { let output = run(&["lint", path, "--file", &reported_path, "-f", "json"]); let envelope = parse_json(&output.stdout); assert_eq!(envelope["result"]["findings"].as_array().unwrap().len(), 10); + let output = run(&["lint", path, "--file", path, "-f", "json"]); + let envelope = parse_json(&output.stdout); + assert_eq!( + envelope["result"]["findings"].as_array().unwrap().len(), + 10, + "the input path as passed resolves to the reported root-relative span.path" + ); let output = run(&["lint", path, "--file", "other.ws", "-f", "json"]); let envelope = parse_json(&output.stdout); assert!( diff --git a/crates/wright-driver/src/result.rs b/crates/wright-driver/src/result.rs index d55d75ec..db76807d 100644 --- a/crates/wright-driver/src/result.rs +++ b/crates/wright-driver/src/result.rs @@ -29,7 +29,7 @@ pub struct Envelope { /// Diagnostics remaining after selection; `ok`/`exit` always reflect the /// full set, never the selected remainder. pub diagnostics: Vec, - /// Present when a finding selection reduced `diagnostics` (#430). + /// Present when a finding selection was applied to `diagnostics` (#430). #[serde(skip_serializing_if = "Option::is_none")] pub selection: Option, pub result: T, diff --git a/crates/wright-driver/src/select.rs b/crates/wright-driver/src/select.rs index f853d850..7cd1e4f5 100644 --- a/crates/wright-driver/src/select.rs +++ b/crates/wright-driver/src/select.rs @@ -8,10 +8,14 @@ //! [`SelectionOutcome`] keeps the true total, the truncation count, and the //! full set's highest severity so no surface can understate the result. +use std::collections::BTreeSet; +use std::path::{Path, PathBuf}; + use serde::{Deserialize, Serialize}; use wright_analyzer::registry::{LintConfig, LintRegistry}; use crate::diag::{Diagnostic, Severity}; +use crate::input::ResolvedInput; /// A finding/diagnostic selection: `severity`/`rule`/`file` filter the set, /// `max` truncates what remains. @@ -24,8 +28,10 @@ pub struct FindingSelection { /// Select only findings produced by this rule id (the finding `code`). #[serde(default, skip_serializing_if = "Option::is_none")] pub rule: Option, - /// Select only findings located in this source file, matched exactly - /// against the resolved `span.path`. + /// Select only findings located in this source file, matched by file + /// identity: the reported `span.path` spelling differs per surface + /// (root-relative findings, cwd-relative diagnostics), so any spelling + /// that resolves to the same file selects it. #[serde(default, skip_serializing_if = "Option::is_none")] pub file: Option, /// Report at most this many findings after filtering. @@ -90,46 +96,64 @@ impl FindingSelection { Ok(()) } - /// Apply the selection to envelope diagnostics. + /// Apply the selection to envelope diagnostics. `file_bases` are the + /// bases under which `file` and reported `span.path` spellings resolve + /// (see [`file_bases`]); an empty slice keeps exact-match semantics. pub fn apply_diagnostics( &self, diagnostics: Vec, + file_bases: &[PathBuf], ) -> (Vec, Option) { - self.apply(diagnostics, |diagnostic| { - ( - diagnostic.severity, - diagnostic.code.as_str(), - diagnostic.span.as_ref().map(|span| span.path.as_str()), - ) - }) + self.apply( + diagnostics, + |diagnostic| { + ( + diagnostic.severity, + diagnostic.code.as_str(), + diagnostic.span.as_ref().map(|span| span.path.as_str()), + ) + }, + file_bases, + ) } /// Apply the selection to serialized findings (`code`, `severity`, - /// `span.path` fields). + /// `span.path` fields); `file_bases` resolves `file` like + /// [`apply_diagnostics`](Self::apply_diagnostics). pub fn apply_findings( &self, findings: Vec, + file_bases: &[PathBuf], ) -> (Vec, Option) { - self.apply(findings, |finding| { - ( - parse_severity(finding.get("severity").and_then(|v| v.as_str())), - finding.get("code").and_then(|v| v.as_str()).unwrap_or(""), - finding - .get("span") - .and_then(|span| span.get("path")) - .and_then(|path| path.as_str()), - ) - }) + self.apply( + findings, + |finding| { + ( + parse_severity(finding.get("severity").and_then(|v| v.as_str())), + finding.get("code").and_then(|v| v.as_str()).unwrap_or(""), + finding + .get("span") + .and_then(|span| span.get("path")) + .and_then(|path| path.as_str()), + ) + }, + file_bases, + ) } fn apply( &self, items: Vec, key: impl Fn(&T) -> (Severity, &str, Option<&str>), + file_bases: &[PathBuf], ) -> (Vec, Option) { if !self.is_active() { return (items, None); } + let file_matcher = self + .file + .as_deref() + .map(|file| FileMatch::new(file, file_bases)); let total = items.len(); let max_severity = items .iter() @@ -142,10 +166,9 @@ impl FindingSelection { self.severity .is_none_or(|min| severity_rank(severity) >= severity_rank(min)) && self.rule.as_ref().is_none_or(|rule| rule == code) - && self - .file + && file_matcher .as_ref() - .is_none_or(|file| Some(file.as_str()) == path) + .is_none_or(|matcher| matcher.matches(path)) }) .collect(); let withheld = self.max.map_or(0, |max| kept.len().saturating_sub(max)); @@ -163,6 +186,60 @@ impl FindingSelection { } } +/// The `file` dimension matched by file identity rather than one string +/// spelling: `span.path` is reported root-relative on finding surfaces and +/// cwd-relative on diagnostic surfaces, so both sides resolve under the +/// session's bases (invocation cwd and the input root) to a canonical path. +/// Pseudo-paths (``, ``, ``) do not resolve +/// and keep exact-match semantics only. +struct FileMatch { + spelled: String, + canonical: BTreeSet, + bases: Vec, +} + +impl FileMatch { + fn new(file: &str, bases: &[PathBuf]) -> FileMatch { + FileMatch { + canonical: canonical_forms(Path::new(file), bases), + spelled: file.to_string(), + bases: bases.to_vec(), + } + } + + fn matches(&self, path: Option<&str>) -> bool { + let Some(path) = path else { return false }; + path == self.spelled + || canonical_forms(Path::new(path), &self.bases) + .iter() + .any(|form| self.canonical.contains(form)) + } +} + +/// The bases under which `file` selection arguments and reported +/// `span.path` spellings resolve for one session input: the invocation +/// working directory and the input include root. +pub(crate) fn file_bases(input: &ResolvedInput) -> Vec { + vec![input.cwd.clone(), input.root.clone()] +} + +/// The canonical paths `path` resolves to under each base; absolute paths +/// resolve once regardless of base, and unresolvable spellings (virtual or +/// missing files) contribute nothing. +fn canonical_forms(path: &Path, bases: &[PathBuf]) -> BTreeSet { + bases + .iter() + .filter_map(|base| { + let full = if path.is_absolute() { + path.to_path_buf() + } else { + base.join(path) + }; + full.canonicalize().ok() + }) + .collect() +} + #[cfg(test)] mod tests { use super::*; @@ -184,7 +261,7 @@ mod tests { #[test] fn inactive_selection_returns_items_untouched() { let items = vec![finding("a", "warning", None)]; - let (kept, outcome) = FindingSelection::default().apply_findings(items.clone()); + let (kept, outcome) = FindingSelection::default().apply_findings(items.clone(), &[]); assert_eq!(kept, items); assert!(outcome.is_none()); } @@ -196,7 +273,7 @@ mod tests { finding("b", "warning", None), finding("c", "error", None), ]; - let (kept, outcome) = selection(json!({"severity": "warning"})).apply_findings(items); + let (kept, outcome) = selection(json!({"severity": "warning"})).apply_findings(items, &[]); assert_eq!( kept.iter() .map(|f| f["code"].as_str().unwrap()) @@ -216,12 +293,59 @@ mod tests { finding("a", "warning", Some("y.ws")), finding("b", "warning", Some("x.ws")), ]; - let (kept, _) = selection(json!({"rule": "a", "file": "x.ws"})).apply_findings(items); + let (kept, _) = selection(json!({"rule": "a", "file": "x.ws"})).apply_findings(items, &[]); assert_eq!(kept.len(), 1); // No span path never matches a file selection. + let (kept, _) = selection(json!({"file": "x.ws"})) + .apply_findings(vec![finding("a", "warning", None)], &[]); + assert!(kept.is_empty()); + } + + #[test] + fn file_selection_matches_by_canonical_identity() { + // #430 review: `span.path` spellings differ per surface — findings + // report root-relative paths, envelope diagnostics report the + // cwd-relative display path. `file` must select the same file from + // any spelling that resolves to it. + let root = std::env::temp_dir().join(format!("wright-select-{}", std::process::id())); + std::fs::create_dir_all(root.join("sub")).unwrap(); + let abs = root.join("sub/f.ws"); + std::fs::write(&abs, "x").unwrap(); + let input = ResolvedInput { + kind: crate::config::SourceKind::Workshop, + text: String::new(), + path: Some(abs.clone()), + target: crate::input::InputTarget::File, + root: root.clone(), + cwd: std::env::current_dir().unwrap(), + display: abs.display().to_string(), + identity: String::new(), + origin: crate::diag::Origin { + kind: "workshop".to_string(), + locale: None, + }, + }; + + // An absolute `file` selects a finding reported root-relative. + let items = vec![finding("a", "warning", Some("sub/f.ws"))]; + let (kept, _) = selection(json!({"file": abs.to_str().unwrap()})) + .apply_findings(items, &file_bases(&input)); + assert_eq!(kept.len(), 1); + + // A root-relative `file` selects a finding reported absolute/cwd + // relative (the display form). + let items = vec![finding("a", "warning", Some(abs.to_str().unwrap()))]; let (kept, _) = - selection(json!({"file": "x.ws"})).apply_findings(vec![finding("a", "warning", None)]); + selection(json!({"file": "sub/f.ws"})).apply_findings(items, &file_bases(&input)); + assert_eq!(kept.len(), 1); + + // A different file name still selects nothing. + let items = vec![finding("a", "warning", Some("sub/f.ws"))]; + let (kept, _) = + selection(json!({"file": "sub/other.ws"})).apply_findings(items, &file_bases(&input)); assert!(kept.is_empty()); + + std::fs::remove_dir_all(&root).unwrap(); } #[test] @@ -232,7 +356,7 @@ mod tests { finding("c", "warning", None), finding("d", "warning", None), ]; - let (kept, outcome) = selection(json!({"max": 1})).apply_findings(items); + let (kept, outcome) = selection(json!({"max": 1})).apply_findings(items, &[]); assert_eq!(kept.len(), 1); let outcome = outcome.unwrap(); assert_eq!(outcome.total, 4); @@ -246,7 +370,7 @@ mod tests { Diagnostic::warning("warn-code", crate::diag::Stage::Analysis, "b"), ]; items[0].severity = Severity::Error; - let (kept, outcome) = selection(json!({"severity": "error"})).apply_diagnostics(items); + let (kept, outcome) = selection(json!({"severity": "error"})).apply_diagnostics(items, &[]); assert_eq!(kept.len(), 1); assert_eq!(kept[0].code, "parse-error"); assert_eq!(outcome.unwrap().max_severity, Some(Severity::Error)); diff --git a/crates/wright-driver/src/service.rs b/crates/wright-driver/src/service.rs index a5e51471..54eb5047 100644 --- a/crates/wright-driver/src/service.rs +++ b/crates/wright-driver/src/service.rs @@ -429,7 +429,8 @@ impl<'a> ToolService<'a> { serde_json::Value::Array(findings) => findings, _ => Vec::new(), }; - let (findings, outcome) = selection.apply_findings(findings); + let (findings, outcome) = selection + .apply_findings(findings, &crate::select::file_bases(&self.loaded.input)); match outcome { Some(outcome) => self.ok(json!({ "findings": findings, "selection": outcome })), None => self.ok(serde_json::Value::Array(findings)), @@ -501,7 +502,8 @@ impl<'a> ToolService<'a> { serde_json::Value::Array(findings) => findings, _ => Vec::new(), }; - let (findings, outcome) = selection.apply_findings(findings); + let (findings, outcome) = + selection.apply_findings(findings, &crate::select::file_bases(&self.loaded.input)); let mut result = json!({ "inputIdentity": self.loaded.input.identity, "rules": crate::result::compact_lint_rules(lint_rules.get("rules")), @@ -586,7 +588,8 @@ impl<'a> ToolService<'a> { }) }) .collect::>(); - let (findings, outcome) = selection.apply_findings(findings); + let (findings, outcome) = + selection.apply_findings(findings, &crate::select::file_bases(&self.loaded.input)); let mut result = json!({ "exact": { "emittedBytes": text.len(), diff --git a/crates/wright-driver/src/session.rs b/crates/wright-driver/src/session.rs index 40572dff..c2e6a441 100644 --- a/crates/wright-driver/src/session.rs +++ b/crates/wright-driver/src/session.rs @@ -6,7 +6,7 @@ mod semantic; pub(crate) use semantic::resolve_span_paths; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::sync::Arc; use workshop_rs::Program; @@ -771,12 +771,43 @@ impl CompilerSession { &self.lint_registry } + /// The bases under which a `file` selection and reported `span.path` + /// spellings resolve (#430): the loaded input's cwd and root, or the + /// config-derived equivalents so pre-load diagnostics select the same + /// way. Extra bases are safe — they only ever add true file identities. + fn selection_file_bases(&self) -> Vec { + if let Some(loaded) = &self.loaded { + return crate::select::file_bases(&loaded.input); + } + let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); + let absolute = |path: &Path| { + if path.is_absolute() { + path.to_path_buf() + } else { + cwd.join(path) + } + }; + let mut bases = vec![cwd.clone()]; + if let Some(root) = &self.config.root { + bases.push(absolute(root)); + } + if let InputSpec::Path(path) = &self.config.input { + let path = absolute(path); + bases.push(path.clone()); + if let Some(parent) = path.parent() { + bases.push(parent.to_path_buf()); + } + } + bases + } + fn finish(&mut self, command: &str, result: T) -> Envelope { let diagnostics = std::mem::take(&mut self.diagnostics); let exit = exit_code_from(&diagnostics); // Selection narrows the reported diagnostics after the exit code is // fixed; filtering must never change the verdict. - let (diagnostics, selection) = self.config.selection.apply_diagnostics(diagnostics); + let bases = self.selection_file_bases(); + let (diagnostics, selection) = self.config.selection.apply_diagnostics(diagnostics, &bases); Envelope { wright: version_info(), command: command.to_string(), diff --git a/crates/wright-driver/src/session/semantic.rs b/crates/wright-driver/src/session/semantic.rs index 2fd31002..d62923ab 100644 --- a/crates/wright-driver/src/session/semantic.rs +++ b/crates/wright-driver/src/session/semantic.rs @@ -245,7 +245,10 @@ impl CompilerSession { serde_json::Value::Array(findings) => findings, _ => Vec::new(), }; - let (findings, selection) = session.config.selection.apply_findings(findings); + let (findings, selection) = session + .config + .selection + .apply_findings(findings, &crate::select::file_bases(&loaded.input)); let (rules, config, skipped) = if let serde_json::Value::Object(mut object) = lint_rules { ( diff --git a/docs/agent-contract.md b/docs/agent-contract.md index 0b0f003f..31b49c10 100644 --- a/docs/agent-contract.md +++ b/docs/agent-contract.md @@ -100,8 +100,10 @@ the successful `result` payload. warnings, `info` everything. * `rule`: one lint rule id (the finding `code`). An unknown id is a structured `invalid-selection` error, never a silent empty result. -* `file`: one source file, matched exactly against the resolved `span.path`. - `costEstimate` findings carry no span, so `file` selects nothing there. +* `file`: one source file. The reported `span.path` spelling differs per + surface, so the argument resolves to the same canonical file — the path as + passed, root-relative, or absolute spellings all select it. `costEstimate` + findings carry no span, so `file` selects nothing there. * `max`: a bound on the reported count, applied after filtering. The CLI options `--severity`, `--rule-id`, `--file`, and `--max` on `lint`, diff --git a/docs/cli/lint.md b/docs/cli/lint.md index 65da3708..67b36ea0 100644 --- a/docs/cli/lint.md +++ b/docs/cli/lint.md @@ -30,8 +30,10 @@ or the exit code, which always reflect the complete set: threshold (`error` reports errors only, `info` reports everything). * `--rule-id `: report findings produced by one rule id. An unknown id is a usage error (exit 2), never a silent empty result. -* `--file `: report findings located in one source file, matched - exactly against the resolved `span.path`. +* `--file `: report findings located in one source file. The reported + `span.path` spelling differs per surface (root-relative findings, + cwd-relative diagnostics), so any spelling that resolves to the same file — + as passed, root-relative, or absolute — selects it. * `--max `: report at most N findings. When `max` drops findings, the result reports how many were withheld: the