Skip to content

Commit 8b7b966

Browse files
authored
fix(lsp): fix hover-on-read hook — strip cat-n prefix and correct MCP param (#1541)
The hover-on-read hook was completely non-functional due to two bugs: 1. extract_symbol_positions() applied the symbol regex against the full Read tool output which uses cat-n format (" N\t" prefix). Since the regex is anchored at `^`, no line started with a Rust keyword and the function always returned an empty vec, causing an early return before any MCP calls were made. Fix: iterate lines, detect and strip the digit-only prefix before the first tab, convert the 1-based line number to 0-based for LSP, then apply the regex against the stripped source line. Also removes the now- redundant `(?m)` flag since matching is per-line. 2. The MCP `get_hover` call passed `"path"` instead of `"file_path"`, which is the required parameter name in the mcpls tool schema. Fixes #1532
1 parent 21df630 commit 8b7b966

1 file changed

Lines changed: 83 additions & 9 deletions

File tree

crates/zeph-core/src/lsp_hooks/hover.rs

Lines changed: 83 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,24 +21,37 @@ const MAX_CONCURRENT_HOVER_CALLS: usize = 3;
2121

2222
/// Matches Rust top-level definition lines: `fn`, `struct`, `enum`, `trait`, `impl`, `type`.
2323
static SYMBOL_LINE_RE: LazyLock<Regex> = LazyLock::new(|| {
24-
Regex::new(
25-
r"(?m)^(?:pub(?:\([^)]*\))?\s+)?(?:async\s+)?(?:fn|struct|enum|trait|impl|type)\s+\w",
26-
)
27-
.expect("valid regex")
24+
Regex::new(r"^(?:pub(?:\([^)]*\))?\s+)?(?:async\s+)?(?:fn|struct|enum|trait|impl|type)\s+\w")
25+
.expect("valid regex")
2826
});
2927

3028
/// Extract (`line_number`, `character_offset`) pairs for symbol definitions.
3129
/// Lines and characters are 0-indexed (LSP convention).
30+
///
31+
/// Handles both raw source content and `cat -n` formatted output (` N\t` prefix)
32+
/// emitted by the native `read` tool.
3233
fn extract_symbol_positions(content: &str, max_symbols: usize) -> Vec<(u64, u64)> {
3334
let mut positions = Vec::new();
34-
for m in SYMBOL_LINE_RE.find_iter(content) {
35+
for (raw_idx, raw_line) in content.lines().enumerate() {
3536
if positions.len() >= max_symbols {
3637
break;
3738
}
38-
let line = content[..m.start()].chars().filter(|c| *c == '\n').count() as u64;
39-
let line_start = content[..m.start()].rfind('\n').map_or(0, |p| p + 1);
40-
let character = (m.start() - line_start) as u64;
41-
positions.push((line, character));
39+
// Detect and strip `cat -n` prefix: optional spaces, 1+ digits, one tab.
40+
// The number is 1-based; convert to 0-based for LSP.
41+
let (lsp_line, source_line) = if let Some(tab) = raw_line.find('\t') {
42+
let prefix = raw_line[..tab].trim();
43+
if !prefix.is_empty() && prefix.chars().all(|c| c.is_ascii_digit()) {
44+
let one_based: u64 = prefix.parse().unwrap_or(0);
45+
(one_based.saturating_sub(1), &raw_line[tab + 1..])
46+
} else {
47+
(raw_idx as u64, raw_line)
48+
}
49+
} else {
50+
(raw_idx as u64, raw_line)
51+
};
52+
if let Some(m) = SYMBOL_LINE_RE.find(source_line) {
53+
positions.push((lsp_line, m.start() as u64));
54+
}
4255
}
4356
positions
4457
}
@@ -160,4 +173,65 @@ mod tests {
160173
let positions = extract_symbol_positions("", 10);
161174
assert!(positions.is_empty());
162175
}
176+
177+
#[test]
178+
fn handles_cat_n_prefix() {
179+
// Simulate output from the native `read` tool (cat -n format, 1-based lines).
180+
let src = " 1\t// comment\n 2\tuse std::fmt;\n 30\tpub struct Foo {\n 31\t x: u32,\n 32\t}\n 40\tpub fn bar() {}";
181+
let positions = extract_symbol_positions(src, 10);
182+
assert_eq!(positions.len(), 2);
183+
// `pub struct Foo` is on cat-n line 30 → LSP line 29
184+
assert_eq!(positions[0].0, 29);
185+
// `pub fn bar` is on cat-n line 40 → LSP line 39
186+
assert_eq!(positions[1].0, 39);
187+
}
188+
189+
#[test]
190+
fn cat_n_character_offset_starts_at_zero() {
191+
let src = " 1\tpub fn top() {}";
192+
let positions = extract_symbol_positions(src, 10);
193+
assert_eq!(positions.len(), 1);
194+
assert_eq!(positions[0].0, 0); // LSP line 0
195+
assert_eq!(positions[0].1, 0); // character 0
196+
}
197+
198+
#[test]
199+
fn non_digit_tab_prefix_no_symbol_match() {
200+
// Non-digit prefix before tab: treated as raw source. Symbol regex anchored at `^`
201+
// won't match if the raw line doesn't start with a Rust keyword, so no position extracted.
202+
let src = " abc\tpub fn foo() {}";
203+
let positions = extract_symbol_positions(src, 10);
204+
// The line is not stripped, so SYMBOL_LINE_RE sees " abc\tpub fn foo() {}" and
205+
// anchored ^ matches the indented `abc` prefix — no `pub fn` at start → no match.
206+
assert!(positions.is_empty());
207+
}
208+
209+
#[test]
210+
fn empty_prefix_before_tab_no_symbol_match() {
211+
// Line starting with a tab: empty prefix → not cat-n, treated as raw source.
212+
// Raw line "\tpub fn foo() {}" does not start with a Rust keyword at ^.
213+
let src = "\tpub fn foo() {}";
214+
let positions = extract_symbol_positions(src, 10);
215+
assert!(positions.is_empty());
216+
}
217+
218+
#[test]
219+
fn max_symbols_zero_returns_empty() {
220+
let src = "pub fn a() {}\npub fn b() {}";
221+
let positions = extract_symbol_positions(src, 0);
222+
assert!(positions.is_empty());
223+
}
224+
225+
#[test]
226+
fn mixed_cat_n_and_raw_lines_not_supported() {
227+
// A file with inconsistent format: first line has cat-n prefix, second is raw.
228+
// The function processes each line independently, so each line is handled by its own prefix check.
229+
let src = " 5\tpub struct Baz;\npub fn raw() {}";
230+
let positions = extract_symbol_positions(src, 10);
231+
assert_eq!(positions.len(), 2);
232+
// First line: cat-n line 5 → LSP line 4
233+
assert_eq!(positions[0].0, 4);
234+
// Second line: no tab → raw_idx 1 → LSP line 1
235+
assert_eq!(positions[1].0, 1);
236+
}
163237
}

0 commit comments

Comments
 (0)