From ad4c5917a4e9085261476047dcd9d86a1d20c2bf Mon Sep 17 00:00:00 2001 From: can1357 Date: Tue, 7 Apr 2026 03:36:57 +0200 Subject: [PATCH] refactor(chunk): restructured selector resolution and state lookup with unified classification - Refactored chunk selector resolution to extract and preserve raw selector strings separately from parsed CRC checksums. - Reorganized chunk state lookup infrastructure with dedicated maps for checksum, leaf, and suffix-based chunk resolution. - Simplified batch operation CRC validation to auto-accept stale checksums for subsequent operations on the same chunk. - Unified TypeScript interface and Nix attrset chunk classification with consistent naming and nested expression handling. - Extracted clipboard utility functions to dedicated module with improved OSC 52 and Termux support. - Updated chunk read formatting with anchorStyle parameter and ellipsis standardization for improved consistency. --- crates/pi-natives/src/chunk/ast_js_ts.rs | 4 +- crates/pi-natives/src/chunk/ast_nix_hcl.rs | 176 +++++---- crates/pi-natives/src/chunk/edit.rs | 33 +- crates/pi-natives/src/chunk/mod.rs | 213 ++++++++++- crates/pi-natives/src/chunk/resolve.rs | 338 +++++++++++++----- crates/pi-natives/src/chunk/state.rs | 198 +++++----- packages/coding-agent/CHANGELOG.md | 4 + .../src/prompts/tools/chunk-edit.md | 19 +- packages/coding-agent/src/tools/chunk-tree.ts | 6 +- packages/coding-agent/src/tools/read.ts | 6 +- .../coding-agent/test/core/chunk-tree.test.ts | 76 ++-- .../test/tools/chunk-mode.test.ts | 39 +- 12 files changed, 761 insertions(+), 351 deletions(-) diff --git a/crates/pi-natives/src/chunk/ast_js_ts.rs b/crates/pi-natives/src/chunk/ast_js_ts.rs index 32ab4f996..1519661d8 100644 --- a/crates/pi-natives/src/chunk/ast_js_ts.rs +++ b/crates/pi-natives/src/chunk/ast_js_ts.rs @@ -43,7 +43,7 @@ impl LangClassifier for JsTsClassifier { Some(container_candidate(node, "class", source, recurse_class(node))) }, "interface_declaration" => { - Some(container_candidate(node, "iface", source, recurse_interface(node))) + Some(container_candidate(node, "interface", source, recurse_interface(node))) }, "enum_declaration" => Some(container_candidate(node, "enum", source, recurse_enum(node))), "internal_module" => { @@ -281,7 +281,7 @@ fn classify_export_statement<'t>(node: Node<'t>, source: &str) -> RawChunkCandid make_container_chunk_from( node, child, - prefixed_name("iface", child, source), + prefixed_name("interface", child, source), source, recurse, ) diff --git a/crates/pi-natives/src/chunk/ast_nix_hcl.rs b/crates/pi-natives/src/chunk/ast_nix_hcl.rs index 338e77fa1..50c64b188 100644 --- a/crates/pi-natives/src/chunk/ast_nix_hcl.rs +++ b/crates/pi-natives/src/chunk/ast_nix_hcl.rs @@ -42,89 +42,109 @@ fn extract_nix_binding_name(node: Node<'_>, source: &str) -> Option { }) } +fn recurse_nix_attrset(node: Node<'_>) -> Option> { + recurse_into(node, ChunkContext::ClassBody, &[], &["binding_set"]) +} + +fn recurse_nix_binding_value(node: Node<'_>) -> Option> { + let expression = node.child_by_field_name("expression")?; + if matches!( + expression.kind(), + "attrset_expression" | "let_attrset_expression" | "rec_attrset_expression" + ) { + return recurse_nix_attrset(expression); + } + recurse_value_container(node) +} + +fn classify_nix_binding<'t>(node: Node<'t>, source: &str) -> RawChunkCandidate<'t> { + let name = extract_nix_binding_name(node, source).unwrap_or_else(|| "anonymous".to_string()); + let chunk_name = format!("attr_{name}"); + let expression = node.child_by_field_name("expression"); + if let Some(expression) = expression + && matches!( + expression.kind(), + "attrset_expression" | "let_attrset_expression" | "rec_attrset_expression" + ) + { + return make_container_chunk(node, chunk_name, source, recurse_nix_attrset(expression)); + } + make_named_chunk(node, chunk_name, source, recurse_nix_binding_value(node)) +} + impl LangClassifier for NixHclClassifier { fn classify_root<'t>(&self, node: Node<'t>, source: &str) -> Option> { - match node.kind() { - // Nix top-level attribute set entries - "attribute" => { - let name = child_by_kind(node, &["identifier"]) - .and_then(|c| sanitize_identifier(node_text(source, c.start_byte(), c.end_byte()))) - .unwrap_or_else(|| "anonymous".to_string()); - Some(make_named_chunk( - node, - format!("attr_{name}"), - source, - recurse_value_container(node), - )) - }, - "binding" => { - let name = - extract_nix_binding_name(node, source).unwrap_or_else(|| "anonymous".to_string()); - Some(make_named_chunk( - node, - format!("binding_{name}"), - source, - recurse_value_container(node), - )) - }, - // HCL top-level block, or diff hunk fallback - "block" => { - if let Some(name) = extract_hcl_block_name(node, source) { - Some(make_container_chunk( - node, - format!("block_{name}"), - source, - recurse_into(node, ChunkContext::ClassBody, &[], &["body"]), - )) - } else { - Some(group_candidate(node, "hunks", source)) - } - }, - // Nix expressions - "function_expression" | "let_expression" => { - Some(named_candidate(node, "expr", source, recurse_value_container(node))) - }, - // Nix inherit - "inherit" => Some(group_candidate(node, "imports", source)), - // Variable/assignment declarations - "variable_declaration" | "assignment" => Some(group_candidate(node, "decls", source)), - // HCL top-level block types - "provider" | "resource" | "data" | "locals" | "variable" | "output" | "module" => { - Some(container_candidate( - node, - sanitize_node_kind(node.kind()).as_str(), - source, - recurse_into(node, ChunkContext::ClassBody, &[], &["body"]), - )) - }, - _ => None, - } + match node.kind() { + // Nix top-level attrsets should recurse into their binding_set so the file exposes + // structural attr chunks instead of a single opaque attrset_expr leaf. + "attrset_expression" | "let_attrset_expression" | "rec_attrset_expression" => { + Some(make_container_chunk( + node, + sanitize_node_kind(node.kind()), + source, + recurse_nix_attrset(node), + )) + }, + // Older tree-sitter-nix revisions used `attribute`; current grammars expose `binding`. + "attribute" | "binding" => Some(classify_nix_binding(node, source)), + // HCL top-level block, or diff hunk fallback + "block" => { + if let Some(name) = extract_hcl_block_name(node, source) { + Some(make_container_chunk( + node, + format!("block_{name}"), + source, + recurse_into(node, ChunkContext::ClassBody, &[], &["body"]), + )) + } else { + Some(group_candidate(node, "hunks", source)) + } + }, + // Nix expressions + "function_expression" | "let_expression" => { + Some(named_candidate(node, "expr", source, recurse_value_container(node))) + }, + // Nix inherit + "inherit" => Some(group_candidate(node, "imports", source)), + // Variable/assignment declarations + "variable_declaration" | "assignment" => Some(group_candidate(node, "decls", source)), + // HCL top-level block types + "provider" | "resource" | "data" | "locals" | "variable" | "output" | "module" => { + Some(container_candidate( + node, + sanitize_node_kind(node.kind()).as_str(), + source, + recurse_into(node, ChunkContext::ClassBody, &[], &["body"]), + )) + }, + _ => None, + } } fn classify_class<'t>(&self, node: Node<'t>, source: &str) -> Option> { - match node.kind() { - // Nested HCL block — only promote if it has an identifiable block name - "block" => extract_hcl_block_name(node, source).map(|name| { - make_container_chunk( - node, - format!("block_{name}"), - source, - recurse_into(node, ChunkContext::ClassBody, &[], &["body"]), - ) - }), - // Nested Nix binding - "binding" => { - let name = - extract_nix_binding_name(node, source).unwrap_or_else(|| "anonymous".to_string()); - Some(make_named_chunk( - node, - format!("binding_{name}"), - source, - recurse_value_container(node), - )) - }, - _ => None, - } + match node.kind() { + // Nested HCL block — only promote if it has an identifiable block name + "block" => extract_hcl_block_name(node, source).map(|name| { + make_container_chunk( + node, + format!("block_{name}"), + source, + recurse_into(node, ChunkContext::ClassBody, &[], &["body"]), + ) + }), + // Nested Nix attrset values recurse into their binding_set just like top-level ones. + "attrset_expression" | "let_attrset_expression" | "rec_attrset_expression" => { + Some(make_container_chunk( + node, + sanitize_node_kind(node.kind()), + source, + recurse_nix_attrset(node), + )) + }, + // Nested Nix binding + "binding" => Some(classify_nix_binding(node, source)), + _ => None, + } } fn classify_function<'t>(&self, node: Node<'t>, source: &str) -> Option> { diff --git a/crates/pi-natives/src/chunk/edit.rs b/crates/pi-natives/src/chunk/edit.rs index 10528e735..6dc62b06b 100644 --- a/crates/pi-natives/src/chunk/edit.rs +++ b/crates/pi-natives/src/chunk/edit.rs @@ -261,8 +261,7 @@ fn apply_replace( } }); 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 batch_auto_accepted = ensure_batch_operation_target_current(scheduled, crc, touched_paths); let resolved = resolve_chunk_with_crc(state, anchor_selector, crc, warnings)?; if !batch_auto_accepted { validate_batch_crc(resolved.chunk, resolved.crc.as_deref(), requires_checksum)?; @@ -355,8 +354,7 @@ fn apply_delete( } }); 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 batch_auto_accepted = ensure_batch_operation_target_current(scheduled, crc, touched_paths); let resolved = resolve_chunk_with_crc(state, anchor_selector, crc, warnings)?; if !batch_auto_accepted { validate_batch_crc(resolved.chunk, resolved.crc.as_deref(), requires_checksum)?; @@ -390,8 +388,7 @@ fn apply_insert( None } }); - let batch_auto_accepted = - ensure_batch_operation_target_current(scheduled, crc, touched_paths)?; + let batch_auto_accepted = ensure_batch_operation_target_current(scheduled, crc, touched_paths); let resolved = resolve_chunk_with_crc(state, anchor_selector, crc, warnings)?; if !batch_auto_accepted { validate_batch_crc(resolved.chunk, resolved.crc.as_deref(), resolved.crc.is_some())?; @@ -610,29 +607,29 @@ fn touches_chunk_path(touched_paths: &[String], selector: &str) -> bool { }) } -/// Returns `Ok(true)` when the CRC was auto-accepted (chunk was touched by an -/// earlier batch op and the model supplied the pre-batch CRC). The caller should -/// skip CRC validation in that case. +/// Returns `true` when the CRC was auto-accepted (chunk was touched by an +/// earlier batch op and the model supplied the pre-batch CRC). The caller +/// should skip CRC validation in that case. fn ensure_batch_operation_target_current( scheduled: &ScheduledEditOperation, crc: Option<&str>, touched_paths: &[String], -) -> Result { +) -> bool { let Some(selector) = scheduled.requested_selector.as_deref() else { - return Ok(false); + return false; }; let Some(initial_chunk) = scheduled.initial_chunk.as_ref() else { - return Ok(false); + return false; }; let Some(cleaned_crc) = sanitize_crc(crc) else { - return Ok(false); + return false; }; if !touches_chunk_path(touched_paths, selector) || cleaned_crc != initial_chunk.checksum { - return Ok(false); + return false; } // The chunk was touched by an earlier operation in this batch, and the model // supplied the pre-batch CRC (which is all it could know). Auto-accept. - Ok(true) + true } fn describe_scheduled_operation(scheduled: &ScheduledEditOperation) -> String { @@ -652,13 +649,13 @@ fn normalize_inserted_content( ) -> String { let mut normalized = normalize_chunk_source(content); normalized = strip_content_prefixes(&normalized); - if !target_indent.is_empty() { - normalized = reindent_inserted_block(&normalized, target_indent, file_indent_step); - } else { + 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). normalized = normalize_leading_whitespace_char(&normalized, file_indent_char, file_indent_step); + } else { + normalized = reindent_inserted_block(&normalized, target_indent, file_indent_step); } normalized } diff --git a/crates/pi-natives/src/chunk/mod.rs b/crates/pi-natives/src/chunk/mod.rs index 98c0106e9..0fe6a1b4d 100644 --- a/crates/pi-natives/src/chunk/mod.rs +++ b/crates/pi-natives/src/chunk/mod.rs @@ -764,7 +764,11 @@ pub(crate) fn rename_chunk_subtree( mod tests { use std::fmt::Write as _; - use super::{build_chunk_tree, line_to_chunk_path, resolve_chunk_lang}; + use super::{ + build_chunk_tree, line_to_chunk_path, resolve_chunk_lang, + state::ChunkState, + types::{ChunkAnchorStyle, ReadRenderParams}, + }; use crate::language::SupportLang; fn assert_supported_sample(language: &str, source: &str) { @@ -1139,15 +1143,15 @@ function main(): void {{ #[test] fn small_interfaces_collapse() { let source = r"interface Config { - name: string; - getValue(): number; + name: string; + getValue(): number; }"; let tree = build_chunk_tree(source, "typescript").expect("tree should build"); let iface = tree .chunks .iter() - .find(|c| c.path == "iface_Config") - .expect("iface_Config"); + .find(|c| c.path == "interface_Config") + .expect("interface_Config"); assert!(iface.leaf); assert!(iface.children.is_empty()); } @@ -1378,6 +1382,35 @@ impl Config { assert!(reader.children.is_empty(), "single-line interfaces should render inline"); } + #[test] + fn nix_chunk_tree_exposes_attr_bindings() { + let source = r#"{ + hello = "world"; + nested = { + value = 1; + }; + } + "#; + let tree = build_chunk_tree(source, "nix").expect("tree should build"); + let attrset = tree + .chunks + .iter() + .find(|chunk| chunk.path == "attrset_expr") + .expect("attrset_expr chunk"); + assert!(!tree.fallback, "nix should use tree-sitter chunking"); + assert!(!attrset.leaf, "top-level attrset should recurse into bindings"); + assert!( + attrset.children.iter().any(|child| child == "attrset_expr.attr_hello"), + "expected attr_hello child, got {:?}", + attrset.children + ); + assert!( + attrset.children.iter().any(|child| child == "attrset_expr.attr_nested"), + "expected attr_nested child, got {:?}", + attrset.children + ); + } + #[test] fn go_receiver_methods_attach_to_receiver_type() { let source = r"package main @@ -1597,6 +1630,176 @@ func (s *Server) GetAddress() string { assert!(iface.children.is_empty(), "single-line interface methods should render inline"); } + #[test] + fn typescript_interfaces_use_interface_prefix() { + let source = r"interface Settings { + enabled: boolean; +} +"; + let tree = build_chunk_tree(source, "typescript").expect("tree should build"); + assert!( + tree + .chunks + .iter() + .any(|chunk| chunk.path == "interface_Settings"), + "expected interface_Settings in {:?}", + tree + .chunks + .iter() + .map(|chunk| chunk.path.as_str()) + .collect::>() + ); + assert!( + !tree + .chunks + .iter() + .any(|chunk| chunk.path == "iface_Settings"), + "legacy iface_ prefix should not remain addressable" + ); + } + + #[test] + fn read_resolves_partial_selectors_and_bare_checksums() { + let filler = (0..60) + .map(|index| format!(" const value{index} = {index};")) + .collect::>() + .join("\n"); + let source = format!( + "function handleTerraform() {{\n{filler}\n try {{\n if (ready) {{\n \ + work();\n }}\n }} catch (error) {{\n throw error;\n }}\n}}\n" + ); + let state = ChunkState::parse(source, "typescript".to_string()).expect("state should parse"); + let chunk = state + .chunks() + .into_iter() + .find(|candidate| candidate.path == "fn_handleTerraform.try") + .expect("try chunk path should exist"); + let selectors = vec![ + format!("sample.ts:{}", "fn_handleTerraform.try"), + format!("sample.ts:{}", "handleTerraform.try"), + format!("sample.ts:{}", "try"), + format!("sample.ts:try#{}", chunk.checksum), + format!("sample.ts:#{}", chunk.checksum), + format!("sample.ts:{}", chunk.checksum), + ]; + for selector in selectors { + let result = state + .render_read(ReadRenderParams { + read_path: selector.clone(), + display_path: "sample.ts".to_string(), + language_tag: Some("ts".to_string()), + omit_checksum: false, + anchor_style: Some(ChunkAnchorStyle::Full), + absolute_line_range: None, + tab_replacement: Some(" ".to_string()), + }) + .unwrap_or_else(|err| panic!("selector {selector} should resolve: {err}")); + let resolved = result + .chunk + .expect("selector read should resolve a chunk target"); + assert_eq!(resolved.selector, "fn_handleTerraform.try"); + } + } + + #[test] + fn read_lists_chunks_for_question_selector() { + let source = "function run() {\n return 1;\n}\n"; + let state = ChunkState::parse(source.to_string(), "typescript".to_string()) + .expect("state should parse"); + let result = state + .render_read(ReadRenderParams { + read_path: "sample.ts:?".to_string(), + display_path: "sample.ts".to_string(), + language_tag: Some("ts".to_string()), + omit_checksum: false, + anchor_style: Some(ChunkAnchorStyle::Full), + absolute_line_range: None, + tab_replacement: Some(" ".to_string()), + }) + .expect("listing should succeed"); + assert!(result.text.contains("sample.ts chunks:")); + assert!(result.text.contains("fn_run#")); + assert!(!result.text.contains("return 1")); + } + + #[test] + fn read_missing_chunk_returns_error_with_suggestions() { + let filler = (0..60) + .map(|index| format!(" const value{index} = {index};")) + .collect::>() + .join("\n"); + let source = format!( + "function loadSkills() {{\n{filler}\n try {{\n work();\n }} catch (error) \ + {{\n throw error;\n }}\n}}\n\nfunction handleTerraform() {{\n{filler}\n \ + try {{\n work();\n }} catch (error) {{\n throw error;\n }}\n}}\n" + ); + let state = ChunkState::parse(source, "typescript".to_string()).expect("state should parse"); + let error = state + .render_read(ReadRenderParams { + read_path: "sample.ts:fn_loadSkills.try_2".to_string(), + display_path: "sample.ts".to_string(), + language_tag: Some("ts".to_string()), + omit_checksum: false, + anchor_style: Some(ChunkAnchorStyle::Full), + absolute_line_range: None, + tab_replacement: Some(" ".to_string()), + }) + .err() + .expect("missing chunk should surface as an error"); + let message = error.to_string(); + assert!(message.contains("Chunk path not found: \"fn_loadSkills.try_2\""), "{message}"); + assert!(message.contains("Direct children of \"fn_loadSkills\""), "{message}"); + assert!(message.contains("fn_loadSkills.try"), "{message}"); + } + + #[test] + fn go_struct_checksum_ignores_method_body_changes() { + let before = r"package main + +type Server struct { + Addr string +} + +func (s *Server) Start() string { + return s.Addr +} +"; + let after = r#"package main + +type Server struct { + Addr string +} + +func (s *Server) Start() string { + return s.Addr + ":80" +} +"#; + let before_tree = build_chunk_tree(before, "go").expect("before tree should build"); + let after_tree = build_chunk_tree(after, "go").expect("after tree should build"); + let before_struct = before_tree + .chunks + .iter() + .find(|chunk| chunk.path == "type_Server") + .expect("before struct chunk"); + let after_struct = after_tree + .chunks + .iter() + .find(|chunk| chunk.path == "type_Server") + .expect("after struct chunk"); + let before_method = before_tree + .chunks + .iter() + .find(|chunk| chunk.path == "type_Server.fn_Start") + .expect("before method chunk"); + let after_method = after_tree + .chunks + .iter() + .find(|chunk| chunk.path == "type_Server.fn_Start") + .expect("after method chunk"); + assert_eq!(before_struct.checksum, after_struct.checksum); + assert_ne!(before_method.checksum, after_method.checksum); + } + #[test] fn keeps_trivial_typescript_enum_variants_addressable() { let source = r#"enum Status { diff --git a/crates/pi-natives/src/chunk/resolve.rs b/crates/pi-natives/src/chunk/resolve.rs index c4b5e2bb0..1093aabe7 100644 --- a/crates/pi-natives/src/chunk/resolve.rs +++ b/crates/pi-natives/src/chunk/resolve.rs @@ -7,6 +7,7 @@ use crate::chunk::{ const CHUNK_NAME_PREFIXES: &[&str] = &["fn_", "var_", "class_", "stmts_", "type_", "interface_", "enum_", "const_"]; +const CHECKSUM_ALPHABET: &str = "ZPMQVRWSNKTXJBYH"; pub struct ResolvedChunk<'a> { pub chunk: &'a ChunkNode, @@ -40,73 +41,38 @@ 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 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> { - resolve_chunk_selector_in_tree(&state.tree, selector, warnings) -} - -pub fn resolve_chunk_selector_in_tree<'a>( - tree: &'a ChunkTree, - selector: Option<&str>, - warnings: &mut Vec, -) -> Result<&'a ChunkNode, String> { - let Some(cleaned) = sanitize_chunk_selector(selector) else { - return find_chunk_by_path(tree, "") - .ok_or_else(|| "Chunk tree is missing the root chunk".to_owned()); - }; - - if let Some(chunk) = find_chunk_by_path(tree, &cleaned) { - return Ok(chunk); - } - - let suffix = format!(".{cleaned}"); - if let Some(chunk) = resolve_unique_chunks( - collect_matches(tree, |candidate| { - candidate.path == cleaned || candidate.path.ends_with(&suffix) - }), - &cleaned, - warnings, - "chunk selector", - "Auto-resolved chunk selector", - )? { - return Ok(chunk); - } - - if !cleaned.contains('.') { - let prefixed = CHUNK_NAME_PREFIXES - .iter() - .map(|prefix| format!("{prefix}{cleaned}")) - .collect::>(); - if let Some(chunk) = resolve_unique_chunks( - collect_matches(tree, |candidate| { - prefixed - .iter() - .any(|name| candidate.path == *name || candidate.path.ends_with(&format!(".{name}"))) - }), - &cleaned, - warnings, - "chunk selector", - "Auto-resolved chunk selector", - )? { - return Ok(chunk); - } - } - - let kind_segments = cleaned.split('.').collect::>(); - if let Some(chunk) = resolve_unique_chunks( - collect_matches(tree, |candidate| kind_path_matches(candidate, &kind_segments)), - &cleaned, - warnings, - "kind selector", - "Auto-resolved kind selector", - )? { - return Ok(chunk); - } - - Err(build_not_found_error(tree, &cleaned)) + let (cleaned_selector, cleaned_crc) = split_selector_and_crc(selector, None); + resolve_chunk_selector_impl(state, cleaned_selector.as_deref(), cleaned_crc.as_deref(), warnings) } pub fn resolve_chunk_with_crc<'a>( @@ -115,40 +81,30 @@ pub fn resolve_chunk_with_crc<'a>( crc: Option<&str>, warnings: &mut Vec, ) -> Result, String> { - resolve_chunk_with_crc_in_tree(&state.tree, selector, crc, warnings) -} - -pub fn resolve_chunk_with_crc_in_tree<'a>( - tree: &'a ChunkTree, - selector: Option<&str>, - crc: Option<&str>, - warnings: &mut Vec, -) -> Result, String> { - let cleaned_crc = sanitize_crc(crc); - let cleaned_selector = sanitize_chunk_selector(selector); + let (cleaned_selector, cleaned_crc) = split_selector_and_crc(selector, crc); if cleaned_selector.is_none() && let Some(cleaned_crc) = cleaned_crc.clone() { - let chunk = resolve_chunk_by_checksum(tree, &cleaned_crc)?; + let chunk = resolve_chunk_by_checksum(state, &cleaned_crc)?; return Ok(ResolvedChunk { chunk, crc: Some(cleaned_crc) }); } - let chunk = resolve_chunk_selector_in_tree(tree, cleaned_selector.as_deref(), warnings)?; + let chunk = resolve_chunk_selector_impl( + state, + cleaned_selector.as_deref(), + cleaned_crc.as_deref(), + warnings, + )?; Ok(ResolvedChunk { chunk, crc: cleaned_crc }) } pub fn resolve_chunk_by_checksum<'a>( - tree: &'a ChunkTree, + state: &'a ChunkStateInner, crc: &str, ) -> Result<&'a ChunkNode, String> { let cleaned_crc = sanitize_crc(Some(crc)).ok_or_else(|| "Checksum is required".to_owned())?; - let matches = tree - .chunks - .iter() - .filter(|chunk| chunk.checksum == cleaned_crc) - .collect::>(); - + let matches = state.chunks_by_checksum(&cleaned_crc); match matches.len() { 0 => Err(format!( "Checksum \"{cleaned_crc}\" did not match any chunk. Re-read the file to get current \ @@ -171,23 +127,180 @@ pub fn resolve_chunk_by_checksum<'a>( } } -fn find_chunk_by_path<'a>(tree: &'a ChunkTree, path: &str) -> Option<&'a ChunkNode> { - tree.chunks.iter().find(|chunk| chunk.path == path) +fn resolve_chunk_selector_impl<'a>( + state: &'a ChunkStateInner, + selector: Option<&str>, + crc: Option<&str>, + warnings: &mut Vec, +) -> Result<&'a ChunkNode, String> { + let Some(cleaned) = selector else { + return state + .chunk("") + .ok_or_else(|| "Chunk tree is missing the root chunk".to_owned()); + }; + + if let Some(chunk) = state.chunk(cleaned) { + return match_crc_filter(cleaned, vec![chunk], crc); + } + + if is_checksum_token(cleaned) { + let matches = state.chunks_by_checksum(cleaned); + if !matches.is_empty() { + return resolve_matches( + matches, + cleaned, + crc, + warnings, + "checksum selector", + "Auto-resolved checksum selector", + ); + } + } + + let suffix_matches = state.chunks_by_suffix(cleaned); + if !suffix_matches.is_empty() { + return resolve_matches( + suffix_matches, + cleaned, + crc, + warnings, + "chunk selector", + "Auto-resolved chunk selector", + ); + } + + if !cleaned.contains('.') { + let prefixed = CHUNK_NAME_PREFIXES + .iter() + .map(|prefix| format!("{prefix}{cleaned}")) + .collect::>(); + let prefixed_matches = + collect_unique_matches(prefixed.iter().flat_map(|name| state.chunks_by_leaf(name))); + if !prefixed_matches.is_empty() { + return resolve_matches( + prefixed_matches, + cleaned, + crc, + warnings, + "chunk selector", + "Auto-resolved chunk selector", + ); + } + } + + let kind_segments = cleaned.split('.').collect::>(); + let kind_candidates = collect_unique_matches( + state + .chunks_by_leaf(kind_segments.last().copied().unwrap_or(cleaned)) + .into_iter() + .filter(|candidate| kind_path_matches(candidate, &kind_segments)), + ); + if !kind_candidates.is_empty() { + return resolve_matches( + kind_candidates, + cleaned, + crc, + warnings, + "kind selector", + "Auto-resolved kind selector", + ); + } + + Err(build_not_found_error(state.tree(), cleaned)) } -fn collect_matches(tree: &ChunkTree, mut predicate: F) -> Vec<&ChunkNode> -where - F: FnMut(&ChunkNode) -> bool, -{ +fn match_crc_filter<'a>( + cleaned: &str, + matches: Vec<&'a ChunkNode>, + crc: Option<&str>, +) -> Result<&'a ChunkNode, String> { + let Some(cleaned_crc) = crc else { + return Ok(matches[0]); + }; + let filtered = filter_by_crc(matches, cleaned_crc); + match filtered.len() { + 1 => Ok(filtered[0]), + 0 => Err(format!( + "Chunk selector \"{cleaned}\" did not match checksum \"{cleaned_crc}\". Re-read the file \ + to get current checksums." + )), + _ => Err(format!( + "Ambiguous chunk selector \"{cleaned}\" with checksum \"{cleaned_crc}\" matches {} \ + chunks: {}. Use the full path from read output.", + filtered.len(), + filtered + .iter() + .map(|chunk| chunk.path.as_str()) + .collect::>() + .join(", "), + )), + } +} + +fn resolve_matches<'a>( + matches: Vec<&'a ChunkNode>, + cleaned: &str, + crc: Option<&str>, + warnings: &mut Vec, + selector_label: &str, + warning_label: &str, +) -> Result<&'a ChunkNode, String> { + let matches = if let Some(cleaned_crc) = crc { + let filtered = filter_by_crc(matches, cleaned_crc); + if filtered.is_empty() { + return Err(format!( + "{selector_label} \"{cleaned}\" did not match checksum \"{cleaned_crc}\". Re-read the \ + file to get current checksums." + )); + } + filtered + } else { + matches + }; + let outermost = retain_outermost_matches(matches); + resolve_unique_chunks(outermost, cleaned, warnings, selector_label, warning_label)?.ok_or_else( + || { + format!( + "{selector_label} \"{cleaned}\" did not match any chunk. Re-read the file to see \ + available chunk paths." + ) + }, + ) +} + +fn filter_by_crc<'a>(matches: Vec<&'a ChunkNode>, crc: &str) -> Vec<&'a ChunkNode> { + matches + .into_iter() + .filter(|chunk| chunk.checksum == crc) + .collect() +} + +fn collect_unique_matches<'a>( + matches: impl IntoIterator, +) -> Vec<&'a ChunkNode> { let mut seen = BTreeSet::new(); - let mut matches = Vec::new(); - for chunk in &tree.chunks { - if chunk.path.is_empty() || !predicate(chunk) || !seen.insert(chunk.path.as_str()) { + let mut out = Vec::new(); + for chunk in matches { + if chunk.path.is_empty() || !seen.insert(chunk.path.as_str()) { continue; } - matches.push(chunk); + out.push(chunk); } + out +} + +fn retain_outermost_matches(matches: Vec<&ChunkNode>) -> Vec<&ChunkNode> { + let Some(min_depth) = matches + .iter() + .map(|chunk| chunk.path.split('.').count()) + .min() + else { + return matches; + }; matches + .into_iter() + .filter(|chunk| chunk.path.split('.').count() == min_depth) + .collect() } fn resolve_unique_chunks<'a>( @@ -228,7 +341,17 @@ fn kind_path_matches(candidate: &ChunkNode, kind_segments: &[&str]) -> bool { && kind_segments .iter() .zip(path_segments) - .all(|(kind, segment)| segment == *kind || segment.starts_with(&format!("{kind}_"))) + .all(|(kind, segment)| { + segment == *kind + || segment.starts_with(&format!("{kind}_")) + || strip_known_chunk_prefix(segment) == Some(*kind) + }) +} + +fn strip_known_chunk_prefix(segment: &str) -> Option<&str> { + CHUNK_NAME_PREFIXES + .iter() + .find_map(|prefix| segment.strip_prefix(prefix)) } fn build_not_found_error(tree: &ChunkTree, cleaned: &str) -> String { @@ -351,13 +474,40 @@ fn strip_trailing_checksum(value: &str) -> &str { let Some((prefix, suffix)) = value.rsplit_once('#') else { return value; }; - if suffix.len() == 4 && suffix.chars().all(|ch| ch.is_ascii_hexdigit()) { + 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 + .chars() + .all(|ch| CHECKSUM_ALPHABET.contains(ch.to_ascii_uppercase())) +} + +fn find_chunk_by_path<'a>(tree: &'a ChunkTree, path: &str) -> Option<&'a ChunkNode> { + tree.chunks.iter().find(|chunk| chunk.path == path) +} + /// Find the `:` separating a file path from a chunk selector in /// `file.ts:chunk_path`. Skips Windows `C:\` / `C:/` drive prefixes. fn chunk_read_path_separator_index(value: &str) -> Option { diff --git a/crates/pi-natives/src/chunk/state.rs b/crates/pi-natives/src/chunk/state.rs index d86bdf24e..867267770 100644 --- a/crates/pi-natives/src/chunk/state.rs +++ b/crates/pi-natives/src/chunk/state.rs @@ -7,19 +7,19 @@ use napi::{Error, Result}; use napi_derive::napi; use regex::Regex; -use super::build_chunk_tree; +use super::{ + build_chunk_tree, + resolve::{resolve_chunk_selector, resolve_chunk_with_crc, split_selector_and_crc}, +}; use crate::chunk::types::{ ChunkInfo, ChunkNode, ChunkReadStatus, ChunkReadTarget, ChunkTree, EditParams, EditResult, ReadRenderParams, ReadResult, RenderParams, VisibleLineRange, }; -const CHECKSUM_SUFFIX_RE: &str = r"^(.*?)(?:\s+)?#([0-9A-Fa-f]{4})$"; const LINE_RANGE_SELECTOR_RE: &str = r"^L(\d+)(?:-L?(\d+))?$"; const TLAPLUS_BEGIN_TRANSLATION_RE: &str = r"^\s*\\\*\s*BEGIN TRANSLATION\s*$"; const TLAPLUS_END_TRANSLATION_RE: &str = r"^\s*\\\*\s*END TRANSLATION\s*$"; -static CHECKSUM_SUFFIX_REGEX: LazyLock = - LazyLock::new(|| Regex::new(CHECKSUM_SUFFIX_RE).expect("checksum selector regex must compile")); static LINE_RANGE_SELECTOR_REGEX: LazyLock = LazyLock::new(|| Regex::new(LINE_RANGE_SELECTOR_RE).expect("line range regex must compile")); static TLAPLUS_BEGIN_TRANSLATION_REGEX: LazyLock = LazyLock::new(|| { @@ -35,6 +35,9 @@ pub struct ChunkStateInner { pub(crate) language: String, pub(crate) tree: ChunkTree, lookup: HashMap, + checksum_lookup: HashMap>, + leaf_lookup: HashMap>, + suffix_lookup: HashMap>, } impl ChunkStateInner { @@ -45,13 +48,34 @@ impl ChunkStateInner { } pub(crate) fn new(source: String, language: String, tree: ChunkTree) -> Self { - let lookup = tree - .chunks - .iter() - .enumerate() - .map(|(index, chunk)| (chunk.path.clone(), index)) - .collect(); - Self { source, language, tree, lookup } + let mut lookup = HashMap::new(); + let mut checksum_lookup = HashMap::new(); + let mut leaf_lookup = HashMap::new(); + let mut suffix_lookup = HashMap::new(); + for (index, chunk) in tree.chunks.iter().enumerate() { + lookup.insert(chunk.path.clone(), index); + checksum_lookup + .entry(chunk.checksum.clone()) + .or_insert_with(Vec::new) + .push(index); + if chunk.path.is_empty() { + continue; + } + if let Some(leaf) = chunk.path.rsplit('.').next() { + leaf_lookup + .entry(leaf.to_string()) + .or_insert_with(Vec::new) + .push(index); + } + let segments = chunk.path.split('.').collect::>(); + for start in 1..segments.len() { + suffix_lookup + .entry(segments[start..].join(".")) + .or_insert_with(Vec::new) + .push(index); + } + } + Self { source, language, tree, lookup, checksum_lookup, leaf_lookup, suffix_lookup } } pub(crate) const fn source(&self) -> &str { @@ -77,8 +101,38 @@ impl ChunkStateInner { .and_then(|index| self.tree.chunks.get(*index)) } - pub(crate) fn chunk_info(&self, path: &str) -> Option { - self.chunk(path).map(chunk_info) + pub(crate) fn chunk_by_index(&self, index: usize) -> Option<&ChunkNode> { + self.tree.chunks.get(index) + } + + pub(crate) fn chunks_by_checksum(&self, checksum: &str) -> Vec<&ChunkNode> { + self + .checksum_lookup + .get(checksum) + .into_iter() + .flatten() + .filter_map(|index| self.chunk_by_index(*index)) + .collect() + } + + pub(crate) fn chunks_by_leaf(&self, leaf: &str) -> Vec<&ChunkNode> { + self + .leaf_lookup + .get(leaf) + .into_iter() + .flatten() + .filter_map(|index| self.chunk_by_index(*index)) + .collect() + } + + pub(crate) fn chunks_by_suffix(&self, suffix: &str) -> Vec<&ChunkNode> { + self + .suffix_lookup + .get(suffix) + .into_iter() + .flatten() + .filter_map(|index| self.chunk_by_index(*index)) + .collect() } pub(crate) fn chunks(&self) -> impl Iterator { @@ -193,7 +247,10 @@ impl ChunkState { /// Look up [`ChunkInfo`] for a chunk selector path. #[napi(js_name = "chunk")] pub fn chunk_info_for_path(&self, chunk_path: String) -> Option { - self.inner.chunk_info(chunk_path.as_str()) + let mut warnings = Vec::new(); + resolve_chunk_selector(self.inner(), Some(chunk_path.as_str()), &mut warnings) + .ok() + .map(chunk_info) } /// Every chunk node as a [`ChunkInfo`] list. @@ -206,16 +263,19 @@ impl ChunkState { /// the path is missing. #[napi] pub fn children(&self, chunk_path: Option) -> Result> { - let parent_path = chunk_path.unwrap_or_default(); - if self.inner.chunk(parent_path.as_str()).is_none() { - return Err(Error::from_reason(format!( - "Chunk path not found: \"{parent_path}\". Re-read the file to see the full chunk tree \ - with paths and checksums." - ))); - } + let parent = if let Some(chunk_path) = chunk_path { + let mut warnings = Vec::new(); + resolve_chunk_selector(self.inner(), Some(chunk_path.as_str()), &mut warnings) + .map_err(Error::from_reason)? + } else { + self + .inner + .root() + .ok_or_else(|| Error::from_reason("Chunk tree is missing the root chunk".to_string()))? + }; Ok(self .inner - .child_chunks(parent_path.as_str()) + .child_chunks(parent.path.as_str()) .into_iter() .map(chunk_info) .collect()) @@ -237,7 +297,7 @@ impl ChunkState { /// errors. #[napi(js_name = "renderRead")] pub fn render_read(&self, params: ReadRenderParams) -> Result { - let ParsedChunkReadPath { selector } = parse_chunk_read_path(params.read_path.as_str()); + let ParsedChunkReadPath { selector, crc } = parse_chunk_read_path(params.read_path.as_str()); let visible_range = selector.as_deref().and_then(parse_visible_line_range); let Some(root) = self.inner.root() else { return Ok(ReadResult { @@ -258,29 +318,27 @@ impl ChunkState { }; return Ok(ReadResult { text: format!( - "Line {} is beyond end of file ({} lines total). {}", + "Line {} is beyond end of file ({} lines total). {suggestion}", visible_range.start_line, self.inner.tree().line_count, - suggestion, ), chunk: None, }); } - let clamped_range = VisibleLineRange { - start_line: visible_range.start_line, - end_line: visible_range.end_line.min(self.inner.tree().line_count), + let notice = if visible_range.start_line == visible_range.end_line { + format!("{}:L{}", params.display_path, visible_range.start_line) + } else { + format!( + "{}:L{}-L{}", + params.display_path, visible_range.start_line, visible_range.end_line + ) }; - let notice = format!( - "[Notice: chunk view scoped to requested lines L{}-L{}; non-overlapping lines \ - omitted.]", - clamped_range.start_line, clamped_range.end_line - ); let text = self.render(RenderParams { chunk_path: Some(root.path.clone()), title: params.display_path.clone(), language_tag: params.language_tag.clone(), - visible_range: Some(clamped_range), + visible_range: Some(visible_range), render_children_only: true, omit_checksum: params.omit_checksum, anchor_style: params.anchor_style, @@ -290,7 +348,7 @@ impl ChunkState { return Ok(ReadResult { text: format!("{notice}\n\n{text}"), chunk: None }); } - if selector.as_deref().is_none_or(str::is_empty) { + if selector.as_deref().is_none_or(str::is_empty) && crc.is_none() { return Ok(ReadResult { text: self.render(RenderParams { chunk_path: Some(root.path.clone()), @@ -307,13 +365,22 @@ impl ChunkState { }); } - let selector = selector.unwrap_or_default(); - let Some(chunk) = self.inner.chunk(selector.as_str()) else { - return Ok(ReadResult { - text: format!("{}:{}\n\n[Chunk not found]", params.display_path, selector), - chunk: Some(ChunkReadTarget { status: ChunkReadStatus::NotFound, selector }), - }); - }; + 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()) { + lines.push(format!( + " {}#{} L{}-L{}", + chunk.path, chunk.checksum, chunk.start_line, chunk.end_line + )); + } + return Ok(ReadResult { text: lines.join("\n"), chunk: None }); + } + + let mut warnings = Vec::new(); + let resolved = + resolve_chunk_with_crc(self.inner(), selector.as_deref(), crc.as_deref(), &mut warnings) + .map_err(Error::from_reason)?; + let chunk = resolved.chunk; if let Some(absolute_line_range) = params.absolute_line_range { let req_start = absolute_line_range.start_line; @@ -328,9 +395,8 @@ impl ChunkState { }; return Ok(ReadResult { text: format!( - "Requested lines {requested} do not overlap chunk \"{}\" (file lines {}-{}). \ - Use sel=L{}-L{} to read this chunk.", - chunk.path, chunk.start_line, chunk.end_line, chunk.start_line, chunk.end_line + "Requested range {requested} does not overlap {}:{} (lines {}-{}).", + params.display_path, chunk.path, chunk.start_line, chunk.end_line ), chunk: Some(ChunkReadTarget { status: ChunkReadStatus::Ok, @@ -338,23 +404,6 @@ impl ChunkState { }), }); } - return Ok(ReadResult { - text: self.render(RenderParams { - chunk_path: Some(chunk.path.clone()), - title: format!("{}:{}", params.display_path, chunk.path), - language_tag: params.language_tag.clone(), - visible_range: Some(VisibleLineRange { start_line: low, end_line: high }), - render_children_only: false, - omit_checksum: params.omit_checksum, - anchor_style: params.anchor_style, - show_leaf_preview: true, - tab_replacement: params.tab_replacement, - }), - chunk: Some(ChunkReadTarget { - status: ChunkReadStatus::Ok, - selector: chunk.path.clone(), - }), - }); } Ok(ReadResult { @@ -405,6 +454,7 @@ impl ChunkState { #[derive(Clone)] struct ParsedChunkReadPath { selector: Option, + crc: Option, } fn normalize_language(language: &str) -> String { @@ -422,23 +472,6 @@ fn chunk_info(chunk: &ChunkNode) -> ChunkInfo { } } -fn sanitize_chunk_selector(selector: Option<&str>) -> Option { - let mut selector = selector?.trim().to_string(); - if selector.is_empty() || selector == "null" || selector == "undefined" { - return None; - } - if let Some(colon_index) = chunk_read_path_separator_index(selector.as_str()) { - selector = selector[(colon_index + 1)..].to_string(); - } - if let Some(captures) = CHECKSUM_SUFFIX_REGEX.captures(selector.as_str()) - && let Some(prefix) = captures.get(1) - { - selector = prefix.as_str().to_string(); - } - let selector = selector.trim().to_string(); - (!selector.is_empty()).then_some(selector) -} - fn chunk_read_path_separator_index(read_path: &str) -> Option { if read_path.len() >= 3 { let bytes = read_path.as_bytes(); @@ -450,9 +483,10 @@ fn chunk_read_path_separator_index(read_path: &str) -> Option { } fn parse_chunk_read_path(read_path: &str) -> ParsedChunkReadPath { - let selector = chunk_read_path_separator_index(read_path) - .and_then(|index| sanitize_chunk_selector(Some(&read_path[(index + 1)..]))); - ParsedChunkReadPath { selector } + 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_visible_line_range(selector: &str) -> Option { diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 90f05386d..ccb64e99c 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## [Unreleased] + ### Added - Added `anchorStyle` parameter to chunk read formatting to control chunk path display format (full, kind, or bare) @@ -21,6 +22,9 @@ ### Changed +- Updated chunk edit prompt documentation to use ellipsis (…) instead of ellipsis (...) for consistency in operation examples +- Modified chunk path parsing to preserve raw selector strings and extract CRC separately, enabling accurate chunk reference round-tripping in read/edit workflows +- Changed chunk edit behavior to auto-accept stale CRC checksums for subsequent operations on the same chunk within a batch, improving usability when applying multiple edits to the same target - Moved `copyToClipboard` and `readImageFromClipboard` functions to new `utils/clipboard.ts` module with improved OSC 52 support and Termux compatibility - Updated grep output mode to use `GrepOutputMode` enum from pi-natives instead of string literals - Changed macOS appearance observer to use `MacAppearanceObserver.start()` class method with error-first callback signature diff --git a/packages/coding-agent/src/prompts/tools/chunk-edit.md b/packages/coding-agent/src/prompts/tools/chunk-edit.md index 995509ee5..5c7ca275c 100644 --- a/packages/coding-agent/src/prompts/tools/chunk-edit.md +++ b/packages/coding-agent/src/prompts/tools/chunk-edit.md @@ -8,7 +8,7 @@ Successful edit responses include the updated chunk tree with checksums. Do not **Choosing the right edit shape:** -- To rewrite an entire chunk → `{ "target": "chunk#CRC", "content": "..." }` +- To rewrite an entire chunk → `{ "target": "chunk#CRC", "content": "…" }` - To fix a single line → add `"line": 13` - To fix a contiguous range → add `"line": 13, "end_line": 17` - To delete a chunk → `{ "target": "chunk#CRC", "delete": true }` @@ -17,15 +17,14 @@ Successful edit responses include the updated chunk tree with checksums. Do not |shape|effect| |---|---| -|`{ "target": "chunk#CRC", "content": "..." }`|replace the target chunk| -|`{ "target": "chunk#CRC", "line": 13, "content": "..." }`|replace one line within the target chunk| -|`{ "target": "chunk#CRC", "line": 13, "end_line": 17, "content": "..." }`|replace an inclusive line range within the target chunk| +|`{ "target": "chunk#CRC", "content": "…" }`|replace the target chunk| +|`{ "target": "chunk#CRC", "line": 13, "content": "…" }`|replace one line within the target chunk| +|`{ "target": "chunk#CRC", "line": 13, "end_line": 17, "content": "…" }`|replace an inclusive line range within the target chunk| |`{ "target": "chunk#CRC", "delete": true }`|delete the target chunk| -|`{ "target": "chunk", "append": true, "content": "..." }`|append as last child of the target chunk| -|`{ "target": "chunk", "prepend": true, "content": "..." }`|prepend as first child of the target chunk| -|`{ "target": "parent", "after": "child", "content": "..." }`|insert after the named child within `parent`| -|`{ "target": "parent", "before": "child", "content": "..." }`|insert before the named child within `parent`| - +|`{ "target": "chunk", "append": true, "content": "…" }`|append as last child of the target chunk| +|`{ "target": "chunk", "prepend": true, "content": "…" }`|prepend as first child of the target chunk| +|`{ "target": "parent", "after": "child", "content": "…" }`|insert after the named child within `parent`| +|`{ "target": "parent", "before": "child", "content": "…" }`|insert before the named child within `parent`| - `line`/`end_line` are **absolute file line numbers** from the `read` gutter. `line` alone = single line. `line` + `end_line` = inclusive range. `line` with `end_line` = `line`-1 = zero-width insert. - `path` is always just the file path. Do not embed `:chunk` selectors in `path`. - `target` is the chunk path, optionally followed by `#CRC` copied from the anchor. Example: `"class_Server.fn_start#HTST"`. @@ -130,4 +129,4 @@ All examples reference this `read` output: - For line-scoped replace edits, use file line numbers from the `read` gutter. - Multiple line-scoped edits on the same chunk in one batch are fine — the engine auto-updates the checksum between operations. - \ No newline at end of file + diff --git a/packages/coding-agent/src/tools/chunk-tree.ts b/packages/coding-agent/src/tools/chunk-tree.ts index 143ddd475..50e0659c0 100644 --- a/packages/coding-agent/src/tools/chunk-tree.ts +++ b/packages/coding-agent/src/tools/chunk-tree.ts @@ -62,6 +62,7 @@ export type ChunkEditResult = { export type ParsedChunkReadPath = { filePath: string; selector?: string; + crc?: string; }; type ChunkCacheEntry = { @@ -128,9 +129,12 @@ export function parseChunkReadPath(readPath: string): ParsedChunkReadPath { if (colonIndex === -1) { return { filePath: readPath }; } + const rawSelector = readPath.slice(colonIndex + 1) || undefined; + const parsedSelector = parseChunkSelector(rawSelector); return { filePath: readPath.slice(0, colonIndex), - selector: parseChunkSelector(readPath.slice(colonIndex + 1) || undefined).selector, + selector: rawSelector, + crc: parsedSelector.crc, }; } diff --git a/packages/coding-agent/src/tools/read.ts b/packages/coding-agent/src/tools/read.ts index 94b04b888..d0033d971 100644 --- a/packages/coding-agent/src/tools/read.ts +++ b/packages/coding-agent/src/tools/read.ts @@ -771,6 +771,7 @@ export class ReadTool implements AgentTool { const pathSelectorParsed = chunkMode ? parseSel(parsedReadPath.selector) : { kind: "none" as const }; const pathChunkSelector = pathSelectorParsed.kind === "chunk" ? pathSelectorParsed.selector : undefined; const selectorInput = sel ?? parsedReadPath.selector; + const rawSelectorInput = sel ?? parsedReadPath.selector; const parsed = parseSel(selectorInput); const archivePath = await this.#resolveArchiveReadPath(localReadPath, signal); @@ -837,10 +838,11 @@ export class ReadTool implements AgentTool { : undefined; // sel= wins over path:chunk when both are provided (explicit param > embedded path). const effectiveSelector = sel ? selectorInput : (pathChunkSelector ?? selectorInput); + const rawEffectiveSelector = sel ? selectorInput : (rawSelectorInput ?? effectiveSelector); const chunkReadPath = parsed.kind === "chunk" || (pathChunkSelector && !sel) - ? effectiveSelector - ? `${localReadPath}:${effectiveSelector}` + ? rawEffectiveSelector + ? `${localReadPath}:${rawEffectiveSelector}` : localReadPath : parsed.kind === "lines" ? parsed.endLine !== undefined diff --git a/packages/coding-agent/test/core/chunk-tree.test.ts b/packages/coding-agent/test/core/chunk-tree.test.ts index 6a0957803..34a8898e1 100644 --- a/packages/coding-agent/test/core/chunk-tree.test.ts +++ b/packages/coding-agent/test/core/chunk-tree.test.ts @@ -465,24 +465,24 @@ describe("edit safety invariants", () => { }); } - test("rejects a second same-path replace in one batch when the checksum stays stale", () => { + test("auto-accepts stale CRC for a second same-path replace in one batch", () => { const checksum = getChecksum(testSource, runChunkPath); - expect(() => - edit([ - { - op: "replace", - sel: runChunkPath, - crc: checksum, - content: '\trun(): void {\n\t\tconsole.log("first");\n\t}', - }, - { - op: "replace", - sel: runChunkPath, - crc: checksum, - content: '\trun(): void {\n\t\tconsole.log("second");\n\t}', - }, - ]), - ).toThrow(/changed by an earlier batch operation/); + const result = edit([ + { + op: "replace", + sel: runChunkPath, + crc: checksum, + content: '\trun(): void {\n\t\tconsole.log("first");\n\t}', + }, + { + op: "replace", + sel: runChunkPath, + crc: checksum, + content: '\trun(): void {\n\t\tconsole.log("second");\n\t}', + }, + ]); + expect(result.diffSourceAfter).toContain('console.log("second")'); + expect(result.diffSourceAfter).not.toContain('console.log("first")'); }); test("applies two same-path replaces in one batch when the second checksum matches the post-first state", () => { @@ -506,28 +506,28 @@ describe("edit safety invariants", () => { expect(result.diffSourceAfter).not.toContain('console.log("first")'); }); - test("rejects a second same-path splice in one batch when the checksum stays stale", () => { + test("auto-accepts stale CRC for a second same-path splice in one batch", () => { const checksum = getChecksum(testSource, runChunkPath); - expect(() => - edit([ - { - op: "replace", - sel: runChunkPath, - crc: checksum, - line: 6, - endLine: 6, - content: '\trun(task = "default"): void {', - }, - { - op: "replace", - sel: runChunkPath, - crc: checksum, - line: 7, - endLine: 7, - content: "\t\tconsole.log(task);", - }, - ]), - ).toThrow(/changed by an earlier batch operation/); + const result = edit([ + { + op: "replace", + sel: runChunkPath, + crc: checksum, + line: 6, + endLine: 6, + content: '\trun(task = "default"): void {', + }, + { + op: "replace", + sel: runChunkPath, + crc: checksum, + line: 7, + endLine: 7, + content: "\t\tconsole.log(task);", + }, + ]); + expect(result.diffSourceAfter).toContain('run(task = "default")'); + expect(result.diffSourceAfter).toContain("console.log(task)"); }); test("applies two same-path splices in one batch when the second checksum matches the post-first state", () => { diff --git a/packages/coding-agent/test/tools/chunk-mode.test.ts b/packages/coding-agent/test/tools/chunk-mode.test.ts index 5011d0c4a..979d56141 100644 --- a/packages/coding-agent/test/tools/chunk-mode.test.ts +++ b/packages/coding-agent/test/tools/chunk-mode.test.ts @@ -447,7 +447,7 @@ describe("chunk mode tools", () => { expect(await Bun.file(filePath).text()).toBe(originalSource); }); - it("reports stale mixed batches against the listed splice operation", async () => { + it("auto-accepts stale CRC in mixed batches on the same chunk", async () => { const filePath = path.join(tmpDir, "server.ts"); const originalSource = buildLargeTypescriptFixture(); await Bun.write(filePath, originalSource); @@ -455,27 +455,24 @@ describe("chunk mode tools", () => { const editTool = new EditTool(session); const checksum = getChunkChecksum(originalSource, "typescript", "class_Server.fn_handleError"); - await expect( - editTool.execute("chunk-edit-stale-mixed-batch", { - path: filePath, - edits: [ - { - target: `class_Server.fn_handleError#${checksum}`, - content: " private handleError(err: Error): string {\n return err.message;\n }", - }, - { - target: `class_Server.fn_handleError#${checksum}`, - line: 63, - end_line: 63, - content: " return err.message.toUpperCase();", - }, - ], - }), - ).rejects.toThrow( - /Edit operation 2\/2 failed \(replace on "class_Server\.fn_handleError"\): Chunk "class_Server\.fn_handleError" was changed by an earlier batch operation/, - ); + const _result = await editTool.execute("chunk-edit-stale-mixed-batch", { + path: filePath, + edits: [ + { + target: `class_Server.fn_handleError#${checksum}`, + content: " private handleError(err: Error): string {\n return err.message;\n }", + }, + { + target: `class_Server.fn_handleError#${checksum}`, + line: 3, + end_line: 3, + content: " return err.message.toUpperCase();", + }, + ], + }); - expect(await Bun.file(filePath).text()).toBe(originalSource); + const updatedSource = await Bun.file(filePath).text(); + expect(updatedSource).toContain("toUpperCase"); }); it("rejects missing CRC", async () => {