diff --git a/crates/pi-natives/src/chunk/ast_go.rs b/crates/pi-natives/src/chunk/ast_go.rs index 2b5a5496a..3355eaddb 100644 --- a/crates/pi-natives/src/chunk/ast_go.rs +++ b/crates/pi-natives/src/chunk/ast_go.rs @@ -14,14 +14,14 @@ pub struct GoClassifier; const ROOT_RULES: &[super::classify::SemanticRule] = &[ // ── Imports / package ── semantic_rule( - "import_declaration", - ChunkKind::Imports, - RuleStyle::Group, - NamingMode::None, + "package_clause", + ChunkKind::Module, + RuleStyle::Named, + NamingMode::AutoIdentifier, RecurseMode::None, ), semantic_rule( - "package_clause", + "import_declaration", ChunkKind::Imports, RuleStyle::Group, NamingMode::None, diff --git a/crates/pi-natives/src/chunk/ast_markup.rs b/crates/pi-natives/src/chunk/ast_markup.rs index c738b620b..ce24aeab5 100644 --- a/crates/pi-natives/src/chunk/ast_markup.rs +++ b/crates/pi-natives/src/chunk/ast_markup.rs @@ -3,9 +3,11 @@ use tree_sitter::Node; use super::{ + chunk_checksum, classify::{ClassifierTables, LangClassifier}, common::*, kind::ChunkKind, + types::ChunkNode, }; use crate::language::SupportLang; @@ -93,6 +95,15 @@ impl LangClassifier for MarkupClassifier { _ => None, } } + + fn post_process( + &self, + chunks: &mut Vec, + _root_children: &mut Vec, + source: &str, + ) { + add_markdown_table_row_chunks(chunks, source); + } } const fn force_container(mut candidate: RawChunkCandidate<'_>) -> RawChunkCandidate<'_> { @@ -121,6 +132,131 @@ fn classify_html_block<'t>(node: Node<'t>, source: &str) -> RawChunkCandidate<'t with_injected_subtree(candidate, SupportLang::Html, node) } +fn add_markdown_table_row_chunks(chunks: &mut Vec, source: &str) { + let original_len = chunks.len(); + let mut additions = Vec::<(usize, Vec)>::new(); + for (index, chunk) in chunks.iter().enumerate().take(original_len) { + if !chunk.children.is_empty() || matches!(chunk.kind, ChunkKind::Code | ChunkKind::Html) { + continue; + } + let rows = markdown_table_rows_for_chunk(source, chunk); + if rows.len() < 2 + || !rows + .iter() + .any(|row| markdown_table_separator_row(row.text)) + { + continue; + } + let nodes = rows + .into_iter() + .enumerate() + .map(|(row_index, row)| { + let identifier = (row_index + 1).to_string(); + let path = format!("{}.row_{}", chunk.path, identifier); + let (indent, indent_char) = detect_indent(source, row.start_byte); + let row_source = source.get(row.start_byte..row.end_byte).unwrap_or_default(); + ChunkNode { + path, + identifier: Some(identifier), + kind: ChunkKind::Row, + leaf: true, + virtual_content: None, + parent_path: Some(chunk.path.clone()), + children: Vec::new(), + signature: Some(row.text.trim().to_owned()), + start_line: row.line, + end_line: row.line, + line_count: 1, + start_byte: row.start_byte as u32, + end_byte: row.end_byte as u32, + checksum_start_byte: row.start_byte as u32, + prologue_end_byte: None, + epilogue_start_byte: None, + checksum: chunk_checksum(row_source.as_bytes()), + error: false, + indent, + indent_char, + group: false, + } + }) + .collect(); + additions.push((index, nodes)); + } + + for (index, nodes) in additions { + let child_paths = nodes.iter().map(|node| node.path.clone()).collect(); + chunks[index].leaf = false; + chunks[index].children = child_paths; + chunks.extend(nodes); + } +} + +struct MarkdownTableRow<'a> { + line: u32, + start_byte: usize, + end_byte: usize, + text: &'a str, +} + +fn markdown_table_rows_for_chunk<'a>( + source: &'a str, + chunk: &ChunkNode, +) -> Vec> { + let line_offsets = source_line_offsets(source); + let mut rows = Vec::new(); + let mut saw_non_empty = false; + for line in chunk.start_line..=chunk.end_line { + let Some((start_byte, end_byte)) = line_bounds(source, &line_offsets, line) else { + continue; + }; + let text = source + .get(start_byte..end_byte) + .unwrap_or_default() + .trim_end_matches('\n'); + if text.trim().is_empty() { + continue; + } + saw_non_empty = true; + if !markdown_table_row(text) { + return Vec::new(); + } + rows.push(MarkdownTableRow { line, start_byte, end_byte, text }); + } + if saw_non_empty { rows } else { Vec::new() } +} + +fn source_line_offsets(source: &str) -> Vec { + let mut offsets = vec![0usize]; + for (index, ch) in source.char_indices() { + if ch == '\n' { + offsets.push(index + 1); + } + } + offsets +} + +fn line_bounds(source: &str, offsets: &[usize], line: u32) -> Option<(usize, usize)> { + if line == 0 { + return None; + } + let start = *offsets.get((line - 1) as usize)?; + let end = offsets.get(line as usize).copied().unwrap_or(source.len()); + Some((start, end)) +} + +fn markdown_table_row(line: &str) -> bool { + let trimmed = line.trim(); + trimmed.starts_with('|') && trimmed.ends_with('|') && trimmed.matches('|').count() >= 2 +} + +fn markdown_table_separator_row(line: &str) -> bool { + let trimmed = line.trim(); + trimmed.contains('-') + && trimmed + .chars() + .all(|ch| matches!(ch, '|' | '-' | ':' | ' ' | '\t')) +} + /// Extract heading text from a Markdown `section` node's `atx_heading` or /// `setext_heading` child. fn extract_markdown_heading(node: Node<'_>, source: &str) -> Option { diff --git a/crates/pi-natives/src/chunk/common.rs b/crates/pi-natives/src/chunk/common.rs index d320421ef..515261b51 100644 --- a/crates/pi-natives/src/chunk/common.rs +++ b/crates/pi-natives/src/chunk/common.rs @@ -654,7 +654,8 @@ fn summarize_function_node( raw_signature: &str, ) -> String { let name = identifier.unwrap_or_else(|| kind.prefix()); - let tail = function_signature(raw_signature) + let tail = go_method_signature(raw_signature, identifier) + .or_else(|| function_signature(raw_signature)) .or_else(|| python_function_signature(raw_signature)) .or_else(|| rust_function_signature(raw_signature)) .unwrap_or_else(|| raw_signature.to_string()); @@ -727,6 +728,46 @@ fn function_signature(header: &str) -> Option { } } +fn go_method_signature(header: &str, identifier: Option<&str>) -> Option { + let name = identifier?; + let declaration = header + .trim() + .trim_end_matches('{') + .trim_end_matches(';') + .trim(); + let rest = declaration.strip_prefix("func")?.trim_start(); + if !rest.starts_with('(') { + return None; + } + let receiver_end = find_matching_paren(rest, 0)?; + let after_receiver = rest.get(receiver_end + 1..)?.trim_start(); + let after_name = after_receiver.strip_prefix(name)?.trim_start(); + if !after_name.starts_with('(') { + return None; + } + Some(after_name.to_string()) +} + +fn find_matching_paren(text: &str, open_index: usize) -> Option { + let mut depth = 0usize; + for (index, ch) in text + .char_indices() + .skip_while(|(index, _)| *index < open_index) + { + match ch { + '(' => depth = depth.saturating_add(1), + ')' => { + depth = depth.checked_sub(1)?; + if depth == 0 { + return Some(index); + } + }, + _ => {}, + } + } + None +} + fn python_function_signature(header: &str) -> Option { let start = header.find('(')?; let end = header.rfind(':').unwrap_or(header.len()); diff --git a/crates/pi-natives/src/chunk/edit.rs b/crates/pi-natives/src/chunk/edit.rs index 38b7196fe..40a4c6f0d 100644 --- a/crates/pi-natives/src/chunk/edit.rs +++ b/crates/pi-natives/src/chunk/edit.rs @@ -235,10 +235,12 @@ pub fn apply_edits(state: &ChunkState, params: &EditParams) -> Result Result Result Result String { - if replacement.contains('\n') { - return replacement.to_string(); - } - +fn reindent_replacement( + original: &str, + replacement: &str, + file_indent_char: char, + file_indent_step: usize, +) -> String { let orig_indent = original .lines() - .next() - .map_or("", |l| &l[..l.len() - l.trim_start().len()]); - let repl_indent = replacement - .lines() - .find(|l| !l.trim().is_empty()) - .map_or("", |l| &l[..l.len() - l.trim_start().len()]); - - if orig_indent == repl_indent { - return replacement.to_string(); - } - - replacement - .lines() - .enumerate() - .map(|(i, line)| { - if line.trim().is_empty() { - line.to_string() - } else if i == 0 { - format!("{orig_indent}{}", line.trim_start()) - } else { - let stripped = line.strip_prefix(repl_indent).unwrap_or(line); - format!("{orig_indent}{stripped}") - } - }) + .find(|line| !line.trim().is_empty()) + .map_or("", |line| &line[..line.len() - line.trim_start().len()]); + let normalized = normalize_chunk_source(replacement) + .split('\n') + .map(|line| denormalize_from_tabs(line, file_indent_char, file_indent_step)) .collect::>() - .join("\n") + .join("\n"); + let dedented = dedent_python_style(&normalized); + indent_non_empty_lines(&dedented, orig_indent) } fn expanded_match_start_for_multiline_replacement( @@ -863,12 +867,29 @@ fn find_indent_normalized(haystack: &str, needle: &str) -> Option<(usize, usize) } } +fn find_replace_region_range( + state: &ChunkStateInner, + anchor: &ChunkNode, + region: Option, +) -> (usize, usize) { + match region { + Some(r) => chunk_region_range(anchor, r), + None if state.language == "rust" && anchor.kind == ChunkKind::Variant => { + let offsets = line_offsets(&state.source); + (anchor.start_byte as usize, line_end_offset(&offsets, anchor.end_line, &state.source)) + }, + None => (anchor.start_byte as usize, anchor.end_byte as usize), + } +} + fn apply_find_replace( state: &mut ChunkStateInner, operation: &EditOperation, scheduled: &ScheduledEditOperation, default_selector: Option<&str>, default_crc: Option<&str>, + file_indent_step: usize, + file_indent_char: char, normalize_indent: bool, touched_paths: &mut Vec, current_batch_targets: &CurrentBatchTargets, @@ -886,10 +907,7 @@ fn apply_find_replace( let anchor = target.chunk; let target_before = anchor.clone(); - let (region_start, region_end) = match target.region { - None => (anchor.start_byte as usize, anchor.end_byte as usize), - Some(r) => chunk_region_range(&anchor, r), - }; + let (region_start, region_end) = find_replace_region_range(state, &anchor, target.region); let find = operation.find.as_deref().unwrap_or_default(); if find.is_empty() { @@ -939,7 +957,7 @@ fn apply_find_replace( // indent normalization is active. let matched_source = &state.source[abs_start..abs_end]; let replacement = if normalize_indent { - reindent_replacement(matched_source, raw_replacement) + reindent_replacement(matched_source, raw_replacement, file_indent_char, file_indent_step) } else { raw_replacement.to_string() }; @@ -986,6 +1004,7 @@ fn apply_put( .to_owned(), ); } + validate_region_edit_safety(state, &anchor, operation.op, target.region)?; let requested_region = requested_region_for_operation(operation, default_selector, default_crc); @@ -993,13 +1012,17 @@ fn apply_put( target_indent_for_region(state, &anchor, target.region, file_indent_char, file_indent_step); let content = operation.content.as_deref().unwrap_or_default(); - let mut replacement = normalize_inserted_content( - content, - &initial_target_indent, - Some(file_indent_step), - file_indent_char, - normalize_indent, - ); + let mut replacement = if should_preserve_put_content_verbatim(state, &anchor, target.region) { + normalize_chunk_source(content) + } else { + normalize_inserted_content( + content, + &initial_target_indent, + Some(file_indent_step), + file_indent_char, + normalize_indent, + ) + }; let effective_region = target.region; let full_chunk_removed = effective_region.is_none() && replacement.is_empty(); @@ -1126,6 +1149,54 @@ fn should_preserve_existing_epilogue(epilogue: &str) -> bool { || first.starts_with("end") } +fn should_preserve_put_content_verbatim( + state: &ChunkStateInner, + anchor: &ChunkNode, + region: Option, +) -> bool { + state.language == "markdown" + && anchor.path.is_empty() + && matches!(region, None | Some(ChunkRegion::Body)) +} + +fn python_head_has_decorator(state: &ChunkStateInner, anchor: &ChunkNode) -> bool { + let (head_start, head_end) = chunk_region_range(anchor, ChunkRegion::Head); + state.source[head_start..head_end] + .lines() + .any(|line| line.trim_start().starts_with('@')) +} + +fn validate_region_edit_safety( + state: &ChunkStateInner, + anchor: &ChunkNode, + op: ChunkEditOp, + region: Option, +) -> Result<(), String> { + if state.language != "python" || region != Some(ChunkRegion::Head) { + return Ok(()); + } + if matches!(op, ChunkEditOp::Delete) { + return Err(format!( + "Deleting the Python head region of {} is unsafe because it can leave an indented body \ + attached to the previous block while still parsing. Delete the whole chunk, replace the \ + whole chunk, or use replace on a specific head line instead.", + chunk_path_opt(anchor) + )); + } + if matches!(op, ChunkEditOp::Put) + && matches!(anchor.kind, ChunkKind::Class | ChunkKind::Function) + && python_head_has_decorator(state, anchor) + { + return Err(format!( + "Head writes on decorated Python {} are unsafe because decorator/signature indentation \ + changes can move the existing body while still parsing. Replace the whole chunk or use \ + replace on the exact decorator/signature line instead.", + chunk_path_opt(anchor) + )); + } + Ok(()) +} + fn should_preserve_head_for_fallback_body_replace( state: &ChunkStateInner, anchor: &ChunkNode, @@ -1250,6 +1321,7 @@ fn apply_delete( )?; let anchor = target.chunk; let target_before = anchor.clone(); + validate_region_edit_safety(state, &anchor, operation.op, target.region)?; if target.region.is_none() { match anchor.kind { ChunkKind::Ours => { @@ -1362,8 +1434,12 @@ fn apply_insert( file_indent_char, file_indent_step, )?; - let suppress_chunk_adjacency = - matches!(operation.op, ChunkEditOp::Prepend | ChunkEditOp::Append) + warn_on_container_boundary_insert(&anchor, target.region, operation.op, pos, warnings); + let suppress_markdown_table_row_append = matches!(pos, InsertPosition::After) + && markdown_table_row_append_insertion_point(state, &anchor, operation.content.as_deref()) + .is_some_and(|point| point.offset == insertion.offset); + let suppress_chunk_adjacency = suppress_markdown_table_row_append + || matches!(operation.op, ChunkEditOp::Prepend | ChunkEditOp::Append) && !(matches!(operation.op, ChunkEditOp::Append) && pos == InsertPosition::After && owned_container_end_line(state, &anchor) > anchor.end_line); @@ -1411,6 +1487,33 @@ fn apply_insert( Ok(AppliedEditTarget { before: target_before, full_chunk_removed: false }) } +fn warn_on_container_boundary_insert( + anchor: &ChunkNode, + region: Option, + op: ChunkEditOp, + pos: InsertPosition, + warnings: &mut Vec, +) { + if region.is_some() || !is_container_like_chunk(anchor) { + return; + } + match (op, pos) { + (ChunkEditOp::Append, InsertPosition::After) => warnings.push(format!( + "append on container {} without `~` inserts after the chunk, not inside its body. Use \ + {}~ with append to insert inside the container.", + chunk_path_opt(anchor), + chunk_path_opt(anchor), + )), + (ChunkEditOp::Prepend, InsertPosition::Before) => warnings.push(format!( + "prepend on container {} without `~` inserts before the chunk, not inside its body. Use \ + {}~ with prepend to insert inside the container.", + chunk_path_opt(anchor), + chunk_path_opt(anchor), + )), + _ => {}, + } +} + fn normalize_operation_literals(operation: &EditOperation) -> EditOperation { let mut operation = operation.clone(); if matches!(operation.sel.as_deref(), Some("null" | "undefined")) { @@ -1873,6 +1976,63 @@ fn after_chunk_insertion_point(state: &ChunkStateInner, anchor: &ChunkNode) -> I } } +fn line_looks_markdown_table_row(line: &str) -> bool { + let trimmed = line.trim(); + trimmed.starts_with('|') && trimmed.ends_with('|') && trimmed.matches('|').count() >= 2 +} + +fn content_looks_like_markdown_table_rows(content: &str) -> bool { + let mut saw_row = false; + for line in content.lines() { + if line.trim().is_empty() { + continue; + } + if !line_looks_markdown_table_row(line) { + return false; + } + saw_row = true; + } + saw_row +} + +fn markdown_table_row_append_insertion_point( + state: &ChunkStateInner, + anchor: &ChunkNode, + content: Option<&str>, +) -> Option { + if state.language != "markdown" || !content.is_some_and(content_looks_like_markdown_table_rows) { + return None; + } + + let mut last_table_line = None; + let mut saw_table_line = false; + for (index, line) in state.source.split('\n').enumerate() { + let line_number = index as u32 + 1; + if line_number < anchor.start_line || line_number > anchor.end_line { + continue; + } + if line.trim().is_empty() { + continue; + } + if !line_looks_markdown_table_row(line) { + return None; + } + saw_table_line = true; + last_table_line = Some(line_number); + } + + if !saw_table_line { + return None; + } + + let offsets = line_offsets(&state.source); + let line = last_table_line?; + Some(InsertionPoint { + offset: line_end_offset(&offsets, line, &state.source), + indent: anchor.indent_char.repeat(anchor.indent as usize), + }) +} + fn body_insertion_point( state: &ChunkStateInner, anchor: &ChunkNode, @@ -1925,7 +2085,7 @@ fn resolve_insertion_point( anchor: &ChunkNode, region: Option, op: ChunkEditOp, - _file_content: Option<&str>, + file_content: Option<&str>, file_indent_char: char, file_indent_step: usize, ) -> Result<(InsertionPoint, InsertPosition), String> { @@ -1935,9 +2095,11 @@ fn resolve_insertion_point( Ok((before_chunk_insertion_point(state, anchor), InsertPosition::Before)) }, // After chunk boundary - (None, ChunkEditOp::After | ChunkEditOp::Append) => { - Ok((after_chunk_insertion_point(state, anchor), InsertPosition::After)) - }, + (None, ChunkEditOp::After | ChunkEditOp::Append) => Ok(( + markdown_table_row_append_insertion_point(state, anchor, file_content) + .unwrap_or_else(|| after_chunk_insertion_point(state, anchor)), + InsertPosition::After, + )), // Inner first-child position (Some(ChunkRegion::Body), ChunkEditOp::Before | ChunkEditOp::Prepend) | (Some(ChunkRegion::Head), ChunkEditOp::After | ChunkEditOp::Append) => Ok(( @@ -2663,6 +2825,7 @@ fn render_error_context( display_path: &str, anchor_style: Option, normalize_indent: bool, + label: &str, ) -> String { // 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 @@ -2702,7 +2865,7 @@ fn render_error_context( normalize_indent: Some(normalize_indent), focused_paths, }); - format!("\n\nFresh content:\n{rendered}") + format!("\n\n{label}:\n{rendered}") } fn render_unchanged_response( @@ -3130,11 +3293,11 @@ const betaValue = 2; ); assert!( err.contains("const alphaValue = 1;"), - "fresh content should show the original saved snapshot: {err}" + "current content should show the original saved snapshot: {err}" ); assert!( !err.contains("const alphaValue = 10;"), - "fresh content must not show partial in-memory edits: {err}" + "current content must not show partial in-memory edits: {err}" ); } @@ -3637,6 +3800,81 @@ function helper(): void { ); } + #[test] + fn python_multiline_replace_dedents_user_base_indent_before_reindenting() { + let source = concat!( + "class Server:\n", + "\tdef outer(self):\n", + "\t\tdef inner():\n", + "\t\t\tresult = compute()\n", + "\t\t\treturn result\n", + "\t\treturn inner()\n", + ); + let state = state_for(source, "python"); + let chunk = state + .inner() + .chunk("cls_Ser.fn_out") + .expect("outer function"); + + let result = apply_edits(&state, &EditParams { + operations: vec![EditOperation { + op: ChunkEditOp::Replace, + sel: Some("cls_Ser.fn_out".to_owned()), + crc: Some(chunk.checksum.clone()), + region: None, + find: Some("\t\t\t\tresult = compute()\n\t\t\t\treturn result".to_owned()), + content: Some( + "\t\t\t\tvalue = compute()\n\t\t\t\tif value:\n\t\t\t\t\treturn value".to_owned(), + ), + }], + default_selector: None, + default_crc: None, + anchor_style: None, + cwd: ".".to_owned(), + file_path: "test.py".to_owned(), + normalize_indent: None, + }) + .expect("replace should apply"); + + assert!( + result + .diff_after + .contains("\t\t\tvalue = compute()\n\t\t\tif value:\n\t\t\t\treturn value"), + "replacement should be reindented to the matched source base indent: {:?}", + result.diff_after + ); + assert!( + !result.diff_after.contains("\t\t\t\tvalue = compute()"), + "replacement must not keep the caller-supplied base indent and compound it: {:?}", + result.diff_after + ); + } + + #[test] + fn rust_enum_variant_replace_accepts_trailing_comma_boundary() { + let source = "enum LogLevel {\n Info,\n Warn,\n}\n"; + let state = state_for(source, "rust"); + let variant = state + .inner() + .chunk("en_Log.vr_War") + .expect("Warn variant should exist"); + + let result = apply_single_edit(&state, "test.rs", EditOperation { + op: ChunkEditOp::Replace, + sel: Some(format!("{}#{}", variant.path, variant.checksum)), + crc: None, + region: None, + content: Some("Warning,".to_owned()), + find: Some("Warn,".to_owned()), + }); + + assert!( + result.diff_after.contains(" Warning,\n"), + "replace should see the same trailing comma boundary as write:\n{}", + result.diff_after + ); + } + #[test] fn focus_emits_only_changed_chain() { let source = "const a = 1;\n\nconst b = 2;\n\nconst c = 3;\n\nconst d = 4;\n\nconst e = 5;\n"; @@ -3690,7 +3928,7 @@ function helper(): void { #[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 + // than the edit target, the rejected post-edit preview 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 @@ -3736,11 +3974,14 @@ function helper(): void { err.contains("Edit rejected: introduced"), "should be a parse-error rejection: {err}", ); - assert!(err.contains("Fresh content:"), "should include fresh content: {err}"); + assert!( + err.contains("Hypothetical post-edit content (edit rejected; file unchanged):"), + "should include rejected post-edit context: {err}" + ); // The edit target must always be visible. assert!(err.contains("fn_alp"), "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 + // lands should also appear in the focused rejected preview. 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_bra", "fn_cha", "fn_del", "fn_ech"] @@ -4070,7 +4311,10 @@ function helper(): void { }); let err = result.err().expect("should fail with stale CRC"); - assert!(err.contains("Fresh content:"), "error should include fresh content: {err}"); + assert!( + err.contains("Current content (file unchanged):"), + "error should include current content: {err}" + ); assert!(err.contains("fn_bar"), "error should show the chunk with fresh anchor: {err}"); assert!(err.contains("cls_Foo"), "error should show ancestor context: {err}"); } @@ -4275,6 +4519,36 @@ function helper(): void { ); } + #[test] + fn container_root_append_warns_that_insert_lands_outside_container() { + let source = "class Foo {\n bar() {\n return 1;\n }\n}\n"; + let state = state_for(source, "typescript"); + let class_chunk = state.inner().chunk("cls_Foo").expect("cls_Foo"); + + let result = apply_single_edit(&state, "test.ts", EditOperation { + op: ChunkEditOp::Append, + sel: Some(format!("cls_Foo#{}", class_chunk.checksum)), + crc: None, + region: None, + content: Some("\nfunction outside() {}\n".to_owned()), + find: None, + }); + + assert!( + result + .warnings + .iter() + .any(|warning| warning.contains("without `~` inserts after the chunk")), + "container-root append should warn about outside insertion: {:?}", + result.warnings + ); + assert!( + result.diff_after.ends_with("}\nfunction outside() {}"), + "append without ~ should still preserve existing outside semantics:\n{}", + result.diff_after + ); + } + #[test] fn body_prepend_inserts_after_opening_brace() { // Prepending to ~ of an enum should insert after the opening brace, @@ -4304,6 +4578,88 @@ function helper(): void { assert!(white_pos < red_pos, "White should be before Red: {}", result.diff_after); } + #[test] + fn successful_edit_response_contains_fresh_chunk_id() { + let source = "const count = 1;\n"; + let state = state_for(source, "typescript"); + let chunk_path = state + .inner() + .tree + .root_children + .first() + .expect("root child should exist") + .clone(); + let chunk = state.inner().chunk(&chunk_path).expect("root child chunk"); + + let result = apply_single_edit(&state, "test.ts", EditOperation { + op: ChunkEditOp::Put, + sel: Some(chunk_path.clone()), + crc: Some(chunk.checksum.clone()), + region: None, + content: Some("const count = 2;".to_owned()), + find: None, + }); + + let fresh = result + .state + .inner() + .chunk(&chunk_path) + .expect("edited chunk should still exist"); + assert_ne!(fresh.checksum, chunk.checksum, "checksum should change after edit"); + assert!( + result + .response_text + .contains(format!("{chunk_path}#{}", fresh.checksum).as_str()), + "edit response should include the fresh chunk ID. Response:\n{}", + result.response_text + ); + } + + #[test] + fn rust_enum_body_write_preserves_following_impl_block() { + let source = concat!( + "struct Server;\n", + "\n", + "enum LogLevel {\n", + " Info,\n", + " Warn,\n", + "}\n", + "\n", + "impl Server {\n", + " fn start(&self) {\n", + " println!(\"start\");\n", + " }\n", + "}\n", + ); + let state = state_for(source, "rust"); + let enum_chunk = state.inner().chunk("en_Log").expect("en_Log"); + + let result = apply_single_edit(&state, "test.rs", EditOperation { + op: ChunkEditOp::Put, + sel: Some(format!("en_Log#{}~", enum_chunk.checksum)), + crc: None, + region: None, + content: Some("Debug,\nInfo,\nWarn,\nError,\n".to_owned()), + find: None, + }); + + assert!( + result.diff_after.contains(" Error,\n"), + "enum body write should add the new variant:\n{}", + result.diff_after + ); + assert!( + result.diff_after.contains("impl Server"), + "enum body write must not delete the following impl block:\n{}", + result.diff_after + ); + assert!( + result.diff_after.contains("fn start(&self)"), + "enum body write must preserve methods inside the following impl block:\n{}", + result.diff_after + ); + } + #[test] fn markdown_list_replace_preserves_trailing_blank_line() { let source = "# Title\n\n- item 1\n- item 2\n\n## Next\n"; @@ -4427,6 +4783,44 @@ function helper(): void { ); } + #[test] + fn markdown_section_region_fallback_warns_when_children_would_be_replaced() { + let source = "# Title\n\n## Embedded\n\n```python\ndef greet():\n return \"hi\"\n```\n"; + let state = state_for(source, "markdown"); + let section = state + .inner() + .chunk("sct_Tit.sct_Emb") + .expect("embedded section"); + assert!( + !section.children.is_empty(), + "fixture section should have child chunks: {:?}", + section.children + ); + + let result = apply_single_edit(&state, "test.md", EditOperation { + op: ChunkEditOp::Put, + sel: Some(format!("{}#{}^", section.path, section.checksum)), + crc: None, + region: None, + content: Some("## Embedded\n\nreplacement\n".to_owned()), + find: None, + }); + + assert!( + result + .warnings + .iter() + .any(|warning| warning.contains("has child chunks that will be replaced")), + "section fallback should warn when child chunks are being replaced: {:?}", + result.warnings + ); + assert!( + !result.diff_after.contains("def greet"), + "fallback whole-section replacement should still reflect current semantics:\n{}", + result.diff_after + ); + } + #[test] fn markdown_table_body_append_keeps_row_continuity() { let source = "## Section\n\n| A |\n| --- |\n| one |\n\n## Next\n"; @@ -4464,6 +4858,69 @@ function helper(): void { ); } + #[test] + fn markdown_table_append_without_body_selector_keeps_row_continuity() { + let source = "## Section\n\n| A |\n| --- |\n| one |\n\n## Next\n"; + let state = state_for(source, "markdown"); + let table = state + .inner() + .tree + .chunks + .iter() + .find(|chunk| chunk.start_line == 3 && chunk.end_line == 5) + .expect("table chunk"); + + let result = apply_single_edit(&state, "test.md", EditOperation { + op: ChunkEditOp::After, + sel: Some(format!("{}#{}", table.path, table.checksum)), + crc: None, + region: None, + content: Some("| two |\n".to_owned()), + find: None, + }); + + assert!( + result.diff_after.contains("| one |\n| two |\n\n## Next"), + "table-row append should land before the trailing blank-line separator: {:?}", + result.diff_after + ); + } + + #[test] + fn markdown_table_row_chunk_delete_removes_only_that_row() { + let source = "## Section\n\n| A | B |\n| --- | --- |\n| one | 1 |\n| two | 2 |\n\n## Next\n"; + let state = state_for(source, "markdown"); + let table = state + .inner() + .tree + .chunks + .iter() + .find(|chunk| chunk.start_line == 3 && chunk.end_line == 6) + .expect("table chunk"); + let row_path = table.children[2].clone(); + let row = state.inner().chunk(&row_path).expect("row chunk"); + + let result = apply_single_edit(&state, "test.md", EditOperation { + op: ChunkEditOp::Delete, + sel: Some(format!("{}#{}", row.path, row.checksum)), + crc: None, + region: None, + content: None, + find: None, + }); + + assert!( + !result.diff_after.contains("| one | 1 |"), + "target row should be deleted: {:?}", + result.diff_after + ); + assert!( + result.diff_after.contains("| two | 2 |\n\n## Next"), + "other rows and section spacing should survive: {:?}", + result.diff_after + ); + } + #[test] fn markdown_fenced_python_body_write_preserves_code_indent() { let source = "```python\ndef outer():\n if cond:\n return 1\n```\n"; @@ -4494,6 +4951,40 @@ function helper(): void { ); } + #[test] + fn markdown_root_body_write_preserves_fenced_code_indentation_verbatim() { + let source = + "Intro\n\n# Title\n\n```python\ndef outer():\n return 1\n```\n\n## Notes\n\ntext\n"; + let state = state_for(source, "markdown"); + let root = state.inner().chunk("").expect("root chunk should exist"); + + let result = apply_single_edit(&state, "test.md", EditOperation { + op: ChunkEditOp::Put, + sel: Some(format!("#{}~", root.checksum)), + crc: None, + region: None, + content: Some( + "Intro changed\n\n# Title\n\n```python\ndef outer():\n return 1\n```\n\n## \ + Notes\n\ntext\n" + .to_owned(), + ), + find: None, + }); + + assert!( + result + .diff_after + .contains("def outer():\n return 1\n```"), + "root markdown body write should not add extra indentation inside fences: {:?}", + result.diff_after + ); + assert!( + !result.diff_after.contains("def outer():\n return 1"), + "fenced code indentation should not be inflated: {:?}", + result.diff_after + ); + } + #[test] fn rust_trait_members_are_addressable() { let source = "trait Handler {\n fn handle(&self, req: &str) -> String;\n fn \ @@ -4542,6 +5033,72 @@ function helper(): void { ); } + #[test] + fn python_decorated_class_head_write_is_rejected() { + let source = "@dataclass\nclass Server:\n host: str\n port: int\n"; + let state = state_for(source, "python"); + let class_chunk = state.inner().chunk("cls_Ser").expect("cls_Ser"); + + let err = apply_edits(&state, &EditParams { + operations: vec![EditOperation { + op: ChunkEditOp::Put, + sel: Some(format!("cls_Ser#{}^", class_chunk.checksum)), + crc: None, + region: None, + content: Some("@dataclass\nclass Server:\n".to_owned()), + find: None, + }], + default_selector: None, + default_crc: None, + anchor_style: None, + cwd: ".".to_owned(), + file_path: "test.py".to_owned(), + normalize_indent: None, + }) + .err() + .expect("decorated Python class head write should be rejected"); + + assert!( + err.contains("Head writes on decorated Python cls_Ser are unsafe"), + "error should explain decorated Python head safety: {err}" + ); + } + + #[test] + fn python_head_delete_is_rejected_to_avoid_orphaned_body() { + let source = + "class Server:\n @property\n def address(self):\n return self.host\n"; + let state = state_for(source, "python"); + let function = state + .inner() + .chunk("cls_Ser.fn_add") + .expect("property function"); + + let err = apply_edits(&state, &EditParams { + operations: vec![EditOperation { + op: ChunkEditOp::Delete, + sel: Some(format!("{}#{}^", function.path, function.checksum)), + crc: None, + region: None, + content: None, + find: None, + }], + default_selector: None, + default_crc: None, + anchor_style: None, + cwd: ".".to_owned(), + file_path: "test.py".to_owned(), + normalize_indent: None, + }) + .err() + .expect("Python head delete should be rejected"); + + assert!( + err.contains("Deleting the Python head region of cls_Ser.fn_add is unsafe"), + "error should explain orphaned-body risk: {err}" + ); + } + #[test] fn body_region_on_leaf_without_delimiters_is_rejected() { let source = "enum LogLevel {\n Debug,\n Info,\n Warn,\n Fatal,\n}\n"; diff --git a/crates/pi-natives/src/chunk/kind.rs b/crates/pi-natives/src/chunk/kind.rs index 49b2d6393..96fde3e3a 100644 --- a/crates/pi-natives/src/chunk/kind.rs +++ b/crates/pi-natives/src/chunk/kind.rs @@ -105,6 +105,7 @@ pub enum ChunkKind { Relations, Render, Return, + Row, Root, Rule, Schema, @@ -318,6 +319,7 @@ impl ChunkKind { Self::Relations => "rels", Self::Render => "rnd", Self::Return => "ret", + Self::Row => "row", Self::Root => "root", Self::Rule => "rule", Self::Schema => "sch", @@ -476,6 +478,7 @@ impl ChunkKind { Self::Relations => &DEFAULT_TRAITS, Self::Render => &DEFAULT_TRAITS, Self::Return => &DEFAULT_TRAITS, + Self::Row => &PACKED_LEAF_TRAITS, Self::Root => &DEFAULT_TRAITS, Self::Rule => &DEFAULT_TRAITS, Self::Schema => &DEFAULT_TRAITS, @@ -641,6 +644,7 @@ impl ChunkKind { "relations" => Self::Relations, "render" => Self::Render, "ret" | "return" => Self::Return, + "row" => Self::Row, "root" => Self::Root, "rule" => Self::Rule, "schema" => Self::Schema, diff --git a/crates/pi-natives/src/chunk/mod.rs b/crates/pi-natives/src/chunk/mod.rs index dd3239b43..c494c38ce 100644 --- a/crates/pi-natives/src/chunk/mod.rs +++ b/crates/pi-natives/src/chunk/mod.rs @@ -1939,6 +1939,7 @@ impl Config { }"#; let tree = build_chunk_tree(source, "go").expect("tree should build"); let names: Vec<&str> = tree.root_children.iter().map(String::as_str).collect(); + assert!(names.contains(&"mod_mai"), "expected package module, got {names:?}"); assert!(names.contains(&"imp"), "expected imports, got {names:?}"); assert!(names.contains(&"ty_Con"), "expected ty_Con, got {names:?}"); assert!(names.contains(&"ty_Rea"), "expected ty_Rea, got {names:?}"); @@ -1966,6 +1967,26 @@ impl Config { assert!(reader.children.is_empty(), "single-line interfaces should render inline"); } + #[test] + fn go_method_summary_omits_receiver_duplication() { + let source = r"package main + +type MemorySink struct{} + +func (s *MemorySink) Write(p []byte) (int, error) { + return len(p), nil +} +"; + let tree = build_chunk_tree(source, "go").expect("tree should build"); + let method = tree + .chunks + .iter() + .find(|chunk| chunk.path == "fn_Wri") + .expect("Write method"); + + assert_eq!(method.signature.as_deref(), Some("fn Write(p []byte) (int, error)")); + } + #[test] fn nix_chunk_tree_exposes_attr_bindings() { let source = r#"{ @@ -2692,6 +2713,36 @@ end ); } + #[test] + fn markdown_tables_expose_row_chunks() { + let source = "# Top\n\n## Data\n\n| Name | Status |\n| --- | --- |\n| Ada | Active |\n| Lin \ + | Idle |\n\n## Next\n"; + let tree = build_chunk_tree(source, "markdown").expect("markdown tree"); + let table = tree + .chunks + .iter() + .find(|chunk| chunk.start_line == 5 && chunk.end_line == 8) + .expect("table chunk"); + + assert!(!table.leaf, "recognized table should expose row children"); + assert_eq!(table.children, vec![ + format!("{}.row_1", table.path), + format!("{}.row_2", table.path), + format!("{}.row_3", table.path), + format!("{}.row_4", table.path), + ]); + + let data_row = tree + .chunks + .iter() + .find(|chunk| chunk.path == table.children[2]) + .expect("third row chunk"); + assert_eq!(data_row.kind, ChunkKind::Row); + assert_eq!(data_row.start_line, 7); + assert_eq!(data_row.end_line, 7); + assert_eq!(data_row.signature.as_deref(), Some("| Ada | Active |")); + } + #[test] fn adjacent_toml_tables_do_not_overlap() { let source = "[package]\nname = \"x\"\n\n[deps]\na = 1\n\n[tool]\nb = 2\n"; diff --git a/crates/pi-natives/src/chunk/resolve.rs b/crates/pi-natives/src/chunk/resolve.rs index b7a325334..88b01798b 100644 --- a/crates/pi-natives/src/chunk/resolve.rs +++ b/crates/pi-natives/src/chunk/resolve.rs @@ -843,7 +843,7 @@ fn build_not_found_error(tree: &ChunkTree, cleaned: &str) -> String { } else { let tree_lines = format_selector_tree(tree, &tree.root_children, false); if tree_lines.is_empty() { - " Re-read the file to see available chunk paths.".to_owned() + " Use sel=\"?\" to see available chunk paths.".to_owned() } else { format!(" Available top-level chunks:\n{}", tree_lines.join("\n")) } @@ -851,13 +851,13 @@ fn build_not_found_error(tree: &ChunkTree, cleaned: &str) -> String { if hint.contains('\n') { format!( - "Chunk path not found: \"{cleaned}\".{hint}\nRe-read the file to see the full chunk tree \ - with paths and checksums." + "Chunk path not found: \"{cleaned}\".{hint}\nUse sel=\"?\" if you need the full chunk \ + tree with paths and checksums." ) } else { format!( - "Chunk path not found: \"{cleaned}\".{hint} Re-read the file to see the full chunk tree \ - with paths and checksums." + "Chunk path not found: \"{cleaned}\".{hint} Use sel=\"?\" if you need the full chunk \ + tree with paths and checksums." ) } } diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index fd24cab74..74e80ac9a 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -3,6 +3,8 @@ ## [Unreleased] ### Added +- Added a non-mutating chunk edit `read: true` operation for inspecting chunk content without relying on edit failures or delete previews. +- Added Markdown pipe-table `row_N` chunk selectors for row-level table edits. - Added `resolveToolAlias` export so tool names in CLI and session setup are normalized to canonical names, including mapping legacy `read` references to `open` - Added new `open` and `open-chunk` tool prompt documentation pages to describe canonical `open` usage for local files/directories, chunk reads, and URLs - Added full-output retrieval metadata to minimized shell command output by appending an `artifact://` footer with byte counts, allowing users to open the original unminimized command output @@ -14,6 +16,11 @@ - Changed the canonical file/URL reader tool from `read` to `open` across default tool lists and routing, including system prompts, plan mode, cursor handlers, and runtime tool registration - Changed runtime and UI handling to render and track `open` tool calls as first-class (with `read` accepted as legacy alias), including ACP mapping, session observers, and streaming message groups - Changed chunk edit guidance to document parser-specific region behavior, including TypeScript decorator/JSDoc sibling chunks, Python docstrings as body content, Python opaque nested chunks, Markdown whole-chunk fallbacks, ID volatility, and indentation display differences +- Changed chunk edit guidance to point agents at `open` as the primary chunk read/discovery operation, with `read: true` documented as a convenience for known selectors. +- Changed chunk deletion in chunk edit mode to require explicit `delete: true`; `write: null` and bare `{ path }` entries now fail with guidance instead of deleting content. +- Changed chunk edit validation to reject entries with multiple operation fields instead of choosing one and ignoring the rest. +- Changed chunk edit validation to reject `write: ""` as an accidental destructive empty replacement; use `read: true` for inspection or `delete: true` for deletion. +- Changed chunk edit responses to warn when appending or prepending to a container without `~`, since that inserts outside the container rather than inside its body. - Changed fetch output logging so URL-fetch artifacts now use `.open.log` naming instead of `.read.log` - Changed Bash interception guidance and errors to recommend `open` in place of `read` for cat/head/tail-style commands - Changed exported SDK tool surface to expose `OpenTool` as canonical and keep `ReadTool` as a compatibility alias @@ -23,6 +30,10 @@ - Changed default behavior so shell output minimization can now be toggled from settings without code changes - Changed shell output minimization to leave compound and piped commands unchanged; only a single eligible whole command is captured and minimized after it exits +### Removed + +- Removed the `replace: { old, new }` chunk edit operation. Use `write` or `insert` for chunk edits instead. + ### Fixed - Fixed session list metadata extraction to better populate session titles and first-user summaries from partial session data when full JSONL parsing is unavailable @@ -31,8 +42,16 @@ - Fixed streaming chunk previews that could display an incomplete trailing edit as a deletion when partial JSON temporarily converted in-flight values to `null` - Fixed edit streaming preview updates to cancel obsolete in-flight computations and avoid rendering stale previews as args change - Fixed chunk edits to reject unsafe `^`/`~` writes on code leaf chunks instead of falling back to whole-chunk replacement and risking structural indentation corruption -- Fixed chunk `replace` operations to preserve multiline replacement indentation literally instead of stripping leading whitespace from inserted lines +- Fixed chunk `replace` operations to dedent multiline replacement snippets before reapplying the matched source indentation, preventing Python nested replacements from compounding indentation on repeated edits. +- Fixed Go chunk trees to classify `package` clauses separately from imports and to avoid duplicating method receivers in method summaries. +- Fixed chunk path-not-found guidance so it recommends `sel="?"` without claiming the already-shown listing must be re-read. - Fixed Markdown chunk appends to preserve blank-line separators after line-oriented inserts such as table rows +- Fixed Markdown section region-fallback warnings to call out child chunks that will be replaced by whole-section edits. +- Fixed rejected chunk-edit errors to distinguish current file content from hypothetical post-edit parse-error previews and to state when a same-file batch was rolled back. +- Fixed unsafe Python head-region edits by rejecting decorated Python `^` writes and Python `^` deletes that can orphan indented bodies while still parsing. +- Fixed Markdown table-row appends so row-shaped content lands inside the table block instead of after the trailing blank-line separator. +- Fixed Markdown root writes to preserve fenced-code indentation verbatim. +- Fixed Rust enum-variant replacement matching so trailing commas are included consistently with whole-variant writes. - Fixed streaming edit call headers to keep showing the target file path while the edit arguments are still arriving - Fixed Mermaid fenced markdown rendering in assistant messages on terminals without image protocol support ([#650](https://github.com/can1357/oh-my-pi/issues/650)) - Fixed chunk edit path parsing so plan-mode edits to section-addressed `local://PLAN.md:` paths are classified as writes to the plan file diff --git a/packages/coding-agent/src/edit/index.ts b/packages/coding-agent/src/edit/index.ts index b510fdb32..b01286303 100644 --- a/packages/coding-agent/src/edit/index.ts +++ b/packages/coding-agent/src/edit/index.ts @@ -324,7 +324,7 @@ export class EditTool implements AgentTool { }), parameters: chunkEditParamsSchema, invalidParamsMessage: - "Invalid edit parameters for chunk mode. Expected `{ edits: [{ path: 'file:selector', ...op }, ...] }` with at least one edit. Each edit needs a `path`; supply one of `write` (string content; pass an empty string or omit it together with `replace`/`insert` to delete the chunk), `replace: { old, new }`, or `insert: { loc, body }`.", + "Invalid edit parameters for chunk mode. Expected `{ edits: [{ path: 'file:selector', ...op }, ...] }` with at least one edit. Each edit needs a `path`; supply exactly one of `read: true`, `write: 'content'`, `insert: { loc, body }`, or `delete: true`.", validate: isChunkParams, execute: ( tool: EditTool, diff --git a/packages/coding-agent/src/edit/modes/chunk.ts b/packages/coding-agent/src/edit/modes/chunk.ts index 4cf6a3650..51d997391 100644 --- a/packages/coding-agent/src/edit/modes/chunk.ts +++ b/packages/coding-agent/src/edit/modes/chunk.ts @@ -32,7 +32,6 @@ export type { ChunkReadTarget }; export type ChunkEditOperation = | { op: "put"; sel?: string; content: string } - | { op: "replace"; sel?: string; content: string; find: string } | { op: "delete"; sel?: string } | { op: "before"; sel?: string; content: string } | { op: "after"; sel?: string; content: string } @@ -120,6 +119,8 @@ type ChunkSourceContext = { chunkLanguage: string | undefined; }; +type ChunkSourceIntent = "read" | "write"; + function normalizeLanguage(language: string | undefined): string { return language?.trim().toLowerCase() || ""; } @@ -140,11 +141,17 @@ function fileLanguageTag(filePath: string, language?: string): string | undefine return ext.length > 0 ? ext : undefined; } -async function resolveChunkSourceContext(session: ToolSession, path: string): Promise { +async function resolveChunkSourceContext( + session: ToolSession, + path: string, + options?: { intent?: ChunkSourceIntent }, +): Promise { const resolvedPath = resolvePlanPath(session, path); const sourceFile = Bun.file(resolvedPath); const sourceExists = await sourceFile.exists(); - enforcePlanModeWrite(session, path, { op: sourceExists ? "update" : "create" }); + if ((options?.intent ?? "write") === "write") { + enforcePlanModeWrite(session, path, { op: sourceExists ? "update" : "create" }); + } let rawContent = ""; if (sourceExists) { @@ -191,6 +198,12 @@ export async function computeChunkDiff( options?.signal?.throwIfAborted?.(); const { filePath } = parseChunkEditPath(input.path); if (!filePath) return { error: "chunk edit path is empty" }; + if (input.edits.every(isChunkReadOperation)) { + return { diff: "", firstChangedLine: undefined }; + } + if (input.edits.some(hasChunkReadFlag)) { + return { error: "`read: true` cannot be mixed with mutating chunk edit operations." }; + } const { resolvedPath, rawContent, language } = await loadChunkSource({ cwd, path: filePath }); options?.signal?.throwIfAborted?.(); const { operations } = normalizeChunkEditOperations(input.edits); @@ -441,15 +454,6 @@ function toNativeEditOperation( region: nativeRegion, content: operation.content, }; - case "replace": - return { - op: ChunkEditOp.Replace, - sel: selector, - crc, - region: nativeRegion, - find: operation.find, - content: operation.content, - }; case "before": return { op: ChunkEditOp.Before, sel: selector, crc, region: nativeRegion, content: operation.content }; case "after": @@ -554,19 +558,22 @@ export const chunkToolEditSchema = Type.Object( path: Type.String({ description: "File path with chunk selector. Examples: 'src/app.ts:fn_foo#ABCD~', 'src/app.ts:class_Bar'.", }), - write: Type.Optional( - Type.Union([Type.String(), Type.Null()], { - description: "Write complete new content to the targeted region. Use null to delete the chunk.", + read: Type.Optional( + Type.Boolean({ + description: + "Return a known chunk selector without modifying the file. Prefer the open tool for normal chunk reads and discovery.", }), ), - replace: Type.Optional( - Type.Object( - { - old: Type.String({ description: "Literal substring to find. Must match exactly once." }), - new: Type.String({ description: "Replacement text." }), - }, - { description: "Find and replace a substring within the chunk." }, - ), + write: Type.Optional( + Type.Union([Type.String(), Type.Null()], { + description: + "Write complete new content to the targeted region. Null is rejected; use delete: true for deletion.", + }), + ), + delete: Type.Optional( + Type.Boolean({ + description: "Explicitly delete the targeted chunk. Must be true; include the current chunk ID.", + }), ), insert: Type.Optional( Type.Object( @@ -614,11 +621,8 @@ export function isChunkParams(params: unknown): params is ChunkParams { return false; } const first = params.edits[0]; - // Accept a bare `{ path }` entry: it is interpreted downstream as a chunk - // delete. Some providers strip `null` values from tool-call JSON, so a - // documented `{ path, write: null }` delete can arrive here as just - // `{ path }`. Rejecting that surfaced as a misleading - // "Invalid edit parameters for chunk mode." error. + // Accept a bare `{ path }` entry so the executor can return a targeted + // "missing operation" error instead of the generic schema failure. return typeof first === "object" && first !== null && "path" in first; } @@ -670,6 +674,39 @@ function autoCorrectBodyIndent(content: string, index: number): { content: strin return { content, warnings }; } +function chunkEditOperationFields(edit: ChunkToolEdit): string[] { + const fields: string[] = []; + if (edit.read === true) fields.push("read"); + if (edit.write !== undefined) fields.push("write"); + if (edit.insert != null) fields.push("insert"); + if (edit.delete === true) fields.push("delete"); + return fields; +} + +function hasChunkReadFlag(edit: ChunkToolEdit): boolean { + return edit.read === true; +} + +function isChunkReadOperation(edit: ChunkToolEdit): boolean { + const fields = chunkEditOperationFields(edit); + return fields.length === 1 && fields[0] === "read"; +} + +function assertSingleChunkOperation(edit: ChunkToolEdit, index: number): string { + const fields = chunkEditOperationFields(edit); + if (fields.length === 0) { + throw new Error( + `Edit ${index + 1}: no operation specified. Use open to inspect chunks, read:true only for a known selector, write:"..." to replace, insert:{loc,body}, or delete:true to delete.`, + ); + } + if (fields.length > 1) { + throw new Error( + `Edit ${index + 1}: multiple operation fields set (${fields.join(", ")}). Each chunk edit entry must have exactly one operation.`, + ); + } + return fields[0]; +} + function normalizeChunkEditOperations(edits: ChunkToolEdit[]): { operations: ChunkEditOperation[]; warnings: string[]; @@ -677,26 +714,20 @@ function normalizeChunkEditOperations(edits: ChunkToolEdit[]): { const warnings: string[] = []; const operations = edits.map((edit, index): ChunkEditOperation => { const { selector } = parseChunkEditPath(edit.path); - // When multiple ops are present (model confusion), prefer write (total replacement) as the - // safest default, then replace (surgical), then insert (additive), then delete. - const hasInsert = edit.insert != null && typeof edit.insert.body === "string" && edit.insert.body.length > 0; - const hasReplace = - edit.replace != null && - ((typeof edit.replace.old === "string" && edit.replace.old.length > 0) || - (typeof edit.replace.new === "string" && edit.replace.new.length > 0)); - const hasWrite = typeof edit.write === "string" && edit.write.length > 0; - const opCount = [hasInsert, hasReplace, hasWrite].filter(Boolean).length; - if (opCount > 1) { - const chosen = hasWrite ? "write" : hasReplace ? "replace" : "insert"; - const present = [hasWrite && "write", hasReplace && "replace", hasInsert && "insert"] - .filter(Boolean) - .join(", "); - warnings.push( - `Edit ${index + 1}: multiple operation fields set (${present}). Each edit entry must have exactly ONE of write/replace/insert — not multiple. Used "${chosen}", ignored the rest.`, - ); + const operation = assertSingleChunkOperation(edit, index); + if (operation === "read") { + throw new Error("`read: true` is non-mutating and cannot be normalized as an edit operation."); } - if (hasWrite) { - let writeContent = edit.write!; + if (operation === "write") { + if (edit.write === null) { + throw new Error( + `Edit ${index + 1}: write:null no longer deletes chunks. Use delete:true to delete, or open/read:true to inspect chunk content without modifying the file.`, + ); + } + if (typeof edit.write !== "string") { + throw new Error(`Edit ${index + 1}: write must be a string.`); + } + let writeContent = edit.write; if (selector?.endsWith("~")) { const corrected = autoCorrectBodyIndent(writeContent, index); writeContent = corrected.content; @@ -704,15 +735,12 @@ function normalizeChunkEditOperations(edits: ChunkToolEdit[]): { } return { op: "put", sel: selector, content: writeContent }; } - if (typeof edit.write === "string" && !hasInsert && !hasReplace) { - return { op: "put", sel: selector, content: edit.write }; - } - if (hasReplace) { - return { op: "replace", sel: selector, content: edit.replace!.new, find: edit.replace!.old }; - } - if (hasInsert) { - const op = edit.insert!.loc === "prepend" ? "before" : "after"; - let insertContent = edit.insert!.body; + if (operation === "insert") { + if (edit.insert == null || typeof edit.insert.body !== "string" || edit.insert.body.length === 0) { + throw new Error(`Edit ${index + 1}: insert.body must be a non-empty string.`); + } + const op = edit.insert.loc === "prepend" ? "before" : "after"; + let insertContent = edit.insert.body; if (selector?.endsWith("~")) { const corrected = autoCorrectBodyIndent(insertContent, index); insertContent = corrected.content; @@ -720,7 +748,9 @@ function normalizeChunkEditOperations(edits: ChunkToolEdit[]): { } return { op, sel: selector, content: insertContent }; } - // write: null or no op specified → delete + if (operation !== "delete") { + throw new Error(`Edit ${index + 1}: unsupported chunk edit operation "${operation}".`); + } return { op: "delete", sel: selector }; }); return { operations, warnings }; @@ -775,14 +805,58 @@ async function writeChunkResult(params: { }; } +async function readChunkResult(params: { + session: ToolSession; + resolvedPath: string; + sourceExists: boolean; + chunkLanguage: string | undefined; + edits: ChunkToolEdit[]; +}): Promise> { + const { session, resolvedPath, sourceExists, chunkLanguage, edits } = params; + if (!sourceExists) { + throw new Error(`File does not exist: ${resolvedPath}. Cannot read chunk selectors on a non-existent file.`); + } + + const texts: string[] = []; + for (const edit of edits) { + const { selector } = parseChunkEditPath(edit.path); + const readPath = selector ? `${resolvedPath}:${selector}` : resolvedPath; + const result = await formatChunkedRead({ + filePath: resolvedPath, + readPath, + cwd: session.cwd, + language: chunkLanguage, + anchorStyle: resolveAnchorStyle(session.settings), + }); + texts.push(result.text); + } + + return { + content: [{ type: "text", text: texts.join("\n\n") }], + details: { + diff: "", + meta: outputMeta().get(), + }, + }; +} + export async function executeChunkSingle( options: ExecuteChunkSingleOptions, ): Promise> { const { session, path, edits, signal, batchRequest, writethrough, beginDeferredDiagnosticsForPath } = options; + const readOnly = edits.every(isChunkReadOperation); + if (edits.some(hasChunkReadFlag) && !readOnly) { + throw new Error("`read: true` cannot be mixed with mutating chunk edit operations."); + } const { resolvedPath, sourceFile, sourceExists, rawContent, chunkLanguage } = await resolveChunkSourceContext( session, path, + { intent: readOnly ? "read" : "write" }, ); + if (readOnly) { + return readChunkResult({ session, resolvedPath, sourceExists, chunkLanguage, edits }); + } + const parentDir = nodePath.dirname(resolvedPath); if (parentDir && parentDir !== ".") { await fs.mkdir(parentDir, { recursive: true }); diff --git a/packages/coding-agent/src/edit/streaming.ts b/packages/coding-agent/src/edit/streaming.ts index d416dbcca..02b4b37eb 100644 --- a/packages/coding-agent/src/edit/streaming.ts +++ b/packages/coding-agent/src/edit/streaming.ts @@ -64,7 +64,8 @@ export interface EditStreamingStrategy { * * This guards against `partial-json` silently coercing truncated tails like * `"write":nu` / `"write":nul` into `{ write: null }`, which would make the - * last entry render as a spurious delete until the value finishes streaming. + * last entry render a spurious null-write error until the value finishes + * streaming. */ export function dropIncompleteLastEdit(edits: readonly T[], partialJson: string | undefined, listKey: string): T[] { if (!Array.isArray(edits) || edits.length === 0) return [...(edits ?? [])]; @@ -228,9 +229,9 @@ const chunkStrategy: EditStreamingStrategy = { // `null` literals), `partial-json` may have already surfaced the last // entry with `write === null`. When that entry's `}` hasn't closed // yet, it has already been dropped above. But if dropping was not - // triggered (e.g. list still open and no new `{` after), also drop - // a trailing entry that is *only* `{ path }` because partial-json - // stripped the in-flight value entirely. + // triggered (e.g. list still open and no new `{` after), also drop the + // trailing null-write entry so the preview does not flicker with an + // error for an incomplete string/null literal. if (partialJson && edits.length > 0) { const last = edits[edits.length - 1] as Partial | undefined; const endsInPartialNull = /:\s*nu?l?\s*$/.test(partialJson.trimEnd()); diff --git a/packages/coding-agent/src/prompts/tools/chunk-edit.md b/packages/coding-agent/src/prompts/tools/chunk-edit.md index 95e174efa..6f3127d57 100644 --- a/packages/coding-agent/src/prompts/tools/chunk-edit.md +++ b/packages/coding-agent/src/prompts/tools/chunk-edit.md @@ -1,13 +1,16 @@ -Edits files via syntax-aware chunks. Run `open(path="file.ts")` first. +Edits files via syntax-aware chunks. Use `open(path="file.ts")` to read and discover chunks before editing. +- `open` is the canonical read path for chunk source and `sel="?"` tree listings. +- `read:true` is a non-mutating convenience for an already-known selector. - `write` rewrites the entire targeted region — best for most edits. -- `replace` does surgical find-and-replace within a chunk — use when making small changes to a large chunk, or batching multiple substitutions. - `insert` adds content before/after a chunk. +- `delete` deletes a targeted chunk and must be explicit. Call format: `{"edits": [{"path": "file:chunk#ID~", "write": "new body"}, …]}` -- **MUST** `read` first. Never invent chunk paths or IDs. Copy them from the latest `read` output or edit response. -- `path` format: `file:selector` — e.g. `src/app.ts:fn_foo#ABCD~`. Append `~` for body, `^` for head, or nothing for the whole chunk. Include `#ID` for `put`/`find`+`replace`/`delete`. +- **MUST** inspect first with `open`. Never invent chunk paths or IDs. Copy them from the latest `open` output or edit response. +- `path` format: `file:selector` — e.g. `src/app.ts:fn_foo#ABCD~`. Append `~` for body, `^` for head, or nothing for the whole chunk. Include `#ID` for `write`/`delete`. +- To inspect a known chunk through this tool, use `{"path":"file:chunk#ID","read":true}`. `read:true` is non-mutating, cannot be mixed with write operations in the same entry, and is not a replacement for `open(path="file", sel="?")` when discovering targets. - If the exact chunk path is unclear, run `open(path="file", sel="?")` and copy a selector from that listing. {{#if chunkAutoIndent}} - Use `\t` for indentation in `content`. Write content at indent-level 0 — the tool re-indents it to match the chunk's position in the file. For example, to replace `~` of a method, write the body starting at column 0: @@ -17,6 +20,7 @@ Call format: `{"edits": [{"path": "file:chunk#ID~", "write": "new body"}, …]}` The tool adds the correct base indent automatically. Never manually pad with the chunk's own indentation. Multiple sibling body lines at the same level all start at column 0: `"print(a)\nprint(b)\nprint(c)\n"`. Only use `\t` when nesting deeper (e.g. `"if cond:\n\tinner\nouter\n"`). Before applying the target's base indent, the tool strips any common leading whitespace shared by all non-empty `write` lines as a safety net. Do not rely on that cleanup for mixed indentation; write `~` bodies at column 0 and use one `\t` per relative nesting level. + Multi-line replacements use the same relative-indentation model: the replacement text is dedented, then re-indented to the matched source line. Do not include the chunk's base indentation in replacement text. **Common mistake** when replacing `~` of a function body: do NOT include the function's own indentation. Wrong: `"if b == 0:\n\t\treturn None\n\treturn a / b\n"` — adds the function's base `\t` to every line. Correct: `"if b == 0:\n\treturn None\nreturn a / b\n"` — `if` and `return a / b` at column 0, only `return None` gets `\t` for nesting. @@ -28,11 +32,14 @@ Call format: `{"edits": [{"path": "file:chunk#ID~", "write": "new body"}, …]}` ``` The tool adds the correct base indent automatically, then preserves the tabs/spaces you used inside the snippet. Never manually pad with the chunk's own indentation. Before applying the target's base indent, the tool strips any common leading whitespace shared by all non-empty `write` lines as a safety net. Do not rely on that cleanup for mixed indentation; write `~` bodies at column 0. + Multi-line replacements use the same relative-indentation model: the replacement text is dedented, then re-indented to the matched source line. Do not include the chunk's base indentation in replacement text. {{/if}} - Region suffixes only apply to chunks with a real head/body boundary (classes, functions, impl blocks, and similar containers). On code leaf chunks (enum variants, fields, single statements, and compound statements like `if`/`for`/`while`/`match`/`try`), `~` and `^` are rejected. Use the unsuffixed selector and supply the complete replacement content, or edit the parent container's `~` body. -- `replace: {old, new}` treats multiline `new` text literally. If `new` contains a newline, include the exact leading whitespace required on every inserted line; it is not stripped and re-indented like `write` content. If `old` starts after existing line indentation and multiline `new` includes that same indentation on its first line, the existing prefix is consumed to avoid double-indenting the first inserted line. -- `put`, `find`+`replace`, and `delete` require the current ID. `prepend`/`append` do not. +- Unsuffixed `write` on a leaf chunk uses your content verbatim after normal replacement; it is not a body-region rewrite. Include the exact indentation and punctuation the leaf needs in the file. +- `^` head writes and `~` body writes use the same base-indent model: write content at column 0 relative to the target region, and the tool applies the chunk's file indentation. +- `write` and `delete` require the current ID. `prepend`/`append` do not. - **IDs change after every edit.** The edit response always carries the new IDs — use those for the next call or run `open(path="file", sel="?")` to refresh. Never reuse an ID from before the latest edit. +- Same-file edit batches are transactional: if any operation in that file fails, no changes from that file's batch are saved. Multi-file edit calls run per file, so a later file error does not roll back earlier files that already succeeded. @@ -40,36 +47,36 @@ You **MUST** use the narrowest region that covers your change. Putting without a **`put` is total, not surgical.** The `content` you supply becomes the *complete* new content for the targeted region. Everything in the original region that you omit from `content` is deleted. Before using `put` on any chunk's `~`, verify the chunk does not contain children you intend to keep. If a chunk spans hundreds of lines and your change touches only a few, target a specific child chunk — not the parent. -**Group chunks (`stmts_*`, `imports_*`, `decls_*`) are containers.** They hold many sibling items (test functions, import statements, declarations). `put` on a group chunk's `~` overwrites **all** of its children. To edit one item inside a group, target that item's own chunk path. If no child chunk exists, use the specific child's chunk selector from `read` output — do not `put` the parent group. +**Group chunks (`stmts_*`, `imports_*`, `decls_*`) are containers.** They hold many sibling items (test functions, import statements, declarations). `put` on a group chunk's `~` overwrites **all** of its children. To edit one item inside a group, target that item's own chunk path. If no child chunk exists, use the specific child's chunk selector from `open` output — do not `put` the parent group. -In `read` output, lines marked `^` between the line number and `|` are **head** lines (doc comments, attributes/decorators, signature). Lines without `^` are **body** lines. Use this to decide which region to target: +In `open` or `read:true` output, lines marked `^` between the line number and `|` are **head** lines (doc comments, attributes/decorators, signature). Lines without `^` are **body** lines. Use this to decide which region to target: - `fn_foo#ID~` — **body only (the default choice for most edits).** Head lines (`^`) are preserved automatically — doc comments, attributes, and signature stay untouched. On code leaf chunks, this is rejected because there is no safe body boundary. - `fn_foo#ID^` — head only (decorators, attributes, doc comments, signature, opening delimiter). Body stays untouched. - `fn_foo#ID` — entire chunk including leading trivia. **You must include doc comments and attributes in `content`; omitting them deletes them.** -- `chunk~` + `append`/`prepend` inserts *inside* the container. `chunk` + `append`/`prepend` inserts *outside*. +- `chunk~` + `append`/`prepend` inserts *inside* the container. `chunk` + `append`/`prepend` inserts *outside*. Appending to a container without `~` emits a warning because it lands after the closing delimiter, not before it. **Note on leading trivia:** whether a decorator/doc comment belongs to `^` depends on the parser. In Rust and Python, attributes and decorators are attached to the function chunk, so `^` covers them. In TypeScript/JavaScript, a `@decorator` + `/** jsdoc */` block immediately above a method often surfaces as a **separate sibling chunk** (shown as `chunk#ID` in the `?` listing) rather than as part of the function's `^`. JSDoc directly above a plain function is more likely to be absorbed into that function's `^`. If you need to rewrite a decorated member, run `open(path="file", sel="?")` and check for a sibling `chunk#ID` directly above your target. -**Python notes:** Python docstrings are body lines, not head lines. A `~` body write on a function that has a docstring deletes the docstring unless you include the docstring in `content`. Python enum members and nested functions/closures are often opaque inside their parent chunk and may not appear as addressable child chunks; use `replace` on the parent chunk or rewrite the parent container body. +**Python notes:** Python docstrings are body lines, not head lines. A `~` body write on a function that has a docstring deletes the docstring unless you include the docstring in `content`. Python enum members and nested functions/closures are often opaque inside their parent chunk and may not appear as addressable child chunks; rewrite the parent container body. Python decorated class/function `^` writes and Python `^` deletes are rejected because indentation-sensitive bodies can become attached to the wrong block while still parsing. -**Note on non-code formats:** for prose and data formats (markdown, YAML, JSON, frontmatter), unsupported `^` and `~` suffixes warn and fall back to whole-chunk editing. Always replace the entire chunk and include any delimiter syntax (fence backticks, `---` frontmatter markers, list markers, table rows, headings) in your `content` — omitting them deletes them. For markdown sections (`sect_*`), prefer unsuffixed whole-chunk replace because `^`/`~` on prose sections can replace the heading too. Fenced code blocks are the exception when the embedded language parser exposes inner chunks; otherwise read with `raw` first and preserve the exact whitespace inside fences. Be cautious appending to markdown tables or lists: if spacing is delicate, rewrite the whole table/list chunk so blank lines and row continuity stay under your control. To insert content after a markdown section heading, use `after` on the heading chunk (`sect_*.chunk` or `sect_*.chunk_1`) — not `before`/`prepend` on the section itself, which lands physically before the heading and gets absorbed by the preceding section on reparse. +**Note on non-code formats:** for prose and data formats (markdown, YAML, JSON, frontmatter), unsupported `^` and `~` suffixes warn and fall back to whole-chunk editing. Always replace the entire chunk and include any delimiter syntax (fence backticks, `---` frontmatter markers, list markers, table rows, headings) in your `content` — omitting them deletes them. For markdown sections (`sect_*`), prefer unsuffixed whole-chunk replace because `^`/`~` on prose sections can replace the heading and child content too; if you only need the heading, target the heading child chunk shown in `sel="?"`. Fenced code blocks with a declared language are parsed again and can expose inner chunks such as `code_py#ID.fn_gre#ID`; target those inner chunks when available. Markdown root writes preserve fenced code indentation verbatim. Recognized pipe tables expose `row_N` children for row-level edits; table cells and list items are not independently addressable, so rewrite the whole list/table chunk for those structural changes. Appending a table-row-shaped string (`| value |`) to a table chunk inserts it before the trailing blank-line separator so it remains part of the table. Otherwise read with `raw` first and preserve the exact whitespace inside fences. To insert content after a markdown section heading, use `after` on the heading chunk (`sect_*.chunk` or `sect_*.chunk_1`) — not `before`/`prepend` on the section itself, which lands physically before the heading and gets absorbed by the preceding section on reparse. -Each edit entry has `path` (`file:selector`) plus **exactly one** operation field — `write`, `replace`, or `insert`. Never set more than one on the same entry. +Each edit entry has `path` (`file:selector`) plus **exactly one** operation field — `read`, `write`, `insert`, or `delete`. Never set more than one on the same entry. `write:null`, `write:""`, and bare `{path}` entries are rejected; they do not read or delete. |fields|path (selector part)|effect| |---|---|---| +|`read: true`|`file`, `file:?`, `file:chunk#ID`, `file:chunk#ID~`, or `file:chunk#ID^`|return chunk source without modifying the file| |`write: "content"`|`file:chunk#ID`, `file:chunk#ID~`, or `file:chunk#ID^`|write complete new content to the region| -|`write: null`|`file:chunk#ID`|delete the chunk| -|`replace: {old, new}`|`file:chunk#ID`|find a literal substring in the chunk and replace it| +|`delete: true`|`file:chunk#ID`|delete the chunk explicitly| |`insert: {loc, body}`|`file:chunk` or `file:chunk~`|insert before/after the chunk (`loc`: `"prepend"` or `"append"`)| -Given this `read` output for `counter.rs`: +Given this `open` output for `counter.rs`: ``` | counter.rs·62L·rust·#ZRPW | @@ -151,7 +158,7 @@ Given this `read` output for `counter.rs`: 61 |} ``` -**Understanding `^` markers in `read` output:** Lines marked with `^` between the line number and `|` (e.g. ` 3^|`) are **head** lines — doc comments, attributes, and the signature. Lines without `^` (e.g. ` 7 |`) are **body** lines. `~` replaces body lines only, keeping head lines intact. +**Understanding `^` markers in `open` output:** Lines marked with `^` between the line number and `|` (e.g. ` 3^|`) are **head** lines — doc comments, attributes, and the signature. Lines without `^` (e.g. ` 7 |`) are **body** lines. `~` replaces body lines only, keeping head lines intact. **Put body** (`~` — the common case): ``` @@ -190,12 +197,6 @@ Result — the head (all `^` lines + opening brace) changes, body untouched: } ``` -**Find and replace** (surgical edit within a chunk): -``` -{ "path": "counter.rs:impl_Counte.fn_increm#MNHV", "replace": { "old": "self.value += 1;", "new": "self.value = (self.value + 1).min(self.max);" } } -``` -Result — only the matched substring changes, everything else is preserved. - **Insert before a chunk** (`prepend`): ``` { "path": "counter.rs:impl_Counte.fn_get", "insert": { "loc": "prepend", "body": "/// Resets the counter to zero.\npub fn reset(&mut self) {\n\tself.value = 0;\n}\n\n" } } @@ -263,7 +264,7 @@ Result — a new method is added at the end of the impl body, before the closing **Delete a chunk**: ``` -{ "path": "counter.rs:impl_Counte.fn_decrem#TTWB", "write": null } +{ "path": "counter.rs:impl_Counte.fn_decrem#TTWB", "delete": true } ``` Result — the method (including its doc comment and signature) is removed. - Indentation rules (important): @@ -275,11 +276,10 @@ Result — the method (including its doc comment and signature) is removed. - Do NOT include the chunk's base indentation — only indent relative to the region's opening level. - For `write`, the tool strips common leading whitespace shared by all non-empty lines, then adds the target region's base indent. If lines have mixed relative indentation, write them at column 0 so the common-margin cleanup cannot change the structure. - For `~` of a function: write at column 0, and use `\t` for *relative* nesting. Flat body: `"return x;\n"`. Multiple sibling lines: `"print(a)\nprint(b)\nprint(c)\n"` — all at column 0, the tool adds the function's base indent. Nested body: `"if (cond) {\n\treturn x;\n}\n"` — the `if` is at column 0, the `return` is one tab in. Python example — to replace `~` of `def divide(a, b):`, write: `"if b == 0:\n\treturn None\nreturn a / b\n"` — the `if` and `return a / b` are at column 0, `return None` is one `\t` in. - - For `^`: write at the chunk's own depth. A class member's head uses `"/// doc\n#[attr]\npub fn start() {"`. + - For `^`: write at column 0 relative to the head region, just like `~`. A class member's head uses `"/// doc\n#[attr]\npub fn start() {"` — do not include the class/member base indentation. {{#if chunkAutoIndent}} - For a top-level item: start at zero indent. Write `"fn foo() {\n\treturn 1;\n}\n"`. {{else}} - For a top-level item: start at zero indent. Write `"fn foo() {\n return 1;\n}\n"`. {{/if}} - - `replace: {old, new}` does not use the same multiline reindent model. Single-line `new` text inherits the matched line's indentation. Multiline `new` text is inserted literally, so include the exact leading whitespace for every inserted line; when the first replacement line repeats the matched line's existing indentation prefix, the tool consumes that prefix to prevent double indentation. diff --git a/packages/coding-agent/src/prompts/tools/open-chunk.md b/packages/coding-agent/src/prompts/tools/open-chunk.md index c5b409595..1cef17f8f 100644 --- a/packages/coding-agent/src/prompts/tools/open-chunk.md +++ b/packages/coding-agent/src/prompts/tools/open-chunk.md @@ -1,7 +1,7 @@ Reads files using syntax-aware chunks. Also inspects directories, archives, SQLite databases, images, documents (PDF/DOCX/PPTX/XLSX/RTF/EPUB/ipynb), **and URLs**. -The chunk-aware `open` variant returns AST-scoped chunks with stable IDs for structural editing, and otherwise behaves like `open` for non-code content. +The chunk-aware `open` variant returns AST-scoped chunks with current checksum IDs for structural editing, and otherwise behaves like `open` for non-code content. - You **MUST** parallelize calls when exploring related files - For URLs, `open` fetches the page and returns clean extracted text/markdown by default (reader-mode). It handles HTML pages, GitHub issues/PRs, Stack Overflow, Wikipedia, Reddit, NPM, arXiv, RSS/Atom, JSON endpoints, PDFs, etc. You **SHOULD** reach for `open` — not a browser/puppeteer tool — for fetching and inspecting web content. @@ -28,7 +28,7 @@ Max {{DEFAULT_MAX_LINES}} lines per call. # Chunks Each anchor `@full.chunk.path#CCCC` (with `-` prefixes for nesting depth) in the output identifies a chunk. Use `full.chunk.path#CCCC` as-is to read truncated chunks. -If you need a canonical target list, run `open(path="file", sel="?")`. That listing shows chunk paths with IDs. +If you need a canonical target list, run `open(path="file", sel="?")`. That listing shows chunk paths with IDs and is the safest structural discovery mode. Summary lines in this listing are orientation hints; follow a selector with `open(path="file", sel="chunk#ID")` or use `raw` when you need exact source. Line numbers in the gutter are absolute file line numbers. {{#if chunkAutoIndent}} @@ -38,9 +38,10 @@ Chunk reads preserve literal leading tabs/spaces from the file. When editing, ke {{/if}} `raw` shows the file's literal whitespace. Structured chunk views may normalize or display indentation for edit round-tripping, so use `raw` when exact tabs/spaces matter, especially inside markdown fenced code blocks. -IDs change after every edit. Use the new IDs from the edit response or refresh with `sel="?"` before the next write. +IDs change after every edit. Use the new IDs from the edit response or refresh with `sel="?"` before the next `write`/`delete`. `insert` selectors may omit IDs, but still prefer fresh paths after structural edits. -Parser boundaries vary by language: TypeScript/JavaScript decorators and JSDoc above decorated methods may appear as sibling `chunk#ID` entries, Python docstrings are body lines, and Python enum members or nested closures may remain opaque inside their parent chunk. +Parser boundaries vary by language: TypeScript/JavaScript decorators and JSDoc above decorated methods may appear as sibling `chunk#ID` entries, Python decorators are part of the function/class head, Python docstrings are body lines, and Python enum members or nested closures may remain opaque inside their parent chunk. Decorated Python `^` writes and Python `^` deletes are rejected for safety. +Markdown sections, lists, and tables are structural chunks. Recognized pipe tables expose `row_N` children for row-level edits; list items and table cells are not independently addressable. Fenced code blocks with a declared language are parsed again when possible, so functions inside a markdown fence can appear as addressable nested chunks. Chunk trees: JS, TS, TSX, Python, Rust, Go. Others use blank-line fallback. # Inspection diff --git a/packages/coding-agent/test/edit-diff.test.ts b/packages/coding-agent/test/edit-diff.test.ts index 3069682df..40340236d 100644 --- a/packages/coding-agent/test/edit-diff.test.ts +++ b/packages/coding-agent/test/edit-diff.test.ts @@ -194,6 +194,67 @@ describe("computeChunkDiff", () => { expect("error" in result).toBe(true); }); + test("returns an empty preview for read-only chunk reads", async () => { + const file = path.join(tmpDir, "read.ts"); + await fs.writeFile(file, "export const x = 1;\n"); + const result = await computeChunkDiff( + { + path: "read.ts", + edits: [{ path: "read.ts:?", read: true }], + }, + tmpDir, + ); + expect(result).toEqual({ diff: "", firstChangedLine: undefined }); + }); + + test("rejects write:null instead of previewing a delete", async () => { + const file = path.join(tmpDir, "null-delete.ts"); + await fs.writeFile(file, "export const x = 1;\n"); + const result = await computeChunkDiff( + { + path: "null-delete.ts", + edits: [{ path: "null-delete.ts", write: null }], + }, + tmpDir, + ); + expect("error" in result).toBe(true); + if ("error" in result) { + expect(result.error).toContain("write:null no longer deletes chunks"); + } + }); + + test("rejects bare chunk edit entries instead of treating them as deletes", async () => { + const file = path.join(tmpDir, "bare.ts"); + await fs.writeFile(file, "export const x = 1;\n"); + const result = await computeChunkDiff( + { + path: "bare.ts", + edits: [{ path: "bare.ts" }], + }, + tmpDir, + ); + expect("error" in result).toBe(true); + if ("error" in result) { + expect(result.error).toContain("no operation specified"); + } + }); + + test("rejects write empty string instead of previewing a destructive empty replacement", async () => { + const file = path.join(tmpDir, "empty-write.ts"); + await fs.writeFile(file, "export const x = 1;\n"); + const result = await computeChunkDiff( + { + path: "empty-write.ts", + edits: [{ path: "empty-write.ts", write: "" }], + }, + tmpDir, + ); + expect("error" in result).toBe(true); + if ("error" in result) { + expect(result.error).toContain('write:"" is a destructive empty replacement'); + } + }); + test("aborts when signal fires before compute completes", async () => { const controller = new AbortController(); controller.abort(); diff --git a/packages/coding-agent/test/edit-streaming-preview.test.ts b/packages/coding-agent/test/edit-streaming-preview.test.ts index 888d368ca..b559db165 100644 --- a/packages/coding-agent/test/edit-streaming-preview.test.ts +++ b/packages/coding-agent/test/edit-streaming-preview.test.ts @@ -62,8 +62,8 @@ describe("chunk extractCompleteEdits", () => { __partialJson: '{"edits":[{"path":"a.ts","write":"foo"},{"path":"b.ts","write":nu', }; const out = strategy.extractCompleteEdits(args, args.__partialJson) as typeof args; - // Last entry should be dropped because its `}` hasn't arrived yet — so - // the "delete" rendering is suppressed while streaming. + // Last entry should be dropped because its `}` hasn't arrived yet, so + // incomplete null-write errors are suppressed while streaming. expect(out.edits).toHaveLength(1); expect(out.edits[0].path).toBe("a.ts"); }); diff --git a/packages/coding-agent/test/tools/bash-sixel-render.test.ts b/packages/coding-agent/test/tools/bash-sixel-render.test.ts index dd29d9423..3d6d8ce44 100644 --- a/packages/coding-agent/test/tools/bash-sixel-render.test.ts +++ b/packages/coding-agent/test/tools/bash-sixel-render.test.ts @@ -77,7 +77,7 @@ describe("bashToolRenderer", () => { { content: [{ type: "text", text: "" }], details: { timeoutSeconds: 120 }, isError: false }, { expanded: false, isPartial: false, renderContext: { timeout: 1200 } }, uiTheme, - { command: "python3 scripts/vim-edit-benchmark.py", timeout: 1200 }, + { command: "python3 scripts/edit-benchmark.py", timeout: 1200 }, ); const rendered = sanitizeText(component.render(120).join("\n")); expect(rendered).toContain("Timeout: 120s");