Skip to content

Commit 06c300b

Browse files
committed
clamp file_content tool instead of erroring
makes llms retry less
1 parent 2cb01b2 commit 06c300b

3 files changed

Lines changed: 75 additions & 31 deletions

File tree

src/mcp/server.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,7 @@ fn mcp_tools() -> Vec<Value> {
385385
}),
386386
json!({
387387
"name": "file_content",
388-
"description": "Read raw indexed file content (no syntax highlighting) for an exact repo/branch/path from the index. Supports optional start_line/end_line (1-based, inclusive) for snippets to reduce context usage. Use this after file_list/path_search to inspect implementation details; prefer exact paths returned by those tools. Includes branch freshness metadata.",
388+
"description": "Read raw indexed file content (no syntax highlighting) for an exact repo/branch/path from the index. Supports optional start_line/end_line (1-based, inclusive) for snippets to reduce context usage; out-of-bounds line requests are clamped to valid file bounds and reported in the response message. Use this after file_list/path_search to inspect implementation details; prefer exact paths returned by those tools. Includes branch freshness metadata.",
389389
"inputSchema": {
390390
"type": "object",
391391
"properties": {

src/mcp/tools.rs

Lines changed: 72 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ pub async fn execute_file_content(
9494
.map_err(|err| err.to_string())?;
9595

9696
let line_count = raw.content.lines().count();
97-
let (content, snippet, returned_line_count) = slice_file_content(
97+
let (content, snippet, returned_line_count, message) = slice_file_content(
9898
&raw.content,
9999
payload.start_line,
100100
payload.end_line,
@@ -113,6 +113,7 @@ pub async fn execute_file_content(
113113
line_count,
114114
returned_line_count,
115115
snippet,
116+
message,
116117
index_freshness,
117118
})
118119
}
@@ -122,50 +123,57 @@ fn slice_file_content(
122123
start_line: Option<u32>,
123124
end_line: Option<u32>,
124125
total_line_count: usize,
125-
) -> Result<(String, Option<FileContentSnippet>, usize), String> {
126+
) -> Result<(String, Option<FileContentSnippet>, usize, Option<String>), String> {
126127
if start_line.is_none() && end_line.is_none() {
127-
return Ok((content.to_string(), None, total_line_count));
128-
}
129-
130-
let start = start_line.unwrap_or(1);
131-
let end = end_line.unwrap_or(total_line_count as u32);
132-
133-
if start == 0 {
134-
return Err("start_line must be >= 1".to_string());
135-
}
136-
if end == 0 {
137-
return Err("end_line must be >= 1".to_string());
128+
return Ok((content.to_string(), None, total_line_count, None));
138129
}
139130
if total_line_count == 0 {
140131
return Err("cannot request line snippets from an empty file".to_string());
141132
}
142-
if start as usize > total_line_count {
143-
return Err(format!(
144-
"start_line {} exceeds file line count {}",
145-
start, total_line_count
146-
));
147-
}
148-
if end < start {
133+
134+
let max_line = total_line_count as u32;
135+
let requested_start = start_line.unwrap_or(1);
136+
let requested_end = end_line.unwrap_or(max_line);
137+
let clamped_start = requested_start.clamp(1, max_line);
138+
let clamped_end = requested_end.clamp(clamped_start, max_line);
139+
140+
if end_line.is_some() && requested_end >= 1 && requested_end < requested_start {
149141
return Err("end_line must be >= start_line".to_string());
150142
}
151143

152-
let bounded_end = end.min(total_line_count as u32);
153-
let start_idx = (start - 1) as usize;
154-
let count = (bounded_end - start + 1) as usize;
144+
let start_idx = (clamped_start - 1) as usize;
145+
let count = (clamped_end - clamped_start + 1) as usize;
155146
let snippet_content = content
156147
.lines()
157148
.skip(start_idx)
158149
.take(count)
159150
.collect::<Vec<_>>()
160151
.join("\n");
152+
let was_clamped = requested_start != clamped_start || requested_end != clamped_end;
153+
let message = was_clamped.then(|| match (start_line, end_line) {
154+
(Some(_), Some(_)) => format!(
155+
"Requested line range {}-{} was clamped to {}-{} for file length {}.",
156+
requested_start, requested_end, clamped_start, clamped_end, total_line_count
157+
),
158+
(Some(_), None) => format!(
159+
"Requested start_line {} was clamped to {} for file length {}.",
160+
requested_start, clamped_start, total_line_count
161+
),
162+
(None, Some(_)) => format!(
163+
"Requested end_line {} was clamped to {} for file length {}.",
164+
requested_end, clamped_end, total_line_count
165+
),
166+
(None, None) => unreachable!("unbounded requests return early"),
167+
});
161168

162169
Ok((
163170
snippet_content,
164171
Some(FileContentSnippet {
165-
start_line: start,
166-
end_line: bounded_end,
172+
start_line: clamped_start,
173+
end_line: clamped_end,
167174
}),
168175
count,
176+
message,
169177
))
170178
}
171179

@@ -1376,29 +1384,63 @@ mod tests {
13761384
#[test]
13771385
fn slice_file_content_returns_full_content_without_bounds() {
13781386
let content = "a\nb\nc\n";
1379-
let (sliced, snippet, returned_line_count) =
1387+
let (sliced, snippet, returned_line_count, message) =
13801388
slice_file_content(content, None, None, 3).expect("slice should succeed");
13811389
assert_eq!(sliced, content);
13821390
assert!(snippet.is_none());
13831391
assert_eq!(returned_line_count, 3);
1392+
assert!(message.is_none());
13841393
}
13851394

13861395
#[test]
13871396
fn slice_file_content_returns_requested_snippet() {
13881397
let content = "a\nb\nc\nd";
1389-
let (sliced, snippet, returned_line_count) =
1398+
let (sliced, snippet, returned_line_count, message) =
13901399
slice_file_content(content, Some(2), Some(3), 4).expect("slice should succeed");
13911400
assert_eq!(sliced, "b\nc");
13921401
assert_eq!(returned_line_count, 2);
13931402
let snippet = snippet.expect("snippet metadata must be present");
13941403
assert_eq!(snippet.start_line, 2);
13951404
assert_eq!(snippet.end_line, 3);
1405+
assert!(message.is_none());
1406+
}
1407+
1408+
#[test]
1409+
fn slice_file_content_clamps_out_of_bounds_request() {
1410+
let content = "a\nb\nc";
1411+
let (sliced, snippet, returned_line_count, message) =
1412+
slice_file_content(content, Some(4), None, 3).expect("must clamp");
1413+
assert_eq!(sliced, "c");
1414+
assert_eq!(returned_line_count, 1);
1415+
let snippet = snippet.expect("snippet metadata must be present");
1416+
assert_eq!(snippet.start_line, 3);
1417+
assert_eq!(snippet.end_line, 3);
1418+
assert_eq!(
1419+
message.as_deref(),
1420+
Some("Requested start_line 4 was clamped to 3 for file length 3.")
1421+
);
1422+
}
1423+
1424+
#[test]
1425+
fn slice_file_content_clamps_zero_bounds() {
1426+
let content = "a\nb\nc";
1427+
let (sliced, snippet, returned_line_count, message) =
1428+
slice_file_content(content, Some(0), Some(0), 3).expect("must clamp");
1429+
assert_eq!(sliced, "a");
1430+
assert_eq!(returned_line_count, 1);
1431+
let snippet = snippet.expect("snippet metadata must be present");
1432+
assert_eq!(snippet.start_line, 1);
1433+
assert_eq!(snippet.end_line, 1);
1434+
assert_eq!(
1435+
message.as_deref(),
1436+
Some("Requested line range 0-0 was clamped to 1-1 for file length 3.")
1437+
);
13961438
}
13971439

13981440
#[test]
1399-
fn slice_file_content_rejects_invalid_bounds() {
1441+
fn slice_file_content_rejects_reversed_in_range_bounds() {
14001442
let content = "a\nb\nc";
1401-
let err = slice_file_content(content, Some(4), None, 3).expect_err("must reject");
1402-
assert!(err.contains("exceeds file line count"));
1443+
let err = slice_file_content(content, Some(3), Some(2), 3).expect_err("must reject");
1444+
assert!(err.contains("end_line must be >= start_line"));
14031445
}
14041446
}

src/mcp/types.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,8 @@ pub struct FileContentToolResponse {
196196
pub returned_line_count: usize,
197197
#[serde(skip_serializing_if = "Option::is_none")]
198198
pub snippet: Option<FileContentSnippet>,
199+
#[serde(skip_serializing_if = "Option::is_none")]
200+
pub message: Option<String>,
199201
pub index_freshness: IndexFreshness,
200202
}
201203

0 commit comments

Comments
 (0)