Skip to content

Composer V2: Replace replaceInFile with editFile as the primary targeted-edit tool#2305

Merged
wenzhengjiang merged 20 commits intomasterfrom
feat/editfile-write-first-inline-diff
Mar 17, 2026
Merged

Composer V2: Replace replaceInFile with editFile as the primary targeted-edit tool#2305
wenzhengjiang merged 20 commits intomasterfrom
feat/editfile-write-first-inline-diff

Conversation

@wenzhengjiang
Copy link
Collaborator

@wenzhengjiang wenzhengjiang commented Mar 16, 2026

Motivation

Previously, making targeted edits to files required either:

  • writeFile — outputs the entire file content every time, consuming many output tokens even for a one-line change. As the conversation context grows or the file being edited gets longer, the LLM increasingly struggles to reproduce the full content correctly, leading to truncated or corrupted output.
  • replaceInFile — only outputs the diff, but used a custom ------- SEARCH / ======= / +++++++ REPLACE block syntax that was complex, error-prone to parse, and allowed multiple edits bundled into one call.

This PR replaces replaceInFile with a new editFile tool as the primary mechanism for targeted edits.

Key Changes

New editFile tool interface

The tool interface is radically simplified: instead of a diff string with custom markers, it takes plain { oldText, newText } parameters — one edit per call. This eliminates the need for an LLM to learn and reliably produce a custom diff syntax, and removes the complex parseSearchReplaceBlocks parser entirely.

The constraint of one edit per call is intentional: it keeps each tool invocation atomic and predictable. For multiple edits, the LLM calls the tool multiple times.

Output token reduction

editFile only sends the changed text snippet, not the whole file. For typical targeted edits this is an order-of-magnitude reduction in output tokens compared to writeFile.

Improved matching robustness

The old replaceInFile matched text with basic line-ending normalization. The new editFile uses a two-stage matching strategy:

  1. Exact match (after LF normalization)
  2. Fuzzy match — NFKC normalization, smart quotes/dashes, trailing whitespace, BOM stripping, non-breaking spaces

This makes the tool tolerant of common LLM-introduced artifacts without sacrificing correctness.

ApplyView integration with simple mode

editFile now shows the same Apply View preview as writeFile and respects the autoAcceptEdits setting. Because editFile always represents a single, atomic change, the per-block accept/reject/revert buttons are hidden — the view shows only the global Accept/Reject bar (simple flag on ApplyViewState).

Removed file size restriction

replaceInFile refused to operate on files smaller than 3,000 characters, pushing small-file edits back to writeFile. editFile has no such restriction — it is the right tool regardless of file size.

Screenshot 2026-03-17 at 18 28 04

🤖 Generated with Claude Code

@wenzhengjiang wenzhengjiang changed the title feat(editFile): write-first with inline diff in chat feat: replace replaceInFile with editFile as the primary targeted-edit tool Mar 17, 2026
@wenzhengjiang wenzhengjiang force-pushed the feat/editfile-write-first-inline-diff branch from 1934e0d to fc58610 Compare March 17, 2026 00:14
wenzhengjiang and others added 5 commits March 17, 2026 10:21
Replace the blocking accept/reject modal in editFile with a write-first
approach: changes are written to disk immediately, and a unified diff is
returned inline in the chat as a fenced \`\`\`diff block. Multiple
editFile calls in a single agent turn each produce their own diff
without requiring any user interaction.

Also cleans up associated rename/refactor of writeToFileTool →
writeFileTool across chain runner, streamer, and related tests.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…plyView

- Pipe editFileDiffs through message types (ChatMessage, StoredMessage) and
  MessageRepository so writeFile/editFile diffs are stored as a typed field
  (same pattern as sources)
- Collect diffs in AutonomousAgentChainRunner.runReActLoop() by parsing the
  JSON result of successful writeFile/editFile tool calls, then pass through
  ReActLoopResult → handleResponse() → ChatMessage.editFileDiffs
- ChatSingleMessage reads message.editFileDiffs directly (instead of trying
  to parse TOOL_CALL markers which don't exist in agent mode)
- ChatButtons shows a "Show diff" button when editFileDiffs is present;
  clicking it opens ApplyView for each diff
- ApplyView redesigned: renders change blocks side-by-side (original left
  with red highlights, modified right with green highlights) using the
  SideBySideBlock component, unchanged blocks shown as plain text between
  change blocks — restores the classic diff block presentation

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- editFile now shows the Apply View preview and blocks until user accepts/rejects
- Respects autoAcceptEdits setting (bypasses preview when enabled)
- Adds `simple` flag to ApplyViewState to hide per-block accept/reject/revert buttons
- Removes unused createPatch import

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… in your vault"

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@wenzhengjiang wenzhengjiang force-pushed the feat/editfile-write-first-inline-diff branch from fc58610 to 5dfaf82 Compare March 17, 2026 01:21
@wenzhengjiang wenzhengjiang marked this pull request as ready for review March 17, 2026 01:22
@wenzhengjiang wenzhengjiang changed the title feat: replace replaceInFile with editFile as the primary targeted-edit tool Composer V2: Replace replaceInFile with editFile as the primary targeted-edit tool Mar 17, 2026
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d60186515d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

…igration

- modelAdapter: update GPT prompt to use editFile's oldText/newText params
  instead of the old SEARCH/REPLACE block syntax
- ComposerTools: fix fuzzy match to replace only the matched span in the
  original content; previously the entire file was rebuilt from the
  fuzzy-normalized string, inadvertently applying trimEnd and quote/dash
  normalization to text outside the matched span
- settings/model: migrate persisted autonomousAgentEnabledToolIds from
  writeToFile→writeFile and replaceInFile→editFile on upgrade

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@wenzhengjiang
Copy link
Collaborator Author

@codex review

Extract pure replacement logic into applyEditToContent() for testability,
then wire editFileTool.func to call it (removes duplicated logic).

Tests cover:
- Exact match: basic replacement, deletion, NOT_FOUND, AMBIGUOUS
- Fuzzy via smart quotes (single, double), en-dash, NBSP
- Fuzzy via trailing whitespace (spaces, tabs)
- Key invariant: fuzzy match only replaces the matched span — smart quotes
  and trailing spaces outside the span are left untouched
- Multiline fuzzy match with mixed artifacts
- CRLF and LF preservation
- BOM preservation

Also fixes mapFuzzyIndexToNormal to extend matchEnd to the full original
line length when the fuzzy match ends at end-of-line, preventing orphaned
trailing whitespace after replacement.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 79217c624f

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

- ComposerTools: fix mapFuzzyIndexToNormal to handle NFKC expansion
  (e.g. Ⅳ → IV) by walking char-by-char within each line instead of
  assuming a 1:1 column mapping; adds two unit tests covering this case
- ChatSingleMessage: normalise legacy <writeToFile> tags to <writeFile>
  before rendering so saved chat history from before the tool rename
  continues to display correctly

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@wenzhengjiang
Copy link
Collaborator Author

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 372fc54f28

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +462 to +466
if (modifiedContent === "NOT_FOUND") {
return `Could not find the specified text in ${path}. The oldText must match the file content — try including more surrounding context lines to locate the right spot.`;
}

let changesApplied = 0;

// Apply each SEARCH/REPLACE block in order
for (const block of searchReplaceBlocks) {
let { searchText, replaceText } = block;

// Check if the search text exists in the current content (with line ending normalization)
const normalizedContent = normalizeLineEndings(modifiedContent);
const normalizedSearchText = normalizeLineEndings(searchText);

if (!normalizedContent.includes(normalizedSearchText)) {
// Handle corner case where the search text is at the end of the file
if (normalizedContent.includes(normalizedSearchText.trimEnd())) {
searchText = searchText.trimEnd();
replaceText = replaceText.trimEnd();
} else {
return `Search text not found in file ${path} : "${searchText}".`;
}
}

// Replace all occurrences using line ending aware replacement
const beforeReplace = modifiedContent;
modifiedContent = replaceWithLineEndingAwareness(modifiedContent, searchText, replaceText);

// Check if any replacements were made
if (modifiedContent !== beforeReplace) {
changesApplied++;
}
if (modifiedContent.startsWith("AMBIGUOUS:")) {
const count = modifiedContent.split(":")[1];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Replace sentinel-string errors with structured edit results

The editFile flow uses raw strings ("NOT_FOUND" and "AMBIGUOUS:*") as control signals, so legitimate edits are rejected when the new file content happens to match those strings. For example, replacing an entire file with "NOT_FOUND", or with text that begins "AMBIGUOUS:", will be treated as an error even though the edit succeeded. Returning a structured result (status + content/error) avoids this collision and prevents false failures on valid user edits.

Useful? React with 👍 / 👎.

src/utils.ts Outdated
// Remove standalone writeToFile blocks
cleanedMessage = cleanedMessage.replace(/<writeToFile>[\s\S]*?<\/writeToFile>/g, "");
// Remove standalone writeFile blocks
cleanedMessage = cleanedMessage.replace(/<writeFile>[\s\S]*?<\/writeFile>/g, "");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Handle legacy writeToFile tags when cleaning copied messages

After the rename to <writeFile>, cleanMessageForCopy only strips the new tag, so older persisted chats that still contain <writeToFile>...</writeToFile> will leak raw action XML into copied/inserted text. This is a backward-compatibility regression for users with pre-upgrade chat history; the cleaner should remove both legacy and new tag variants.

Useful? React with 👍 / 👎.

wenzhengjiang and others added 2 commits March 17, 2026 11:53
…ompt

Replace stale SEARCH/REPLACE references with the new parameter names.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…criminated union

- `applyEditToContent` now returns `{ ok: true; content: string } | { ok: false; reason: 'NOT_FOUND' } | { ok: false; reason: 'AMBIGUOUS'; occurrences: number }` instead of raw sentinel strings, eliminating false positives when file content matches 'NOT_FOUND' or starts with 'AMBIGUOUS:'
- Update `editFileTool` to pattern-match on the discriminated union
- Update `cleanMessageForCopy` regexes to also strip legacy `<writeToFile>` tags from saved chat history
- Update all `applyEditToContent` test assertions to use the new structured return type

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@wenzhengjiang
Copy link
Collaborator Author

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e928dde081

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines 461 to 463
const file = app.vault.getAbstractFileByPath(path);

if (!file || !(file instanceof TFile)) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Sanitize editFile path before vault lookup

editFile reads path directly via app.vault.getAbstractFileByPath(path) without applying sanitizeFilePath, unlike writeFile. This introduces a regression for long basenames: writeFile can truncate and create the file successfully, but a follow-up editFile call using the original unsanitized path will report “File not found” and the edit cannot proceed. Because this change makes editFile the primary targeted-edit path (including small files), that failure mode is now much easier to hit in normal usage.

Useful? React with 👍 / 👎.

private findCompleteBlock(str: string) {
// Regex for both formats
const regex = /<writeToFile>[\s\S]*?<\/writeToFile>/;
const regex = /<writeFile>[\s\S]*?<\/writeFile>/;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Accept legacy writeToFile blocks in stream parser

The stream parser now only matches <writeFile>...</writeFile> blocks, so any model output that still emits legacy <writeToFile> tags will be ignored and never executed. This is a practical compatibility break during upgrades/ongoing chats (and there are still prompt strings in the repo that mention writeToFile), so users can get raw XML in the response instead of an applied file change.

Useful? React with 👍 / 👎.

Apply `sanitizeFilePath` in `editFileTool` to match the same treatment
`writeFileTool` already does, preventing "File not found" failures when
the path has a basename that exceeds 255 bytes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@wenzhengjiang
Copy link
Collaborator Author

@codex review

1 similar comment
@wenzhengjiang
Copy link
Collaborator Author

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 64c66ccf14

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

private findCompleteBlock(str: string) {
// Regex for both formats
const regex = /<writeToFile>[\s\S]*?<\/writeToFile>/;
const regex = /<writeFile>[\s\S]*?<\/writeFile>/;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve compatibility with legacy writeToFile blocks

Keep matching legacy <writeToFile> tags in the stream parser, because this commit removes that path entirely (findCompleteBlock now only recognizes <writeFile>) while prompt text in the same snapshot still instructs writeToFile in at least one flow (e.g. src/tools/builtinTools.ts templates guidance). In those cases the model can emit <writeToFile>...</writeToFile>, and the block will never be executed, so users get raw XML output instead of an applied edit.

Useful? React with 👍 / 👎.

Comment on lines +366 to +368
const expandedLen = normalLine[ni].normalize("NFKC").length;
if (fi + expandedLen > remaining) break; // target is inside this char's expansion
fi += expandedLen;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject fuzzy matches that land inside NFKC expansions

Avoid treating a fuzzy hit inside an expanded code point as a valid replacement boundary: when remaining falls in the middle of an NFKC expansion, the current mapping breaks and returns the start of the original character, which can replace a different span than requested. For example, searching oldText="V" can match inside fuzzy "IV" derived from "Ⅳ", then replace the entire character, causing unintended edits.

Useful? React with 👍 / 👎.

…sion

When `oldText` fuzzy-matches a span whose boundaries both map to the same
position in the original (e.g. searching "I" inside fuzzy "IV" derived from
"Ⅳ"), `matchStart >= matchEnd` and the replacement would be zero-width or
corrupt. Return NOT_FOUND instead of silently producing a wrong edit.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@wenzhengjiang
Copy link
Collaborator Author

@codex review

1 similar comment
@wenzhengjiang
Copy link
Collaborator Author

@codex review

@wenzhengjiang
Copy link
Collaborator Author

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7dacb39879

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +317 to +318
const fuzzySearch = normalizeForFuzzyMatch(normalizedOldText);
const fuzzyCount = countOccurrences(fuzzyContent, fuzzySearch);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Handle EOF newline mismatch in fuzzy search

editFile now fails to match a valid target when oldText includes a trailing newline but the file ends without one (a common LLM output pattern for end-of-file edits). In findTextForReplacement, both exact and fuzzy stages keep that final \n in fuzzySearch, so the lookup returns NOT_FOUND instead of falling back to the same trim-end behavior the previous implementation had. This regresses targeted edits at EOF and will cause avoidable tool failures unless the model guesses newline presence exactly.

Useful? React with 👍 / 👎.

LLMs frequently append \n to the last line of oldText even when the file
has no final newline. Neither exact nor fuzzy match would find this, causing
avoidable NOT_FOUND failures. Stage 3 retries with one trailing \n stripped
from oldText (and mirrors the trim on newText to preserve the file's
no-trailing-newline format).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@wenzhengjiang
Copy link
Collaborator Author

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5555369445

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

private findCompleteBlock(str: string) {
// Regex for both formats
const regex = /<writeToFile>[\s\S]*?<\/writeToFile>/;
const regex = /<writeFile>[\s\S]*?<\/writeFile>/;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve legacy write tag parsing in action streaming

Restricting block detection to writeFile means streamed <writeToFile>...</writeToFile> blocks are now ignored, so the file tool is never invoked and the assistant appears to “emit edits” without applying them; this is a real compatibility gap because legacy tags are still handled elsewhere (e.g., chat rendering/copy cleanup) and can still surface from prior conversations or model output drift. Consider matching both tag names (or normalizing input before parsing) to avoid silently dropping edit actions.

Useful? React with 👍 / 👎.

Comment on lines +324 to 327
toolName: "editFile",
// Check if args.diff contains the correct search text
argumentValidator: (args) => {
expect(args.path).toBe("test.md");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 Badge Update integration validator to editFile argument shape

This test case was renamed to expect editFile, but the validator still asserts the old args.diff SEARCH/REPLACE payload; with the new tool contract (oldText/newText), correct model behavior will fail this integration test and produce false negatives during prompt/tool regression checks.

Useful? React with 👍 / 👎.

The argumentValidator was still asserting the legacy args.diff SEARCH/REPLACE
payload. Updated to check args.oldText and args.newText to match the new
editFile tool contract.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@wenzhengjiang
Copy link
Collaborator Author

@codex review

2 similar comments
@wenzhengjiang
Copy link
Collaborator Author

@codex review

@wenzhengjiang
Copy link
Collaborator Author

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c40d362257

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +254 to +256
while ((pos = content.indexOf(searchText, pos)) !== -1) {
count++;
pos += searchText.length;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Detect overlapping matches as ambiguous in editFile

countOccurrences advances by searchText.length, so it only counts non-overlapping matches; with inputs like content="ababa" and oldText="aba", it returns one match and applyEditToContent proceeds, even though there are two valid start positions. Because editFile is documented as requiring a unique match, this can apply an edit to the wrong location instead of returning the ambiguous-match error.

Useful? React with 👍 / 👎.

private findCompleteBlock(str: string) {
// Regex for both formats
const regex = /<writeToFile>[\s\S]*?<\/writeToFile>/;
const regex = /<writeFile>[\s\S]*?<\/writeFile>/;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep legacy writeToFile tags executable in stream parser

In the CopilotPlus streaming path, findCompleteBlock now recognizes only <writeFile>...</writeFile>, so any model output that still uses legacy <writeToFile> tags will be passed through without calling the file tool. Since this commit also adds legacy-tag normalization elsewhere, older chat histories can still surface that format, and those sessions lose automatic file-write execution.

Useful? React with 👍 / 👎.

countOccurrences was advancing by searchText.length, so overlapping patterns
like "aba" in "ababa" were counted as 1 (non-overlapping) instead of 2. This
caused an ambiguous match to be silently applied at the first position rather
than returning AMBIGUOUS. Advance by 1 to catch all overlapping occurrences.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@wenzhengjiang
Copy link
Collaborator Author

@codex review

3 similar comments
@wenzhengjiang
Copy link
Collaborator Author

@codex review

@wenzhengjiang
Copy link
Collaborator Author

@codex review

@wenzhengjiang
Copy link
Collaborator Author

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1ba9690a20

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

private findCompleteBlock(str: string) {
// Regex for both formats
const regex = /<writeToFile>[\s\S]*?<\/writeToFile>/;
const regex = /<writeFile>[\s\S]*?<\/writeFile>/;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Accept legacy writeToFile blocks in streaming parser

The updated parser now matches only <writeFile>...</writeFile>, so any model output that still emits legacy <writeToFile> tags will no longer trigger tool execution and file edits will be silently skipped. This is a realistic post-migration path (e.g., continued chats with old examples or stale prompt text), and the repo still contains legacy guidance in src/tools/builtinTools.ts (obsidianTemplates instructions) that can prompt that older tag name. Please keep backward-compatible matching (both tag names) or provide an explicit alias path.

Useful? React with 👍 / 👎.

@wenzhengjiang
Copy link
Collaborator Author

@codex review

1 similar comment
@wenzhengjiang
Copy link
Collaborator Author

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1ba9690a20

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

private findCompleteBlock(str: string) {
// Regex for both formats
const regex = /<writeToFile>[\s\S]*?<\/writeToFile>/;
const regex = /<writeFile>[\s\S]*?<\/writeFile>/;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Parse legacy writeToFile action blocks during migration

The stream parser now matches only <writeFile>...</writeFile>, so any model output that still emits legacy <writeToFile> blocks is ignored and the file write never runs. This is a functional regression during migration because legacy naming is still present elsewhere in the repo and can still appear from persisted/custom prompts; previously these blocks were executed. Keep backward-compatible matching for both tags until all prompt surfaces and stored histories are fully migrated.

Useful? React with 👍 / 👎.

@wenzhengjiang
Copy link
Collaborator Author

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1ba9690a20

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +419 to +423
if (fi + expandedLen > remaining) break; // target is inside this char's expansion
fi += expandedLen;
ni++;
}
return normalPos + ni;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject fuzzy matches inside NFKC-expanded characters

When fuzzy matching maps indices back to the original text, mapFuzzyIndexToNormal currently snaps positions that fall inside an NFKC expansion to the start of that source character, which can turn a partial match into a full-character replacement. For example, editing oldText: "V" against content containing "Ⅳ" (which normalizes to "IV") is treated as a valid single match and replaces the entire , even though the requested text never exists as a standalone character in the file. This can apply unintended edits in Unicode-heavy notes and should be rejected as non-exact/ambiguous instead.

Useful? React with 👍 / 👎.

…atches

The existing matchStart >= matchEnd guard only caught degenerate zero-width
spans (e.g. "I" in "Ⅳ"). A partial match like "V" against "Ⅳ" produces a
valid non-zero span [0,1) but would replace the entire source character even
though "V" doesn't exist standalone in the file.

Fix: after mapping indices back to the original, verify that
normalizeForFuzzyMatch(original[matchStart:matchEnd]) === workingSearch.
If not, the match straddled an NFKC expansion boundary → return NOT_FOUND.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@wenzhengjiang
Copy link
Collaborator Author

@codex review

2 similar comments
@wenzhengjiang
Copy link
Collaborator Author

@codex review

@wenzhengjiang
Copy link
Collaborator Author

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 29282aaa79

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

wenzhengjiang and others added 2 commits March 17, 2026 18:35
Replace plain string returns with `{ result: "failed", message }` objects
for all failure paths in editFileTool (file not found, text not found,
ambiguous match, and exceptions), matching writeFileTool's pattern so
the LLM and result formatters can reliably detect failures via the
`result` field. No-op case returns `result: "accepted"` since the edit
technically succeeded.

Also fix a stale tool name reference in builtinTools prompt instructions
(writeToFile → writeFile).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@wenzhengjiang wenzhengjiang merged commit d308423 into master Mar 17, 2026
2 checks passed
@wenzhengjiang wenzhengjiang deleted the feat/editfile-write-first-inline-diff branch March 17, 2026 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant