From a30ee4a1110f39c4adc691c2e4c1df3bb465d7f7 Mon Sep 17 00:00:00 2001 From: can1357 Date: Tue, 7 Apr 2026 04:20:31 +0200 Subject: [PATCH] refactor(chunk): restructured path parsing and edit operations for cleaner CRC handling - Simplified chunk path parsing to extract selector separately from CRC field. - Refactored chunk edit operations to conditionally strip CRC for auto-accepted batch operations. - Enhanced chunk edit preview formatting to display operation types with target and line ranges. - Improved chunk state rendering with clamped visible range and unified notice message format. - Refactored error handling in chunk resolution from map_err to let-else pattern. --- crates/pi-natives/src/chunk/edit.rs | 11 ++- crates/pi-natives/src/chunk/resolve.rs | 7 +- crates/pi-natives/src/chunk/state.rs | 27 ++++--- packages/coding-agent/CHANGELOG.md | 4 +- packages/coding-agent/src/patch/shared.ts | 71 +++++++++++++++---- packages/coding-agent/src/tools/chunk-tree.ts | 5 +- 6 files changed, 86 insertions(+), 39 deletions(-) diff --git a/crates/pi-natives/src/chunk/edit.rs b/crates/pi-natives/src/chunk/edit.rs index 6dc62b06b..b8e4306b7 100644 --- a/crates/pi-natives/src/chunk/edit.rs +++ b/crates/pi-natives/src/chunk/edit.rs @@ -262,7 +262,10 @@ 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 resolved = resolve_chunk_with_crc(state, anchor_selector, crc, warnings)?; + // 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)?; } @@ -355,7 +358,8 @@ 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 resolved = resolve_chunk_with_crc(state, anchor_selector, crc, warnings)?; + 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)?; } @@ -389,7 +393,8 @@ fn apply_insert( } }); let batch_auto_accepted = ensure_batch_operation_target_current(scheduled, crc, touched_paths); - let resolved = resolve_chunk_with_crc(state, anchor_selector, crc, warnings)?; + 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())?; } diff --git a/crates/pi-natives/src/chunk/resolve.rs b/crates/pi-natives/src/chunk/resolve.rs index 1093aabe7..a6dce75e4 100644 --- a/crates/pi-natives/src/chunk/resolve.rs +++ b/crates/pi-natives/src/chunk/resolve.rs @@ -90,12 +90,7 @@ pub fn resolve_chunk_with_crc<'a>( return Ok(ResolvedChunk { chunk, crc: Some(cleaned_crc) }); } - let chunk = resolve_chunk_selector_impl( - state, - cleaned_selector.as_deref(), - cleaned_crc.as_deref(), - warnings, - )?; + let chunk = resolve_chunk_selector_impl(state, cleaned_selector.as_deref(), None, warnings)?; Ok(ResolvedChunk { chunk, crc: cleaned_crc }) } diff --git a/crates/pi-natives/src/chunk/state.rs b/crates/pi-natives/src/chunk/state.rs index 867267770..8db1ef7b2 100644 --- a/crates/pi-natives/src/chunk/state.rs +++ b/crates/pi-natives/src/chunk/state.rs @@ -326,19 +326,20 @@ impl ChunkState { }); } - 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 clamped_range = VisibleLineRange { + start_line: visible_range.start_line, + end_line: visible_range.end_line.min(self.inner.tree().line_count), }; + 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(visible_range), + visible_range: Some(clamped_range), render_children_only: true, omit_checksum: params.omit_checksum, anchor_style: params.anchor_style, @@ -377,9 +378,15 @@ impl ChunkState { } let mut warnings = Vec::new(); - let resolved = + let Ok(resolved) = resolve_chunk_with_crc(self.inner(), selector.as_deref(), crc.as_deref(), &mut warnings) - .map_err(Error::from_reason)?; + else { + let sel = selector.unwrap_or_default(); + return Ok(ReadResult { + text: format!("{}:{}\n\n[Chunk not found]", params.display_path, sel), + chunk: Some(ChunkReadTarget { status: ChunkReadStatus::NotFound, selector: sel }), + }); + }; let chunk = resolved.chunk; if let Some(absolute_line_range) = params.absolute_line_range { diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 411c4ed98..7ed60a5f4 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,7 +1,6 @@ # Changelog ## [Unreleased] - ### Added - Added `anchorStyle` parameter to chunk read formatting to control chunk path display format (full, kind, or bare) @@ -22,6 +21,9 @@ ### Changed +- Updated streaming edit preview to display chunk edits alongside hashline edits with operation-specific formatting +- Improved chunk edit preview labels to show operation type (delete, append, prepend, insert, replace) with target and line ranges +- Simplified chunk read path parsing to extract only the selector string without redundant CRC field - Agent sessions now hold a macOS power assertion for their lifetime so the host stays awake during active coding-agent runs - 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 diff --git a/packages/coding-agent/src/patch/shared.ts b/packages/coding-agent/src/patch/shared.ts index 1030c8117..29653a261 100644 --- a/packages/coding-agent/src/patch/shared.ts +++ b/packages/coding-agent/src/patch/shared.ts @@ -22,7 +22,7 @@ import { truncateDiffByHunk, } from "../tools/render-utils"; import { Hasher, type RenderCache, renderStatusLine, truncateToWidth } from "../tui"; -import type { HashlineToolEdit } from "./index"; +import type { ChunkToolEdit, HashlineToolEdit } from "./index"; import type { DiffError, DiffResult, Operation } from "./types"; // ═══════════════════════════════════════════════════════════════════════════ @@ -83,8 +83,8 @@ interface EditRenderArgs { * Computed preview diff (used when tool args don't include a diff, e.g. hashline mode). */ previewDiff?: string; - // Hashline mode fields - edits?: Partial[]; + // Hashline / chunk mode fields + edits?: Partial[]; } /** Extended context for edit tool rendering */ @@ -117,18 +117,24 @@ function formatStreamingDiff(diff: string, rawPath: string, uiTheme: Theme, labe return text; } -function formatStreamingHashlineEdits(edits: Partial[], uiTheme: Theme): string { +function formatStreamingHashlineEdits(edits: Partial[], uiTheme: Theme): string { const MAX_EDITS = 4; const MAX_DST_LINES = 8; let text = "\n\n"; - text += uiTheme.fg("dim", `[${edits.length} hashline edit${edits.length === 1 ? "" : "s"}]`); + + // Detect whether these are chunk edits (target field) or hashline edits (loc field) + const isChunk = edits.length > 0 && "target" in edits[0]; + const label = isChunk ? "chunk edit" : "hashline edit"; + text += uiTheme.fg("dim", `[${edits.length} ${label}${edits.length === 1 ? "" : "s"}]`); text += "\n"; let shownEdits = 0; let shownDstLines = 0; for (const edit of edits) { shownEdits++; if (shownEdits > MAX_EDITS) break; - const formatted = formatHashlineEdit(edit); + const formatted = isChunk + ? formatChunkEdit(edit as Partial) + : formatHashlineEdit(edit as Partial); text += uiTheme.fg("toolOutput", truncateToWidth(replaceTabs(formatted.srcLabel), 120)); text += "\n"; if (formatted.dst === "") { @@ -145,39 +151,74 @@ function formatStreamingHashlineEdits(edits: Partial[], uiThem if (shownDstLines > MAX_DST_LINES) break; } if (edits.length > MAX_EDITS) { - text += uiTheme.fg("dim", `… (${edits.length - MAX_EDITS} more edits)`); + text += uiTheme.fg("dim", `\u2026 (${edits.length - MAX_EDITS} more edits)`); } if (shownDstLines > MAX_DST_LINES) { - text += uiTheme.fg("dim", `\n… (${shownDstLines - MAX_DST_LINES} more dst lines)`); + text += uiTheme.fg("dim", `\n\u2026 (${shownDstLines - MAX_DST_LINES} more dst lines)`); } return text.trimEnd(); + function formatHashlineEdit(edit: Partial): { srcLabel: string; dst: string } { if (typeof edit !== "object" || !edit) { - return { srcLabel: "• (incomplete edit)", dst: "" }; + return { srcLabel: "\u2022 (incomplete edit)", dst: "" }; } const contentLines = Array.isArray(edit.content) ? (edit.content as string[]).join("\n") : ""; const loc = edit.loc; if (loc === "append" || loc === "prepend") { - return { srcLabel: `• ${loc} (file-level)`, dst: contentLines }; + return { srcLabel: `\u2022 ${loc} (file-level)`, dst: contentLines }; } if (typeof loc === "object" && loc) { if ("range" in loc && typeof loc.range === "object" && loc.range) { - return { srcLabel: `• range ${loc.range.pos ?? "?"}…${loc.range.end ?? "?"}`, dst: contentLines }; + return { srcLabel: `\u2022 range ${loc.range.pos ?? "?"}\u2026${loc.range.end ?? "?"}`, dst: contentLines }; } if ("line" in loc) { - return { srcLabel: `• line ${(loc as { line: string }).line}`, dst: contentLines }; + return { srcLabel: `\u2022 line ${(loc as { line: string }).line}`, dst: contentLines }; } if ("append" in loc) { - return { srcLabel: `• append ${(loc as { append: string }).append}`, dst: contentLines }; + return { srcLabel: `\u2022 append ${(loc as { append: string }).append}`, dst: contentLines }; } if ("prepend" in loc) { - return { srcLabel: `• prepend ${(loc as { prepend: string }).prepend}`, dst: contentLines }; + return { srcLabel: `\u2022 prepend ${(loc as { prepend: string }).prepend}`, dst: contentLines }; } } - return { srcLabel: "• (unknown edit)", dst: contentLines }; + return { srcLabel: "\u2022 (unknown edit)", dst: contentLines }; + } + + function formatChunkEdit(edit: Partial): { srcLabel: string; dst: string } { + if (typeof edit !== "object" || !edit) { + return { srcLabel: "\u2022 (incomplete edit)", dst: "" }; + } + + const contentLines = Array.isArray(edit.content) + ? (edit.content as string[]).join("\n") + : typeof edit.content === "string" + ? edit.content + : ""; + const target = edit.target ?? "?"; + + if (edit.delete) { + return { srcLabel: `\u2022 delete ${target}`, dst: "" }; + } + if (edit.append) { + return { srcLabel: `\u2022 append child ${target}`, dst: contentLines }; + } + if (edit.prepend) { + return { srcLabel: `\u2022 prepend child ${target}`, dst: contentLines }; + } + if (edit.after) { + return { srcLabel: `\u2022 insert after ${target}.${edit.after}`, dst: contentLines }; + } + if (edit.before) { + return { srcLabel: `\u2022 insert before ${target}.${edit.before}`, dst: contentLines }; + } + if (edit.line != null) { + const range = edit.end_line != null ? `${edit.line}\u2026${edit.end_line}` : `${edit.line}`; + return { srcLabel: `\u2022 replace ${target} L${range}`, dst: contentLines }; + } + return { srcLabel: `\u2022 replace ${target}`, dst: contentLines }; } } function formatMetadataLine(lineCount: number | null, language: string | undefined, uiTheme: Theme): string { diff --git a/packages/coding-agent/src/tools/chunk-tree.ts b/packages/coding-agent/src/tools/chunk-tree.ts index 50e0659c0..008bf59fe 100644 --- a/packages/coding-agent/src/tools/chunk-tree.ts +++ b/packages/coding-agent/src/tools/chunk-tree.ts @@ -129,12 +129,9 @@ 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: rawSelector, - crc: parsedSelector.crc, + selector: parseChunkSelector(readPath.slice(colonIndex + 1) || undefined).selector, }; }