wip: ai auto feedback ai editing improvements - #5511
Open
404Wolf wants to merge 21 commits into
Open
Conversation
The judge compares each replay against its own original prod session rather than against a prose rubric, so all 428 cases are judgeable instead of only the 74 with a hand-written report; where a report exists it is passed as extra evidence. Verdict covers correctness, directness, and purpose-met on both sides, and carries the objective lint metrics alongside so reports need not trust the model's arithmetic. Also fixes a real prod bug the judge run surfaced: renderTraceMarkdown threw 'text.split is not a function' on array-valued snippets, taking down the whole trace render. Coders regularly send an array (composing a list) despite the Record<string,string> schema, and a debugging artifact has to survive whatever the model actually produced.
…rop redundant readDocument Three changes, all aimed at the measured 108x context amplification (median session burning 97k input tokens to edit a ~900-token document): 1. Anthropic cache breakpoint on each agent's opening user message. The breakpoint must sit on the content PART — message-level providerOptions are silently dropped before reaching the provider, verified by inspecting the request body (cache_creation_input_tokens was 0). Worth ~10k tokens of cache read per coder. Providers that don't understand cacheControl ignore it, so this is safe under the cerebras/openai fallbacks. 2. compactDocumentHistory: every dispatch result carries the full post-edit document so the supervisor can verify it, but once a newer result exists the older copies describe a document that no longer exists and are re-billed on every remaining step. Keeping only the newest turns O(N^2*D) into O(N*D). 3. Withhold readDocument when the coder's window already is the whole document. Instrumentation showed coders calling it anyway — a full model round trip that can only return what they were already given, and which permanently adds a duplicate copy to their history. Also accepts array-valued snippets (z.union) rather than rejecting them: coders send a string[] when composing a list, and the strict Record<string,string> forced a retry of the whole step — 45 occurrences across the prod corpus.
Parallel coders share one LexicalSession. Two dispatches naming the same node
interleave edits on the same subtree, which is the largest failure class in the
trace corpus (159 of 428 cases) and the one that yields silently corrupted
documents rather than clean errors.
Each coder now publishes the node ids it is working on before it starts, and a
new coder waits out only those in-flight coders whose targets overlap.
Independent edits still run fully in parallel, so latency is spent exactly
where the alternative is corruption. The conflicting coder also serializes the
document AFTER waiting, so its window reflects the edit it waited for rather
than a pre-conflict snapshot.
Candidate ids are filtered against ids actually present in the document, so
prose matching the id shape ('Suggested', 'constraint') can't create phantom
conflicts that serialize unrelated work.
Withholding readDocument when the coder's window already covered the whole
document looked like free savings — the call can only return what the coder was
handed. The bench says the opposite. Attributed across runs (n=38):
baseline fix2 (no readDocument) fix3 (+serialize)
coder input tokens 5.20M 7.12M 8.45M
runCode calls 129 168 218
coder retries 22 28 39
The re-read is not redundant: it is the coder's only way to observe the
document AFTER its own edits. Without that feedback it makes more blind runCode
attempts than the re-read ever cost.
Keeping the supervisor-side compaction, which is the one change that did what
it was designed to do (supervisor input 2.79M -> 2.32M).
… the snippet bugs
The supervisor chain is opus-4-8 -> sonnet-4-6 -> gpt-5.5, and endpoints/edit.ts
resolved OpenAI through the default factory, i.e. the Responses API. That API
references reasoning items across steps by id and this org has Zero Data
Retention, so the ids never persist and the call dies with
'Item with id rs_... not found. Items are not persisted for Zero Data
Retention organizations.'
The last-resort supervisor fallback was therefore broken outright — it would
fail whenever both Anthropic models were unavailable, which is exactly when it
gets used. Found via the bench, where gpt-5.5 failed all 20 cases this way.
Adds regression tests built from the snippet payloads that actually appear in
the 495-session prod corpus (all 51 non-string values: 45 arrays, 3 objects,
2 numbers, 1 bare string). Verified they fail on the pre-fix code — 4 failures
across the two files — and pass after:
- runCode's schema rejected array values, costing the coder its whole step
- renderTraceMarkdown threw 'text.split is not a function' on them, taking
down the render of the entire trace
… guessing
Root-caused the corpus's dominant defect (45% of sessions have a coder retrying
inside one dispatch) by reading the code coders actually wrote. Across 651
consecutive runCode transitions in 495 prod sessions, 37% re-touch a node the
previous call already touched:
identical call repeated insertParagraphAfter x24, remove x18,
insertHeadingAfter x17, bold x14, setText x14
same target, method swapped appendText/setText x9, replace/setText x8,
remove/setText x8, bulletList/insertBlockAfter x8
The mechanism is the reply, not the model. runCode answered with the bare string
"ok" whenever nothing threw, which cannot distinguish "changed the document"
from "ran but changed nothing" from "targeted the wrong node". The coder's only
move was to try again, usually with a different method name.
Worst observed case (trace dd3c373b): three check-list items were ALREADY
unchecked, and the coder spent six calls oscillating between uncheck() and
setChecked(false), getting "ok" every time.
runCode now diffs the document around the ops and reports the observed effect:
"CHANGED -- modified/added/removed <ids>", or an explicit "NO CHANGE" telling
the coder the method was not the problem and not to retry. CODER.md documents
both replies.
The trace stored the code a coder wrote but not what it was told back, which is exactly the signal needed to explain a retry. Diagnosing the thrashing meant inferring intent from successive code blocks; now CoderRunCode carries the reply verbatim.
…y doing nothing
This is the cause behind the corpus's dominant defect, found by capturing what
coders are actually told back.
replace/bold/italic/inlineCode/link/mark all locate their target by substring.
Their match engines return a count; every caller in doc.ts discarded it. A miss
therefore applied nothing and reported success. Those methods account for ~2,600
calls across the 495-session corpus.
Trace 213282e2 shows the consequence: six consecutive calls, every one reporting
success, none doing anything --
editor.replace('_UgddKRe', '408', '414.8')
editor.replace('_UgddKRe', '/ 408 s NDS', '/ 414.8 s NDS')
editor.replace('_UgddKRe', '<the entire sentence>', '...')
The needle straddled a text-run boundary and the matchers work per run, so it
could never match. The coder debugged correctly -- narrowing from a substring to
the node's whole text -- and learned nothing from any attempt.
Each op now raises, distinguishing the two real causes:
- the text is absent: quotes the block's actual content
- the text is present but SPLIT ACROSS runs: quotes the individual runs so the
boundary is visible, and points at setText as the way out
Also adds the earlier effect report (CHANGED / NO CHANGE) which made this
diagnosable: measured on a self-correction-only sweep, 8 of 9 NO CHANGE replies
were followed by another retry, i.e. telling the model not to retry does not
work -- the underlying op has to fail loudly instead.
…ross runs Two more individual defects, taken from the specific issue statements in 279 judge verdicts (376 statements; largest clusters: 77 structure-wrong, 68 lost-content, 49 formatting-not-applied). 1. setText silently destroys inline content. It is the most-used editor method in the corpus (1,409 calls) and flattens a block to one plain run: every bold/code span, link, mention and line break in it is deleted and formatting on the survivor stripped. Nothing told the coder, and the prompt documented it as a plain "set the text" op. This is the mechanism behind judged issues like "a heading lost its bold formatting" and "the mention disappeared". It now returns what it removed and Doc surfaces that as a WARNING on the runCode reply, so the loss is visible and recoverable. 2. replace could not match across text runs. The matchers walked one run at a time, so a needle interrupted by a bold or code span matched nothing -- silently, before this branch. That pushed coders onto setText, destroying the very formatting that caused the miss. $replaceString now matches on the block's flattened text: the replacement lands in the first run it touched, inheriting that run's formatting, and the matched remainder is removed from the runs that follow. Durable ids survive. Formatting a span that straddles a boundary still raises, because there is no single sensible answer to which run's formatting should win; the error names the individual runs so the coder can pick a span inside one. Also trims two prose additions in favour of the mechanisms. The NO CHANGE reply no longer instructs the model not to retry -- that was measured ineffective (8 of 9 such replies were retried anyway) -- it just states the fact. CODER.md keeps one line describing the reply contract instead of a paragraph.
…Text warning Generalises the substring matcher rather than special-casing. mutateMatches now locates the needle in the block's FLATTENED text and splits every run the match overlaps, applying to each segment -- the same result as selecting across a bold boundary in the editor and applying the format. That removes the last class of silently-impossible calls: inlineCode/bold over a span that already contains a formatted sub-span, which is exactly the defect the original hand-written report hypothesised about function names being split. Residual split-run errors in the previous run were 7x code, 1x bold, 1x replace; this addresses all of them at the mechanism level. Also reverts the setText destruction warning on measured evidence: on the 7 cases where it fired it drove runCode calls 68 -> 99 while purpose-met (6/7) and correctness (6/7) stayed flat, i.e. it provoked repair attempts that did not land. $setText still returns what it removed and the API prompt states the semantics in one line; cross-run replace is the non-destructive alternative.
Both sounded like improvements and both measured worse. Progression on the
self-correction population (n=10), each column adding one change:
effect report +substr errors +setText warn +cross-run
purpose met 7/10 9/10 7/10 8/10
fully correct 7/10 9/10 7/10 7/10
runCode calls 55 22 27 37
coders retried 7 3 4 6
cost $12.78 $7.11 $8.03 $8.14
The optimum is the substring-miss error on its own.
Cross-run matching did eliminate the split-run error class (9 -> 0 on the
stratified sweep) but cost more than it saved: letting a formatting op succeed
across a boundary produces structure the coder did not intend, and it then tries
to repair that. Erroring makes it pick a working approach immediately. The
motivating case (213282e2, the VAT total) is fixed by the error alone -- it went
partial -> correct in that configuration.
The stratified sweep agreed: cross-run took one case from correct to damaging
(0527e62e, 30 runCode calls and 14 dispatch rounds against 2 in the original),
and pushed runCode calls +17% and dispatch rounds +20% versus baseline.
Conclusion worth keeping: making a silent failure visible is sufficient; making
an impossible operation possible is not necessary and is net harmful here.
Retains the two general pieces from those commits: the terse NO CHANGE reply
(the exhortation not to retry was measured ineffective) and the one-line
setText/replace semantics in API_COMPLETE.md.
runCode executes arbitrary JS against `editor`, so any edit -- however many nodes it touches -- can be expressed in ONE call. Extra steps are therefore either error recovery or the coder voluntarily splitting one edit across round trips. Instrumented across four runs: 33% of coders made more than one runCode call, and of those continuations 74 followed a SUCCESSFUL call against 82 after an error. So roughly half the thrashing is not recovery at all -- it is pure round-trip waste, another full model turn plus its accumulated history each time. The cap drops from 7 to 3 (one attempt, two recoveries), configurable end to end so it can be A/B'd from the bench. CODER.md states the matching general rule -- do the whole task in one call -- rather than any per-case guidance. Also adds a gpt-coder preset (prod supervisor, gpt-5.5 coder) to test the model sweep's strongest single lever in combination with these fixes.
…ng inside it Diagnosed from the 74 hand-written failure reports, which were structured into 262 defects (186 corrupting or wrong-output). Ten of them trace to editor.link. Reproduced verbatim: editor.link(p, 'docs', 'https://new.test') on go to [docs](https://old.test) <a href="https://old.test"> <a href="https://new.test"><t>docs</t></a> </a> A malformed nested anchor that also left the stale href in place -- and there was no way to retarget a link at all, so a coder asked to "change its URL" had no working primitive and burned extra dispatch rounds discovering that. $wrapInBlock now takes an optional ExistingWrapper: when the match already sits inside a wrapper of the kind being created, the existing one is updated rather than a new one nested inside it. Applied to link via setURL; the same hook fits mark. One claim in the reports did NOT reproduce: that link greedily swallowed the rest of the block's text (trace 90f5450c). The matcher wraps exactly the needle. Test kept as a guard.
…missed
Structures the 74 hand-written failure reports into 262 defects (186 corrupting
or wrong-output) via extract-defects.ts, then reproduces the top claims as tests
so each is confirmed or dismissed before anything is changed for it.
Confirmed and fixed in the previous commit:
- link() nests a second anchor inside an existing one and leaves the stale href
Reproduced and DISMISSED -- the reports were wrong or the behaviour is already
correct:
- link greedily swallows the rest of the block (90f5450c): it wraps exactly the
needle
- concurrent writers interleave / lose an insert (20fbd0f4): with unique refs,
which runInSandbox guarantees via nanoid, animated concurrent writers do NOT
interleave. The interleaving first observed here was caused by a shared ref
pool in the test file itself. An op-application lock added on that false
premise has been reverted -- no evidence supports it.
- <li><ul><li> cannot be un-nested (02d942d1): outdent flattens it
- snippet text leaks backslash escapes (1259cd5a): it does not
- a fabricated node id could remove the wrong node (02d942d1): rejected with the
id named, document untouched
Recorded as a real capability gap, not yet addressed:
- link() has no surface for rel/target (d8f37ff1), so they are silently omitted
…text-run id
A real bug, reachable on ordinary input, and only on the animated path -- which
is what prod runs, since the Rust caller never sets typingAnimations.
editor.setText('<t> id', 'replaced')
direct apply -> <t>replaced</t> ok
animated (prod) -> error: No node with id "..." doc: <p/> CONTENT GONE
The `retype` animator plans removeText -> typeText -> setText against the ref it
was given. When that ref is an inline text run, the delete step empties it,
Lexical drops empty text nodes, and every later step then targets an id that no
longer exists: the paragraph is left empty and the error claims the id was never
there.
Mutating ops already resolve to the enclosing block (Doc.setText via
$blockById); the animator did not. DocReader gains blockRef so the animation
plans against the same node the apply will mutate, which is the invariant that
was missing.
This matters because every text run in the XML the model reads carries a
<t id="...">, and dispatch instructions name those ids routinely -- report
034f576f has the supervisor explicitly specifying a text-node id.
Found by reproducing the concurrency claims at the scale the reports describe
(four to five writers doing setText, not two doing inserts). Three of those
claims -- an emptied sibling heading (305047cd), damage to rows a coder never
named (034f576f), mutual truncation (15c0d49a) -- turned out to be this single
non-concurrent bug, which the earlier two-writer reproduction was too weak to
surface.
…worse The argument was sound (runCode takes arbitrary JS, so any edit fits in one call) but the measurement disagrees. Over the same 40 cases, cap 7 -> cap 3: purpose met 35/40 -> 31/40 fully correct 34/40 -> 28/40 runCode calls 139 -> 118 dispatch rounds 79 -> 89 cost $36.37 -> $38.81 The work moved up a level rather than disappearing: the supervisor re-dispatched coders it had cut off, so total turns and cost rose while quality fell. A coder that needs a fourth step is better off finishing than being truncated into a fresh dispatch. The general one-call guidance in CODER.md stays; the clause promising very few retries is dropped since it is no longer true. The maxCoderSteps plumbing stays so the bench can keep A/B-ing it.
…lock
The largest remaining source of coder retries, found by classifying every retry
in the current-HEAD runs by what preceded it. Of 102 retry transitions, 26
followed "No node with id", and the raw replies show why:
editor.setText('n9qP6lV-', <user prose>) -> ok
editor.setText('eNt8jsxy', 'him') -> No node with id
editor.setText('rYT9Q8LF', '. Reaching for ') -> No node with id
CHANGED -- removed eNt8jsxy, rYT9Q8LF, IoUm7NTJ, TxiFCp9C, uSEQxHHy
Given an inline run id, setText widened to the enclosing block, and $setText
keeps only the block's FIRST text child. A coder rewriting several runs in one
paragraph therefore destroyed its own remaining targets with the first call --
and when the named run was not the first child, it destroyed that too.
setText now sets the run it is handed and leaves siblings alone, matching
appendText/replace/bold, all of which already act on the named run. A block id
still replaces the block.
The animator has to branch for the same reason: a block survives being emptied
and retyped, an inline run does not (Lexical drops empty text nodes, which is the
crash fixed in d7fcb4379). A run is therefore selected and set atomically,
trading character-by-character typing on that path for correctness. Whole-block
rewrites keep the full animation.
… node ids
Two generic reliability fixes aimed at the largest remaining retry trigger.
Classifying every retry in the current animated run: 42 retry transitions, of
which 20 followed an unknown-id error -- the top cause.
1. Cascading failures were reported as peers. A failed op leaves its ref
dangling, so every dependent op failed too and the writer saw a wall of
equally-weighted `No node with id "<nanoid>"` lines with nothing indicating
which one to act on:
editor.remove('UnOE1FwX');
const h2 = editor.insertHeadingAfter(...); <- failed
editor.insertListItemAfter('A356NFEXgoAKyjy9MLBM3', ...) <- and 8 more
error: insertListItemAfter: No node with id "A356NFEXgoAKyjy9MLBM3"
error: insertListItemAfter: No node with id "bO-T89Eb2FCcqUR6aRpNz"
...
summarize now states the root failure once and collapses the knock-on ones:
"N later ops referenced a node the failed op above would have created ...
Fix the first error; the rest are consequences of it." Dependence is tracked
through an allowlist of id-bearing op fields, so genuinely unrelated failures
stay visible as peers.
2. `unknown id "[object Object]"` -- a non-string argument was stringified into
the message, telling the writer nothing about what it had passed. requireId
now names the type and the likely cause (insert helpers return the id
directly, not an object), and rejects an empty id explicitly.
Testbench evidence, replaying the same 16 production sessions in the animated
configuration prod actually runs, both sides fingerprint-verified:
cerebras/haiku gpt-5.5
purpose met 14/16 15/16
fully correct 12/16 14/16
damaging 0/16 0/16
coders that retried 16 2 -88%
retry rate 37% 9%
runCode calls 76 28 -63%
input tokens 3.80M 1.49M -61%
wall clock (median) 35.6s 30.9s -13%
cost $20.2 $13.0 -35%
Better on every axis, and it is by far the largest single reduction in coder
thrashing measured this cycle -- larger than every harness fix combined.
haiku stays as the fallback rather than being dropped. OpenAI throttles hard
under concurrency: a bench run at 4-way parallelism timed out on 24 of 40 cases
where the identical serial run timed out on none. Worth watching real
concurrency and quota after rollout; the fallback is what absorbs it.
Requires the Chat Completions pinning already in endpoints/edit.ts -- this org
has Zero Data Retention, and the Responses API's cross-step reasoning items
break every multi-step loop without it.
…es not exist Mined from all 622 prod traces: 21 calls to 12 editor methods that do not exist, in two groups. reads getText, getNode, getBlock, readBlock, getInlineNode, lastListItem inserts insertListAfter, appendListItemAfter, insertEquationAfter The read group is the interesting one -- the editor is write-only, so a writer that wants to branch on document state has no way to do it inside a snippet and invents an accessor. Either way the reply was a bare `editor.getText is not a function`, which names the mistake but not the way out, so the next call is another guess. `editor` is now proxied in the sandbox: an unknown method throws naming the closest real methods, and points read attempts at the readDocument tool. The init snippet moves to a shared module so the wrangler and bun sandboxes cannot drift -- the bench is only meaningful while it executes what production does. Also syncs the bench `prod` chain preset with the Rust default now that gpt-5.5 leads coding, and keeps the previous chain as `legacy-prod` so older runs stay reproducible. Adds --newest N to extract-raw (top the corpus up with unseen sessions without re-downloading 40MB) and --since to run.ts (select cases created after a date), so fixes can be validated on sessions that did not inform them.
Contributor
|
Important Review skippedIgnore keyword(s) in the title. ⛔ Ignored keywords (3)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The sandbox-init and run-code tests were written against text lifted from real prod sessions, and sandbox-init borrowed its QuickJS loader from the replay harness. Both are now synthetic and standalone: no user content in the repo, and no test that only passes when the harness is checked out.
404Wolf
force-pushed
the
wolf/ai-editing-improvements-3
branch
from
August 7, 2026 20:48
4297a56 to
3a85a1b
Compare
404Wolf
marked this pull request as ready for review
August 7, 2026 21:50
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.