refactor(coding-agent/edit): removed chunk read:true operation from chunk editing
- Removed `read: true` from chunk edit schema and validation logic, including mixed read/write checks and read-only execution paths. - Updated chunk edit normalization to treat chunk operations as mutating and removed unused read-result handling. - Updated prompts and changelog guidance to require `open` for inspection and document that chunk edits use only `write`, `insert`, or `delete`.
This commit is contained in:
can1357
@@ -4,7 +4,6 @@
|
||||
|
||||
### Added
|
||||
|
||||
- Added a non-mutating chunk edit `read: true` operation for inspecting chunk content without relying on edit failures or delete previews.
|
||||
- Added Markdown pipe-table `row_N` chunk selectors for row-level table edits.
|
||||
- Added `resolveToolAlias` export so tool names in CLI and session setup are normalized to canonical names, including mapping legacy `read` references to `open`
|
||||
- Added new `open` and `open-chunk` tool prompt documentation pages to describe canonical `open` usage for local files/directories, chunk reads, and URLs
|
||||
@@ -17,10 +16,9 @@
|
||||
- Changed the canonical file/URL reader tool from `read` to `open` across default tool lists and routing, including system prompts, plan mode, cursor handlers, and runtime tool registration
|
||||
- Changed runtime and UI handling to render and track `open` tool calls as first-class (with `read` accepted as legacy alias), including ACP mapping, session observers, and streaming message groups
|
||||
- Changed chunk edit guidance to document parser-specific region behavior, including TypeScript decorator/JSDoc sibling chunks, Python docstrings as body content, Python opaque nested chunks, Markdown whole-chunk fallbacks, ID volatility, and indentation display differences
|
||||
- Changed chunk edit guidance to point agents at `open` as the primary chunk read/discovery operation, with `read: true` documented as a convenience for known selectors.
|
||||
- Changed chunk deletion in chunk edit mode to require explicit `delete: true`; `write: null` and bare `{ path }` entries now fail with guidance instead of deleting content.
|
||||
- Changed chunk edit validation to reject entries with multiple operation fields instead of choosing one and ignoring the rest.
|
||||
- Changed chunk edit validation to reject `write: ""` as an accidental destructive empty replacement; use `read: true` for inspection or `delete: true` for deletion.
|
||||
- Changed chunk edit validation to reject `write: ""` as an accidental destructive empty replacement; use the open tool for inspection or `delete: true` for deletion.
|
||||
- Changed chunk edit responses to warn when appending or prepending to a container without `~`, since that inserts outside the container rather than inside its body.
|
||||
- Changed fetch output logging so URL-fetch artifacts now use `.open.log` naming instead of `.read.log`
|
||||
- Changed Bash interception guidance and errors to recommend `open` in place of `read` for cat/head/tail-style commands
|
||||
@@ -33,6 +31,7 @@
|
||||
|
||||
### Removed
|
||||
|
||||
- Removed the chunk edit `read: true` operation; use the open tool to inspect chunks without modifying files.
|
||||
- Removed the `replace: { old, new }` chunk edit operation. Use `write` or `insert` for chunk edits instead.
|
||||
|
||||
### Fixed
|
||||
|
||||
@@ -324,7 +324,7 @@ export class EditTool implements AgentTool<TInput> {
|
||||
}),
|
||||
parameters: chunkEditParamsSchema,
|
||||
invalidParamsMessage:
|
||||
"Invalid edit parameters for chunk mode. Expected `{ edits: [{ path: 'file:selector', ...op }, ...] }` with at least one edit. Each edit needs a `path`; supply exactly one of `read: true`, `write: 'content'`, `insert: { loc, body }`, or `delete: true`.",
|
||||
"Invalid edit parameters for chunk mode. Expected `{ edits: [{ path: 'file:selector', ...op }, ...] }` with at least one edit. Each edit needs a `path`; supply exactly one of `write: 'content'`, `insert: { loc, body }`, or `delete: true`.",
|
||||
validate: isChunkParams,
|
||||
execute: (
|
||||
tool: EditTool,
|
||||
|
||||
@@ -198,12 +198,6 @@ export async function computeChunkDiff(
|
||||
options?.signal?.throwIfAborted?.();
|
||||
const { filePath } = parseChunkEditPath(input.path);
|
||||
if (!filePath) return { error: "chunk edit path is empty" };
|
||||
if (input.edits.every(isChunkReadOperation)) {
|
||||
return { diff: "", firstChangedLine: undefined };
|
||||
}
|
||||
if (input.edits.some(hasChunkReadFlag)) {
|
||||
return { error: "`read: true` cannot be mixed with mutating chunk edit operations." };
|
||||
}
|
||||
const { resolvedPath, rawContent, language } = await loadChunkSource({ cwd, path: filePath });
|
||||
options?.signal?.throwIfAborted?.();
|
||||
const { operations } = normalizeChunkEditOperations(input.edits);
|
||||
@@ -558,12 +552,6 @@ export const chunkToolEditSchema = Type.Object(
|
||||
path: Type.String({
|
||||
description: "File path with chunk selector. Examples: 'src/app.ts:fn_foo#ABCD~', 'src/app.ts:class_Bar'.",
|
||||
}),
|
||||
read: Type.Optional(
|
||||
Type.Boolean({
|
||||
description:
|
||||
"Return a known chunk selector without modifying the file. Prefer the open tool for normal chunk reads and discovery.",
|
||||
}),
|
||||
),
|
||||
write: Type.Optional(
|
||||
Type.Union([Type.String(), Type.Null()], {
|
||||
description:
|
||||
@@ -676,27 +664,17 @@ function autoCorrectBodyIndent(content: string, index: number): { content: strin
|
||||
|
||||
function chunkEditOperationFields(edit: ChunkToolEdit): string[] {
|
||||
const fields: string[] = [];
|
||||
if (edit.read === true) fields.push("read");
|
||||
if (edit.write !== undefined) fields.push("write");
|
||||
if (edit.insert != null) fields.push("insert");
|
||||
if (edit.delete === true) fields.push("delete");
|
||||
return fields;
|
||||
}
|
||||
|
||||
function hasChunkReadFlag(edit: ChunkToolEdit): boolean {
|
||||
return edit.read === true;
|
||||
}
|
||||
|
||||
function isChunkReadOperation(edit: ChunkToolEdit): boolean {
|
||||
const fields = chunkEditOperationFields(edit);
|
||||
return fields.length === 1 && fields[0] === "read";
|
||||
}
|
||||
|
||||
function assertSingleChunkOperation(edit: ChunkToolEdit, index: number): string {
|
||||
const fields = chunkEditOperationFields(edit);
|
||||
if (fields.length === 0) {
|
||||
throw new Error(
|
||||
`Edit ${index + 1}: no operation specified. Use open to inspect chunks, read:true only for a known selector, write:"..." to replace, insert:{loc,body}, or delete:true to delete.`,
|
||||
`Edit ${index + 1}: no operation specified. Use write:"..." to replace, insert:{loc,body} to insert, or delete:true to delete. Use the open tool to inspect chunks.`,
|
||||
);
|
||||
}
|
||||
if (fields.length > 1) {
|
||||
@@ -715,13 +693,10 @@ function normalizeChunkEditOperations(edits: ChunkToolEdit[]): {
|
||||
const operations = edits.map((edit, index): ChunkEditOperation => {
|
||||
const { selector } = parseChunkEditPath(edit.path);
|
||||
const operation = assertSingleChunkOperation(edit, index);
|
||||
if (operation === "read") {
|
||||
throw new Error("`read: true` is non-mutating and cannot be normalized as an edit operation.");
|
||||
}
|
||||
if (operation === "write") {
|
||||
if (edit.write === null) {
|
||||
throw new Error(
|
||||
`Edit ${index + 1}: write:null no longer deletes chunks. Use delete:true to delete, or open/read:true to inspect chunk content without modifying the file.`,
|
||||
`Edit ${index + 1}: write:null no longer deletes chunks. Use delete:true to delete, or open the chunk to inspect its content without modifying the file.`,
|
||||
);
|
||||
}
|
||||
if (typeof edit.write !== "string") {
|
||||
@@ -805,58 +780,15 @@ async function writeChunkResult(params: {
|
||||
};
|
||||
}
|
||||
|
||||
async function readChunkResult(params: {
|
||||
session: ToolSession;
|
||||
resolvedPath: string;
|
||||
sourceExists: boolean;
|
||||
chunkLanguage: string | undefined;
|
||||
edits: ChunkToolEdit[];
|
||||
}): Promise<AgentToolResult<EditToolDetails, typeof chunkEditParamsSchema>> {
|
||||
const { session, resolvedPath, sourceExists, chunkLanguage, edits } = params;
|
||||
if (!sourceExists) {
|
||||
throw new Error(`File does not exist: ${resolvedPath}. Cannot read chunk selectors on a non-existent file.`);
|
||||
}
|
||||
|
||||
const texts: string[] = [];
|
||||
for (const edit of edits) {
|
||||
const { selector } = parseChunkEditPath(edit.path);
|
||||
const readPath = selector ? `${resolvedPath}:${selector}` : resolvedPath;
|
||||
const result = await formatChunkedRead({
|
||||
filePath: resolvedPath,
|
||||
readPath,
|
||||
cwd: session.cwd,
|
||||
language: chunkLanguage,
|
||||
anchorStyle: resolveAnchorStyle(session.settings),
|
||||
});
|
||||
texts.push(result.text);
|
||||
}
|
||||
|
||||
return {
|
||||
content: [{ type: "text", text: texts.join("\n\n") }],
|
||||
details: {
|
||||
diff: "",
|
||||
meta: outputMeta().get(),
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
export async function executeChunkSingle(
|
||||
options: ExecuteChunkSingleOptions,
|
||||
): Promise<AgentToolResult<EditToolDetails, typeof chunkEditParamsSchema>> {
|
||||
const { session, path, edits, signal, batchRequest, writethrough, beginDeferredDiagnosticsForPath } = options;
|
||||
const readOnly = edits.every(isChunkReadOperation);
|
||||
if (edits.some(hasChunkReadFlag) && !readOnly) {
|
||||
throw new Error("`read: true` cannot be mixed with mutating chunk edit operations.");
|
||||
}
|
||||
const { resolvedPath, sourceFile, sourceExists, rawContent, chunkLanguage } = await resolveChunkSourceContext(
|
||||
session,
|
||||
path,
|
||||
{ intent: readOnly ? "read" : "write" },
|
||||
{ intent: "write" },
|
||||
);
|
||||
if (readOnly) {
|
||||
return readChunkResult({ session, resolvedPath, sourceExists, chunkLanguage, edits });
|
||||
}
|
||||
|
||||
const parentDir = nodePath.dirname(resolvedPath);
|
||||
if (parentDir && parentDir !== ".") {
|
||||
await fs.mkdir(parentDir, { recursive: true });
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
Edits files via syntax-aware chunks. Use `read(path="file.ts")` to read and discover chunks before editing.
|
||||
- `read` is the canonical read path for chunk source and `sel="?"` tree listings.
|
||||
- `read:true` is a non-mutating convenience for an already-known selector.
|
||||
- `write` rewrites the entire targeted region — best for most edits.
|
||||
- `insert` adds content before/after a chunk.
|
||||
- `delete` deletes a targeted chunk and must be explicit.
|
||||
@@ -10,7 +9,6 @@ Call format: `{"edits": [{"path": "file:chunk#ID~", "write": "new body"}, …]}`
|
||||
<rules>
|
||||
- **MUST** inspect first with `read`. Never invent chunk paths or IDs. Copy them from the latest `read` output or edit response.
|
||||
- `path` format: `file:selector` — e.g. `src/app.ts:fn_foo#ABCD~`. Append `~` for body, `^` for head, or nothing for the whole chunk. Include `#ID` for `write`/`delete`.
|
||||
- To inspect a known chunk through this tool, use `{"path":"file:chunk#ID","read":true}`. `read:true` is non-mutating, cannot be mixed with write operations in the same entry, and is not a replacement for `read(path="file", sel="?")` when discovering targets.
|
||||
- If the exact chunk path is unclear, run `read(path="file", sel="?")` and copy a selector from that listing.
|
||||
{{#if chunkAutoIndent}}
|
||||
- Use `\t` for indentation in `content`. Write content at indent-level 0 — the tool re-indents it to match the chunk's position in the file. For example, to replace `~` of a method, write the body starting at column 0:
|
||||
@@ -51,7 +49,7 @@ You **MUST** use the narrowest region that covers your change. Putting without a
|
||||
</critical>
|
||||
|
||||
<regions>
|
||||
In `read` or `read:true` output, lines marked `^` between the line number and `|` are **head** lines (doc comments, attributes/decorators, signature). Lines without `^` are **body** lines. Use this to decide which region to target:
|
||||
In `read` output, lines marked `^` between the line number and `|` are **head** lines (doc comments, attributes/decorators, signature). Lines without `^` are **body** lines. Use this to decide which region to target:
|
||||
- `fn_foo#ID~` — **body only (the default choice for most edits).** Head lines (`^`) are preserved automatically — doc comments, attributes, and signature stay untouched. On code leaf chunks, this is rejected because there is no safe body boundary.
|
||||
- `fn_foo#ID^` — head only (decorators, attributes, doc comments, signature, opening delimiter). Body stays untouched.
|
||||
- `fn_foo#ID` — entire chunk including leading trivia. **You must include doc comments and attributes in `content`; omitting them deletes them.**
|
||||
@@ -65,11 +63,10 @@ In `read` or `read:true` output, lines marked `^` between the line number and `|
|
||||
</regions>
|
||||
|
||||
<ops>
|
||||
Each edit entry has `path` (`file:selector`) plus **exactly one** operation field — `read`, `write`, `insert`, or `delete`. Never set more than one on the same entry. `write:null`, `write:""`, and bare `{path}` entries are rejected; they do not read or delete.
|
||||
Each edit entry has `path` (`file:selector`) plus **exactly one** operation field — `write`, `insert`, or `delete`. Never set more than one on the same entry. `write:null`, `write:""`, and bare `{path}` entries are rejected; they do not delete.
|
||||
|
||||
|fields|path (selector part)|effect|
|
||||
|---|---|---|
|
||||
|`read: true`|`file`, `file:?`, `file:chunk#ID`, `file:chunk#ID~`, or `file:chunk#ID^`|return chunk source without modifying the file|
|
||||
|`write: "content"`|`file:chunk#ID`, `file:chunk#ID~`, or `file:chunk#ID^`|write complete new content to the region|
|
||||
|`delete: true`|`file:chunk#ID`|delete the chunk explicitly|
|
||||
|`insert: {loc, body}`|`file:chunk` or `file:chunk~`|insert before/after the chunk (`loc`: `"prepend"` or `"append"`)|
|
||||
|
||||
@@ -194,19 +194,6 @@ describe("computeChunkDiff", () => {
|
||||
expect("error" in result).toBe(true);
|
||||
});
|
||||
|
||||
test("returns an empty preview for read-only chunk reads", async () => {
|
||||
const file = path.join(tmpDir, "read.ts");
|
||||
await fs.writeFile(file, "export const x = 1;\n");
|
||||
const result = await computeChunkDiff(
|
||||
{
|
||||
path: "read.ts",
|
||||
edits: [{ path: "read.ts:?", read: true }],
|
||||
},
|
||||
tmpDir,
|
||||
);
|
||||
expect(result).toEqual({ diff: "", firstChangedLine: undefined });
|
||||
});
|
||||
|
||||
test("rejects write:null instead of previewing a delete", async () => {
|
||||
const file = path.join(tmpDir, "null-delete.ts");
|
||||
await fs.writeFile(file, "export const x = 1;\n");
|
||||
|
||||
Reference in New Issue
Block a user