From 110a39d7ba1720203c273b7f573f47b51728bd22 Mon Sep 17 00:00:00 2001 From: Teakowa <27560638+Teakowa@users.noreply.github.com> Date: Mon, 19 Oct 2026 23:05:23 +0800 Subject: [PATCH 1/2] feat!: remove semantic rename from CLI and agent surfaces Drop `wright rename` and the `semanticRename`/`providerSemanticRename` operations from `wright serve`, MCP, `wright agent tools`, and the `wright-agent/v1` schema, plus the raw-Workshop rename implementation only they used. Agents edit source directly and re-verify with `check`, or propose transactions through `validateEditTransaction` and `providerValidateEdit`, which keep the stale-source, atomic-write, and rollback contract unchanged. Editor rename is unchanged: LSP `textDocument/rename` still routes through the provider (`wright_driver::provider_edit::semantic_rename`), and the deprecated `rename_symbol` helper stays for `wright-embedding/v1` source compatibility. Fixes #780 --- README.md | 1 - agent-guide/wright/SKILL.md | 2 +- .../wright/references/language-notes.md | 8 +- .../wright-analyzer/src/canonical/symbols.rs | 4 +- crates/wright-cli/src/cli.rs | 29 +- crates/wright-cli/src/main.rs | 6 - crates/wright-cli/src/mcp.rs | 16 +- crates/wright-cli/src/present.rs | 42 +- crates/wright-cli/src/tooldefs.rs | 18 +- crates/wright-cli/tests/agent_contract.rs | 52 - crates/wright-cli/tests/cli.rs | 437 +------- crates/wright-cli/tests/serve.rs | 101 +- crates/wright-consumer/src/workflow.rs | 80 +- crates/wright-driver/src/edit.rs | 932 +----------------- crates/wright-driver/src/fix.rs | 2 +- crates/wright-driver/src/provider_edit.rs | 581 +---------- crates/wright-driver/src/service.rs | 126 +-- crates/wright-driver/src/session/edit.rs | 238 +---- crates/wright-driver/tests/driver.rs | 21 +- crates/wright-driver/tests/lint_fix.rs | 21 - crates/wright-driver/tests/provider_edit.rs | 885 +---------------- crates/wright-driver/tests/service.rs | 223 +---- crates/wright-language/src/service.rs | 2 +- crates/wright-transform/src/pipeline.rs | 4 +- docs/agent-contract.md | 78 +- docs/architecture/tooling.md | 2 +- docs/cli/commands.md | 42 - docs/cli/lint.md | 3 +- docs/cli/machine-contract.md | 2 +- docs/embedding.md | 55 +- docs/language-services.md | 4 +- docs/v1-matrix.md | 2 +- schemas/wright-agent-v1.schema.json | 70 +- 33 files changed, 297 insertions(+), 3792 deletions(-) diff --git a/README.md b/README.md index 59ba6b9b..28addcb4 100644 --- a/README.md +++ b/README.md @@ -54,7 +54,6 @@ wright check [INPUT] # correctness diagnostics wright lint [INPUT] # lint findings wright analyze [INPUT] # cost, hotspots, and risk indicators wright inspect [INPUT] # semantic summary -wright rename OLD NEW [INPUT] # preview a diff; --write applies it wright compile [INPUT] # write .ow; --to LOCALE selects the output locale wright serve [INPUT] # agent session over stdio or MCP ``` diff --git a/agent-guide/wright/SKILL.md b/agent-guide/wright/SKILL.md index 4e729aed..f2f0a31b 100644 --- a/agent-guide/wright/SKILL.md +++ b/agent-guide/wright/SKILL.md @@ -1,6 +1,6 @@ --- name: wright -description: Use in a project of Overwatch Workshop rules or OverPy (.opy) source when you must judge what is wrong, risky, or expensive, or how a change propagates, and text search cannot answer it. Covers Workshop server freezes, infinite loops, rule cost, symbol and rule flow, safe renames, and OPY-to-Workshop compilation, through the `wright` CLI. +description: Use in a project of Overwatch Workshop rules or OverPy (.opy) source when you must judge what is wrong, risky, or expensive, or how a change propagates, and text search cannot answer it. Covers Workshop server freezes, infinite loops, rule cost, symbol and rule flow, validated edits, and OPY-to-Workshop compilation, through the `wright` CLI. license: AGPL-3.0 --- diff --git a/agent-guide/wright/references/language-notes.md b/agent-guide/wright/references/language-notes.md index fba63b0f..e0e5177a 100644 --- a/agent-guide/wright/references/language-notes.md +++ b/agent-guide/wright/references/language-notes.md @@ -4,14 +4,14 @@ Confirm against `wright --help` and the `capabilities` session operation; suppor ## Raw Workshop (`.ws`, `.txt`) -- Fully owned by `workshop-rs`. Semantic queries, validated edit transactions, and semantic rename are supported here where the installed version declares them. +- Fully owned by `workshop-rs`. Semantic queries and validated edit transactions are supported here where the installed version declares them. - `compile --to ` re-emits the program in another Workshop client locale (e.g. `en-US`, `zh-CN`) without changing the input; the serve `compile` operation takes the same `to` field. A target locale missing mappings for the program fails explicitly, with no fallback. -- A semantic rename previews first and refuses on stale source, name collision, or unsupported kind (rules are not rename targets). Treat a refusal as final. +- There is no rename operation. Rename a variable or subroutine by editing the source directly — `inspect refs ` lists every occurrence the rename must touch — then re-run `check`. Where the version offers it, `validateEditTransaction` validates a proposed edit against the current sources and refuses a stale source; treat a refusal as final. ## OverPy (`.opy`) -- Compiled natively by Wright with the first-party OPY provider (`--opy-provider` overrides it). `check`, `lint`, `analyze`, `inspect`, `rename`, and `compile` apply; the `inspect` query subcommands read the provider's compiled artifact. A provider without compile capability declines with its own diagnostic. -- Edits are provider-owned. `rename` resolves the declared name through the provider's own symbol listing and previews validated edits by default (`--write` applies them); unknown, ambiguous, and provider-declined names refuse with structured diagnostics — treat a refusal as final. The raw edit operations refuse OPY input and name the provider operations; use those when the version offers them, otherwise edit the source directly and re-verify. +- Compiled natively by Wright with the first-party OPY provider (`--opy-provider` overrides it). `check`, `lint`, `analyze`, `inspect`, and `compile` apply; the `inspect` query subcommands read the provider's compiled artifact. A provider without compile capability declines with its own diagnostic. +- Edits are provider-owned. The raw edit operation refuses OPY input and names `providerValidateEdit`; where the version offers it, propose a transaction there and apply its validated edits, otherwise edit the source directly and re-verify with `check`. Refusals carry structured diagnostics — treat a refusal as final. Editor rename stays in the editor: LSP `textDocument/rename` routes through the provider and is not a CLI or agent operation. - `convert` runs Workshop to OPY only, and only when the provider negotiates the `reconstruct` capability; a `capability-unavailable` refusal means the installed provider does not offer it. When available, output is canonical, without comments, macros, or formatting, and unrepresentable constructs are rejected without partial output. ## DEL/OSTW diff --git a/crates/wright-analyzer/src/canonical/symbols.rs b/crates/wright-analyzer/src/canonical/symbols.rs index 3965ef8f..3f7974e6 100644 --- a/crates/wright-analyzer/src/canonical/symbols.rs +++ b/crates/wright-analyzer/src/canonical/symbols.rs @@ -131,8 +131,8 @@ pub struct UsageSummary { } /// The outcome of resolving a 1-based source position to the symbol whose -/// identifier occurrence covers it (#429, #555). Every surface — CLI/agent -/// rename targeting and editor hover/definition/references — shares this +/// identifier occurrence covers it (#429, #555). The editor surfaces — +/// hover, definition, and references — share this /// one resolution rule, so a position resolves to exactly one symbol, to /// none, or reports the ambiguity; no surface guesses by iteration order. #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/crates/wright-cli/src/cli.rs b/crates/wright-cli/src/cli.rs index 6de27f4f..37db8958 100644 --- a/crates/wright-cli/src/cli.rs +++ b/crates/wright-cli/src/cli.rs @@ -25,8 +25,8 @@ pub(crate) struct Cli { pub(crate) const LONG_ABOUT: &str = "Wright compiler and Workshop tooling CLI. Commands check correctness, summarize semantic hotspots, lint, compile, -rename Workshop symbols, resolve owner names and signatures (`lookup`), or -reconstruct source through the typed wright-driver result envelope. +resolve owner names and signatures (`lookup`), 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` writes the program to @@ -97,9 +97,6 @@ pub(crate) enum Command { subcommand_precedence_over_arg = true )] Inspect(InspectArgs), - /// Rename a Workshop variable or subroutine semantically (#434): previews - /// the validated source diff by default; `--write` applies it atomically. - Rename(RenameArgs), /// Resolve a display name, near spelling, or guess to the language /// owner's accepted spelling and signature (#529). Use it before writing /// in an unfamiliar language and when a name is rejected. @@ -277,28 +274,6 @@ pub(crate) struct CommonArgs { pub(crate) color: ColorArg, } -/// Arguments of `rename` (#434, #739): the declared symbol name and the -/// new identifier as positionals, then `[INPUT]` through the shared -/// workflow options. Raw Workshop input renames through `workshop-rs`; -/// OPY input resolves `NAME` through the provider's `lpp/symbols` and -/// renames through `lpp/rename` — the same semantic rename the agent -/// surface exposes. -#[derive(Debug, Args)] -pub(crate) struct RenameArgs { - /// The declared name of the symbol to rename. - #[arg(value_name = "NAME")] - pub(crate) name: String, - /// The new identifier. - #[arg(value_name = "NEW_NAME")] - pub(crate) to: String, - /// Apply the validated rename to the input file atomically instead of - /// previewing the diff. - #[arg(long)] - pub(crate) write: bool, - #[command(flatten)] - pub(crate) common: CommonArgs, -} - /// Arguments of `lookup` (#529): the free-text query, the language selector, /// and the optional narrowing fields of the `lookup` operation. It answers /// from language vocabulary alone, so it takes no input path. diff --git a/crates/wright-cli/src/main.rs b/crates/wright-cli/src/main.rs index 83e71fa5..3bc38f36 100644 --- a/crates/wright-cli/src/main.rs +++ b/crates/wright-cli/src/main.rs @@ -142,12 +142,6 @@ fn run_workflow(command: Command) -> ExitCode { ) }) } - Command::Rename(args) => run_configured( - config_from_common(&args.common), - present::Presentation::from_common(&args.common), - None, - move |session| session.rename(&args.name, &args.to, args.write), - ), Command::Lint(args) => { let mut config = config_from_common(&args.common); config.selection = selection_from_args(&args.select); diff --git a/crates/wright-cli/src/mcp.rs b/crates/wright-cli/src/mcp.rs index 4e20e48f..1ab89ca5 100644 --- a/crates/wright-cli/src/mcp.rs +++ b/crates/wright-cli/src/mcp.rs @@ -213,32 +213,30 @@ mod tests { "wright_lint_rules", "wright_cost_estimate", "wright_target_metadata", - "wright_semantic_rename", "wright_validate_edit_transaction", - "wright_provider_semantic_rename", "wright_provider_validate_edit", "wright_lookup", ] { assert!(names.iter().any(|n| n == name), "missing {name}"); } - assert_eq!(names.len(), 21); + assert_eq!(names.len(), 19); assert!(names.iter().all(|name| name.starts_with("wright_"))); } #[test] fn input_schemas_drop_op_and_edit_sources() { let defs = crate::tooldefs::schema_defs(); - let rename = TOOL_SPECS + let edit = TOOL_SPECS .iter() - .find(|spec| spec.op == "semanticRename") + .find(|spec| spec.op == "validateEditTransaction") .unwrap(); - let input = input_schema(&defs, rename, rename.drop_fields); + let input = input_schema(&defs, edit, edit.drop_fields); assert_eq!(input["type"], "object"); assert!(input["properties"].get("op").is_none()); assert!(input["properties"].get("sources").is_none()); - assert!(input["properties"].get("target").is_some()); - assert_eq!(input["required"], json!(["target"])); - assert!(input["$defs"].get("RenameTarget").is_some()); + assert!(input["properties"].get("transaction").is_some()); + assert_eq!(input["required"], json!(["transaction"])); + assert!(input["$defs"].get("EditTransaction").is_some()); let project = TOOL_SPECS.iter().find(|spec| spec.op == "project").unwrap(); let input = input_schema(&defs, project, &[]); assert!(input["required"].is_null()); diff --git a/crates/wright-cli/src/present.rs b/crates/wright-cli/src/present.rs index 1e6f9d7b..cd797a95 100644 --- a/crates/wright-cli/src/present.rs +++ b/crates/wright-cli/src/present.rs @@ -12,7 +12,6 @@ use std::time::Duration; use wright_driver::Severity; use wright_driver::config::OutputFormat; -use wright_driver::edit::RenameResult; use wright_driver::progress::{ProgressEvent, ProgressObserver, ProgressPhase, ProgressUnit}; use wright_driver::result::{ CallGraphResult, CfgResult, CheckResult, CompileResult, ConvertResult, CostResult, Envelope, @@ -283,7 +282,7 @@ pub(crate) fn render( ) { if pres.format == OutputFormat::Json { let mut value = serde_json::to_value(envelope).expect("serializes"); - if !matches!(envelope.command.as_str(), "compile" | "convert" | "rename") { + if !matches!(envelope.command.as_str(), "compile" | "convert") { wright_driver::essential::envelope(&mut value, pres.all); } if envelope.command == "check" { @@ -360,7 +359,7 @@ fn render_report( let mut ordered: Vec<&wright_driver::Diagnostic> = envelope.diagnostics.iter().collect(); ordered.sort_by_key(|diagnostic| severity_rank(diagnostic.severity)); let diagnostic_total = ordered.len(); - let bounded = !matches!(envelope.command.as_str(), "compile" | "convert" | "rename"); + let bounded = !matches!(envelope.command.as_str(), "compile" | "convert"); let limit = if pres.all || !bounded { usize::MAX } else { 20 }; for diagnostic in ordered.into_iter().take(limit) { let marks = pres.marks(); @@ -847,22 +846,6 @@ impl ResultPresentation for CheckResult { fn render_body(&self, _ctx: &RenderContext<'_>) {} } -impl ResultPresentation for RenameResult { - fn metadata(&self) -> Option { - let transaction = self.transaction.as_ref()?; - let edits = transaction.edits.len(); - let sources = self.preview.as_ref().map_or(0, Vec::len); - Some(if self.written.is_empty() { - format!("{edits} edit(s) across {sources} source(s); preview — pass --write to apply") - } else { - format!("{edits} edit(s) applied to {}", self.written.join(", ")) - }) - } - fn render_body(&self, _ctx: &RenderContext<'_>) { - render_rename(self); - } -} - impl ResultPresentation for LintResult { /// The lint verdict leads with finding counts by severity — the counts a /// human needs to judge priority. Under a selection the per-severity @@ -1706,27 +1689,6 @@ fn render_convert(result: &ConvertResult) { print!("{}", result.text); } -/// The `rename` human report (#434): per-source, the lines the validated -/// transaction changes, shown as `-`/`+` pairs with their line numbers — -/// rename edits only ever rewrite identifier occurrences in place. Written -/// files are listed after the diff when `--write` applied them. -fn render_rename(result: &RenameResult) { - let Some(previews) = &result.preview else { - return; - }; - for preview in previews { - println!("\n{}", preview.source); - let original = result - .originals - .get(&preview.source) - .map_or("", String::as_str); - render_diff_lines(original, &preview.new_text, " "); - } - for written in &result.written { - println!("wrote {written}"); - } -} - /// A line diff of `new_text` against `original`: the common prefix and /// suffix collapse, and the changed middle aligns on its longest common /// subsequence so lines that moved through the edit stay paired. Changed diff --git a/crates/wright-cli/src/tooldefs.rs b/crates/wright-cli/src/tooldefs.rs index 9f428121..ec9b376b 100644 --- a/crates/wright-cli/src/tooldefs.rs +++ b/crates/wright-cli/src/tooldefs.rs @@ -183,28 +183,12 @@ pub(crate) const TOOL_SPECS: &[ToolSpec] = &[ description: "Validate and preview a source-edit transaction atomically against the session's project; no filesystem writes. Sources default to the on-disk text.", result: "the transaction's validation status, diagnostics, and previews when valid", }, - ToolSpec { - op: "semanticRename", - request_def: "SemanticRenameRequest", - drop_fields: &["sources"], - mcp: true, - description: "Propose a semantic rename: returns the validated transaction or structured refusal diagnostics. The target is a symbol id or declared name, or a source/line/col position. Sources default to the on-disk text.", - result: "the validated rename transaction or a structured refusal", - }, - ToolSpec { - op: "providerSemanticRename", - request_def: "ProviderSemanticRenameRequest", - drop_fields: &["documents", "sources"], - mcp: true, - description: "Provider-owned rename for source-language projects (e.g. OverPy): the configured provider computes the edits, Wright verifies document versions and source preconditions, the provider validates the transaction, and the edited project is rechecked — returning validated edits or a structured refusal. The document set defaults to the session's loaded project (every member's disk text at version 0, keyed by `file://` URI); `position_document_uri` must be one of those URIs. `position` is 0-based line/UTF-16-character.", - result: "the provider-resolved rename transaction or a structured refusal", - }, ToolSpec { op: "providerValidateEdit", request_def: "ProviderValidateEditRequest", drop_fields: &["documents", "sources"], mcp: true, - description: "Validate a caller-proposed source-edit transaction for a provider-owned language through the same provider-backed pipeline as providerSemanticRename; no filesystem writes. The document set defaults to the session's loaded project (every member's disk text at version 0, keyed by `file://` URI), so every `edits[].source` must be one of those URIs and each `source_identity` verifies against the member's disk text — errors in any member document block the transaction.", + description: "Validate a caller-proposed source-edit transaction for a provider-owned language through the provider-backed pipeline: the provider validates each edited document and the edited project is rechecked; no filesystem writes. The document set defaults to the session's loaded project (every member's disk text at version 0, keyed by `file://` URI), so every `edits[].source` must be one of those URIs and each `source_identity` verifies against the member's disk text — errors in any member document block the transaction.", result: "the provider-validated transaction or a structured refusal", }, ToolSpec { diff --git a/crates/wright-cli/tests/agent_contract.rs b/crates/wright-cli/tests/agent_contract.rs index 4ebc23ef..5217930e 100644 --- a/crates/wright-cli/tests/agent_contract.rs +++ b/crates/wright-cli/tests/agent_contract.rs @@ -28,8 +28,6 @@ const EXPECTED_V1_OPERATIONS: &[&str] = &[ "costEstimate", "targetMetadata", "validateEditTransaction", - "semanticRename", - "providerSemanticRename", "providerValidateEdit", "lookup", ]; @@ -93,23 +91,6 @@ fn requests() -> Vec { "new_text":"" }]} }), - json!({ - "op":"semanticRename", - "sources":{"file:///project.ws":"rule(\"Example\") {}"}, - "target":{"source":"file:///project.ws","line":1,"col":1,"to":"Renamed"} - }), - json!({ - "op":"providerSemanticRename", - "language_id":"opy", - "documents":{"file:///project.opy":{ - "uri":"file:///project.opy","languageId":"opy","version":1,"text":"rule(\"Example\") {}" - }}, - "position_document_uri":"file:///project.opy", - "position":{"line":0,"character":0}, - "new_name":"Renamed", - "project_root":null, - "sources":{"file:///project.opy":"rule(\"Example\") {}"} - }), json!({ "op":"providerValidateEdit", "language_id":"opy", @@ -295,30 +276,6 @@ fn agent_v1_schema_covers_every_advertised_request_and_response() { ); } - // #434: `semanticRename` targets address a symbol by id or name, or by a - // position inside one identifier occurrence; both forms validate and - // deserialize. - for request in [ - json!({"op":"semanticRename","sources":{},"target":{"symbol":0,"to":"renamed"}}), - json!({"op":"semanticRename","sources":{},"target":{"symbol":"score","to":"renamed"}}), - json!({"op":"semanticRename","sources":{},"target":{"source":"a.ws","line":1,"col":3,"to":"renamed"}}), - ] { - assert!( - request_schema.is_valid(&request), - "invalid request: {request}" - ); - serde_json::from_value::(request).expect("request deserializes"); - } - for request in [ - json!({"op":"semanticRename","sources":{},"target":{"symbol":true,"to":"x"}}), - json!({"op":"semanticRename","sources":{},"target":{"to":"x","extra":1}}), - ] { - assert!( - !request_schema.is_valid(&request), - "schema accepted an invalid rename target: {request}" - ); - } - // #472: `sources` is optional on the raw edit operations — a request // without it, or with an explicit null, validates and deserializes; the // service then reads the files the request names from disk. @@ -331,9 +288,6 @@ fn agent_v1_schema_covers_every_advertised_request_and_response() { "kind":"edit","source":"a.ws","source_identity":"sha256:x", "range":{"start_line":1,"start_col":1,"end_line":1,"end_col":1}, "new_text":"x"}]}}), - json!({"op":"semanticRename","target":{"symbol":"score","to":"renamed"}}), - json!({"op":"semanticRename","sources":null,"target":{"source":"a.ws","line":1,"col":3,"to":"renamed"}}), - json!({"op":"semanticRename","target":{"source":"a.ws","line":1,"col":3,"to":"renamed"}}), ] { assert!( request_schema.is_valid(&request), @@ -346,12 +300,6 @@ fn agent_v1_schema_covers_every_advertised_request_and_response() { // a request without them, or with explicit nulls, validates and // deserializes; the service then derives the set from the loaded project. for request in [ - json!({"op":"providerSemanticRename","language_id":"opy", - "position_document_uri":"file:///main.opy", - "position":{"line":0,"character":0},"new_name":"Renamed"}), - json!({"op":"providerSemanticRename","language_id":"opy","documents":null,"sources":null, - "position_document_uri":"file:///main.opy", - "position":{"line":0,"character":0},"new_name":"Renamed"}), json!({"op":"providerValidateEdit","language_id":"opy","transaction":{"edits":[{ "kind":"edit","source":"file:///main.opy","source_identity":"sha256:x", "range":{"start_line":1,"start_col":1,"end_line":1,"end_col":1}, diff --git a/crates/wright-cli/tests/cli.rs b/crates/wright-cli/tests/cli.rs index 38aa7dba..10880b9d 100644 --- a/crates/wright-cli/tests/cli.rs +++ b/crates/wright-cli/tests/cli.rs @@ -4036,39 +4036,14 @@ fn inspect_names_unnamed_rules_by_index() { } #[test] -fn rename_previews_a_validated_diff_without_writing() { - // #434: `wright rename` defaults to a source diff; the file stays - // untouched and no temporary file leaks next to it. - let original = corpus_workshop("synthetic/declarations-numbers"); - let path = temp_file("rename.ws", &original); - let output = run(&[ - "rename", - "score", - "total", - path.to_str().unwrap(), - "--kind", - "workshop", - ]); - assert!( - output.status.success(), - "{}", - String::from_utf8_lossy(&output.stderr) - ); - let stdout = String::from_utf8_lossy(&output.stdout); - assert!(stdout.contains("PASS rename"), "{stdout}"); - assert!(stdout.contains("- 0: score"), "{stdout}"); - assert!(stdout.contains("+ 0: total"), "{stdout}"); - assert!( - stdout.contains("--write"), - "the preview names the apply flag" - ); - assert_eq!(std::fs::read_to_string(&path).unwrap(), original); - assert_eq!( - path.parent().unwrap().read_dir().unwrap().count(), - 1, - "no temporary sibling remains" - ); - let _ = std::fs::remove_dir_all(path.parent().unwrap()); +fn rename_is_an_ordinary_unknown_subcommand() { + // #780: semantic rename is removed from the CLI — `rename` parses as an + // unknown subcommand, the same usage error any other misspelling gives. + let output = run(&["rename", "score", "total"]); + assert_eq!(output.status.code(), Some(2)); + assert!(output.stdout.is_empty(), "usage errors write stderr only"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("unrecognized subcommand"), "{stderr}"); } #[test] @@ -4107,402 +4082,6 @@ fn inspect_overview_and_cfg_bound_large_detail() { let _ = std::fs::remove_dir_all(Path::new(path).parent().unwrap()); } -#[test] -fn rename_write_applies_the_validated_change_atomically() { - // #434: --write replaces the input; the result reparses cleanly. - let path = temp_file( - "rename.ws", - &corpus_workshop("synthetic/declarations-numbers"), - ); - let path_str = path.to_str().unwrap(); - let output = run(&[ - "rename", "score", "total", path_str, "--kind", "workshop", "--write", - ]); - assert!( - output.status.success(), - "{}", - String::from_utf8_lossy(&output.stderr) - ); - let stdout = String::from_utf8_lossy(&output.stdout); - assert!(stdout.contains(&format!("wrote {path_str}")), "{stdout}"); - let text = std::fs::read_to_string(&path).unwrap(); - assert!(text.contains("0: total"), "{text}"); - assert!(text.contains("Set Global Variable(total, 5)"), "{text}"); - assert!(!text.contains("score"), "{text}"); - let check = run(&["check", path_str, "--kind", "workshop"]); - assert!(check.status.success(), "{}", command_result(&check)); - let _ = std::fs::remove_dir_all(path.parent().unwrap()); -} - -#[test] -fn rename_reports_the_envelope_in_json_mode() { - let path = temp_file( - "rename.ws", - &corpus_workshop("synthetic/declarations-numbers"), - ); - let output = run(&[ - "rename", - "score", - "total", - path.to_str().unwrap(), - "--kind", - "workshop", - "-f", - "json", - ]); - assert!(output.status.success(), "{}", command_result(&output)); - let envelope = parse_json(&output.stdout); - assert_eq!(envelope["command"], "rename"); - assert_eq!(envelope["ok"], true); - assert_eq!(envelope["wright"]["contract"], "wright-result/v1"); - assert!( - envelope["result"]["transaction"]["edits"] - .as_array() - .unwrap() - .len() - >= 2, - "{envelope}" - ); - assert!( - envelope["result"]["preview"][0]["new_text"] - .as_str() - .unwrap() - .contains("0: total"), - "{envelope}" - ); - let _ = std::fs::remove_dir_all(path.parent().unwrap()); -} - -#[test] -fn rename_refusals_carry_structured_diagnostics_and_write_nothing() { - // #434: unknown names, collisions, and non-Workshop input refuse with the - // structured codes; --write still writes nothing. - let original = corpus_workshop("synthetic/declarations-numbers"); - let path = temp_file("rename.ws", &original); - for (name, code) in [ - ("missing", "unknown-symbol"), - ("numbers", "rename-unsupported-kind"), - ] { - let output = run(&[ - "rename", - name, - "renamed", - path.to_str().unwrap(), - "--kind", - "workshop", - "-f", - "json", - "--write", - ]); - assert_eq!(output.status.code(), Some(1), "{name}"); - let envelope = parse_json(&output.stdout); - assert_eq!(envelope["diagnostics"][0]["code"], code, "{envelope}"); - assert_eq!(std::fs::read_to_string(&path).unwrap(), original); - } - - // OPY input renames through the provider (#739): without a reachable - // provider binary the command refuses as a provider failure, still - // writing nothing. - let opy = temp_file("program.opy", "rule \"r\":\n pass\n"); - let missing_provider = opy.parent().unwrap().join("missing-opy-provider"); - let output = run(&[ - "rename", - "r", - "renamed", - opy.to_str().unwrap(), - "--kind", - "opy", - "--opy-provider", - missing_provider.to_str().unwrap(), - "-f", - "json", - ]); - assert_eq!(output.status.code(), Some(4)); - let envelope = parse_json(&output.stdout); - assert_eq!(envelope["diagnostics"][0]["code"], "provider-missing"); - assert_eq!( - std::fs::read_to_string(&opy).unwrap(), - "rule \"r\":\n pass\n" - ); - let _ = std::fs::remove_dir_all(path.parent().unwrap()); - let _ = std::fs::remove_dir_all(opy.parent().unwrap()); -} - -/// #739: `wright rename` on `.opy` input resolves `NAME` through the -/// provider's `lpp/symbols`, renames at the declaration position through -/// `lpp/rename`, and applies the same preview/`--write` contract the raw -/// Workshop path holds. The provider is a staged LPP script so the test -/// stays hermetic. -#[cfg(unix)] -#[test] -fn rename_opy_input_runs_the_provider_rename() { - use std::os::unix::fs::PermissionsExt; - - const PROVIDER: &str = r#"#!/usr/bin/env python3 -import json, os, sys - -HERE = os.path.dirname(os.path.abspath(__file__)) - -def read(name): - with open(os.path.join(HERE, name)) as handle: - return json.load(handle) - -def reply(id, result=None, error=None): - message = {"jsonrpc": "2.0", "id": id} - message.update({"error": error} if error is not None else {"result": result}) - print(json.dumps(message), flush=True) - -for line in sys.stdin: - request = json.loads(line) - id, method = request["id"], request["method"] - if method == "lpp/initialize": - reply(id, {"protocolVersion": request["params"]["protocolVersion"], - "serverInfo": {"name": "fake-opy", "version": "0"}, - "languages": [{"id": "opy", "extensions": ["opy"]}], - "capabilities": {"check": True, "compile": True, "reconstruct": False, - "symbols": True, "definition": False, "references": False, - "rename": True, "editValidation": True, - "projectLoading": True, "lookup": False}}) - elif method == "lpp/compile": - reply(id, {"diagnostics": [], "artifact": read("artifact.json")}) - elif method == "lpp/symbols": - reply(id, read("symbols.json")) - elif method == "lpp/rename": - params = request["params"] - with open(os.path.join(HERE, "rename-calls.jsonl"), "a") as calls: - calls.write(json.dumps({"uri": params["positionDocumentUri"], - "position": params["position"]}) + "\n") - reply(id, read("rename.json")) - elif method == "lpp/validateEdits": - reply(id, {"valid": True, "version": request["params"]["document"]["version"]}) - elif method == "lpp/check": - reply(id, {"documents": [{"uri": uri, "version": doc["version"], "diagnostics": []} - for uri, doc in request["params"]["documents"].items()]}) - else: - reply(id, {}) -"#; - - let dir = temp_dir(); - let script = dir.join("fake-opy-provider"); - std::fs::write(&script, PROVIDER).unwrap(); - std::fs::set_permissions(&script, std::fs::Permissions::from_mode(0o755)).unwrap(); - - let source = "def helper():\n pass\n\nrule \"setup\":\n helper()\n"; - let input = dir.join("main.opy"); - std::fs::write(&input, source).unwrap(); - let uri = format!("file://{}", input.display()); - let symbol = |line, character| { - serde_json::json!({"name": "helper", "kind": "def", - "range": {"start": {"line": line, "character": character}, - "end": {"line": line, "character": character + 6}}}) - }; - let edit = |line, character| { - serde_json::json!({"range": {"start": {"line": line, "character": character}, - "end": {"line": line, "character": character + 6}}, - "newText": "assist"}) - }; - std::fs::write( - dir.join("artifact.json"), - serde_json::json!({"format": "workshop-rs/text-v1", - "content": corpus_workshop("synthetic/basic-rule")}) - .to_string(), - ) - .unwrap(); - std::fs::write( - dir.join("symbols.json"), - serde_json::json!({"documents": [{"uri": uri, "version": 0, - "symbols": [symbol(0, 4)]}]}) - .to_string(), - ) - .unwrap(); - std::fs::write( - dir.join("rename.json"), - serde_json::json!({"edits": [{"documentUri": uri, "version": 0, - "textEdits": [edit(0, 4), edit(4, 4)]}]}) - .to_string(), - ) - .unwrap(); - - let provider = script.to_str().unwrap().to_string(); - let input = input.to_str().unwrap().to_string(); - - // The default run previews the provider's validated transaction and - // writes nothing; `lpp/rename` was called at the declaration position - // `lpp/symbols` reported for `helper`. - let output = run(&[ - "rename", - "helper", - "assist", - &input, - "--opy-provider", - &provider, - "-f", - "json", - ]); - assert!(output.status.success(), "{}", command_result(&output)); - let envelope = parse_json(&output.stdout); - assert_eq!(envelope["ok"], true); - assert_eq!(envelope["command"], "rename"); - let transaction = envelope["result"]["transaction"].as_object().unwrap(); - assert_eq!(transaction["edits"].as_array().unwrap().len(), 2); - let preview = envelope["result"]["preview"].as_array().unwrap(); - assert_eq!(preview[0]["source"], input.as_str()); - assert!( - preview[0]["new_text"] - .as_str() - .unwrap() - .contains("def assist():") - ); - let calls = std::fs::read_to_string(dir.join("rename-calls.jsonl")).unwrap(); - let call: serde_json::Value = serde_json::from_str(calls.lines().next().unwrap()).unwrap(); - assert_eq!(call["uri"], uri); - assert_eq!(call["position"]["line"], 0); - assert_eq!(call["position"]["character"], 4); - assert_eq!(std::fs::read_to_string(&input).unwrap(), source); - - // --write applies the same transaction atomically. - let output = run(&[ - "rename", - "helper", - "assist", - &input, - "--opy-provider", - &provider, - "-f", - "json", - "--write", - ]); - assert!(output.status.success(), "{}", command_result(&output)); - let envelope = parse_json(&output.stdout); - assert_eq!(envelope["ok"], true); - assert_eq!( - envelope["result"]["written"], - serde_json::json!([input.as_str()]) - ); - assert_eq!( - std::fs::read_to_string(&input).unwrap(), - "def assist():\n pass\n\nrule \"setup\":\n assist()\n" - ); - let _ = std::fs::remove_dir_all(&dir); -} - -/// #739: a name the project does not declare is `unknown-symbol`, and a -/// provider refusal keeps its `refusalCode` — both write nothing. -#[cfg(unix)] -#[test] -fn rename_opy_input_reports_unknown_names_and_provider_refusals() { - use std::os::unix::fs::PermissionsExt; - - const PROVIDER: &str = r#"#!/usr/bin/env python3 -import json, os, sys - -HERE = os.path.dirname(os.path.abspath(__file__)) - -def read(name): - with open(os.path.join(HERE, name)) as handle: - return json.load(handle) - -def reply(id, result=None, error=None): - message = {"jsonrpc": "2.0", "id": id} - message.update({"error": error} if error is not None else {"result": result}) - print(json.dumps(message), flush=True) - -def refusal(id, code, text): - reply(id, error={"code": -32000, "message": text, - "data": {"lpp": {"kind": "refusal", "details": {"refusalCode": code}}}}) - -for line in sys.stdin: - request = json.loads(line) - id, method = request["id"], request["method"] - if method == "lpp/initialize": - reply(id, {"protocolVersion": request["params"]["protocolVersion"], - "serverInfo": {"name": "fake-opy", "version": "0"}, - "languages": [{"id": "opy", "extensions": ["opy"]}], - "capabilities": {"check": True, "compile": True, "reconstruct": False, - "symbols": True, "definition": False, "references": False, - "rename": True, "editValidation": True, - "projectLoading": True, "lookup": False}}) - elif method == "lpp/compile": - reply(id, {"diagnostics": [], "artifact": read("artifact.json")}) - elif method == "lpp/symbols": - reply(id, read("symbols.json")) - elif method == "lpp/rename": - refusal(id, "rename.invalidName", "the name is not a valid identifier") - elif method == "lpp/validateEdits": - reply(id, {"valid": True, "version": request["params"]["document"]["version"]}) - elif method == "lpp/check": - reply(id, {"documents": [{"uri": uri, "version": doc["version"], "diagnostics": []} - for uri, doc in request["params"]["documents"].items()]}) - else: - reply(id, {}) -"#; - - let dir = temp_dir(); - let script = dir.join("fake-opy-provider"); - std::fs::write(&script, PROVIDER).unwrap(); - std::fs::set_permissions(&script, std::fs::Permissions::from_mode(0o755)).unwrap(); - - let source = "def helper():\n pass\n"; - let input = dir.join("main.opy"); - std::fs::write(&input, source).unwrap(); - let uri = format!("file://{}", input.display()); - std::fs::write( - dir.join("artifact.json"), - serde_json::json!({"format": "workshop-rs/text-v1", - "content": corpus_workshop("synthetic/basic-rule")}) - .to_string(), - ) - .unwrap(); - std::fs::write( - dir.join("symbols.json"), - serde_json::json!({"documents": [{"uri": uri, "version": 0, - "symbols": [{"name": "helper", "kind": "def", - "range": {"start": {"line": 0, "character": 4}, - "end": {"line": 0, "character": 10}}}]}]}) - .to_string(), - ) - .unwrap(); - - let provider = script.to_str().unwrap().to_string(); - let input = input.to_str().unwrap().to_string(); - - // A name `lpp/symbols` never declared is `unknown-symbol` — exit 1. - let output = run(&[ - "rename", - "missing", - "assist", - &input, - "--opy-provider", - &provider, - "-f", - "json", - ]); - assert_eq!(output.status.code(), Some(1), "{}", command_result(&output)); - let envelope = parse_json(&output.stdout); - assert_eq!(envelope["diagnostics"][0]["code"], "unknown-symbol"); - - // A rename the provider declines keeps the provider's refusal code. - let output = run(&[ - "rename", - "helper", - "not a name", - &input, - "--opy-provider", - &provider, - "-f", - "json", - "--write", - ]); - assert_eq!(output.status.code(), Some(1), "{}", command_result(&output)); - let envelope = parse_json(&output.stdout); - assert_eq!( - envelope["diagnostics"][0]["code"], - "provider-refusal-rename.invalidName" - ); - assert_eq!(std::fs::read_to_string(&input).unwrap(), source); - let _ = std::fs::remove_dir_all(&dir); -} - #[test] fn agent_install_mcp_configures_a_project_whose_server_answers_mcp() { use std::io::{BufRead, BufReader}; diff --git a/crates/wright-cli/tests/serve.rs b/crates/wright-cli/tests/serve.rs index 7d53f3b7..81a09822 100644 --- a/crates/wright-cli/tests/serve.rs +++ b/crates/wright-cli/tests/serve.rs @@ -685,9 +685,9 @@ fn stdio_transport_recovers_in_the_same_process() { #[test] fn stdio_transport_serves_mutation_operations() { // #130/#434: the stdio adapter exposes the shared mutation operations as - // thin mappings — validated edit preview and semantic rename — with the - // same structured all-or-nothing results as in-process consumers. On raw - // Workshop input both validate through `workshop-rs` reparse. + // thin mappings — a validated edit preview — with the same structured + // all-or-nothing results as in-process consumers. On raw Workshop input + // it validates through `workshop-rs` reparse. let input = corpus_workshop("synthetic/control-flow"); let source = std::fs::read_to_string(&input).unwrap(); let identity = wright_driver::input_identity(&source); @@ -721,21 +721,6 @@ fn stdio_transport_serves_mutation_operations() { .contains("\"bounded loop\""), "the preview carries the edited source: {responses:?}" ); - - // Semantic rename through the same transport (#434): the declared name - // addresses the symbol; every occurrence rewrites through provenance. - let rename = serde_json::json!({ - "op": "semanticRename", - "sources": { input.to_string_lossy().into_owned(): source.clone() }, - "target": { "symbol": "index", "to": "counter" } - }); - let responses = run_lines("stdio", &input, &[&serde_json::to_string(&rename).unwrap()]); - assert_eq!(responses[0]["result"]["ok"], true, "{responses:?}"); - let preview = responses[0]["result"]["preview"][0]["new_text"] - .as_str() - .unwrap(); - assert!(preview.contains("0: counter"), "{preview}"); - assert!(preview.contains("Global.counter"), "{preview}"); } #[test] @@ -887,14 +872,29 @@ fn transports_are_equivalent_for_mutation_operations() { // in-process behavior. let input = corpus_workshop("synthetic/control-flow"); let source = std::fs::read_to_string(&input).unwrap(); - let rename = serde_json::json!({ - "op": "semanticRename", - "sources": { input.to_string_lossy().into_owned(): source }, - "target": { "symbol": "index", "to": "counter" } + let validate = serde_json::json!({ + "op": "validateEditTransaction", + "sources": { input.to_string_lossy().into_owned(): source.clone() }, + "transaction": { + "edits": [{ + "kind": "edit", + "source": input.to_string_lossy().into_owned(), + "source_identity": wright_driver::input_identity(&source), + "range": { + "start_line": 6, "start_col": 8, + "end_line": 6, "end_col": 21, + }, + "new_text": "bounded loop" + }] + } }); - let stdio = run_lines("stdio", &input, &[&serde_json::to_string(&rename).unwrap()]); + let stdio = run_lines( + "stdio", + &input, + &[&serde_json::to_string(&validate).unwrap()], + ); let jsonrpc_request = serde_json::json!({ - "jsonrpc": "2.0", "id": 1, "method": "request", "params": rename + "jsonrpc": "2.0", "id": 1, "method": "request", "params": validate }); let jsonrpc = run_lines( "jsonrpc", @@ -967,8 +967,6 @@ fn mcp_transport_lists_the_initial_tool_set_within_capabilities() { "wright_analyze", "wright_inspect", "wright_validate_edit_transaction", - "wright_semantic_rename", - "wright_provider_semantic_rename", "wright_provider_validate_edit", "wright_lookup", ] @@ -988,9 +986,7 @@ fn mcp_transport_lists_the_initial_tool_set_within_capabilities() { "call_graph" => "callGraph", "cost_estimate" => "costEstimate", "target_metadata" => "targetMetadata", - "semantic_rename" => "semanticRename", "validate_edit_transaction" => "validateEditTransaction", - "provider_semantic_rename" => "providerSemanticRename", "provider_validate_edit" => "providerValidateEdit", other => other, }; @@ -1004,26 +1000,31 @@ fn mcp_transport_lists_the_initial_tool_set_within_capabilities() { assert!(tool["inputSchema"]["properties"].get("op").is_none()); assert!(tool["description"].is_string()); } - for name in ["wright_validate_edit_transaction", "wright_semantic_rename"] { - let tool = tools.iter().find(|t| t["name"] == name).unwrap(); - assert!( - tool["inputSchema"]["properties"].get("sources").is_none(), - "{name} omits sources" - ); - } - // #548: the provider tools omit `documents`/`sources` — the service + let validate_tool = tools + .iter() + .find(|t| t["name"] == "wright_validate_edit_transaction") + .unwrap(); + assert!( + validate_tool["inputSchema"]["properties"] + .get("sources") + .is_none(), + "wright_validate_edit_transaction omits sources" + ); + // #548: the provider tool omits `documents`/`sources` — the service // derives them from the session's loaded project. - for name in [ - "wright_provider_semantic_rename", - "wright_provider_validate_edit", - ] { - let tool = tools.iter().find(|t| t["name"] == name).unwrap(); - assert!( - tool["inputSchema"]["properties"].get("documents").is_none() - && tool["inputSchema"]["properties"].get("sources").is_none(), - "{name} omits documents and sources" - ); - } + let provider_tool = tools + .iter() + .find(|t| t["name"] == "wright_provider_validate_edit") + .unwrap(); + assert!( + provider_tool["inputSchema"]["properties"] + .get("documents") + .is_none() + && provider_tool["inputSchema"]["properties"] + .get("sources") + .is_none(), + "wright_provider_validate_edit omits documents and sources" + ); // #532: the report tools expose `all` in their derived input schemas. for name in ["wright_analyze", "wright_inspect", "wright_lint"] { let tool = tools.iter().find(|t| t["name"] == name).unwrap(); @@ -1064,10 +1065,6 @@ fn mcp_transport_results_match_the_service_contract() { ("wright_cost_estimate", serde_json::json!({})), // #472: the edit tools run without `sources` — the service reads the // on-disk text. - ( - "wright_semantic_rename", - serde_json::json!({"target": {"symbol": "index", "to": "counter"}}), - ), ( "wright_validate_edit_transaction", serde_json::json!({ @@ -1099,9 +1096,7 @@ fn mcp_transport_results_match_the_service_contract() { let op = match tool.trim_start_matches("wright_") { "call_graph" => "callGraph", "cost_estimate" => "costEstimate", - "semantic_rename" => "semanticRename", "validate_edit_transaction" => "validateEditTransaction", - "provider_semantic_rename" => "providerSemanticRename", "provider_validate_edit" => "providerValidateEdit", other => other, }; diff --git a/crates/wright-consumer/src/workflow.rs b/crates/wright-consumer/src/workflow.rs index 25162cba..543f6cd8 100644 --- a/crates/wright-consumer/src/workflow.rs +++ b/crates/wright-consumer/src/workflow.rs @@ -103,60 +103,38 @@ pub fn run_consumer(input: &str) -> Result<(), String> { } } - if input.ends_with(".ws") { - let renamed = match service.handle(&ToolRequest::Symbols { - kind: Some("globalVariable".to_string()), - file: None, - max: None, - all: true, + if input.ends_with(".ws") && !source.is_empty() { + // `validateEditTransaction` exercises the shared validated-edit path + // with a caller-proposed no-op replacement of the source's first + // character. + let transaction = wright_driver::edit::EditTransaction { + edits: vec![wright_driver::edit::SourceEdit { + edit_kind: "edit".to_string(), + source: input.to_string(), + source_identity: wright_driver::input_identity(&source), + range: wright_driver::edit::EditRange { + start_line: 1, + start_col: 1, + end_line: 1, + end_col: 2, + }, + new_text: source.chars().next().unwrap().to_string(), + }], + }; + match service.handle(&ToolRequest::ValidateEdit { + sources: Some(std::collections::BTreeMap::from([( + input.to_string(), + source.clone(), + )])), + transaction, }) { - wright_driver::service::ToolResponse::Ok { result } => result - .as_array() - .and_then(|symbols| symbols.first()) - .and_then(|symbol| symbol.get("name")) - .and_then(serde_json::Value::as_str) - .map(str::to_string), - wright_driver::service::ToolResponse::Error { error } => { - panic!("symbols failed: {error:?}") + wright_driver::service::ToolResponse::Ok { result } => { + assert!(result["ok"].as_bool().unwrap_or(false), "edit validates"); + println!("edit: transaction validated and previewed"); } - }; - match renamed { - Some(name) => { - let rename = service.handle(&ToolRequest::SemanticRename { - sources: Some(std::collections::BTreeMap::from([( - input.to_string(), - source.clone(), - )])), - target: wright_driver::edit::RenameTarget { - symbol: Some(wright_driver::service::Address::Name(name)), - source: None, - line: None, - col: None, - to: "renamed_by_consumer".to_string(), - }, - }); - match rename { - wright_driver::service::ToolResponse::Ok { result } => { - assert!(result["ok"].as_bool().unwrap_or(false), "rename validates"); - assert!( - result["preview"] - .as_array() - .unwrap() - .iter() - .any(|p| p["new_text"] - .as_str() - .unwrap_or_default() - .contains("renamed_by_consumer")), - "the preview rewrites the identifier" - ); - println!("edit: semantic rename validated and previewed"); - } - wright_driver::service::ToolResponse::Error { error } => { - panic!("rename failed: {error:?}") - } - } + wright_driver::service::ToolResponse::Error { error } => { + panic!("validateEditTransaction failed: {error:?}") } - None => println!("edit: no global variable to rename (skipped)"), } } diff --git a/crates/wright-driver/src/edit.rs b/crates/wright-driver/src/edit.rs index cac2a6b0..8e690686 100644 --- a/crates/wright-driver/src/edit.rs +++ b/crates/wright-driver/src/edit.rs @@ -2,29 +2,22 @@ //! [`SourceEdit`]s — never as mutations of Wright's internal IR. //! //! Raw Workshop input is edited through `workshop-rs` itself (#434): a -//! transaction applies to the caller-supplied current sources, the edited -//! result is reparsed and validated through the session's own Workshop -//! parse path, and a semantic rename rewrites exactly the identifier spans -//! the parsed program reports — declarations, references, and positions -//! carry the same provenance. Source languages stay at the provider -//! boundary (`providerValidateEdit` / `providerSemanticRename`); there is -//! no textual-search rename and no static fallback for them. +//! transaction applies to the caller-supplied current sources and the +//! edited result is reparsed and validated through the session's own +//! Workshop parse path. Source languages stay at the provider boundary +//! (`providerValidateEdit`); there is no static fallback for them. use std::collections::BTreeMap; use std::path::{Path, PathBuf}; use serde::{Deserialize, Serialize}; use workshop_rs::catalog::{Catalog, Locale}; -use wright_analyzer::canonical::{ - PositionResolution, ReferenceKind, SemanticIndex, Symbol, SymbolId, SymbolKind, -}; use crate::config::{SessionConfig, SourceKind}; use crate::diag::{Diagnostic, Stage, source_provider_unavailable}; use crate::input::{self, ResolvedInput}; use crate::result::exit_code_from; -use crate::service::Address; -use crate::session::{Loaded, workshop_diag}; +use crate::session::workshop_diag; /// One proposed source edit. #[derive(Debug, Clone, Serialize, Deserialize)] @@ -301,9 +294,9 @@ fn edit_stdin_refusal() -> Diagnostic { /// The source-kind boundary shared by the raw edit operations (#434): raw /// Workshop input validates through `workshop-rs`; OPY and other source -/// languages belong to the provider operations (`providerValidateEdit` / -/// `providerSemanticRename`), which keep their own contract. The refusal -/// names the operation a caller should route to. +/// languages belong to the provider operation (`providerValidateEdit`), +/// which keeps its own contract. The refusal names the operation a caller +/// should route to. pub(crate) fn edit_kind_gate(kind: SourceKind, provider_operation: &str) -> Option { match kind { SourceKind::Workshop => None, @@ -326,404 +319,6 @@ pub(crate) fn edit_kind_gate(kind: SourceKind, provider_operation: &str) -> Opti } } -/// The `semanticRename` target (#434): either a symbol address — the numeric -/// id or the declared name, exactly as `references`/`usage` address symbols -/// (#429) — or a `source`/`line`/`col` position inside one identifier -/// occurrence. -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct RenameTarget { - /// The symbol to rename, by numeric id or declared name. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub symbol: Option
, - /// Position addressing: the source file the position is interpreted in. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub source: Option, - /// Position addressing: the 1-based line. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub line: Option, - /// Position addressing: the 1-based column. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub col: Option, - /// The new identifier. - pub to: String, -} - -/// The `semanticRename` result: the validated transaction and its previews, -/// or refusal diagnostics with no transaction and no partial preview. -#[derive(Debug, Clone, Serialize)] -pub struct SemanticRename { - pub ok: bool, - pub transaction: Option, - pub diagnostics: Vec, - #[serde(skip_serializing_if = "Option::is_none")] - pub preview: Option>, -} - -/// The `wright rename` result (#434): the validated transaction and the -/// per-source previews; `written` lists the files `--write` updated. -#[derive(Debug, Clone, Default, Serialize)] -pub struct RenameResult { - /// The validated rename transaction (absent on refusal). - #[serde(skip_serializing_if = "Option::is_none")] - pub transaction: Option, - /// Per-source previews of the validated transaction. - #[serde(skip_serializing_if = "Option::is_none")] - pub preview: Option>, - /// The files updated by `--write` (empty in preview mode). - #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub written: Vec, - /// The source text the transaction was validated against — presentation - /// input for the human diff, not part of the JSON contract. - #[serde(skip)] - pub originals: BTreeMap, -} - -/// `semanticRename` on raw Workshop input (#434): the target resolves -/// against the loaded program's semantic index — the same symbol and -/// position addressing `references`/`usage` use — and every occurrence is -/// rewritten through the program's own identifier provenance, never a -/// textual search. When `sources` is absent (#472) the loaded input's -/// current text is read from disk; a supplied `sources` stays the -/// caller-side precondition text. The proposed transaction is validated by -/// reparsing the edited source; every refusal carries structured -/// diagnostics and no partial edit set. -pub fn semantic_rename( - loaded: &Loaded, - catalog: &Catalog, - sources: Option<&BTreeMap>, - target: &RenameTarget, -) -> SemanticRename { - let refuse = |diagnostics: Vec| SemanticRename { - ok: false, - transaction: None, - diagnostics, - preview: None, - }; - if target.to.is_empty() { - return refuse(vec![Diagnostic::error( - "rename-invalid-name", - Stage::Discovery, - "rename requires a non-empty new name", - )]); - } - if let Some(diagnostic) = edit_kind_gate(loaded.input.kind, "providerSemanticRename") { - return refuse(vec![diagnostic]); - } - let Some(input_path) = loaded.input.path.clone() else { - return refuse(vec![edit_stdin_refusal()]); - }; - // #472: an absent `sources` means the current text of the file the - // request names comes from disk — the loaded input, keyed by the - // `target.source` spelling when position addressing names it, else by - // the input's display path, so the produced transaction names the - // source as the caller would have. - let defaulted_sources; - let sources = match sources { - Some(sources) => sources, - None => { - let bytes = match std::fs::read(&input_path) { - Ok(bytes) => bytes, - Err(error) => { - return refuse(vec![Diagnostic::error( - "input-io", - Stage::Discovery, - format!("cannot read '{}': {error}", input_path.display()), - )]); - } - }; - let source = target - .source - .clone() - .filter(|source| same_file(source, &input_path)) - .unwrap_or_else(|| loaded.input.display.clone()); - defaulted_sources = - BTreeMap::from([(source, crate::input::decode_source_text(&bytes))]); - &defaulted_sources - } - }; - let Some((source, current)) = sources - .iter() - .find(|(source, _)| same_file(source, &input_path)) - .map(|(source, text)| (source.clone(), text.clone())) - else { - return refuse(vec![Diagnostic::error( - "edit-unknown-source", - Stage::Discovery, - format!( - "semantic rename needs the current text of '{}' in `sources`", - loaded.input.display - ), - )]); - }; - if current != loaded.input.text { - return refuse(vec![Diagnostic::error( - "edit-stale-source", - Stage::Discovery, - format!( - "the supplied text of '{source}' differs from the loaded program; reload the project and retry" - ), - )]); - } - - let index = SemanticIndex::build(&loaded.program); - let symbol = match resolve_rename_target(&index, loaded, &input_path, target) { - Ok(symbol) => symbol, - Err(diagnostic) => return refuse(vec![diagnostic]), - }; - match symbol.kind { - SymbolKind::GlobalVariable | SymbolKind::PlayerVariable | SymbolKind::Subroutine => {} - other => { - return refuse(vec![Diagnostic::error( - "rename-unsupported-kind", - Stage::Discovery, - format!( - "semantic rename covers variables and subroutines, not '{}' symbols", - other.as_str() - ), - )]); - } - } - if target.to == symbol.name { - return refuse(vec![Diagnostic::error( - "rename-invalid-name", - Stage::Discovery, - "the new name already names this symbol", - )]); - } - // Same-namespace collision: `to` must not already name another symbol of - // the same kind — global and player variables are separate namespaces, - // so the check is kind-scoped. - if let Some(other) = index - .symbols() - .find(|other| other.id != symbol.id && other.kind == symbol.kind && other.name == target.to) - { - return refuse(vec![Diagnostic::error( - "rename-name-collision", - Stage::Discovery, - format!( - "'{}' already names {} #{}; renaming '{}' would collide", - target.to, - other.kind.as_str(), - other.id.index(), - symbol.name - ), - )]); - } - - let mut spans: Vec = Vec::new(); - for reference in index.references(symbol.id) { - let Some(occurrence) = reference.occurrence else { - return refuse(vec![rename_unmapped(loaded, symbol)]); - }; - let Ok(edit) = loaded.program.edit_source(occurrence, &target.to) else { - return refuse(vec![rename_unmapped(loaded, symbol)]); - }; - if current.get(edit.range()) != Some(symbol.name.as_str()) { - return refuse(vec![rename_unmapped(loaded, symbol)]); - } - if !spans.contains(&occurrence) { - spans.push(occurrence); - } - } - if spans.is_empty() { - return refuse(vec![Diagnostic::error( - "rename-invalid-target", - Stage::Discovery, - format!("symbol '{}' carries no identifier occurrences", symbol.name), - )]); - } - spans.sort_by_key(|span| { - ( - span.file.index(), - span.start.line, - span.start.col, - span.end.line, - span.end.col, - ) - }); - let identity = crate::input_identity(¤t); - let transaction = match EditTransaction::new( - spans - .iter() - .map(|span| SourceEdit { - edit_kind: "rename".to_string(), - source: source.clone(), - source_identity: identity.clone(), - range: EditRange { - start_line: span.start.line, - start_col: span.start.col, - end_line: span.end.line, - end_col: span.end.col, - }, - new_text: target.to.clone(), - }) - .collect(), - ) { - Ok(transaction) => transaction, - Err(diagnostic) => return refuse(vec![diagnostic]), - }; - let previews = match transaction.apply(sources) { - Ok(previews) => previews, - Err(diagnostic) => return refuse(vec![diagnostic]), - }; - let edited = previews - .iter() - .find(|preview| same_file(&preview.source, &input_path)) - .expect("rename edits only ever touch the loaded input") - .new_text - .clone(); - match verify_rename(&index, symbol, &target.to, &edited, loaded, catalog) { - Ok(()) => SemanticRename { - ok: true, - transaction: Some(transaction), - diagnostics: Vec::new(), - preview: Some(previews), - }, - Err(diagnostics) => refuse(diagnostics), - } -} - -/// Resolve the rename target: a numeric or name `symbol` address (#429), or -/// a `source`/`line`/`col` position inside one identifier occurrence. -fn resolve_rename_target<'a>( - index: &'a SemanticIndex, - loaded: &Loaded, - input_path: &Path, - target: &RenameTarget, -) -> Result<&'a Symbol, Diagnostic> { - match &target.symbol { - Some(Address::Id(id)) => { - index - .symbol(SymbolId::from_index(*id as usize)) - .ok_or_else(|| { - Diagnostic::error( - "invalid-id", - Stage::Discovery, - format!("unknown symbol {id}"), - ) - }) - } - Some(Address::Name(name)) => index - .resolve_symbol(name) - .map_err(|error| Diagnostic::error(error.code, Stage::Discovery, error.message)), - None => symbol_at_position(index, loaded, input_path, target), - } -} - -fn symbol_at_position<'a>( - index: &'a SemanticIndex, - loaded: &Loaded, - input_path: &Path, - target: &RenameTarget, -) -> Result<&'a Symbol, Diagnostic> { - let (Some(source), Some(line), Some(col)) = (target.source.as_deref(), target.line, target.col) - else { - return Err(Diagnostic::error( - "rename-invalid-target", - Stage::Discovery, - "semanticRename needs a `symbol` (numeric id or declared name) or a `source`/`line`/`col` position", - )); - }; - if !same_file(source, input_path) { - return Err(Diagnostic::error( - "rename-invalid-target", - Stage::Discovery, - format!( - "the position source '{source}' does not name the loaded input '{}'", - loaded.input.display - ), - )); - } - match index.symbol_at(line, col) { - PositionResolution::Resolved { symbol, .. } => Ok(symbol), - PositionResolution::Miss => Err(Diagnostic::error( - "rename-invalid-target", - Stage::Discovery, - format!("no symbol identifier covers {source}:{line}:{col}"), - )), - PositionResolution::Ambiguous(symbols) => Err(Diagnostic::error( - "ambiguous-symbol", - Stage::Discovery, - format!( - "more than one symbol covers {source}:{line}:{col}: {}", - symbols - .iter() - .map(|symbol| format!("{} {}", symbol.kind.as_str(), symbol.id.index())) - .collect::>() - .join(", ") - ), - )), - } -} - -fn rename_unmapped(loaded: &Loaded, symbol: &Symbol) -> Diagnostic { - Diagnostic::error( - "rename-unmapped-span", - Stage::Discovery, - format!( - "a '{}' occurrence of {} '{}' does not map to an exact identifier span in '{}'", - symbol.name, - symbol.kind.as_str(), - symbol.id.index(), - loaded.input.display - ), - ) -} - -/// Reparse `edited` through the loaded program's parse context and verify -/// the rename invariant: the text stays a valid Workshop program and the -/// `(kind, to)` symbol binds the same reference-kind multiset that `(kind, -/// from)` bound before. A name that does not survive reparsing — or a -/// reference set that changed — refuses with no transaction. -fn verify_rename( - index: &SemanticIndex, - symbol: &Symbol, - to: &str, - edited: &str, - loaded: &Loaded, - catalog: &Catalog, -) -> Result<(), Vec> { - let locale = Locale::new( - loaded - .origin - .locale - .as_deref() - .expect("Workshop loads record their resolved locale"), - ); - let program = workshop_rs::parser::parse_with_context(edited, catalog, &locale, catalog) - .map_err(|error| vec![workshop_diag(error, &loaded.input)])?; - program - .validate() - .map_err(|error| vec![workshop_diag(error, &loaded.input)])?; - let reparsed = SemanticIndex::build(&program); - let expected = reference_signature(index, symbol.id); - let actual = reparsed - .symbols() - .find(|candidate| candidate.kind == symbol.kind && candidate.name == to) - .map(|candidate| reference_signature(&reparsed, candidate.id)); - match actual { - Some(actual) if actual == expected => Ok(()), - _ => Err(vec![Diagnostic::error( - "rename-mismatch", - Stage::Validation, - format!( - "the edited source no longer binds the same references to '{to}' as '{}' did; refusing rather than reporting a partial rename", - symbol.name - ), - )]), - } -} - -fn reference_signature(index: &SemanticIndex, id: SymbolId) -> Vec { - let mut kinds: Vec = index - .references(id) - .iter() - .map(|reference| reference.kind) - .collect(); - kinds.sort_by_key(|kind| *kind as usize); - kinds -} - /// Whether a `sources` key names the resolved input file: plain path /// spellings compare by canonical identity, and a `file://` URI spelling /// of the same path — including a percent-encoded one — binds too. @@ -1007,7 +602,7 @@ fn char_col(line: &str, col: u32) -> usize { /// Deprecated `wright-embedding/v1` compatibility surface (#514): the /// request consumed by [`rename_symbol`]. Kept source-compatible for v1 /// callers; removal requires a breaking embedding-contract transition. -#[deprecated = "v1 compatibility helper for `rename_symbol`; the supported rename is the semantic `semanticRename` path"] +#[deprecated = "v1 compatibility helper for `rename_symbol`; no supported rename path supersedes it"] #[derive(Debug, Clone, Serialize, Deserialize)] pub struct RenameRequest { pub symbol_kind: String, @@ -1022,12 +617,9 @@ pub struct RenameRequest { /// a name in `source` (#129). The replacement is textual only — it cannot /// distinguish semantic identity from strings, comments, or unrelated /// same-named symbols — so it carries no semantic-identity guarantee and no -/// Wright product, CLI, LSP, agent, or embedding workflow may use it as a -/// semantic-rename fallback. The supported rename is the semantic path -/// (#434): `semanticRename`, [`CompilerSession::semantic_rename`], or -/// `ToolRequest::SemanticRename`, which rewrites exact identifier spans. +/// Wright product, CLI, LSP, agent, or embedding workflow may use it. /// Removal requires a breaking embedding-contract transition. -#[deprecated = "textual whole-word proposal helper with no semantic-identity guarantee; use `semanticRename` instead"] +#[deprecated = "textual whole-word proposal helper with no semantic-identity guarantee"] #[allow(deprecated)] pub fn rename_symbol(source: &str, request: &RenameRequest) -> Result { if request.from.is_empty() || request.to.is_empty() { @@ -1417,324 +1009,32 @@ mod tests { let _ = std::fs::remove_dir_all(dir); } - #[test] - fn semantic_rename_rewrites_only_identifier_spans() { - let (dir, mut session) = workshop_session(WORKSHOP); - let loaded = session.load().expect("loaded"); - let sources = BTreeMap::from([(loaded.input.display.clone(), loaded.input.text.clone())]); - let rename = semantic_rename( - &loaded, - session.catalog(), - Some(&sources), - &RenameTarget { - symbol: Some(Address::Name("score".to_string())), - source: None, - line: None, - col: None, - to: "total".to_string(), - }, - ); - assert!(rename.ok, "{:?}", rename.diagnostics); - let transaction = rename.transaction.expect("transaction"); - // Declaration + `Set Global Variable` arg + `Global.score` read. - assert_eq!(transaction.edits.len(), 3); - for edit in &transaction.edits { - assert_eq!(edit.edit_kind, "rename"); - assert_eq!(edit.new_text, "total"); - let lines: Vec<&str> = loaded.input.text.split('\n').collect(); - let line = lines[edit.range.start_line as usize - 1]; - assert_eq!(edit.range.start_line, edit.range.end_line); - assert_eq!( - &line[edit.range.start_col as usize - 1..edit.range.end_col as usize - 1], - "score" - ); - } - let preview = rename.preview.expect("preview"); - assert!(preview[0].new_text.contains("0: total")); - assert!(preview[0].new_text.contains("Set Global Variable(total,")); - assert!(preview[0].new_text.contains("Add(Global.total, 1)")); - let _ = std::fs::remove_dir_all(dir); - } - - #[test] - fn semantic_rename_by_position_and_stale_sources_refuse() { - let (dir, mut session) = workshop_session(WORKSHOP); - let loaded = session.load().expect("loaded"); - let sources = BTreeMap::from([(loaded.input.display.clone(), loaded.input.text.clone())]); - // Position inside the `score` occurrence of `Set Global Variable`. - let by_position = semantic_rename( - &loaded, - session.catalog(), - Some(&sources), - &RenameTarget { - symbol: None, - source: Some(loaded.input.display.clone()), - line: Some(11), - col: Some(30), - to: "total".to_string(), - }, - ); - assert!(by_position.ok, "{:?}", by_position.diagnostics); - - let stale = BTreeMap::from([( - loaded.input.display.clone(), - loaded.input.text.replacen("score", "other", 1), - )]); - let rename = semantic_rename( - &loaded, - session.catalog(), - Some(&stale), - &RenameTarget { - symbol: Some(Address::Name("score".to_string())), - source: None, - line: None, - col: None, - to: "total".to_string(), - }, - ); - assert!(!rename.ok); - assert_eq!(rename.diagnostics[0].code, "edit-stale-source"); - - for (target, code) in [ - ( - RenameTarget { - symbol: Some(Address::Name("missing".to_string())), - source: None, - line: None, - col: None, - to: "x".to_string(), - }, - "unknown-symbol", - ), - ( - RenameTarget { - symbol: None, - source: Some(loaded.input.display.clone()), - line: Some(1), - col: Some(1), - to: "x".to_string(), - }, - "rename-invalid-target", - ), - ] { - let rename = semantic_rename(&loaded, session.catalog(), Some(&sources), &target); - assert!(!rename.ok); - assert_eq!(rename.diagnostics[0].code, code, "{target:?}"); - } - let _ = std::fs::remove_dir_all(dir); - } - - #[test] - fn semantic_rename_refuses_collisions_and_unsupported_kinds() { - let (dir, mut session) = workshop_session(WORKSHOP); - let loaded = session.load().expect("loaded"); - let sources = BTreeMap::from([(loaded.input.display.clone(), loaded.input.text.clone())]); - let collision = semantic_rename( - &loaded, - session.catalog(), - Some(&sources), - &RenameTarget { - symbol: Some(Address::Name("score".to_string())), - source: None, - line: None, - col: None, - to: "score2".to_string(), - }, - ); - assert!(collision.ok); - let (dir2, mut session2) = - workshop_session(&WORKSHOP.replacen("0: score", "0: score\n 1: other", 1)); - let loaded2 = session2.load().expect("loaded"); - let sources2 = - BTreeMap::from([(loaded2.input.display.clone(), loaded2.input.text.clone())]); - let collision = semantic_rename( - &loaded2, - session2.catalog(), - Some(&sources2), - &RenameTarget { - symbol: Some(Address::Name("score".to_string())), - source: None, - line: None, - col: None, - to: "other".to_string(), - }, - ); - assert!(!collision.ok); - assert_eq!(collision.diagnostics[0].code, "rename-name-collision"); - let rule = semantic_rename( - &loaded, - session.catalog(), - Some(&sources), - &RenameTarget { - symbol: Some(Address::Name("r".to_string())), - source: None, - line: None, - col: None, - to: "renamed".to_string(), - }, - ); - assert!(!rule.ok); - assert_eq!(rule.diagnostics[0].code, "rename-unsupported-kind"); - let typo = semantic_rename( - &loaded, - session.catalog(), - Some(&sources), - &RenameTarget { - symbol: Some(Address::Name("scor".to_string())), - source: None, - line: None, - col: None, - to: "renamed".to_string(), - }, - ); - assert!(!typo.ok); - assert_eq!(typo.diagnostics[0].code, "unknown-symbol"); - assert!( - typo.diagnostics[0] - .message - .contains("nearest declared: 'score'"), - "{:?}", - typo.diagnostics[0] - ); - let _ = std::fs::remove_dir_all(dir); - let _ = std::fs::remove_dir_all(dir2); - } - - #[test] - fn the_rename_invariant_catches_edits_that_drop_a_reference() { - // Ablation: a transaction missing one occurrence still fails the - // post-reparse reference-shape check rather than "succeeding". - let (dir, mut session) = workshop_session(WORKSHOP); - let loaded = session.load().expect("loaded"); - let index = SemanticIndex::build(&loaded.program); - let symbol = index - .symbols() - .find(|symbol| symbol.name == "score") - .expect("score symbol"); - let edited = loaded.input.text.replacen("0: score", "0: total", 1); - let result = verify_rename(&index, symbol, "total", &edited, &loaded, session.catalog()); - let diagnostics = result.expect_err("a partial rename refuses"); - assert_eq!(diagnostics[0].code, "rename-mismatch"); - let _ = std::fs::remove_dir_all(dir); - } - - #[test] - fn semantic_rename_covers_player_variables_and_subroutines() { - // #434 acceptance: player variables rewrite their declaration plus - // `Event Player.name` occurrences (prefix preserved), and - // subroutines rewrite the declaration, the event binding, and calls. - let source = "variables {\n player:\n 1: streak\n}\n\nsubroutines {\n 0: helper\n}\n\nrule (\"r\") {\n event {\n Ongoing - Each Player;\n All;\n All;\n }\n actions {\n Set Player Variable(Event Player, streak, Add(Event Player.streak, 1));\n Call Subroutine(helper);\n }\n}\n\nrule (\"sub\") {\n event {\n Subroutine;\n helper;\n }\n actions {\n Set Player Variable(Event Player, streak, 0);\n }\n}\n"; - let (dir, mut session) = workshop_session(source); - let loaded = session.load().expect("loaded"); - let sources = BTreeMap::from([(loaded.input.display.clone(), loaded.input.text.clone())]); - - let player = semantic_rename( - &loaded, - session.catalog(), - Some(&sources), - &RenameTarget { - symbol: Some(Address::Name("streak".to_string())), - source: None, - line: None, - col: None, - to: "combo".to_string(), - }, - ); - assert!(player.ok, "{:?}", player.diagnostics); - let text = &player.preview.as_ref().unwrap()[0].new_text; - assert!(text.contains("1: combo"), "{text}"); - assert!(text.contains("Event Player, combo,"), "{text}"); - assert!(text.contains("Event Player.combo"), "{text}"); - assert!(!text.contains("streak"), "{text}"); - assert!(text.contains("Call Subroutine(helper)"), "{text}"); - - let subroutine = semantic_rename( - &loaded, - session.catalog(), - Some(&sources), - &RenameTarget { - symbol: Some(Address::Name("helper".to_string())), - source: None, - line: None, - col: None, - to: "assist".to_string(), - }, - ); - assert!(subroutine.ok, "{:?}", subroutine.diagnostics); - let text = &subroutine.preview.as_ref().unwrap()[0].new_text; - assert!(text.contains("0: assist"), "{text}"); - assert!(text.contains("Call Subroutine(assist)"), "{text}"); - assert!( - text.contains("Subroutine;\n assist;"), - "the event binding rewrites: {text}" - ); - assert!(!text.contains("helper"), "{text}"); - - // Position addressing inside the `Subroutine; helper;` binding. - let helper_line = source - .lines() - .enumerate() - .find(|(_, line)| line.trim() == "helper;") - .map(|(index, _)| index as u32 + 1) - .expect("event binding"); - let by_position = semantic_rename( - &loaded, - session.catalog(), - Some(&sources), - &RenameTarget { - symbol: None, - source: Some(loaded.input.display.clone()), - line: Some(helper_line), - col: Some(9), - to: "assist".to_string(), - }, - ); - assert!(by_position.ok, "{:?}", by_position.diagnostics); - assert_eq!( - by_position.transaction.unwrap().edits.len(), - subroutine.transaction.unwrap().edits.len(), - "position addressing resolves the same symbol" - ); - let _ = std::fs::remove_dir_all(dir); - } - #[test] fn write_previews_refuses_stale_sources_and_updates_atomically() { // #434: `write_previews` rechecks each file's identity before // writing; a file that changed since validation refuses with no // write at all. - let (dir, mut session) = workshop_session(WORKSHOP); - let loaded = session.load().expect("loaded"); - let sources = BTreeMap::from([(loaded.input.display.clone(), loaded.input.text.clone())]); - let rename = semantic_rename( - &loaded, - session.catalog(), - Some(&sources), - &RenameTarget { - symbol: Some(Address::Name("score".to_string())), - source: None, - line: None, - col: None, - to: "total".to_string(), - }, + let (dir, path) = temp_workshop(WORKSHOP); + let new_text = WORKSHOP.replacen("0: score", "0: total", 1).replacen( + "Global.score", + "Global.total", + 1, ); - assert!(rename.ok, "{:?}", rename.diagnostics); - let transaction = rename.transaction.as_ref().unwrap(); - let previews = rename.preview.as_ref().unwrap(); + let (previews, transaction) = write_fixtures(&[(path.as_path(), WORKSHOP, &new_text)]); - std::fs::write(loaded.input.path.as_ref().unwrap(), "// changed\n").unwrap(); - let error = write_previews(previews, transaction).unwrap_err(); + std::fs::write(&path, "// changed\n").unwrap(); + let error = write_previews(&previews, &transaction).unwrap_err(); assert_eq!(error.code, "edit-stale-source"); assert_eq!( - std::fs::read_to_string(loaded.input.path.as_ref().unwrap()).unwrap(), + std::fs::read_to_string(&path).unwrap(), "// changed\n", "a stale write leaves the file untouched" ); - std::fs::write(loaded.input.path.as_ref().unwrap(), &loaded.input.text).unwrap(); - let written = write_previews(previews, transaction).expect("writes"); + std::fs::write(&path, WORKSHOP).unwrap(); + let written = write_previews(&previews, &transaction).expect("writes"); assert_eq!(written.len(), 1); - let text = std::fs::read_to_string(loaded.input.path.as_ref().unwrap()).unwrap(); + let text = std::fs::read_to_string(&path).unwrap(); assert!(text.contains("0: total"), "{text}"); assert!(text.contains("Global.total"), "{text}"); let leftovers: Vec<_> = std::fs::read_dir(&dir) @@ -1951,7 +1251,7 @@ mod tests { std::fs::create_dir_all(&dir).unwrap(); let path = dir.join("input.opy"); std::fs::write(&path, "rule \"r\":\n pass\n").unwrap(); - let mut session = crate::CompilerSession::new(SessionConfig { + let session = crate::CompilerSession::new(SessionConfig { input: crate::InputSpec::Path(path.clone()), kind: SourceKind::Opy, ..SessionConfig::default() @@ -1987,24 +1287,6 @@ mod tests { .contains("providerValidateEdit"), "the refusal names the provider operation" ); - let rename = session.semantic_rename( - Some(&sources), - &RenameTarget { - symbol: Some(Address::Name("x".to_string())), - source: None, - line: None, - col: None, - to: "y".to_string(), - }, - ); - assert!(!rename.ok); - assert_eq!(rename.diagnostics[0].code, "edit-requires-provider"); - assert!( - rename.diagnostics[0] - .message - .contains("providerSemanticRename"), - "the refusal names the provider operation" - ); let _ = std::fs::remove_dir_all(dir); } @@ -2038,150 +1320,6 @@ mod tests { assert_eq!(positions, vec![2, 3], "edits are ordered by position"); } - #[test] - fn semantic_rename_rewrites_start_rule_callee() { - // A `Start Rule` subroutine argument is a value-position reference: - // its exact identifier span must rewrite with the rename. - let source = "subroutines {\n 0: helper\n}\n\nrule (\"sub\") {\n event {\n Subroutine;\n helper;\n }\n actions {\n Call Subroutine(helper);\n }\n}\n\nrule (\"r\") {\n event {\n Ongoing - Global;\n }\n actions {\n Start Rule(helper, Restart Rule);\n }\n}\n"; - let (dir, mut session) = workshop_session(source); - let loaded = session.load().expect("loaded"); - let sources = BTreeMap::from([(loaded.input.display.clone(), loaded.input.text.clone())]); - let rename = semantic_rename( - &loaded, - session.catalog(), - Some(&sources), - &RenameTarget { - symbol: Some(Address::Name("helper".to_string())), - source: None, - line: None, - col: None, - to: "assist".to_string(), - }, - ); - assert!(rename.ok, "{:?}", rename.diagnostics); - let text = &rename.preview.as_ref().unwrap()[0].new_text; - assert!(text.contains("0: assist"), "{text}"); - assert!(text.contains("Subroutine;\n assist;"), "{text}"); - assert!(text.contains("Call Subroutine(assist)"), "{text}"); - assert!( - text.contains("Start Rule(assist, Restart Rule)"), - "the Start Rule callee rewrites: {text}" - ); - assert!(!text.contains("helper"), "{text}"); - let _ = std::fs::remove_dir_all(dir); - } - - #[test] - fn semantic_rename_binds_redeclared_names_to_the_last_declaration() { - // workshop-rs resolves a redeclared name to its last declaration: - // every use of `helper` binds #1. Renaming #1 rewrites the uses with - // it; renaming #0 touches only its own declaration and leaves the - // surviving `helper` bindings intact. - let source = "subroutines {\n 0: helper\n 1: helper\n}\n\nrule (\"sub\") {\n event {\n Subroutine;\n helper;\n }\n actions {\n Call Subroutine(helper);\n }\n}\n\nrule (\"r\") {\n event {\n Ongoing - Global;\n }\n actions {\n Start Rule(helper, Restart Rule);\n }\n}\n"; - let (dir, mut session) = workshop_session(source); - let loaded = session.load().expect("loaded"); - let sources = BTreeMap::from([(loaded.input.display.clone(), loaded.input.text.clone())]); - let target = |symbol: Address| RenameTarget { - symbol: Some(symbol), - source: None, - line: None, - col: None, - to: "assist".to_string(), - }; - - let last = semantic_rename( - &loaded, - session.catalog(), - Some(&sources), - &target(Address::Id(1)), - ); - assert!(last.ok, "{:?}", last.diagnostics); - let text = &last.preview.as_ref().unwrap()[0].new_text; - assert!(text.contains("0: helper"), "{text}"); - assert!(text.contains("1: assist"), "{text}"); - assert!(text.contains("Subroutine;\n assist;"), "{text}"); - assert!(text.contains("Call Subroutine(assist)"), "{text}"); - assert!(text.contains("Start Rule(assist, Restart Rule)"), "{text}"); - assert_eq!(text.matches("helper").count(), 1, "{text}"); - - let first = semantic_rename( - &loaded, - session.catalog(), - Some(&sources), - &target(Address::Id(0)), - ); - assert!(first.ok, "{:?}", first.diagnostics); - let text = &first.preview.as_ref().unwrap()[0].new_text; - assert!(text.contains("0: assist"), "{text}"); - assert_eq!( - text.matches("helper").count(), - 4, - "the surviving declaration keeps every binding: {text}" - ); - let _ = std::fs::remove_dir_all(dir); - } - - #[test] - fn semantic_rename_leaves_other_namespaces_and_strings_untouched() { - // #434: a global variable, a player variable, and a rule name may - // share one identifier text. The namespaces are separate symbols and - // the rule name is a string literal, so renaming the global variable - // rewrites only its own declaration and references — same-named - // player occurrences, the rule name, and comments stay untouched. - let source = "// score tracks the global score\nvariables {\n global:\n 0: score\n player:\n 1: score\n}\n\nrule (\"score\") {\n event {\n Ongoing - Each Player;\n All;\n All;\n }\n actions {\n Set Global Variable(score, Add(Global.score, 1));\n Set Player Variable(Event Player, score, Add(Event Player.score, 1));\n }\n}\n"; - let (dir, mut session) = workshop_session(source); - let loaded = session.load().expect("loaded"); - let sources = BTreeMap::from([(loaded.input.display.clone(), loaded.input.text.clone())]); - let index = SemanticIndex::build(&loaded.program); - let id_of = |kind| { - index - .symbols() - .find(|symbol| symbol.name == "score" && symbol.kind == kind) - .map(|symbol| symbol.id.index() as u32) - .expect("score symbol") - }; - let target = |id| RenameTarget { - symbol: Some(Address::Id(id)), - source: None, - line: None, - col: None, - to: "streak".to_string(), - }; - - let global = semantic_rename( - &loaded, - session.catalog(), - Some(&sources), - &target(id_of(SymbolKind::GlobalVariable)), - ); - assert!(global.ok, "{:?}", global.diagnostics); - let text = &global.preview.as_ref().unwrap()[0].new_text; - assert!(text.contains("0: streak"), "{text}"); - assert!(text.contains("Set Global Variable(streak,"), "{text}"); - assert!(text.contains("Global.streak"), "{text}"); - assert!(text.contains("1: score"), "{text}"); - assert!(text.contains("Event Player, score,"), "{text}"); - assert!(text.contains("Event Player.score"), "{text}"); - assert!(text.contains("rule (\"score\")"), "{text}"); - assert!(text.contains("// score tracks the global score"), "{text}"); - - let player = semantic_rename( - &loaded, - session.catalog(), - Some(&sources), - &target(id_of(SymbolKind::PlayerVariable)), - ); - assert!(player.ok, "{:?}", player.diagnostics); - let text = &player.preview.as_ref().unwrap()[0].new_text; - assert!(text.contains("1: streak"), "{text}"); - assert!(text.contains("Event Player, streak,"), "{text}"); - assert!(text.contains("Event Player.streak"), "{text}"); - assert!(text.contains("0: score"), "{text}"); - assert!(text.contains("Set Global Variable(score,"), "{text}"); - assert!(text.contains("Global.score"), "{text}"); - let _ = std::fs::remove_dir_all(dir); - } - #[test] fn wire_transactions_recheck_the_transaction_invariants() { let (dir, session) = workshop_session(WORKSHOP); @@ -2264,26 +1402,4 @@ mod tests { assert_eq!(validation.diagnostics[0].code, "edit-unknown-source"); let _ = std::fs::remove_dir_all(dir); } - - #[test] - fn semantic_rename_refuses_an_unchanged_name() { - let (dir, mut session) = workshop_session(WORKSHOP); - let loaded = session.load().expect("loaded"); - let sources = BTreeMap::from([(loaded.input.display.clone(), loaded.input.text.clone())]); - let rename = semantic_rename( - &loaded, - session.catalog(), - Some(&sources), - &RenameTarget { - symbol: Some(Address::Name("score".to_string())), - source: None, - line: None, - col: None, - to: "score".to_string(), - }, - ); - assert!(!rename.ok); - assert_eq!(rename.diagnostics[0].code, "rename-invalid-name"); - let _ = std::fs::remove_dir_all(dir); - } } diff --git a/crates/wright-driver/src/fix.rs b/crates/wright-driver/src/fix.rs index 40f49138..33b5c2a0 100644 --- a/crates/wright-driver/src/fix.rs +++ b/crates/wright-driver/src/fix.rs @@ -19,7 +19,7 @@ use crate::session::{Loaded, Provenance}; /// A finding's materialized fix as it appears in the structured result: /// a stable `kind`, a one-line `summary`, and the validated-edit /// `transaction` a consumer previews through `validateEditTransaction` and -/// applies through the same source-edit path as a semantic rename. +/// applies through the same validated source-edit path. #[derive(Debug, Clone, Serialize)] pub struct LintFixProposal { pub kind: &'static str, diff --git a/crates/wright-driver/src/provider_edit.rs b/crates/wright-driver/src/provider_edit.rs index 657187eb..69670abf 100644 --- a/crates/wright-driver/src/provider_edit.rs +++ b/crates/wright-driver/src/provider_edit.rs @@ -33,23 +33,6 @@ pub struct ProviderRenameRequest { pub sources: BTreeMap, } -/// `wright rename ` on provider-owned input (#739): the CLI keeps its -/// name addressing while `lpp/rename` is position-addressed, so the name -/// first resolves to a declaration position through the provider's own -/// `lpp/symbols` listing — Wright never searches the source text for it. -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct ProviderNamedRenameRequest { - /// The declared project, on the same contract as - /// [`ProviderRenameRequest::documents`]. - pub documents: wright_lpp::DocumentSet, - /// The declared name to rename, exactly as `lpp/symbols` reports it. - pub name: String, - pub new_name: String, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub project_root: Option, - pub sources: BTreeMap, -} - #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ProviderValidateRequest { /// The declared project: a post-edit error in any member blocks the @@ -85,177 +68,9 @@ pub fn semantic_rename( ) } -/// The name-addressed rename behind `wright rename` on provider-owned input -/// (#739): `NAME` resolves through `lpp/symbols`, the declared project the -/// session loaded, then `lpp/rename` at the resolved declaration position -/// produces the edits the shared pipeline validates. Refusals and failures -/// keep the provider's own codes; a name with no declaration site is -/// `unknown-symbol` and one spelling several distinct symbols is -/// `ambiguous-symbol`. -pub fn named_rename( - provider: &mut dyn wright_lpp::LanguageProvider, - request: &ProviderNamedRenameRequest, -) -> ProviderMutation { - let listed = match provider.symbols(&request.documents, request.project_root.as_deref()) { - Ok(result) => result, - Err(error) => return provider_failure(&error), - }; - // Candidates are the distinct declaration sites spelling the name: the - // position `lpp/rename` would be pointed at, deduplicated because a - // provider can report one declaration under several kinds. - let mut sites: Vec = Vec::new(); - for document in &listed.documents { - if !request.documents.contains_key(&document.uri) { - continue; - } - for symbol in &document.symbols { - if symbol.name != request.name { - continue; - } - if sites - .iter() - .any(|site| site.uri == document.uri && site.position == symbol.range.start) - { - continue; - } - sites.push(NameSite { - uri: document.uri.clone(), - position: symbol.range.start, - kind: symbol.kind.clone(), - }); - } - } - match sites.as_slice() { - [] => refusal(vec![Diagnostic::error( - "unknown-symbol", - Stage::Discovery, - format!( - "unknown symbol '{}': the loaded project declares no symbol with that name", - request.name - ), - )]), - [site] => rename_at(provider, request, site), - sites => disambiguated_rename(provider, request, sites), - } -} - -/// One declaration site `lpp/symbols` reported for the requested name. -struct NameSite { - uri: String, - position: wright_lpp::Position, - kind: String, -} - -impl NameSite { - /// `kind at path:line:col` — the candidate spelling `ambiguous-symbol` - /// lists so the caller can tell the declarations apart. - fn describe(&self, documents: &wright_lpp::DocumentSet) -> String { - let (line, column) = documents - .get(&self.uri) - .and_then(|document| { - let lines: Vec<&str> = document.text.split('\n').collect(); - position_to_column(&lines, self.position, "declaration").ok() - }) - .unwrap_or((self.position.line + 1, self.position.character + 1)); - format!( - "{} at {}:{}:{}", - self.kind, - crate::source_provider::provider_uri_path(&self.uri), - line, - column - ) - } -} - -/// `lpp/rename` at the resolved declaration site, then the shared validated -/// pipeline. -fn rename_at( - provider: &mut dyn wright_lpp::LanguageProvider, - request: &ProviderNamedRenameRequest, - site: &NameSite, -) -> ProviderMutation { - let result = match provider.rename( - &request.documents, - &site.uri, - site.position, - &request.new_name, - request.project_root.as_deref(), - ) { - Ok(result) => result, - Err(error) => return provider_failure(&error), - }; - rename_result( - provider, - &request.documents, - &site.uri, - result, - &request.sources, - request.project_root.as_deref(), - ) -} - -/// Several declaration sites spell the requested name. Whether they denote -/// one rename target or several is the provider's call, so each site is -/// probed through `lpp/rename`: identical outcomes mean the sites belong to -/// the same symbol — one entity the provider itself declares under several -/// spellings — and the shared result is validated; divergent outcomes, or a -/// refusal only some sites share, mean the name is genuinely ambiguous and -/// it refuses `ambiguous-symbol` naming every candidate site. A failure -/// that is not a deliberate provider refusal still aborts the flow: it is -/// provider health, not evidence about the name. -fn disambiguated_rename( - provider: &mut dyn wright_lpp::LanguageProvider, - request: &ProviderNamedRenameRequest, - sites: &[NameSite], -) -> ProviderMutation { - let mut first: Option> = None; - for site in sites { - let outcome = match provider.rename( - &request.documents, - &site.uri, - site.position, - &request.new_name, - request.project_root.as_deref(), - ) { - Ok(result) => Ok(result), - Err(error) if is_refusal(&error) => Err(error), - Err(error) => return provider_failure(&error), - }; - match &first { - None => first = Some(outcome), - Some(prior) if outcome == *prior => {} - Some(_) => { - return refusal(vec![Diagnostic::error( - "ambiguous-symbol", - Stage::Discovery, - format!( - "ambiguous symbol '{}': {}", - request.name, - sites - .iter() - .map(|site| site.describe(&request.documents)) - .collect::>() - .join(", ") - ), - )]); - } - } - } - match first.expect("a multi-site name probes at least one site") { - Ok(result) => rename_result( - provider, - &request.documents, - &sites[0].uri, - result, - &request.sources, - request.project_root.as_deref(), - ), - Err(error) => provider_failure(&error), - } -} - -/// A deliberate provider decline (`kind: "refusal"`) is name-resolution -/// evidence; any other error is a provider failure on its own contract. +/// A deliberate provider decline (`kind: "refusal"`) is reported through +/// the refusal channel; any other error is a provider failure on its own +/// contract. fn is_refusal(error: &wright_lpp::ProviderError) -> bool { matches!( error, @@ -1441,394 +1256,4 @@ mod tests { assert!(mutation.transaction.is_none()); assert!(mutation.preview.is_none()); } - - /// The name-addressed rename surface (#739): `named_rename` resolves - /// `NAME` through the scripted `lpp/symbols` listing and scripts - /// `lpp/rename` outcomes per call so equivalence probing is observable. - struct NamedProvider { - symbols: Result, - renames: std::collections::VecDeque>, - rename_sites: Vec<(String, wright_lpp::Position)>, - validate_edits: Result, - check: Result, - } - - impl wright_lpp::LanguageProvider for NamedProvider { - fn initialize( - &mut self, - _client_info: Option<&ClientInfo>, - ) -> Result { - unreachable!("not used by flow unit tests") - } - fn capabilities(&self) -> Result<&NegotiatedCapabilities, ProviderError> { - unreachable!("not used by flow unit tests") - } - fn check( - &mut self, - _documents: &DocumentSet, - _project_root: Option<&str>, - ) -> Result { - self.check.clone() - } - fn compile( - &mut self, - _documents: &DocumentSet, - _project_root: Option<&str>, - ) -> Result { - unreachable!("not used by flow unit tests") - } - fn reconstruct( - &mut self, - _artifact: &WorkshopArtifact, - ) -> Result { - unreachable!("not used by flow unit tests") - } - fn symbols( - &mut self, - _documents: &DocumentSet, - _project_root: Option<&str>, - ) -> Result { - self.symbols.clone() - } - fn definition( - &mut self, - _document: &Document, - _position: wright_lpp::Position, - ) -> Result { - unreachable!("not used by flow unit tests") - } - fn references( - &mut self, - _document: &Document, - _position: wright_lpp::Position, - _include_declaration: bool, - ) -> Result { - unreachable!("not used by flow unit tests") - } - fn rename( - &mut self, - _documents: &DocumentSet, - position_document_uri: &str, - position: wright_lpp::Position, - _new_name: &str, - _project_root: Option<&str>, - ) -> Result { - self.rename_sites - .push((position_document_uri.to_string(), position)); - self.renames - .pop_front() - .expect("the test scripts every expected rename call") - } - fn validate_edits( - &mut self, - _document: &Document, - _edits: &[wright_lpp::TextEdit], - ) -> Result { - self.validate_edits.clone() - } - fn shutdown(&mut self) -> Result<(), ProviderError> { - unreachable!("not used by flow unit tests") - } - fn exit_status(&self) -> Option { - None - } - } - - fn position(line: u32, character: u32) -> wright_lpp::Position { - wright_lpp::Position { line, character } - } - - fn declared(name: &str, kind: &str, start: wright_lpp::Position) -> wright_lpp::Symbol { - wright_lpp::Symbol { - name: name.to_string(), - kind: kind.to_string(), - range: wright_lpp::Range { - start, - end: wright_lpp::Position { - line: start.line, - character: start.character + name.len() as u32, - }, - }, - selection_range: None, - } - } - - /// A `lpp/symbols` listing: `(uri, name, kind, line, character)` tuples - /// grouped per document. - fn symbols_result(sites: &[(&str, &str, &str, u32, u32)]) -> SymbolsResult { - let mut documents: BTreeMap<&str, Vec> = BTreeMap::new(); - for (uri, name, kind, line, character) in sites { - documents.entry(uri).or_default().push(declared( - name, - kind, - position(*line, *character), - )); - } - SymbolsResult { - documents: documents - .into_iter() - .map(|(uri, symbols)| wright_lpp::DocumentSymbols { - uri: uri.to_string(), - version: 3, - symbols, - }) - .collect(), - } - } - - fn named_request(name: &str) -> ProviderNamedRenameRequest { - ProviderNamedRenameRequest { - documents: document_set(), - name: name.to_string(), - new_name: "twice".to_string(), - project_root: Some("file:///project".to_string()), - sources: sources(CLEAN), - } - } - - fn refused(code: &str) -> ProviderError { - ProviderError::lpp( - wright_lpp::LppErrorKind::Refusal, - serde_json::json!({ "refusalCode": code }), - format!("declined with {code}"), - ) - } - - fn ok_edits(version: i64) -> ValidateEditsResult { - ValidateEditsResult { - valid: true, - version, - reason: None, - failing_edit_index: None, - } - } - - #[test] - fn named_rename_resolves_the_name_through_symbols() { - let mut provider = NamedProvider { - symbols: Ok(symbols_result(&[(URI, "double", "op", 4, 4)])), - renames: [Ok(clean_rename_result())].into(), - rename_sites: Vec::new(), - validate_edits: Ok(ok_edits(3)), - check: Ok(ok_check()), - }; - let mutation = named_rename(&mut provider, &named_request("double")); - assert!(mutation.ok, "rename succeeds: {:?}", mutation.diagnostics); - assert_eq!( - provider.rename_sites, - vec![(URI.to_string(), position(4, 4))], - "the declared name resolves to its `lpp/symbols` declaration start" - ); - let transaction = mutation.transaction.expect("transaction"); - assert_eq!(transaction.edits.len(), 3); - let preview = mutation.preview.expect("preview"); - assert!(preview[0].new_text.contains("twice: x => x * 2")); - } - - #[test] - fn named_rename_deduplicates_a_site_listed_under_several_kinds() { - // A provider may report one declaration under several kinds — an OPY - // `def` also reports its materialized subroutine declaration. The - // same position is one candidate, not an ambiguity. - let mut provider = NamedProvider { - symbols: Ok(symbols_result(&[ - (URI, "double", "subroutine", 4, 4), - (URI, "double", "def", 4, 4), - ])), - renames: [Ok(clean_rename_result())].into(), - rename_sites: Vec::new(), - validate_edits: Ok(ok_edits(3)), - check: Ok(ok_check()), - }; - let mutation = named_rename(&mut provider, &named_request("double")); - assert!(mutation.ok, "{:?}", mutation.diagnostics); - assert_eq!(provider.rename_sites.len(), 1, "one site is probed once"); - } - - #[test] - fn named_rename_unknown_name_is_a_structured_refusal() { - let mut provider = NamedProvider { - symbols: Ok(symbols_result(&[(URI, "plus1", "op", 5, 4)])), - renames: [].into(), - rename_sites: Vec::new(), - validate_edits: Ok(ok_edits(3)), - check: Ok(ok_check()), - }; - let mutation = named_rename(&mut provider, &named_request("missing")); - assert!(!mutation.ok); - assert_eq!(mutation.diagnostics[0].code, "unknown-symbol"); - assert!( - provider.rename_sites.is_empty(), - "no `lpp/rename` is sent for an undeclared name" - ); - assert!(mutation.transaction.is_none()); - assert!(mutation.preview.is_none()); - } - - #[test] - fn named_rename_symbols_capability_gap_stays_structured() { - let mut provider = NamedProvider { - symbols: Err(ProviderError::lpp( - wright_lpp::LppErrorKind::CapabilityUnavailable, - serde_json::json!({ "capability": "symbols", "method": "lpp/symbols" }), - "capability 'symbols' is not available in this provider client", - )), - renames: [].into(), - rename_sites: Vec::new(), - validate_edits: Ok(ok_edits(3)), - check: Ok(ok_check()), - }; - let mutation = named_rename(&mut provider, &named_request("double")); - assert!(!mutation.ok); - assert_eq!(mutation.diagnostics[0].code, "capability-unavailable"); - assert_eq!( - mutation.provider_code.as_deref(), - Some("capability-unavailable") - ); - } - - #[test] - fn named_rename_passes_a_provider_refusal_through_unchanged() { - let mut provider = NamedProvider { - symbols: Ok(symbols_result(&[(URI, "double", "op", 4, 4)])), - renames: [Err(refused("rename.invalidName"))].into(), - rename_sites: Vec::new(), - validate_edits: Ok(ok_edits(3)), - check: Ok(ok_check()), - }; - let mutation = named_rename(&mut provider, &named_request("double")); - assert!(!mutation.ok); - assert_eq!( - mutation.diagnostics[0].code, - "provider-refusal-rename.invalidName" - ); - assert_eq!( - mutation.provider_code.as_deref(), - Some("rename.invalidName"), - "the provider's refusal code is preserved verbatim" - ); - assert!(mutation.transaction.is_none()); - assert!(mutation.preview.is_none()); - } - - #[test] - fn named_rename_multiplies_declaration_sites_of_one_symbol_into_one_rename() { - // One symbol declared at two sites — a `subroutine` declaration and - // its `def` implementation — answers `lpp/rename` identically from - // either site; the name resolves to that one rename. - let mut provider = NamedProvider { - symbols: Ok(symbols_result(&[ - (URI, "double", "subroutine", 4, 4), - (URI, "double", "def", 1, 0), - ])), - renames: [Ok(clean_rename_result()), Ok(clean_rename_result())].into(), - rename_sites: Vec::new(), - validate_edits: Ok(ok_edits(3)), - check: Ok(ok_check()), - }; - let mutation = named_rename(&mut provider, &named_request("double")); - assert!(mutation.ok, "{:?}", mutation.diagnostics); - assert_eq!( - provider.rename_sites.len(), - 2, - "both sites were probed before the identical outcome collapsed" - ); - let preview = mutation.preview.expect("preview"); - assert!(preview[0].new_text.contains("twice: x => x * 2")); - } - - #[test] - fn named_rename_distinct_sites_refuse_ambiguous_with_locations() { - let mut second = document("puzzle other {\n target = 1\n}"); - second.uri = "file:///project/other.xdl".to_string(); - let mut request = named_request("shared"); - request.documents.insert(second.uri.clone(), second); - request.sources.insert( - "file:///project/other.xdl".to_string(), - "puzzle other {\n target = 1\n}".to_string(), - ); - let mut divergent = clean_rename_result(); - divergent.edits[0].text_edits.pop(); - let mut provider = NamedProvider { - symbols: Ok(symbols_result(&[ - (URI, "shared", "op", 4, 4), - ("file:///project/other.xdl", "shared", "op", 1, 0), - ])), - renames: [Ok(clean_rename_result()), Ok(divergent)].into(), - rename_sites: Vec::new(), - validate_edits: Ok(ok_edits(3)), - check: Ok(ok_check()), - }; - let mutation = named_rename(&mut provider, &request); - assert!(!mutation.ok); - assert_eq!(mutation.diagnostics[0].code, "ambiguous-symbol"); - let message = &mutation.diagnostics[0].message; - assert!( - message.contains("op at /project/puzzle.xdl:5:5"), - "the first candidate's path:line:col is named: {message}" - ); - assert!( - message.contains("op at /project/other.xdl:2:1"), - "the second candidate's path:line:col is named: {message}" - ); - assert!(mutation.transaction.is_none()); - assert!(mutation.preview.is_none()); - } - - #[test] - fn named_rename_collapses_an_identical_refusal_across_sites() { - // Every site declines the same way — the refusal is the answer for - // whichever symbol the caller meant, so it passes through rather - // than reporting ambiguity. - let mut provider = NamedProvider { - symbols: Ok(symbols_result(&[ - (URI, "double", "subroutine", 4, 4), - (URI, "double", "def", 1, 0), - ])), - renames: [ - Err(refused("rename.invalidName")), - Err(refused("rename.invalidName")), - ] - .into(), - rename_sites: Vec::new(), - validate_edits: Ok(ok_edits(3)), - check: Ok(ok_check()), - }; - let mutation = named_rename(&mut provider, &named_request("double")); - assert!(!mutation.ok); - assert_eq!( - mutation.diagnostics[0].code, - "provider-refusal-rename.invalidName" - ); - assert_eq!( - mutation.provider_code.as_deref(), - Some("rename.invalidName") - ); - } - - #[test] - fn named_rename_provider_health_failure_is_not_ambiguity() { - let mut provider = NamedProvider { - symbols: Ok(symbols_result(&[ - (URI, "double", "subroutine", 4, 4), - (URI, "double", "def", 1, 0), - ])), - renames: [ - Ok(clean_rename_result()), - Err(ProviderError::Io { - message: "broken pipe".to_string(), - }), - ] - .into(), - rename_sites: Vec::new(), - validate_edits: Ok(ok_edits(3)), - check: Ok(ok_check()), - }; - let mutation = named_rename(&mut provider, &named_request("double")); - assert!(!mutation.ok); - assert_eq!(mutation.diagnostics[0].code, "provider-io"); - assert_eq!(mutation.provider_code.as_deref(), Some("provider-io")); - assert!(mutation.transaction.is_none()); - assert!(mutation.preview.is_none()); - } } diff --git a/crates/wright-driver/src/service.rs b/crates/wright-driver/src/service.rs index 561564dc..ad1df02b 100644 --- a/crates/wright-driver/src/service.rs +++ b/crates/wright-driver/src/service.rs @@ -60,8 +60,6 @@ pub const OPERATIONS: &[&str] = &[ "analyze", "inspect", "validateEditTransaction", - "semanticRename", - "providerSemanticRename", "providerValidateEdit", "lookup", ]; @@ -277,64 +275,6 @@ pub enum ToolRequest { sources: Option>, transaction: crate::edit::EditTransaction, }, - /// Request a semantic rename through the shared refactoring contract - /// (#129/#130/#434): returns the validated exact-range transaction or - /// structured refusal diagnostics. On raw Workshop input the target is - /// a `symbol` (numeric id or declared name, as `references`/`usage` - /// address them) or a `source`/`line`/`col` position, and occurrences - /// rewrite through `workshop-rs` identifier provenance. Source - /// languages route to `providerSemanticRename`. Wright - /// proposes/validates; applying edits to disk is an explicit consumer - /// responsibility. - SemanticRename { - /// The current text of every source the rename may edit, keyed by - /// the same source identities the target names. Optional (#472): - /// when absent the service reads the loaded input's current text - /// from disk; when present it stays the caller-side precondition - /// text. - #[serde(default, skip_serializing_if = "Option::is_none")] - sources: Option>, - target: crate::edit::RenameTarget, - }, - /// Provider-driven semantic rename (#139): rename target resolution and - /// edit generation route through the LPP `rename` capability of the - /// provider configured for `language_id`; the resulting source edits are - /// wrapped in Wright's own transaction (identity/version preconditions, - /// deterministic ordering, overlap checks, atomic preview) and validated - /// through the provider's project semantics (`lpp/validateEdits` per - /// edited document, then `lpp/check` over the edited project) before - /// success. Provider refusals, unsupported capabilities, stale sources, - /// and semantic validation failures are structured refusals with no - /// partial edit set; there is no fallback to textual search/replace. - #[serde(rename = "providerSemanticRename")] - ProviderSemanticRename { - /// The opaque language id of the configured provider. - language_id: String, - /// The document set the rename is computed against (the provider's - /// view: text, language id, version). Optional (#548): when omitted, - /// the session's loaded project supplies it — every member file read - /// from disk at `version` 0, keyed by `file://` URI. An omitted set - /// therefore reads the program the way the other program-reading - /// operations do. - #[serde(default, skip_serializing_if = "Option::is_none")] - documents: Option, - /// The URI (a key of `documents`) in which `position` is - /// interpreted. - position_document_uri: String, - /// The position of the symbol to rename (0-based LSP conventions). - position: wright_lpp::Position, - /// The new name; the provider validates it against the language's - /// identifier rules. - new_name: String, - /// The project the documents belong to (informational). - #[serde(default, skip_serializing_if = "Option::is_none")] - project_root: Option, - /// The caller's current text for every source the rename may edit, - /// keyed by document URI (the identity/version precondition view). - /// Optional (#548): defaults to each document's text. - #[serde(default, skip_serializing_if = "Option::is_none")] - sources: Option>, - }, /// Provider-driven edit validation (#139): validate a caller-proposed /// source-edit transaction against the provider's project semantics /// (`lpp/validateEdits` per edited document, then `lpp/check` over the @@ -631,10 +571,6 @@ impl<'a> ToolService<'a> { | ToolRequest::Compile { .. } | ToolRequest::Capabilities | ToolRequest::TargetMetadata - | ToolRequest::ProviderSemanticRename { - documents: Some(_), - .. - } | ToolRequest::ProviderValidateEdit { documents: Some(_), .. @@ -817,9 +753,6 @@ impl<'a> ToolService<'a> { rule: Some(Address::Id(_)), .. } => !self.rule_ids_current, - ToolRequest::SemanticRename { target, .. } => { - matches!(target.symbol, Some(Address::Id(_))) && !self.symbol_ids_current - } _ => false, }; stale.then(|| ToolErrorInfo { @@ -830,18 +763,12 @@ impl<'a> ToolService<'a> { /// A request that names the current id space re-establishes it (#471): a /// successful `symbols`/`rules` listing, or an `ambiguous-*` refusal that - /// already reports the current candidates. `semanticRename` reports its - /// `ambiguous-symbol` as a result diagnostic rather than a refusal. + /// already reports the current candidates. fn note_id_space(&mut self, request: &ToolRequest, response: &ToolResponse) { match response { - ToolResponse::Ok { result } => match request { + ToolResponse::Ok { .. } => match request { ToolRequest::Symbols { .. } => self.symbol_ids_current = true, ToolRequest::Rules { .. } => self.rule_ids_current = true, - ToolRequest::SemanticRename { .. } - if diagnostics_list_code(result, "ambiguous-symbol") => - { - self.symbol_ids_current = true; - } _ => {} }, ToolResponse::Error { error } => match error.code.as_str() { @@ -957,40 +884,6 @@ impl<'a> ToolService<'a> { .validate_edit_transaction(sources.as_ref(), transaction), ) .expect("serializes")), - ToolRequest::SemanticRename { sources, target } => { - let rename = self.session.semantic_rename(sources.as_ref(), target); - self.ok(serde_json::to_value(rename).expect("serializes")) - } - ToolRequest::ProviderSemanticRename { - language_id, - documents, - position_document_uri, - position, - new_name, - project_root, - sources, - } => { - let documents = match self.provider_documents(language_id, documents) { - Ok(documents) => documents, - Err(error) => return ToolResponse::Error { error }, - }; - let req = crate::provider_edit::ProviderRenameRequest { - position_document_uri: position_document_uri.clone(), - position: *position, - new_name: new_name.clone(), - project_root: project_root.clone(), - sources: sources - .clone() - .unwrap_or_else(|| document_sources(&documents)), - documents, - }; - self.ok( - serde_json::to_value(self.run_provider_flow(language_id, |p| { - crate::provider_edit::semantic_rename(p, &req) - })) - .expect("serializes"), - ) - } ToolRequest::ProviderValidateEdit { language_id, documents, @@ -1929,22 +1822,7 @@ fn request_label(request: &ToolRequest) -> &'static str { ToolRequest::CostEstimate { .. } => "req::costEstimate", ToolRequest::TargetMetadata => "req::targetMetadata", ToolRequest::ValidateEdit { .. } => "req::validateEdit", - ToolRequest::SemanticRename { .. } => "req::semanticRename", - ToolRequest::ProviderSemanticRename { .. } => "req::providerSemanticRename", ToolRequest::ProviderValidateEdit { .. } => "req::providerValidateEdit", ToolRequest::Lookup { .. } => "req::lookup", } } - -/// Whether a result payload's `diagnostics` include `code` (#471): the -/// `semanticRename` refusal surface embeds `ambiguous-symbol` this way. -fn diagnostics_list_code(result: &serde_json::Value, code: &str) -> bool { - result - .get("diagnostics") - .and_then(serde_json::Value::as_array) - .is_some_and(|diagnostics| { - diagnostics - .iter() - .any(|d| d.get("code").and_then(serde_json::Value::as_str) == Some(code)) - }) -} diff --git a/crates/wright-driver/src/session/edit.rs b/crates/wright-driver/src/session/edit.rs index 74df89eb..6fde6d2f 100644 --- a/crates/wright-driver/src/session/edit.rs +++ b/crates/wright-driver/src/session/edit.rs @@ -1,23 +1,20 @@ //! Validated source-edit operations on the session (#434): -//! `validateEditTransaction` and `semanticRename` run on raw Workshop -//! input through `workshop-rs` reparse and identifier provenance; other -//! source languages stay at the provider boundary. `rename` is the -//! `wright rename` CLI workflow — a validated diff by default, an atomic -//! filesystem update with `--write` — and on OPY input it routes the same -//! name addressing through the provider's `lpp/symbols` and `lpp/rename` -//! (#739). +//! `validateEditTransaction` runs on raw Workshop input through +//! `workshop-rs` reparse; other source languages stay at the provider +//! boundary. `lint --fix` resolves finding fixes through the same +//! validated source-edit path — a validated diff by default, an atomic +//! filesystem update with `--write`. use std::collections::BTreeMap; -use super::{CompilerSession, Provenance, ProviderOperation}; -use crate::config::{InputSpec, SourceKind}; +use super::{CompilerSession, Provenance}; +use crate::config::InputSpec; use crate::diag::{Diagnostic, Stage}; -use crate::edit::{EditTransaction, EditValidation, RenameResult, RenameTarget, SemanticRename}; +use crate::edit::{EditTransaction, EditValidation}; use crate::input; use crate::result::{ Envelope, LintFixOutcome, LintFixPreview, LintFixStatus, LintResult, exit_code_from, }; -use crate::service::Address; impl CompilerSession { /// `validateEditTransaction` (#434): apply `transaction` to the supplied @@ -44,225 +41,6 @@ impl CompilerSession { crate::edit::validate_transaction(&self.config, self.catalog, sources, transaction) } - /// `semanticRename` (#434): resolve `target` against the loaded - /// program's semantic index and rewrite every identifier occurrence - /// through `workshop-rs` provenance — never a textual search. When - /// `sources` is absent (#472) the loaded input's current text is read - /// from disk; when present it stays the caller's precondition text. - /// OPY and other source languages refuse with `edit-requires-provider` - /// naming `providerSemanticRename`, whether or not a provider is - /// configured. - pub fn semantic_rename( - &mut self, - sources: Option<&BTreeMap>, - target: &RenameTarget, - ) -> SemanticRename { - let refuse = |diagnostics: Vec| SemanticRename { - ok: false, - transaction: None, - diagnostics, - preview: None, - }; - if let Err(diagnostic) = self.verify_fixed_config() { - return refuse(vec![diagnostic]); - } - match crate::edit::resolve_edit_input(&self.config) { - Ok(resolved) => { - if let Some(diagnostic) = - crate::edit::edit_kind_gate(resolved.kind, "providerSemanticRename") - { - return refuse(vec![diagnostic]); - } - } - Err(diagnostic) => return refuse(vec![diagnostic]), - } - match self.load() { - Ok(loaded) => crate::edit::semantic_rename(&loaded, self.catalog, sources, target), - Err(diagnostic) => refuse(vec![diagnostic]), - } - } - - /// `wright rename [INPUT]` (#434, #739): the validated - /// semantic rename — a Workshop variable or subroutine through the raw - /// Workshop path, an OPY symbol through the provider path. Either way - /// the result is the transaction and a source diff by default, an - /// atomic filesystem update with `write`. A stale input or an invalid - /// rename refuses with structured diagnostics and no partial write. - pub fn rename(&mut self, name: &str, to: &str, write: bool) -> Envelope { - let mut result = RenameResult::default(); - if let Err(diagnostic) = self.verify_fixed_config() { - self.diagnostics.push(diagnostic); - return self.finish("rename", result); - } - match crate::edit::resolve_edit_input(&self.config) { - Ok(resolved) => { - // #739: OPY input renames through the provider — the same - // `NAME` addressing resolves to a declaration position - // through the provider's `lpp/symbols`, never Wright-side - // text search, and `lpp/rename` produces the validated - // transaction. - if resolved.kind == SourceKind::Opy { - return self.provider_rename(name, to, write); - } - if let Some(diagnostic) = - crate::edit::edit_kind_gate(resolved.kind, "providerSemanticRename") - { - self.diagnostics.push(diagnostic); - return self.finish("rename", result); - } - } - Err(diagnostic) => { - self.diagnostics.push(diagnostic); - return self.finish("rename", result); - } - } - let loaded = match self.load() { - Ok(loaded) => loaded, - Err(diagnostic) => { - self.diagnostics.push(diagnostic); - return self.finish("rename", result); - } - }; - let sources = BTreeMap::from([(loaded.input.display.clone(), loaded.input.text.clone())]); - let rename = crate::edit::semantic_rename( - &loaded, - self.catalog, - Some(&sources), - &RenameTarget { - symbol: Some(Address::Name(name.to_string())), - source: None, - line: None, - col: None, - to: to.to_string(), - }, - ); - self.diagnostics.extend(rename.diagnostics); - result.transaction = rename.transaction; - result.preview = rename.preview; - result.originals = sources; - if !rename.ok { - return self.finish("rename", result); - } - if write { - match write_input_refusal(&self.config.input, "rename --write") { - Some(diagnostic) => self.diagnostics.push(diagnostic), - None => match crate::edit::write_previews( - result.preview.as_deref().unwrap_or_default(), - result - .transaction - .as_ref() - .expect("a successful rename carries its transaction"), - ) { - Ok(written) => result.written = written, - Err(diagnostic) => self.diagnostics.push(diagnostic), - }, - } - } - self.finish("rename", result) - } - - /// `wright rename [INPUT]` on OPY input (#739): the - /// provider-owned rename the agent surface already exposes as - /// `providerSemanticRename`, reached through the CLI's `NAME` - /// addressing. The loaded project's members build the provider document - /// set; [`crate::provider_edit::named_rename`] resolves `NAME` to a - /// declaration position through `lpp/symbols` and drives `lpp/rename` - /// with `lpp/validateEdits` plus a post-edit project check — the same - /// transaction, stale-source, and atomicity contract the Workshop path - /// holds. `write` applies the validated previews atomically. - fn provider_rename(&mut self, name: &str, to: &str, write: bool) -> Envelope { - let mut result = RenameResult::default(); - let loaded = match self.load_with_operation(ProviderOperation::Compile) { - Ok(loaded) => loaded, - Err(diagnostic) => { - self.diagnostics.push(diagnostic); - return self.finish("rename", result); - } - }; - // Mapped members spell their `file://` URIs relative to the project - // root the provider compiled under; an unmapped load's members are - // the input's own display spellings relative to the invocation cwd — - // the same convention `ToolService::provider_documents` applies. - let base = if loaded.provenance == Provenance::Mapped { - &loaded.input.root - } else { - &loaded.input.cwd - }; - let documents = match crate::source_provider::provider_document_set( - &loaded.source_files, - base, - crate::opy_provider::OPY_LANGUAGE_ID, - ) { - Ok(documents) => documents, - Err(info) => { - self.diagnostics - .push(Diagnostic::error(info.code, Stage::Discovery, info.message)); - return self.finish("rename", result); - } - }; - let sources = crate::source_provider::document_sources(&documents); - let request = crate::provider_edit::ProviderNamedRenameRequest { - documents, - name: name.to_string(), - new_name: to.to_string(), - project_root: crate::source_provider::provider_project_root(&loaded.input.root), - sources, - }; - let mutation = self.run_provider_flow( - crate::opy_provider::OPY_LANGUAGE_ID, - &wright_lpp::ClientInfo { - name: wright_lpp::LPP_CLIENT_NAME.to_string(), - version: crate::result::DRIVER_VERSION.to_string(), - }, - |provider| crate::provider_edit::named_rename(provider, &request), - ); - self.diagnostics.extend(mutation.diagnostics); - let previews = mutation.preview.unwrap_or_default(); - result.transaction = mutation.transaction; - if !mutation.ok { - return self.finish("rename", result); - } - if write { - match write_input_refusal(&self.config.input, "rename --write") { - Some(diagnostic) => self.diagnostics.push(diagnostic), - None => match crate::edit::write_previews( - &previews, - result - .transaction - .as_ref() - .expect("a successful rename carries its transaction"), - ) { - Ok(written) => { - result.written = written - .iter() - .map(|source| crate::source_provider::provider_uri_path(source)) - .collect(); - } - Err(diagnostic) => self.diagnostics.push(diagnostic), - }, - } - } - // The transaction and previews name sources by the member `file://` - // URIs the provider worked on; the human report and `--write` list - // show their disk paths instead, matching the Workshop preview. - result.originals = request - .sources - .iter() - .map(|(uri, text)| (crate::source_provider::provider_uri_path(uri), text.clone())) - .collect(); - result.preview = Some( - previews - .iter() - .map(|preview| crate::edit::SourcePreview { - source: crate::source_provider::provider_uri_path(&preview.source), - new_text: preview.new_text.clone(), - source_identity: preview.source_identity.clone(), - }) - .collect(), - ); - self.finish("rename", result) - } - /// `wright lint --fix [--write]` (#556): resolve the fixes findings /// carry through the validated source-edit path. A preview run /// validates every offered fix's transaction against the current input diff --git a/crates/wright-driver/tests/driver.rs b/crates/wright-driver/tests/driver.rs index 5c8d3337..270a8d69 100644 --- a/crates/wright-driver/tests/driver.rs +++ b/crates/wright-driver/tests/driver.rs @@ -1,7 +1,7 @@ use std::path::{Path, PathBuf}; -use wright_driver::edit::{EditTransaction, RenameTarget}; -use wright_driver::service::{Address, ToolService}; +use wright_driver::edit::EditTransaction; +use wright_driver::service::ToolService; use wright_driver::{ CompilerSession, Diagnostic, InputSpec, SessionConfig, SourceBackend, SourceKind, }; @@ -537,9 +537,7 @@ fn session_config_mutation_refuses_workflows() { assert!(refused_config_change( &session.symbols(None, None, None).diagnostics )); - assert!(refused_config_change( - &session.rename("x", "y", false).diagnostics - )); + assert!(refused_config_change(&session.lint_fix(false).diagnostics)); session.config.kind = SourceKind::Workshop; assert!(session.check().ok, "check: {:?}", session.diagnostics()); @@ -571,19 +569,6 @@ fn session_config_mutation_refuses_edits_and_service_construction() { validation.diagnostics ); - let rename = session.semantic_rename( - None, - &RenameTarget { - symbol: Some(Address::Name("x".to_string())), - source: None, - line: None, - col: None, - to: "y".to_string(), - }, - ); - assert!(!rename.ok); - assert!(refused_config_change(&rename.diagnostics)); - let error = ToolService::new(&mut session) .err() .expect("service refuses"); diff --git a/crates/wright-driver/tests/lint_fix.rs b/crates/wright-driver/tests/lint_fix.rs index b2bba752..93731f63 100644 --- a/crates/wright-driver/tests/lint_fix.rs +++ b/crates/wright-driver/tests/lint_fix.rs @@ -256,25 +256,4 @@ fn lint_fix_write_refuses_a_caller_held_text_input() { SOURCE, "the refusal writes nothing" ); - - let mut session = CompilerSession::new(SessionConfig { - input: InputSpec::Text { - text: SOURCE.to_string(), - path: Some(path.clone()), - }, - kind: SourceKind::Workshop, - ..SessionConfig::default() - }) - .unwrap(); - let renamed = session.rename("index", "renamed", true); - assert!( - renamed - .diagnostics - .iter() - .any(|d| d.code == "edit-input-stdin"), - "rename --write refuses a non-path input: {:?}", - renamed.diagnostics - ); - assert!(renamed.result.written.is_empty(), "nothing is written"); - assert_eq!(std::fs::read_to_string(&path).unwrap(), SOURCE); } diff --git a/crates/wright-driver/tests/provider_edit.rs b/crates/wright-driver/tests/provider_edit.rs index 29c7920b..b8e437ef 100644 --- a/crates/wright-driver/tests/provider_edit.rs +++ b/crates/wright-driver/tests/provider_edit.rs @@ -8,17 +8,14 @@ use wright_driver::config::{InputSpec, SessionConfig, SourceKind}; use wright_driver::edit::{EditRange, EditTransaction, SourceEdit}; use wright_driver::provider_edit::ProviderMutation; use wright_driver::service::{ToolRequest, ToolResponse, ToolService}; -use wright_lpp::{Document, DocumentSet, Position}; +use wright_lpp::{Document, DocumentSet}; const DEMO_LANGUAGE_ID: &str = "x-demo-lang"; /// A clean x-demo-lang puzzle document (from the LPP v1 spec transcript). const CLEAN_PUZZLE: &str = "puzzle clean {\n target = 40\n start = 10\n ops {\n double: x => x * 2\n plus1: x => x + 1\n }\n solution = [ double, double ]\n}"; -const SINGLE_OP_PUZZLE: &str = "puzzle clean {\n target = 40\n start = 10\n ops {\n double: x => x * 2\n }\n solution = [ double, double ]\n}"; - const URI: &str = "file:///project/puzzle.xdl"; -const URI_SECOND: &str = "file:///project/puzzle2.xdl"; fn workspace_root() -> PathBuf { Path::new(env!("CARGO_MANIFEST_DIR")).join("..").join("..") @@ -87,16 +84,6 @@ fn single_document_set() -> DocumentSet { documents } -fn two_document_set() -> DocumentSet { - let mut documents = DocumentSet::new(); - documents.insert(URI.to_string(), demo_document(URI, CLEAN_PUZZLE, 3)); - documents.insert( - URI_SECOND.to_string(), - demo_document(URI_SECOND, CLEAN_PUZZLE, 5), - ); - documents -} - fn sources_of(documents: &DocumentSet) -> BTreeMap { documents .iter() @@ -104,30 +91,6 @@ fn sources_of(documents: &DocumentSet) -> BTreeMap { .collect() } -fn rename_request( - documents: DocumentSet, - position_document_uri: &str, - position: Position, - new_name: &str, - sources: BTreeMap, -) -> ToolRequest { - ToolRequest::ProviderSemanticRename { - language_id: DEMO_LANGUAGE_ID.to_string(), - documents: Some(documents), - position_document_uri: position_document_uri.to_string(), - position, - new_name: new_name.to_string(), - project_root: Some("file:///project".to_string()), - sources: Some(sources), - } -} - -/// The position of the `double` op declaration (0-based line 4, char 6). -const DOUBLE_POSITION: Position = Position { - line: 4, - character: 6, -}; - fn handle(service: &mut ToolService<'_>, request: &ToolRequest) -> ProviderMutation { match service.handle(request) { ToolResponse::Ok { result } => { @@ -137,272 +100,6 @@ fn handle(service: &mut ToolService<'_>, request: &ToolRequest) -> ProviderMutat } } -#[test] -fn single_file_rename_routes_through_the_provider() { - let Some(_) = mock_provider_path() else { - return; - }; - let mut service = tool_service(&[]); - let documents = single_document_set(); - let mutation = handle( - &mut service, - &rename_request( - documents, - URI, - DOUBLE_POSITION, - "twice", - sources_of(&single_document_set()), - ), - ); - assert!(mutation.ok, "rename succeeds: {:?}", mutation.diagnostics); - - let transaction = mutation.transaction.expect("transaction"); - assert_eq!( - transaction.edits.len(), - 3, - "declaration plus both references" - ); - assert!( - transaction - .edits - .iter() - .all(|edit| edit.edit_kind == "rename") - ); - assert!( - transaction.edits.iter().all(|edit| { - edit.source == URI - && edit.source_identity == wright_driver::input_identity(CLEAN_PUZZLE) - }), - "every edit carries the source identity precondition" - ); - - let preview = mutation.preview.expect("preview"); - assert_eq!(preview.len(), 1); - assert_eq!(preview[0].source, URI); - assert!(preview[0].new_text.contains("twice: x => x * 2")); - assert!(preview[0].new_text.contains("solution = [ twice, twice ]")); - assert!( - !preview[0].new_text.contains("double"), - "every occurrence renamed" - ); -} - -#[test] -fn multi_file_rename_edits_every_received_document_consistently() { - let Some(_) = mock_provider_path() else { - return; - }; - let mut service = tool_service(&[]); - let documents = two_document_set(); - let mutation = handle( - &mut service, - &rename_request( - documents, - URI, - DOUBLE_POSITION, - "twice", - sources_of(&two_document_set()), - ), - ); - assert!(mutation.ok, "rename succeeds: {:?}", mutation.diagnostics); - - let transaction = mutation.transaction.expect("transaction"); - let per_source: BTreeMap<&str, usize> = - transaction - .edits - .iter() - .fold(BTreeMap::new(), |mut counts, edit| { - *counts.entry(edit.source.as_str()).or_default() += 1; - counts - }); - assert_eq!( - per_source.get(URI), - Some(&3), - "first document: declaration plus references" - ); - assert_eq!( - per_source.get(URI_SECOND), - Some(&3), - "second document: declaration plus references" - ); - - let preview = mutation.preview.expect("preview"); - assert_eq!(preview.len(), 2); - for source_preview in &preview { - assert!(source_preview.new_text.contains("twice: x => x * 2")); - assert!( - source_preview - .new_text - .contains("solution = [ twice, twice ]") - ); - } -} - -#[test] -fn rename_collision_is_a_structured_atomic_refusal() { - let Some(_) = mock_provider_path() else { - return; - }; - let mut service = tool_service(&[]); - let documents = single_document_set(); - let mutation = handle( - &mut service, - &rename_request( - documents, - URI, - DOUBLE_POSITION, - "plus1", // already declared: the provider refuses the collision - sources_of(&single_document_set()), - ), - ); - assert!(!mutation.ok); - // #570: a deliberate refusal is a request outcome (frontend), not a - // provider process failure — the refusalCode rides in the diagnostic - // code under the `provider-refusal-` namespace and verbatim in - // `provider_code`. - assert_eq!( - mutation.diagnostics[0].code, - "provider-refusal-rename.nameCollision" - ); - assert_eq!( - mutation.provider_code.as_deref(), - Some("rename.nameCollision"), - "the provider's machine-readable refusal code is preserved" - ); - assert!(mutation.transaction.is_none(), "no partial edit set"); - assert!(mutation.preview.is_none(), "no partial preview"); -} - -#[test] -fn invalid_new_name_is_a_structured_atomic_refusal() { - let Some(_) = mock_provider_path() else { - return; - }; - let mut service = tool_service(&[]); - let documents = single_document_set(); - let mutation = handle( - &mut service, - &rename_request( - documents, - URI, - DOUBLE_POSITION, - "not a name!", - sources_of(&single_document_set()), - ), - ); - assert!(!mutation.ok); - assert_eq!( - mutation.provider_code.as_deref(), - Some("rename.invalidName") - ); - assert!(mutation.transaction.is_none()); - assert!(mutation.preview.is_none()); -} - -#[test] -fn stale_current_sources_refuse_with_no_partial_edit_set() { - let Some(_) = mock_provider_path() else { - return; - }; - let mut service = tool_service(&[]); - let documents = single_document_set(); - let mut stale_sources = sources_of(&documents); - stale_sources.insert(URI.to_string(), format!("{CLEAN_PUZZLE}\n")); - let mutation = handle( - &mut service, - &rename_request(documents, URI, DOUBLE_POSITION, "twice", stale_sources), - ); - assert!(!mutation.ok); - assert_eq!(mutation.diagnostics[0].code, "edit-stale-source"); - assert!(mutation.transaction.is_none(), "no partial edit set"); - assert!(mutation.preview.is_none(), "no partial preview"); -} - -#[test] -fn missing_rename_capability_refuses_explicitly() { - let Some(_) = mock_provider_path() else { - return; - }; - let mut service = tool_service(&["rename"]); - let documents = single_document_set(); - let mutation = handle( - &mut service, - &rename_request( - documents, - URI, - DOUBLE_POSITION, - "twice", - sources_of(&single_document_set()), - ), - ); - assert!(!mutation.ok); - assert_eq!(mutation.diagnostics[0].code, "capability-unavailable"); - assert_eq!( - mutation.provider_code.as_deref(), - Some("capability-unavailable"), - "no silent fallback to textual search/replace" - ); - assert!(mutation.transaction.is_none()); - assert!(mutation.preview.is_none()); -} - -#[test] -fn provider_failure_mid_rename_applies_nothing() { - let Some(_) = mock_provider_path() else { - return; - }; - let mut service = tool_service(&["editValidation"]); - let documents = single_document_set(); - let mutation = handle( - &mut service, - &rename_request( - documents, - URI, - DOUBLE_POSITION, - "twice", - sources_of(&single_document_set()), - ), - ); - assert!(!mutation.ok); - assert_eq!(mutation.diagnostics[0].code, "capability-unavailable"); - assert_eq!( - mutation.provider_code.as_deref(), - Some("capability-unavailable"), - "the missing validation capability refuses after edit generation" - ); - assert!(mutation.transaction.is_none(), "no partial edit set"); - assert!(mutation.preview.is_none(), "no partial preview"); -} - -#[test] -fn semantic_validation_failure_is_atomic_across_documents() { - let Some(_) = mock_provider_path() else { - return; - }; - let mut service = tool_service(&[]); - // Document 1 declares only `double`; document 2 declares `double` and - // `plus1`. Renaming `double` to `plus1` in document 1 passes the - // provider's rename-time collision check (which is scoped to the - // position document) but creates a duplicate `plus1` op in document 2. - // The provider's project-aware semantic validation of the edit set - // catches it: the whole mutation refuses with no partial application. - let mut documents = DocumentSet::new(); - documents.insert(URI.to_string(), demo_document(URI, SINGLE_OP_PUZZLE, 3)); - documents.insert( - URI_SECOND.to_string(), - demo_document(URI_SECOND, CLEAN_PUZZLE, 5), - ); - let sources = sources_of(&documents); - let mutation = handle( - &mut service, - &rename_request(documents, URI, DOUBLE_POSITION, "plus1", sources), - ); - assert!(!mutation.ok); - assert_eq!(mutation.diagnostics[0].code, "provider-validation-failed"); - assert!(mutation.transaction.is_none(), "no partial edit set"); - assert!(mutation.preview.is_none(), "no partial preview"); -} - #[test] fn unconfigured_language_id_refuses_explicitly() { let mut session = CompilerSession::new(SessionConfig { @@ -554,21 +251,6 @@ fn provider_mutations_do_not_require_the_session_project() { let mut service = ToolService::new(session).expect("service starts without a snapshot"); assert!(service.loaded().is_none()); - let rename = handle( - &mut service, - &rename_request( - single_document_set(), - URI, - DOUBLE_POSITION, - "twice", - sources_of(&single_document_set()), - ), - ); - assert!( - rename.ok, - "the provider rename succeeds over its own documents: {:?}", - rename.diagnostics - ); let validate = handle( &mut service, &ToolRequest::ProviderValidateEdit { @@ -589,10 +271,9 @@ fn provider_mutations_do_not_require_the_session_project() { } /// An LPP fake provider that accepts the mutation surface unconditionally: -/// `lpp/rename` answers one five-character edit on the first document, -/// `lpp/validateEdits` accepts every edit, `lpp/check` is clean. It records -/// every document URI it is shown so the test can prove which set the -/// session supplied. +/// `lpp/validateEdits` accepts every edit and `lpp/check` reports a clean +/// project. It records every document URI the project-level `lpp/check` is +/// shown so the test can prove which set the session supplied. #[cfg(unix)] const SESSION_DOCS_PROVIDER: &str = r#"#!/usr/bin/env python3 import json, sys @@ -614,21 +295,14 @@ for line in sys.stdin: "symbols": False, "definition": False, "references": False, "rename": True, "editValidation": True, "projectLoading": False, "lookup": False}}) - elif method == "lpp/rename": - documents = request["params"]["documents"] - with open(CAPTURE, "a") as capture: - capture.write("\n".join(sorted(documents)) + "\n") - uri = sorted(documents)[0] - reply(id, {"edits": [{"documentUri": uri, - "version": documents[uri]["version"], - "textEdits": [{"range": {"start": {"line": 0, "character": 0}, - "end": {"line": 0, "character": 5}}, - "newText": "RENAMED"}]}]}) elif method == "lpp/validateEdits": reply(id, {"valid": True, "version": request["params"]["document"]["version"]}) elif method == "lpp/check": + documents = request["params"]["documents"] + with open(CAPTURE, "a") as capture: + capture.write("\n".join(sorted(documents)) + "\n") reply(id, {"documents": [{"uri": uri, "version": doc["version"], "diagnostics": []} - for uri, doc in request["params"]["documents"].items()]}) + for uri, doc in documents.items()]}) else: reply(id, {}) "#; @@ -703,34 +377,35 @@ fn an_omitted_document_set_derives_from_the_loaded_project() { } = session_docs_service(); let member_uri = member_uri(); let member_text = std::fs::read_to_string(workshop_path()).unwrap(); + let transaction = EditTransaction::new(vec![SourceEdit { + edit_kind: "edit".to_string(), + source: member_uri.clone(), + source_identity: wright_driver::input_identity(&member_text), + range: EditRange { + start_line: 1, + start_col: 1, + end_line: 1, + end_col: 6, + }, + new_text: "RENAMED".to_string(), + }]) + .expect("transaction"); let mutation = handle( &mut service, - &ToolRequest::ProviderSemanticRename { + &ToolRequest::ProviderValidateEdit { language_id: DEMO_LANGUAGE_ID.to_string(), documents: None, - position_document_uri: member_uri.clone(), - position: Position { - line: 0, - character: 0, - }, - new_name: "renamed".to_string(), - project_root: None, + transaction, sources: None, + project_root: None, }, ); assert!( mutation.ok, - "the session-derived rename succeeds: {:?}", + "the session-derived validation succeeds: {:?}", mutation.diagnostics ); - let transaction = mutation.transaction.expect("transaction"); - assert_eq!(transaction.edits[0].source, member_uri); - assert_eq!( - transaction.edits[0].source_identity, - wright_driver::input_identity(&member_text), - "`sources` defaulted to the member's disk text" - ); let preview = mutation.preview.expect("preview"); assert!( preview[0].new_text.starts_with("RENAMED"), @@ -788,513 +463,41 @@ fn an_omitted_document_set_validates_a_session_member_edit() { } /// `sources` alone may be omitted (#548): it defaults to each supplied -/// document's text. +/// document's text — the transaction's identity precondition verifies +/// against that text. #[cfg(unix)] #[test] fn omitted_sources_default_to_each_documents_text() { let SessionDocs { dir, mut service, .. } = session_docs_service(); + let transaction = EditTransaction::new(vec![SourceEdit { + edit_kind: "edit".to_string(), + source: URI.to_string(), + source_identity: wright_driver::input_identity(CLEAN_PUZZLE), + range: EditRange { + start_line: 1, + start_col: 1, + end_line: 1, + end_col: 6, + }, + new_text: "RENAMED".to_string(), + }]) + .expect("transaction"); let mutation = handle( &mut service, - &ToolRequest::ProviderSemanticRename { + &ToolRequest::ProviderValidateEdit { language_id: DEMO_LANGUAGE_ID.to_string(), documents: Some(single_document_set()), - position_document_uri: URI.to_string(), - position: DOUBLE_POSITION, - new_name: "renamed".to_string(), - project_root: None, + transaction, sources: None, + project_root: None, }, ); assert!( mutation.ok, - "the rename succeeds with defaulted sources: {:?}", + "the validation succeeds with defaulted sources: {:?}", mutation.diagnostics ); - let transaction = mutation.transaction.expect("transaction"); - assert_eq!( - transaction.edits[0].source_identity, - wright_driver::input_identity(CLEAN_PUZZLE), - "`sources` defaulted to the document text" - ); let _ = std::fs::remove_dir_all(&dir); } - -/// #739: `wright rename` on `.opy` input is the provider-owned rename the -/// agent surface exposes, reached through the CLI's `NAME` addressing. -/// This staged `opy` provider serves a canned compile artifact (the -/// session's project load) and a scripted `lpp/symbols` listing plus an -/// ordered queue of `lpp/rename` outcomes, so each test controls which -/// declaration sites the name resolves to and what each site answers. -/// `lpp/validateEdits` accepts and `lpp/check` reports a clean project; -/// every `lpp/rename` call site is recorded to `rename-calls.jsonl`. -#[cfg(unix)] -const OPY_RENAME_PROVIDER: &str = r#"#!/usr/bin/env python3 -import json, os, sys - -HERE = os.path.dirname(os.path.abspath(__file__)) - -def read(name): - with open(os.path.join(HERE, name)) as handle: - return json.load(handle) - -def reply(id, result=None, error=None): - message = {"jsonrpc": "2.0", "id": id} - message.update({"error": error} if error is not None else {"result": result}) - print(json.dumps(message), flush=True) - -def refusal(id, code, text): - reply(id, error={"code": -32000, "message": text, - "data": {"lpp": {"kind": "refusal", "details": {"refusalCode": code}}}}) - -for line in sys.stdin: - request = json.loads(line) - id, method = request["id"], request["method"] - if method == "lpp/initialize": - reply(id, {"protocolVersion": request["params"]["protocolVersion"], - "serverInfo": {"name": "fake-opy", "version": "0"}, - "languages": [{"id": "opy", "extensions": ["opy"]}], - "capabilities": {"check": True, "compile": True, "reconstruct": False, - "symbols": True, "definition": False, "references": False, - "rename": True, "editValidation": True, - "projectLoading": True, "lookup": False}}) - elif method == "lpp/compile": - reply(id, {"diagnostics": [], "artifact": read("artifact.json")}) - elif method == "lpp/symbols": - reply(id, read("symbols.json")) - elif method == "lpp/rename": - params = request["params"] - with open(os.path.join(HERE, "rename-calls.jsonl"), "a") as calls: - calls.write(json.dumps({"uri": params["positionDocumentUri"], - "position": params["position"], - "newName": params["newName"]}) + "\n") - queue = read("renames.json") - state_file = os.path.join(HERE, "rename-state") - index = int(open(state_file).read().strip()) if os.path.exists(state_file) else 0 - with open(state_file, "w") as state: - state.write(str(index + 1)) - outcome = queue[index] if index < len(queue) else queue[-1] - if "refusalCode" in outcome: - refusal(id, outcome["refusalCode"], outcome.get("message", "the provider declined")) - else: - reply(id, outcome["result"]) - elif method == "lpp/validateEdits": - reply(id, {"valid": True, "version": request["params"]["document"]["version"]}) - elif method == "lpp/check": - reply(id, {"documents": [{"uri": uri, "version": doc["version"], "diagnostics": []} - for uri, doc in request["params"]["documents"].items()]}) - else: - reply(id, {}) -"#; - -/// An OPY project staged on disk next to its scripted provider: the member -/// files `lpp/symbols`/`lpp/rename` report on, the canned compile artifact, -/// and the session wired to the provider under the `opy` language id. -#[cfg(unix)] -struct FakeOpyProject { - dir: PathBuf, - entry: PathBuf, - calls: PathBuf, - session: CompilerSession, -} - -/// The document URI `provider_document_set` keys a member under — the -/// `file://` spelling of its disk path. -#[cfg(unix)] -fn member_file_uri(dir: &Path, name: &str) -> String { - url::Url::from_file_path(dir.join(name)) - .expect("absolute member path") - .to_string() -} - -/// The `lpp/compile` artifact the load handshake consumes: `None` maps an -/// unmapped `workshop-rs/text-v1` answer (the entry file is the only -/// member), `Some(members)` a `workshop-rs/mapped-text-v1` answer whose -/// file table lists every member URI — the shape `#!include` projects -/// load under. -#[cfg(unix)] -fn compile_artifact(member_uris: Option>) -> serde_json::Value { - let text = std::fs::read_to_string(workshop_path()).expect("workshop fixture"); - match member_uris { - None => serde_json::json!({"format": "workshop-rs/text-v1", "content": text}), - Some(files) => { - let catalog = workshop_rs::catalog::Catalog::builtin_ref().expect("catalog"); - let locale = workshop_rs::catalog::Locale::new("en-US"); - let program = workshop_rs::parser::parse_with_context(&text, catalog, &locale, catalog) - .expect("workshop fixture parses"); - let map = workshop_rs::program::SourceMap::extract(&program); - let mut artifact: serde_json::Value = - serde_json::from_str(&workshop_rs::program::MappedText::new(&text, map).to_json()) - .expect("mapped text serializes"); - artifact["files"] = serde_json::json!( - files - .iter() - .map(|uri| { serde_json::json!({ "path": uri }) }) - .collect::>() - ); - artifact["spans"] = serde_json::json!([]); - serde_json::json!({"format": "workshop-rs/mapped-text-v1", "content": artifact.to_string()}) - } - } -} - -/// One `lpp/symbols` listing entry: a declaration `symbol` for `name` at -/// the zero-based `line`/`character` in `uri`'s document. -#[cfg(unix)] -fn symbol_doc(uri: &str, name: &str, kind: &str, line: u32, character: u32) -> serde_json::Value { - serde_json::json!({ - "uri": uri, - "version": 0, - "symbols": [{ - "name": name, - "kind": kind, - "range": {"start": {"line": line, "character": character}, - "end": {"line": line, "character": character + name.len() as u32}} - }] - }) -} - -/// `lpp/rename` edits: `(uri, line, character)` spans replaced with -/// `new_name` — every span covers `name`'s length on its own line. -#[cfg(unix)] -fn rename_edits(name: &str, new_name: &str, sites: &[(&str, u32, u32)]) -> serde_json::Value { - let mut per_document: BTreeMap<&str, Vec> = BTreeMap::new(); - for (uri, line, character) in sites { - per_document - .entry(uri) - .or_default() - .push(serde_json::json!({ - "range": {"start": {"line": line, "character": character}, - "end": {"line": line, "character": character + name.len() as u32}}, - "newText": new_name - })); - } - serde_json::json!({ - "edits": per_document.iter().map(|(uri, edits)| serde_json::json!({ - "documentUri": uri, - "version": 0, - "textEdits": edits - })).collect::>() - }) -} - -#[cfg(unix)] -impl FakeOpyProject { - /// Stage one scripted answer file the provider serves (`artifact.json`, - /// `symbols.json`, `renames.json`). The provider reads them at request - /// time, so tests stage them once the staged member paths — and their - /// `file://` URIs — exist. - fn stage(&self, name: &str, value: serde_json::Value) { - std::fs::write(self.dir.join(name), value.to_string()).unwrap(); - } - - fn member_file_uri(&self, name: &str) -> String { - member_file_uri(&self.dir, name) - } - - fn member_path(&self, name: &str) -> PathBuf { - self.dir.join(name) - } -} - -#[cfg(unix)] -fn fake_opy_project(members: &[(&str, &str)]) -> FakeOpyProject { - use std::os::unix::fs::PermissionsExt; - use std::sync::atomic::{AtomicUsize, Ordering}; - static NEXT: AtomicUsize = AtomicUsize::new(0); - let dir = std::env::temp_dir().join(format!( - "wright-opy-rename-739-{}-{}", - std::process::id(), - NEXT.fetch_add(1, Ordering::Relaxed) - )); - std::fs::create_dir_all(&dir).unwrap(); - for (name, text) in members { - std::fs::write(dir.join(name), text).unwrap(); - } - let provider_path = dir.join("fake-opy-provider"); - std::fs::write(&provider_path, OPY_RENAME_PROVIDER).unwrap(); - std::fs::set_permissions(&provider_path, std::fs::Permissions::from_mode(0o755)).unwrap(); - - let mut registry = wright_lpp::ProviderRegistry::new(); - registry - .register(wright_lpp::ProviderConfig::new( - wright_driver::opy_provider::OPY_LANGUAGE_ID, - provider_path, - Vec::new(), - )) - .expect("registered"); - let session = CompilerSession::new(SessionConfig { - input: InputSpec::Path(dir.join("main.opy")), - kind: SourceKind::Opy, - source_backend: wright_driver::source_provider::SourceBackend::Provider, - providers: registry, - ..SessionConfig::default() - }) - .expect("session"); - FakeOpyProject { - calls: dir.join("rename-calls.jsonl"), - entry: dir.join("main.opy"), - dir, - session, - } -} - -/// The `lpp/rename` call sites the provider saw, `(uri, line, character)`. -#[cfg(unix)] -fn rename_calls(calls: &Path) -> Vec<(String, u64, u64)> { - std::fs::read_to_string(calls) - .unwrap_or_default() - .lines() - .map(|line| { - let call: serde_json::Value = serde_json::from_str(line).unwrap(); - ( - call["uri"].as_str().unwrap().to_string(), - call["position"]["line"].as_u64().unwrap(), - call["position"]["character"].as_u64().unwrap(), - ) - }) - .collect() -} - -/// `helper` is declared at 0-based line 0 char 4 and called at line 4 -/// char 4 of `main.opy`; `util.opy` calls it at line 1 char 4. -#[cfg(unix)] -const MAIN_OPY: &str = "def helper():\n pass\n\nrule \"setup\":\n helper()\n"; - -#[cfg(unix)] -const UTIL_OPY: &str = "def util():\n helper()\n"; - -#[cfg(unix)] -#[test] -fn opy_rename_resolves_the_name_to_its_declaration_site_and_previews() { - let mut project = fake_opy_project(&[("main.opy", MAIN_OPY)]); - let uri = project.member_file_uri("main.opy"); - project.stage("artifact.json", compile_artifact(None)); - project.stage( - "symbols.json", - serde_json::json!({"documents": [symbol_doc(&uri, "helper", "def", 0, 4)]}), - ); - project.stage( - "renames.json", - serde_json::json!([{"result": rename_edits("helper", "assist", &[(&uri, 0, 4), (&uri, 4, 4)])}]), - ); - let rename = project.session.rename("helper", "assist", false); - assert!(rename.ok, "{:?}", rename.diagnostics); - assert_eq!( - rename_calls(&project.calls), - vec![(uri.clone(), 0, 4)], - "NAME resolved through `lpp/symbols` to the declaration position" - ); - let transaction = rename.result.transaction.expect("transaction"); - assert_eq!(transaction.edits.len(), 2); - assert!(transaction.edits.iter().all(|edit| edit.source == uri)); - let preview = rename.result.preview.expect("preview"); - assert_eq!(preview[0].source, project.entry.display().to_string()); - assert!(preview[0].new_text.contains("def assist():")); - assert!(preview[0].new_text.contains(" assist()")); - assert_eq!( - std::fs::read_to_string(&project.entry).unwrap(), - MAIN_OPY, - "a preview run writes nothing" - ); - let _ = std::fs::remove_dir_all(&project.dir); -} - -/// #739: a multi-member project — the shape a `#!include` graph or a -/// directory input loads under a mapped artifact — renames across every -/// member the provider edited; `--write` applies the previews atomically. -#[cfg(unix)] -#[test] -fn opy_rename_writes_every_member_the_provider_edited() { - let mut project = fake_opy_project(&[("main.opy", MAIN_OPY), ("util.opy", UTIL_OPY)]); - let main_uri = project.member_file_uri("main.opy"); - let util_uri = project.member_file_uri("util.opy"); - project.stage( - "artifact.json", - compile_artifact(Some(vec![main_uri.clone(), util_uri.clone()])), - ); - project.stage( - "symbols.json", - serde_json::json!({"documents": [ - symbol_doc(&main_uri, "helper", "def", 0, 4), - {"uri": util_uri, "version": 0, "symbols": [{"name": "util", "kind": "def", - "range": {"start": {"line": 0, "character": 4}, - "end": {"line": 0, "character": 8}}}]} - ]}), - ); - project.stage( - "renames.json", - serde_json::json!([{"result": rename_edits("helper", "assist", &[ - (&main_uri, 0, 4), - (&main_uri, 4, 4), - (&util_uri, 1, 4), - ])}]), - ); - let rename = project.session.rename("helper", "assist", true); - assert!(rename.ok, "{:?}", rename.diagnostics); - assert_eq!(rename_calls(&project.calls), vec![(main_uri.clone(), 0, 4)]); - let transaction = rename.result.transaction.expect("transaction"); - let per_source: BTreeMap<&str, usize> = - transaction - .edits - .iter() - .fold(BTreeMap::new(), |mut counts, edit| { - *counts.entry(edit.source.as_str()).or_default() += 1; - counts - }); - assert_eq!(per_source.get(main_uri.as_str()), Some(&2)); - assert_eq!(per_source.get(util_uri.as_str()), Some(&1)); - let mut written = rename.result.written.clone(); - written.sort(); - assert_eq!( - written, - vec![ - project.member_path("main.opy").display().to_string(), - project.member_path("util.opy").display().to_string(), - ], - "--write reports disk paths, not provider URIs" - ); - assert_eq!( - std::fs::read_to_string(project.member_path("main.opy")).unwrap(), - "def assist():\n pass\n\nrule \"setup\":\n assist()\n" - ); - assert_eq!( - std::fs::read_to_string(project.member_path("util.opy")).unwrap(), - "def util():\n assist()\n" - ); - let _ = std::fs::remove_dir_all(&project.dir); -} - -/// #739: a name the loaded project does not declare refuses -/// `unknown-symbol` without ever calling `lpp/rename`. -#[cfg(unix)] -#[test] -fn opy_rename_unknown_name_never_reaches_the_provider_rename() { - let mut project = fake_opy_project(&[("main.opy", MAIN_OPY)]); - let uri = project.member_file_uri("main.opy"); - project.stage("artifact.json", compile_artifact(None)); - project.stage( - "symbols.json", - serde_json::json!({"documents": [symbol_doc(&uri, "helper", "def", 0, 4)]}), - ); - project.stage("renames.json", serde_json::json!([])); - let rename = project.session.rename("missing", "assist", false); - assert!(!rename.ok); - assert_eq!(rename.diagnostics[0].code, "unknown-symbol"); - assert!( - !project.calls.exists(), - "no `lpp/rename` is sent for an undeclared name" - ); - assert!(rename.result.transaction.is_none()); - assert!(rename.result.preview.is_none()); - let _ = std::fs::remove_dir_all(&project.dir); -} - -/// #739: two declaration sites spelling the name that `lpp/rename` answers -/// identically — a `subroutine` declaration and its `def` — are one -/// symbol; divergent answers are a real `ambiguous-symbol` refusal naming -/// each candidate's `path:line:col`. -#[cfg(unix)] -#[test] -fn opy_rename_probes_multi_site_names_and_refuses_only_real_ambiguity() { - // Same answer from both sites: one semantic symbol, one rename. - let mut project = fake_opy_project(&[("main.opy", MAIN_OPY)]); - let uri = project.member_file_uri("main.opy"); - project.stage("artifact.json", compile_artifact(None)); - project.stage( - "symbols.json", - serde_json::json!({"documents": [{ - "uri": uri, "version": 0, - "symbols": [ - {"name": "helper", "kind": "subroutine", - "range": {"start": {"line": 0, "character": 4}, - "end": {"line": 0, "character": 10}}}, - {"name": "helper", "kind": "def", - "range": {"start": {"line": 4, "character": 4}, - "end": {"line": 4, "character": 10}}} - ]}]}), - ); - let edits = rename_edits("helper", "assist", &[(&uri, 0, 4), (&uri, 4, 4)]); - project.stage( - "renames.json", - serde_json::json!([{"result": edits}, {"result": edits}]), - ); - let rename = project.session.rename("helper", "assist", false); - assert!(rename.ok, "{:?}", rename.diagnostics); - assert_eq!(rename_calls(&project.calls).len(), 2); - let _ = std::fs::remove_dir_all(&project.dir); - - // Divergent answers: two distinct symbols share the name. - let mut project = fake_opy_project(&[("main.opy", MAIN_OPY), ("util.opy", UTIL_OPY)]); - let main_uri = project.member_file_uri("main.opy"); - let util_uri = project.member_file_uri("util.opy"); - project.stage( - "artifact.json", - compile_artifact(Some(vec![main_uri.clone(), util_uri.clone()])), - ); - project.stage( - "symbols.json", - serde_json::json!({"documents": [ - symbol_doc(&main_uri, "helper", "def", 0, 4), - symbol_doc(&util_uri, "helper", "def", 0, 4) - ]}), - ); - project.stage( - "renames.json", - serde_json::json!([ - {"result": rename_edits("helper", "assist", &[(&main_uri, 0, 4)])}, - {"result": rename_edits("helper", "assist", &[(&util_uri, 0, 4)])} - ]), - ); - let rename = project.session.rename("helper", "assist", true); - assert!(!rename.ok); - assert_eq!(rename.diagnostics[0].code, "ambiguous-symbol"); - let message = &rename.diagnostics[0].message; - for name in ["main.opy", "util.opy"] { - let path = project.member_path(name).display().to_string(); - assert!(message.contains(&path), "{message}"); - } - assert!(rename.result.transaction.is_none()); - assert!(rename.result.preview.is_none()); - assert_eq!( - std::fs::read_to_string(project.member_path("main.opy")).unwrap(), - MAIN_OPY, - "an ambiguous rename writes nothing" - ); - let _ = std::fs::remove_dir_all(&project.dir); -} - -/// #739: the provider's own refusal — here `rename.invalidName` — keeps -/// its machine-readable code under the `provider-refusal-` namespace and -/// previews/writes nothing. -#[cfg(unix)] -#[test] -fn opy_rename_passes_provider_refusals_through() { - let mut project = fake_opy_project(&[("main.opy", MAIN_OPY)]); - let uri = project.member_file_uri("main.opy"); - project.stage("artifact.json", compile_artifact(None)); - project.stage( - "symbols.json", - serde_json::json!({"documents": [symbol_doc(&uri, "helper", "def", 0, 4)]}), - ); - project.stage( - "renames.json", - serde_json::json!([{"refusalCode": "rename.invalidName", - "message": "'not a name' is not a valid identifier"}]), - ); - let rename = project.session.rename("helper", "not a name", true); - assert!(!rename.ok); - assert_eq!( - rename.diagnostics[0].code, - "provider-refusal-rename.invalidName" - ); - assert!(rename.result.transaction.is_none()); - assert!(rename.result.preview.is_none()); - assert_eq!( - std::fs::read_to_string(&project.entry).unwrap(), - MAIN_OPY, - "a refused rename writes nothing" - ); - let _ = std::fs::remove_dir_all(&project.dir); -} diff --git a/crates/wright-driver/tests/service.rs b/crates/wright-driver/tests/service.rs index aa4e946c..0e5bc58e 100644 --- a/crates/wright-driver/tests/service.rs +++ b/crates/wright-driver/tests/service.rs @@ -1,7 +1,7 @@ use std::collections::BTreeMap; use std::path::{Path, PathBuf}; -use wright_driver::edit::{EditRange, EditTransaction, RenameTarget, SourceEdit}; +use wright_driver::edit::{EditRange, EditTransaction, SourceEdit}; use wright_driver::service::{Address, ToolRequest, ToolResponse, ToolService}; use wright_driver::{ CompilerSession, FindingSelection, InputSpec, SessionConfig, Severity, SourceKind, @@ -2419,16 +2419,6 @@ fn stale_numeric_ids_refuse_until_the_new_space_is_observed() { all: true, }, ToolRequest::Usage { symbol: 0.into() }, - ToolRequest::SemanticRename { - sources: Some(std::collections::BTreeMap::new()), - target: wright_driver::edit::RenameTarget { - symbol: Some(0.into()), - source: None, - line: None, - col: None, - to: "renamed".to_string(), - }, - }, ToolRequest::Cfg { rule: 0.into(), kind: None, @@ -2991,45 +2981,16 @@ fn every_operation_is_fresh_after_a_reload_or_refuses_stale_ids() { }, ); assert_eq!(validate["ok"], true, "{validate}"); - let rename = result_of( - &mut service, - &ToolRequest::SemanticRename { - sources: Some(sources), - target: wright_driver::edit::RenameTarget { - symbol: Some("points".into()), - source: None, - line: None, - col: None, - to: "score".to_string(), - }, - }, - ); - assert_eq!(rename["ok"], true, "{rename}"); - for request in [ - ToolRequest::ProviderSemanticRename { - language_id: "not-a-language".to_string(), - documents: Some(Default::default()), - position_document_uri: String::new(), - position: wright_lpp::Position { - line: 0, - character: 0, - }, - new_name: String::new(), - project_root: None, - sources: None, - }, - ToolRequest::ProviderValidateEdit { - language_id: "not-a-language".to_string(), - documents: Some(Default::default()), - transaction: wright_driver::edit::EditTransaction { edits: vec![] }, - sources: None, - project_root: None, - }, - ] { - let result = result_of(&mut service, &request); - assert_eq!(result["ok"], false, "{request:?}: {result}"); - } + let request = ToolRequest::ProviderValidateEdit { + language_id: "not-a-language".to_string(), + documents: Some(Default::default()), + transaction: wright_driver::edit::EditTransaction { edits: vec![] }, + sources: None, + project_root: None, + }; + let result = result_of(&mut service, &request); + assert_eq!(result["ok"], false, "{request:?}: {result}"); let _ = std::fs::remove_dir_all(&dir); } @@ -3440,17 +3401,7 @@ fn a_failed_refresh_invalidates_the_snapshot_even_when_bytes_return() { const RENAMEABLE: &str = "variables {\n global:\n 0: score\n}\n\nrule (\"r\") {\n event {\n Ongoing - Global;\n }\n actions {\n Set Global Variable(score, Add(Global.score, 1));\n }\n}\n"; -fn rename_target() -> RenameTarget { - RenameTarget { - symbol: Some(Address::Name("score".to_string())), - source: None, - line: None, - col: None, - to: "total".to_string(), - } -} - -fn rename_edit_transaction(key: &str, source_identity: String) -> EditTransaction { +fn score_edit_transaction(key: &str, source_identity: String) -> EditTransaction { EditTransaction::new(vec![SourceEdit { edit_kind: "edit".to_string(), source: key.to_string(), @@ -3479,28 +3430,9 @@ fn edit_operations_default_sources_to_the_on_disk_text() { .unwrap(); let mut service = ToolService::new(&mut session).unwrap(); - let rename = |sources| ToolRequest::SemanticRename { - sources, - target: rename_target(), - }; - let defaulted = result_of(&mut service, &rename(None)); - let explicit = result_of( - &mut service, - &rename(Some(BTreeMap::from([(key.clone(), on_disk.clone())]))), - ); - assert_eq!(defaulted["ok"], true, "{defaulted:?}"); - assert_eq!(defaulted, explicit); - assert!( - defaulted["preview"][0]["new_text"] - .as_str() - .unwrap() - .contains("0: total"), - "{defaulted:?}" - ); - let validate = |sources| ToolRequest::ValidateEdit { sources, - transaction: rename_edit_transaction(&key, wright_driver::input_identity(&on_disk)), + transaction: score_edit_transaction(&key, wright_driver::input_identity(&on_disk)), }; let defaulted = result_of(&mut service, &validate(None)); let explicit = result_of( @@ -3520,10 +3452,10 @@ fn edit_operations_default_sources_to_the_on_disk_text() { } #[test] -fn rename_accepts_percent_encoded_file_uri_sources() { - // #636: a `sources` key spelled `file:///…/my%20dir/main.ws` binds the - // member whose path contains a space — the rename succeeds exactly as - // with the plain spelling. +fn edits_accept_percent_encoded_file_uri_sources() { + // #636: an edit `source` spelled `file:///…/my%20dir/main.ws` binds the + // member whose path contains a space — the validation succeeds exactly + // as with the plain spelling. use std::sync::atomic::{AtomicUsize, Ordering}; static COUNTER: AtomicUsize = AtomicUsize::new(0); let dir = std::env::temp_dir().join(format!( @@ -3545,22 +3477,23 @@ fn rename_accepts_percent_encoded_file_uri_sources() { let mut service = ToolService::new(&mut session).unwrap(); let uri = format!("file://{}", path.display().to_string().replace(' ', "%20")); + let identity = wright_driver::input_identity(&on_disk); let encoded = result_of( &mut service, - &ToolRequest::SemanticRename { - sources: Some(BTreeMap::from([(uri, on_disk.clone())])), - target: rename_target(), + &ToolRequest::ValidateEdit { + sources: Some(BTreeMap::from([(uri.clone(), on_disk.clone())])), + transaction: score_edit_transaction(&uri, identity.clone()), }, ); assert_eq!(encoded["ok"], true, "{encoded:?}"); let plain = result_of( &mut service, - &ToolRequest::SemanticRename { + &ToolRequest::ValidateEdit { sources: Some(BTreeMap::from([( path.to_string_lossy().into_owned(), on_disk, )])), - target: rename_target(), + transaction: score_edit_transaction(&path.to_string_lossy(), identity), }, ); assert_eq!(plain["ok"], true, "{plain:?}"); @@ -3581,45 +3514,23 @@ fn edit_operations_keep_supplied_source_and_project_boundaries() { .unwrap(); let mut service = ToolService::new(&mut session).unwrap(); - for (op, result) in [ - ( - "semanticRename", - result_of( - &mut service, - &ToolRequest::SemanticRename { - sources: Some(BTreeMap::from([( - key.clone(), - on_disk.replacen("score", "other", 1), - )])), - target: rename_target(), - }, - ), - ), - ( - "validateEditTransaction", - result_of( - &mut service, - &ToolRequest::ValidateEdit { - sources: Some(BTreeMap::from([(key.clone(), "changed".to_string())])), - transaction: rename_edit_transaction( - &key, - wright_driver::input_identity(&on_disk), - ), - }, - ), - ), - ] { - assert_eq!(result["ok"], false, "{op}: {result:?}"); - assert_eq!( - result["diagnostics"][0]["code"], "edit-stale-source", - "{op}: {result:?}" - ); - assert!(result["preview"].is_null(), "{op}: no partial preview"); - } + let result = result_of( + &mut service, + &ToolRequest::ValidateEdit { + sources: Some(BTreeMap::from([(key.clone(), "changed".to_string())])), + transaction: score_edit_transaction(&key, wright_driver::input_identity(&on_disk)), + }, + ); + assert_eq!(result["ok"], false, "{result:?}"); + assert_eq!( + result["diagnostics"][0]["code"], "edit-stale-source", + "{result:?}" + ); + assert!(result["preview"].is_null(), "no partial preview"); let foreign = |sources| ToolRequest::ValidateEdit { sources, - transaction: rename_edit_transaction("other.ws", wright_driver::input_identity(&on_disk)), + transaction: score_edit_transaction("other.ws", wright_driver::input_identity(&on_disk)), }; for (op, result) in [ ("absent sources", result_of(&mut service, &foreign(None))), @@ -3643,29 +3554,17 @@ fn edit_operations_keep_supplied_source_and_project_boundaries() { // The default reads disk, not a stale snapshot: rewriting the input is // observed by the service's freshness check (#471), so the unsourced - // rename resolves against the reloaded program — the new `other` - // symbol renames cleanly while the old `score` name is gone. + // validation resolves against the reloaded program. std::fs::write(&path, on_disk.replace("score", "other")).unwrap(); - let renamed = result_of( - &mut service, - &ToolRequest::SemanticRename { - sources: None, - target: RenameTarget { - symbol: Some(Address::Name("other".to_string())), - ..rename_target() - }, - }, - ); - assert_eq!(renamed["ok"], true, "{renamed:?}"); - let stale = result_of( + let reloaded = std::fs::read_to_string(&path).unwrap(); + let validation = result_of( &mut service, - &ToolRequest::SemanticRename { + &ToolRequest::ValidateEdit { sources: None, - target: rename_target(), + transaction: score_edit_transaction(&key, wright_driver::input_identity(&reloaded)), }, ); - assert_eq!(stale["ok"], false, "{stale:?}"); - assert_eq!(stale["diagnostics"][0]["code"], "unknown-symbol"); + assert_eq!(validation["ok"], true, "{validation:?}"); let _ = std::fs::remove_dir_all(path.parent().unwrap()); } @@ -4103,11 +4002,10 @@ fn a_fold_that_would_be_non_finite_keeps_the_off_profile_verdict() { } /// #639: the `Or(True, X)`/`And(False, X)` short-circuit folds discard X — -/// an identifier-bearing operand must survive, so `references` and -/// `semanticRename` for `g` return the same result under `compat` as under -/// `off`. +/// an identifier-bearing operand must survive, so `references` for `g` +/// returns the same result under `compat` as under `off`. #[test] -fn short_circuit_folds_preserve_occurrences_for_references_and_rename() { +fn short_circuit_folds_preserve_occurrences_for_references() { let path = temp_workshop( "or-fold.ws", "variables\n{\n global:\n 0: g\n}\n\nrule (\"r\")\n{\n conditions\n {\n Or(True, Global.g) == True;\n }\n actions\n {\n Wait(1, Abort When False);\n }\n}\n", @@ -4121,7 +4019,7 @@ fn short_circuit_folds_preserve_occurrences_for_references_and_rename() { }) .expect("session creates"); let mut service = ToolService::new(&mut session).expect("service"); - let references = result_of( + result_of( &mut service, &ToolRequest::References { symbol: "g".into(), @@ -4131,33 +4029,15 @@ fn short_circuit_folds_preserve_occurrences_for_references_and_rename() { max: None, all: true, }, - ); - let rename = result_of( - &mut service, - &ToolRequest::SemanticRename { - sources: None, - target: RenameTarget { - symbol: Some(Address::Name("g".to_string())), - source: None, - line: None, - col: None, - to: "renamed".to_string(), - }, - }, - ); - (references, rename) + ) }; - let (off_references, off_rename) = query(wright_driver::Profile::Off); - let (compat_references, compat_rename) = query(wright_driver::Profile::Compat); + let off_references = query(wright_driver::Profile::Off); + let compat_references = query(wright_driver::Profile::Compat); assert_eq!( compat_references, off_references, "references see the same occurrences under both profiles" ); - assert_eq!( - compat_rename, off_rename, - "semanticRename resolves the same occurrences under both profiles" - ); assert!( off_references["references"] .as_array() @@ -4166,11 +4046,6 @@ fn short_circuit_folds_preserve_occurrences_for_references_and_rename() { .any(|reference| reference["kind"] == "read"), "the Or-folded operand keeps its authored read: {off_references:?}" ); - assert_eq!( - off_rename["ok"], - serde_json::Value::Bool(true), - "rename validates: {off_rename:?}" - ); let _ = std::fs::remove_dir_all(path.parent().unwrap()); } diff --git a/crates/wright-language/src/service.rs b/crates/wright-language/src/service.rs index 8219a84e..557b8b4a 100644 --- a/crates/wright-language/src/service.rs +++ b/crates/wright-language/src/service.rs @@ -788,7 +788,7 @@ fn is_workshop_document(document: &Document) -> bool { /// The symbol whose identifier occurrence covers `position`, and that /// occurrence — resolved through the index's shared `symbol_at` rule, the -/// same addressability `semanticRename` applies (#434, #555). A miss and an +/// same addressability editor rename applies (#434, #555). A miss and an /// ambiguous position both map to `None`: the editor surfaces only a /// resolved symbol. fn symbol_at<'index>( diff --git a/crates/wright-transform/src/pipeline.rs b/crates/wright-transform/src/pipeline.rs index 92b4345a..4521020c 100644 --- a/crates/wright-transform/src/pipeline.rs +++ b/crates/wright-transform/src/pipeline.rs @@ -229,8 +229,8 @@ fn fold_value_once(value: &mut workshop_rs::Value, nodes: &mut usize) -> bool { // The short-circuit folds discard the other operand // entirely — apply them only when that operand is // literal-only, so a folded `Or(True, Global.g)` cannot - // drop the identifier occurrence `references` and - // `semanticRename` index (#639). + // drop the identifier occurrence `references` indexes + // (#639). let boolean_result = match (&args[0], &args[1]) { (Value::Bool(left), Value::Bool(right)) if name == "and" => { Some(*left && *right) diff --git a/docs/agent-contract.md b/docs/agent-contract.md index bef0cd6f..ba646a4a 100644 --- a/docs/agent-contract.md +++ b/docs/agent-contract.md @@ -111,9 +111,7 @@ plus the operation in snake case: | `lintRules` | `wright_lint_rules` | | `costEstimate` | `wright_cost_estimate` | | `targetMetadata` | `wright_target_metadata` | -| `semanticRename` | `wright_semantic_rename` | | `validateEditTransaction` | `wright_validate_edit_transaction` | -| `providerSemanticRename` | `wright_provider_semantic_rename` | | `providerValidateEdit` | `wright_provider_validate_edit` | | `lookup` | `wright_lookup` | @@ -195,8 +193,6 @@ the successful `result` payload. | `costEstimate` | optional selection, optional `rule`, optional `all` | Exact generated-resource counts, `elementCount` (the canonical Workshop structure counted against the client element budget; `rules` ranked by count, largest first. `index` is the canonical rule position and each entry carries a resolved `span`; the 5 largest by default, `all: true` for every rule; `null` plus `elementCountUnavailable` when the count cannot run), findings (each with a resolved `span`), and `selection` when applied. With `rule` the result is the rule drill-down (#724): `rule` (identity, `count`, `span`), `statements[]` (direct children ranked by recursive `count`, each with `name`, `count`, `span`) and `byNode[]` (each node's own `base_count + adjustment` cost summed per canonical node name with `occurrences`; the entries sum to the rule's `count`) | The default result carries no run-cache comparison — an agent requests one explicitly by passing the reference record as inline data, not a file path (ADR-0027, #723). | | `targetMetadata` | none | Canonical target/catalog metadata | | `validateEditTransaction` | `transaction`, optional `sources` | Atomic validation status, diagnostics, and previews when valid | -| `semanticRename` | `target`, optional `sources` | Validated rename transaction or structured refusal | -| `providerSemanticRename` | `language_id`, `position_document_uri`, `position`, `new_name`, optional `documents`, `sources`, `project_root` | Provider-resolved rename transaction or structured refusal | | `providerValidateEdit` | `language_id`, `transaction`, optional `documents`, `sources`, `project_root` | Provider-validated transaction or structured refusal | | `lookup` | required `language`; optional `query`, `kind`, `within`, `locale`, `limit` | Owner vocabulary entries with accepted spellings and rendered signatures, or an explicit `unavailable` payload naming the owner | @@ -277,7 +273,7 @@ diagnostic a snapshot query refuses with), matching the embedding API's Numeric symbol ids and rule indexes are valid only for the program that issued them. After a content-changing reload, a request carrying a numeric -`symbol`, `rule`, or `semanticRename` target id is refused `stale-id` until +`symbol` or `rule` is refused `stale-id` until the client observes the new space: a successful `symbols` response re-establishes symbol ids, a successful `rules` response re-establishes rule indexes, and an `ambiguous-symbol`/`ambiguous-rule` refusal @@ -326,8 +322,7 @@ characters and 2 otherwise, ties in declaration order), or the `symbols` / message only, as ADR-0021 decision 6 does for owner vocabulary. 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; `semanticRename` -addressed by name reports the same refusals. `usage` +Resolution never guesses or returns an empty success. `usage` also echoes the resolved `id` and `kind` so a name-addressed caller can correlate the result with `symbols`. @@ -435,9 +430,9 @@ proposes and validates edits; a caller remains responsible for applying them. Stale, overlapping, unsupported, or semantically invalid edits return an explicit refusal without a partial edit set. -### Validated edits and semantic rename for raw Workshop (#434) +### Validated edits for raw Workshop (#434) -On raw Workshop input both operations are supported directly. `workshop-rs` +On raw Workshop input the edit operation is supported directly. `workshop-rs` owns the source semantics and Wright orchestrates the transaction: * `validateEditTransaction` applies the caller's transaction to the current @@ -449,35 +444,17 @@ owns the source semantics and Wright orchestrates the transaction: keeps that ability. A transaction that produces malformed or invalid Workshop refuses with the real parse/validation diagnostics and no partial preview; a valid one returns `ok: true` with a `preview` of each - edited source (edited text plus the post-edit identity). -* `semanticRename` resolves `target` either by `symbol`, a numeric id or a - declared name that is exactly the addressing `references`/`usage` use, or by a - `source`/`line`/`col` position inside one identifier occurrence. Global - variables, player variables, and subroutines rename through the exact - identifier spans `workshop-rs` records: the declaration, the `Subroutine` - event binding, `Call Subroutine` callees, `Start Rule` subroutine - arguments, `Set`/`Modify`/`For` variable - arguments, and `Global.name`/`Event Player.name` value references all - rewrite, while prefixes, comments, strings, and unrelated identifiers stay - untouched. There is no textual-search fallback. An occurrence whose - recorded span does not cover exactly the identifier refuses with - `rename-unmapped-span`. - `sources` is optional (#472): when omitted or null, the service reads the - loaded input file's current text from disk; when supplied, it must carry - the current text of the loaded input file. A text - that differs from the loaded program's refuses with `edit-stale-source` - because provenance spans would no longer index it. A `to` name that - already declares a same-kind symbol refuses with `rename-name-collision`, - and one that does not survive reparsing refuses with the parse diagnostic; - rule names are not rename targets (`rename-unsupported-kind`). + edited source (edited text plus the post-edit identity). There is no + Wright-side rename operation: a rename is a caller-proposed transaction + over the identifier spans `references`/`symbols` report. * Non-Workshop input stays at the provider boundary: OPY refuses with - `edit-requires-provider` naming `providerValidateEdit` / - `providerSemanticRename`, and kinds without a shipped provider keep their + `edit-requires-provider` naming `providerValidateEdit`, and kinds without + a shipped provider keep their `source-provider-unavailable` refusal. * Findings on `lint` and `findings` carry a `fix` member when a rule offers a mechanically determined correction (#556): `fix.kind` names the remediation, `fix.summary` describes it, and `fix.transaction` is the - same `EditTransaction` shape `semanticRename` produces. Preview it + `EditTransaction` shape `validateEditTransaction` validates. Preview it through `validateEditTransaction` and apply it through the caller's own write path; there is no separate apply operation. Fixes exist only on raw Workshop input for `exact`-evidence findings whose correction is @@ -494,14 +471,9 @@ owns the source semantics and Wright orchestrates the transaction: positions it freezes nothing, it marks the repeated evaluation as intentional so the finding retires. -`wright rename [INPUT]` exposes the same semantic rename -on the CLI: it prints the validated diff by default and applies it -atomically with `--write`, refusing `edit-stale-source` when the file -changed since validation. - ### Provider-backed edits -`providerValidateEdit` and `providerSemanticRename` run the same +`providerValidateEdit` runs the same validated-edit pipeline through the configured source-language provider (`opy` today): Wright checks the caller's preconditions and translates coordinates, the provider validates each edited document, and the edited @@ -518,36 +490,26 @@ caller-supplied or session-derived: the document set the session already holds. The omitted-set request reads the program like any other program-reading operation (#471): it triggers the deferred load and refuses with the loader's diagnostic on an - unloadable project. A `providerValidateEdit` `edits[].source` outside - the set refuses `edit-unknown-source`; `providerSemanticRename` requires - `position_document_uri` inside the set and refuses provider-returned - edits outside it with `provider-edit-outside-set`. + unloadable project. An `edits[].source` outside + the set refuses `edit-unknown-source`. * `sources` is optional (#548) and defaults to each document's text. It is a map from URI to current text covering every source the transaction may - edit: each `edits[].source` on `providerValidateEdit` and every document - the provider's rename may touch on `providerSemanticRename`. It is the + edit: each `edits[].source` on `providerValidateEdit`. It is the precondition each `source_identity` verifies against (SHA-256 hex of the text; for the entry document this equals `project.inputIdentity`) and the text the edits apply to. * `providerValidateEdit`'s post-edit check blocks on errors in any supplied document, so the set should cover the project documents whose diagnostics may break. -* `providerSemanticRename` resolves the analyzable project from the - supplied documents themselves. Include the project entry document - (e.g. `main.opy`), not only the position file. A member file alone can - refuse `rename.noSymbolAtPosition` even when the position is valid; - supply the entry plus every file the rename may touch. The session- - derived set already contains every loaded member, entry included. -* `providerSemanticRename`'s `position` uses the provider convention, - 0-based line and UTF-16 character, while `transaction` ranges are the +* `transaction` ranges are the `EditRange` 1-based half-open columns every edit operation shares. The returned transaction's `source` fields are document URIs and each `source_identity` is the identity of that document's current text, so the caller can replay or apply the edits itself. Operations carrying caller-supplied documents answer without consulting -the session's loaded project (#471); both refuse cleanly when no provider -is configured. +the session's loaded project (#471); the operation refuses cleanly when no +provider is configured. ### Name and signature lookup (#529, ADR-0021) @@ -636,10 +598,10 @@ metadata, meaning summary, rationale, documentation, known limits, evidence, and tags, is served once by `lintRules`. ADR-0020 records a second pre-freeze exception (#472): `sources` is optional -on `validateEditTransaction` and `semanticRename` rather than required, so +on `validateEditTransaction` rather than required, so agent callers stop resending whole files. The same exception extends to the -provider operations (#548): `documents` and `sources` are optional on -`providerSemanticRename` and `providerValidateEdit`, defaulting to the +provider operation (#548): `documents` and `sources` are optional on +`providerValidateEdit`, defaulting to the session's loaded project and each document's text. The optional guide installed by `wright agent install` (and distributed by diff --git a/docs/architecture/tooling.md b/docs/architecture/tooling.md index 34df0d64..1e4c9c2c 100644 --- a/docs/architecture/tooling.md +++ b/docs/architecture/tooling.md @@ -49,7 +49,7 @@ re-check / re-analyze Normal source tooling does not require full-file regeneration. Preserve comments/trivia/formatting/unchanged structure where practical. Unsupported or unsafe edits fail explicitly; do not silently degrade to textual search/replace when semantic correctness is required. -For raw Workshop input this model runs through `workshop-rs` itself (#434): `validateEditTransaction` applies a caller's transaction to the current sources and reparses/revalidates the edited project, and `semanticRename` resolves a symbol- or position-addressed target against the loaded program's semantic index and rewrites exactly the identifier spans the parsed program records, never a textual search. `wright rename [INPUT]` exposes the same validated rename to users (a diff by default, an atomic write with `--write`). OPY and other source languages remain provider-owned edit surfaces: the raw operations refuse them by naming the `providerValidateEdit`/`providerSemanticRename` operations rather than guessing at source semantics Wright does not own. +For raw Workshop input this model runs through `workshop-rs` itself (#434): `validateEditTransaction` applies a caller's transaction to the current sources and reparses/revalidates the edited project. Agents and users propose edits directly — there is no Wright-side semantic rename operation — and `lint --fix` uses the same validated transaction machinery for its offered fixes. OPY and other source languages remain provider-owned edit surfaces: the raw operation refuses them by naming `providerValidateEdit`, and editor rename stays provider-owned through the LSP `textDocument/rename` path, rather than guessing at source semantics Wright does not own. ## Shared services diff --git a/docs/cli/commands.md b/docs/cli/commands.md index 7994948c..5f678afd 100644 --- a/docs/cli/commands.md +++ b/docs/cli/commands.md @@ -43,7 +43,6 @@ result. | `wright inspect cfg [INPUT] [--only KIND] [--max N]` | Control-flow graph of one rule, addressed by name | block/edge listing | | `wright inspect callgraph [INPUT] [--caller RULE] [--callee SUBROUTINE] [--max N]` | Subroutine call graph (caller rules to callee subroutines) | call edges | | `wright inspect cost [INPUT] [--rule RULE] [--no-cache] [--baseline NAME] [--save-baseline NAME] [--max-total N]` | Exact generated-resource counts plus static findings; `--rule` drills into one rule's element cost, and by default the run compares against the previous run's element cost | resource counts and findings plus the change against the previous run or named baseline, or the rule's statements and node-name shares | -| `wright rename [INPUT]` | Semantically rename a Workshop variable or subroutine, or an OPY symbol through the provider | per-source diff of the validated edits; `--write` applies them | | `wright lookup [QUERY]` | Resolve a Workshop or OverPy display name, near spelling, or guess to the owner-accepted spelling and signature (#529); takes no input | one line per entry: signature or spelling, kind, identity | | `wright serve [INPUT] [--transport T] [--kind K] [--profile P] [--lint-config YAML] [--rule PATH] [--disable-rule ID] [--rule-severity ID:SEV]` | Serve `wright-agent/v1` over stdio, JSON-RPC 2.0, or MCP | one structured response per request | | `wright completion ` | Generate static completion script for bash, zsh, fish, or powershell | the generated completion script | @@ -307,47 +306,6 @@ block detail for `cfg`, fan-in/fan-out highlights before the edge list for first page of 20 entries followed by the total and an expansion hint; `--format json` reports the essential result in structured form, with the same list cap as text. -## `wright rename`: semantic rename for Workshop and OPY (#434, #739) - -`wright rename [INPUT]` renames a global variable, player -variable, or subroutine in raw Workshop input. `NAME` is the declared symbol -name, the same addressing `inspect refs` uses, resolved against the loaded -program's semantic index; an unmatched name is `unknown-symbol` and a name -shared by several symbols is `ambiguous-symbol` (exit 1). - -The rename is semantic, not textual: every edit rewrites exactly the -identifier span `workshop-rs` records for one declaration or reference, so -`Global.score` rewrites `score` and leaves the `Global.` prefix, and comments -or string literals containing the name stay untouched. The proposed -transaction is validated by reparsing the edited source through the session's -own `workshop-rs` path; a name that does not survive reparsing, or one whose -references no longer bind to the renamed symbol, refuses with -`rename-mismatch` and no partial edit set. - -* Default is **preview only**: text mode prints the diff as `-`/`+` line - pairs, and JSON mode returns the validated `transaction` and per-source - `preview` inside the `wright-result/v1` envelope. Callers (and agents via - `semanticRename`) carry the same atomic edit set. -* `--write` applies the validated transaction to the input file atomically - (a sibling temporary file and rename). The write rechecks each source's - identity hash first; a file that changed since validation refuses with - `edit-stale-source` and writes nothing. -* OPY input runs the same command through the configured OPY provider - (#739) — the rename `wright serve` exposes as `providerSemanticRename`, - reached through the CLI's `NAME` addressing. The session loads the - project (a single file, a `#!include` graph, or a directory input) and - resolves `NAME` to a declaration position through the provider's - `lpp/symbols` — never a Wright-side text search — then `lpp/rename` - produces the edit set, `lpp/validateEdits` and a post-edit project check - validate it, and the same preview/`--write` contract applies. A name the - project does not declare is `unknown-symbol`; a name spelling several - distinct symbols is `ambiguous-symbol` listing each candidate's - `path:line:col`; and a name the provider refuses — an invalid - `NEW_NAME`, an unrenamable kind, a collision — reports the provider's own - `provider-refusal-` diagnostic with no partial edit set. -* Other source kinds stay provider surfaces: kinds without a shipped - provider keep their `source-provider-unavailable` refusal. - ## `wright lookup`: owner name and signature lookup (#529) `wright lookup [QUERY]` runs the agent contract's `lookup` operation diff --git a/docs/cli/lint.md b/docs/cli/lint.md index 116f30a4..608167e3 100644 --- a/docs/cli/lint.md +++ b/docs/cli/lint.md @@ -207,7 +207,8 @@ operations identically: } ``` -`fix.transaction` is the same `EditTransaction` `semanticRename` produces: +`fix.transaction` is the same `EditTransaction` `validateEditTransaction` +validates: the agent path previews it through `validateEditTransaction` and applies it through the caller's own write path. There is no separate fix operation. On the CLI, `wright lint --fix` renders each offered fix's validated diff, diff --git a/docs/cli/machine-contract.md b/docs/cli/machine-contract.md index efc54261..69193482 100644 --- a/docs/cli/machine-contract.md +++ b/docs/cli/machine-contract.md @@ -79,7 +79,7 @@ analyzer's codes, including the name-addressing refusals `unknown-symbol`, `source-provider-unavailable` marks the explicit DEL/OSTW provider boundary and is reported at the internal stage. Provider failures keep the provider's typed classification in the diagnostic code, on the -source-provider seam and in `providerSemanticRename`/`providerValidateEdit` +source-provider seam and in `providerValidateEdit` mutation results alike: an LPP refusal is `provider-refusal-` (a Wright-originated refusal keeps its own code, e.g. `session-config-changed`), a capability gap is diff --git a/docs/embedding.md b/docs/embedding.md index e2ce7beb..b0def6ca 100644 --- a/docs/embedding.md +++ b/docs/embedding.md @@ -13,7 +13,7 @@ safe source-edit contracts, and the transport adapters | `wright_driver::{Envelope, CompileResult, CheckResult, AnalyzeResult, InspectResult, LintResult, Diagnostic, CompiledOutput}` | **stable** | `wright-result/v1` machine contract ([`docs/cli.md`](cli.md)) | | `wright_driver::service::{ToolService, ToolRequest, ToolResponse, Capabilities}` | **stable** | Session-aware tool contract `wright-agent/v1` ([`docs/agent-contract.md`](agent-contract.md)); capability discovery, queries, workflows, and validated mutation | | `wright_driver::edit::{SourceEdit, EditRange, EditTransaction, SourcePreview, EditValidation, validate_transaction}` | **stable** | Source-edit transactions; validated through the correct owner-backed project semantics (#128); `EditTransaction::apply` applies ranges against one original source snapshot | -| `wright_driver::edit::{RenameRequest, rename_symbol}` | **deprecated** | `wright-embedding/v1` compatibility helpers retained for source compatibility (#514): a whole-file textual whole-word proposal with no semantic-identity guarantee. Strings, comments, and unrelated same-named symbols are not distinguished. The supported rename is `semanticRename` (see below); the helpers may be removed only with a breaking embedding-contract transition | +| `wright_driver::edit::{RenameRequest, rename_symbol}` | **deprecated** | `wright-embedding/v1` compatibility helpers retained for source compatibility (#514): a whole-file textual whole-word proposal with no semantic-identity guarantee. Strings, comments, and unrelated same-named symbols are not distinguished. There is no Wright-side semantic rename on the tool surface; a rename is a caller-proposed `EditTransaction` validated through `validateEditTransaction` (see below). The helpers may be removed only with a breaking embedding-contract transition | | `wright_driver::{input_identity, EMBEDDING_CONTRACT}` | **stable** | `wright-embedding/v1` | | Internal HIR/WIR arenas, parser/CST, emitter internals | **internal** | Never part of the public contract | | `wright serve` stdio/JSON-RPC/MCP adapters | **stable** | Thin mappings over `ToolService` and `wright-agent/v1`; `wright-serve` remains a workspace binary alias | @@ -83,7 +83,7 @@ remains readable and writable for source compatibility within `wright-embedding/v1`, but post-construction mutation is not a supported capability: the next workflow (`load`, `compile`, `check`, `analyze`, `inspect`, `lint`, `convert`, `symbols`, `refs`, `cfg`, `callgraph`, `cost`, -`rename`, `validate_edit_transaction`, `semantic_rename`) or `ToolService` +`validate_edit_transaction`) or `ToolService` construction refuses with a `session-config-changed` diagnostic naming the changed fields, rather than reusing state derived from the earlier configuration. The provider surfaces that consume the provider @@ -123,8 +123,8 @@ actions, values, events, enum domains, and locales. ## Validated mutation (#130) -Agents and embedding consumers request mutation through two structured -tool operations over the session's project: +Agents and embedding consumers request mutation through a structured +tool operation over the session's project: * `validateEditTransaction`: validate and preview a caller-supplied [`EditTransaction`] against the session project. The request may carry the @@ -132,20 +132,14 @@ tool operations over the session's project: `sources` is omitted the service reads those files from disk (#472). The response returns `ok`, structured diagnostics, and per-source previews with the edited text and its new SHA-256 identity. -* `semanticRename`: request a semantic rename by `symbol` or at a 1-based - position (`source`/`line`/`col`/`to`) through the shared #129 refactoring - contract, with `sources` optional as for `validateEditTransaction` (#472). - The response returns the validated exact-range transaction - (`ok: true`) or structured refusal diagnostics (`ok: false`, no - transaction). - -Both preserve the #128 all-or-nothing semantics: an unsafe, stale, + +It preserves the #128 all-or-nothing semantics: an unsafe, stale, overlapping, colliding, or unsupported request returns structured diagnostics and never a partially applicable edit set. Wright **proposes and validates** edits; applying them to the filesystem is an explicit consumer responsibility because the semantic and tooling core never writes -files. Capability discovery advertises `validateEditTransaction` and -`semanticRename`; the `serve` transport adapters forward the same operations +files. Capability discovery advertises `validateEditTransaction`; the +`serve` transport adapters forward the same operation unchanged (behaviorally equivalent, transport-tested). ## Safe edits @@ -164,24 +158,27 @@ overlapping/conflicting edits, invalid ranges, and compilation errors, and returns the previewed edited sources atomically (any failed validation returns `ok = false` and no validated preview). Validation runs through the provider-backed project/session semantics where the provider negotiates edit -validation. OPY edit validation and rename are provider-backed: they run -through `providerValidateEdit` / `providerSemanticRename` against the -configured provider's negotiated `editValidation` and `rename` capabilities, -and the shipped first-party provider negotiates both. Only a provider that -does not negotiate them refuses (`capability-unavailable`); Wright never -invokes a removed static frontend. The raw edit operations keep their source +validation. OPY edit validation is provider-backed: it runs +through `providerValidateEdit` against the +configured provider's negotiated `editValidation` capability, +and the shipped first-party provider negotiates it. Only a provider that +does not negotiate it refuses (`capability-unavailable`); Wright never +invokes a removed static frontend. The raw edit operation keeps its source kind gate: OPY input refuses with `edit-requires-provider` naming the provider operation, DEL/OSTW input refuses with `source-provider-unavailable`, and other non-Workshop kinds refuse with -`edit-unsupported-kind`. On the provider operations, a `language_id` with no -configured provider refuses explicitly (`provider-not-configured`). The -supported refactoring is semantic rename (`semanticRename` above, -`ToolRequest::SemanticRename`, or [`CompilerSession::semantic_rename`]): on -raw Workshop it rewrites exactly the identifier spans the parsed program -records and revalidates the edited project. The deprecated `rename_symbol` helper remains only for +`edit-unsupported-kind`. On the provider operation, a `language_id` with no +configured provider refuses explicitly (`provider-not-configured`). There is +no Wright-side semantic rename on the tool surface: a rename is a +caller-proposed `EditTransaction` over the identifier spans `symbols`/`refs` +report, validated through `validateEditTransaction` (raw Workshop) or +`providerValidateEdit` (provider-owned source). Provider-backed rename still +exists for editors only, through the LSP `textDocument/rename` path +(`wright_driver::provider_edit::semantic_rename`). The deprecated +`rename_symbol` helper remains only for `wright-embedding/v1` source compatibility. It is a whole-word textual -proposal with no semantic-identity guarantee and is never a fallback when -semantic rename is unavailable or refused. Raw HIR/WIR mutation is never public, and +proposal with no semantic-identity guarantee and is never a fallback for a +refused validation. Raw HIR/WIR mutation is never public, and application/writing stays an explicit caller responsibility. ## Transports @@ -217,4 +214,4 @@ omitted. `crates/wright-consumer` is a committed consumer that depends only on `wright-driver` and runs compile/check/analyze/lint, all tool queries, and a -validated rename over the corpus (`wright-consumer/tests/consumer.rs`). +validated edit transaction over the corpus (`wright-consumer/tests/consumer.rs`). diff --git a/docs/language-services.md b/docs/language-services.md index ae2c16b1..b394466d 100644 --- a/docs/language-services.md +++ b/docs/language-services.md @@ -64,8 +64,8 @@ documents and OPY documents without a working provider publish an explicit its structured error code on the document. A document that is neither publishes no diagnostics. -`textDocument/rename` on a source-language document routes through the same -provider-owned mutation path as the CLI and agent surfaces (#156): the +`textDocument/rename` on a source-language document routes through the +provider-owned mutation path (#156): the provider computes edits over the open document set (only the provider can compute project membership, so the set stays language-wide), Wright verifies document versions and source preconditions, asks the provider to validate diff --git a/docs/v1-matrix.md b/docs/v1-matrix.md index 271d0b20..4ae29d15 100644 --- a/docs/v1-matrix.md +++ b/docs/v1-matrix.md @@ -8,7 +8,7 @@ Status: current provider-boundary baseline | DEL / OSTW source workflows | `deltin-rs` or a future provider | Explicit `source-provider-unavailable` diagnostics | | Canonical Workshop text and WIR | `workshop-rs` | `wright-analyzer`, `wright-transform`, and embedding tests | | Driver and CLI result contracts | Wright | `wright-driver` and `wright-cli` tests | -| Language services / LSP | Wright | `wright-lsp` advertises document synchronization plus raw-Workshop diagnostics/hover/definition/references (and rename with versioned edits); source-language documents report `source-provider-unavailable` | +| Language services / LSP | Wright | `wright-lsp` advertises document synchronization plus raw-Workshop diagnostics/hover/definition/references, and provider-backed rename on source-language documents (with versioned edits); unbacked source-language documents report `source-provider-unavailable` | Wright does not claim OPY or DEL/OSTW language completeness from a successful build. Provider capabilities and source-language semantics remain bounded by diff --git a/schemas/wright-agent-v1.schema.json b/schemas/wright-agent-v1.schema.json index 047b82b8..f0df862a 100644 --- a/schemas/wright-agent-v1.schema.json +++ b/schemas/wright-agent-v1.schema.json @@ -24,8 +24,6 @@ { "$ref": "#/$defs/CostEstimateRequest" }, { "$ref": "#/$defs/TargetMetadataRequest" }, { "$ref": "#/$defs/ValidateEditTransactionRequest" }, - { "$ref": "#/$defs/SemanticRenameRequest" }, - { "$ref": "#/$defs/ProviderSemanticRenameRequest" }, { "$ref": "#/$defs/ProviderValidateEditRequest" }, { "$ref": "#/$defs/LookupRequest" } ] @@ -78,8 +76,6 @@ { "$ref": "#/$defs/CostEstimateResult" }, { "$ref": "#/$defs/TargetMetadataResult" }, { "$ref": "#/$defs/ValidateEditTransactionResult" }, - { "$ref": "#/$defs/SemanticRenameResult" }, - { "$ref": "#/$defs/ProviderSemanticRenameResult" }, { "$ref": "#/$defs/ProviderValidateEditResult" }, { "$ref": "#/$defs/LookupResult" } ] @@ -323,17 +319,6 @@ }, "additionalProperties": true }, - "SemanticRenamePayload": { - "type": "object", - "required": ["ok", "transaction", "diagnostics"], - "properties": { - "ok": { "type": "boolean" }, - "transaction": { "anyOf": [{ "type": "null" }, { "$ref": "#/$defs/EditTransaction" }] }, - "diagnostics": { "type": "array", "items": { "$ref": "#/$defs/Diagnostic" } }, - "preview": { "type": "array", "items": { "$ref": "#/$defs/SourcePreview" } } - }, - "additionalProperties": true - }, "ProviderMutationPayload": { "type": "object", "required": ["ok", "diagnostics"], @@ -716,8 +701,6 @@ "additionalProperties": true }, "ValidateEditTransactionResult": { "$ref": "#/$defs/EditValidationPayload" }, - "SemanticRenameResult": { "$ref": "#/$defs/SemanticRenamePayload" }, - "ProviderSemanticRenameResult": { "$ref": "#/$defs/ProviderMutationPayload" }, "ProviderValidateEditResult": { "$ref": "#/$defs/ProviderMutationPayload" }, "LookupEntry": { "type": "object", @@ -864,7 +847,7 @@ }, "Sources": { "type": "object", - "description": "Source name → full current text. On `validateEditTransaction`/`semanticRename` the key is the loaded input's source spelling and the field may be omitted (the service reads the named files from disk, #472). On `providerValidateEdit` the keys are `documents` URIs covering every `edits[].source`; on `providerSemanticRename` they cover every document the provider's computed rename may edit — an edit into an unsupplied source refuses `edit-unknown-source`. The text is the precondition each edit's `source_identity` verifies against and the text edits apply to.", + "description": "Source name → full current text. On `validateEditTransaction` the key is the loaded input's source spelling and the field may be omitted (the service reads the named files from disk, #472). On `providerValidateEdit` the keys are `documents` URIs covering every `edits[].source` — an edit into an unsupplied source refuses `edit-unknown-source`. The text is the precondition each edit's `source_identity` verifies against and the text edits apply to.", "additionalProperties": { "type": "string" } }, "EditRange": { @@ -883,7 +866,7 @@ "type": "object", "required": ["kind", "source", "source_identity", "range", "new_text"], "properties": { - "kind": { "type": "string", "description": "The edit kind: `edit` for a caller-proposed replacement; other kinds such as `rename` appear only on service-computed results." }, + "kind": { "type": "string", "description": "The edit kind: `edit` for a caller-proposed replacement; other kinds may appear on service-computed results." }, "source": { "type": "string", "description": "The source this edit applies to: the loaded input's spelling on `validateEditTransaction`, or a `documents` URI key on the provider operations." }, "source_identity": { "type": "string", "description": "The SHA-256 hex digest of the source's current text — the same digest `project.inputIdentity` reports for the entry source; a text that hashes differently refuses `edit-stale-source`." }, "range": { "$ref": "#/$defs/EditRange" }, @@ -899,18 +882,6 @@ }, "additionalProperties": false }, - "RenameTarget": { - "type": "object", - "required": ["to"], - "properties": { - "symbol": { "type": ["integer", "string"], "minimum": 0, "maximum": 4294967295, "description": "Symbol addressing (#434): the numeric symbol id or the declared name, exactly as `references`/`usage` address symbols. When present, `source`/`line`/`col` are ignored. A numeric id issued before a disk reload is refused `stale-id` until a `symbols` listing re-establishes the space." }, - "source": { "type": "string", "description": "Position addressing: the source file the position is interpreted in; requires `line` and `col`." }, - "line": { "type": "integer", "minimum": 0, "maximum": 4294967295, "description": "Position addressing: the 1-based line of a character inside one identifier occurrence." }, - "col": { "type": "integer", "minimum": 0, "maximum": 4294967295, "description": "Position addressing: the 1-based column of a character inside one identifier occurrence." }, - "to": { "type": "string", "description": "The replacement identifier." } - }, - "additionalProperties": false - }, "Document": { "type": "object", "required": ["uri", "languageId", "version", "text"], @@ -924,19 +895,9 @@ }, "DocumentSet": { "type": "object", - "description": "URI → `Document` map: the caller-owned project document set the provider operates on. `providerValidateEdit` refuses an `edits[].source` outside the set with `edit-unknown-source`; `providerSemanticRename` requires `position_document_uri` inside the set and refuses provider-returned edits outside it with `provider-edit-outside-set`.", + "description": "URI → `Document` map: the caller-owned project document set the provider operates on. `providerValidateEdit` refuses an `edits[].source` outside the set with `edit-unknown-source`.", "additionalProperties": { "$ref": "#/$defs/Document" } }, - "Position": { - "type": "object", - "description": "A 0-based line and UTF-16 character offset — the provider convention, unlike `EditRange`'s 1-based columns.", - "required": ["line", "character"], - "properties": { - "line": { "type": "integer", "minimum": 0, "maximum": 4294967295 }, - "character": { "type": "integer", "minimum": 0, "maximum": 4294967295 } - }, - "additionalProperties": false - }, "CapabilitiesRequest": { "type": "object", "properties": { "op": { "const": "capabilities" } }, "required": ["op"], "additionalProperties": false }, "CompileRequest": { "type": "object", @@ -1222,31 +1183,6 @@ "required": ["op", "transaction"], "additionalProperties": false }, - "SemanticRenameRequest": { - "type": "object", - "properties": { - "op": { "const": "semanticRename" }, - "sources": { "anyOf": [{ "$ref": "#/$defs/Sources" }, { "type": "null" }], "description": "Optional current texts for the edited sources; omitted or null reads the named files from disk (#472)." }, - "target": { "$ref": "#/$defs/RenameTarget" } - }, - "required": ["op", "target"], - "additionalProperties": false - }, - "ProviderSemanticRenameRequest": { - "type": "object", - "properties": { - "op": { "const": "providerSemanticRename" }, - "language_id": { "type": "string", "description": "The configured source provider's language id — `opy` for the shipped provider." }, - "documents": { "anyOf": [{ "$ref": "#/$defs/DocumentSet" }, { "type": "null" }], "description": "Optional: the caller-owned document set — must contain `position_document_uri`, the project entry document (the provider resolves the analyzable view from a supplied entry — a member file alone can refuse `rename.noSymbolAtPosition`), and every document the rename may edit. Omitted or null: the session's loaded project supplies the set — every member read from disk at version 0, keyed by `file://` URI." }, - "position_document_uri": { "type": "string", "description": "URI of the `documents` document holding the rename position." }, - "position": { "$ref": "#/$defs/Position" }, - "new_name": { "type": "string", "description": "The replacement identifier." }, - "project_root": { "type": ["string", "null"], "description": "Optional project root the provider resolves the project from." }, - "sources": { "anyOf": [{ "$ref": "#/$defs/Sources" }, { "type": "null" }], "description": "Optional URI → current text covering every document the provider's rename may edit (omitted or null defaults to each document's text); a returned edit into an unsupplied or mismatched source refuses `edit-unknown-source`/`edit-stale-source`." } - }, - "required": ["op", "language_id", "position_document_uri", "position", "new_name"], - "additionalProperties": false - }, "ProviderValidateEditRequest": { "type": "object", "properties": { From 7b4958c0fd626688701c2e21acc2060cea1abdc3 Mon Sep 17 00:00:00 2001 From: Teakowa <27560638+Teakowa@users.noreply.github.com> Date: Mon, 19 Oct 2026 23:33:44 +0800 Subject: [PATCH 2/2] docs: sync vendored agent guide and drop stale rename references The vendored guide was hand-edited instead of synced; re-vendor it from wrightkit/skills@cdd694f (post-skills#26) so the pinned copies match and the CI agent-guide check passes. Also reword the README feature bullet and the metrics comment that still named the removed operation. Refs #780 --- README.md | 2 +- agent-guide/UPSTREAM.md | 2 +- agent-guide/wright/SKILL.md | 2 +- agent-guide/wright/references/language-notes.md | 4 ++-- benchmarks/metrics/metrics.py | 2 +- crates/wright-cli/src/agent.rs | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 28addcb4..b32739a5 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ agents. - **Cost and risk analysis.** Workshop element cost, hotspots, and server-load risk indicators, each labeled exact, static, or heuristic. - **Semantic queries.** Symbols, references, control flow, call graph, cost. -- **Validated edits.** Renames that preview a diff before writing. +- **Validated edits.** Source edits that preview a diff before writing. - **Agent and editor integration.** `wright serve` (JSON-RPC and MCP) and `wright-lsp`. - **CI-friendly output.** JSON reports and GitHub Actions annotations. diff --git a/agent-guide/UPSTREAM.md b/agent-guide/UPSTREAM.md index 8836e64c..b215e113 100644 --- a/agent-guide/UPSTREAM.md +++ b/agent-guide/UPSTREAM.md @@ -1,7 +1,7 @@ # Vendored agent guide `wright/` is a verbatim copy of the `wright` skill from -`https://github.com/wrightkit/skills` at commit `478ea04`, embedded in the +`https://github.com/wrightkit/skills` at commit `cdd694f`, embedded in the binary for `wright agent install`. The skills repository is the authoring home; never edit this copy. To sync it, clone `wrightkit/skills` and run diff --git a/agent-guide/wright/SKILL.md b/agent-guide/wright/SKILL.md index f2f0a31b..660fd5cd 100644 --- a/agent-guide/wright/SKILL.md +++ b/agent-guide/wright/SKILL.md @@ -1,6 +1,6 @@ --- name: wright -description: Use in a project of Overwatch Workshop rules or OverPy (.opy) source when you must judge what is wrong, risky, or expensive, or how a change propagates, and text search cannot answer it. Covers Workshop server freezes, infinite loops, rule cost, symbol and rule flow, validated edits, and OPY-to-Workshop compilation, through the `wright` CLI. +description: Use in a project of Overwatch Workshop rules or OverPy (.opy) source when you must judge what is wrong, risky, or expensive, or how a change propagates, and text search cannot answer it. Covers Workshop server freezes, infinite loops, rule cost, symbol and rule flow, and OPY-to-Workshop compilation, through the `wright` CLI. license: AGPL-3.0 --- diff --git a/agent-guide/wright/references/language-notes.md b/agent-guide/wright/references/language-notes.md index e0e5177a..79242d60 100644 --- a/agent-guide/wright/references/language-notes.md +++ b/agent-guide/wright/references/language-notes.md @@ -6,12 +6,12 @@ Confirm against `wright --help` and the `capabilities` session operation; suppor - Fully owned by `workshop-rs`. Semantic queries and validated edit transactions are supported here where the installed version declares them. - `compile --to ` re-emits the program in another Workshop client locale (e.g. `en-US`, `zh-CN`) without changing the input; the serve `compile` operation takes the same `to` field. A target locale missing mappings for the program fails explicitly, with no fallback. -- There is no rename operation. Rename a variable or subroutine by editing the source directly — `inspect refs ` lists every occurrence the rename must touch — then re-run `check`. Where the version offers it, `validateEditTransaction` validates a proposed edit against the current sources and refuses a stale source; treat a refusal as final. +- To rename a symbol, edit the source directly and re-verify with `check`. ## OverPy (`.opy`) - Compiled natively by Wright with the first-party OPY provider (`--opy-provider` overrides it). `check`, `lint`, `analyze`, `inspect`, and `compile` apply; the `inspect` query subcommands read the provider's compiled artifact. A provider without compile capability declines with its own diagnostic. -- Edits are provider-owned. The raw edit operation refuses OPY input and names `providerValidateEdit`; where the version offers it, propose a transaction there and apply its validated edits, otherwise edit the source directly and re-verify with `check`. Refusals carry structured diagnostics — treat a refusal as final. Editor rename stays in the editor: LSP `textDocument/rename` routes through the provider and is not a CLI or agent operation. +- Edits are provider-owned. The raw edit operations refuse OPY input and name the provider operations; use those when the version offers them, otherwise edit the source directly and re-verify with `check` — that same edit-and-check path covers renames. The provider-backed editor rename (F2) remains in `wright-lsp`. - `convert` runs Workshop to OPY only, and only when the provider negotiates the `reconstruct` capability; a `capability-unavailable` refusal means the installed provider does not offer it. When available, output is canonical, without comments, macros, or formatting, and unrepresentable constructs are rejected without partial output. ## DEL/OSTW diff --git a/benchmarks/metrics/metrics.py b/benchmarks/metrics/metrics.py index fb7dd078..3dcf89dd 100644 --- a/benchmarks/metrics/metrics.py +++ b/benchmarks/metrics/metrics.py @@ -70,7 +70,7 @@ METRICS_DIR = Path(__file__).resolve().parent CACHE_DIR = METRICS_DIR / ".cache" -# Mutation operations (semanticRename, validateEditTransaction, provider*) +# Mutation operations (validateEditTransaction and the provider* ops) # are absent: they need caller-supplied transactions and are not measured. AGENT_OPS = [ "capabilities", diff --git a/crates/wright-cli/src/agent.rs b/crates/wright-cli/src/agent.rs index 0fda9f3b..4b55a9b6 100644 --- a/crates/wright-cli/src/agent.rs +++ b/crates/wright-cli/src/agent.rs @@ -22,7 +22,7 @@ mod exit { const SKILL_NAME: &str = "wright"; const INSTALLED_BY: &str = "wright agent install"; -const UPSTREAM: &str = "wrightkit/skills@478ea04"; +const UPSTREAM: &str = "wrightkit/skills@cdd694f"; const DEFAULT_DEST: &str = ".agents/skills"; /// The guide files `install` writes, as `(relative path, content)`.