diff --git a/crates/wright-analyzer/src/canonical/service.rs b/crates/wright-analyzer/src/canonical/service.rs index ee284f3..752562d 100644 --- a/crates/wright-analyzer/src/canonical/service.rs +++ b/crates/wright-analyzer/src/canonical/service.rs @@ -7,7 +7,9 @@ use workshop_rs::{Event, Program}; use super::analysis::Finding; use super::cfg::cfg_response; use super::facts::persistent_objects; -use super::symbols::{Reference, ReferenceKind, RuleId, SemanticIndex, Symbol, SymbolId}; +use super::symbols::{ + Reference, ReferenceKind, RuleId, SemanticIndex, Symbol, SymbolId, SymbolKind, +}; use crate::analysis::Boundedness; use crate::registry::{LintConfig, SkippedRule}; use crate::service::{ErrorInfo, Origin, Request, Response}; @@ -155,6 +157,66 @@ impl<'a> SemanticService<'a> { .collect(), ) } + + /// Resolve a symbol by its exact declared name (#429). A name matching no + /// symbol is `unknown-symbol`; a name shared by more than one symbol is + /// `ambiguous-symbol` with each candidate's kind and numeric id — callers + /// may fall back to the numeric form. No match is ever guessed. + pub fn resolve_symbol(&self, name: &str) -> Result<&Symbol, ErrorInfo> { + let matches: Vec<&Symbol> = self + .index + .symbols() + .filter(|symbol| symbol.name == name) + .collect(); + match matches.as_slice() { + [symbol] => Ok(symbol), + [] => Err(ErrorInfo { + code: "unknown-symbol".to_string(), + message: format!("unknown symbol '{name}'"), + }), + _ => Err(ErrorInfo { + code: "ambiguous-symbol".to_string(), + message: format!( + "ambiguous symbol '{name}': {}", + matches + .iter() + .map(|symbol| format!("{} {}", symbol.kind.as_str(), symbol.id.index())) + .collect::>() + .join(", ") + ), + }), + } + } + + /// Resolve a rule by its exact declared name to its rule index (#429). + /// Only rule symbols participate: a rule named `x` stays reachable even + /// when a variable or subroutine is also named `x`. Unmatched and + /// duplicate rule names are `unknown-rule`/`ambiguous-rule`. + pub fn resolve_rule(&self, name: &str) -> Result { + let matches: Vec<&Symbol> = self + .index + .symbols() + .filter(|symbol| symbol.kind == SymbolKind::Rule && symbol.name == name) + .collect(); + match matches.as_slice() { + [symbol] => Ok(symbol.rule.expect("rule symbols carry their rule index")), + [] => Err(ErrorInfo { + code: "unknown-rule".to_string(), + message: format!("unknown rule '{name}'"), + }), + _ => Err(ErrorInfo { + code: "ambiguous-rule".to_string(), + message: format!( + "ambiguous rule '{name}': {}", + matches + .iter() + .map(|symbol| format!("rule {}", symbol.rule.expect("rule symbol"))) + .collect::>() + .join(", ") + ), + }), + } + } pub fn handle_json(&self, request_json: &str) -> String { let request: Request = match serde_json::from_str(request_json) { Ok(req) => req, @@ -180,7 +242,7 @@ impl<'a> SemanticService<'a> { Request::ListSymbols { kind } => Response::Ok { result: json!(self.index.symbols().filter(|symbol| kind.as_deref().is_none_or(|kind| symbol.kind.as_str() == kind)).map(symbol_json).collect::>()) }, Request::GetSymbol { symbol } => self.index.symbol(SymbolId::from_index(*symbol as usize)).map_or_else(|| self.error("invalid-id", format!("unknown symbol {symbol}")), |symbol| Response::Ok { result: symbol_json(symbol) }), Request::FindReferences { symbol } => { let id = SymbolId::from_index(*symbol as usize); if self.index.symbol(id).is_none() { self.error("invalid-id", format!("unknown symbol {symbol}")) } else { Response::Ok { result: json!(self.index.references(id).into_iter().map(reference_json).collect::>()) } } } - Request::GetUsage { symbol } => { let id = SymbolId::from_index(*symbol as usize); self.index.symbol(id).map_or_else(|| self.error("invalid-id", format!("unknown symbol {symbol}")), |data| { let usage = self.index.usage(id); Response::Ok { result: json!({"symbol": data.name, "reads": usage.reads, "writes": usage.writes, "calls": usage.calls, "rules": usage.rules}) } }) } + Request::GetUsage { symbol } => { let id = SymbolId::from_index(*symbol as usize); self.index.symbol(id).map_or_else(|| self.error("invalid-id", format!("unknown symbol {symbol}")), |data| { let usage = self.index.usage(id); Response::Ok { result: json!({"id": id.index(), "kind": data.kind.as_str(), "symbol": data.name, "reads": usage.reads, "writes": usage.writes, "calls": usage.calls, "rules": usage.rules}) } }) } Request::GetCfg { rule } => cfg_response(self.program.as_ref(), *rule as usize), Request::GetFindings => Response::Ok { result: json!(self.findings.iter().map(finding_json).collect::>()) }, Request::GetPersistentObjects => Response::Ok { result: json!(persistent_objects(self.program.as_ref())) }, diff --git a/crates/wright-analyzer/tests/workshop_integration.rs b/crates/wright-analyzer/tests/workshop_integration.rs index 4cbad47..0ada636 100644 --- a/crates/wright-analyzer/tests/workshop_integration.rs +++ b/crates/wright-analyzer/tests/workshop_integration.rs @@ -1078,3 +1078,73 @@ fn workshop_references_without_provenance_are_unmapped() { ); } } + +// ── Name addressing (#429) ──────────────────────────────────────────────────── + +const DUPLICATE_NAMES: &str = r#" +variables { + global: + 0: dup + 1: solo +} +rule ("dup") { + event { + Ongoing - Global; + } + actions { + Set Global Variable(dup, 1); + } +} +rule ("dup") { + event { + Ongoing - Global; + } + actions { + Set Global Variable(dup, 2); + } +} +"#; + +#[test] +fn name_resolution_finds_unique_symbols_and_rules() { + let service = workshop_service("synthetic/declarations-rules"); + let score = service.resolve_symbol("score").unwrap(); + assert_eq!(score.name, "score"); + assert_eq!(score.id.index(), 0); + // `player starts` is rule index 1 but symbol id 4 — the two spaces are + // different, which is why callers address by name. + assert_eq!(service.resolve_rule("player starts").unwrap(), 1); + assert_eq!( + service.resolve_symbol("player starts").unwrap().id.index(), + 4 + ); +} + +#[test] +fn name_resolution_rejects_unknown_and_ambiguous_names() { + let service = workshop_service_from_text(DUPLICATE_NAMES); + let error = service.resolve_symbol("nope").unwrap_err(); + assert_eq!(error.code, "unknown-symbol"); + assert!(error.message.contains("nope")); + let error = service.resolve_rule("nope").unwrap_err(); + assert_eq!(error.code, "unknown-rule"); + + // A name matching only a non-rule symbol is still unknown as a rule: + // the rule space resolves rules only. + let error = service.resolve_rule("solo").unwrap_err(); + assert_eq!(error.code, "unknown-rule"); + assert!(service.resolve_symbol("solo").is_ok()); + + // `dup` names one variable and two rules; both resolutions fail with + // the candidate list instead of guessing. + let error = service.resolve_symbol("dup").unwrap_err(); + assert_eq!(error.code, "ambiguous-symbol"); + for candidate in ["globalVariable 0", "rule 2", "rule 3"] { + assert!(error.message.contains(candidate), "{error:?}"); + } + let error = service.resolve_rule("dup").unwrap_err(); + assert_eq!(error.code, "ambiguous-rule"); + for candidate in ["rule 0", "rule 1"] { + assert!(error.message.contains(candidate), "{error:?}"); + } +} diff --git a/crates/wright-bench/src/main.rs b/crates/wright-bench/src/main.rs index e493196..6b3b5bb 100644 --- a/crates/wright-bench/src/main.rs +++ b/crates/wright-bench/src/main.rs @@ -281,9 +281,9 @@ fn semantic_query_trial( let requests = [ ToolRequest::Rules, ToolRequest::Symbols { kind: None }, - ToolRequest::References { symbol: 0 }, - ToolRequest::Usage { symbol: 0 }, - ToolRequest::Cfg { rule: 0 }, + ToolRequest::References { symbol: 0.into() }, + ToolRequest::Usage { symbol: 0.into() }, + ToolRequest::Cfg { rule: 0.into() }, ToolRequest::Findings(FindingSelection::default()), ToolRequest::PersistentObjects, ToolRequest::Lint(FindingSelection::default()), diff --git a/crates/wright-cli/src/cli.rs b/crates/wright-cli/src/cli.rs index 0ff414a..438442a 100644 --- a/crates/wright-cli/src/cli.rs +++ b/crates/wright-cli/src/cli.rs @@ -23,8 +23,10 @@ pub(crate) struct Cli { pub(crate) const LONG_ABOUT: &str = "Wright compiler and Workshop tooling CLI. -Commands check correctness, summarize semantic hotspots, lint, inspect exhaustive -facts, compile, or reconstruct source through the typed wright-driver result envelope. `compile` and `convert` +Commands check correctness, summarize semantic hotspots, lint, compile, or +reconstruct source through the typed wright-driver result envelope. `inspect` +prints the semantic summary, and its query subcommands (symbols, refs, cfg, +callgraph, cost) expose each detail area. `compile` and `convert` keep their source artifact stdout contracts; JSON mode prints only one wright-result/v1 envelope to stdout. `serve` exposes the versioned wright-agent/v1 session contract over stdio or JSON-RPC 2.0. @@ -54,7 +56,7 @@ LINT OPTIONS: --disable-rule Disable a lint rule (repeatable) --rule-severity : Override a lint rule severity (repeatable) -FINDING SELECTION (check, analyze, lint): +FINDING SELECTION (check, analyze, lint, inspect cost): --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) @@ -78,8 +80,13 @@ pub(crate) enum Command { Analyze(ReportArgs), /// Parse, lower, and report lint findings. Lint(LintArgs), - /// Parse, lower, and show exhaustive structural/semantic facts. - Inspect(CommonArgs), + /// Parse, lower, and inspect semantic facts: the bare command prints the + /// summary, and its query subcommands expose each detail area (#429). + #[command( + args_conflicts_with_subcommands = true, + subcommand_precedence_over_arg = true + )] + Inspect(InspectArgs), /// Generate static shell completion from the command model. Completion(CompletionArgs), /// Update a standalone installation. @@ -133,9 +140,9 @@ pub(crate) struct ReportArgs { 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. +/// Finding-selection options shared by `check`, `analyze`, `lint`, and +/// `inspect cost` (#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. @@ -187,6 +194,67 @@ pub(crate) struct CommonArgs { pub(crate) color: ColorArg, } +/// Arguments of `inspect`: an optional query subcommand naming one detail +/// area, plus the shared workflow options used by the bare summary. +#[derive(Debug, Args)] +pub(crate) struct InspectArgs { + #[command(flatten)] + pub(crate) common: CommonArgs, + #[command(subcommand)] + pub(crate) query: Option, +} + +/// The `inspect` query subcommands (#429): each is the CLI entry for the +/// agent operation of the same name, so the top-level command surface stays +/// small (#439). Agent requests keep their flat operation names. +#[derive(Debug, Subcommand)] +pub(crate) enum InspectQuery { + /// List semantic symbols; `--only` narrows to one symbol kind. + Symbols(SymbolsArgs), + /// Show the references and usage counts of one symbol, addressed by name. + Refs(RefsArgs), + /// Show the control-flow graph of one rule, addressed by name. + Cfg(CfgArgs), + /// Show the subroutine call graph. + Callgraph(CommonArgs), + /// Report generated-resource counts and static findings. + Cost(ReportArgs), +} + +/// Arguments of `inspect symbols`: shared workflow options plus the +/// symbol-kind filter. The filter is `--only` rather than `--kind` because +/// `CommonArgs` already assigns `--kind` to input-frontend selection. +#[derive(Debug, Args)] +pub(crate) struct SymbolsArgs { + #[command(flatten)] + pub(crate) common: CommonArgs, + /// Report only symbols of this kind. + #[arg(long, value_enum, value_name = "KIND")] + pub(crate) only: Option, +} + +/// Arguments of `inspect refs`: the symbol name, then the shared workflow +/// options. +#[derive(Debug, Args)] +pub(crate) struct RefsArgs { + /// The declared name of the symbol to look up. + #[arg(value_name = "NAME")] + pub(crate) name: String, + #[command(flatten)] + pub(crate) common: CommonArgs, +} + +/// Arguments of `inspect cfg`: the rule name, then the shared workflow +/// options. +#[derive(Debug, Args)] +pub(crate) struct CfgArgs { + /// The declared name of the rule to look up. + #[arg(value_name = "RULE")] + pub(crate) rule: String, + #[command(flatten)] + pub(crate) common: CommonArgs, +} + #[derive(Debug, Args)] pub(crate) struct LintArgs { #[command(flatten)] @@ -340,6 +408,35 @@ pub(crate) enum ColorArg { Never, } +/// Symbol kinds as spelled by the semantic index; `--only` accepts the +/// canonical names plus kebab-case aliases. +#[derive(Clone, Copy, Debug, Eq, PartialEq, ValueEnum)] +pub(crate) enum SymbolKindArg { + /// `variables.global` symbols. + #[value(name = "globalVariable", alias = "global-variable")] + GlobalVariable, + /// `variables.player` symbols. + #[value(name = "playerVariable", alias = "player-variable")] + PlayerVariable, + /// Subroutines. + #[value(name = "subroutine")] + Subroutine, + /// Rules. + #[value(name = "rule")] + Rule, +} + +impl SymbolKindArg { + pub(crate) fn as_str(&self) -> &'static str { + match self { + Self::GlobalVariable => "globalVariable", + Self::PlayerVariable => "playerVariable", + Self::Subroutine => "subroutine", + Self::Rule => "rule", + } + } +} + #[derive(Clone, Copy, Debug, Eq, PartialEq, ValueEnum)] pub(crate) enum SeverityArg { Error, diff --git a/crates/wright-cli/src/main.rs b/crates/wright-cli/src/main.rs index ab5f219..e5531df 100644 --- a/crates/wright-cli/src/main.rs +++ b/crates/wright-cli/src/main.rs @@ -197,11 +197,45 @@ fn run_workflow(command: Command) -> ExitCode { wright_driver::CompilerSession::lint, ) } - Command::Inspect(args) => run_configured( - config_from_common(&args, false), - present::Presentation::from_common(&args), - wright_driver::CompilerSession::inspect, - ), + Command::Inspect(args) => match args.query { + None => run_configured( + config_from_common(&args.common, false), + present::Presentation::from_common(&args.common), + wright_driver::CompilerSession::inspect, + ), + Some(cli::InspectQuery::Symbols(query)) => { + let kind = query.only.map(|kind| kind.as_str().to_string()); + run_configured( + config_from_common(&query.common, false), + present::Presentation::from_common(&query.common), + move |session| session.symbols(kind), + ) + } + Some(cli::InspectQuery::Refs(query)) => run_configured( + config_from_common(&query.common, false), + present::Presentation::from_common(&query.common), + move |session| session.refs(&query.name), + ), + Some(cli::InspectQuery::Cfg(query)) => run_configured( + config_from_common(&query.common, false), + present::Presentation::from_common(&query.common), + move |session| session.cfg(&query.rule), + ), + Some(cli::InspectQuery::Callgraph(query)) => run_configured( + config_from_common(&query, false), + present::Presentation::from_common(&query), + wright_driver::CompilerSession::callgraph, + ), + Some(cli::InspectQuery::Cost(query)) => { + let mut config = config_from_common(&query.common, true); + config.selection = selection_from_args(&query.select); + run_configured( + config, + present::Presentation::from_common(&query.common), + wright_driver::CompilerSession::cost, + ) + } + }, Command::Completion(_) | Command::Update(_) | Command::Provider(_) diff --git a/crates/wright-cli/src/present.rs b/crates/wright-cli/src/present.rs index ac3d35a..e8c3c76 100644 --- a/crates/wright-cli/src/present.rs +++ b/crates/wright-cli/src/present.rs @@ -12,7 +12,8 @@ use wright_driver::Severity; use wright_driver::config::OutputFormat; use wright_driver::progress::{ProgressEvent, ProgressObserver, ProgressPhase, ProgressUnit}; use wright_driver::result::{ - AnalyzeResult, CheckResult, CompileResult, ConvertResult, Envelope, InspectResult, LintResult, + AnalyzeResult, CallGraphResult, CfgResult, CheckResult, CompileResult, ConvertResult, + CostResult, Envelope, InspectResult, LintResult, RefsResult, SymbolsResult, }; use crate::cli::{ColorArg, CommonArgs, OutputFormatArg, RendererArg}; @@ -548,6 +549,180 @@ impl ResultPresentation for InspectResult { } } +impl ResultPresentation for SymbolsResult { + fn metadata(&self) -> Option { + Some(format!("{} symbol(s)", array_len(&self.0))) + } + fn render_body(&self) { + let symbols = self.0.as_array().map_or(&[][..], Vec::as_slice); + println!("\nSymbols"); + if symbols.is_empty() { + println!(" none"); + } + for symbol in symbols { + println!( + " {} {}", + symbol["kind"].as_str().unwrap_or("symbol"), + symbol["name"].as_str().unwrap_or("") + ); + if let Some(span) = symbol.get("span").filter(|span| span.is_object()) { + print_location(span, " "); + } + } + } +} + +impl ResultPresentation for RefsResult { + fn metadata(&self) -> Option { + Some(format!( + "{} ({}): {} read(s), {} write(s), {} call(s) across {} rule(s)", + self.0["symbol"].as_str().unwrap_or(""), + self.0["kind"].as_str().unwrap_or("symbol"), + count(&self.0, "reads"), + count(&self.0, "writes"), + count(&self.0, "calls"), + count(&self.0, "rules"), + )) + } + fn render_body(&self) { + let references = self.0["references"] + .as_array() + .map_or(&[][..], Vec::as_slice); + println!( + "\nReferences to {}", + self.0["symbol"].as_str().unwrap_or("") + ); + if references.is_empty() { + println!(" none"); + } + for reference in references { + let context = match (reference["rule"].as_u64(), reference["action"].as_u64()) { + (Some(rule), Some(action)) => format!(" (rule {rule}, action {action})"), + (Some(rule), None) => format!(" (rule {rule})"), + _ => String::new(), + }; + println!( + " {}{}", + reference["kind"].as_str().unwrap_or("reference"), + context + ); + if let Some(span) = reference.get("span").filter(|span| span.is_object()) { + print_location(span, " "); + } + } + } +} + +impl ResultPresentation for CfgResult { + fn metadata(&self) -> Option { + Some(format!("{} block(s)", array_len(&self.0["blocks"]))) + } + fn render_body(&self) { + let blocks = self.0["blocks"].as_array().map_or(&[][..], Vec::as_slice); + println!("\nControl-flow graph"); + if blocks.is_empty() { + println!(" none"); + } + for block in blocks { + let mut flags = Vec::new(); + if block["waits"].as_bool().unwrap_or(false) { + flags.push("waits"); + } + if block["calls"].as_bool().unwrap_or(false) { + flags.push("calls"); + } + let flags = if flags.is_empty() { + String::new() + } else { + format!(" [{}]", flags.join(" ")) + }; + println!( + " block {} ({}): {} action(s){}", + block["id"].as_u64().unwrap_or(0), + block["kind"].as_str().unwrap_or("block"), + array_len(&block["actions"]), + flags + ); + if let Some(successors) = block["successors"].as_array() + && !successors.is_empty() + { + let successors = successors + .iter() + .map(|successor| { + format!( + "{} ({})", + successor["to"].as_u64().unwrap_or(0), + successor["kind"].as_str().unwrap_or("edge") + ) + }) + .collect::>() + .join(", "); + println!(" -> {successors}"); + } + } + } +} + +impl ResultPresentation for CallGraphResult { + fn metadata(&self) -> Option { + Some(format!("{} edge(s)", array_len(&self.0))) + } + fn render_body(&self) { + let edges = self.0.as_array().map_or(&[][..], Vec::as_slice); + println!("\nCall graph"); + if edges.is_empty() { + println!(" none"); + } + for edge in edges { + println!( + " {} -> {}", + edge["caller"].as_str().unwrap_or(""), + edge["callee"].as_str().unwrap_or("") + ); + } + } +} + +impl ResultPresentation for CostResult { + fn metadata(&self) -> Option { + let exact = &self.0["exact"]; + Some(format!( + "{} emitted byte(s), {} action(s), {} rule(s), {} wait(s)", + count(exact, "emittedBytes"), + count(exact, "programActions"), + count(exact, "programRules"), + count(exact, "waitActions"), + )) + } + fn render_body(&self) { + let exact = &self.0["exact"]; + println!("\nGenerated resources (exact)"); + println!(" emitted bytes: {}", count(exact, "emittedBytes")); + println!(" actions: {}", count(exact, "programActions")); + println!(" rules: {}", count(exact, "programRules")); + println!(" waits: {}", count(exact, "waitActions")); + let findings = self.0["findings"].as_array().map_or(&[][..], Vec::as_slice); + println!("\nStatic findings (evidence: static)"); + if findings.is_empty() { + println!(" none"); + } + for finding in findings { + println!( + " {}[{}]: {}", + finding["severity"].as_str().unwrap_or("info"), + finding["code"].as_str().unwrap_or("finding"), + finding["message"].as_str().unwrap_or_default() + ); + } + if let Some(withheld) = self.0["selection"]["withheld"] + .as_u64() + .filter(|withheld| *withheld > 0) + { + println!(" ... {withheld} finding(s) withheld (--max)"); + } + } +} + fn summary_status( envelope: &Envelope, ) -> &'static str { @@ -668,15 +843,20 @@ fn render_analyze(result: &AnalyzeResult) { .get("rules") .and_then(serde_json::Value::as_array) .map_or(&[][..], Vec::as_slice); + let objects = facts + .get("persistentObjects") + .and_then(serde_json::Value::as_array) + .map_or(&[][..], Vec::as_slice); println!("\nProgram overview"); println!( - " {} file(s), {} rule(s), {} global variable(s), {} player variable(s), {} subroutine(s)", + " {} file(s), {} rule(s), {} global variable(s), {} player variable(s), {} subroutine(s), {} persistent object(s)", count(p, "files"), count(p, "rules"), count(p, "globalVariables"), count(p, "playerVariables"), count(p, "subroutines"), + objects.len(), ); println!(" evidence: [static] parsed program inventory"); @@ -816,6 +996,14 @@ fn render_inspect(result: &InspectResult) { s["name"].as_str().unwrap_or("") ); } + // The summary stays small; each area names the query subcommand that + // serves its full detail (#429). + println!("\nDetail commands"); + println!(" wright inspect symbols [--only KIND] the full or filtered symbol list"); + println!(" wright inspect refs references and usage counts for one symbol"); + println!(" wright inspect cfg the control-flow graph of one rule"); + println!(" wright inspect callgraph the subroutine call graph"); + println!(" wright inspect cost generated-resource counts and findings"); } fn render_diagnostic(diagnostic: &wright_driver::Diagnostic, color: bool) { diff --git a/crates/wright-cli/tests/agent_contract.rs b/crates/wright-cli/tests/agent_contract.rs index f034691..02c367a 100644 --- a/crates/wright-cli/tests/agent_contract.rs +++ b/crates/wright-cli/tests/agent_contract.rs @@ -69,9 +69,11 @@ fn requests() -> Vec { json!({"op":"project"}), json!({"op":"rules"}), json!({"op":"symbols","kind":null}), - json!({"op":"references","symbol":1}), - json!({"op":"usage","symbol":1}), - json!({"op":"cfg","rule":1}), + // #429: symbol/rule addresses accept the declared name as well as the + // numeric id; `service_responses` substitutes discovered ids. + json!({"op":"references","symbol":"name"}), + json!({"op":"usage","symbol":"name"}), + json!({"op":"cfg","rule":"name"}), json!({"op":"findings"}), json!({"op":"persistentObjects"}), json!({"op":"lint"}), @@ -232,6 +234,33 @@ fn agent_v1_schema_covers_every_advertised_request_and_response() { serde_json::from_value::(request).expect("request deserializes"); } + // #429: both address spellings validate and deserialize; other types are + // rejected rather than coerced. + for request in [ + json!({"op":"references","symbol":0}), + json!({"op":"references","symbol":"counter"}), + json!({"op":"usage","symbol":1}), + json!({"op":"usage","symbol":"counter"}), + json!({"op":"cfg","rule":0}), + json!({"op":"cfg","rule":"intro"}), + ] { + assert!( + request_schema.is_valid(&request), + "invalid request: {request}" + ); + serde_json::from_value::(request).expect("request deserializes"); + } + for request in [ + json!({"op":"references","symbol":true}), + json!({"op":"usage","symbol":1.5}), + json!({"op":"cfg","rule":null}), + ] { + assert!( + !request_schema.is_valid(&request), + "schema accepted a non-id/name address: {request}" + ); + } + let capabilities_response = json!({"result":current}); assert!(response_schema.is_valid(&capabilities_response)); assert!(response_schema.is_valid(&json!({ diff --git a/crates/wright-cli/tests/cli.rs b/crates/wright-cli/tests/cli.rs index 3516ed7..d552045 100644 --- a/crates/wright-cli/tests/cli.rs +++ b/crates/wright-cli/tests/cli.rs @@ -303,6 +303,10 @@ fn analyze_over_workshop_input_reports_semantic_facts() { .unwrap() .is_empty() ); + assert!( + envelope["result"]["facts"]["persistentObjects"].is_array(), + "analyze reports persistent-object facts (#429)" + ); let _ = std::fs::remove_dir_all(path.parent().unwrap()); } @@ -941,6 +945,17 @@ fn version_and_help_are_documented_contract_surfaces() { for command in ["compile", "convert", "check", "analyze", "lint", "inspect"] { assert!(help.contains(command), "help documents {command}"); } + + // #439: semantic queries live under `inspect`, not the top level. + let output = run(&["inspect", "--help"]); + assert!(output.status.success()); + let inspect_help = String::from_utf8_lossy(&output.stdout); + for subcommand in ["symbols", "refs", "cfg", "callgraph", "cost"] { + assert!( + inspect_help.contains(subcommand), + "inspect help documents {subcommand}" + ); + } for option in [ "--kind", "--target", @@ -1406,3 +1421,264 @@ fn convert_rejects_non_workshop_input() { ); let _ = std::fs::remove_dir_all(path.parent().unwrap()); } + +// ── Semantic query commands (#429) ─────────────────────────────────────────── +// `inspect symbols`, `inspect refs`, `inspect cfg`, `inspect callgraph`, +// and `inspect cost` run the same operations the agent contract serves; +// `refs`/`cfg` address their target by name. + +#[test] +fn symbols_lists_program_symbols_and_filters_by_kind() { + let path = temp_file("cake.txt", &corpus_workshop("real-world/overpy-cake")); + let path = path.to_str().unwrap(); + + let output = run(&["inspect", "symbols", path]); + assert!(output.status.success(), "{}", command_result(&output)); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("PASS symbols"), "{stdout}"); + assert!(stdout.contains("5 symbol(s)"), "{stdout}"); + assert!(stdout.contains("globalVariable cakePos"), "{stdout}"); + assert!(stdout.contains("rule cake"), "{stdout}"); + + let output = run(&["inspect", "symbols", path, "-f", "json"]); + assert!(output.status.success(), "{}", command_result(&output)); + let envelope = parse_json(&output.stdout); + assert_eq!(envelope["command"], "symbols"); + assert_eq!(envelope["wright"]["contract"], "wright-result/v1"); + let symbols = envelope["result"].as_array().unwrap(); + assert_eq!(symbols.len(), 5); + let rule = symbols + .iter() + .find(|symbol| symbol["name"] == "cake") + .expect("the cake rule symbol"); + assert_eq!(rule["kind"], "rule"); + assert_eq!(rule["span"]["path"], "cake.txt"); + + // --only narrows to one symbol kind; --kind stays the input frontend. + let output = run(&["inspect", "symbols", path, "--only", "rule", "-f", "json"]); + let envelope = parse_json(&output.stdout); + let symbols = envelope["result"].as_array().unwrap(); + assert_eq!(symbols.len(), 2); + assert!(symbols.iter().all(|symbol| symbol["kind"] == "rule")); + + let _ = std::fs::remove_dir_all(Path::new(path).parent().unwrap()); +} + +#[test] +fn refs_reports_references_and_usage_for_a_named_symbol() { + // The issue's example: cakePos has 16 reads and 1 write across 1 rule. + let path = temp_file("cake.txt", &corpus_workshop("real-world/overpy-cake")); + let path = path.to_str().unwrap(); + + let output = run(&["inspect", "refs", "cakePos", path]); + assert!(output.status.success(), "{}", command_result(&output)); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("PASS refs"), "{stdout}"); + assert!( + stdout.contains("16 read(s), 1 write(s)"), + "the usage header: {stdout}" + ); + + let output = run(&["inspect", "refs", "cakePos", path, "-f", "json"]); + assert!(output.status.success(), "{}", command_result(&output)); + let envelope = parse_json(&output.stdout); + assert_eq!(envelope["command"], "refs"); + let result = &envelope["result"]; + assert_eq!(result["symbol"], "cakePos"); + assert_eq!(result["kind"], "globalVariable"); + assert_eq!(result["reads"], 16); + assert_eq!(result["writes"], 1); + assert_eq!(result["calls"], 0); + assert_eq!(result["rules"], 1); + let references = result["references"].as_array().unwrap(); + assert_eq!(references.len(), 18, "16 reads + 1 write + 1 declaration"); + assert_eq!( + references + .iter() + .filter(|r| r["kind"] == "declaration") + .count(), + 1 + ); + for reference in references { + assert_eq!(reference["span"]["path"], "cake.txt"); + } + let _ = std::fs::remove_dir_all(Path::new(path).parent().unwrap()); +} + +#[test] +fn cfg_reports_the_named_rules_control_flow_graph() { + let path = temp_file("cake.txt", &corpus_workshop("real-world/overpy-cake")); + let path = path.to_str().unwrap(); + + let output = run(&["inspect", "cfg", "cake", path]); + assert!(output.status.success(), "{}", command_result(&output)); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("PASS cfg"), "{stdout}"); + assert!(stdout.contains("Control-flow graph"), "{stdout}"); + + let output = run(&["inspect", "cfg", "cake", path, "-f", "json"]); + assert!(output.status.success(), "{}", command_result(&output)); + let envelope = parse_json(&output.stdout); + assert_eq!(envelope["command"], "cfg"); + let result = &envelope["result"]; + assert!(result["entry"].is_number() && result["exit"].is_number()); + assert!(!result["blocks"].as_array().unwrap().is_empty()); + let _ = std::fs::remove_dir_all(Path::new(path).parent().unwrap()); +} + +#[test] +fn callgraph_reports_subroutine_call_edges() { + let path = temp_file("decl.txt", &corpus_workshop("synthetic/declarations-rules")); + let path = path.to_str().unwrap(); + + let output = run(&["inspect", "callgraph", path, "-f", "json"]); + assert!(output.status.success(), "{}", command_result(&output)); + let envelope = parse_json(&output.stdout); + assert_eq!(envelope["command"], "callgraph"); + assert_eq!( + envelope["result"], + serde_json::json!([{ "caller": "player starts", "callee": "showStatus" }]) + ); + + let output = run(&["inspect", "callgraph", path]); + assert!(output.status.success(), "{}", command_result(&output)); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("PASS callgraph"), "{stdout}"); + assert!(stdout.contains("player starts -> showStatus"), "{stdout}"); + let _ = std::fs::remove_dir_all(Path::new(path).parent().unwrap()); +} + +#[test] +fn cost_reports_exact_counts_and_selected_findings() { + let path = temp_file("cake.txt", &corpus_workshop("real-world/overpy-cake")); + let path = path.to_str().unwrap(); + + let output = run(&["inspect", "cost", path]); + assert!(output.status.success(), "{}", command_result(&output)); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("PASS cost"), "{stdout}"); + assert!(stdout.contains("3960 emitted byte(s)"), "{stdout}"); + assert!(stdout.contains("29 action(s)"), "{stdout}"); + + let output = run(&["inspect", "cost", path, "-f", "json"]); + assert!(output.status.success(), "{}", command_result(&output)); + let envelope = parse_json(&output.stdout); + assert_eq!(envelope["command"], "cost"); + let exact = &envelope["result"]["exact"]; + assert_eq!(exact["emittedBytes"], 3960); + assert_eq!(exact["programActions"], 29); + assert_eq!(exact["programRules"], 2); + assert_eq!(exact["waitActions"], 1); + assert_eq!( + envelope["result"]["findings"].as_array().unwrap().len(), + 10, + "cost reports the same findings set as `findings`/`lint`" + ); + + // The shared finding selection applies exactly as on `costEstimate` (#430). + let output = run(&["inspect", "cost", path, "--max", "2", "-f", "json"]); + let envelope = parse_json(&output.stdout); + assert_eq!(envelope["result"]["findings"].as_array().unwrap().len(), 2); + assert_eq!(envelope["result"]["selection"]["total"], 10); + assert_eq!(envelope["result"]["selection"]["withheld"], 8); + let _ = std::fs::remove_dir_all(Path::new(path).parent().unwrap()); +} + +#[test] +fn query_commands_reject_unknown_and_ambiguous_names() { + let path = temp_file("cake.txt", &corpus_workshop("real-world/overpy-cake")); + let path = path.to_str().unwrap(); + for (command_args, code) in [ + (&["inspect", "refs", "nope"][..], "unknown-symbol"), + (&["inspect", "cfg", "nope"][..], "unknown-rule"), + ] { + let output = run(&[command_args, &[path, "-f", "json"]].concat()); + assert_eq!( + output.status.code(), + Some(1), + "{command_args:?}: {}", + command_result(&output) + ); + assert!(output.stderr.is_empty(), "JSON mode: no stderr"); + let envelope = parse_json(&output.stdout); + assert_eq!(envelope["ok"], false); + assert_eq!(envelope["diagnostics"][0]["code"], code); + assert_eq!(envelope["diagnostics"][0]["stage"], "analysis"); + assert!( + envelope["result"].is_null(), + "a failed query carries no result" + ); + } + + // A name shared across symbol kinds (or rules) is ambiguous, never + // silently resolved to one candidate. + let dup = temp_file( + "dup.ws", + r#"variables { + global: + 0: dup +} +rule ("dup") { + event { + Ongoing - Global; + } + actions { + Set Global Variable(dup, 1); + } +} +rule ("dup") { + event { + Ongoing - Global; + } + actions { + Set Global Variable(dup, 2); + } +} +"#, + ); + for (command_args, code) in [ + (&["inspect", "refs", "dup"][..], "ambiguous-symbol"), + (&["inspect", "cfg", "dup"][..], "ambiguous-rule"), + ] { + let output = run(&[command_args, &[dup.to_str().unwrap(), "-f", "json"]].concat()); + assert_eq!(output.status.code(), Some(1), "{command_args:?}"); + let envelope = parse_json(&output.stdout); + assert_eq!(envelope["diagnostics"][0]["code"], code); + assert!( + envelope["diagnostics"][0]["message"] + .as_str() + .unwrap() + .contains("dup"), + "the diagnostic lists candidates: {}", + envelope["diagnostics"][0]["message"] + ); + } + + // Text mode reports the same structured diagnostic on stderr. + let output = run(&["inspect", "refs", "nope", path]); + assert_eq!(output.status.code(), Some(1)); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("unknown-symbol"), "{stderr}"); + assert!(stderr.contains("unknown symbol 'nope'"), "{stderr}"); + + let _ = std::fs::remove_dir_all(Path::new(path).parent().unwrap()); + let _ = std::fs::remove_dir_all(dup.parent().unwrap()); +} + +#[test] +fn inspect_names_the_detail_query_commands() { + let path = temp_file("cake.txt", &corpus_workshop("real-world/overpy-cake")); + let output = run(&["inspect", path.to_str().unwrap()]); + assert!(output.status.success(), "{}", command_result(&output)); + let stdout = String::from_utf8_lossy(&output.stdout); + for pointer in [ + "wright inspect symbols", + "wright inspect refs ", + "wright inspect cfg ", + "wright inspect callgraph", + "wright inspect cost", + ] { + assert!(stdout.contains(pointer), "{pointer}: {stdout}"); + } + let _ = std::fs::remove_dir_all(path.parent().unwrap()); +} diff --git a/crates/wright-driver/src/result.rs b/crates/wright-driver/src/result.rs index db76807..7bd6950 100644 --- a/crates/wright-driver/src/result.rs +++ b/crates/wright-driver/src/result.rs @@ -91,6 +91,37 @@ pub struct InspectResult { pub references: serde_json::Value, } +/// `symbols` result (#429): the `symbols` agent operation's payload verbatim +/// — an array of `{id, kind, name, span}` entries. +#[derive(Debug, Clone, Default, Serialize)] +#[serde(transparent)] +pub struct SymbolsResult(pub serde_json::Value); + +/// `refs` result (#429): the `usage` operation's payload — the resolved +/// symbol's `id`, `kind`, `symbol` name, and read/write/call counts — plus a +/// `references` member holding the `references` operation's payload. +#[derive(Debug, Clone, Default, Serialize)] +#[serde(transparent)] +pub struct RefsResult(pub serde_json::Value); + +/// `cfg` result (#429): the `cfg` operation's payload verbatim — +/// `{entry, exit, blocks}` for the addressed rule. +#[derive(Debug, Clone, Default, Serialize)] +#[serde(transparent)] +pub struct CfgResult(pub serde_json::Value); + +/// `callgraph` result (#429): the `callGraph` operation's payload verbatim — +/// an array of `{caller, callee}` edges. +#[derive(Debug, Clone, Default, Serialize)] +#[serde(transparent)] +pub struct CallGraphResult(pub serde_json::Value); + +/// `cost` result (#429): the `costEstimate` operation's payload verbatim — +/// exact generated-resource counts plus static findings. +#[derive(Debug, Clone, Default, Serialize)] +#[serde(transparent)] +pub struct CostResult(pub serde_json::Value); + #[derive(Debug, Clone, Default, Serialize)] pub struct LintResult { pub input_identity: String, diff --git a/crates/wright-driver/src/service.rs b/crates/wright-driver/src/service.rs index 54eb504..180c77a 100644 --- a/crates/wright-driver/src/service.rs +++ b/crates/wright-driver/src/service.rs @@ -28,6 +28,41 @@ pub const SERVICE_NAME: &str = "wright-tool-service"; pub const SERVICE_VERSION: &str = env!("CARGO_PKG_VERSION"); pub const AGENT_CONTRACT: &str = "wright-agent/v1"; +/// A semantic query target on the agent contract (#429): a numeric id or a +/// declared name. +/// +/// Numeric ids keep their established meaning — the semantic index's symbol +/// id for `references`/`usage`, the rule index for `cfg` — so existing +/// numeric requests are unchanged. A name resolves against the loaded +/// program's semantic index: unmatched or ambiguous names produce a +/// structured error, never a guess. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(untagged)] +pub enum Address { + /// The numeric id assigned by the loaded program's semantic addressing. + Id(u32), + /// The declared name (`rule("name")`, variable, or subroutine name). + Name(String), +} + +impl From for Address { + fn from(id: u32) -> Self { + Self::Id(id) + } +} + +impl From for Address { + fn from(name: String) -> Self { + Self::Name(name) + } +} + +impl From<&str> for Address { + fn from(name: &str) -> Self { + Self::Name(name.to_string()) + } +} + /// A tool request: the owned query surface plus agent-oriented operations. #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(tag = "op", rename_all = "camelCase")] @@ -51,12 +86,14 @@ pub enum ToolRequest { #[serde(default)] kind: Option, }, - /// References to a symbol. - References { symbol: u32 }, - /// Usage counts for a symbol. - Usage { symbol: u32 }, - /// The control-flow graph of one rule. - Cfg { rule: u32 }, + /// References to a symbol, addressed by its numeric id or its name (#429). + References { symbol: Address }, + /// Usage counts for a symbol, addressed by its numeric id or its name + /// (#429). + Usage { symbol: Address }, + /// The control-flow graph of one rule, addressed by its numeric index or + /// its name (#429). + Cfg { rule: Address }, /// Every static-analysis finding, optionally narrowed by an inline /// selection (`severity`, `rule`, `file`, `max`; #430). Findings(crate::select::FindingSelection), @@ -264,15 +301,24 @@ impl<'a> ToolService<'a> { ToolRequest::Project => self.ok(self.project()), ToolRequest::Rules => self.semantic_query(Request::ListRules), ToolRequest::Symbols { kind } => { - self.semantic_query(Request::ListSymbols { kind: kind.clone() }) - } - ToolRequest::References { symbol } => { - self.semantic_query(Request::FindReferences { symbol: *symbol }) - } - ToolRequest::Usage { symbol } => { - self.semantic_query(Request::GetUsage { symbol: *symbol }) + self.semantic_query_with_resolved_span_paths(Request::ListSymbols { + kind: kind.clone(), + }) } - ToolRequest::Cfg { rule } => self.semantic_query(Request::GetCfg { rule: *rule }), + ToolRequest::References { symbol } => match self.symbol_address(symbol) { + Ok(symbol) => { + self.semantic_query_with_resolved_span_paths(Request::FindReferences { symbol }) + } + Err(error) => ToolResponse::Error { error }, + }, + ToolRequest::Usage { symbol } => match self.symbol_address(symbol) { + Ok(symbol) => self.semantic_query(Request::GetUsage { symbol }), + Err(error) => ToolResponse::Error { error }, + }, + ToolRequest::Cfg { rule } => match self.rule_address(rule) { + Ok(rule) => self.semantic_query(Request::GetCfg { rule }), + Err(error) => ToolResponse::Error { error }, + }, ToolRequest::Findings(selection) => self.findings(selection), ToolRequest::PersistentObjects => self.persistent_objects(), ToolRequest::Lint(selection) => self.lint(selection), @@ -445,6 +491,27 @@ impl<'a> ToolService<'a> { self.semantic_query_with_resolved_span_paths(Request::GetPersistentObjects) } + /// Resolve a symbol `Address` to its numeric id (#429). A name resolves + /// through the shared semantic index; unmatched and ambiguous names are + /// the analyzer's structured `unknown-symbol`/`ambiguous-symbol` errors. + fn symbol_address(&self, address: &Address) -> Result { + match address { + Address::Id(id) => Ok(*id), + Address::Name(name) => self + .semantic + .resolve_symbol(name) + .map(|symbol| symbol.id.index() as u32), + } + } + + /// Resolve a rule `Address` to its numeric index (#429). + fn rule_address(&self, address: &Address) -> Result { + match address { + Address::Id(id) => Ok(*id), + Address::Name(name) => self.semantic.resolve_rule(name).map(|rule| rule as u32), + } + } + fn semantic_query_with_resolved_span_paths(&self, request: Request) -> ToolResponse { match self.semantic_query(request) { ToolResponse::Ok { mut result } => { diff --git a/crates/wright-driver/src/session/semantic.rs b/crates/wright-driver/src/session/semantic.rs index d62923a..511d317 100644 --- a/crates/wright-driver/src/session/semantic.rs +++ b/crates/wright-driver/src/session/semantic.rs @@ -4,8 +4,13 @@ use super::{CompilerSession, Loaded, Provenance, ProviderOperation, root_relativ use wright_analyzer::canonical::SemanticService; use wright_analyzer::service::Request; +use crate::diag::{Diagnostic, Stage}; use crate::progress::{ProgressEvent, ProgressPhase, ProgressUnit}; -use crate::result::{AnalyzeResult, Envelope, InspectResult, LintResult}; +use crate::result::{ + AnalyzeResult, CallGraphResult, CfgResult, CostResult, Envelope, InspectResult, LintResult, + RefsResult, SymbolsResult, +}; +use crate::service::{Address, ToolErrorInfo, ToolRequest, ToolResponse, ToolService}; /// Extract the `result` payload of a semantic-service request as JSON. fn service_response(service: &SemanticService<'_>, request: &Request) -> serde_json::Value { @@ -87,6 +92,9 @@ fn semantic_facts(service: &SemanticService<'_>) -> serde_json::Value { serde_json::json!({ "symbols": symbols, "rules": rules, + // Persistent Workshop object facts join the semantic report rather + // than living behind a query-only operation (#429). + "persistentObjects": service_response(service, &Request::GetPersistentObjects), }) } @@ -99,6 +107,14 @@ fn inspect_result(service: &SemanticService<'_>) -> InspectResult { } } +/// Unwrap a [`ToolResponse`] into its result or its structured error. +fn tool_result(response: ToolResponse) -> Result { + match response { + ToolResponse::Ok { result } => Ok(result), + ToolResponse::Error { error } => Err(error), + } +} + /// Add the resolved `path` to every semantic `span` in a JSON result. /// /// File 0 is the main input and resolves root-relative to the include root @@ -196,6 +212,95 @@ impl CompilerSession { ) } + /// `symbols` (#429): the `symbols` operation's payload; `kind` narrows to + /// one symbol kind. + pub fn symbols(&mut self, kind: Option) -> Envelope { + self.service_query("symbols", move |service| { + tool_result(service.handle(&ToolRequest::Symbols { kind })).map(SymbolsResult) + }) + } + + /// `refs` (#429): the `usage` operation's payload — the header counts — + /// plus the `references` operation's payload under `references`, for one + /// symbol addressed by name. + pub fn refs(&mut self, name: &str) -> Envelope { + let symbol = Address::Name(name.to_string()); + self.service_query("refs", move |service| { + let references = tool_result(service.handle(&ToolRequest::References { + symbol: symbol.clone(), + }))?; + let usage = tool_result(service.handle(&ToolRequest::Usage { symbol }))?; + let mut result = usage.as_object().cloned().unwrap_or_default(); + result.insert("references".to_string(), references); + Ok(RefsResult(serde_json::Value::Object(result))) + }) + } + + /// `cfg` (#429): the `cfg` operation's payload for one rule, addressed by + /// name. + pub fn cfg(&mut self, rule: &str) -> Envelope { + let rule = Address::Name(rule.to_string()); + self.service_query("cfg", move |service| { + tool_result(service.handle(&ToolRequest::Cfg { rule })).map(CfgResult) + }) + } + + /// `callgraph` (#429): the `callGraph` operation's payload. + pub fn callgraph(&mut self) -> Envelope { + self.service_query("callgraph", |service| { + tool_result(service.handle(&ToolRequest::CallGraph)).map(CallGraphResult) + }) + } + + /// `cost` (#429): the `costEstimate` operation's payload; the session's + /// finding selection applies exactly as on the agent surface. + pub fn cost(&mut self) -> Envelope { + let selection = self.config.selection.clone(); + self.service_query("cost", move |service| { + tool_result(service.handle(&ToolRequest::CostEstimate(selection))).map(CostResult) + }) + } + + /// Run one query operation through the session's [`ToolService`] — the + /// same dispatch the agent surface uses, so CLI payloads equal the + /// operation payloads by construction, including shared name resolution + /// (#429). A structured service error becomes an analysis-stage + /// diagnostic; a failed envelope carries a `null` result. + fn service_query( + &mut self, + command: &str, + run: impl FnOnce(&mut ToolService<'_>) -> Result, + ) -> Envelope + where + T: Default + serde::Serialize, + { + self.with_loaded( + command, + |session| session.load(), + |session, _loaded| { + session.progress(ProgressEvent::new(ProgressPhase::SemanticAnalysis)); + let mut service = match ToolService::new(session) { + Ok(service) => service, + Err(diagnostic) => { + session.diagnostics.push(diagnostic); + return T::default(); + } + }; + match run(&mut service) { + Ok(result) => result, + Err(error) => { + session.diagnostics.push(Diagnostic::error( + error.code, + Stage::Analysis, + error.message, + )); + T::default() + } + } + }, + ) + } + pub(crate) fn inspect_loaded( &mut self, loaded: Loaded, diff --git a/crates/wright-driver/tests/service.rs b/crates/wright-driver/tests/service.rs index 9b12697..6e45d4a 100644 --- a/crates/wright-driver/tests/service.rs +++ b/crates/wright-driver/tests/service.rs @@ -276,3 +276,310 @@ fn tool_service_keeps_provider_refusals_structured() { }; assert_eq!(diagnostic.code, "source-provider-unavailable"); } + +// ── Semantic query commands and name addressing (#429) ─────────────────────── + +fn declarations_path() -> PathBuf { + workspace_root().join("tests/fixtures/workshop/synthetic/declarations-rules.ws") +} + +fn result_of(service: &mut ToolService<'_>, request: &ToolRequest) -> serde_json::Value { + match service.handle(request) { + ToolResponse::Ok { result } => result, + ToolResponse::Error { error } => panic!("{request:?} failed: {error:?}"), + } +} + +fn temp_workshop(name: &str, text: &str) -> PathBuf { + use std::sync::atomic::{AtomicUsize, Ordering}; + static COUNTER: AtomicUsize = AtomicUsize::new(0); + let dir = std::env::temp_dir().join(format!( + "wright-driver-429-{}-{}", + std::process::id(), + COUNTER.fetch_add(1, Ordering::SeqCst) + )); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join(name); + std::fs::write(&path, text).unwrap(); + path +} + +#[test] +fn name_addressing_matches_numeric_addressing_and_resolves_span_paths() { + let mut session = CompilerSession::new(SessionConfig { + input: InputSpec::Path(declarations_path()), + kind: SourceKind::Workshop, + ..SessionConfig::default() + }) + .unwrap(); + let mut service = ToolService::new(&mut session).unwrap(); + + // Discover the numeric ids through the shared semantic index rather than + // hardcoding them. + let symbols = result_of(&mut service, &ToolRequest::Symbols { kind: None }); + let symbol_id = |name: &str| { + symbols + .as_array() + .unwrap() + .iter() + .find(|symbol| symbol["name"] == name) + .and_then(|symbol| symbol["id"].as_u64()) + .unwrap_or_else(|| panic!("no symbol named {name}")) as u32 + }; + let rules = result_of(&mut service, &ToolRequest::Rules); + let rule_index = |name: &str| { + rules + .as_array() + .unwrap() + .iter() + .find(|rule| rule["name"] == name) + .and_then(|rule| rule["id"].as_u64()) + .unwrap_or_else(|| panic!("no rule named {name}")) as u32 + }; + + // Names and ids produce the same payloads. + for (by_name, by_id) in [ + ( + ToolRequest::References { + symbol: "score".into(), + }, + ToolRequest::References { + symbol: symbol_id("score").into(), + }, + ), + ( + ToolRequest::Usage { + symbol: "score".into(), + }, + ToolRequest::Usage { + symbol: symbol_id("score").into(), + }, + ), + ( + ToolRequest::Cfg { + rule: "player starts".into(), + }, + ToolRequest::Cfg { + rule: rule_index("player starts").into(), + }, + ), + ] { + assert_eq!( + result_of(&mut service, &by_name), + result_of(&mut service, &by_id) + ); + } + + // `usage` echoes the resolved identity so a name-addressed caller can + // correlate with `symbols`. + let usage = result_of( + &mut service, + &ToolRequest::Usage { + symbol: "score".into(), + }, + ); + assert_eq!(usage["id"], symbol_id("score")); + assert_eq!(usage["kind"], "globalVariable"); + assert_eq!(usage["symbol"], "score"); + + // The split numbering spaces are why names exist: "player starts" is + // symbol id 4 but rule index 1; its symbol id is an invalid `cfg` target. + assert_eq!(symbol_id("player starts"), 4); + assert_eq!(rule_index("player starts"), 1); + match service.handle(&ToolRequest::Cfg { rule: 4.into() }) { + ToolResponse::Error { error } => assert_eq!(error.code, "invalid-id"), + ToolResponse::Ok { result } => panic!("rule index 4 must not exist: {result}"), + } + + // References and symbols carry resolved source paths, not bare file ids. + let references = result_of( + &mut service, + &ToolRequest::References { + symbol: "score".into(), + }, + ); + for reference in references.as_array().unwrap() { + assert_eq!( + reference["span"]["path"].as_str().unwrap(), + "declarations-rules.ws", + "reference spans resolve to the root-relative path: {reference}" + ); + } + // Symbols whose model carries a span (rules, subroutine symbols) resolve + // the same path; declaration-less spans stay `null`. + let spanned: Vec<_> = symbols + .as_array() + .unwrap() + .iter() + .filter(|symbol| symbol["span"].is_object()) + .collect(); + assert!(!spanned.is_empty()); + for symbol in spanned { + assert_eq!(symbol["span"]["path"], "declarations-rules.ws"); + } +} + +#[test] +fn name_addressing_reports_unknown_and_ambiguous_names_as_structured_errors() { + let source = r#" +variables { + global: + 0: dup +} +rule ("dup") { + event { + Ongoing - Global; + } + actions { + Set Global Variable(dup, 1); + } +} +rule ("dup") { + event { + Ongoing - Global; + } + actions { + Set Global Variable(dup, 2); + } +} +"#; + let path = temp_workshop("dup.ws", source); + let mut session = CompilerSession::new(SessionConfig { + input: InputSpec::Path(path.clone()), + kind: SourceKind::Workshop, + ..SessionConfig::default() + }) + .unwrap(); + let mut service = ToolService::new(&mut session).unwrap(); + + for (request, code) in [ + ( + ToolRequest::References { + symbol: "nope".into(), + }, + "unknown-symbol", + ), + ( + ToolRequest::Usage { + symbol: "nope".into(), + }, + "unknown-symbol", + ), + ( + ToolRequest::Cfg { + rule: "nope".into(), + }, + "unknown-rule", + ), + ( + ToolRequest::References { + symbol: "dup".into(), + }, + "ambiguous-symbol", + ), + (ToolRequest::Cfg { rule: "dup".into() }, "ambiguous-rule"), + ] { + match service.handle(&request) { + ToolResponse::Error { error } => { + assert_eq!(error.code, code, "{request:?}"); + assert!(error.message.contains("dup") || error.message.contains("nope")); + } + ToolResponse::Ok { result } => { + panic!("{request:?} must not return a result: {result}") + } + } + } + let _ = std::fs::remove_dir_all(path.parent().unwrap()); +} + +#[test] +fn session_query_workflows_return_the_agent_operation_payloads() { + // The CLI's `symbols`/`refs`/`cfg`/`callgraph`/`cost` results are the + // same payloads the agent operations serve, by construction (#429). + let input = || SessionConfig { + input: InputSpec::Path(declarations_path()), + kind: SourceKind::Workshop, + ..SessionConfig::default() + }; + + let mut agent = CompilerSession::new(input()).unwrap(); + let mut service = ToolService::new(&mut agent).unwrap(); + let agent_symbols = result_of(&mut service, &ToolRequest::Symbols { kind: None }); + let agent_references = result_of( + &mut service, + &ToolRequest::References { + symbol: "score".into(), + }, + ); + let agent_usage = result_of( + &mut service, + &ToolRequest::Usage { + symbol: "score".into(), + }, + ); + let agent_cfg = result_of( + &mut service, + &ToolRequest::Cfg { + rule: "player starts".into(), + }, + ); + let agent_callgraph = result_of(&mut service, &ToolRequest::CallGraph); + let agent_cost = result_of( + &mut service, + &ToolRequest::CostEstimate(FindingSelection::default()), + ); + drop(service); + + let mut cli = CompilerSession::new(input()).unwrap(); + assert_eq!( + serde_json::to_value(cli.symbols(None).result).unwrap(), + agent_symbols + ); + + // `refs` is the usage payload plus the references list under `references`. + let refs = serde_json::to_value(cli.refs("score").result).unwrap(); + let mut expected_refs = agent_usage.as_object().unwrap().clone(); + expected_refs.insert("references".to_string(), agent_references); + assert_eq!(refs, serde_json::Value::Object(expected_refs)); + + assert_eq!( + serde_json::to_value(cli.cfg("player starts").result).unwrap(), + agent_cfg + ); + assert_eq!( + serde_json::to_value(cli.callgraph().result).unwrap(), + agent_callgraph + ); + assert_eq!(serde_json::to_value(cli.cost().result).unwrap(), agent_cost); +} + +#[test] +fn analyze_reports_persistent_object_facts() { + let source = r#" +rule ("effect") { + event { + Ongoing - Global; + } + actions { + Create Effect(All Players(All), Orb, Red, Vector(0, 0, 0), 1, None); + } +} +"#; + let path = temp_workshop("effect.ws", source); + let mut session = CompilerSession::new(SessionConfig { + input: InputSpec::Path(path.clone()), + kind: SourceKind::Workshop, + ..SessionConfig::default() + }) + .unwrap(); + let envelope = session.analyze(); + assert!(envelope.ok); + let objects = envelope.result.facts["persistentObjects"] + .as_array() + .expect("analyze reports facts.persistentObjects"); + assert_eq!(objects.len(), 1); + assert_eq!(objects[0]["kind"], "effect"); + assert_eq!(objects[0]["executionScope"], "global"); + assert_eq!(objects[0]["visibility"], "all-players"); + let _ = std::fs::remove_dir_all(path.parent().unwrap()); +} diff --git a/docs/agent-contract.md b/docs/agent-contract.md index 31b49c1..b085985 100644 --- a/docs/agent-contract.md +++ b/docs/agent-contract.md @@ -52,8 +52,13 @@ they returned before this contract was introduced. One-shot CLI workflows such as `wright check --format json` continue to return their `wright-result/v1` envelope. They use the same `CompilerSession` -workflows as the session service. The CLI does not add agent-specific semantic -results. The in-process embedding API can call `ToolService::handle` directly. +workflows as the session service. The semantic query commands `wright inspect +symbols`, `wright inspect refs`, `wright inspect cfg`, `wright inspect +callgraph`, and `wright inspect cost` likewise run through `ToolService` +operations (`symbols`, `references` + `usage`, `cfg`, `callGraph`, +`costEstimate`) and report the same result payloads (#429); the CLI adds no +divergent semantics. The in-process embedding API can call +`ToolService::handle` directly. The released `wright` binary includes `serve`, so each supported installation channel can use the session contract without a separate runtime. MCP is not a @@ -77,9 +82,9 @@ the successful `result` payload. | `project` | none | Loaded program origin, files, counts, and findings summary | | `rules` | none | Canonical Workshop rules | | `symbols` | optional `kind` | Symbols, optionally filtered by kind | -| `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 | +| `references` | required `symbol` (id or name) | References for the symbol | +| `usage` | required `symbol` (id or name) | Usage counts for the symbol, plus its resolved `id` and `kind` | +| `cfg` | required `rule` (index or name) | Control-flow graph for the rule | | `findings` | optional selection | Wright static-analysis findings; `{"findings": [...], "selection": {...}}` when a selection is applied | | `persistentObjects` | none | Persistent Workshop object facts | | `lint` | optional selection | Lint findings, per-rule id/effective severity, effective configuration, and `selection` when applied | @@ -92,6 +97,23 @@ the successful `result` payload. | `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 | +### Name addressing (#429) + +`references` and `usage` accept `symbol` as either a numeric symbol id or the +declared symbol name; `cfg` accepts `rule` as either the rule index or the +declared rule name. Names resolve against the loaded program's semantic index +in the driver, so a request never has to learn the program's numbering — +symbol ids and rule indexes are different spaces (a rule's symbol id is not +its rule index). Numeric ids keep their established meaning, and both +addressings return the same payload for the same target. + +An unmatched name returns a structured `unknown-symbol` or `unknown-rule` +error; a name shared by more than one symbol — or more than one rule, for +`cfg` — returns `ambiguous-symbol`/`ambiguous-rule` listing the candidate +numeric ids. Resolution never guesses or returns an empty success. `usage` +additionally echoes the resolved `id` and `kind` so a name-addressed caller +can correlate the result with `symbols`. + ### Finding selection (#430) `findings`, `lint`, and `costEstimate` accept optional selection fields: diff --git a/docs/cli.md b/docs/cli.md index 7d05daf..b9348a0 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -20,6 +20,13 @@ See [architecture, commands, and conversion](cli/commands.md). See [architecture, commands, and conversion](cli/commands.md). +## Semantic query commands (#429) + +See [architecture, commands, and conversion](cli/commands.md): `inspect` owns +the semantic query surface — `inspect symbols`, `inspect refs`, `inspect +cfg`, `inspect callgraph`, and `inspect cost` serve the same results as the +agent operations, and `refs`/`cfg` address symbols and rules by name. + ## `wright convert` and the reconstruction surface (#126) See [architecture, commands, and conversion](cli/commands.md). diff --git a/docs/cli/commands.md b/docs/cli/commands.md index c3b95ac..1adf0d4 100644 --- a/docs/cli/commands.md +++ b/docs/cli/commands.md @@ -35,7 +35,12 @@ result. | `wright check [INPUT]` | Parse, lower, validate, and report correctness diagnostics | verdict and validation diagnostics | | `wright analyze [INPUT]` | Summarize project structure, ranked CFG hotspots, and cross-cutting state | bounded semantic report with static evidence labels | | `wright lint [INPUT]` | Parse, lower, lint; report findings | findings, rule id/severity summary, and effective configuration | -| `wright inspect [INPUT]` | Parse, lower, and inspect exhaustive semantic facts | rules, symbols, references summary | +| `wright inspect [INPUT]` | Parse, lower, and inspect exhaustive semantic facts | rules, symbols, references summary, and the detail command per area | +| `wright inspect symbols [INPUT] [--only KIND]` | List semantic symbols, optionally narrowed to one kind | the symbol list with resolved locations | +| `wright inspect refs [INPUT]` | References and usage counts for one symbol, addressed by name | usage-count header plus the reference list | +| `wright inspect cfg [INPUT]` | Control-flow graph of one rule, addressed by name | block/edge listing | +| `wright inspect callgraph [INPUT]` | Subroutine call graph (caller rules → callee subroutines) | call edges | +| `wright inspect cost [INPUT]` | Exact generated-resource counts plus static findings | resource counts and findings | | `wright serve [INPUT]` | Serve `wright-agent/v1` over stdio or JSON-RPC 2.0 | one structured response per request | | `wright completion ` | Generate static completion script for bash, zsh, fish, or powershell | the generated completion script | | `wright completion install [SHELL]` | Install generated completion into standard user-local directory | installation progress and guidance | @@ -66,12 +71,46 @@ 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 +Commands that report findings (`check`, `analyze`, `lint`, `inspect cost`) +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). +## Semantic query commands (#429) + +`inspect` owns the semantic query surface: the bare command prints the +bounded summary, and its five subcommands — `inspect symbols`, +`inspect refs`, `inspect cfg`, `inspect callgraph`, `inspect cost` — expose +each detail area. They run the same operations the agent contract serves +(`symbols`, `references` + `usage`, `cfg`, `callGraph`, `costEstimate`) +through the session's `ToolService`, so a CLI `result` payload equals the +agent operation's payload for the same input. Nesting them under `inspect` +keeps the top-level command surface to distinct user intents (#439); the +agent request names stay flat operation names, not command paths. + +`inspect refs ` and `inspect cfg ` address their target by its +declared name — a `variables`/`subroutines` entry or a `rule("name")` — +resolved against the loaded program's semantic index inside the driver. +Callers never need the program's numbering, where symbol ids and rule +indexes are different spaces (the agent contract keeps accepting numeric +ids too). An unmatched name is a structured `unknown-symbol`/`unknown-rule` +diagnostic with exit 1; a name shared by several candidates is +`ambiguous-symbol`/`ambiguous-rule` listing the numeric ids to fall back to — +resolution never guesses. + +* `inspect refs` reports the `usage` counts (`reads`, `writes`, `calls`, + `rules`) as the header of the reference list; there is no separate usage + command. The reference list includes the declaration entry alongside + reads and writes, with identifier spans reported by the analyzer (#433). +* `inspect symbols --only ` narrows the list to `globalVariable`, + `playerVariable`, `subroutine`, or `rule` (kebab-case aliases work). It is + spelled `--only` because `--kind` already selects the input frontend. +* `inspect cost` accepts the finding-selection options, applied to its + findings list exactly as on `costEstimate`. +* `persistentObjects` has no standalone command; `analyze` reports the same + facts under `result.facts.persistentObjects`. + ## `wright convert` and the reconstruction surface (#126) `wright convert [INPUT] --target opy|ostw` reconstructs **validated Workshop diff --git a/docs/cli/machine-contract.md b/docs/cli/machine-contract.md index aa1cc1d..f4b8596 100644 --- a/docs/cli/machine-contract.md +++ b/docs/cli/machine-contract.md @@ -55,7 +55,9 @@ Diagnostic codes are stable per stage: `parse-error`, `unknown-*`, `settings-unknown-key`, `settings-unknown-value` (validation), `convert-error`/ `lower-error` (lowering), `validation-error` (validation), `input-*`/ `stdin-*` (discovery), `output-io` (emission), analysis findings reuse the -analyzer's codes, and `*-internal` / `*-unavailable` (internal). +analyzer's codes — including the name-addressing refusals `unknown-symbol`, +`ambiguous-symbol`, `unknown-rule`, and `ambiguous-rule` (#429) — and +`*-internal` / `*-unavailable` (internal). `source-provider-unavailable` marks the explicit DEL/OSTW provider boundary and is reported at the internal stage. A `convert` diff --git a/docs/cli/presentation.md b/docs/cli/presentation.md index 4da0cfa..4829f5d 100644 --- a/docs/cli/presentation.md +++ b/docs/cli/presentation.md @@ -76,10 +76,11 @@ 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 +`check`, `analyze`, `lint`, and `inspect cost` 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 @@ -102,7 +103,8 @@ truthful session phases such as input resolution, parsing, semantic analysis, linting, emission, or conversion. A lightweight spinner starts only after a short anti-flicker threshold; phase output is transient and is fully cleared before the final verdict, diagnostics, report, or source artifact is rendered. -Completed `check`, `lint`, `analyze`, and `inspect` commands print a +Completed `check`, `lint`, `analyze`, `inspect`, and the `inspect` query +subcommands (`symbols`, `refs`, `cfg`, `callgraph`, `cost`) print a command-specific PASS/WARN/ERROR verdict and compact summary before details; diagnostics and findings include a one-line source context when the reported provenance path is readable. The driver exposes typed progress events through diff --git a/schemas/wright-agent-v1.schema.json b/schemas/wright-agent-v1.schema.json index 72ad58c..c811745 100644 --- a/schemas/wright-agent-v1.schema.json +++ b/schemas/wright-agent-v1.schema.json @@ -377,8 +377,10 @@ "ReferencesResult": { "type": "array", "items": { "$ref": "#/$defs/Reference" } }, "UsageResult": { "type": "object", - "required": ["symbol", "reads", "writes", "calls", "rules"], + "required": ["id", "kind", "symbol", "reads", "writes", "calls", "rules"], "properties": { + "id": { "type": "integer", "minimum": 0 }, + "kind": { "type": "string" }, "symbol": { "type": "string" }, "reads": { "type": "integer", "minimum": 0 }, "writes": { "type": "integer", "minimum": 0 }, @@ -555,7 +557,7 @@ "program": { "$ref": "#/$defs/AnalyzedProgramSummary" }, "facts": { "type": "object", - "required": ["symbols", "rules"], + "required": ["symbols", "rules", "persistentObjects"], "properties": { "symbols": { "type": "array", @@ -595,7 +597,8 @@ }, "additionalProperties": true } - } + }, + "persistentObjects": { "$ref": "#/$defs/PersistentObjectsResult" } }, "additionalProperties": true } @@ -716,19 +719,19 @@ }, "ReferencesRequest": { "type": "object", - "properties": { "op": { "const": "references" }, "symbol": { "type": "integer", "minimum": 0, "maximum": 4294967295 } }, + "properties": { "op": { "const": "references" }, "symbol": { "type": ["integer", "string"], "minimum": 0, "maximum": 4294967295 } }, "required": ["op", "symbol"], "additionalProperties": false }, "UsageRequest": { "type": "object", - "properties": { "op": { "const": "usage" }, "symbol": { "type": "integer", "minimum": 0, "maximum": 4294967295 } }, + "properties": { "op": { "const": "usage" }, "symbol": { "type": ["integer", "string"], "minimum": 0, "maximum": 4294967295 } }, "required": ["op", "symbol"], "additionalProperties": false }, "CfgRequest": { "type": "object", - "properties": { "op": { "const": "cfg" }, "rule": { "type": "integer", "minimum": 0, "maximum": 4294967295 } }, + "properties": { "op": { "const": "cfg" }, "rule": { "type": ["integer", "string"], "minimum": 0, "maximum": 4294967295 } }, "required": ["op", "rule"], "additionalProperties": false },