diff --git a/crates/pi-natives/src/chunk/ast_ipynb.rs b/crates/pi-natives/src/chunk/ast_ipynb.rs index fccafd29c..ff6e28687 100644 --- a/crates/pi-natives/src/chunk/ast_ipynb.rs +++ b/crates/pi-natives/src/chunk/ast_ipynb.rs @@ -406,8 +406,8 @@ pub fn build_notebook_tree_from_virtual( start_byte: 0, end_byte: virtual_source.len() as u32, checksum_start_byte: 0, - body_start_byte: None, - body_end_byte: None, + prologue_end_byte: None, + epilogue_start_byte: None, checksum: root_checksum.clone(), error: false, indent: 0, @@ -471,11 +471,11 @@ pub fn build_notebook_tree_from_virtual( checksum_start_byte: sub_chunk .checksum_start_byte .saturating_add(region.content_start as u32), - body_start_byte: sub_chunk - .body_start_byte + prologue_end_byte: sub_chunk + .prologue_end_byte .map(|b| b.saturating_add(region.content_start as u32)), - body_end_byte: sub_chunk - .body_end_byte + epilogue_start_byte: sub_chunk + .epilogue_start_byte .map(|b| b.saturating_add(region.content_start as u32)), checksum: sub_chunk.checksum, error: sub_chunk.error, @@ -518,8 +518,8 @@ pub fn build_notebook_tree_from_virtual( start_byte: region.marker_start as u32, end_byte: region.content_end as u32, checksum_start_byte: region.content_start as u32, - body_start_byte: Some(region.content_start as u32), - body_end_byte: Some(region.content_end as u32), + prologue_end_byte: Some(region.content_start as u32), + epilogue_start_byte: Some(region.content_end as u32), checksum: cell_checksum, error: false, indent: 0, diff --git a/crates/pi-natives/src/chunk/ast_tlaplus.rs b/crates/pi-natives/src/chunk/ast_tlaplus.rs index 94af892a9..4404902ad 100644 --- a/crates/pi-natives/src/chunk/ast_tlaplus.rs +++ b/crates/pi-natives/src/chunk/ast_tlaplus.rs @@ -266,8 +266,8 @@ fn translation_chunk( start_byte, end_byte, checksum_start_byte: start_byte, - body_start_byte: None, - body_end_byte: None, + prologue_end_byte: None, + epilogue_start_byte: None, checksum, error: false, indent: 0, diff --git a/crates/pi-natives/src/chunk/common.rs b/crates/pi-natives/src/chunk/common.rs index 1f931df5e..815ab7ad7 100644 --- a/crates/pi-natives/src/chunk/common.rs +++ b/crates/pi-natives/src/chunk/common.rs @@ -270,6 +270,62 @@ pub fn child_by_field_or_kind<'tree>( child_by_kind(node, kinds) } +pub fn compute_body_inner_boundaries( + source: &str, + body_start: usize, + body_end: usize, +) -> (usize, usize) { + let bounded_start = body_start.min(source.len()); + let bounded_end = body_end.min(source.len()).max(bounded_start); + let slice = &source[bounded_start..bounded_end]; + + let Some((first_non_ws_rel, first_non_ws)) = slice + .char_indices() + .find(|(_, ch)| !matches!(ch, ' ' | '\t' | '\n' | '\r')) + else { + return (bounded_start, bounded_end); + }; + let Some((last_non_ws_rel, last_non_ws)) = slice + .char_indices() + .rev() + .find(|(_, ch)| !matches!(ch, ' ' | '\t' | '\n' | '\r')) + else { + return (bounded_start, bounded_end); + }; + + let has_delimiters = matches!((first_non_ws, last_non_ws), ('{', '}') | ('(', ')') | ('[', ']')); + if !has_delimiters { + let line_start = source[..bounded_start].rfind('\n').map_or(0, |pos| pos + 1); + let leading_indent = &source[line_start..bounded_start]; + if !leading_indent.is_empty() && leading_indent.chars().all(|ch| matches!(ch, ' ' | '\t')) { + let mut inner_end = bounded_end; + let trailing = &source[bounded_end..]; + if let Some(rel_newline) = trailing.find('\n') { + if trailing[..rel_newline] + .chars() + .all(|ch| matches!(ch, ' ' | '\t' | '\r')) + { + inner_end = bounded_end + rel_newline + 1; + } + } else if trailing.chars().all(|ch| matches!(ch, ' ' | '\t' | '\r')) { + inner_end = source.len(); + } + return (line_start, inner_end); + } + return (bounded_start, bounded_end); + } + + let mut inner_start = bounded_start + first_non_ws_rel + first_non_ws.len_utf8(); + if source[inner_start..].starts_with("\r\n") { + inner_start += 2; + } else if source[inner_start..].starts_with('\n') { + inner_start += 1; + } + + let inner_end = bounded_start + last_non_ws_rel; + (inner_start.min(bounded_end), inner_end.max(inner_start).min(bounded_end)) +} + // ── Recurse helpers ────────────────────────────────────────────────────── pub fn recurse_into<'tree>( @@ -859,3 +915,37 @@ pub fn first_scalar_child(node: Node<'_>) -> Option> { ) }) } + +#[cfg(test)] +mod tests { + use super::compute_body_inner_boundaries; + + #[test] + fn compute_body_inner_boundaries_handles_brace_and_indent_bodies() { + let ts = "function main() {\n\treturn 1;\n}\n"; + let ts_start = ts.find('{').expect("open brace"); + let ts_end = ts.rfind('}').expect("close brace") + 1; + let (ts_inner_start, ts_inner_end) = compute_body_inner_boundaries(ts, ts_start, ts_end); + assert_eq!(&ts[ts_inner_start..ts_inner_end], "\treturn 1;\n"); + + let rust = "fn main() {\n println!(\"hi\");\n}\n"; + let rust_start = rust.find('{').expect("open brace"); + let rust_end = rust.rfind('}').expect("close brace") + 1; + let (rust_inner_start, rust_inner_end) = + compute_body_inner_boundaries(rust, rust_start, rust_end); + assert_eq!(&rust[rust_inner_start..rust_inner_end], " println!(\"hi\");\n"); + + let go = "func main() {\n\treturn\n}\n"; + let go_start = go.find('{').expect("open brace"); + let go_end = go.rfind('}').expect("close brace") + 1; + let (go_inner_start, go_inner_end) = compute_body_inner_boundaries(go, go_start, go_end); + assert_eq!(&go[go_inner_start..go_inner_end], "\treturn\n"); + + let py = "def main():\n return 1\n"; + let py_body_start = py.find(" return 1").expect("body start"); + let py_body_end = py_body_start + " return 1".len(); + let (py_inner_start, py_inner_end) = + compute_body_inner_boundaries(py, py_body_start, py_body_end); + assert_eq!(&py[py_inner_start..py_inner_end], " return 1"); + } +} diff --git a/crates/pi-natives/src/chunk/edit.rs b/crates/pi-natives/src/chunk/edit.rs index 1f15290d3..3e550d2ff 100644 --- a/crates/pi-natives/src/chunk/edit.rs +++ b/crates/pi-natives/src/chunk/edit.rs @@ -1,17 +1,18 @@ -use std::{cmp::Ordering, path::Path}; +use std::path::Path; use crate::chunk::{ indent::{ - detect_file_indent_char, detect_file_indent_step, normalize_leading_whitespace_char, - reindent_inserted_block, strip_content_prefixes, + denormalize_from_tabs, detect_file_indent_char, detect_file_indent_step, + normalize_leading_whitespace_char, reindent_inserted_block, strip_content_prefixes, }, resolve::{ - resolve_chunk_selector, resolve_chunk_with_crc, sanitize_chunk_selector, sanitize_crc, + chunk_region_range, chunk_supports_region, resolve_chunk_selector, resolve_chunk_with_crc, + sanitize_chunk_selector, sanitize_crc, }, state::{ChunkState, ChunkStateInner}, types::{ - ChunkAnchorStyle, ChunkEditOp, ChunkFocusMode, ChunkNode, EditOperation, EditParams, - EditResult, FocusedPath, RenderParams, + ChunkAnchorStyle, ChunkEditOp, ChunkFocusMode, ChunkNode, ChunkRegion, EditOperation, + EditParams, EditResult, FocusedPath, RenderParams, }, }; @@ -23,7 +24,7 @@ struct ScheduledEditOperation { initial_chunk: Option, } -#[derive(Clone, Copy)] +#[derive(Clone, Copy, PartialEq, Eq)] enum InsertPosition { Before, After, @@ -56,6 +57,12 @@ struct InsertionPoint { indent: String, } +#[derive(Clone)] +struct ResolvedEditTarget { + chunk: ChunkNode, + region: ChunkRegion, +} + pub fn apply_edits(state: &ChunkState, params: &EditParams) -> Result { let original_text = normalize_chunk_source(state.inner().source()); let initial_notebook_ctx = state.inner().notebook.clone(); @@ -122,31 +129,19 @@ pub fn apply_edits(state: &ChunkState, params: &EditParams) -> Result apply_replace_body( - &mut state, - &operation, - &scheduled, - current_default_selector, - current_default_crc.as_deref(), - file_indent_step, - file_indent_char, - &mut touched_paths, - &mut warnings, - ), - ChunkEditOp::AppendChild - | ChunkEditOp::PrependChild - | ChunkEditOp::AppendSibling - | ChunkEditOp::PrependSibling => apply_insert( - &mut state, - &operation, - &scheduled, - current_default_selector, - current_default_crc.as_deref(), - file_indent_step, - file_indent_char, - &mut touched_paths, - &mut warnings, - ), + ChunkEditOp::Before | ChunkEditOp::After | ChunkEditOp::Prepend | ChunkEditOp::Append => { + apply_insert( + &mut state, + &operation, + &scheduled, + current_default_selector, + current_default_crc.as_deref(), + file_indent_step, + file_indent_char, + &mut touched_paths, + &mut warnings, + ) + }, }; if let Err(err) = result { @@ -271,6 +266,38 @@ pub fn apply_edits(state: &ChunkState, params: &EditParams) -> Result, + default_crc: Option<&str>, + requires_checksum: bool, + touched_paths: &[String], + warnings: &mut Vec, +) -> Result { + let selector = operation.sel.as_deref().or(default_selector); + let crc = operation.crc.as_deref().or_else(|| { + if operation.sel.is_none() { + default_crc + } else { + None + } + }); + let batch_auto_accepted = ensure_batch_operation_target_current(scheduled, crc, touched_paths); + let resolve_crc = if batch_auto_accepted { None } else { crc }; + let resolved = resolve_chunk_with_crc(state, selector, resolve_crc, warnings)?; + let region = operation.region.unwrap_or(resolved.region); + if !batch_auto_accepted { + validate_batch_crc(resolved.chunk, resolved.crc.as_deref(), requires_checksum)?; + } + let chunk = resolved.chunk.clone(); + if !chunk_supports_region(&chunk, region) { + return Err(format!("Chunk \"{}\" does not support @{}.", chunk.path, region.as_str())); + } + Ok(ResolvedEditTarget { chunk, region }) +} + fn apply_replace( state: &mut ChunkStateInner, operation: &EditOperation, @@ -282,24 +309,21 @@ fn apply_replace( touched_paths: &mut Vec, warnings: &mut Vec, ) -> Result<(), String> { - let anchor_selector = operation.sel.as_deref().or(default_selector); - let crc = operation.crc.as_deref().or_else(|| { - if operation.sel.is_none() { - default_crc - } else { - None - } - }); - let requires_checksum = operation.sel.is_some() || default_crc.is_some(); - let batch_auto_accepted = ensure_batch_operation_target_current(scheduled, crc, touched_paths); - // When auto-accepted, strip CRC so resolution finds by path only (CRC is stale - // from pre-batch). - let resolve_crc = if batch_auto_accepted { None } else { crc }; - let resolved = resolve_chunk_with_crc(state, anchor_selector, resolve_crc, warnings)?; - if !batch_auto_accepted { - validate_batch_crc(resolved.chunk, resolved.crc.as_deref(), requires_checksum)?; - } - let anchor = resolved.chunk.clone(); + let target = resolve_edit_target( + state, + operation, + scheduled, + default_selector, + default_crc, + true, + touched_paths.as_slice(), + warnings, + )?; + let anchor = target.chunk; + let (region_start, region_end) = match target.region { + ChunkRegion::Container => (anchor.start_byte as usize, anchor.end_byte as usize), + _ => chunk_region_range(&anchor, target.region)?, + }; // Scoped find/replace: locate a literal substring inside the chunk and replace // it. @@ -311,9 +335,7 @@ fn apply_replace( )); } - let chunk_start = anchor.start_byte as usize; - let chunk_end = anchor.end_byte as usize; - let chunk_source = &state.source[chunk_start..chunk_end]; + let chunk_source = &state.source[region_start..region_end]; let mut matches = chunk_source.match_indices(find); let Some((rel_offset, _)) = matches.next() else { return Err(format!( @@ -332,7 +354,7 @@ fn apply_replace( } let replacement = operation.content.as_deref().unwrap_or_default(); - let abs_start = chunk_start + rel_offset; + let abs_start = region_start + rel_offset; let abs_end = abs_start + find.len(); let mut new_source = String::with_capacity(state.source.len() - find.len() + replacement.len()); @@ -344,119 +366,29 @@ fn apply_replace( return Ok(()); } - let target_indent = anchor.indent_char.repeat(anchor.indent as usize); + let target_indent = + 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, &target_indent, Some(file_indent_step), file_indent_char); - replacement = preserve_attached_leading_trivia(state, &anchor, &replacement); - if !replacement.is_empty() - && !replacement.ends_with('\n') - && anchor.end_line < state.tree.line_count - { - replacement.push('\n'); - } - let offsets = line_offsets(&state.source); - let range_start = line_start_offset(&offsets, anchor.start_line, &state.source); - state.source = - replace_range_by_lines(&state.source, anchor.start_line, anchor.end_line, &replacement); - if replacement.is_empty() { - state.source = cleanup_blank_line_artifacts_at_offset(&state.source, range_start); - } - touched_paths.push(anchor.path); - Ok(()) -} - -/// Replace only the inner body of a chunk, preserving the signature line(s) -/// and closing delimiter. -fn apply_replace_body( - state: &mut ChunkStateInner, - operation: &EditOperation, - scheduled: &ScheduledEditOperation, - default_selector: Option<&str>, - default_crc: Option<&str>, - file_indent_step: usize, - file_indent_char: char, - touched_paths: &mut Vec, - warnings: &mut Vec, -) -> Result<(), String> { - let anchor_selector = operation.sel.as_deref().or(default_selector); - let crc = operation.crc.as_deref().or_else(|| { - if operation.sel.is_none() { - default_crc - } else { - None + if target.region == ChunkRegion::Container { + replacement = preserve_attached_leading_trivia(state, &anchor, &replacement); + if !replacement.is_empty() + && !replacement.ends_with('\n') + && anchor.end_line < state.tree.line_count + { + replacement.push('\n'); + } + let offsets = line_offsets(&state.source); + let range_start = line_start_offset(&offsets, anchor.start_line, &state.source); + state.source = + replace_range_by_lines(&state.source, anchor.start_line, anchor.end_line, &replacement); + if replacement.is_empty() { + state.source = cleanup_blank_line_artifacts_at_offset(&state.source, range_start); } - }); - let requires_checksum = operation.sel.is_some() || default_crc.is_some(); - let batch_auto_accepted = ensure_batch_operation_target_current(scheduled, crc, touched_paths); - let resolve_crc = if batch_auto_accepted { None } else { crc }; - let resolved = resolve_chunk_with_crc(state, anchor_selector, resolve_crc, warnings)?; - if !batch_auto_accepted { - validate_batch_crc(resolved.chunk, resolved.crc.as_deref(), requires_checksum)?; - } - let anchor = resolved.chunk.clone(); - - let body_start = anchor.body_start_byte.ok_or_else(|| { - format!( - "replace_body on {}: chunk has no body range (not a function/method/class).", - anchor.path - ) - })? as usize; - let body_end = anchor.body_end_byte.ok_or_else(|| { - format!( - "replace_body on {}: chunk has no body range (not a function/method/class).", - anchor.path - ) - })? as usize; - - // Determine inner body boundaries. For brace-delimited bodies (most - // languages), we want to replace only between the opening `{` and - // closing `}`. For Python (colon + indented block), body_start is - // already the first statement. - let body_slice = &state.source[body_start..body_end]; - - let (inner_start, inner_end) = if let Some(open_brace) = body_slice.find('{') { - // Find the closing brace from the end. - let close_brace = body_slice.rfind('}').unwrap_or(body_slice.len()); - let abs_open = body_start + open_brace + 1; // after '{' - let abs_close = body_start + close_brace; // before '}' - // Skip the newline after '{' if present. - let start = if state.source.as_bytes().get(abs_open) == Some(&b'\n') { - abs_open + 1 - } else { - abs_open - }; - // Include trailing newline before '}' if the close is on its own line. - let end = if abs_close > 0 && state.source.as_bytes().get(abs_close - 1) == Some(&b'\n') { - abs_close - 1 - } else { - abs_close - }; - (start, end.max(start)) } else { - // No braces (e.g. Python): replace the entire body range. - (body_start, body_end) - }; - - // Determine the target indent level for the body content. - let target_indent_level = anchor.indent as usize + file_indent_step; - let target_indent = - std::iter::repeat_n(file_indent_char, target_indent_level).collect::(); - let content = operation.content.as_deref().unwrap_or_default(); - let replacement = - normalize_inserted_content(content, &target_indent, Some(file_indent_step), file_indent_char); - - // Build the new source: [before inner] + replacement + newline + [after inner] - let mut new_source = String::with_capacity(state.source.len()); - new_source.push_str(&state.source[..inner_start]); - if !replacement.is_empty() { - new_source.push_str(&replacement); - if !replacement.ends_with('\n') { - new_source.push('\n'); - } + state.source = replace_byte_range(&state.source, region_start, region_end, &replacement); } - new_source.push_str(&state.source[inner_end..]); - state.source = new_source; touched_paths.push(anchor.path); Ok(()) } @@ -470,27 +402,27 @@ fn apply_delete( touched_paths: &mut Vec, warnings: &mut Vec, ) -> Result<(), String> { - let anchor_selector = operation.sel.as_deref().or(default_selector); - let crc = operation.crc.as_deref().or_else(|| { - if operation.sel.is_none() { - default_crc - } else { - None - } - }); - let requires_checksum = operation.sel.is_some() || default_crc.is_some(); - let batch_auto_accepted = ensure_batch_operation_target_current(scheduled, crc, touched_paths); - let resolve_crc = if batch_auto_accepted { None } else { crc }; - let resolved = resolve_chunk_with_crc(state, anchor_selector, resolve_crc, warnings)?; - if !batch_auto_accepted { - validate_batch_crc(resolved.chunk, resolved.crc.as_deref(), requires_checksum)?; - } - let anchor = resolved.chunk.clone(); + let target = resolve_edit_target( + state, + operation, + scheduled, + default_selector, + default_crc, + true, + touched_paths.as_slice(), + warnings, + )?; + let anchor = target.chunk; - let offsets = line_offsets(&state.source); - let range_start = line_start_offset(&offsets, anchor.start_line, &state.source); - state.source = replace_range_by_lines(&state.source, anchor.start_line, anchor.end_line, ""); - state.source = cleanup_blank_line_artifacts_at_offset(&state.source, range_start); + if target.region == ChunkRegion::Container { + let offsets = line_offsets(&state.source); + let range_start = line_start_offset(&offsets, anchor.start_line, &state.source); + state.source = replace_range_by_lines(&state.source, anchor.start_line, anchor.end_line, ""); + state.source = cleanup_blank_line_artifacts_at_offset(&state.source, range_start); + } else { + let (range_start, range_end) = chunk_region_range(&anchor, target.region)?; + state.source = replace_byte_range(&state.source, range_start, range_end, ""); + } touched_paths.push(anchor.path); Ok(()) } @@ -506,40 +438,25 @@ fn apply_insert( touched_paths: &mut Vec, warnings: &mut Vec, ) -> Result<(), String> { - let anchor_selector = operation.sel.as_deref().or(default_selector); - let crc = operation.crc.as_deref().or_else(|| { - if operation.sel.is_none() { - default_crc - } else { - None - } - }); - let batch_auto_accepted = ensure_batch_operation_target_current(scheduled, crc, touched_paths); - let resolve_crc = if batch_auto_accepted { None } else { crc }; - let resolved = resolve_chunk_with_crc(state, anchor_selector, resolve_crc, warnings)?; - if !batch_auto_accepted { - validate_batch_crc(resolved.chunk, resolved.crc.as_deref(), resolved.crc.is_some())?; - } - let anchor = resolved.chunk.clone(); - - let pos = match operation.op { - ChunkEditOp::AppendChild => InsertPosition::LastChild, - ChunkEditOp::PrependChild => InsertPosition::FirstChild, - ChunkEditOp::AppendSibling => InsertPosition::After, - ChunkEditOp::PrependSibling => InsertPosition::Before, - ChunkEditOp::Replace | ChunkEditOp::Delete | ChunkEditOp::ReplaceBody => { - return Err("Internal error: insert position requested for non-insert op".to_owned()); - }, - }; - let insertion = get_insertion_point_for_position( + let target = resolve_edit_target( + state, + operation, + scheduled, + default_selector, + default_crc, + false, + touched_paths.as_slice(), + warnings, + )?; + let anchor = target.chunk; + let (insertion, pos) = resolve_insertion_point( state, &anchor, - pos, - if matches!(pos, InsertPosition::FirstChild | InsertPosition::LastChild) { - operation.content.as_deref() - } else { - None - }, + target.region, + operation.op, + operation.content.as_deref(), + file_indent_char, + file_indent_step, )?; let spacing = compute_insert_spacing(state, &anchor, pos); let content = operation.content.as_deref().unwrap_or_default(); @@ -552,7 +469,7 @@ fn apply_insert( replacement = normalize_insertion_boundary_content(state, insertion.offset, &replacement, spacing); - if operation.op == ChunkEditOp::PrependChild { + if pos == InsertPosition::FirstChild { let body = replacement.trim_matches('\n'); let comment_only = !body.is_empty() && body.lines().all(|line| { @@ -568,14 +485,14 @@ fn apply_insert( && anchor.children.iter().any(|child| child == "preamble") { return Err( - "Comment-only prepend_child on root is not allowed when the file has a preamble \ + "Comment-only @body.prepend on root is not allowed when the file has a preamble \ chunk. Use replace on the preamble chunk instead." .to_owned(), ); } if comment_only && !anchor.children.is_empty() { warnings.push( - "Comment-only prepend_child can merge into the following chunk's first line; it is \ + "Comment-only @body.prepend can merge into the following chunk's first line; it is \ not a separate named chunk." .to_owned(), ); @@ -634,19 +551,24 @@ fn validate_batch_crc(chunk: &ChunkNode, crc: Option<&str>, required: bool) -> R fn validate_crc(chunk: &ChunkNode, crc: Option<&str>) -> Result<(), String> { let cleaned = sanitize_crc(crc).ok_or_else(|| { + let selector = if chunk.path.is_empty() { + format!("#{}@container", chunk.checksum) + } else { + format!("{}#{}@container", chunk.path, chunk.checksum) + }; format!( - "Checksum required for {}. Re-read the chunk to get the current checksum, then pass crc: \ - \"XXXX\" in your edit operation. Hint: use crc: \"{}\" with sel: \"{}\".", + "Checksum required for {}. Re-read the chunk to get the current checksum, then include \ + it in the selector. Hint: use target \"{}\" for container replacement, or append \ + another region such as @body.", chunk_path_opt(chunk), - chunk.checksum, - chunk.path + selector ) })?; if chunk.checksum != cleaned { return Err(format!( "Checksum mismatch for {}: expected \"{}\", got \"{}\". The chunk content has changed \ since you last read it. Re-read the file to get updated checksums, then retry with the \ - new crc value.", + refreshed target selector.", chunk_path_opt(chunk), chunk.checksum, cleaned @@ -705,6 +627,36 @@ fn describe_scheduled_operation(scheduled: &ScheduledEditOperation) -> String { } } +fn replace_byte_range(source: &str, start: usize, end: usize, replacement: &str) -> String { + let mut new_source = String::with_capacity( + source + .len() + .saturating_sub(end.saturating_sub(start)) + .saturating_add(replacement.len()), + ); + new_source.push_str(&source[..start]); + new_source.push_str(replacement); + new_source.push_str(&source[end..]); + new_source +} + +fn target_indent_for_region( + state: &ChunkStateInner, + anchor: &ChunkNode, + region: ChunkRegion, + file_indent_char: char, + file_indent_step: usize, +) -> String { + match region { + ChunkRegion::Container | ChunkRegion::Prologue | ChunkRegion::Epilogue => { + anchor.indent_char.repeat(anchor.indent as usize) + }, + ChunkRegion::Body => { + compute_insert_indent(state, anchor, true, file_indent_char, file_indent_step) + }, + } +} + fn normalize_inserted_content( content: &str, target_indent: &str, @@ -713,6 +665,11 @@ fn normalize_inserted_content( ) -> String { let mut normalized = normalize_chunk_source(content); normalized = strip_content_prefixes(&normalized); + normalized = normalized + .split('\n') + .map(|line| denormalize_from_tabs(line, file_indent_char, file_indent_step.unwrap_or(1))) + .collect::>() + .join("\n"); if target_indent.is_empty() { // Even at indent level 0, normalize the content's indent character // to match the file's convention (e.g. LLM sends spaces for a tab file). @@ -922,24 +879,184 @@ fn is_container_like_chunk(chunk: &ChunkNode) -> bool { .any(|prefix| chunk.name.starts_with(prefix)) } -fn find_container_delimiter_offset( - state: &ChunkStateInner, - anchor: &ChunkNode, - placement: Ordering, -) -> Option { - let slice = &state.source[anchor.start_byte as usize..anchor.end_byte as usize]; - let open_brace = slice.find('{'); - let close_brace = slice.rfind('}'); - match (open_brace, close_brace) { - (Some(open), Some(close)) if close >= open => Some(match placement { - Ordering::Less => anchor.start_byte as usize + open + 1, - _ => anchor.start_byte as usize + close, - }), - _ => None, +fn go_receiver_belongs_to_type(source: &str, chunk: &ChunkNode, type_name: &str) -> bool { + let header = source[chunk.start_byte as usize..chunk.end_byte as usize] + .lines() + .next() + .unwrap_or_default() + .trim_start(); + header.starts_with("func ") + && (header.contains(&format!(" {type_name})")) || header.contains(&format!("*{type_name})"))) +} + +fn owned_container_end_line(state: &ChunkStateInner, anchor: &ChunkNode) -> u32 { + if state.language != "go" || !anchor.path.starts_with("type_") { + return anchor.end_line; + } + + let type_name = anchor.path.trim_start_matches("type_"); + let mut owned_end_line = anchor.end_line; + let mut top_level_chunks = state + .tree + .chunks + .iter() + .filter(|chunk| chunk.parent_path.as_deref() == Some("")) + .collect::>(); + top_level_chunks.sort_by_key(|chunk| chunk.start_line); + + let Some(start_index) = top_level_chunks + .iter() + .position(|chunk| chunk.path == anchor.path) + else { + return anchor.end_line; + }; + + for chunk in top_level_chunks.into_iter().skip(start_index + 1) { + if chunk.start_line < owned_end_line { + continue; + } + if chunk.name.starts_with("fn_") + && go_receiver_belongs_to_type(&state.source, chunk, type_name) + { + owned_end_line = chunk.end_line; + continue; + } + break; + } + + owned_end_line +} + +fn before_chunk_insertion_point(state: &ChunkStateInner, anchor: &ChunkNode) -> InsertionPoint { + if anchor.path.is_empty() { + return InsertionPoint { offset: 0, indent: String::new() }; + } + let offsets = line_offsets(&state.source); + InsertionPoint { + offset: line_start_offset(&offsets, anchor.start_line, &state.source), + indent: anchor.indent_char.repeat(anchor.indent as usize), } } -fn compute_insert_indent(state: &ChunkStateInner, anchor: &ChunkNode, inside: bool) -> String { +fn after_chunk_insertion_point(state: &ChunkStateInner, anchor: &ChunkNode) -> InsertionPoint { + if anchor.path.is_empty() { + return InsertionPoint { offset: state.source.len(), indent: String::new() }; + } + let offsets = line_offsets(&state.source); + let end_line = owned_container_end_line(state, anchor); + InsertionPoint { + offset: line_end_offset(&offsets, end_line, &state.source), + indent: anchor.indent_char.repeat(anchor.indent as usize), + } +} + +fn body_insertion_point( + state: &ChunkStateInner, + anchor: &ChunkNode, + at_end: bool, + file_indent_char: char, + file_indent_step: usize, +) -> Result { + let offsets = line_offsets(&state.source); + let indent = compute_insert_indent(state, anchor, true, file_indent_char, file_indent_step); + if at_end { + if let Some(last_child_path) = anchor.children.last() + && let Some(last_child) = state + .tree + .chunks + .iter() + .find(|chunk| &chunk.path == last_child_path) + { + let child_indent = if last_child.indent_char.is_empty() { + indent + } else { + last_child.indent_char.repeat(last_child.indent as usize) + }; + return Ok(InsertionPoint { + offset: line_end_offset(&offsets, last_child.end_line, &state.source), + indent: child_indent, + }); + } + let (_, body_end) = chunk_region_range(anchor, ChunkRegion::Body)?; + return Ok(InsertionPoint { offset: body_end, indent }); + } + + if let Some(first_child_path) = anchor.children.first() + && let Some(first_child) = state + .tree + .chunks + .iter() + .find(|chunk| &chunk.path == first_child_path) + { + return Ok(InsertionPoint { + offset: line_start_offset(&offsets, first_child.start_line, &state.source), + indent, + }); + } + let (body_start, _) = chunk_region_range(anchor, ChunkRegion::Body)?; + Ok(InsertionPoint { offset: body_start, indent }) +} + +fn resolve_insertion_point( + state: &ChunkStateInner, + anchor: &ChunkNode, + region: ChunkRegion, + op: ChunkEditOp, + _file_content: Option<&str>, + file_indent_char: char, + file_indent_step: usize, +) -> Result<(InsertionPoint, InsertPosition), String> { + match (region, op) { + ( + ChunkRegion::Container | ChunkRegion::Prologue, + ChunkEditOp::Before | ChunkEditOp::Prepend, + ) => Ok((before_chunk_insertion_point(state, anchor), InsertPosition::Before)), + ( + ChunkRegion::Container | ChunkRegion::Epilogue, + ChunkEditOp::After | ChunkEditOp::Append, + ) => Ok((after_chunk_insertion_point(state, anchor), InsertPosition::After)), + (ChunkRegion::Body, ChunkEditOp::Before | ChunkEditOp::Prepend) + | (ChunkRegion::Prologue, ChunkEditOp::After | ChunkEditOp::Append) => Ok(( + body_insertion_point(state, anchor, false, file_indent_char, file_indent_step)?, + InsertPosition::FirstChild, + )), + (ChunkRegion::Body, ChunkEditOp::After | ChunkEditOp::Append) + | (ChunkRegion::Epilogue, ChunkEditOp::Before | ChunkEditOp::Prepend) => Ok(( + body_insertion_point(state, anchor, true, file_indent_char, file_indent_step)?, + InsertPosition::LastChild, + )), + (_, ChunkEditOp::Replace | ChunkEditOp::Delete) => { + Err("Internal error: insertion point requested for non-insert op".to_owned()) + }, + } +} + +fn indent_prefix_for_level( + anchor: &ChunkNode, + file_indent_char: char, + file_indent_step: usize, + extra_levels: usize, +) -> String { + let step = file_indent_step.max(1); + let indent_char = if matches!(file_indent_char, ' ' | '\t') { + file_indent_char + } else { + anchor.indent_char.chars().next().unwrap_or(' ') + }; + if indent_char == '\t' { + return "\t".repeat(anchor.indent as usize + extra_levels); + } + let indent_levels = (anchor.indent as usize / step).saturating_add(extra_levels); + " ".repeat(step * indent_levels) +} + +fn compute_insert_indent( + state: &ChunkStateInner, + anchor: &ChunkNode, + inside: bool, + file_indent_char: char, + file_indent_step: usize, +) -> String { if !inside || anchor.path.is_empty() { return String::new(); } @@ -974,194 +1091,14 @@ fn compute_insert_indent(state: &ChunkStateInner, anchor: &ChunkNode, inside: bo } let indent_char = if anchor.indent_char.is_empty() { - "\t" + file_indent_char.to_string() } else { - anchor.indent_char.as_str() + anchor.indent_char.clone() }; - indent_char.repeat(anchor.indent as usize + 1) -} - -fn go_type_append_child_insertion_point( - state: &ChunkStateInner, - anchor: &ChunkNode, - insertion_content: Option<&str>, -) -> Option<(usize, String)> { - if state.language != "go" - || !anchor.path.starts_with("type_") - || !is_container_like_chunk(anchor) - { - return None; - } - - let offsets = line_offsets(&state.source); - let mut child_chunks = anchor - .children - .iter() - .filter_map(|path| state.tree.chunks.iter().find(|chunk| &chunk.path == path)) - .cloned() - .collect::>(); - let probe = insertion_content - .map(normalize_chunk_source) - .unwrap_or_default(); - let looks_like_file_scope_func = probe - .lines() - .any(|line| line.trim_start().starts_with("func")); - - if child_chunks.is_empty() { - if !looks_like_file_scope_func { - return None; - } - return Some((line_end_offset(&offsets, anchor.end_line, &state.source), String::new())); - } - - child_chunks.sort_by_key(|chunk| chunk.start_line); - if let Some(last_fn) = child_chunks - .iter() - .filter(|chunk| chunk.name.starts_with("fn_")) - .max_by_key(|chunk| chunk.end_line) - { - return Some((line_end_offset(&offsets, last_fn.end_line, &state.source), String::new())); - } - if looks_like_file_scope_func { - return Some((line_end_offset(&offsets, anchor.end_line, &state.source), String::new())); - } - None -} - -fn get_insertion_point( - state: &ChunkStateInner, - anchor: &ChunkNode, - placement: Ordering, - insertion_content: Option<&str>, -) -> InsertionPoint { - let offsets = line_offsets(&state.source); - let is_container = is_container_like_chunk(anchor); - - if placement == Ordering::Less { - if anchor.path.is_empty() { - return InsertionPoint { offset: 0, indent: String::new() }; - } - if is_container { - let indent = compute_insert_indent(state, anchor, true); - if let Some(first_child_path) = anchor.children.first() - && let Some(first_child) = state - .tree - .chunks - .iter() - .find(|chunk| &chunk.path == first_child_path) - { - return InsertionPoint { - offset: line_start_offset(&offsets, first_child.start_line, &state.source), - indent, - }; - } - if let Some(delimiter_offset) = - find_container_delimiter_offset(state, anchor, Ordering::Less) - { - return InsertionPoint { offset: delimiter_offset, indent }; - } - let fallback = line_end_offset(&offsets, anchor.start_line, &state.source); - return InsertionPoint { offset: fallback, indent }; - } - return InsertionPoint { - offset: line_start_offset(&offsets, anchor.start_line, &state.source), - indent: anchor.indent_char.repeat(anchor.indent as usize), - }; - } - - if anchor.path.is_empty() { - return InsertionPoint { offset: state.source.len(), indent: String::new() }; - } - if is_container { - if let Some((offset, indent)) = - go_type_append_child_insertion_point(state, anchor, insertion_content) - { - return InsertionPoint { offset, indent }; - } - if let Some(last_child_path) = anchor.children.last() - && let Some(last_child) = state - .tree - .chunks - .iter() - .find(|chunk| &chunk.path == last_child_path) - { - let indent_char = if last_child.indent_char.is_empty() { - if anchor.indent_char.is_empty() { - "\t" - } else { - anchor.indent_char.as_str() - } - } else { - last_child.indent_char.as_str() - }; - return InsertionPoint { - offset: line_end_offset(&offsets, last_child.end_line, &state.source), - indent: indent_char.repeat(last_child.indent as usize), - }; - } - let indent = compute_insert_indent(state, anchor, true); - if let Some(delimiter_offset) = - find_container_delimiter_offset(state, anchor, Ordering::Greater) - { - return InsertionPoint { offset: delimiter_offset, indent }; - } - return InsertionPoint { - offset: line_start_offset(&offsets, anchor.end_line, &state.source), - indent, - }; - } - InsertionPoint { - offset: line_end_offset(&offsets, anchor.end_line, &state.source), - indent: anchor.indent_char.repeat(anchor.indent as usize), - } -} - -fn get_insertion_point_for_position( - state: &ChunkStateInner, - anchor: &ChunkNode, - pos: InsertPosition, - insertion_content: Option<&str>, -) -> Result { - let offsets = line_offsets(&state.source); - match pos { - InsertPosition::Before => { - if anchor.path.is_empty() { - return Ok(InsertionPoint { offset: 0, indent: String::new() }); - } - Ok(InsertionPoint { - offset: line_start_offset(&offsets, anchor.start_line, &state.source), - indent: anchor.indent_char.repeat(anchor.indent as usize), - }) - }, - InsertPosition::After => { - if anchor.path.is_empty() { - return Ok(InsertionPoint { offset: state.source.len(), indent: String::new() }); - } - Ok(InsertionPoint { - offset: line_end_offset(&offsets, anchor.end_line, &state.source), - indent: anchor.indent_char.repeat(anchor.indent as usize), - }) - }, - InsertPosition::FirstChild => { - if !anchor.path.is_empty() - && anchor.leaf - && !is_container_like_chunk(anchor) - && !anchor.group - { - return Err(format!("Cannot use prepend_child on leaf chunk {}", anchor.path)); - } - Ok(get_insertion_point(state, anchor, Ordering::Less, insertion_content)) - }, - InsertPosition::LastChild => { - if !anchor.path.is_empty() - && anchor.leaf - && !is_container_like_chunk(anchor) - && !anchor.group - { - return Err(format!("Cannot use append_child on leaf chunk {}", anchor.path)); - } - Ok(get_insertion_point(state, anchor, Ordering::Greater, insertion_content)) - }, + if indent_char == "\t" { + "\t".repeat(anchor.indent as usize + 1) + } else { + indent_prefix_for_level(anchor, file_indent_char, file_indent_step, 1) } } @@ -1456,6 +1393,8 @@ fn render_changed_hunks( // has children (and therefore a closing tag in the tree output). let tree = state.tree(); let tab_replacement = " "; + let file_indent_char = detect_file_indent_char(state.source(), tree); + let file_indent_step = detect_file_indent_step(tree) as usize; let lookup: HashMap<&str, &ChunkNode> = tree.chunks.iter().map(|c| (c.path.as_str(), c)).collect(); @@ -1472,6 +1411,7 @@ fn render_changed_hunks( chunk_path, state.source(), tab_replacement, + Some((file_indent_char, file_indent_step)), ); let mut lines = Vec::with_capacity(hunk.lines.len() + 1); lines.push(format!("{indent}{}", hunk.header)); @@ -1499,6 +1439,7 @@ fn render_changed_hunks( anchor_style, show_leaf_preview, tab_replacement: Some(tab_replacement.to_owned()), + normalize_indent: Some(true), focused_paths, }, inline_hunks, @@ -1604,6 +1545,7 @@ fn render_unchanged_response( focused_paths: None, show_leaf_preview: true, tab_replacement: Some(" ".to_owned()), + normalize_indent: Some(true), }) } @@ -1617,6 +1559,22 @@ mod tests { ChunkState::from_inner(ChunkStateInner::new(source.to_owned(), language.to_owned(), tree)) } + fn apply_single_edit( + state: &ChunkState, + file_path: &str, + operation: EditOperation, + ) -> EditResult { + apply_edits(state, &EditParams { + operations: vec![operation], + default_selector: None, + default_crc: None, + anchor_style: None, + cwd: ".".to_owned(), + file_path: file_path.to_owned(), + }) + .expect("edit should apply") + } + #[test] fn root_level_replace_preserves_space_indentation() { let source = "fn main() {\n println!(\"old\");\n}\n"; @@ -1628,6 +1586,7 @@ mod tests { op: ChunkEditOp::Replace, sel: Some("fn_main".to_owned()), crc: Some(chunk.checksum.clone()), + region: None, content: Some("fn main() {\n println!(\"new\");\n}".to_owned()), find: None, }], @@ -1665,6 +1624,7 @@ mod tests { op: ChunkEditOp::Replace, sel: Some("run".to_owned()), crc: Some(chunk.checksum.clone()), + region: None, content: Some("run(): void {\n\tconsole.log(\"resolved\");\n}".to_owned()), find: None, }], @@ -1703,6 +1663,7 @@ mod tests { op: ChunkEditOp::Replace, sel: Some("fuzzyMatch".to_owned()), crc: Some(chunk.checksum.clone()), + region: None, content: Some( "function fuzzyMatch(): void {\n\tconsole.log(\"resolved\");\n}".to_owned(), ), @@ -1736,6 +1697,7 @@ mod tests { op: ChunkEditOp::Replace, sel: Some("box.ts".to_owned()), crc: Some(chunk.checksum.clone()), + region: None, content: Some("function main(): void {\n\tconsole.log(\"normalized\");\n}".to_owned()), find: None, }], @@ -1760,6 +1722,7 @@ mod tests { op: ChunkEditOp::Replace, sel: Some("L2".to_owned()), crc: None, + region: None, content: Some("function main(): void {\n\tconsole.log(\"new\");\n}".to_owned()), find: None, }], @@ -1791,6 +1754,7 @@ mod tests { op: ChunkEditOp::Replace, sel: Some("section_Top.section_Building".to_owned()), crc: Some(chunk.checksum.clone()), + region: None, content: Some("## Building\n\nNew content.\n".to_owned()), find: None, }], @@ -1825,6 +1789,7 @@ mod tests { op: ChunkEditOp::Replace, sel: Some("fn_main".to_owned()), crc: Some(chunk.checksum.clone()), + region: None, content: Some("warn!(\"hello\")".to_owned()), find: Some("println!(\"hello\")".to_owned()), }], @@ -1859,6 +1824,7 @@ mod tests { op: ChunkEditOp::Replace, sel: Some("fn_main".to_owned()), crc: Some(chunk.checksum.clone()), + region: None, content: Some("replacement".to_owned()), find: Some("nonexistent text".to_owned()), }], @@ -1890,6 +1856,7 @@ mod tests { op: ChunkEditOp::Replace, sel: Some("fn_main".to_owned()), crc: Some(chunk.checksum.clone()), + region: None, content: Some("2".to_owned()), find: Some("= 1".to_owned()), }], @@ -1917,6 +1884,7 @@ mod tests { op: ChunkEditOp::Replace, sel: Some("fn_main".to_owned()), crc: Some(chunk.checksum.clone()), + region: None, content: Some("replacement".to_owned()), find: Some(String::new()), }], @@ -1944,6 +1912,7 @@ mod tests { op: ChunkEditOp::Replace, sel: Some("fn_main".to_owned()), crc: Some(chunk.checksum.clone()), + region: None, content: Some("goodbye".to_owned()), find: Some("hello".to_owned()), }], @@ -1974,6 +1943,7 @@ mod tests { op: ChunkEditOp::Replace, sel: Some("var_c".to_owned()), crc: Some(chunk.checksum.clone()), + region: None, content: Some("const c = 33;".to_owned()), find: None, }], @@ -2002,7 +1972,7 @@ mod tests { } #[test] - fn append_child_on_group_chunk_inserts_at_end() { + fn append_on_group_chunk_container_inserts_at_end() { // A file with only a `stmts` group chunk (e.g. a describe() call in a test // file). Appending to it should insert content at the end of the statement // list. @@ -2018,21 +1988,14 @@ mod tests { .expect("stmts chunk should exist"); assert!(stmts.group, "stmts chunk should be marked as group"); - let result = apply_edits(&state, &EditParams { - operations: vec![EditOperation { - op: ChunkEditOp::AppendChild, - sel: Some(stmts.path.clone()), - crc: None, - content: Some("\nit(\"b\", () => {});".to_owned()), - find: None, - }], - default_selector: None, - default_crc: None, - anchor_style: None, - cwd: ".".to_owned(), - file_path: "test.ts".to_owned(), - }) - .expect("append_child on group chunk should succeed"); + let result = apply_single_edit(&state, "test.ts", EditOperation { + op: ChunkEditOp::Append, + sel: Some(stmts.path.clone()), + crc: None, + region: None, + content: Some("\nit(\"b\", () => {});".to_owned()), + find: None, + }); assert!( result.diff_after.contains("it(\"b\""), @@ -2040,4 +2003,166 @@ mod tests { result.diff_after ); } + + #[test] + fn replace_body_preserves_typescript_closing_brace_indentation() { + let source = "function main() {\n work();\n}\n"; + let state = state_for(source, "typescript"); + let chunk = state.inner().chunk("fn_main").expect("fn_main"); + + let result = apply_single_edit(&state, "test.ts", EditOperation { + op: ChunkEditOp::Replace, + sel: Some(format!("fn_main#{}@body", chunk.checksum)), + crc: None, + region: None, + content: Some("\treturn next();\n".to_owned()), + find: None, + }); + + assert_eq!(result.diff_after, "function main() {\n return next();\n}\n"); + } + + #[test] + fn replace_body_preserves_rust_closing_brace_indentation() { + let source = "fn main() {\n println!(\"old\");\n}\n"; + let state = state_for(source, "rust"); + let chunk = state.inner().chunk("fn_main").expect("fn_main"); + + let result = apply_single_edit(&state, "test.rs", EditOperation { + op: ChunkEditOp::Replace, + sel: Some(format!("fn_main#{}@body", chunk.checksum)), + crc: None, + region: None, + content: Some("\tprintln!(\"new\");\n".to_owned()), + find: None, + }); + + assert_eq!(result.diff_after, "fn main() {\n println!(\"new\");\n}\n"); + } + + #[test] + fn replace_body_preserves_go_closing_brace_indentation() { + let source = "func main() {\n work()\n}\n"; + let state = state_for(source, "go"); + let chunk = state.inner().chunk("fn_main").expect("fn_main"); + + let result = apply_single_edit(&state, "test.go", EditOperation { + op: ChunkEditOp::Replace, + sel: Some(format!("fn_main#{}@body", chunk.checksum)), + crc: None, + region: None, + content: Some("\treturn\n".to_owned()), + find: None, + }); + + assert_eq!(result.diff_after, "func main() {\n return\n}\n"); + } + + #[test] + fn three_space_body_replace_denormalizes_tabs_back_to_file_style() { + let source = "def run():\n return 1\n"; + let state = state_for(source, "python"); + let chunk = state.inner().chunk("fn_run").expect("fn_run"); + + let result = apply_single_edit(&state, "test.py", EditOperation { + op: ChunkEditOp::Replace, + sel: Some(format!("fn_run#{}@body", chunk.checksum)), + crc: None, + region: None, + content: Some("\treturn 2\n".to_owned()), + find: None, + }); + + assert_eq!(result.diff_after, "def run():\n return 2\n"); + } + + #[test] + fn after_targets_chunk_directly_for_top_level_sibling_insertion() { + let source = "function alpha(): void {\n\twork();\n}\n"; + let state = state_for(source, "typescript"); + + let result = apply_single_edit(&state, "test.ts", EditOperation { + op: ChunkEditOp::After, + sel: Some("fn_alpha".to_owned()), + crc: None, + region: None, + content: Some("function beta(): void {\n\twork();\n}\n".to_owned()), + find: None, + }); + + assert!(result.diff_after.contains("function alpha(): void"), "{}", result.diff_after); + assert!(result.diff_after.contains("function beta(): void"), "{}", result.diff_after); + assert!( + result + .diff_after + .find("function alpha(): void") + .expect("alpha") + < result + .diff_after + .find("function beta(): void") + .expect("beta") + ); + } + + #[test] + fn go_body_and_container_append_are_not_interchangeable() { + let source = "package main\n\ntype Server struct {\n Addr string\n}\n\nfunc (s *Server) \ + Start() {\n work()\n}\n"; + + let body_state = state_for(source, "go"); + let body_result = apply_single_edit(&body_state, "test.go", EditOperation { + op: ChunkEditOp::Append, + sel: Some("type_Server@body".to_owned()), + crc: None, + region: None, + content: Some("\tPort int\n".to_owned()), + find: None, + }); + assert!( + body_result + .diff_after + .contains("Addr string\n Port int\n}"), + "{}", + body_result.diff_after + ); + assert!( + !body_result.diff_after.contains("func (s *Server) Port"), + "{}", + body_result.diff_after + ); + + let container_state = state_for(source, "go"); + let container_result = apply_single_edit(&container_state, "test.go", EditOperation { + op: ChunkEditOp::Append, + sel: Some("type_Server@container".to_owned()), + crc: None, + region: None, + content: Some("func (s *Server) Stop() {\n\twork()\n}\n".to_owned()), + find: None, + }); + assert!( + container_result + .diff_after + .contains("func (s *Server) Start()"), + "{}", + container_result.diff_after + ); + assert!( + container_result + .diff_after + .contains("func (s *Server) Stop()"), + "{}", + container_result.diff_after + ); + assert!( + container_result + .diff_after + .find("func (s *Server) Start()") + .expect("start") + < container_result + .diff_after + .find("func (s *Server) Stop()") + .expect("stop") + ); + } } diff --git a/crates/pi-natives/src/chunk/indent.rs b/crates/pi-natives/src/chunk/indent.rs index 36dfc5e01..a74672540 100644 --- a/crates/pi-natives/src/chunk/indent.rs +++ b/crates/pi-natives/src/chunk/indent.rs @@ -74,6 +74,50 @@ pub fn count_indent_columns(whitespace: &str, space_step: usize) -> usize { .sum() } +pub fn normalize_to_tabs(line: &str, indent_char: char, indent_step: usize) -> String { + if indent_char == '\t' { + return line.to_owned(); + } + + let whitespace = leading_whitespace(line); + if whitespace.is_empty() { + return line.to_owned(); + } + + let step = indent_step.max(1); + let total_columns = count_indent_columns(whitespace, step); + let tabs = total_columns / step; + let remainder = total_columns % step; + format!("{}{}{}", "\t".repeat(tabs), " ".repeat(remainder), &line[whitespace.len()..]) +} + +pub fn denormalize_from_tabs( + line: &str, + file_indent_char: char, + file_indent_step: usize, +) -> String { + if file_indent_char != ' ' && file_indent_char != '\t' { + return line.to_owned(); + } + + let whitespace = leading_whitespace(line); + if whitespace.is_empty() { + return line.to_owned(); + } + + let step = file_indent_step.max(1); + let mut converted = String::with_capacity(whitespace.len() * step.max(1)); + for ch in whitespace.chars() { + match ch { + '\t' if file_indent_char == '\t' => converted.push('\t'), + '\t' => converted.push_str(&file_indent_char.to_string().repeat(step)), + ' ' => converted.push(' '), + _ => converted.push(ch), + } + } + format!("{converted}{}", &line[whitespace.len()..]) +} + pub fn normalize_target_indent(target_indent: &str, sample_text: &str) -> String { if target_indent.is_empty() { return String::new(); @@ -481,8 +525,8 @@ mod tests { start_byte: 0, end_byte: 0, checksum_start_byte: 0, - body_start_byte: None, - body_end_byte: None, + prologue_end_byte: None, + epilogue_start_byte: None, checksum: "ABCD".to_owned(), error: false, indent, @@ -506,6 +550,27 @@ mod tests { ); } + #[test] + fn canonical_indent_round_trips_common_profiles() { + let cases = [ + (" value()", ' ', 4, "\tvalue()", " value()"), + (" value()", ' ', 3, "\t\tvalue()", " value()"), + (" value()", ' ', 2, "\tvalue()", " value()"), + ("\tvalue()", '\t', 4, "\tvalue()", "\tvalue()"), + (" \t value()", ' ', 4, "\t value()", " value()"), + ]; + + for (input, indent_char, indent_step, canonical, restored) in cases { + let normalized = normalize_to_tabs(input, indent_char, indent_step); + assert_eq!(normalized, canonical, "unexpected canonical indent for {input:?}"); + assert_eq!( + denormalize_from_tabs(&normalized, indent_char, indent_step), + restored, + "unexpected restored indent for {input:?}" + ); + } + } + #[test] fn reindent_inserted_block_preserves_first_line_hanging_indent() { let input = "call(\n alpha,\n beta,\n )"; diff --git a/crates/pi-natives/src/chunk/mod.rs b/crates/pi-natives/src/chunk/mod.rs index 859c4330f..08e39a45d 100644 --- a/crates/pi-natives/src/chunk/mod.rs +++ b/crates/pi-natives/src/chunk/mod.rs @@ -136,8 +136,8 @@ pub(crate) fn build_chunk_tree(source: &str, language: &str) -> Result [1, usize::MAX]; } +fn normalize_rendered_line( + line: &str, + normalize_indent: Option<(char, usize)>, + tab_replacement: &str, +) -> String { + match normalize_indent { + Some((indent_char, indent_step)) => normalize_to_tabs(line, indent_char, indent_step), + None => line.replace('\t', tab_replacement), + } +} + pub fn render_state(state: &ChunkStateInner, params: &RenderParams) -> String { let tree = state.tree(); let lookup = build_lookup(tree); @@ -43,6 +55,9 @@ pub fn render_state(state: &ChunkStateInner, params: &RenderParams) -> String { let preview_head_lines = *PREVIEW_HEAD_LINES; let preview_tail_lines = *PREVIEW_TAIL_LINES; let tab_replacement = params.tab_replacement.as_deref().unwrap_or(" "); + let normalize_indent = params.normalize_indent.unwrap_or(false).then(|| { + (detect_file_indent_char(state.source(), tree), detect_file_indent_step(tree) as usize) + }); let anchor_style = params.anchor_style.unwrap_or_default(); let focus: Option> = params .focused_paths @@ -57,6 +72,7 @@ pub fn render_state(state: &ChunkStateInner, params: &RenderParams) -> String { params.show_leaf_preview, &source_lines, tab_replacement, + normalize_indent, full_display_threshold, preview_head_lines, preview_tail_lines, @@ -70,6 +86,7 @@ pub fn render_state(state: &ChunkStateInner, params: &RenderParams) -> String { params.show_leaf_preview, &source_lines, tab_replacement, + normalize_indent, full_display_threshold, preview_head_lines, preview_tail_lines, @@ -90,6 +107,7 @@ pub fn render_state(state: &ChunkStateInner, params: &RenderParams) -> String { preview_head_lines, preview_tail_lines, tab_replacement, + normalize_indent, focus, inline_hunks: HashMap::new(), }; @@ -247,10 +265,14 @@ fn chunk_body_anchor_indent( source_lines: &[&str], chunk: &ChunkNode, tab_replacement: &str, + normalize_indent: Option<(char, usize)>, ) -> String { source_lines .get(chunk.start_line.saturating_sub(1) as usize) - .map_or(String::new(), |line| leading_whitespace(line).replace('\t', tab_replacement)) + .map_or(String::new(), |line| { + leading_whitespace(&normalize_rendered_line(line, normalize_indent, tab_replacement)) + .to_owned() + }) } const fn chunk_anchor_label(chunk: &ChunkNode, style: ChunkAnchorStyle) -> &str { @@ -299,6 +321,7 @@ fn build_leaf_entries( source_lines: &[&str], span: VisibleSpan, tab_replacement: &str, + normalize_indent: Option<(char, usize)>, full_display_threshold: usize, preview_head_lines: usize, preview_tail_lines: usize, @@ -311,7 +334,9 @@ fn build_leaf_entries( abs_line: line, text: source_lines .get(line.saturating_sub(1) as usize) - .map_or(String::new(), |text| text.replace('\t', tab_replacement)), + .map_or(String::new(), |text| { + normalize_rendered_line(text, normalize_indent, tab_replacement) + }), }) .collect::>(); @@ -393,6 +418,7 @@ fn for_each_rendered_source_line( show_leaf_preview: bool, source_lines: &[&str], tab_replacement: &str, + normalize_indent: Option<(char, usize)>, full_display_threshold: usize, preview_head_lines: usize, preview_tail_lines: usize, @@ -415,6 +441,7 @@ fn for_each_rendered_source_line( source_lines, span, tab_replacement, + normalize_indent, full_display_threshold, preview_head_lines, preview_tail_lines, @@ -448,6 +475,7 @@ fn for_each_rendered_source_line( show_leaf_preview, source_lines, tab_replacement, + normalize_indent, full_display_threshold, preview_head_lines, preview_tail_lines, @@ -474,6 +502,7 @@ fn for_each_rendered_source_line( show_leaf_preview, source_lines, tab_replacement, + normalize_indent, full_display_threshold, preview_head_lines, preview_tail_lines, @@ -491,6 +520,7 @@ fn compute_rendered_line_count( show_leaf_preview: bool, source_lines: &[&str], tab_replacement: &str, + normalize_indent: Option<(char, usize)>, full_display_threshold: usize, preview_head_lines: usize, preview_tail_lines: usize, @@ -523,6 +553,7 @@ fn compute_rendered_line_count( show_leaf_preview, source_lines, tab_replacement, + normalize_indent, full_display_threshold, preview_head_lines, preview_tail_lines, @@ -548,6 +579,7 @@ struct RenderCtx<'a> { preview_head_lines: usize, preview_tail_lines: usize, tab_replacement: &'a str, + normalize_indent: Option<(char, usize)>, focus: Option>, inline_hunks: HashMap>, } @@ -601,7 +633,9 @@ fn emit_line_gap(ctx: &mut RenderCtx<'_>, from: u32, to: u32) { let text = ctx .source_lines .get(line.saturating_sub(1) as usize) - .map_or(String::new(), |text| text.replace('\t', ctx.tab_replacement)); + .map_or(String::new(), |text| { + normalize_rendered_line(text, ctx.normalize_indent, ctx.tab_replacement) + }); push_code(ctx, line, &text); } } @@ -611,6 +645,7 @@ fn emit_leaf_body(ctx: &mut RenderCtx<'_>, _chunk: &ChunkNode, span: VisibleSpan ctx.source_lines, span, ctx.tab_replacement, + ctx.normalize_indent, ctx.full_display_threshold, ctx.preview_head_lines, ctx.preview_tail_lines, @@ -657,8 +692,12 @@ fn emit_chunk_subtree( if options.between_top_level_definitions && depth == 0 && !options.is_first_top_level { push_blank_meta(ctx); } - let anchor_indent = - chunk_body_anchor_indent(ctx.source_lines, chunk, ctx.tab_replacement); + let anchor_indent = chunk_body_anchor_indent( + ctx.source_lines, + chunk, + ctx.tab_replacement, + ctx.normalize_indent, + ); let style = ctx.anchor_style.with_omit_checksum(ctx.omit_checksum); let anchor_label = chunk_anchor_label(chunk, style); push_meta(ctx, style.render(&anchor_indent, anchor_label, chunk.checksum.as_str())); @@ -683,7 +722,12 @@ fn emit_chunk_subtree( push_blank_meta(ctx); } if !chunk.path.is_empty() { - let anchor_indent = chunk_body_anchor_indent(ctx.source_lines, chunk, ctx.tab_replacement); + let anchor_indent = chunk_body_anchor_indent( + ctx.source_lines, + chunk, + ctx.tab_replacement, + ctx.normalize_indent, + ); let style = ctx.anchor_style.with_omit_checksum(ctx.omit_checksum); let anchor_label = chunk_anchor_label(chunk, style); push_meta(ctx, style.render(&anchor_indent, anchor_label, chunk.checksum.as_str())); @@ -732,7 +776,12 @@ fn emit_chunk_subtree( } // Closing tag for chunks with children if !chunk.path.is_empty() { - let anchor_indent = chunk_body_anchor_indent(ctx.source_lines, chunk, ctx.tab_replacement); + let anchor_indent = chunk_body_anchor_indent( + ctx.source_lines, + chunk, + ctx.tab_replacement, + ctx.normalize_indent, + ); let style = ctx.anchor_style.with_omit_checksum(ctx.omit_checksum); let anchor_label = chunk_anchor_label(chunk, style); push_meta(ctx, style.render_close(&anchor_indent, anchor_label, chunk.checksum.as_str())); @@ -756,6 +805,7 @@ fn compute_num_width( show_leaf_preview: bool, source_lines: &[&str], tab_replacement: &str, + normalize_indent: Option<(char, usize)>, full_display_threshold: usize, preview_head_lines: usize, preview_tail_lines: usize, @@ -782,6 +832,7 @@ fn compute_num_width( show_leaf_preview, source_lines, tab_replacement, + normalize_indent, full_display_threshold, preview_head_lines, preview_tail_lines, @@ -827,13 +878,17 @@ pub fn hunk_indent_for_chunk( chunk_path: &str, source: &str, tab_replacement: &str, + normalize_indent: Option<(char, usize)>, ) -> String { let source_lines: Vec<&str> = source.split('\n').collect(); let Some(chunk) = lookup.get(chunk_path) else { return String::new(); }; - let base = chunk_body_anchor_indent(&source_lines, chunk, tab_replacement); - format!("{base}{tab_replacement}") + let base = chunk_body_anchor_indent(&source_lines, chunk, tab_replacement, normalize_indent); + match normalize_indent { + Some(_) => format!("{base}\t"), + None => format!("{base}{tab_replacement}"), + } } /// Render a chunk tree with diff hunks inlined into their owning chunk blocks. @@ -857,6 +912,9 @@ pub fn render_state_with_hunks( let preview_head_lines = *PREVIEW_HEAD_LINES; let preview_tail_lines = *PREVIEW_TAIL_LINES; let tab_replacement = params.tab_replacement.as_deref().unwrap_or(" "); + let normalize_indent = params.normalize_indent.unwrap_or(false).then(|| { + (detect_file_indent_char(state.source(), tree), detect_file_indent_step(tree) as usize) + }); let anchor_style = params.anchor_style.unwrap_or_default(); let focus: Option> = params .focused_paths @@ -871,6 +929,7 @@ pub fn render_state_with_hunks( params.show_leaf_preview, &source_lines, tab_replacement, + normalize_indent, full_display_threshold, preview_head_lines, preview_tail_lines, @@ -884,6 +943,7 @@ pub fn render_state_with_hunks( params.show_leaf_preview, &source_lines, tab_replacement, + normalize_indent, full_display_threshold, preview_head_lines, preview_tail_lines, @@ -904,6 +964,7 @@ pub fn render_state_with_hunks( preview_head_lines, preview_tail_lines, tab_replacement, + normalize_indent, focus, inline_hunks, }; diff --git a/crates/pi-natives/src/chunk/resolve.rs b/crates/pi-natives/src/chunk/resolve.rs index 5fea2a64e..bc378cec7 100644 --- a/crates/pi-natives/src/chunk/resolve.rs +++ b/crates/pi-natives/src/chunk/resolve.rs @@ -2,7 +2,7 @@ use std::{cmp::Ordering, collections::BTreeSet}; use crate::chunk::{ state::ChunkStateInner, - types::{ChunkNode, ChunkTree}, + types::{ChunkNode, ChunkRegion, ChunkTree}, }; const CHUNK_NAME_PREFIXES: &[&str] = @@ -10,26 +10,106 @@ const CHUNK_NAME_PREFIXES: &[&str] = const CHECKSUM_ALPHABET: &str = "ZPMQVRWSNKTXJBYH"; pub struct ResolvedChunk<'a> { - pub chunk: &'a ChunkNode, - pub crc: Option, + pub chunk: &'a ChunkNode, + pub crc: Option, + pub region: ChunkRegion, +} + +fn parse_region_name(value: &str) -> Option { + match value.trim() { + "container" => Some(ChunkRegion::Container), + "prologue" => Some(ChunkRegion::Prologue), + "body" => Some(ChunkRegion::Body), + "epilogue" => Some(ChunkRegion::Epilogue), + _ => None, + } +} + +pub fn split_region_suffix(selector: &str) -> (&str, Option) { + let Some((prefix, suffix)) = selector.rsplit_once('@') else { + return (selector, None); + }; + let Some(region) = parse_region_name(suffix.trim()) else { + return (selector, None); + }; + (prefix.trim_end(), Some(region)) +} + +pub fn split_selector_crc_and_region( + selector: Option<&str>, + crc: Option<&str>, + region: Option, +) -> Result<(Option, Option, ChunkRegion), String> { + let mut raw = selector + .map(str::trim) + .filter(|value| !matches!(*value, "" | "null" | "undefined")) + .unwrap_or_default() + .to_owned(); + if let Some(index) = chunk_read_path_separator_index(&raw) { + raw = raw[index + 1..].to_owned(); + } + + let (without_region, parsed_region) = if raw.is_empty() { + (raw.as_str(), None) + } else { + let (prefix, parsed_region) = split_region_suffix(raw.as_str()); + if parsed_region.is_some() { + (prefix, parsed_region) + } else if let Some((_, suffix)) = raw.rsplit_once('@') { + return Err(format!( + "Unknown chunk region \"{}\". Valid regions: container, prologue, body, epilogue.", + suffix.trim() + )); + } else { + (raw.as_str(), None) + } + }; + + let mut selector_part = without_region.trim(); + let embedded_crc = if let Some((prefix, suffix)) = selector_part.rsplit_once('#') { + if is_checksum_token(suffix.trim()) { + selector_part = prefix.trim_end(); + sanitize_crc(Some(suffix)) + } else { + None + } + } else if let Some(suffix) = selector_part.strip_prefix('#') { + if is_checksum_token(suffix.trim()) { + selector_part = ""; + sanitize_crc(Some(suffix)) + } else { + None + } + } else if is_checksum_token(selector_part) { + let cleaned = sanitize_crc(Some(selector_part)); + selector_part = ""; + cleaned + } else { + None + }; + + let cleaned_selector = if selector_part.is_empty() { + None + } else { + Some(selector_part.to_owned()) + }; + let cleaned_crc = sanitize_crc(crc).or(embedded_crc); + let region = region.or(parsed_region).unwrap_or(ChunkRegion::Container); + + if let Some(cleaned_selector) = cleaned_selector.as_deref() + && cleaned_crc.is_some() + && looks_like_file_target(cleaned_selector) + { + return Ok((None, cleaned_crc, region)); + } + + Ok((cleaned_selector, cleaned_crc, region)) } pub fn sanitize_chunk_selector(selector: Option<&str>) -> Option { - let mut value = selector?.trim().to_owned(); - if matches!(value.as_str(), "null" | "undefined") { - return None; - } - - if let Some(index) = chunk_read_path_separator_index(&value) { - value = value[index + 1..].to_owned(); - } - - let value = strip_trailing_checksum(&value).trim(); - if value.is_empty() { - None - } else { - Some(value.to_owned()) - } + split_selector_crc_and_region(selector, None, None) + .ok() + .and_then(|(cleaned_selector, ..)| cleaned_selector) } pub fn sanitize_crc(crc: Option<&str>) -> Option { @@ -41,44 +121,12 @@ pub fn sanitize_crc(crc: Option<&str>) -> Option { } } -pub fn split_selector_and_crc( - selector: Option<&str>, - crc: Option<&str>, -) -> (Option, Option) { - let cleaned_selector = sanitize_chunk_selector(selector); - let selector_crc = selector.and_then(extract_crc_token); - let cleaned_crc = sanitize_crc(crc).or(selector_crc); - - if cleaned_selector.is_none() - && let Some(selector) = selector - && let Some(raw) = selector_after_read_path(selector) - && let Some(raw_crc) = raw - .strip_prefix('#') - .or_else(|| is_checksum_token(raw).then_some(raw)) - { - return (None, sanitize_crc(Some(raw_crc)).or(cleaned_crc)); - } - - if let Some(cleaned_selector) = cleaned_selector.as_deref() - && cleaned_crc.is_some() - && looks_like_file_target(cleaned_selector) - { - return (None, cleaned_crc); - } - - if cleaned_selector.is_some() { - (cleaned_selector, cleaned_crc) - } else { - (None, cleaned_crc) - } -} - pub fn resolve_chunk_selector<'a>( state: &'a ChunkStateInner, selector: Option<&str>, warnings: &mut Vec, ) -> Result<&'a ChunkNode, String> { - let (cleaned_selector, cleaned_crc) = split_selector_and_crc(selector, None); + let (cleaned_selector, cleaned_crc, _) = split_selector_crc_and_region(selector, None, None)?; resolve_chunk_selector_impl(state, cleaned_selector.as_deref(), cleaned_crc.as_deref(), warnings) } @@ -88,17 +136,18 @@ pub fn resolve_chunk_with_crc<'a>( crc: Option<&str>, warnings: &mut Vec, ) -> Result, String> { - let (cleaned_selector, cleaned_crc) = split_selector_and_crc(selector, crc); + let (cleaned_selector, cleaned_crc, region) = + split_selector_crc_and_region(selector, crc, None)?; if cleaned_selector.is_none() && let Some(cleaned_crc) = cleaned_crc.clone() { let chunk = resolve_chunk_by_checksum(state, &cleaned_crc)?; - return Ok(ResolvedChunk { chunk, crc: Some(cleaned_crc) }); + return Ok(ResolvedChunk { chunk, crc: Some(cleaned_crc), region }); } let chunk = resolve_chunk_selector_impl(state, cleaned_selector.as_deref(), None, warnings)?; - Ok(ResolvedChunk { chunk, crc: cleaned_crc }) + Ok(ResolvedChunk { chunk, crc: cleaned_crc, region }) } pub fn resolve_chunk_by_checksum<'a>( @@ -131,6 +180,54 @@ fn root_chunk(state: &ChunkStateInner) -> Result<&ChunkNode, String> { .ok_or_else(|| "Chunk tree is missing the root chunk".to_owned()) } +pub const fn chunk_supports_region(chunk: &ChunkNode, region: ChunkRegion) -> bool { + match region { + ChunkRegion::Container => true, + ChunkRegion::Prologue | ChunkRegion::Body | ChunkRegion::Epilogue => { + chunk.prologue_end_byte.is_some() && chunk.epilogue_start_byte.is_some() + }, + } +} + +pub fn chunk_region_range( + chunk: &ChunkNode, + region: ChunkRegion, +) -> Result<(usize, usize), String> { + match region { + ChunkRegion::Container => Ok((chunk.start_byte as usize, chunk.end_byte as usize)), + ChunkRegion::Prologue => Ok(( + chunk.start_byte as usize, + chunk + .prologue_end_byte + .ok_or_else(|| format!("Chunk \"{}\" does not support @prologue.", chunk.path))? + as usize, + )), + ChunkRegion::Body => Ok(( + chunk + .prologue_end_byte + .ok_or_else(|| format!("Chunk \"{}\" does not support @body.", chunk.path))? as usize, + chunk + .epilogue_start_byte + .ok_or_else(|| format!("Chunk \"{}\" does not support @body.", chunk.path))? as usize, + )), + ChunkRegion::Epilogue => Ok(( + chunk + .epilogue_start_byte + .ok_or_else(|| format!("Chunk \"{}\" does not support @epilogue.", chunk.path))? + as usize, + chunk.end_byte as usize, + )), + } +} + +pub fn format_region_ref(chunk: &ChunkNode, region: ChunkRegion) -> String { + if chunk.path.is_empty() { + format!("#{}@{}", chunk.checksum, region.as_str()) + } else { + format!("{}#{}@{}", chunk.path, chunk.checksum, region.as_str()) + } +} + fn resolve_chunk_selector_impl<'a>( state: &'a ChunkStateInner, selector: Option<&str>, @@ -537,33 +634,6 @@ fn is_line_number_selector(selector: &str) -> bool { !end.is_empty() && end.chars().all(|ch| ch.is_ascii_digit()) } -fn strip_trailing_checksum(value: &str) -> &str { - let Some((prefix, suffix)) = value.rsplit_once('#') else { - return value; - }; - if is_checksum_token(suffix) { - prefix - } else { - value - } -} - -fn extract_crc_token(value: &str) -> Option { - let raw = selector_after_read_path(value)?; - let token = raw - .rsplit_once('#') - .map(|(_, suffix)| suffix) - .or_else(|| raw.strip_prefix('#')) - .or_else(|| is_checksum_token(raw).then_some(raw))?; - sanitize_crc(Some(token)) -} - -fn selector_after_read_path(value: &str) -> Option<&str> { - chunk_read_path_separator_index(value) - .map(|index| &value[index + 1..]) - .or(Some(value)) -} - fn is_checksum_token(value: &str) -> bool { value.len() == 4 && value @@ -615,8 +685,8 @@ mod tests { start_byte: 0, end_byte: 0, checksum_start_byte: 0, - body_start_byte: None, - body_end_byte: None, + prologue_end_byte: None, + epilogue_start_byte: None, checksum: checksum.to_owned(), error: false, indent: 0, diff --git a/crates/pi-natives/src/chunk/state.rs b/crates/pi-natives/src/chunk/state.rs index dccf01a6a..446810f1e 100644 --- a/crates/pi-natives/src/chunk/state.rs +++ b/crates/pi-natives/src/chunk/state.rs @@ -9,11 +9,15 @@ use regex::Regex; use super::{ build_chunk_tree, - resolve::{resolve_chunk_selector, resolve_chunk_with_crc, split_selector_and_crc}, + indent::{detect_file_indent_char, detect_file_indent_step, normalize_to_tabs}, + resolve::{ + chunk_region_range, chunk_supports_region, format_region_ref, resolve_chunk_selector, + resolve_chunk_with_crc, split_selector_crc_and_region, + }, }; use crate::chunk::types::{ - ChunkInfo, ChunkNode, ChunkReadStatus, ChunkReadTarget, ChunkTree, EditParams, EditResult, - ReadRenderParams, ReadResult, RenderParams, VisibleLineRange, + ChunkInfo, ChunkNode, ChunkReadStatus, ChunkReadTarget, ChunkRegion, ChunkTree, EditParams, + EditResult, ReadRenderParams, ReadResult, RenderParams, VisibleLineRange, }; const LINE_RANGE_SELECTOR_RE: &str = r"^L(\d+)(?:-L?(\d+))?$"; @@ -321,7 +325,19 @@ impl ChunkState { /// errors. #[napi(js_name = "renderRead")] pub fn render_read(&self, params: ReadRenderParams) -> Result { - let ParsedChunkReadPath { selector, crc } = parse_chunk_read_path(params.read_path.as_str()); + let ParsedChunkReadPath { selector, crc, region } = + match parse_chunk_read_path(params.read_path.as_str()) { + Ok(parsed) => parsed, + Err(err) => { + return Ok(ReadResult { + text: format!("{}\n\n{}", params.display_path, err), + chunk: Some(ChunkReadTarget { + status: ChunkReadStatus::UnsupportedRegion, + selector: params.read_path.clone(), + }), + }); + }, + }; let visible_range = selector.as_deref().and_then(parse_visible_line_range); let Some(root) = self.inner.root() else { return Ok(ReadResult { @@ -369,12 +385,16 @@ impl ChunkState { anchor_style: params.anchor_style, show_leaf_preview: true, tab_replacement: params.tab_replacement, + normalize_indent: params.normalize_indent, focused_paths: None, }); return Ok(ReadResult { text: format!("{notice}\n\n{text}"), chunk: None }); } - if selector.as_deref().is_none_or(str::is_empty) && crc.is_none() { + if selector.as_deref().is_none_or(str::is_empty) + && crc.is_none() + && region == ChunkRegion::Container + { return Ok(ReadResult { text: self.render(RenderParams { chunk_path: Some(root.path.clone()), @@ -386,6 +406,7 @@ impl ChunkState { anchor_style: params.anchor_style, show_leaf_preview: true, tab_replacement: params.tab_replacement, + normalize_indent: params.normalize_indent, focused_paths: None, }), chunk: None, @@ -395,9 +416,14 @@ impl ChunkState { if selector.as_deref() == Some("?") { let mut lines = vec![format!("{} chunks:", params.display_path)]; for chunk in self.inner.chunks().filter(|chunk| !chunk.path.is_empty()) { + let supported_regions = if chunk_supports_region(chunk, ChunkRegion::Body) { + "container, prologue, body, epilogue" + } else { + "container" + }; lines.push(format!( - " {}#{} L{}-L{}", - chunk.path, chunk.checksum, chunk.start_line, chunk.end_line + " {}#{} L{}-L{} regions: {}", + chunk.path, chunk.checksum, chunk.start_line, chunk.end_line, supported_regions )); } return Ok(ReadResult { text: lines.join("\n"), chunk: None }); @@ -420,6 +446,23 @@ impl ChunkState { }, }; let chunk = resolved.chunk; + let selector_ref = format_region_ref(chunk, resolved.region); + + if !chunk_supports_region(chunk, resolved.region) { + return Ok(ReadResult { + text: format!( + "{}:{}\n\nChunk \"{}\" does not support @{}.", + params.display_path, + chunk.path, + chunk.path, + resolved.region.as_str(), + ), + chunk: Some(ChunkReadTarget { + status: ChunkReadStatus::UnsupportedRegion, + selector: selector_ref, + }), + }); + } if let Some(absolute_line_range) = params.absolute_line_range { let req_start = absolute_line_range.start_line; @@ -439,16 +482,65 @@ impl ChunkState { ), chunk: Some(ChunkReadTarget { status: ChunkReadStatus::Ok, - selector: chunk.path.clone(), + selector: selector_ref, }), }); } } + if resolved.region != ChunkRegion::Container { + let masked_source = mask_chunk_display_source(self.inner.source(), self.inner.language()); + let (start, end) = match chunk_region_range(chunk, resolved.region) { + Ok(range) => range, + Err(err) => { + return Ok(ReadResult { + text: format!("{}\n\n{}", params.display_path, err), + chunk: Some(ChunkReadTarget { + status: ChunkReadStatus::UnsupportedRegion, + selector: selector_ref, + }), + }); + }, + }; + let tab_replacement = params.tab_replacement.as_deref().unwrap_or(" "); + let normalize_indent = params.normalize_indent.unwrap_or(false).then(|| { + ( + detect_file_indent_char(self.inner.source(), self.inner.tree()), + detect_file_indent_step(self.inner.tree()) as usize, + ) + }); + let region_text = masked_source + .get(start..end) + .unwrap_or_default() + .split('\n') + .map(|line| match normalize_indent { + Some((indent_char, indent_step)) => { + normalize_to_tabs(line, indent_char, indent_step) + }, + None => line.replace('\t', tab_replacement), + }) + .collect::>() + .join("\n"); + let text = if region_text.is_empty() { + format!("{selector_ref}\n\n[Empty @{} region]", resolved.region.as_str()) + } else { + format!("{selector_ref}\n\n{region_text}") + }; + return Ok(ReadResult { + text, + chunk: Some(ChunkReadTarget { status: ChunkReadStatus::Ok, selector: selector_ref }), + }); + } + Ok(ReadResult { text: self.render(RenderParams { chunk_path: Some(chunk.path.clone()), - title: format!("{}:{}", params.display_path, chunk.path), + title: format!( + "{}:{}@{}", + params.display_path, + chunk.path, + resolved.region.as_str() + ), language_tag: params.language_tag.clone(), visible_range: None, render_children_only: false, @@ -456,12 +548,10 @@ impl ChunkState { anchor_style: params.anchor_style, show_leaf_preview: true, tab_replacement: params.tab_replacement, + normalize_indent: params.normalize_indent, focused_paths: None, }), - chunk: Some(ChunkReadTarget { - status: ChunkReadStatus::Ok, - selector: chunk.path.clone(), - }), + chunk: Some(ChunkReadTarget { status: ChunkReadStatus::Ok, selector: selector_ref }), }) } @@ -495,6 +585,7 @@ impl ChunkState { struct ParsedChunkReadPath { selector: Option, crc: Option, + region: ChunkRegion, } fn normalize_language(language: &str) -> String { @@ -522,11 +613,11 @@ fn chunk_read_path_separator_index(read_path: &str) -> Option { read_path.find(':') } -fn parse_chunk_read_path(read_path: &str) -> ParsedChunkReadPath { - let (selector, crc) = chunk_read_path_separator_index(read_path) - .map(|index| split_selector_and_crc(Some(&read_path[(index + 1)..]), None)) - .unwrap_or_default(); - ParsedChunkReadPath { selector, crc } +fn parse_chunk_read_path(read_path: &str) -> std::result::Result { + let raw_selector = + chunk_read_path_separator_index(read_path).map(|index| &read_path[(index + 1)..]); + let (selector, crc, region) = split_selector_crc_and_region(raw_selector, None, None)?; + Ok(ParsedChunkReadPath { selector, crc, region }) } fn parse_visible_line_range(selector: &str) -> Option { diff --git a/crates/pi-natives/src/chunk/types.rs b/crates/pi-natives/src/chunk/types.rs index f9d6fd4cf..b04e4fddf 100644 --- a/crates/pi-natives/src/chunk/types.rs +++ b/crates/pi-natives/src/chunk/types.rs @@ -22,8 +22,12 @@ pub struct ChunkNode { /// such as doc comments or attributes. pub checksum_start_byte: u32, - pub body_start_byte: Option, - pub body_end_byte: Option, + /// End byte of the prologue region. `None` means the chunk only exposes + /// `@container`. + pub prologue_end_byte: Option, + /// Start byte of the epilogue region. `None` means the chunk only exposes + /// `@container`. + pub epilogue_start_byte: Option, pub checksum: String, pub error: bool, @@ -77,34 +81,57 @@ pub enum ChunkReadStatus { /// No chunk matched the requested selector. #[napi(value = "not_found")] NotFound, + /// Chunk matched but does not support the requested region. + #[napi(value = "unsupported_region")] + UnsupportedRegion, +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[napi(string_enum)] +pub enum ChunkRegion { + #[napi(value = "container")] + Container, + #[napi(value = "prologue")] + Prologue, + #[napi(value = "body")] + Body, + #[napi(value = "epilogue")] + Epilogue, +} + +impl ChunkRegion { + pub const fn as_str(self) -> &'static str { + match self { + Self::Container => "container", + Self::Prologue => "prologue", + Self::Body => "body", + Self::Epilogue => "epilogue", + } + } } /// Structural edit to apply relative to a chunk anchor. #[derive(Clone, Copy, Debug, PartialEq, Eq)] #[napi(string_enum)] pub enum ChunkEditOp { - /// Replace the chunk body, or a substring via `find`. + /// Replace the targeted region, or a substring via `find`. #[napi(value = "replace")] Replace, - /// Remove the chunk's source range. + /// Remove the targeted region. #[napi(value = "delete")] Delete, - /// Insert `content` as the last child of the target chunk. - #[napi(value = "append_child")] - AppendChild, - /// Insert `content` as the first child of the target chunk. - #[napi(value = "prepend_child")] - PrependChild, - /// Insert `content` after the target chunk's source range. - #[napi(value = "append_sibling")] - AppendSibling, - /// Insert `content` before the target chunk's source range. - #[napi(value = "prepend_sibling")] - PrependSibling, - /// Replace only the inner body of the chunk, preserving signature and - /// closing delimiter. - #[napi(value = "replace_body")] - ReplaceBody, + /// Insert `content` before the targeted region span. + #[napi(value = "before")] + Before, + /// Insert `content` after the targeted region span. + #[napi(value = "after")] + After, + /// Insert `content` at the start inside the targeted region. + #[napi(value = "prepend")] + Prepend, + /// Insert `content` at the end inside the targeted region. + #[napi(value = "append")] + Append, } impl ChunkEditOp { @@ -112,11 +139,10 @@ impl ChunkEditOp { match self { Self::Replace => "replace", Self::Delete => "delete", - Self::AppendChild => "append_child", - Self::PrependChild => "prepend_child", - Self::AppendSibling => "append_sibling", - Self::PrependSibling => "prepend_sibling", - Self::ReplaceBody => "replace_body", + Self::Before => "before", + Self::After => "after", + Self::Prepend => "prepend", + Self::Append => "append", } } } @@ -267,6 +293,9 @@ pub struct RenderParams { /// Replace tab characters in displayed previews (e.g. two spaces). #[napi(js_name = "tabReplacement")] pub tab_replacement: Option, + /// When true, normalize displayed indentation to canonical tabs. + #[napi(js_name = "normalizeIndent")] + pub normalize_indent: Option, /// When set, restrict rendering to these chunks with their specified focus /// modes. Everything not in this list is skipped. @@ -300,6 +329,9 @@ pub struct ReadRenderParams { /// Replace tabs in embedded previews. #[napi(js_name = "tabReplacement")] pub tab_replacement: Option, + /// When true, normalize displayed indentation to canonical tabs. + #[napi(js_name = "normalizeIndent")] + pub normalize_indent: Option, } /// Rendered chunk text plus optional resolution metadata for the read request. @@ -325,6 +357,8 @@ pub struct EditOperation { /// Optional checksum anchor; falls back to `EditParams.defaultCrc` when /// omitted. pub crc: Option, + /// Region to target. When omitted, defaults to `@container`. + pub region: Option, /// Replacement or inserted text (meaning depends on `op`). pub content: Option, /// For scoped find/replace: literal substring to locate inside the target diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index f62b331b3..167428d99 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,6 +1,11 @@ # Changelog ## [Unreleased] +### Breaking Changes + +- Simplified chunk edit operations: removed `append_child`, `prepend_child`, `append_sibling`, `prepend_sibling`, and `replace_body` ops in favor of unified `replace`, `before`, `after`, `prepend`, and `append` with region targeting (`@container`, `@prologue`, `@body`, `@epilogue`) +- Chunk edit `target` format changed: now accepts `selector#CRC@region` for mutations and `selector@region` for insertions; removed separate `crc` and `anchor` fields from edit operations +- Removed checksum requirement from insert operations (`before`, `after`, `prepend`, `append`); only `replace` requires `#CRC` suffix ### Added @@ -25,6 +30,12 @@ ### Changed +- Chunk edit tool documentation restructured: replaced operation-specific examples with region-based guidance and canonical indentation rules +- Chunk read documentation updated: selectors now support region syntax (e.g., `class_Foo.fn_bar#ABCD@body`) and canonical target listings show supported regions per chunk +- Chunk edit schema simplified: `target` description now documents region format; `op` and `content` descriptions clarified for region-aware operations +- Chunk edit streaming previews updated: labels now reflect region-aware operations (e.g., `append` instead of `append child`, `insert after` without anchor reference) +- Removed CRC parsing from `parseChunkSelector()` and `parseChunkReadPath()`: selectors no longer extract embedded checksums +- Chunk edit normalization simplified: no longer requires async checksum resolution or context-dependent operation mapping - RPC mode now automatically disables session title generation by default; hosts can opt in with `PI_RPC_EMIT_TITLE=1` environment variable to receive title updates - RPC mode now resets workflow-altering `todo.*`, `task.*`, and `async.*` settings to built-in defaults instead of inheriting user overrides - RPC mode now disables automatic session title generation by default and suppresses `setTitle` extension UI requests unless hosts opt in with `PI_RPC_EMIT_TITLE=1` @@ -61,6 +72,7 @@ ### Fixed +- Chunk read output now correctly preserves embedded CRC in selectors (e.g., `class_Foo.fn_bar#ZZPM`) instead of stripping them during path parsing - Chunk edit error messages now consistently report checksum mismatches with format `Checksum mismatch` instead of variable phrasing - Chunk-mode read output now correctly displays scoped response trees showing only touched chunks and adjacent siblings, preventing unrelated distant chunks from appearing in responses - DAP stopped event handling no longer blocks the message reader, preventing potential deadlocks during rapid event sequences diff --git a/packages/coding-agent/src/edit/modes/chunk.ts b/packages/coding-agent/src/edit/modes/chunk.ts index 87ec11c40..90f1e982a 100644 --- a/packages/coding-agent/src/edit/modes/chunk.ts +++ b/packages/coding-agent/src/edit/modes/chunk.ts @@ -1,7 +1,7 @@ import * as fs from "node:fs/promises"; import * as nodePath from "node:path"; import type { AgentToolResult } from "@oh-my-pi/pi-agent-core"; -import { StringEnum } from "@oh-my-pi/pi-ai"; +import { StringEnum } from "@oh-my-pi/pi-coding-agent"; import { ChunkAnchorStyle, ChunkEditOp, @@ -29,12 +29,11 @@ import type { EditToolDetails, LspBatchRequest } from "../renderer"; export type { ChunkReadTarget }; export type ChunkEditOperation = - | { op: "append_child"; sel?: string; crc?: string; content: string } - | { op: "prepend_child"; sel?: string; crc?: string; content: string } - | { op: "append_sibling"; sel?: string; crc?: string; content: string } - | { op: "prepend_sibling"; sel?: string; crc?: string; content: string } - | { op: "replace"; sel?: string; crc?: string; content: string } - | { op: "replace_body"; sel?: string; crc?: string; content: string }; + | { op: "replace"; sel?: string; content: string } + | { op: "before"; sel?: string; content: string } + | { op: "after"; sel?: string; content: string } + | { op: "prepend"; sel?: string; content: string } + | { op: "append"; sel?: string; content: string }; type ChunkEditResult = { diffSourceBefore: string; @@ -49,7 +48,6 @@ type ChunkEditResult = { export type ParsedChunkReadPath = { filePath: string; selector?: string; - crc?: string; }; type ChunkCacheEntry = { @@ -86,13 +84,14 @@ const chunkStateCache = new LRUCache({ max: readEnvInt("PI_CHUNK_CACHE_MAX_ENTRIES", 200), }); -const HASHLINE_NIBBLE_ALPHABET = "ZPMQVRWSNKTXJBYH"; -const CHECKSUM_SUFFIX_RE = new RegExp(`^(.*?)(?:\\s+)?#([${HASHLINE_NIBBLE_ALPHABET}]{4})$`, "i"); - export function invalidateChunkCache(filePath: string): void { chunkStateCache.delete(filePath); } +type ParsedChunkTarget = { + selector: string; +}; + type ChunkSourceContext = { resolvedPath: string; sourceFile: BunFile; @@ -122,19 +121,7 @@ function fileLanguageTag(filePath: string, language?: string): string | undefine } function resolveChunkTarget(target: string): ParsedChunkTarget { - const parsed = parseChunkSelector(target); - return { - selector: parsed.selector ?? target, - crc: parsed.crc, - }; -} - -function resolveChunkSiblingSelector(params: { selector: string; anchor: string | undefined; op: string }): string { - const { selector, anchor, op } = params; - if (!anchor) { - throw new Error(`'anchor' required for op=${op} on ${describeChunkTarget(selector)}.`); - } - return joinChunkPath(selector, anchor); + return { selector: target }; } async function resolveChunkSourceContext(session: ToolSession, path: string): Promise { @@ -185,17 +172,10 @@ function chunkReadPathSeparatorIndex(readPath: string): number { return readPath.indexOf(":"); } -export function parseChunkSelector(selector: string | undefined): { selector?: string; crc?: string } { +export function parseChunkSelector(selector: string | undefined): { selector?: string } { if (!selector || selector.length === 0) { return {}; } - const match = CHECKSUM_SUFFIX_RE.exec(selector); - if (!match) return { selector }; - const normalizedSelector = match[1] ?? ""; - const crc = match[2]?.toUpperCase(); - if (normalizedSelector.length > 0) { - return { selector: normalizedSelector, crc }; - } return { selector }; } @@ -208,7 +188,6 @@ export function parseChunkReadPath(readPath: string): ParsedChunkReadPath { return { filePath: readPath.slice(0, colonIndex), selector: parsedSelector.selector, - crc: parsedSelector.crc, }; } @@ -254,6 +233,7 @@ export async function formatChunkedRead(params: { ? { startLine: absoluteLineRange.startLine, endLine: absoluteLineRange.endLine ?? absoluteLineRange.startLine } : undefined, tabReplacement: " ", + normalizeIndent: true, }); return { text: result.text, resolvedPath: filePath, chunk: result.chunk }; } @@ -276,24 +256,16 @@ function toNativeEditOperation(operation: ChunkEditOperation): NativeEditOperati return { op: ChunkEditOp.Replace, sel: operation.sel, - crc: operation.crc, - content: operation.content, - }; - case "append_child": - return { op: ChunkEditOp.AppendChild, sel: operation.sel, crc: operation.crc, content: operation.content }; - case "prepend_child": - return { op: ChunkEditOp.PrependChild, sel: operation.sel, crc: operation.crc, content: operation.content }; - case "append_sibling": - return { op: ChunkEditOp.AppendSibling, sel: operation.sel, crc: operation.crc, content: operation.content }; - case "prepend_sibling": - return { op: ChunkEditOp.PrependSibling, sel: operation.sel, crc: operation.crc, content: operation.content }; - case "replace_body": - return { - op: ChunkEditOp.ReplaceBody, - sel: operation.sel, - crc: operation.crc, content: operation.content, }; + case "before": + return { op: ChunkEditOp.Before, sel: operation.sel, content: operation.content }; + case "after": + return { op: ChunkEditOp.After, sel: operation.sel, content: operation.content }; + case "prepend": + return { op: ChunkEditOp.Prepend, sel: operation.sel, content: operation.content }; + case "append": + return { op: ChunkEditOp.Append, sel: operation.sel, content: operation.content }; default: { const exhaustive: never = operation; return exhaustive; @@ -339,39 +311,18 @@ export function missingChunkReadTarget(selector: string): ChunkReadTarget { return { status: ChunkReadStatus.NotFound, selector }; } -const CHUNK_OP_VALUES = [ - "replace", - "replace_body", - "append", - "prepend", - "after", - "before", - "append_child", - "prepend_child", - "append_sibling", - "prepend_sibling", -] as const; +const CHUNK_OP_VALUES = ["replace", "after", "before", "prepend", "append"] as const; export const chunkToolEditSchema = Type.Object({ target: Type.String({ description: - "Chunk path from read output, with #CRC suffix for mutations (e.g. 'class_X.fn_y#A14F'). Use parent path without #CRC for insert ops.", + "Chunk selector. Format: 'path@region' for insertions, 'path#CRC@region' for replace. @region defaults to @container. Valid regions: container, prologue, body, epilogue.", }), - op: Type.Optional( - StringEnum(CHUNK_OP_VALUES, { - description: - "Edit op (default: replace). Use replace with empty content to remove a chunk. 'append'/'prepend' insert as last/first child. 'after'/'before' insert at sibling position; require 'anchor'.", - }), - ), + op: Type.Optional(StringEnum(CHUNK_OP_VALUES)), content: Type.String({ - description: - 'New content: required for append/prepend/after/before. For replace, use the full chunk body from read output, or "" to remove the chunk.', + description: "New content. Use \\t for indentation. Do NOT include the chunk's base padding.", }), - anchor: Type.Optional( - Type.String({ description: "Named child to insert relative to (required for op=after/before)." }), - ), }); - export const chunkEditParamsSchema = Type.Object( { path: Type.String({ description: "File path" }), @@ -386,17 +337,6 @@ export const chunkEditParamsSchema = Type.Object( export type ChunkToolEdit = Static; export type ChunkParams = Static; -type ParsedChunkTarget = { - selector: string; - crc?: string; -}; - -type ChunkExecutionContext = { - resolvedPath: string; - sourceExists: boolean; - chunkLanguage: string | undefined; -}; - interface ExecuteChunkModeOptions { session: ToolSession; params: ChunkParams; @@ -423,99 +363,21 @@ function parseChunkTarget(target: string): ParsedChunkTarget { return resolveChunkTarget(target); } -function joinChunkPath(parent: string, child: string): string { - if (child.length === 0) { - throw new Error("Sibling name cannot be empty."); - } - if (parent.length === 0 || child.startsWith(`${parent}.`)) { - return child; - } - return `${parent}.${child}`; -} - -function describeChunkTarget(selector: string): string { - return selector.length > 0 ? `"${selector}"` : "root"; -} - -async function resolveRequiredChunkChecksum(params: { - op: string; - crc: string | undefined; - selector: string; - context: ChunkExecutionContext; -}): Promise { - const { op, crc, selector, context } = params; - if (crc) return crc.toUpperCase(); - - if (selector.length > 0 && context.sourceExists) { - const resolved = await getChunkInfoForFile(context.resolvedPath, context.chunkLanguage, selector); - if (resolved) { - throw new Error( - `Checksum required for ${op} on ${describeChunkTarget(selector)}. ` + - `Re-read the chunk to get its checksum, then pass target: "${selector}#${resolved.checksum}".`, - ); - } - throw new Error(`Chunk not found: "${selector}". Re-read the file to see available chunk paths.`); - } - - throw new Error( - `Checksum required for ${op} on ${describeChunkTarget(selector)}. ` + - "Re-read the file first, then pass target with a #XXXX checksum suffix copied from the read output.", - ); -} - -async function normalizeChunkEditOperation( - edit: ChunkToolEdit, - context: ChunkExecutionContext, -): Promise { - const { selector, crc } = parseChunkTarget(edit.target); +function normalizeChunkEditOperation(edit: ChunkToolEdit): ChunkEditOperation { + const { selector } = parseChunkTarget(edit.target); const op = edit.op ?? "replace"; const content = edit.content; - - switch (op) { - case "append": - case "append_child": - return { op: "append_child", sel: selector, content }; - case "prepend": - case "prepend_child": - return { op: "prepend_child", sel: selector, content }; - case "after": - case "append_sibling": - return { - op: "append_sibling", - sel: resolveChunkSiblingSelector({ selector, anchor: edit.anchor, op }), - content, - }; - case "before": - case "prepend_sibling": - return { - op: "prepend_sibling", - sel: resolveChunkSiblingSelector({ selector, anchor: edit.anchor, op }), - content, - }; - case "replace_body": - return { - op: "replace_body", - sel: selector, - crc: await resolveRequiredChunkChecksum({ op: "replace_body", crc, selector, context }), - content, - }; - default: - return { - op: "replace", - sel: selector, - crc: await resolveRequiredChunkChecksum({ op: "replace", crc, selector, context }), - content, - }; - } + return { + op, + sel: selector, + content, + }; } -async function normalizeChunkEditOperations( - edits: ChunkToolEdit[], - context: ChunkExecutionContext, -): Promise { +function normalizeChunkEditOperations(edits: ChunkToolEdit[]): ChunkEditOperation[] { const operations: ChunkEditOperation[] = []; for (const edit of edits) { - operations.push(await normalizeChunkEditOperation(edit, context)); + operations.push(normalizeChunkEditOperation(edit)); } return operations; } @@ -582,11 +444,7 @@ export async function executeChunkMode( if (parentDir && parentDir !== ".") { await fs.mkdir(parentDir, { recursive: true }); } - const normalizedOperations = await normalizeChunkEditOperations(edits, { - resolvedPath, - sourceExists, - chunkLanguage, - }); + const normalizedOperations = normalizeChunkEditOperations(edits); const chunkResult = applyChunkEdits({ source: rawContent, diff --git a/packages/coding-agent/src/edit/renderer.ts b/packages/coding-agent/src/edit/renderer.ts index 418746c2e..29c19ba9f 100644 --- a/packages/coding-agent/src/edit/renderer.ts +++ b/packages/coding-agent/src/edit/renderer.ts @@ -231,17 +231,13 @@ function formatChunkStreamingEdit(edit: Partial): FormattedStream switch (op) { case "append": - case "append_child": - return { srcLabel: `\u2022 append child ${target}`, dst: contentLines }; + return { srcLabel: `\u2022 append ${target}`, dst: contentLines }; case "prepend": - case "prepend_child": - return { srcLabel: `\u2022 prepend child ${target}`, dst: contentLines }; + return { srcLabel: `\u2022 prepend ${target}`, dst: contentLines }; case "after": - case "append_sibling": - return { srcLabel: `\u2022 insert after ${target}/${edit.anchor ?? "?"}`, dst: contentLines }; + return { srcLabel: `\u2022 insert after ${target}`, dst: contentLines }; case "before": - case "prepend_sibling": - return { srcLabel: `\u2022 insert before ${target}/${edit.anchor ?? "?"}`, dst: contentLines }; + return { srcLabel: `\u2022 insert before ${target}`, dst: contentLines }; default: return { srcLabel: contentLines.length === 0 ? `\u2022 remove ${target}` : `\u2022 replace ${target}`, diff --git a/packages/coding-agent/src/prompts/tools/chunk-edit.md b/packages/coding-agent/src/prompts/tools/chunk-edit.md index 73b836601..ff52e2438 100644 --- a/packages/coding-agent/src/prompts/tools/chunk-edit.md +++ b/packages/coding-agent/src/prompts/tools/chunk-edit.md @@ -1,21 +1,59 @@ -Edits files via syntax-aware chunks. Run `read(path="file.ts")` first — the default read output shows anchors like `class_X.fn_y.if_2#CCCC`. Copy that exact `class_X.fn_y.if_2#CCCC` into `target`. +Edits files via syntax-aware chunks. Run `read(path="file.ts")` first. The edit target is a chunk selector, optionally qualified with a region. -- **MUST** `read` first. NEVER invent chunk names or CRCs — copy them from the latest read output or edit response. -- `target` **MUST** be the **fully-qualified** path: `class_X.fn_y.if_2#CCCC` -- If the exact path is unclear, or your anchor style omits full paths, run `read(path="file", sel="?")` and copy a canonical target from that listing. -- `content` must match the full chunk region you are replacing (same span as read output), with correct inner indentation — except use `content: ""` to remove the chunk. -- Prefer `replace_body` when you are only changing a function/class implementation. It preserves the surrounding declaration shape and avoids accidentally dropping attached doc comments. -- Successful edits return refreshed anchors — use them for follow-ups, don't re-read just for new CRCs. +- **MUST** `read` first. Never invent chunk paths or CRCs. Copy them from the latest `read` output or edit response. +- `target` format: + - insertions: `chunk` or `chunk@region` + - replacements: `chunk#CRC` or `chunk#CRC@region` +- `@region` defaults to `@container`. Valid regions: `container`, `prologue`, `body`, `epilogue`. +- If the exact chunk path is unclear, or your anchor style omits full paths, run `read(path="file", sel="?")` and copy a selector from that listing. The listing also shows which regions each chunk supports. +- Use `\t` for indentation in `content`. Do **NOT** include the chunk's base indentation. Only indent relative to the chunk's opening level. +- `replace` requires the current CRC. Insertions do not. +- Successful edits return refreshed chunk anchors. Use the latest selectors/CRCs for follow-up edits. - -|op|fields|effect| -|---|---|---| -|`replace`|`target#CRC`, `content`|rewrite or, with empty content, entire chunk| -|`replace_body`|`target#CRC`, `content`|rewrite only the inner body of the chunk, preserving signature and closing delimiter| -|`append_child` / `prepend_child`|`target`, `content`|insert as child of target| -|`append_sibling` / `prepend_sibling`|`target`, `anchor` (child name), `content`|insert as sibling of anchor| + +- `@container` — the full owned extent of the chunk. Default when `@region` is omitted. +- `@prologue` — attached trivia, header/signature, and opening delimiter. +- `@body` — the editable interior only. +- `@epilogue` — the closing delimiter or trailing owned trailer. -For file-root edits, `target` is the file header CRC alone (e.g. `"#VSKB"`). +Leaf chunks only support `@container`. + + + +|op|target form|effect| +|---|---|---| +|`replace`|`chunk#CRC` or `chunk#CRC@region`|rewrite the addressed region (default: `@container`)| +|`before`|`chunk` or `chunk@region`|insert before the region span (default: `@container`)| +|`after`|`chunk` or `chunk@region`|insert after the region span (default: `@container`)| +|`prepend`|`chunk` or `chunk@region`|insert at the start inside the region (default: `@container`)| +|`append`|`chunk` or `chunk@region`|insert at the end inside the region (default: `@container`)| + + +- Replace only a function body without touching the closing brace: + - `target: "fn_main#ABCD@body"` + - `op: "replace"` + - `content: "\treturn compute();\n"` +- Insert a new top-level function after another top-level function: + - `target: "fn_prev"` + - `op: "after"` + - `content: "function next(): void {\n\twork();\n}\n"` +- Add a struct field: + - `target: "type_Server@body"` + - `op: "append"` + - `content: "\tport int\n"` +- Add a Go receiver method owned by the type, not a struct field: + - `target: "type_Server@container"` + - `op: "append"` + - `content: "func (s *Server) Stop() error {\n\treturn nil\n}\n"` +- Edit a doc comment or header block: + - `target: "fn_foo#WXYZ@prologue"` + - `op: "replace"` + - `content: "/**\n * Updated docs.\n */\nfunction foo() {"` +- Canonical indentation example: + - if a method body in a 4-space file should contain `return x;`, write `content: "\treturn x;\n"` for `@body.replace` + - do not write four leading spaces + - do not include the method's existing base indentation + diff --git a/packages/coding-agent/src/prompts/tools/read-chunk.md b/packages/coding-agent/src/prompts/tools/read-chunk.md index 09a4f65b3..7e9bd1902 100644 --- a/packages/coding-agent/src/prompts/tools/read-chunk.md +++ b/packages/coding-agent/src/prompts/tools/read-chunk.md @@ -2,11 +2,11 @@ Reads files using syntax-aware chunks. - `path` — file path or URL; may include `:selector` suffix -- `sel` — optional selector: `class_Foo`, `class_Foo.fn_bar`, `?`, `L50`, `L50-L120`, or `raw` +- `sel` — optional selector: `class_Foo`, `class_Foo.fn_bar#ABCD@body`, `?`, `L50`, `L50-L120`, or `raw` - `timeout` — seconds, for URLs only -Each anchor `[full.chunk.path#CCCC]` in the default output is an exact chunk ID. Copy `full.chunk.path#CCCC` into the edit tool's `target` field. -If you need a canonical target list, or your anchor style omits full paths, run `read(path="file", sel="?")` and copy a path from that listing. +Each anchor `[full.chunk.path#CCCC]` in the default output identifies a chunk container. Use `full.chunk.path#CCCC` for `@container`, or add `@prologue`, `@body`, or `@epilogue` when targeting a specific region. +If you need a canonical target list, or your anchor style omits full paths, run `read(path="file", sel="?")`. That listing shows chunk paths plus the regions each chunk supports. Line numbers in the gutter are absolute file line numbers. Chunk trees: JS, TS, TSX, Python, Rust, Go. Others use blank-line fallback. diff --git a/packages/coding-agent/test/core/chunk-tree.test.ts b/packages/coding-agent/test/core/chunk-tree.test.ts index 5c3169fd6..be59b7394 100644 --- a/packages/coding-agent/test/core/chunk-tree.test.ts +++ b/packages/coding-agent/test/core/chunk-tree.test.ts @@ -31,7 +31,7 @@ describe("parseChunkReadPath", () => { test("path with chunk checksum suffix normalizes selector", () => { expect(parseChunkReadPath("file.ts:class_Foo.fn_bar#ZZPM")).toEqual({ filePath: "file.ts", - selector: "class_Foo.fn_bar", + selector: "class_Foo.fn_bar#ZZPM", }); }); @@ -97,9 +97,31 @@ function getChecksum(source: string, chunkPath: string, language = "typescript") return chunk.checksum; } +function targetWithChecksum( + chunkPath: string, + checksum: string, + region: "container" | "prologue" | "body" | "epilogue" = "container", +): string { + return `${chunkPath}#${checksum}${region === "container" ? "" : `@${region}`}`; +} + +function currentTarget( + source: string, + chunkPath: string, + language = "typescript", + region: "container" | "prologue" | "body" | "epilogue" = "container", +): string { + return targetWithChecksum(chunkPath, getChecksum(source, chunkPath, language), region); +} + +function bodyTarget(chunkPath: string): string { + return `${chunkPath}@body`; +} + describe("applyChunkEdits", () => { test("replace accepts a copied chunk header with checksum suffix", () => { - const ac = { sel: "class_Worker.fn_run", crc: getChecksum(testSource, "class_Worker.fn_run") }; + const originalChecksum = getChecksum(testSource, "class_Worker.fn_run"); + const ac = { sel: targetWithChecksum("class_Worker.fn_run", originalChecksum) }; const result = edit([ { op: "replace", @@ -111,7 +133,7 @@ describe("applyChunkEdits", () => { expect(result.diffSourceAfter).toContain("stop()"); expect(result.diffSourceAfter).not.toContain("run()"); const newChecksum = getChecksum(result.diffSourceAfter, "class_Worker.fn_stop"); - expect(newChecksum).not.toBe(ac.crc); + expect(newChecksum).not.toBe(originalChecksum); }); test("replace with wrong checksum throws with mismatch", () => { @@ -119,19 +141,18 @@ describe("applyChunkEdits", () => { edit([ { op: "replace", - sel: "class_Worker.fn_run", - crc: "ZZZZ", + sel: targetWithChecksum("class_Worker.fn_run", "ZZZZ"), content: "replacement", }, ]), ).toThrow(/Checksum mismatch/); }); - test("append_child on branch inserts after existing members", () => { + test("append on a class body inserts after existing members", () => { const result = edit([ { - op: "append_child", - sel: "class_Worker", + op: "append", + sel: bodyTarget("class_Worker"), content: `\tstatus(): string {\n\t\treturn "active";\n\t}`, }, ]); @@ -144,12 +165,12 @@ describe("applyChunkEdits", () => { expect(after).toContain("class Worker"); }); - test("append_child on an empty container inserts inside the container", () => { + test("append on an empty container body inserts inside the container", () => { const result = edit( [ { - op: "append_child", - sel: "class_Empty", + op: "append", + sel: bodyTarget("class_Empty"), content: "method(): void {}\n", }, ], @@ -163,7 +184,7 @@ describe("applyChunkEdits", () => { }); test("replace with empty content removes the target chunk", () => { - const ac = { sel: "class_Worker.fn_run", crc: getChecksum(testSource, "class_Worker.fn_run") }; + const ac = { sel: currentTarget(testSource, "class_Worker.fn_run") }; const result = edit([{ op: "replace", ...ac, content: "" }]); expect(result.diffSourceAfter).not.toContain("run()"); @@ -177,8 +198,7 @@ describe("applyChunkEdits", () => { [ { op: "replace", - sel: "class_Worker.fn_restart", - crc: checksum, + sel: targetWithChecksum("class_Worker.fn_restart", checksum), content: `\trestart(): void {\n\t\tshutdown();\n\t}`, }, ], @@ -197,8 +217,7 @@ describe("applyChunkEdits", () => { [ { op: "replace", - sel: "class_Worker.fn_restart", - crc: checksum, + sel: targetWithChecksum("class_Worker.fn_restart", checksum), content: `\t/** updated restart note */\n\trestart(): void {\n\t\tshutdown();\n\t}`, }, ], @@ -221,14 +240,12 @@ describe("applyChunkEdits", () => { operations: [ { op: "replace", - sel: "class_Worker.constructor", - crc: ctorCrc, + sel: targetWithChecksum("class_Worker.constructor", ctorCrc), content: `\tconstructor(name: string) {\n\t\tthis.name = name.trim();\n\t}`, }, { op: "replace", - sel: "class_Worker.fn_run", - crc: runCrc, + sel: targetWithChecksum("class_Worker.fn_run", runCrc), content: `\trun(): void {\n\t\tconsole.log(this.name + "!");\n\t}`, }, ], @@ -239,11 +256,11 @@ describe("applyChunkEdits", () => { expect(result.diffSourceAfter).toMatch(/this\.name\s*\+\s*"!"/); }); - test("prepend_child on branch inserts before existing members", () => { + test("prepend on a class body inserts before existing members", () => { const result = edit([ { - op: "prepend_child", - sel: "class_Worker", + op: "prepend", + sel: bodyTarget("class_Worker"), content: `\tid = 0;`, }, ]); @@ -258,7 +275,7 @@ describe("applyChunkEdits", () => { }); describe("insertion boundaries", () => { - test("keeps prepend_child separated from the first existing TypeScript class member", () => { + test("keeps prepend separated from the first existing TypeScript class member", () => { const source = `class Box {\n value(): T {\n return this.current;\n }\n}\n`; const result = applyEdit({ source, @@ -266,8 +283,8 @@ describe("insertion boundaries", () => { filePath: "/tmp/box.ts", operations: [ { - op: "prepend_child", - sel: "class_Box", + op: "prepend", + sel: bodyTarget("class_Box"), content: ` items(): T[] {\n return [];\n }`, }, ], @@ -277,7 +294,7 @@ describe("insertion boundaries", () => { expect(result.diffSourceAfter).not.toContain("}\n value(): T {"); }); - test("keeps prepend_child separated from the first existing Rust impl member", () => { + test("keeps prepend separated from the first existing Rust impl member", () => { const source = `impl Widget {\n fn old(&self) -> bool {\n true\n }\n}\n`; const result = applyEdit({ source, @@ -285,8 +302,8 @@ describe("insertion boundaries", () => { filePath: "/tmp/widget.rs", operations: [ { - op: "prepend_child", - sel: "impl_Widget", + op: "prepend", + sel: bodyTarget("impl_Widget"), content: ` fn build(&self) -> bool {\n false\n }`, }, ], @@ -296,7 +313,7 @@ describe("insertion boundaries", () => { expect(result.diffSourceAfter).not.toContain("}\n fn old(&self) -> bool {"); }); - test("keeps prepend_sibling separated before a Go top-level function", () => { + test("keeps before separated before a Go top-level function", () => { const source = `package main\n\nfunc format() {}\n`; const result = applyEdit({ source, @@ -304,7 +321,7 @@ describe("insertion boundaries", () => { filePath: "/tmp/format.go", operations: [ { - op: "prepend_sibling", + op: "before", sel: "fn_format", content: "func formatLog() {}", }, @@ -315,7 +332,7 @@ describe("insertion boundaries", () => { expect(result.diffSourceAfter).not.toContain("func formatLog() {}\nfunc format() {}"); }); - test("keeps append_sibling separated from the next Go top-level function", () => { + test("keeps after separated from the next Go top-level function", () => { const source = `package main\n\nfunc first() {}\nfunc second() {}\n`; const result = applyEdit({ source, @@ -323,7 +340,7 @@ describe("insertion boundaries", () => { filePath: "/tmp/functions.go", operations: [ { - op: "append_sibling", + op: "after", sel: "fn_first", content: "func middle() {}", }, @@ -334,7 +351,7 @@ describe("insertion boundaries", () => { expect(result.diffSourceAfter).not.toContain("func middle() {}\nfunc second() {}"); }); - test("append_child on a Go receiver type inserts after the last receiver method", () => { + test("append on a Go receiver type container inserts after the last receiver method", () => { const source = `package main\n\ntype Server struct {}\n\nfunc (s *Server) Start() {}\nfunc (s *Server) Stop() {}\n`; const result = applyEdit({ source, @@ -342,8 +359,8 @@ describe("insertion boundaries", () => { filePath: "/tmp/server.go", operations: [ { - op: "append_child", - sel: "type_Server", + op: "append", + sel: "type_Server@container", content: "func (s *Server) Restart() {}", }, ], @@ -355,24 +372,21 @@ describe("insertion boundaries", () => { expect(result.diffSourceAfter).not.toContain("type Server struct {\nfunc (s *Server) Restart() {}"); }); - test("append_child on Go type_Server with struct fields still inserts file-scope func at column 0", () => { + test("append on Go type_Server container with struct fields still inserts file-scope func at column 0", () => { const source = `package main type Server struct { Addr string } `; - const crc = ChunkState.parse(source, "go").chunk("type_Server")?.checksum; - expect(crc).toBeDefined(); const result = applyEdit({ source, language: "go", filePath: "/tmp/server.go", operations: [ { - op: "append_child", - sel: "type_Server", - crc, + op: "append", + sel: "type_Server@container", content: "func (s *Server) Ping() {}", }, ], @@ -383,24 +397,21 @@ type Server struct { expect(result.diffSourceAfter).not.toMatch(/Addr string\n[ \t]+func \(s \*Server\) Ping/); }); - test("append_child on Go type_Server keeps receiver method body indentation relative to column 0", () => { + test("append on Go type_Server container keeps receiver method body indentation relative to column 0", () => { const source = `package main type Server struct { Addr string } `; - const crc = ChunkState.parse(source, "go").chunk("type_Server")?.checksum; - expect(crc).toBeDefined(); const result = applyEdit({ source, language: "go", filePath: "/tmp/server.go", operations: [ { - op: "append_child", - sel: "type_Server", - crc, + op: "append", + sel: "type_Server@container", content: "func (s *Server) LogCount() int {\n\ts.mu.Lock()\n\tdefer s.mu.Unlock()\n\treturn 0\n}", }, ], @@ -412,11 +423,11 @@ type Server struct { ); expect(result.diffSourceAfter).not.toContain("\n\tfunc (s *Server) LogCount() int {"); }); - test("keeps append_child separated from the closing delimiter when adding the last child", () => { + test("keeps append separated from the closing delimiter when adding the last child", () => { const result = edit([ { - op: "append_child", - sel: "class_Worker", + op: "append", + sel: bodyTarget("class_Worker"), content: '\tstatus(): string {\n\t\treturn "active";\n\t}', }, ]); @@ -438,7 +449,7 @@ type Server struct { source, language: "rust", filePath: "/tmp/impl.rs", - operations: [{ op: "replace", sel: "impl_S.fn_b", crc, content: "" }], + operations: [{ op: "replace", sel: targetWithChecksum("impl_S.fn_b", crc), content: "" }], }); expect(result.diffSourceAfter).toBe("impl S {\n fn a() {}\n\n}\n"); @@ -453,8 +464,7 @@ describe("edit safety invariants", () => { const source = edit([ { op: "replace", - sel: runChunkPath, - crc: staleChecksum, + sel: targetWithChecksum(runChunkPath, staleChecksum), content: '\trun(): void {\n\t\tconsole.log("updated");\n\t}', }, ]).diffSourceAfter; @@ -475,15 +485,14 @@ describe("edit safety invariants", () => { [ { op: "replace", - sel: runChunkPath, - crc: staleChecksum, + sel: targetWithChecksum(runChunkPath, staleChecksum), content: '\trun(): void {\n\t\tconsole.log("again");\n\t}', }, ], source, ); } - return edit([{ op: "replace", sel: runChunkPath, crc: staleChecksum, content: "" }], source); + return edit([{ op: "replace", sel: targetWithChecksum(runChunkPath, staleChecksum), content: "" }], source); }; expect(invoke).toThrow(new RegExp(`got "${staleChecksum}"`)); @@ -495,14 +504,12 @@ describe("edit safety invariants", () => { const result = edit([ { op: "replace", - sel: runChunkPath, - crc: checksum, + sel: targetWithChecksum(runChunkPath, checksum), content: '\trun(): void {\n\t\tconsole.log("first");\n\t}', }, { op: "replace", - sel: runChunkPath, - crc: checksum, + sel: targetWithChecksum(runChunkPath, checksum), content: '\trun(): void {\n\t\tconsole.log("second");\n\t}', }, ]); @@ -514,15 +521,14 @@ describe("edit safety invariants", () => { const checksum = getChecksum(testSource, runChunkPath); const firstContent = '\trun(): void {\n\t\tconsole.log("first");\n\t}'; const afterFirst = edit([ - { op: "replace", sel: runChunkPath, crc: checksum, content: firstContent }, + { op: "replace", sel: targetWithChecksum(runChunkPath, checksum), content: firstContent }, ]).diffSourceAfter; const checksum2 = getChecksum(afterFirst, runChunkPath); const result = edit([ - { op: "replace", sel: runChunkPath, crc: checksum, content: firstContent }, + { op: "replace", sel: targetWithChecksum(runChunkPath, checksum), content: firstContent }, { op: "replace", - sel: runChunkPath, - crc: checksum2, + sel: targetWithChecksum(runChunkPath, checksum2), content: '\trun(): void {\n\t\tconsole.log("second");\n\t}', }, ]); @@ -536,14 +542,12 @@ describe("edit safety invariants", () => { const result = edit([ { op: "replace", - sel: runChunkPath, - crc: checksum, + sel: targetWithChecksum(runChunkPath, checksum), content: '\trun(task = "default"): void {\n\t\tconsole.log(this.name);\n\t}', }, { op: "replace", - sel: runChunkPath, - crc: checksum, + sel: targetWithChecksum(runChunkPath, checksum), content: '\trun(task = "default"): void {\n\t\tconsole.log(task);\n\t}', }, ]); @@ -556,8 +560,7 @@ describe("edit safety invariants", () => { const afterFirst = edit([ { op: "replace", - sel: runChunkPath, - crc: checksum, + sel: targetWithChecksum(runChunkPath, checksum), content: "\trun(): void {\n\t\tconsole.log(task);\n\t}", }, ]).diffSourceAfter; @@ -565,14 +568,12 @@ describe("edit safety invariants", () => { const result = edit([ { op: "replace", - sel: runChunkPath, - crc: checksum, + sel: targetWithChecksum(runChunkPath, checksum), content: "\trun(): void {\n\t\tconsole.log(task);\n\t}", }, { op: "replace", - sel: runChunkPath, - crc: checksum2, + sel: targetWithChecksum(runChunkPath, checksum2), content: '\trun(task = "default"): void {\n\t\tconsole.log(task);\n\t}', }, ]); @@ -586,14 +587,13 @@ describe("edit safety invariants", () => { expect(() => edit([ { - op: "append_child", - sel: "class_Worker", + op: "append", + sel: bodyTarget("class_Worker"), content: '\tstatus(): string {\n\t\treturn "active";\n\t}', }, { op: "replace", - sel: "class_Worker.fn_run", - crc: "ZZZZ", + sel: targetWithChecksum("class_Worker.fn_run", "ZZZZ"), content: "", }, ]), @@ -605,8 +605,7 @@ describe("edit safety invariants", () => { const after = edit([ { op: "replace", - sel: runChunkPath, - crc: getChecksum(testSource, runChunkPath), + sel: currentTarget(testSource, runChunkPath), content: '\trun(): void {\n\t\tconsole.log("nearby");\n\t}', }, ]).diffSourceAfter; @@ -621,8 +620,7 @@ describe("edit safety invariants", () => { [ { op: "replace", - sel: runChunkPath, - crc: checksum, + sel: targetWithChecksum(runChunkPath, checksum), content: '\trun(): void {\n\t\tconsole.log("updated");\n\t}', }, ], @@ -640,7 +638,7 @@ describe("edit safety invariants", () => { describe("content prefix stripping", () => { test("line-number prefixes are stripped from replacement content", () => { - const ac = { sel: "class_Worker.fn_run", crc: getChecksum(testSource, "class_Worker.fn_run") }; + const ac = { sel: currentTarget(testSource, "class_Worker.fn_run") }; const result = edit([ { op: "replace", @@ -654,7 +652,7 @@ describe("content prefix stripping", () => { }); test("hashline prefixes are stripped from replacement content", () => { - const ac = { sel: "class_Worker.fn_run", crc: getChecksum(testSource, "class_Worker.fn_run") }; + const ac = { sel: currentTarget(testSource, "class_Worker.fn_run") }; const result = edit([ { op: "replace", @@ -679,7 +677,7 @@ describe("chunk path resolution errors", () => { filePath: "/tmp/worker.ts", operations: [ { - op: "prepend_sibling", + op: "before", sel: "class_Worker.fn_ghost", content: "\tghost(): void {}", }, @@ -772,10 +770,10 @@ describe("formatChunkedRead", () => { }); describe("leaf insert indentation", () => { - test("prepend_sibling on a nested method uses the method's indent level", () => { + test("before on a nested method uses the method's indent level", () => { const result = edit([ { - op: "prepend_sibling", + op: "before", sel: "class_Worker.fn_run", content: "validate(): boolean {\n\treturn true;\n}", }, @@ -789,10 +787,10 @@ describe("leaf insert indentation", () => { expect(validatePos).toBeLessThan(runPos); }); - test("append_sibling on a nested method uses the method's indent level", () => { + test("after on a nested method uses the method's indent level", () => { const result = edit([ { - op: "append_sibling", + op: "after", sel: "class_Worker.fn_run", content: 'stop(): void {\n\tconsole.log("stopped");\n}', }, @@ -808,7 +806,7 @@ describe("leaf insert indentation", () => { describe("replace last child formatting", () => { test("replacing last method does not merge with closing brace", () => { - const ac = { sel: "class_Worker.fn_run", crc: getChecksum(testSource, "class_Worker.fn_run") }; + const ac = { sel: currentTarget(testSource, "class_Worker.fn_run") }; const result = edit([ { op: "replace", @@ -824,7 +822,7 @@ describe("replace last child formatting", () => { }); test("replace dedents uniformly over-indented content so the chunk column is not applied twice", () => { - const ac = { sel: "class_Worker.fn_run", crc: getChecksum(testSource, "class_Worker.fn_run") }; + const ac = { sel: currentTarget(testSource, "class_Worker.fn_run") }; const result = edit([ { op: "replace", @@ -978,7 +976,7 @@ describe("addressable member editing", () => { const enumSource = `enum Status {\n Idle = "idle",\n Busy = "busy",\n}\n`; test("replace accepts full-source edits on the parent enum container", () => { - const ac = { sel: "enum_Status", crc: getChecksum(enumSource, "enum_Status") }; + const ac = { sel: currentTarget(enumSource, "enum_Status") }; const result = edit( [ { @@ -994,11 +992,11 @@ describe("addressable member editing", () => { expect(result.diffSourceAfter).not.toContain('Busy = "busy"'); }); - test("append_sibling inserts beside an individually addressable enum variant", () => { + test("after inserts beside an individually addressable enum variant", () => { const result = edit( [ { - op: "append_sibling", + op: "after", sel: "enum_Status.variant_Idle", content: 'Paused = "paused",', }, @@ -1010,7 +1008,7 @@ describe("addressable member editing", () => { }); test("replace with empty content removes an individually addressable enum variant", () => { - const busy = { sel: "enum_Status.variant_Busy", crc: getChecksum(enumSource, "enum_Status.variant_Busy") }; + const busy = { sel: currentTarget(enumSource, "enum_Status.variant_Busy") }; const result = edit([{ op: "replace", ...busy, content: "" }], enumSource); expect(result.diffSourceAfter).toContain('Idle = "idle"'); @@ -1027,7 +1025,7 @@ describe("Go receiver render ownership", () => { filePath: "/tmp/server.go", operations: [ { - op: "prepend_sibling", + op: "before", sel: "type_Server.fn_Start", content: "func DefaultServer() *Server {\n return &Server{}\n}", }, @@ -1059,8 +1057,7 @@ describe("blank-line cleanup", () => { [ { op: "replace", - sel: "class_Worker.fn_restart", - crc: checksum, + sel: targetWithChecksum("class_Worker.fn_restart", checksum), content: "", }, ], @@ -1076,22 +1073,17 @@ describe("blank-line cleanup", () => { // splice // ═══════════════════════════════════════════════════════════════════════════ -describe("prepend_child warnings", () => { - test("warns when comment-only prepend_child may merge into the next chunk", () => { +describe("prepend warnings", () => { + test("warns when comment-only body prepend may merge into the next chunk", () => { const source = `package main\n\nimport "fmt"\n`; - const state = ChunkState.parse(source, "go"); - const root = state.root(); - if (!root) { - throw new Error("expected root chunk"); - } const result = applyChunkEdits({ source, language: "go", cwd: "/", filePath: "main.go", - operations: [{ op: "prepend_child", sel: "", crc: root.checksum, content: "// AUTO-GENERATED\n" }], + operations: [{ op: "prepend", sel: "@body", content: "// AUTO-GENERATED\n" }], }); - expect(result.warnings.some(w => w.includes("Comment-only prepend_child"))).toBe(true); + expect(result.warnings.some(w => w.includes("Comment-only @body.prepend"))).toBe(true); }); }); @@ -1100,8 +1092,7 @@ describe("chunk selector auto-resolution", () => { const result = edit([ { op: "replace", - sel: "fn_run", - crc: getChecksum(testSource, "class_Worker.fn_run"), + sel: targetWithChecksum("fn_run", getChecksum(testSource, "class_Worker.fn_run")), content: "run(): void {\n\tconsole.log(this.name);\n}", }, ]); @@ -1114,8 +1105,7 @@ describe("chunk selector auto-resolution", () => { const result = edit([ { op: "replace", - sel: "run", - crc: getChecksum(testSource, "class_Worker.fn_run"), + sel: targetWithChecksum("run", getChecksum(testSource, "class_Worker.fn_run")), content: "run(): void {\n\tconsole.log(this.name);\n}", }, ]); @@ -1133,8 +1123,7 @@ describe("chunk selector auto-resolution", () => { operations: [ { op: "replace", - sel: "fn_run", - crc: getChecksum(source, "class_Foo.fn_run"), + sel: targetWithChecksum("fn_run", getChecksum(source, "class_Foo.fn_run")), content: "", }, ], @@ -1143,8 +1132,8 @@ describe("chunk selector auto-resolution", () => { }); }); -describe("prepend_child preamble guard", () => { - test("errors when comment-only prepend_child targets root with preamble", () => { +describe("prepend preamble guard", () => { + test("errors when comment-only body prepend targets root with preamble", () => { // JS source with a leading comment block that becomes preamble const source = `/**\n * License header\n */\nconst x = 1;\n`; const state = ChunkState.parse(source, "javascript"); @@ -1153,17 +1142,15 @@ describe("prepend_child preamble guard", () => { // Skip if parser doesn't produce preamble for this source return; } - const root = state.root(); - if (!root) throw new Error("expected root chunk"); expect(() => applyChunkEdits({ source, language: "javascript", cwd: "/", filePath: "index.js", - operations: [{ op: "prepend_child", sel: "", crc: root.checksum, content: "// AUTO-GENERATED\n" }], + operations: [{ op: "prepend", sel: "@body", content: "// AUTO-GENERATED\n" }], }), - ).toThrow(/Comment-only prepend_child on root is not allowed when the file has a preamble/); + ).toThrow(/Comment-only @body.prepend on root is not allowed when the file has a preamble/); }); }); @@ -1203,7 +1190,13 @@ describe("tlaplus chunk rendering", () => { language: "tlaplus", cwd: "/tmp", filePath: "/tmp/Spec.tla", - operations: [{ op: "replace", sel: initChunk.path, crc: initChunk.checksum, content: "Start == x = 0" }], + operations: [ + { + op: "replace", + sel: targetWithChecksum(initChunk.path, initChunk.checksum), + content: "Start == x = 0", + }, + ], }); expect(result.diffSourceAfter).toContain("Start == x = 0"); diff --git a/packages/coding-agent/test/tools/chunk-mode.test.ts b/packages/coding-agent/test/tools/chunk-mode.test.ts index e2c27d279..38800a784 100644 --- a/packages/coding-agent/test/tools/chunk-mode.test.ts +++ b/packages/coding-agent/test/tools/chunk-mode.test.ts @@ -251,8 +251,7 @@ describe("chunk mode tools", () => { operations: [ { op: "replace", - sel: chunkPath, - crc: checksum, + sel: `${chunkPath}#${checksum}`, content: buildHandleErrorMethod({ returnLine: " return err.message.toUpperCase() + total;" }), }, ], diff --git a/packages/natives/CHANGELOG.md b/packages/natives/CHANGELOG.md index d41586d24..3be359350 100644 --- a/packages/natives/CHANGELOG.md +++ b/packages/natives/CHANGELOG.md @@ -1,15 +1,20 @@ # Changelog ## [Unreleased] - ### Breaking Changes +- Replaced `ChunkEditOp` enum values — `AppendChild`, `PrependChild`, `AppendSibling`, `PrependSibling`, and `ReplaceBody` are now `Before`, `After`, `Prepend`, and `Append` with updated semantics for region-scoped operations +- Removed `ReplaceBody` operation — use `Replace` with `region: ChunkRegion.Body` to replace only chunk body content - Moved package entry point from `src/index.ts` to `native/index.js` — consumers must update imports to use the new native module path - Removed TypeScript source files from `src/` directory — all APIs now exported from auto-generated `native/index.js` with types in `native/index.d.ts` - Changed enum exports to runtime objects — `const enum` values are now available at runtime via generated enum exports in `native/index.js` ### Added +- Added `ChunkRegion` enum with `Container`, `Prologue`, `Body`, and `Epilogue` values for targeting specific regions within chunks +- Added `region` parameter to `EditOperation` to specify which chunk region to target (defaults to `Container`) +- Added `UnsupportedRegion` status to `ChunkReadStatus` enum to indicate when a chunk does not support the requested region +- Added `normalizeIndent` parameter to `RenderParams` and `ReadRenderParams` to normalize displayed indentation to canonical tabs - Added `ReplaceBody` chunk edit operation to replace only the inner body of a chunk while preserving signature and closing delimiter - Added `ChunkFocusMode` enum with `Expanded`, `Collapsed`, and `Container` modes for controlling chunk participation in focus-scoped render passes - Added `FocusedPath` interface to pair paths with focus modes for the N-API boundary @@ -21,6 +26,7 @@ ### Changed +- Updated `ChunkEditOp` documentation to reflect region-scoped semantics — operations now target specific regions rather than chunk structure positions - Changed `ChunkEditOp.Replace` documentation to clarify substring replacement via `find` parameter instead of line-based replacement - Changed `EditOperation` interface to use `find` parameter for scoped find/replace operations instead of `line` and `endLine` parameters - Changed `EditParams` documentation to remove mention of scheduling reordering for line-scoped groups diff --git a/packages/natives/native/index.d.ts b/packages/natives/native/index.d.ts index df705df85..11ad79601 100644 --- a/packages/natives/native/index.d.ts +++ b/packages/natives/native/index.d.ts @@ -381,23 +381,18 @@ export declare enum ChunkAnchorStyle { /** Structural edit to apply relative to a chunk anchor. */ export declare enum ChunkEditOp { - /** Replace the chunk body, or a substring via `find`. */ + /** Replace the targeted region, or a substring via `find`. */ Replace = 'replace', - /** Remove the chunk's source range. */ + /** Remove the targeted region. */ Delete = 'delete', - /** Insert `content` as the last child of the target chunk. */ - AppendChild = 'append_child', - /** Insert `content` as the first child of the target chunk. */ - PrependChild = 'prepend_child', - /** Insert `content` after the target chunk's source range. */ - AppendSibling = 'append_sibling', - /** Insert `content` before the target chunk's source range. */ - PrependSibling = 'prepend_sibling', - /** - * Replace only the inner body of the chunk, preserving signature and - * closing delimiter. - */ - ReplaceBody = 'replace_body' + /** Insert `content` before the targeted region span. */ + Before = 'before', + /** Insert `content` after the targeted region span. */ + After = 'after', + /** Insert `content` at the start inside the targeted region. */ + Prepend = 'prepend', + /** Insert `content` at the end inside the targeted region. */ + Append = 'append' } /** How a chunk participates in a focus-scoped render pass. */ @@ -434,7 +429,9 @@ export declare enum ChunkReadStatus { /** Selector matched a chunk and content was produced. */ Ok = 'ok', /** No chunk matched the requested selector. */ - NotFound = 'not_found' + NotFound = 'not_found', + /** Chunk matched but does not support the requested region. */ + UnsupportedRegion = 'unsupported_region' } /** Outcome of resolving which chunk was read for a `renderRead`-style request. */ @@ -445,6 +442,13 @@ export interface ChunkReadTarget { selector: string } +export declare enum ChunkRegion { + Container = 'container', + Prologue = 'prologue', + Body = 'body', + Epilogue = 'epilogue' +} + /** Clipboard image payload encoded as PNG bytes. */ export interface ClipboardImage { /** PNG-encoded image bytes. */ @@ -495,6 +499,8 @@ export interface EditOperation { * omitted. */ crc?: string + /** Region to target. When omitted, defaults to `@container`. */ + region?: ChunkRegion /** Replacement or inserted text (meaning depends on `op`). */ content?: string /** @@ -1134,6 +1140,8 @@ export interface ReadRenderParams { absoluteLineRange?: VisibleLineRange /** Replace tabs in embedded previews. */ tabReplacement?: string + /** When true, normalize displayed indentation to canonical tabs. */ + normalizeIndent?: boolean } /** Rendered chunk text plus optional resolution metadata for the read request. */ @@ -1167,6 +1175,8 @@ export interface RenderParams { showLeafPreview: boolean /** Replace tab characters in displayed previews (e.g. two spaces). */ tabReplacement?: string + /** When true, normalize displayed indentation to canonical tabs. */ + normalizeIndent?: boolean /** * When set, restrict rendering to these chunks with their specified focus * modes. Everything not in this list is skipped. diff --git a/packages/natives/native/index.js b/packages/natives/native/index.js index e439cb85c..fe2d19393 100644 --- a/packages/natives/native/index.js +++ b/packages/natives/native/index.js @@ -244,11 +244,10 @@ exports.ChunkAnchorStyle = { exports.ChunkEditOp = { Replace: 'replace', Delete: 'delete', - AppendChild: 'append_child', - PrependChild: 'prepend_child', - AppendSibling: 'append_sibling', - PrependSibling: 'prepend_sibling', - ReplaceBody: 'replace_body', + Before: 'before', + After: 'after', + Prepend: 'prepend', + Append: 'append', }; exports.ChunkFocusMode = { Expanded: 'expanded', @@ -258,6 +257,13 @@ exports.ChunkFocusMode = { exports.ChunkReadStatus = { Ok: 'ok', NotFound: 'not_found', + UnsupportedRegion: 'unsupported_region', +}; +exports.ChunkRegion = { + Container: 'container', + Prologue: 'prologue', + Body: 'body', + Epilogue: 'epilogue', }; exports.Ellipsis = { Unicode: 0,