diff --git a/crates/pi-natives/src/chunk/ast_ipynb.rs b/crates/pi-natives/src/chunk/ast_ipynb.rs index 054e26510..2b69604d8 100644 --- a/crates/pi-natives/src/chunk/ast_ipynb.rs +++ b/crates/pi-natives/src/chunk/ast_ipynb.rs @@ -576,6 +576,7 @@ pub fn build_notebook_tree_from_virtual( checksum: root_checksum, line_count: total_lines as u32, parse_errors: 0, + parse_error_lines: Vec::new(), fallback: false, root_path: String::new(), root_children, diff --git a/crates/pi-natives/src/chunk/edit.rs b/crates/pi-natives/src/chunk/edit.rs index 104185245..3a4106bb8 100644 --- a/crates/pi-natives/src/chunk/edit.rs +++ b/crates/pi-natives/src/chunk/edit.rs @@ -169,35 +169,42 @@ pub fn apply_edits(state: &ChunkState, params: &EditParams) -> Result"), + )); + } + if error_summaries.is_empty() + && let Some(scheduled) = last_scheduled.as_ref() + { let chunk_label = scheduled .initial_chunk .as_ref() .map(|c| c.path.as_str()) .or(scheduled.requested_selector.as_deref()) .unwrap_or(""); - if let Some(chunk) = scheduled.initial_chunk.as_ref() { - vec![format!( - "L{}-L{} parse error introduced while editing {} (chunk spans file lines {}-{})", - chunk.start_line, chunk.end_line, chunk_label, chunk.start_line, chunk.end_line - )] - } else { - vec![format!("Parse error introduced while editing {chunk_label}")] - } - } else { - Vec::new() + error_summaries.push(format!("Parse error introduced while editing {chunk_label}")); } - } else { - error_summaries - }; - let details = if fallback_summary.is_empty() { + } + let details = if error_summaries.is_empty() { String::new() } else { format!( "\nParse errors:\n{}", - fallback_summary + error_summaries .into_iter() .map(|summary| format!("- {summary}")) .collect::>() @@ -2035,16 +2042,26 @@ fn render_error_context( anchor_style: Option, normalize_indent: bool, ) -> String { - let Ok(ParsedSelector { selector: clean_path, .. }) = - split_selector_crc_and_region(selector, None, None) - else { - return String::new(); + // When an edit introduces a parse error we render the full chunk tree + // (no focus) so the agent can see the failure location and surrounding + // context in a single response, without a follow-up read. For non-parse + // failures (a single operation rejected before parse validation) the + // error is local to the targeted chunk, so we keep a narrow focus. + let has_parse_errors = !state.tree().parse_error_lines.is_empty(); + let focused_paths = if has_parse_errors { + None + } else { + let Ok(ParsedSelector { selector: clean_path, .. }) = + split_selector_crc_and_region(selector, None, None) + else { + return String::new(); + }; + let mut ignored = Vec::new(); + let Ok(chunk) = resolve_chunk_selector(state, clean_path.as_deref(), &mut ignored) else { + return String::new(); + }; + compute_focus(state.tree(), std::slice::from_ref(&chunk.path)) }; - let mut ignored = Vec::new(); - let Ok(chunk) = resolve_chunk_selector(state, clean_path.as_deref(), &mut ignored) else { - return String::new(); - }; - let focused_paths = compute_focus(state.tree(), std::slice::from_ref(&chunk.path)); let tab_replacement = if normalize_indent { NORMALIZED_TAB_REPLACEMENT } else { @@ -2649,6 +2666,72 @@ mod tests { ); } + #[test] + fn error_context_expands_around_parse_failure_in_other_chunk() { + // Regression: when an edit introduces a parse error in a chunk other + // than the edit target, the error-message "Fresh content" view must + // expand around the error location, not only around the targeted chunk. + // Previously the focus only covered the targeted chunk, so the parse + // error could land inside a truncated region and force the agent to + // do a follow-up read to diagnose the failure. + // + // Construct a file with five top-level functions. Edit fn_a's body with + // unbalanced-brace content so the parser bleeds the error into fn_c's + // territory. After the fix, the error message must include identifying + // text from the chunks flagged with parse errors, even though only + // fn_a was the edit target. + let source = concat!( + "fn alpha() {\n let x = 1;\n}\n\n", + "fn bravo() {\n let y = 2;\n}\n\n", + "fn charlie() {\n let z = 3;\n}\n\n", + "fn delta() {\n let w = 4;\n}\n\n", + "fn echo() {\n let v = 5;\n}\n", + ); + let state = parsed_state_for(source, "rust"); + let alpha = state.inner().chunk("fn_alpha").expect("fn_alpha"); + + let Err(err) = apply_edits(&state, &EditParams { + operations: vec![EditOperation { + op: ChunkEditOp::Replace, + sel: Some("fn_alpha".to_owned()), + crc: Some(alpha.checksum.clone()), + region: Some(ChunkRegion::Body), + // Intentionally broken: dangling `{` consumes subsequent + // top-level functions until tree-sitter gives up. + content: Some("let broken = { { {\n".to_owned()), + find: None, + }], + default_selector: None, + default_crc: None, + anchor_style: Some(ChunkAnchorStyle::Full), + cwd: ".".to_owned(), + file_path: "test.rs".to_owned(), + normalize_indent: None, + }) else { + panic!("edit should be rejected due to parse error"); + }; + + assert!( + err.contains("Edit rejected: introduced"), + "should be a parse-error rejection: {err}", + ); + assert!(err.contains("Fresh content:"), "should include fresh content: {err}"); + // The edit target must always be visible. + assert!(err.contains("fn_alpha"), "target chunk should be in focus: {err}"); + // The fix: at least one of the downstream chunks where the parse error + // lands should also appear in the focused "Fresh content" view. Without + // the fix, focus only covers fn_alpha and these downstream anchors are + // skipped, so the agent cannot see the failure location. + let downstream_visible = ["fn_bravo", "fn_charli", "fn_delta", "fn_echo"] + .iter() + .any(|name| err.contains(name)); + assert!( + downstream_visible, + "error-context focus should include at least one downstream chunk where the parse \ + failure lands, got: {err}", + ); + } + #[test] fn append_on_root_stmts_group_inserts_after_grouped_statements() { // This fixture exposes the root-level `stmts` group. Appending to that diff --git a/crates/pi-natives/src/chunk/indent.rs b/crates/pi-natives/src/chunk/indent.rs index 9482bf93d..b8b99ee45 100644 --- a/crates/pi-natives/src/chunk/indent.rs +++ b/crates/pi-natives/src/chunk/indent.rs @@ -547,14 +547,15 @@ mod tests { #[test] fn detect_file_indent_step_prefers_space_children() { let tree = ChunkTree { - language: "rust".to_owned(), - checksum: "ABCD".to_owned(), - line_count: 1, - parse_errors: 0, - fallback: false, - root_path: String::new(), - root_children: vec!["class_A".to_owned()], - chunks: vec![ + language: "rust".to_owned(), + checksum: "ABCD".to_owned(), + line_count: 1, + parse_errors: 0, + parse_error_lines: Vec::new(), + fallback: false, + root_path: String::new(), + root_children: vec!["class_A".to_owned()], + chunks: vec![ chunk("class_A", Some(""), &["fn_b"], 0, " "), chunk("fn_b", Some("class_A"), &[], 2, " "), ], @@ -567,14 +568,15 @@ mod tests { // When the chunk tree has no parent->child pairs (all leaves), // fall back to scanning source lines for the minimum indent width. let tree = ChunkTree { - language: "yaml".to_owned(), - checksum: "ABCD".to_owned(), - line_count: 3, - parse_errors: 0, - fallback: false, - root_path: String::new(), - root_children: vec!["key_server".to_owned()], - chunks: vec![chunk("key_server", Some(""), &[], 0, " ")], + language: "yaml".to_owned(), + checksum: "ABCD".to_owned(), + line_count: 3, + parse_errors: 0, + parse_error_lines: Vec::new(), + fallback: false, + root_path: String::new(), + root_children: vec!["key_server".to_owned()], + chunks: vec![chunk("key_server", Some(""), &[], 0, " ")], }; let source = "server:\n host: localhost\n port: 5432\n"; assert_eq!(detect_file_indent_step(source, &tree), 2); @@ -583,14 +585,15 @@ mod tests { #[test] fn detect_file_indent_char_falls_back_to_source_lines() { let tree = ChunkTree { - language: "rust".to_owned(), - checksum: "ABCD".to_owned(), - line_count: 3, - parse_errors: 0, - fallback: false, - root_path: String::new(), - root_children: vec!["fn_main".to_owned()], - chunks: vec![chunk("fn_main", Some(""), &[], 0, "")], + language: "rust".to_owned(), + checksum: "ABCD".to_owned(), + line_count: 3, + parse_errors: 0, + parse_error_lines: Vec::new(), + fallback: false, + root_path: String::new(), + root_children: vec!["fn_main".to_owned()], + chunks: vec![chunk("fn_main", Some(""), &[], 0, "")], }; let source = "fn main() {\n println!(\"hi\");\n}\n"; @@ -600,14 +603,15 @@ mod tests { #[test] fn detect_file_indent_char_defaults_to_spaces_without_signal() { let tree = ChunkTree { - language: "rust".to_owned(), - checksum: "ABCD".to_owned(), - line_count: 0, - parse_errors: 0, - fallback: false, - root_path: String::new(), - root_children: Vec::new(), - chunks: Vec::new(), + language: "rust".to_owned(), + checksum: "ABCD".to_owned(), + line_count: 0, + parse_errors: 0, + parse_error_lines: Vec::new(), + fallback: false, + root_path: String::new(), + root_children: Vec::new(), + chunks: Vec::new(), }; assert_eq!(detect_file_indent_char("", &tree), ' '); diff --git a/crates/pi-natives/src/chunk/mod.rs b/crates/pi-natives/src/chunk/mod.rs index e3026bff4..3af39651d 100644 --- a/crates/pi-natives/src/chunk/mod.rs +++ b/crates/pi-natives/src/chunk/mod.rs @@ -119,7 +119,7 @@ pub(crate) fn build_chunk_tree(source: &str, language: &str) -> Result Result) -> bool { // ── Utility ────────────────────────────────────────────────────────────── -fn count_parse_errors(node: Node<'_>) -> usize { - let mut count = usize::from(node.is_error() || node.is_missing() || node.kind() == "ERROR"); - for child in named_children(node) { - count += count_parse_errors(child); +fn count_parse_errors(node: Node<'_>) -> (usize, Vec) { + let mut count = 0; + let mut lines = Vec::new(); + collect_parse_errors(node, &mut count, &mut lines); + lines.sort_unstable(); + lines.dedup(); + (count, lines) +} + +fn collect_parse_errors(node: Node<'_>, count: &mut usize, lines: &mut Vec) { + if node.is_error() || node.is_missing() || node.kind() == "ERROR" { + *count += 1; + lines.push(node.start_position().row as u32 + 1); + } + for child in named_children(node) { + collect_parse_errors(child, count, lines); } - count } fn resolve_chunk_lang(language: &str) -> Option { diff --git a/crates/pi-natives/src/chunk/resolve.rs b/crates/pi-natives/src/chunk/resolve.rs index 84824a9c5..5cac2bd7a 100644 --- a/crates/pi-natives/src/chunk/resolve.rs +++ b/crates/pi-natives/src/chunk/resolve.rs @@ -922,14 +922,15 @@ mod tests { fn state_for_resolution() -> ChunkStateInner { ChunkStateInner::new(String::new(), "typescript".to_owned(), ChunkTree { - language: "typescript".to_owned(), - checksum: "ROOT".to_owned(), - line_count: 1, - parse_errors: 0, - fallback: false, - root_path: String::new(), - root_children: vec!["fn_handleTerraform".to_owned()], - chunks: vec![ + language: "typescript".to_owned(), + checksum: "ROOT".to_owned(), + line_count: 1, + parse_errors: 0, + parse_error_lines: Vec::new(), + fallback: false, + root_path: String::new(), + root_children: vec!["fn_handleTerraform".to_owned()], + chunks: vec![ chunk("", "ROOT", None, vec!["fn_handleTerraform"]), chunk("fn_handleTerraform", "HVJB", Some(""), vec!["fn_handleTerraform.try"]), chunk("fn_handleTerraform.try", "RQPB", Some("fn_handleTerraform"), vec![ @@ -978,14 +979,15 @@ mod tests { #[test] fn resolves_stale_selector_by_same_parent_checksum() { let state = ChunkStateInner::new(String::new(), "typescript".to_owned(), ChunkTree { - language: "typescript".to_owned(), - checksum: "ROOT".to_owned(), - line_count: 1, - parse_errors: 0, - fallback: false, - root_path: String::new(), - root_children: vec!["fn_run".to_owned()], - chunks: vec![ + language: "typescript".to_owned(), + checksum: "ROOT".to_owned(), + line_count: 1, + parse_errors: 0, + parse_error_lines: Vec::new(), + fallback: false, + root_path: String::new(), + root_children: vec!["fn_run".to_owned()], + chunks: vec![ chunk("", "ROOT", None, vec!["fn_run"]), chunk("fn_run", "RUNN", Some(""), vec!["fn_run.var_effect_1", "fn_run.var_effect_2"]), chunk("fn_run.var_effect_1", "AAAA", Some("fn_run"), vec![]), @@ -1007,14 +1009,15 @@ mod tests { #[test] fn stale_selector_prefers_best_leaf_name_when_crc_matches_multiple_siblings() { let state = ChunkStateInner::new(String::new(), "typescript".to_owned(), ChunkTree { - language: "typescript".to_owned(), - checksum: "ROOT".to_owned(), - line_count: 1, - parse_errors: 0, - fallback: false, - root_path: String::new(), - root_children: vec!["fn_run".to_owned()], - chunks: vec![ + language: "typescript".to_owned(), + checksum: "ROOT".to_owned(), + line_count: 1, + parse_errors: 0, + parse_error_lines: Vec::new(), + fallback: false, + root_path: String::new(), + root_children: vec!["fn_run".to_owned()], + chunks: vec![ chunk("", "ROOT", None, vec!["fn_run"]), chunk("fn_run", "RUNN", Some(""), vec!["fn_run.var_other", "fn_run.var_effect_1"]), chunk("fn_run.var_other", "BBBB", Some("fn_run"), vec![]), @@ -1031,14 +1034,15 @@ mod tests { #[test] fn stale_selector_fails_closed_when_same_parent_crc_matches_are_ambiguous() { let state = ChunkStateInner::new(String::new(), "typescript".to_owned(), ChunkTree { - language: "typescript".to_owned(), - checksum: "ROOT".to_owned(), - line_count: 1, - parse_errors: 0, - fallback: false, - root_path: String::new(), - root_children: vec!["fn_run".to_owned()], - chunks: vec![ + language: "typescript".to_owned(), + checksum: "ROOT".to_owned(), + line_count: 1, + parse_errors: 0, + parse_error_lines: Vec::new(), + fallback: false, + root_path: String::new(), + root_children: vec!["fn_run".to_owned()], + chunks: vec![ chunk("", "ROOT", None, vec!["fn_run"]), chunk("fn_run", "RUNN", Some(""), vec!["fn_run.var_effect_1", "fn_run.var_effect_2"]), chunk("fn_run.var_effect_1", "BBBB", Some("fn_run"), vec![]), @@ -1057,14 +1061,15 @@ mod tests { #[test] fn resolves_full_untruncated_identifier_paths() { let state = ChunkStateInner::new(String::new(), "typescript".to_owned(), ChunkTree { - language: "typescript".to_owned(), - checksum: "ROOT".to_owned(), - line_count: 1, - parse_errors: 0, - fallback: false, - root_path: String::new(), - root_children: vec!["class_Server".to_owned()], - chunks: vec![ + language: "typescript".to_owned(), + checksum: "ROOT".to_owned(), + line_count: 1, + parse_errors: 0, + parse_error_lines: Vec::new(), + fallback: false, + root_path: String::new(), + root_children: vec!["class_Server".to_owned()], + chunks: vec![ chunk("", "ROOT", None, vec!["class_Server"]), chunk("class_Server", "CLSS", Some(""), vec!["class_Server.fn_handle"]), chunk("class_Server.fn_handle", "ABCD", Some("class_Server"), vec![]), diff --git a/crates/pi-natives/src/chunk/types.rs b/crates/pi-natives/src/chunk/types.rs index 0f9625f04..a2e219559 100644 --- a/crates/pi-natives/src/chunk/types.rs +++ b/crates/pi-natives/src/chunk/types.rs @@ -45,14 +45,18 @@ pub struct ChunkNode { #[derive(Clone)] pub struct ChunkTree { - pub language: String, - pub checksum: String, - pub line_count: u32, - pub parse_errors: u32, - pub fallback: bool, - pub root_path: String, - pub root_children: Vec, - pub chunks: Vec, + pub language: String, + pub checksum: String, + pub line_count: u32, + pub parse_errors: u32, + /// 1-indexed line numbers of tree-sitter ERROR / MISSING nodes surfaced + /// during parsing. Used to focus error messages around failure locations + /// when an edit introduces a parse error far from the edit target. + pub parse_error_lines: Vec, + pub fallback: bool, + pub root_path: String, + pub root_children: Vec, + pub chunks: Vec, } /// Summary of a single chunk node for tool output and navigation. diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 7b1ed8f68..f1651e818 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,6 +1,9 @@ # Changelog ## [Unreleased] +### Added + +- Added support for `computeHashlineDiff` to accept hashline edits with `loc` and `content` payloads without requiring pre-resolved `op` fields ### Changed diff --git a/packages/coding-agent/src/edit/modes/hashline.ts b/packages/coding-agent/src/edit/modes/hashline.ts index a68b75e06..e1a53d7ba 100644 --- a/packages/coding-agent/src/edit/modes/hashline.ts +++ b/packages/coding-agent/src/edit/modes/hashline.ts @@ -180,6 +180,26 @@ function resolveEditAnchors(edits: HashlineToolEdit[]): HashlineEdit[] { return edits.map(resolveEditAnchor); } +type HashlineEditInput = HashlineToolEdit | HashlineEdit; + +function resolveHashlineEditsForDiff(edits: HashlineEditInput[]): HashlineEdit[] { + return edits.map((edit, editIndex) => { + if (!edit || typeof edit !== "object") { + throw new Error(`Invalid hashline edit at index ${editIndex}: expected object.`); + } + + if ("op" in edit) { + return edit; + } + + if ("loc" in edit) { + return resolveEditAnchor(edit); + } + + throw new Error(`Invalid hashline edit at index ${editIndex}: expected op/loc payload.`); + }); +} + function tryParseTag(raw: string): Anchor | undefined { try { return parseTag(raw); @@ -1126,7 +1146,7 @@ export function buildCompactHashlineDiffPreview( } export async function computeHashlineDiff( - input: { path: string; edits: HashlineEdit[]; move?: string }, + input: { path: string; edits: HashlineEditInput[]; move?: string }, cwd: string, ): Promise< | { @@ -1143,6 +1163,7 @@ export async function computeHashlineDiff( const isMoveOnly = Boolean(movePath) && movePath !== absolutePath && edits.length === 0; try { + const resolvedEdits = resolveHashlineEditsForDiff(edits); const file = Bun.file(absolutePath); if (movePath === absolutePath) { @@ -1156,7 +1177,7 @@ export async function computeHashlineDiff( const { text: content } = stripBom(rawContent); const normalizedContent = normalizeToLF(content); - const result = applyHashlineEdits(normalizedContent, edits); + const result = applyHashlineEdits(normalizedContent, resolvedEdits); if (normalizedContent === result.lines && !move) { return { error: `No changes would be made to ${path}. The edits produce identical content.` }; } diff --git a/packages/coding-agent/test/edit-diff.test.ts b/packages/coding-agent/test/edit-diff.test.ts index a2e4aba8e..67670ad1c 100644 --- a/packages/coding-agent/test/edit-diff.test.ts +++ b/packages/coding-agent/test/edit-diff.test.ts @@ -240,6 +240,20 @@ describe("computeHashlineDiff", () => { } }); + test("accepts hashline tool edits without resolved op/lines", async () => { + const sourcePath = path.join(tempDir, "source.txt"); + await Bun.write(sourcePath, "first\n"); + + const result = await computeHashlineDiff( + { path: sourcePath, edits: [{ loc: "append", content: "second" }] }, + tempDir, + ); + expect("diff" in result).toBe(true); + if ("diff" in result) { + expect(result.diff).toContain("second"); + } + }); + test("allows move-only operation when content is unchanged", async () => { const sourcePath = path.join(tempDir, "source.txt"); await Bun.write(sourcePath, "unchanged content\n");