fixed lsp rename

7 files changed, 269 insertions, 42 deletions

diff --git a/docs/lsp.md b/docs/lsp.md
index 258e409e..097386e6 100644
--- a/docs/lsp.md
+++ b/docs/lsp.md
@@ -23,9 +23,9 @@ core                         shell                       worker
  |                           |  snapshot path + content   |
  |                           |--------------------------->|
  |                           |                            | lsp.query(...)
- |                           |    Event .lsp_resp{id,rows}|
+ |                           |  Event .lsp_resp{id,rows}  |
  |<--------------------------|<---------------------------|
- |  lspResponse -> jump, or open a results buffer         |
+ |  lspResponse -> atomic edit, jump, or results buffer   |
 ```
 
 The shell already ran this exact pattern for pty readers, so the async part is
@@ -40,8 +40,10 @@ Three rules make it safe:
   query is in flight; a borrowed slice would be a use-after-free the length of
   one keystroke.
 - **One query in flight, identified by a monotonic id.** A second press bumps
-  the id, which makes the older answer stale — `lspResponse` drops any id it is
-  not waiting for. This is also what makes a closed pane safe.
+  the id, which makes the older answer stale. The pending request also records
+  the pane serial, so a closed-and-reused slot cannot accept its response.
+- **Mutating answers are revision-checked.** Rename records the file revision
+  sent to the worker and applies nothing if the user edited before it answered.
 - **No rows is a legal answer.** A backend that cannot answer appends nothing,
   which is indistinguishable from a language server still starting up, and the
   core does nothing. There is no error path to render.
@@ -67,10 +69,19 @@ already produces and `n`/`N` already step — so:
 - **several rows** → an output buffer, which `n`/`N` walk
 
 which means helix's multi-result picker required **no picker code at all**. The
-`+Search` buffer *is* the picker. Kinds whose answer is prose rather than
-locations (`hover`, `code_action`, `format`, `rename`) open `+Hover`/`+Lsp`
-instead and do not arm the stepper — `n` over a documentation blurb would step
-to nowhere.
+`+Search` buffer *is* the picker. Non-location answers (`hover`, `code_action`,
+`format`) open `+Hover`/`+Lsp` instead and do not arm the stepper — `n` over a
+documentation blurb would step to nowhere.
+
+Rename is deliberately the one exception to rows as presentation. The backend
+emits `@edit START END` records through `lsp.edit()`, using half-open byte
+offsets into the exact `Req.source` snapshot it resolved. The core validates
+that every range is ordered, non-overlapping and in bounds, checks that the
+pane serial and file revision still match, then substitutes the requested name
+across all ranges with one allocation and one undo transaction. A malformed,
+stale, or empty response changes nothing. The current ZLS backend resolves and
+renames references in the **current file only**; it does not claim a workspace
+rename.
 
 **A path UNDER `Req.root` is written relative to it; everything else keeps its
 full absolute path** (`lsp.rel`). `Req.root` is the directory of the file the
@@ -186,7 +197,7 @@ Verified against `helix-term/src/keymap/default.rs`, not from memory.
 | `gi` | implementation | |
 | `gr` | references | |
 | `SPC l k` | hover | opens `+Hover` |
-| `SPC l r` | rename | tag input, like Find/Grep |
+| `SPC l r` | rename | tag input; applies current-file references in one undo step |
 | `SPC l a` | code action | |
 | `SPC l h` | select references | |
 | `SPC l s` / `SPC l S` | document / workspace symbols | `S` takes a query |
@@ -266,9 +277,11 @@ read is in `req` (`path`, `source` (NUL-terminated), `offset`, `arg`, `root`).
 `Io.Writer.Allocating`), so a backend never allocates the result, never frees
 it, and cannot get the allocator wrong. `arena` is freed wholesale on return;
 `gpa` is for a backend's own scratch. Use `lsp.row()` to emit a location,
-`lsp.rel()` to spell its path against `req.root` and `lsp.lineCol()` to convert
-an offset, so every backend's rows are byte-identical in shape. `rel` allocates
-nothing — it returns a slice of what you hand it.
+`lsp.rel()` to spell its path against `req.root`, `lsp.lineCol()` to convert an
+offset, and `lsp.edit()` for each half-open range of a rename response. Location
+rows are byte-identical across backends; rename ranges are consumed by the core
+and never rendered. `rel` allocates nothing — it returns a slice of what you
+hand it.
 
 ## How the implementations are judged
 
@@ -281,6 +294,7 @@ probes, every backend.
   and a kind that answers without claiming is `unclaimed-works`. Correctness
   is a substring the rows must contain, so returning a confident wrong location
   scores worse than returning nothing.
+
 - **Latency.** `cold` (first query, index construction included) and `warm`
   (median of 20). They differ by orders of magnitude for an indexing backend
   and both matter: cold is what the first keypress costs, warm is what every
@@ -292,4 +306,9 @@ probes, every backend.
   but is charged in build time and dependency surface rather than in lines we
   maintain.
 
+The user-visible rename contract is also pinned through the actual TTY,
+leader prompt, worker and ZLS backend by `test/snapshots/lsp-rename.snap`: both
+resolved occurrences change, a shadowed local does not, and undo/redo treats
+the response as one transaction.
+
 Run `zig build lspbench -- --json` for machine-readable output.
diff --git a/src/lsp/lsp.zig b/src/lsp/lsp.zig
index 90d4fc6b..29e82968 100644
--- a/src/lsp/lsp.zig
+++ b/src/lsp/lsp.zig
@@ -6,14 +6,14 @@
 //! execution model — the same shape the pty readers already use, because a
 //! language query is just another thing that answers later.
 //!
-//! Every backend renders into ONE format: `+Search` rows. A location is
+//! Location answers render as `+Search` rows. A location is
 //! `path:LINE:COL text` — or `path:LINE:COL-ENDCOL text` where the protocol
 //! answered with a real range, which a look then SELECTS — and that is what
 //! look.zig already resolves and what n/N already steps, so a multi-result
 //! answer IS helix's picker and a single result IS a jump, with no picker UI
-//! written for it. Free text (hover, a rename's diff) rides the same buffer as
-//! plain lines. A path under `Req.root` is written relative to it and any
-//! other keeps its full absolute self — see `rel`.
+//! written for it. Free text (hover, formatting) rides the same buffer. Rename
+//! is the one mutating answer: it emits byte ranges through `edit`, and the core
+//! applies them atomically only while the source revision is still current.
 //!
 //! `query` is the ONLY thing an implementation supplies. Swapping backends is
 //! swapping this one function, which is also how the three competing
@@ -162,6 +162,14 @@ pub fn spanRow(
     }) catch {};
 }
 
+/// Emit one half-open byte range for a mutating response. Rename is the only
+/// current user: every other answer remains human-readable rows. Byte offsets
+/// avoid converting the displayed 1-based locations back into source offsets
+/// in the core, and the prefix makes malformed or mixed responses fail closed.
+pub fn edit(out: *std.Io.Writer, start: usize, end: usize) void {
+    out.print("@edit {d} {d}\n", .{ start, end }) catch {};
+}
+
 /// Byte offset -> (line, column), both 0-based. Every backend needs it to turn
 /// an AST token into a row, so it lives here rather than three times over.
 pub fn lineCol(source: []const u8, offset: usize) struct { line: usize, col: usize } {
diff --git a/src/lsp/lsp_zls.zig b/src/lsp/lsp_zls.zig
index 76c5594c..bf220c47 100644
--- a/src/lsp/lsp_zls.zig
+++ b/src/lsp/lsp_zls.zig
@@ -1123,9 +1123,9 @@ fn containsIgnoreCase(hay: []const u8, needle: []const u8) bool {
 /// milliseconds, on every keypress, and the store's own workspace iteration
 /// has the same restriction (it can only see handles that were loaded).
 ///
-/// `new_name` non-null makes it a rename PREVIEW: the same rows, annotated
-/// with the replacement. The seam returns rows, not edits, so `SPC r` shows
-/// what would change and changes nothing — an honest half of rename.
+/// `new_name` non-null makes it a rename EDIT: the same resolved tokens become
+/// half-open byte ranges. The core owns the replacement text and applies every
+/// range in one undo transaction after checking the source revision.
 fn references(
     arena: std.mem.Allocator,
     analyser: *Analyser,
@@ -1149,10 +1149,10 @@ fn references(
     const want = offsets.identifierTokenToNameSlice(decl_tree, name_tok);
     if (want.len == 0) return;
 
-    const lines: Lines = try .build(arena, tree.source);
-    // every row names THIS file (the walk is this file's tokens), so the path
-    // is spelled once rather than per hit
-    const path = lsp.rel(base, handle.uri.toFsPath(arena) catch return);
+    // Rename consumes exact byte ranges. Reference rows need the source line
+    // and displayed path; avoid building either for the mutating response.
+    const lines: ?Lines = if (new_name == null) try .build(arena, tree.source) else null;
+    const path = if (new_name == null) lsp.rel(base, handle.uri.toFsPath(arena) catch return) else "";
     var n: usize = 0;
     for (0..tree.tokens.len) |i| {
         if (n >= max_rows) return;
@@ -1163,12 +1163,12 @@ fn references(
         const d = (declAt(arena, analyser, handle, at) catch continue) orelse continue;
         if (!d.eql(target)) continue;
         n += 1;
-        const r = offsets.tokenToRange(tree, tok, enc);
-        const text = if (new_name) |nn|
-            try std.fmt.allocPrint(arena, "{s} -> {s}   {s}", .{ want, nn, std.mem.trim(u8, lines.line(r.start.line), " \t") })
-        else
-            lines.line(r.start.line);
-        lsp.spanRow(out, path, r.start.line, r.start.character, r.end.line, r.end.character, text);
+        if (new_name != null) {
+            lsp.edit(out, at, at + want.len);
+        } else {
+            const r = offsets.tokenToRange(tree, tok, enc);
+            lsp.spanRow(out, path, r.start.line, r.start.character, r.end.line, r.end.character, lines.?.line(r.start.line));
+        }
     }
 }
 
diff --git a/src/output_pane.zig b/src/output_pane.zig
index 7b2d12d9..8c93709c 100644
--- a/src/output_pane.zig
+++ b/src/output_pane.zig
@@ -81,7 +81,7 @@ pub const Traits = struct {
     name: []const u8,
     /// n/N walk the rows: each is a `path:LINE:COL text` location the ordinary
     /// look path resolves, so the buffer IS helix's picker. Prose (a hover
-    /// blurb, a rename diff) has nowhere to step to.
+    /// blurb, a formatting diff) has nowhere to step to.
     steps: bool = false,
     /// ...and what a step DOES with the row it lands on. Off, the row is a
     /// LOCATION and its leading word is LOOKED. On, the row is a COMMAND LINE
@@ -135,7 +135,10 @@ pub fn traits(o: Origin) Traits {
         .query => |k| switch (k) {
             .hover => .{ .name = config.hover_buffer },
             // prose: an action list, a diff, a report about the backend
-            .code_action, .format, .rename, .status, .explain => .{ .name = config.lsp_buffer },
+            .code_action, .format, .status, .explain => .{ .name = config.lsp_buffer },
+            // Rename responses are edits consumed before an output can open;
+            // the exhaustive table still records the otherwise-unused trait.
+            .rename => .{ .name = config.lsp_buffer },
             .definition, .declaration, .type_definition, .implementation, .references => .{
                 .name = config.search_buffer,
                 .steps = true,
diff --git a/src/pardes.zig b/src/pardes.zig
index 4b9e8b1d..4ce4d101 100644
--- a/src/pardes.zig
+++ b/src/pardes.zig
@@ -4321,16 +4321,24 @@ pub const Pardes = struct {
     /// this one" — the id bump makes the older answer stale and lspResponse
     /// drops it. A queue would only buy the right to render an answer nobody
     /// is waiting for any more.
-    /// `arg` rides along only so the buffer the answer opens can record what
-    /// was asked (a rename's new name, a symbol query) — the query itself has
-    /// it in the effect already.
-    /// `row`/`col` are where the cursor was when the question was asked. Only
-    /// `completion` reads them, and only to undo itself: Tab diverted instead
-    /// of indenting, so an empty answer has to put the indent back — but only
-    /// if the cursor has not moved since, or four spaces appear under someone
-    /// who kept typing.
+    /// `arg` holds the replacement name for rename and the query for workspace
+    /// symbols. `serial` rejects a response after its pane slot was reused;
+    /// `revision` makes a mutating rename conditional on the source snapshot
+    /// the worker actually analysed. `row`/`col` are where the cursor was when
+    /// the question was asked. Only `completion` reads them, and only to undo
+    /// itself: Tab diverted instead of indenting, so an empty answer has to put
+    /// the indent back — but only if the cursor has not moved since.
     lsp_seq: u32 = 0,
-    lsp_wait: ?struct { id: u32, kind: lsp.Kind, pane: usize, arg: Effect.Buf(128), row: i32 = 0, col: i32 = 0 } = null,
+    lsp_wait: ?struct {
+        id: u32,
+        kind: lsp.Kind,
+        pane: usize,
+        serial: u32,
+        revision: u32,
+        arg: Effect.Buf(128),
+        row: i32 = 0,
+        col: i32 = 0,
+    } = null,
 
     /// One current shell-filter request. A newer submit frees and supersedes
     /// it; old worker answers then fail the id check. The request itself owns
@@ -7642,6 +7650,7 @@ pub const Pardes = struct {
             if (f.output != null) return;
         }
         if (arg.len > 128) return; // the effect's arg is a Buf(128)
+        if (kind == .rename and (!std.zig.isValidId(arg) or std.zig.isUnderscore(arg))) return;
         const off = if (pane.file) |f| modal.hxOff(f.content, .{
             .row = @intCast(@max(0, pane.cur_row)),
             .col = @intCast(@max(0, pane.cur_col)),
@@ -7651,6 +7660,8 @@ pub const Pardes = struct {
             .id = p.lsp_seq,
             .kind = kind,
             .pane = id,
+            .serial = pane.serial,
+            .revision = if (pane.file) |f| f.revision else 0,
             .arg = .from(arg),
             .row = pane.cur_row,
             .col = pane.cur_col,
@@ -7664,8 +7675,90 @@ pub const Pardes = struct {
         } });
     }
 
-    /// A worker answered. Rows are `+Search` format with ABSOLUTE paths, so
-    /// both dispositions below are the ordinary look path:
+    const LspEdit = struct { start: usize, end: usize };
+
+    fn parseLspEdits(p: *Pardes, bytes: []const u8) ?[]LspEdit {
+        if (bytes.len == 0 or bytes[bytes.len - 1] != '\n') return null;
+        const edits = p.scratch.allocator().alloc(LspEdit, std.mem.count(u8, bytes, "\n")) catch return null;
+        var lines = std.mem.splitScalar(u8, bytes, '\n');
+        var n: usize = 0;
+        while (lines.next()) |line| {
+            if (line.len == 0) {
+                if (lines.peek() == null) break;
+                return null;
+            }
+            var fields = std.mem.tokenizeScalar(u8, line, ' ');
+            if (!std.mem.eql(u8, fields.next() orelse return null, "@edit")) return null;
+            const start = std.fmt.parseInt(usize, fields.next() orelse return null, 10) catch return null;
+            const end = std.fmt.parseInt(usize, fields.next() orelse return null, 10) catch return null;
+            if (fields.next() != null) return null;
+            edits[n] = .{ .start = start, .end = end };
+            n += 1;
+        }
+        return if (n == 0) null else edits[0..n];
+    }
+
+    fn mapLspEditOffset(edits: []const LspEdit, replacement_len: usize, old: usize) usize {
+        var old_at: usize = 0;
+        var new_at: usize = 0;
+        for (edits) |e| {
+            if (old < e.start) return new_at + (old - old_at);
+            new_at += e.start - old_at;
+            if (old < e.end) return new_at + @min(old - e.start, replacement_len - 1);
+            new_at += replacement_len;
+            if (old == e.end) return new_at;
+            old_at = e.end;
+        }
+        return new_at + (old - old_at);
+    }
+
+    fn applyLspRename(p: *Pardes, pane: *Pane, revision: u32, new_name: []const u8, bytes: []const u8) void {
+        const f = if (pane.file) |*file| file else return;
+        if (f.revision != revision) return;
+        const edits = p.parseLspEdits(bytes) orelse return;
+
+        var removed: usize = 0;
+        var previous_end: usize = 0;
+        for (edits) |e| {
+            if (e.start < previous_end or e.start >= e.end or e.end > f.content.len) return;
+            removed = std.math.add(usize, removed, e.end - e.start) catch return;
+            previous_end = e.end;
+        }
+        if (std.mem.eql(u8, f.content[edits[0].start..edits[0].end], new_name)) return;
+        const added = std.math.mul(usize, edits.len, new_name.len) catch return;
+        const final_len = std.math.add(usize, f.content.len - removed, added) catch return;
+        const replacement = p.gpa.alloc(u8, final_len) catch return;
+
+        const old_cursor = modal.hxOff(f.content, .{
+            .row = @intCast(@max(0, pane.cur_row)),
+            .col = @intCast(@max(0, pane.cur_col)),
+        });
+        const mapped_cursor = mapLspEditOffset(edits, new_name.len, old_cursor);
+        var read_at: usize = 0;
+        var write_at: usize = 0;
+        for (edits) |e| {
+            @memcpy(replacement[write_at .. write_at + (e.start - read_at)], f.content[read_at..e.start]);
+            write_at += e.start - read_at;
+            @memcpy(replacement[write_at .. write_at + new_name.len], new_name);
+            write_at += new_name.len;
+            read_at = e.end;
+        }
+        @memcpy(replacement[write_at..], f.content[read_at..]);
+
+        p.pushUndo(pane);
+        file_pane.setContent(p, f, replacement);
+        const cursor = modal.hxPos(f.content, mapped_cursor);
+        pane.cur_row = @intCast(cursor.row);
+        pane.cur_col = @intCast(cursor.col);
+        pane.vsel.active = false;
+        pane.msel.active = false;
+        pane.select = false;
+        pane.sticky_col = -1;
+        pane.ensureCursorVisible();
+    }
+
+    /// A worker answered. Rename's edit records are consumed first and never
+    /// rendered. Every other response is the ordinary look/output path:
     ///   one row, a goto  -> jump straight there (helix jumps on a single
     ///                       location and shows a picker on several)
     ///   anything else    -> an output buffer, which n/N already steps. That
@@ -7675,6 +7768,8 @@ pub const Pardes = struct {
         if (w.id != id) return; // superseded by a newer press, or the pane died
         p.lsp_wait = null;
         const pane = p.panes[w.pane] orelse return;
+        if (pane.serial != w.serial) return;
+        if (w.kind == .rename) return p.applyLspRename(pane, w.revision, w.arg.slice(), rows);
         if (rows.len == 0) {
             // No rows is a legal answer everywhere except here. Tab DIVERTED
             // instead of indenting, so an empty answer would eat the keystroke
diff --git a/test/snapshots/lsp-rename.golden b/test/snapshots/lsp-rename.golden
new file mode 100644
index 00000000..c15092b7
--- /dev/null
+++ b/test/snapshots/lsp-rename.golden
@@ -0,0 +1,75 @@
+== snap renamed grid=110x24 cursor=17,9
+|New Newcol Find Grep Help Tutor Dump NextColor Debug Kill
+|  /tmp/pardes-snap/lsp-rename/cwd/rename.zig                                  Save New Del
+|     1 const std = @import("std");
+|     2
+|     3 fn renamed_helper(x: u32) u32 {
+|     4     return x + 1;
+|     5 }
+|     6
+|     7 pub fn main() void {
+|     8     _ = renamed_helper(41);
+|     9 }
+|    10
+|    11 fn shadowed() void {
+|    12     const helper: u32 = 7;
+|    13     _ = helper;
+|    14 }
+|    15
+|
+|
+|
+|
+|
+|
+|
+== snap undone grid=110x24 cursor=17,9
+|New Newcol Find Grep Help Tutor Dump NextColor Debug Kill
+|  /tmp/pardes-snap/lsp-rename/cwd/rename.zig                                  Save New Del
+|     1 const std = @import("std");
+|     2
+|     3 fn helper(x: u32) u32 {
+|     4     return x + 1;
+|     5 }
+|     6
+|     7 pub fn main() void {
+|     8     _ = helper(41);
+|     9 }
+|    10
+|    11 fn shadowed() void {
+|    12     const helper: u32 = 7;
+|    13     _ = helper;
+|    14 }
+|    15
+|
+|
+|
+|
+|
+|
+|
+== snap redone grid=110x24 cursor=17,9
+|New Newcol Find Grep Help Tutor Dump NextColor Debug Kill
+|  /tmp/pardes-snap/lsp-rename/cwd/rename.zig                                  Save New Del
+|     1 const std = @import("std");
+|     2
+|     3 fn renamed_helper(x: u32) u32 {
+|     4     return x + 1;
+|     5 }
+|     6
+|     7 pub fn main() void {
+|     8     _ = renamed_helper(41);
+|     9 }
+|    10
+|    11 fn shadowed() void {
+|    12     const helper: u32 = 7;
+|    13     _ = helper;
+|    14 }
+|    15
+|
+|
+|
+|
+|
+|
+|
diff --git a/test/snapshots/lsp-rename.snap b/test/snapshots/lsp-rename.snap
new file mode 100644
index 00000000..c29b001e
--- /dev/null
+++ b/test/snapshots/lsp-rename.snap
@@ -0,0 +1,27 @@
+# Rename end to end: the leader prompt, async ZLS resolution, atomic file edit,
+# shadow filtering, and one-step undo/redo. The source has a same-spelled local
+# which must not be changed when the top-level helper is renamed.
+file rename.zig const std = @import("std");\n\nfn helper(x: u32) u32 {\n    return x + 1;\n}\n\npub fn main() void {\n    _ = helper(41);\n}\n\nfn shadowed() void {\n    const helper: u32 = 7;\n    _ = helper;\n}\n
+start 24 110 rename.zig
+wait 8000 helper(41)
+stable 700 20000
+# Put the cursor inside the top-level call, then use the helix rename binding.
+press left 18 10
+release left 18 10
+key space
+key l
+key r
+stable 400 5000
+text renamed_helper
+key enter
+wait 10000 renamed_helper(41)
+stable 700 15000
+snap renamed
+# Both resolved top-level occurrences changed; the shadowed local stayed helper.
+# The whole asynchronous rename is one history transaction.
+key u
+stable 400 5000
+snap undone
+key U
+stable 400 5000
+snap redone