📍 feat: Quote the Current Text Where a Missing Workspace Edit Belongs - #300
Conversation
A missing edit's diagnosis now ends with a bounded excerpt of the region old_text most likely meant: the alignment where most of its distinctive lines fall into place (found in one pass with bounded votes), else its unique first line or closest line. Each of at most 8 rows is `<line>|<mark><text>`, with a space for identical lines, `~` for whitespace-only differences and `!` for changed text; lines are shortened to 160 characters and the excerpt to 1,600. The excerpt is a final `; the current text at lines A-B (...) is "<json>"` hint, so hosts that parse the existing grammar drop it and keep every other hint. Excerpts are the first detail dropped to fit the 3,000-character message bound and never shorten another edit's reason.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85e78a15a3
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const start = index - offset; | ||
| if (start < 0) continue; |
There was a problem hiding this comment.
When a large repetitive old_text contains thousands of identical anchor lines, each matching file line iterates the entire hits array, but offsets producing start < 0 bypass the budget decrement. As a result, the nominal 200,000-operation bound can still perform hundreds of millions of candidate checks; a valid 1 MiB batch with many failed edits can spend several seconds here before returning EDIT_CONFLICT. Decrement the budget for every inspected hit, including negative-start candidates, or avoid iterating offsets that cannot produce a nonnegative start.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 104b8e3. Every inspected offset now decrements the budget, including one that would start before the file. Offsets are collected in ascending order, so the first negative start also ends that line's loop. A new test runs a 5,000-line repetitive old_text against a 20,000-line file within the time bound.
| const nearest = nearestLine(needle.find((line) => line.trim().length > 0), lines); | ||
| const reason = describeMissingReason(text, oldText, needle, matching, lines, nearest); | ||
| const anchor = nearest != null && (!nearest.exact || nearest.count === 1) ? nearest.index : undefined; | ||
| const start = closestRegion(needle, lines()) ?? anchor; |
There was a problem hiding this comment.
When old_text begins with one or more blank lines and has too few distinctive lines for closestRegion, nearestLine locates the first nonblank needle line, but its file index is used directly as the region start. The excerpt is therefore shifted down by the number of leading blank lines: for example, a missing "\nconst total = items.size;" can quote the current const line as differing from the blank first needle line and then quote the unrelated following line. Preserve the nonblank line's needle offset and subtract it from nearest.index before generating the excerpt.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 104b8e3. The fallback anchor now subtracts the first non-blank line's offset in old_text (clamped at the top of the file), so the excerpt starts where old_text starts. A new test covers "\nconst total = items.size;".
The same commit also escapes U+2028/U+2029 in quoted text. JSON.stringify leaves them raw, and a host's per-line batch parser would stop at them.
…t Starts - Count every inspected anchor offset against the region vote budget, and stop at the first offset that would start before the file (offsets ascend), so a repetitive old_text cannot escape the bound. - Start a first-line or closest-line excerpt at the line where old_text starts, not at its first non-blank line. - Escape U+2028/U+2029 in quoted text, which JSON.stringify leaves raw, so each batch edit stays on one line for hosts that read the message line by line.
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Code API returns the diagnostic inside a JSON error body, and LibreChat reads that body through a 4,096-byte buffer, discarding a truncated one. A 3,000-character message with multibyte or escape-heavy excerpts could exceed it. Excerpts are now limited to 1,600 bytes and granted only while the whole message stays within 3,800 bytes, both measured as UTF-8 once JSON-encoded.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1a197e558
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const reasonLength = reasons.reduce((total, reason) => total + reason.length, 0); | ||
| const reasonLimit = reasonLength <= available ? Infinity : Math.floor(available / failures.length); |
There was a problem hiding this comment.
When a batch has multiple missing edits whose nearest-line snippets contain multibyte text, excerpts are correctly declined, but the base reasons are truncated only by UTF-16 character count here. For example, 20 failures quoting emoji-heavy lines produce a 2,997-character diagnostic whose serialized {error, code} body is about 4,411 bytes, exceeding the 4,096-byte host buffer and causing the response to be discarded. Apply the encoded-byte budget while truncating reasons, or validate and shrink the final batch message.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c807ce4. When batch reasons still exceed the bound after excerpts are declined, each one is now shortened against both the per-reason character share and an equal share of the 3,800-byte encoded budget, with the ellipsis counted and surrogate pairs kept whole. A single edit's reason gets the same treatment instead of a bare character slice. New tests run your scenario (20 failures quoting emoji-heavy lines) and a single long multibyte reason. Both assert the serialized {error, code} body is at most 4,096 bytes, every position is present and the output is well-formed UTF-8.
Batch reasons that still exceed the bound after excerpts are declined are now shortened against both the 3,000-character limit and the 3,800-byte encoded budget, and a single edit's reason is shortened the same way instead of being cut by character count. Multibyte closest-line quotes can no longer push the Code API's error body past the 4,096 bytes hosts read.
|
@codex review the latest head, final review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…16698) * 📍 feat: Show the Current Text Where a Missing Workspace Edit Belongs Render the current-text excerpt that LibreChat-AI/code-interpreter#300 workers append to a missing edit's EDIT_CONFLICT diagnosis: a strict parse keeps it only when every row is well formed (contiguous numbers matching the stated range, a known mark, a bounded single line), and LibreChat renders it as a numbered block under its edit with a summary of which lines differ. The excerpt passes the file-content policy like a read_file result and reaches only the model; logs keep the excerpt-free message. Also stop rendering every 409 as a text mismatch: a Code API rejection such as WORKSPACE_QUARANTINED now passes through with its own reason, and the worker's repetitive-candidate refusal is recognized. * 🔒 fix: Name Other Attached Edit 409s by Code Instead of Forwarding Their Body A 409 whose code is not EDIT_CONFLICT (a quarantined workspace) now gets a host-worded message naming only its validated code, with recovery guidance for WORKSPACE_QUARANTINED, and logs keep a body of just that code. * 🧾 fix: Present Quoted Edit Excerpts as Partial Context, Not Text to Copy A worker excerpt can window a long region and shorten wide lines with an ellipsis, so the guidance now asks the model to correct old_text against it and use read_file for lines it leaves out or shortens, here and in the attached edit_file descriptions. * 🔐 fix: Quote Edit Excerpts Only Where the Workspace Allows read_file A workspace that offers edit_file without read_file now never receives the worker's current-text excerpt, so a failing edit cannot read file content the operation policy withholds. --------- Co-authored-by: Lia <lia@librechat.ai>
1 participant
Summary
When an attached-workspace
edit_filemisses, the model gets a line number at best ("its first line appears at line 41, but the lines after it differ", "the closest line is line 3605") or nothing at all ("old_text was not found."). It then re-reads the file, often entirely, to find out what actually changed. This PR makes the worker quote the current text of the region the edit most likely meant, marked line by line, so the next attempt can copy it directly.Evidence (last 7 days)
edit_filecalls; 90 failed withEDIT_CONFLICT. 79 came from a worker build before 🎯 feat: Diagnose Every Failing Workspace Edit and Negotiate Tolerant Matching #271, which returned only "Workspace edit must match exactly once".tolerant_match, and LibreChat sendsmatching: 'tolerant'by default), 11 conflicts remain: 6 ambiguous with line numbers, 3 bare "old_text was not found.", 1 "closest line is line N" and 1 rendered generically.old_textwith the next successful edit to the same file: the most common fix added context (an ambiguous match). The next most common changed 1–3 lines (stale or misremembered content), then whitespace or indentation only. No failure involved line-number prefixes, elisions or CRLF. 35 of 90 failures were followed by aread_fileof the same file before the retry.Whitespace drift is now handled by tolerant matching, and ambiguous matches already report every location. What is left is the stale-content case, where a line number alone is not enough to act on.
Change
describeMissing(packages/code/src/edits.ts) now also finds the region the edit most likely meant:old_textline (at least two consecutive letters, digits or_, so}and);never anchor) votes for the start line at which it would fall into place, compared without surrounding whitespace. The start with the most votes (ties: earliest) wins if at least two lines agree. One pass over the file; at most 200,000 votes.The reason then ends with one more hint:
<line>|<mark><text>. The mark compares the file line with theold_textline at the same position: a space if identical,~if only whitespace differs,!if the text differs.{error, code}body, and LibreChat reads that body through a 4,096-byte buffer. Sizes are therefore counted as UTF-8 bytes of the JSON-encoded string. Each excerpt is capped at 1,600 bytes, dropping trailing rows to fit.JSON.stringifyleaves raw. Each batch edit therefore stays on one line for hosts that parse the message line by line.read_file/preview_editstill get the generic "source diagnostics require read access" settlement fromworker.ts.The README's
EDIT_CONFLICTparagraph documents the hint grammar.Testing
packages/code:npm run build;node --test dist/edits.test.js dist/workspace.test.js dist/workspace-worker.test.js dist/protocol.test.jspasses (246 tests, 1 skipped). Newedits.test.tscoverage:old_text;old_textstarts when it opens with blank lines;