diff --git a/crates/wright-bench/src/main.rs b/crates/wright-bench/src/main.rs index abfc9e2..e493196 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 4eff588..0ff414a 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 (any spelling that resolves to it) + --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,39 @@ 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; 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 + /// 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 +191,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 +340,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 50f1e8f..ab5f219 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 5de00dd..ac3d35a 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 b300429..f034691 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 311cd6c..3516ed7 100644 --- a/crates/wright-cli/tests/cli.rs +++ b/crates/wright-cli/tests/cli.rs @@ -540,6 +540,201 @@ 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 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() + .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", 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!( + 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 0f7ad17..506734b 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 2bb79be..76edc84 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 18a433f..29502f5 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 5ccb890..4e7583e 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 af49b3a..db76807 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 was applied to `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, } /// `lint` results keep only the per-rule identity needed to interpret a diff --git a/crates/wright-driver/src/select.rs b/crates/wright-driver/src/select.rs new file mode 100644 index 0000000..7cd1e4f --- /dev/null +++ b/crates/wright-driver/src/select.rs @@ -0,0 +1,405 @@ +//! 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 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. +#[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 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. + #[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. `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()), + ) + }, + file_bases, + ) + } + + /// Apply the selection to serialized findings (`code`, `severity`, + /// `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()), + ) + }, + 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() + .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) + && file_matcher + .as_ref() + .is_none_or(|matcher| matcher.matches(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, + }), + ) + } +} + +/// 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::*; + 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 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": "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] + 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 c2956eb..54eb504 100644 --- a/crates/wright-driver/src/service.rs +++ b/crates/wright-driver/src/service.rs @@ -57,20 +57,23 @@ 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 per-rule id/effective severity and the effective /// configuration (#98); `lintRules` serves full rule metadata (#431). - Lint, + /// Optionally narrowed by an inline selection (#430). + Lint(crate::select::FindingSelection), /// The registered lint rules with full metadata 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 @@ -270,12 +273,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, @@ -412,9 +415,29 @@ 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, &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)), + } + } + other => other, + } } /// `persistentObjects`: persistent-object facts with resolved source paths. @@ -441,12 +464,30 @@ 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`: per-rule id and effective severity, effective configuration, /// and findings over the loaded program through the same semantic-service /// path as the CLI `lint` workflow (no duplicated rule execution, #98). /// Full rule metadata is served once by `lintRules` rather than inlined - /// into every `lint` response (#431). - fn lint(&self) -> ToolResponse { + /// into every `lint` response (#431). 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, @@ -457,13 +498,23 @@ 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, &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")), "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. @@ -504,7 +555,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) @@ -521,24 +578,36 @@ 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, &crate::select::file_bases(&self.loaded.input)); + 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 b3d1f4d..c2e6a44 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; @@ -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,56 @@ impl CompilerSession { } } + /// The lint registry backing this session, for finding-selection + /// validation (#430). + pub(crate) fn lint_registry(&self) -> &LintRegistry { + &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 bases = self.selection_file_bases(); + let (diagnostics, selection) = self.config.selection.apply_diagnostics(diagnostics, &bases); 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 2f8797f..d62923a 100644 --- a/crates/wright-driver/src/session/semantic.rs +++ b/crates/wright-driver/src/session/semantic.rs @@ -241,6 +241,14 @@ 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, &crate::select::file_bases(&loaded.input)); let (rules, config, skipped) = if let serde_json::Value::Object(mut object) = lint_rules { ( @@ -264,8 +272,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 16a89d3..9b12697 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:?}"), }; @@ -107,7 +109,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:?}"), }; @@ -146,6 +149,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 035a00a..31b49c1 100644 --- a/docs/agent-contract.md +++ b/docs/agent-contract.md @@ -80,18 +80,42 @@ 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, per-rule id/effective severity, and effective configuration | +| `lint` | optional selection | Lint findings, per-rule id/effective severity, effective configuration, and `selection` when applied | | `lintRules` | none | Registered lint rules with full metadata 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. 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`, +`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 9dbafed..c3b95ac 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 a9e9845..67b36ea 100644 --- a/docs/cli/lint.md +++ b/docs/cli/lint.md @@ -20,6 +20,33 @@ 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. 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 +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 3836daa..4da0cfa 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 70540b4..72ad58c 100644 --- a/schemas/wright-agent-v1.schema.json +++ b/schemas/wright-agent-v1.schema.json @@ -420,7 +420,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": { @@ -448,7 +471,8 @@ "rules": { "type": "array", "items": { "$ref": "#/$defs/LintRuleSummary" } }, "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 }, @@ -482,7 +506,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 }, @@ -707,12 +732,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 8561fd5..a16882e 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": {