Skip to content

fix(OMN-20291): code-edit loop stops re-reading, keeps its own absolute paths, applies indent-slipped edits, caps bulk edits by files changed - #3285

Merged
jonahgabriel merged 3 commits into
devfrom
jonah/omn-20291-code-edit-loop-fixes
Oct 3, 2026
Merged

jonahgabriel merged 3 commits into
devfrom
jonah/omn-20291-code-edit-loop-fixes

Conversation

@jonahgabriel

@jonahgabriel jonahgabriel commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Ticket: OMN-20291

Lab: hosts h201 and h202, model turns on the .201 dev lane (Qwen3.8-27B). The OMN-20291 comparison's own run2_<id>.sh replay scripts were run with this branch first on PYTHONPATH, twice per task. Results are in the table below. The comparison's original outputs were set aside and restored afterwards.

Summary

The OMN-20291 comparison replayed 26 crush tasks through the delegated code-edit node. The tool_use rubric passed 18 node runs and 17 crush runs, and the node failed 8 tasks. Reading each failure's loop receipt and turn replies turned up four causes in the node itself. This PR fixes all four:

cause replay evidence change
Read loop a16a3138: 40 turns, 132 views, 0 edits. e3a2c923: 40 turns, 215 views, 1 edit. From turn 3 on, every prompt sat at the 90,068-character cap and its history held only the last 1 to 3 turns. One turn's reads (8 to 12 views of up to 16 KB) were larger than the whole history, so even the newest turn was cut. Every earlier turn keeps its action lines. The newest turns keep their output, a re-viewed window points at the later turn, and a view of a file changed later is marked stale. One turn's reads total at most 30,000 characters. After 3 turns that read and change no file, the next turn's reads are refused, and its prompt says so.
Absolute path inside the worktree refused 58744096 t6a1 grep path=/data/.../wt/58744096/omnimarket/src was refused with leaves the worktree. The model copied the path from the loop's own echo of the check command ($ env PYTHONPATH=/data/.../src ...). Such a path is used and recorded as its worktree-relative form. Any other absolute path is still refused.
Edit anchor indentation slip ea3ee264 t3a7 and t5a1: the view prints 96| text, and the model copied the separator's blank into every line. In re-run 2 it did so on the first line only (' def umbrella_tickets('). t7a2 re-sent an edit it had applied in turn 3. An old_string that matches exactly one place up to that shift is applied, and new_string moves back the same way. An edit whose result is already in the file is reported applied and writes nothing, so a re-sent insertion is not inserted twice.
Bulk-edit cap counted the glob (#3270) In the #3270 AC2 replay (loop 7ea45ffe), replace_in_files glob=src/omnimarket/nodes/*/contract.yaml was refused with targets over 200 files. The model fell back to write /tmp/... and 117 single edits, which is the 8e1b5f72 failure. The cap counts the files the glob would edit, meaning those that hold old_string.

Two parts of the history change depend on the code that #3276 and #3270 brought to dev. History turns are now structured, so the resume block from #3276 stores them as JSON. replace_in_files counts as a file change for the read-pause streak.

How the read-loop fix got here

Each step was driven by a lab re-run:

  1. History rendering alone. e3a2c923 was still only viewing at turn 30, because the newest turn's own reads overflowed the history.
  2. History plus a per-turn read bound. The model kept choosing to read until the turn cap.
  3. History, read bound and read pause. a16a3138 completed: accepted in 29 turns, every declared check passed, against 40 turns and no edits before. e3a2c923 completed: accepted in 38 turns with both checks passing, against budget_exhausted before.

Both tasks still exceed the rubric's 80-tool-call budget. Crush finishes them in 8 and 5 turns. The remaining gap is prompt capacity: onex delegate takes the prompt as one argv word, which caps it at 120 KB. That is a separate omnibase_infra change, and the report describes it.

Tests: RED, then GREEN

The focused tests are all in tests/nodes/node_delegated_code_edit_orchestrator/test_handler_delegated_code_edit_orchestrator.py. Each group failed before its change:

  • History, absolute path and anchors: 8 failed. The uniqueness guard already passed.
  • Read bound: 2 failed.
  • Read pause: 2 failed.
  • First-line anchor slip: 1 failed.
  • Glob cap: 1 failed.

The resume tests from #3276 that asserted string history blocks now assert structured entries. The cut-not-dropped history test now builds HistoryTurns.

GREEN, focused only: uv run pytest tests/nodes/node_delegated_code_edit_orchestrator/ tests/unit/delegated_code_edit tests/unit/cli/test_cli_code_edit.py tests/nodes/node_delegation_rubric_check_compute -q gave 504 passed. ruff format, ruff check and mypy --strict on the node and delegated_code_edit are clean. pre-commit passed on the changed files.

ea3ee264 re-run 2's turn-3 replies, replayed offline against the base file, apply all nine edits with the file's indentation.

Lab re-runs

The code under test is e793efaee. The PR head 82534fca4 is that code plus dev, merged in by the OCC bot, and the ticket contract. Each task ran twice through its own run2_<id>.sh. Loop ids are in brackets.

task class node before fixed, run 1 fixed, run 2
ea3ee264 edit anchors FAIL edits_apply PASS, 7 turns / 404 s (00979abd) PASS, 10 / 390 s (cd48d7c5)
58744096 absolute path FAIL no_phantom_paths PASS, 6 / 162 s (068a6dff) PASS, 7 / 194 s (0fbff7d4)
a16a3138 read loop budget_exhausted at 40 turns, 0 edits accepted, all checks pass; FAIL within_budget (90 calls > 80), 22 / 864 s (a88d8355) PASS, 18 / 801 s (aa8c5957)
e3a2c923 read loop budget_exhausted at 40 turns, 1 edit budget_exhausted at 40 turns, 33 edits (6df81ab0) void: the dev lane failed the delegate twice in a row at turns 23, 37 and 38 (dc0909e6, d36c82e6, 0315be42)
8e1b5f72 bulk-edit cap FAIL, 3 criteria, 23 turns accepted, both checks pass; FAIL edits_apply (a named-list replace_in_files with files lacking old_string), 7 / 394 s (a37e15a5) no_progress; a glob reached files outside the task's list (69210626)
f76301f0 bulk-edit cap FAIL, 3 criteria, 15 turns accepted, both checks pass; FAIL on one refused helper-script write, 5 / 454 s (c7f97755) no_progress; a glob changed 5 contracts outside the list (dbd4ab25)
  • Fixed: edit anchors and absolute paths, with 4 of 4 runs passing.
  • Read loop: a16a3138 now completes, and passed the rubric once. e3a2c923 now edits but still runs out of turns.
  • Bulk class: replace_in_files runs now. Its remaining failures come from how the model uses it, and the report covers them.
  • Earlier commits on this branch: their runs (58744096 6/6 PASS, ea3ee264 3/4 PASS) are in the report.

Report with the full 26-task table and every failure classified: knowledge-base-internal reports/2026-10-03-code-edit-node-vs-crush-replay.md (knowledge-base-internal#986).

Evidence-Ticket: OMN-20291
Evidence-Source: OCC#12497

…te paths, applies indent-slipped edits, caps bulk edits by files changed

The OMN-20291 replay of 26 crush tasks left the node failing 8. Four
node-side causes are fixed here:

- Read loop (a16a3138, e3a2c923: 40 turns, 0 and 1 edits). The prompt sat at
  its 90k-character cap and held only the last one to three turns, and one
  turn's reads (8 to 12 views of up to 16 KB) exceeded the whole history, so
  even the newest turn was cut. Now every earlier turn keeps its action
  lines, the newest turns keep their output, a re-viewed window points at the
  later turn, a view of a file changed later is marked stale, one turn's
  reads total at most 30,000 characters, and after three turns that read and
  change no file the next turn's reads are refused, saying so in its prompt.
  History turns are structured, and the resume block (OMN-20391) stores them
  as JSON.
- Absolute paths inside the worktree (58744096). The loop prints check argv
  with the worktree's absolute PYTHONPATH; the model searched that path and
  was refused as leaving the worktree. Such a path is now used and recorded
  as its worktree-relative form; any other absolute path is still refused.
- Edit anchors (ea3ee264). The view prints '   96|   text'; the model copied
  the separator's blank into old_string, on every line or on the first line
  only. An old_string that matches exactly one place up to that shift is
  applied, with new_string moved back the same way. An edit whose result is
  already in the file is reported applied and writes nothing.
- Bulk edit cap (8e1b5f72, OMN-20392 AC2 loop 7ea45ffe). replace_in_files
  refused a glob matching more node contracts than MAX_BULK_FILES before
  skipping the files without old_string. The cap now counts the files the
  glob would edit.

Onex-Lane: code-edit-node-compare
Onex-Session: 4380aed584814eab8ef70338fafb073d
@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown

✅ Architectural Review — PASSED

Errors: 0
Warnings: 0

What this checks

Rule Description
ARCH-TOPIC-001 No hardcoded Kafka topic strings in handler code
ARCH-DI-001 No event_bus=None bypass
ARCH-DI-002 No direct Handler instantiation outside workflow_runner/adapters
ARCH-DI-003 No reinvented DI containers
ARCH-TOPIC-002 contract.yaml topics follow `onex.{cmd

No architectural violations found.

Static architectural lint — no model inference (OMN-14176).

@github-actions

github-actions Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

✅ Hostile Reviewer — PASSED

Blocking findings (critical/error): 0
Total findings: 10
Models succeeded: qwen3-review,gpt-oss-review


Gate semantics

Verdict Meaning Blocks merge?
passed >=2 models succeeded, no critical/error findings No
blocked CRITICAL or ERROR findings found Yes
degraded Fewer than 2 models succeeded (infra unavailable/timeout) Yes (OMN-15110)

Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524)

#: Characters the reads (view, grep, ls) of one turn may show together. The
#: history holds about 66,000 characters, so a turn's reads fit it whole with
#: room for the turn before (OMN-20291).
MAX_READ_CHARS_PER_TURN = 30_000
#: room for the turn before (OMN-20291).
MAX_READ_CHARS_PER_TURN = 30_000
#: Below this many characters left, a turn's further reads are not run.
MIN_READ_CHARS = 2_000
#: Turns in a row that read and change no file before the next turn's reads are
#: refused, so a model that cannot hold every file it wants to read writes with
#: what it has instead of reading to the turn cap (OMN-20291).
MAX_READ_ONLY_TURNS = 3
…o the pull request's tests

Onex-Lane: code-edit-node-compare
Onex-Session: 4380aed584814eab8ef70338fafb073d
onexbot-occ-writer Bot added a commit to OmniNode-ai/onex_change_control that referenced this pull request Oct 3, 2026
…nimarket#3285

The declared checks were executed for real in the OmniNode-ai/omnimarket checkout at the PR head by the product-repo OCC receipt runner, and the results written append-only into this companion. Producer-minted PENDING receipts are superseded by a net-new record; no existing receipt was edited.

Ticket: OMN-16859 (AC3b)
Onex-Workflow: OCC Receipt Runner
Onex-Run: https://github.com/OmniNode-ai/omnimarket/actions/runs/37098457184
@jonahgabriel
jonahgabriel enabled auto-merge (squash) October 3, 2026 05:11
jonahgabriel pushed a commit to OmniNode-ai/onex_change_control that referenced this pull request Oct 3, 2026
)

* evidence(OMN-20291): author OCC companion for OmniNode-ai/omnimarket#3285

OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head e793efaeef65d40cd00cec7afcc2ffee5cb74b17.

* evidence(OMN-20291): self-bind OCC#12497 + rebind contract_sha256

* evidence(OMN-16859): executed test_passes receipts for OmniNode-ai/omnimarket#3285

The declared checks were executed for real in the OmniNode-ai/omnimarket checkout at the PR head by the product-repo OCC receipt runner, and the results written append-only into this companion. Producer-minted PENDING receipts are superseded by a net-new record; no existing receipt was edited.

Ticket: OMN-16859 (AC3b)
Onex-Workflow: OCC Receipt Runner
Onex-Run: https://github.com/OmniNode-ai/omnimarket/actions/runs/37098457184

---------

Co-authored-by: omnimarket-bot <bot@omninode.ai>
Co-authored-by: onexbot-occ-writer[bot] <307849072+onexbot-occ-writer[bot]@users.noreply.github.com>
@jonahgabriel
jonahgabriel merged commit 117b144 into dev Oct 3, 2026
193 of 223 checks passed
@jonahgabriel
jonahgabriel deleted the jonah/omn-20291-code-edit-loop-fixes branch October 3, 2026 06:09
jonahgabriel added a commit that referenced this pull request Oct 3, 2026
…ests already on dev

dod-verify failed with 'bound test also passes at the merge base' for the
three AC1-AC3 entries shipped in #3285. Drop them; bind AC5 (file list scope,
skip files without old_string, prompt rule) and AC6 (refused calls counted
separately) to this PR's new tests.
jonahgabriel added a commit that referenced this pull request Oct 3, 2026
…ip files without old_string, refused calls leave the budget (#3302)

* fix(OMN-20291): code-edit bulk edits stay inside the task's file list, skip files without old_string, and refused calls leave the budget

Comparison report knowledge-base-internal#986 left bulk edits over a named
file list failing (replay tasks 8e1b5f72 and f76301f0).

The turn prompt now says, before the task, that the task's file list is the
only edit scope and that helper scripts are not available (no shell; a task
that suggests a script is to be ignored). The model had tried a helper script
first because the task text recommended one.

The request takes a file_list (CLI: --file-list, one path per line). When it
is given, replace_in_files reaches only those files: a glob narrows the list
and never widens it, and a named file outside it is refused. Before, a glob
over all node contracts edited 141 files for a 114-file task (8e1b5f72 run 2)
and changed 5 contracts outside the 105 of f76301f0 (run 2).

A named file without old_string is skipped and reported, not a failed call.
A call where no listed file holds old_string still fails.

A refused action (a write outside the writable scope, a read while reads are
paused, an undeclared check) is recorded with refused=true. The within_budget
criterion counts the calls that ran against max_tool_calls and reports
refused_calls apart: a refused call changed and showed nothing, and the other
criteria still score it.

* fix(OMN-20291): replace_in_files skips a file the replacement was already applied to

A repeated insertion over the task's file list inserted its line again in files
that already had it (8e1b5f72, lab re-run attempt 2: the same call sent in
turns 1, 3, 9, 11 and 12, then 31 turns of clean-up). A file whose old_string
occurs only inside an earlier result of the same replacement is now skipped and
reported as already applied; a call that finds every file done is reported
unchanged, not failed.

* fix(OMN-20291): contract binds this PR's new tests, not the AC1-AC3 tests already on dev

dod-verify failed with 'bound test also passes at the merge base' for the
three AC1-AC3 entries shipped in #3285. Drop them; bind AC5 (file list scope,
skip files without old_string, prompt rule) and AC6 (refused calls counted
separately) to this PR's new tests.

* chore(OMN-20291): bump version to 0.4.284

Bump project.version in pyproject.toml and the omnimarket entry in uv.lock from 0.4.283 to 0.4.284.

This resolves a Release Identity Gate (scripts/check_release_identity.py, OMN-16344) failure on PR #3302 head ccf36b1, which reported that the packaged source changed but pyproject version 0.4.283 was not ahead of the latest published version 0.4.283. After the bump, the script confirms version 0.4.284 is ahead of latest published 0.4.283.
jonahgabriel added a commit that referenced this pull request Oct 3, 2026
…nothing, and points a missing path at the paths that end with it (#3315)

* fix(OMN-20291): code-edit prompt names writable files that do not exist yet; a no-op edit says it changes nothing

Replay tasks 0192c135 (loop df4d5ca5) and 8d6e9b45 (loop 692d96fe) failed
no_phantom_paths on omnimarket dev 32b6f90: in turn 1 the model viewed the
test file the task told it to create, before creating it. The FILES index is
cut at 12,000 characters, so on omnibase_core (7,211 files) the model cannot
see that the path is absent. The same failure hit a0cbacca before #3285. The
turn prompt now names each literal writable path missing from the worktree
as a NEW FILE until a turn has written it.

Replay ab8d7ef6 (loop 1c966a4a) sent an edit whose old_string equals its
new_string three times and was told each time it 'was applied before'. Such
an edit is now reported as changing nothing.

Onex-Lane: code-edit-regress-2a21
Onex-Session: ead7bf3424ea48e49decfc434456da87

* chore(OMN-20291): bind the new-file and no-op edit tests in the ticket contract

Onex-Lane: code-edit-regress-2a21
Onex-Session: ead7bf3424ea48e49decfc434456da87

* fix(OMN-20291): code-edit grep says a path that does not exist is missing

Replay ab8d7ef6 failed on omnimarket dev 32b6f90 twice (loops 1c966a4a and
f7c345be). Both times the model guessed the node path without the src/
prefix and grepped 'omnimarket/nodes/...' or 'omnimarket'. git grep answers a
missing pathspec with exit 1, which the port reported as "no matches"; in
loop f7c345be that happened eight times, so the model never learned the path
was wrong and never read the contract its new test asserts on. view and ls
already say a missing path is missing. grep now raises the same
WorkspacePathError, which the loop shows as "error: <path> does not exist".

Onex-Lane: code-edit-regress-2a21-mac
Onex-Session: ead7bf3424ea48e49decfc434456da87

* fix(OMN-20291): a missing view, ls or grep path names the worktree paths that end with it

Replay ab8d7ef6 on the code-edit PR head (loop 33cecc23) still spent 20
turns guessing 'omnimarket/nodes/node_navigation_history_reducer/...' in a
src-layout repo after view and ls said the path was not a file or not a
directory. A missing path now names up to three worktree paths that end
with it, shortest first, e.g. "omnimarket/nodes is not a directory; did you
mean: src/omnimarket/nodes?". A path that exists, or that nothing ends with,
gets no hint.

Onex-Lane: code-edit-regress-2a21-mac
Onex-Session: ead7bf3424ea48e49decfc434456da87
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.

2 participants