diff --git a/contracts/OMN-20291.yaml b/contracts/OMN-20291.yaml new file mode 100644 index 0000000000..adac22b89d --- /dev/null +++ b/contracts/OMN-20291.yaml @@ -0,0 +1,66 @@ +--- +schema_version: "1.0.0" +ticket_id: "OMN-20291" +title: "Fix the delegated code edit node's failure classes found by the crush replay comparison" +summary: "The OMN-20291 replay of 26 crush tasks left the delegated code edit node failing 8. Four causes were in the node's loop: a history that forgot what the model had read, so it re-read to the turn cap; absolute paths inside the worktree refused as leaving it; edits whose anchor carried the view separator's blank; and a replace_in_files cap that counted every file a glob matched. Each is fixed and bound to the pull request's own tests." +is_seam_ticket: false +interface_change: false +interfaces_touched: [] +requirements: + - id: "req-code-edit-replay-failure-classes" + statement: |- + The delegated code edit node must not lose the turns a model has read, must use an absolute path inside its own worktree, must apply an edit whose only difference is the view separator's indentation, and must cap a bulk edit by the files it would change. + acceptance: + - id: "AC1" + statement: |- + AC1: every earlier turn stays in the prompt with its action lines, a re-viewed window is not repeated, a view of a file changed later is marked stale, one turn's reads are bounded so the newest turn is never cut, and reads pause after three turns that read and change no file -- falsifier: uv run pytest tests/nodes/node_delegated_code_edit_orchestrator/test_handler_delegated_code_edit_orchestrator.py -q -k "earlier_turn or stale or reads_are_bounded or shrinks or reads_pause or paused_turn or cut_not_dropped or resume" + - id: "AC2" + statement: |- + AC2: an absolute path inside the worktree is applied and recorded as its worktree-relative form, and any other absolute path is still refused -- falsifier: uv run pytest tests/nodes/node_delegated_code_edit_orchestrator/test_handler_delegated_code_edit_orchestrator.py -q -k "absolute_path_inside or normalise_path_maps or path_rules" + - id: "AC3" + statement: |- + AC3: an edit whose old_string matches exactly one place up to a uniform indentation shift, or a shift of its first line alone, is applied with new_string moved back; an ambiguous match still fails; an edit already applied is reported applied and not applied twice -- falsifier: uv run pytest tests/nodes/node_delegated_code_edit_orchestrator/test_handler_delegated_code_edit_orchestrator.py -q -k "indentation or first_line_alone or already_applied or inserted_twice" + - id: "AC4" + statement: |- + AC4: replace_in_files caps a glob by the files that hold old_string, not every file the glob matches -- falsifier: uv run pytest tests/nodes/node_delegated_code_edit_orchestrator/test_handler_delegated_code_edit_orchestrator.py -q -k "replace_in_files" +evidence_requirements: + - kind: "tests" + description: "The delegated code edit node's handler tests pass." + command: "uv run pytest tests/nodes/node_delegated_code_edit_orchestrator/test_handler_delegated_code_edit_orchestrator.py -q" +emergency_bypass: + enabled: false + justification: "" + follow_up_ticket_id: "" +dod_evidence: + - id: "dod-code-edit-replay-ac1" + description: "The history, read-bound and read-pause tests prove the model keeps what it read and is stopped from reading to the turn cap." + source: "manual" + checks: + - check_type: "test_passes" + check_value: "uv run pytest tests/nodes/node_delegated_code_edit_orchestrator/test_handler_delegated_code_edit_orchestrator.py -q -k \"earlier_turn or stale or reads_are_bounded or shrinks or reads_pause or paused_turn or cut_not_dropped or resume\"" + cwd: "${OMNI_HOME}/omnimarket" + binds_ac: ["AC1"] + - id: "dod-code-edit-replay-ac2" + description: "The path tests prove an absolute in-worktree path is used as its relative form and any other is refused." + source: "manual" + checks: + - check_type: "test_passes" + check_value: "uv run pytest tests/nodes/node_delegated_code_edit_orchestrator/test_handler_delegated_code_edit_orchestrator.py -q -k \"absolute_path_inside or normalise_path_maps or path_rules\"" + cwd: "${OMNI_HOME}/omnimarket" + binds_ac: ["AC2"] + - id: "dod-code-edit-replay-ac3" + description: "The anchor tests prove indentation-shifted and already-applied edits are handled and ambiguous ones still fail." + source: "manual" + checks: + - check_type: "test_passes" + check_value: "uv run pytest tests/nodes/node_delegated_code_edit_orchestrator/test_handler_delegated_code_edit_orchestrator.py -q -k \"indentation or first_line_alone or already_applied or inserted_twice\"" + cwd: "${OMNI_HOME}/omnimarket" + binds_ac: ["AC3"] + - id: "dod-code-edit-replay-ac4" + description: "The replace_in_files tests prove the cap counts the files a glob would edit." + source: "manual" + checks: + - check_type: "test_passes" + check_value: "uv run pytest tests/nodes/node_delegated_code_edit_orchestrator/test_handler_delegated_code_edit_orchestrator.py -q -k \"replace_in_files\"" + cwd: "${OMNI_HOME}/omnimarket" + binds_ac: ["AC4"] diff --git a/src/omnimarket/nodes/node_delegated_code_edit_orchestrator/__init__.py b/src/omnimarket/nodes/node_delegated_code_edit_orchestrator/__init__.py index f6284626cd..0a945b0c23 100644 --- a/src/omnimarket/nodes/node_delegated_code_edit_orchestrator/__init__.py +++ b/src/omnimarket/nodes/node_delegated_code_edit_orchestrator/__init__.py @@ -18,8 +18,11 @@ from omnimarket.nodes.node_delegated_code_edit_orchestrator.handlers.turn_protocol import ( RESPONSE_CONTRACT, TOOL_SCHEMAS, + HistoryAction, + HistoryTurn, build_turn_prompt, parse_turn_reply, + render_history, ) from omnimarket.nodes.node_delegated_code_edit_orchestrator.models.model_delegated_code_edit import ( MAX_ERROR_CHARS, @@ -53,6 +56,8 @@ class NodeDelegatedCodeEditOrchestrator(HandlerDelegatedCodeEditOrchestrator): "EnumCodeEditStatus", "EnumCodeEditTool", "HandlerDelegatedCodeEditOrchestrator", + "HistoryAction", + "HistoryTurn", "LoopReceiptExistsError", "ModelCheckResult", "ModelCodeEditAction", @@ -69,5 +74,6 @@ class NodeDelegatedCodeEditOrchestrator(HandlerDelegatedCodeEditOrchestrator): "build_turn_prompt", "normalise_path", "parse_turn_reply", + "render_history", "writable", ] diff --git a/src/omnimarket/nodes/node_delegated_code_edit_orchestrator/handlers/handler_delegated_code_edit_orchestrator.py b/src/omnimarket/nodes/node_delegated_code_edit_orchestrator/handlers/handler_delegated_code_edit_orchestrator.py index 8c97bf468c..777cd3cfea 100644 --- a/src/omnimarket/nodes/node_delegated_code_edit_orchestrator/handlers/handler_delegated_code_edit_orchestrator.py +++ b/src/omnimarket/nodes/node_delegated_code_edit_orchestrator/handlers/handler_delegated_code_edit_orchestrator.py @@ -11,9 +11,26 @@ CLAIM -> [TURN -> APPLY(actions) -> (FINISH -> CHECKS)]* -> DIFF -> SCORE -> RECEIPT * At most ``max_turns`` turns (the tool_use rubric's budget caps it at 40). -* Writes are confined: a write, edit or replace_in_files whose path is absolute, - climbs out with ``..``, touches ``.git``, or matches no writable glob is - refused and the refusal is fed back. Nothing is written for it. +* One turn's reads (view, grep, ls) show at most ``MAX_READ_CHARS_PER_TURN`` + characters together: a view is shortened to what is left, and a read with + less than ``MIN_READ_CHARS`` left is not run and says so. A turn's reads then + fit the history whole (OMN-20291). +* After ``MAX_READ_ONLY_TURNS`` turns in a row that read and change no file, + the next turn's prompt says its reads are refused, and they are: it may + write, edit, format, run checks or finish. A file change ends the streak; a + refused turn that still changes nothing allows one more reading turn. +* Writes are confined: a write, edit or replace_in_files whose path leaves the + worktree (an absolute path outside it, or one that climbs out with ``..``), + touches ``.git``, or matches no writable glob is refused and the refusal is + fed back. Nothing is written for it. An absolute path inside the worktree is + used, and recorded, as its worktree-relative form: the loop itself prints + such paths in check commands (OMN-20291). +* An edit whose old_string is absent is retried once with each line's leading + and trailing whitespace free, and applied only when that finds exactly one + place whose lines after the first all moved by the same indentation (the + first may move on its own); new_string moves back the same way. An edit + whose result is already in the file is reported applied and writes nothing + (OMN-20291). * ``run_check`` runs only a check the request declares, by name; an undeclared name is refused. There is no shell tool. * ``finish`` runs every declared check. All passing ends the loop @@ -48,14 +65,19 @@ from omnimarket.nodes.node_delegated_code_edit_orchestrator.handlers.turn_protocol import ( RESPONSE_CONTRACT, TOOL_SCHEMAS, + HistoryAction, + HistoryTurn, build_turn_prompt, ) from omnimarket.nodes.node_delegated_code_edit_orchestrator.models.model_delegated_code_edit import ( MAX_BULK_FILES, MAX_OBSERVATION_BYTES, + MAX_READ_CHARS_PER_TURN, + MAX_READ_ONLY_TURNS, MAX_VIEW_BYTES, MAX_VIEW_WINDOW_BYTES, MAX_WRITE_BYTES, + MIN_READ_CHARS, VIEW_WINDOW_LINES, WRITING_TOOLS, EnumCodeEditStatus, @@ -81,10 +103,24 @@ PROMPT_BYTES = 120_000 -def normalise_path(path: str) -> str | None: - """The worktree-relative form of ``path``, or None when it leaves the worktree.""" - if not path or path.startswith("/") or "\x00" in path: +def normalise_path(path: str, *, root: str = "") -> str | None: + """The worktree-relative form of ``path``, or None when it leaves the worktree. + + An absolute path is accepted only under ``root``, the worktree's absolute + path, and comes back relative to it. + """ + if not path or "\x00" in path: return None + if path.startswith("/"): + base = posixpath.normpath(root) if root.startswith("/") else "" + norm = posixpath.normpath(path) + if not base or base == "/": + return None + if norm == base: + return "." + if not norm.startswith(base + "/"): + return None + path = norm[len(base) + 1 :] norm = posixpath.normpath(path) if norm == ".." or norm.startswith("../"): return None @@ -153,8 +189,11 @@ def _cap(text: str, limit: int = MAX_OBSERVATION_BYTES) -> str: _LINE_PREFIX = re.compile(r"^ *\d+\| ?", re.M) -def view_window(text: str, path: str, offset: int) -> str: - """One page of a file with line numbers, and how to see the rest.""" +def view_window( + text: str, path: str, offset: int, *, limit: int = MAX_VIEW_WINDOW_BYTES +) -> str: + """One page of a file with line numbers, at most ``limit`` characters of + rows, and how to see the rest.""" lines = text.splitlines() total = len(lines) start = max(offset, 1) @@ -165,7 +204,7 @@ def view_window(text: str, path: str, offset: int) -> str: end = start - 1 for number in range(start, min(total, start - 1 + VIEW_WINDOW_LINES) + 1): row = f"{number:>5}| {lines[number - 1]}" - if size + len(row) + 1 > MAX_VIEW_WINDOW_BYTES: + if size + len(row) + 1 > min(limit, MAX_VIEW_WINDOW_BYTES): break shown.append(row) size += len(row) + 1 @@ -189,6 +228,91 @@ def _edit_hint(current: str, old: str) -> str: ) +_READING_TOOLS = frozenset( + {EnumCodeEditTool.VIEW, EnumCodeEditTool.GREP, EnumCodeEditTool.LS} +) + + +def _lead(text: str) -> str: + """The blanks a line starts with.""" + return text[: len(text) - len(text.lstrip(" \t"))] + + +def _loose_pattern(old: str) -> re.Pattern[str] | None: + """old_string with each line's leading and trailing blanks free.""" + lines = old.split("\n") + if not any(line.strip() for line in lines): + return None + parts = [r"[ \t]*" + re.escape(line.strip(" \t")) for line in lines] + if not lines[-1].strip(): + # old ends with a newline: the next line's indentation is not part of it. + parts[-1] = "" + return re.compile(r"[ \t]*\n".join(parts)) + + +def _shift(line: str, by: int) -> str | None: + """``line`` moved left by ``by`` spaces (right when negative), or None when + it does not start with the blanks to remove.""" + if not line.strip() or by == 0: + return line + if by > 0: + return line[by:] if line.startswith(" " * by) else None + return " " * -by + line + + +def _indent_shift_edit(current: str, old: str, new: str) -> str | None: + """The file with ``old`` replaced by ``new`` when ``old`` matches exactly one + place up to indentation, and its lines after the first all moved by the same + number of spaces (the first line may have moved on its own); ``new`` is moved + back the same way. Otherwise None. + + A local model copies view rows `` 96| text`` with one blank too many or + too few, on every line or on the first line only; the content is right, + only the indentation shifted. + """ + pattern = _loose_pattern(old) + if pattern is None: + return None + matches = list(pattern.finditer(current)) + if len(matches) != 1: + return None + match = matches[0] + found = current[match.start() : match.end()].split("\n") + wanted = old.split("\n") + at_line_start = match.start() == 0 or current[match.start() - 1] == "\n" + first: int | None = None + rest: set[int] = set() + for number, (have, want) in enumerate(zip(found, wanted, strict=True)): + if not want.strip() or (number == 0 and not at_line_start): + continue + have_lead, want_lead = _lead(have), _lead(want) + if "\t" in have_lead + want_lead: + if have_lead != want_lead: + return None + shift = 0 + else: + shift = len(want_lead) - len(have_lead) + if number == 0: + first = shift + else: + rest.add(shift) + if len(rest) > 1: + return None + later = next(iter(rest)) if rest else (first or 0) + moved: list[str] = [] + for number, line in enumerate(new.split("\n")): + shifted = _shift(line, first if number == 0 and first is not None else later) + if shifted is None: + return None + moved.append(shifted) + replacement = "\n".join(moved) + if not at_line_start or not old[:1].isspace(): + # The match swallowed the file's own leading blanks of a line old did + # not indent; keep them. + replacement = _lead(current[match.start() :]) + replacement.lstrip(" \t") + return current[: match.start()] + replacement + current[match.end() :] + + @dataclass class _Call: """One applied action, as the transcript records it.""" @@ -205,7 +329,7 @@ class _State: paths: tuple[str, ...] = () replies: list[ModelTurnReply] = field(default_factory=list) calls: list[_Call] = field(default_factory=list) - history: list[str] = field(default_factory=list) + history: list[HistoryTurn] = field(default_factory=list) turns: list[dict[str, object]] = field(default_factory=list) refusals: int = 0 last_checks: tuple[ModelCheckResult, ...] = () @@ -215,6 +339,7 @@ class _State: good_history_len: int = 0 superseded: list[dict[str, object]] = field(default_factory=list) resumes: int = 0 + read_only_streak: int = 0 def record_good(self, turn: int) -> None: """Snapshot the history boundary after a received reply was processed.""" @@ -386,7 +511,10 @@ def _seed(receipt: dict[str, object]) -> _State: result = cast(dict[str, object], receipt["result"]) state = _State( turns=[t for t in turns if cast(int, t["turn"]) <= good_turn], - history=list(cast(list[str], block["history"])), + history=[ + HistoryTurn.from_json(entry) + for entry in cast(list[object], block["history"]) + ], refusals=cast(int, result["refusals"]), summary=cast(str, result["summary"]), last_checks=tuple( @@ -462,14 +590,16 @@ def _resume_block( f"{len(state.turns)} turns used of {request.max_turns}; " "only a delegate_failed loop resumes" ) - history: list[str] = [] + history: list[dict[str, object]] = [] size = 0 if resumable: - for block in reversed(state.history[: state.good_history_len]): - if size + len(block) > 120_000: + for entry in reversed(state.history[: state.good_history_len]): + kept = entry.to_json() + length = len(json.dumps(kept)) + if size + length > 120_000: break - history.append(block) - size += len(block) + history.append(kept) + size += length history.reverse() return { "resumable": resumable, @@ -515,7 +645,14 @@ def _turns( index = relevant_first(request, [path for path, _ in manifest]) failed_delegates = 0 for turn in range(start_turn, request.max_turns + 1): - prompt = self._prompt(request, index, context, state, turn) + prompt = self._prompt( + request, + index, + context, + state, + turn, + reads_paused=state.read_only_streak >= MAX_READ_ONLY_TURNS, + ) reply = self._ports.delegate(request, prompt, RESPONSE_CONTRACT, turn) state.replies.append(reply) record: dict[str, object] = { @@ -533,7 +670,11 @@ def _turns( if not reply.ok and not reply.raw_text: failed_delegates += 1 state.history.append( - f"TURN {turn}: the delegate run failed: {reply.invalid_reason[:300]}\n" + HistoryTurn( + turn, + message=f"TURN {turn}: the delegate run failed: " + f"{reply.invalid_reason[:300]}\n", + ) ) if failed_delegates >= 2: raise _TerminalError( @@ -547,8 +688,11 @@ def _turns( if not reply.ok or not reply.actions: reason = reply.invalid_reason or "the reply carried no actions" state.history.append( - f"TURN {turn}: your reply was unusable: {reason[:300]}. " - "Reply with one JSON object with an actions list.\n" + HistoryTurn( + turn, + message=f"TURN {turn}: your reply was unusable: {reason[:300]}. " + "Reply with one JSON object with an actions list.\n", + ) ) state.record_good(turn) continue @@ -560,11 +704,14 @@ def _turns( if all(c.status == "passed" for c in checks): return EnumCodeEditStatus.ACCEPTED state.history.append( - "FINISH REFUSED: these checks did not pass:\n" - + "".join( - f"- {c.name}: {c.status}\n{_cap(c.output_tail, 2_000)}\n" - for c in checks - if c.status != "passed" + HistoryTurn( + turn, + message="FINISH REFUSED: these checks did not pass:\n" + + "".join( + f"- {c.name}: {c.status}\n{_cap(c.output_tail, 2_000)}\n" + for c in checks + if c.status != "passed" + ), ) ) self._progress_or_stop(request, checks, state) @@ -585,6 +732,8 @@ def _prompt( context: tuple[tuple[str, str], ...], state: _State, turn: int, + *, + reads_paused: bool = False, ) -> str: """The turn prompt, shrunk until it fits one argv word.""" limit = PROMPT_CHARS @@ -596,6 +745,7 @@ def _prompt( state.history, turn, max_chars=limit, + reads_paused=reads_paused, ) if len(prompt.encode("utf-8")) <= PROMPT_BYTES or limit <= 20_000: return prompt @@ -609,18 +759,45 @@ def _apply_turn( record: dict[str, object], state: _State, ) -> bool: - block = [f"TURN {turn}\n"] + shown: list[HistoryAction] = [] finished = False actions_log: list[dict[str, object]] = [] - for number, action in enumerate(reply.actions, start=1): + read_left = MAX_READ_CHARS_PER_TURN + paused = state.read_only_streak >= MAX_READ_ONLY_TURNS + read_any = changed_any = False + for number, given in enumerate(reply.actions, start=1): + action = self._rebased(request, given) + reads = action.tool in _READING_TOOLS if action.tool == EnumCodeEditTool.FINISH: finished = True state.summary = action.summary observation = ModelObservation( ok=True, output="finish: checks run next" ) + elif reads and paused: + observation = ModelObservation( + ok=False, + output=f"not read: {state.read_only_streak} turns in a row read " + "and change no file, so this turn's reads are refused; write or " + "edit with what HISTORY shows", + ) + elif reads and read_left < MIN_READ_CHARS: + observation = ModelObservation( + ok=False, + output=f"not read: this turn's reads already show " + f"{MAX_READ_CHARS_PER_TURN - read_left} characters, the most one " + "turn may; edit what you have read, then read more next turn", + ) else: - observation = self._apply(request, action, state) + observation = self._apply(request, action, state, read_left) + if reads and observation.ok: + read_left -= len(observation.output) + read_any = read_any or reads + changed_any = changed_any or ( + observation.ok + and action.tool in WRITING_TOOLS + and not observation.output.startswith("unchanged") + ) arguments = { key: value for key, value in action.model_dump( @@ -652,19 +829,57 @@ def _apply_turn( for k, v in arguments.items() if k not in ("content", "old_string", "new_string") ) - block.append( - f"> {action.tool.value}({shown_args}) -> " - f"{'ok' if observation.ok else 'REFUSED/ERROR'}\n{observation.output}\n" + path = normalise_path(action.target) if action.target else None + shown.append( + HistoryAction( + header=f"> {action.tool.value}({shown_args}) -> " + f"{'ok' if observation.ok else 'REFUSED/ERROR'}", + output=observation.output, + view_key=(path, max(action.offset, 1)) + if observation.ok + and path is not None + and action.tool == EnumCodeEditTool.VIEW + else None, + changed_path=path + if observation.ok + and path is not None + and action.tool in WRITING_TOOLS + and not observation.output.startswith("unchanged") + else "", + ) ) record["actions"] = actions_log - state.history.append("".join(block)) + state.history.append(HistoryTurn(turn, actions=tuple(shown))) + if changed_any: + state.read_only_streak = 0 + elif paused: + # One more reading turn, then the pause again. + state.read_only_streak = MAX_READ_ONLY_TURNS - 1 + elif read_any: + state.read_only_streak += 1 return finished + @staticmethod + def _rebased( + request: ModelDelegatedCodeEditRequest, action: ModelCodeEditAction + ) -> ModelCodeEditAction: + """The action with an absolute path inside the worktree made relative, so + it is applied, shown and recorded as the path it names.""" + updates: dict[str, str] = {} + for name in ("path", "file_path"): + value = getattr(action, name) + if value.startswith("/"): + rel = normalise_path(value, root=request.workspace_root) + if rel is not None: + updates[name] = rel + return action.model_copy(update=updates) if updates else action + def _apply( self, request: ModelDelegatedCodeEditRequest, action: ModelCodeEditAction, state: _State, + read_left: int = MAX_READ_CHARS_PER_TURN, ) -> ModelObservation: tool = action.tool if tool == EnumCodeEditTool.REPLACE_IN_FILES: @@ -704,16 +919,24 @@ def _apply( if tool == EnumCodeEditTool.VIEW: text = self._ports.read_file(request, path) return ModelObservation( - ok=True, output=view_window(text, path, action.offset) + ok=True, + output=view_window(text, path, action.offset, limit=read_left), ) if tool == EnumCodeEditTool.LS: return ModelObservation( - ok=True, output=_cap(self._ports.list_dir(request, path)) + ok=True, + output=_cap( + self._ports.list_dir(request, path), + min(MAX_OBSERVATION_BYTES, read_left), + ), ) if tool == EnumCodeEditTool.GREP: return ModelObservation( ok=True, - output=_cap(self._ports.grep(request, action.pattern, path)), + output=_cap( + self._ports.grep(request, action.pattern, path), + min(MAX_OBSERVATION_BYTES, read_left), + ), ) if tool == EnumCodeEditTool.WRITE: if len(action.content.encode()) > MAX_WRITE_BYTES: @@ -738,14 +961,37 @@ def _apply( old_string = _LINE_PREFIX.sub("", old_string) new_string = _LINE_PREFIX.sub("", new_string) occurrences = current.count(old_string) - if occurrences != 1: + if ( + new_string + and current.count(new_string) == 1 + and ( + occurrences == 0 + or ( + old_string in new_string + and occurrences == new_string.count(old_string) + ) + ) + ): + # The file already holds this edit's result: a model re-sending + # an edit it applied in an earlier turn. Applying an insertion + # again would duplicate it; reporting it failed is untrue. + return ModelObservation( + ok=True, + output=f"unchanged {path}: new_string is already there once " + "(this edit was applied before)", + ) + updated: str | None = None + if occurrences == 0: + updated = _indent_shift_edit(current, old_string, new_string) + if updated is None and occurrences != 1: hint = _edit_hint(current, old_string) if occurrences == 0 else "" return ModelObservation( ok=False, output=f"edit failed: old_string occurs {occurrences} times in " f"{path}; it must occur exactly once.{hint}", ) - updated = current.replace(old_string, new_string, 1) + if updated is None: + updated = current.replace(old_string, new_string, 1) self._ports.write_file(request, path, updated) return ModelObservation(ok=True, output=f"edited {path}") except WorkspacePathError as exc: @@ -762,10 +1008,14 @@ def _replace_in_files( A glob is a scope, not a list: it expands to the manifest files that are writable, a matching file outside the writable globs is counted and skipped, and a file without old_string is not a failure. A named file - that is refused or lacks old_string is. + that is refused or lacks old_string is. The file cap counts the files a + glob would edit, those holding old_string, not every file it matches + (OMN-20291: ``nodes/*/contract.yaml`` matches more than the cap). """ by_glob = not action.file_paths outside = 0 + unmatched = 0 + scope = 0 if not by_glob: targets = list( dict.fromkeys( @@ -777,7 +1027,19 @@ def _replace_in_files( matched = sorted(path for path in state.paths if pattern.match(path)) targets = [path for path in matched if writable(request, path)] outside = len(matched) - len(targets) - if not targets or len(targets) > MAX_BULK_FILES: + scope = len(targets) + holding: list[str] = [] + for name in targets: + try: + if action.old_string not in self._ports.read_file(request, name): + unmatched += 1 + continue + except WorkspacePathError: + pass # reported per file below + holding.append(name) + if scope: + targets = holding + if (not targets and not (by_glob and scope)) or len(targets) > MAX_BULK_FILES: state.refusals += 1 return ModelObservation( ok=False, @@ -787,7 +1049,6 @@ def _replace_in_files( ) failures: list[str] = [] edited: list[str] = [] - unmatched = 0 for raw in targets: path = normalise_path(raw) if path is None: @@ -820,7 +1081,10 @@ def _replace_in_files( edited.append(f"edited {path} ({occurrences}x)") if by_glob and not edited and not failures: failures.append("FAILED glob: no matched file contains old_string") - header = f"replace_in_files: {len(edited)} edited, {len(failures)} failed of {len(targets)}" + header = ( + f"replace_in_files: {len(edited)} edited, {len(failures)} failed " + f"of {scope if by_glob else len(targets)}" + ) if by_glob: header += f"; {unmatched} without old_string, {outside} matched outside the writable globs" return ModelObservation( diff --git a/src/omnimarket/nodes/node_delegated_code_edit_orchestrator/handlers/turn_protocol.py b/src/omnimarket/nodes/node_delegated_code_edit_orchestrator/handlers/turn_protocol.py index 087f34d7be..8d09521012 100644 --- a/src/omnimarket/nodes/node_delegated_code_edit_orchestrator/handlers/turn_protocol.py +++ b/src/omnimarket/nodes/node_delegated_code_edit_orchestrator/handlers/turn_protocol.py @@ -12,6 +12,9 @@ cannot. * ``build_turn_prompt`` renders one turn's prompt from the request, the file index and the history. Same inputs, same bytes. +* ``render_history`` fits the history to its budget (OMN-20291): every earlier + turn keeps its action lines, the newest turns keep their full output, and a + view window shown again later is not repeated. """ from __future__ import annotations @@ -19,12 +22,16 @@ import json import re from collections.abc import Sequence +from dataclasses import dataclass +from typing import cast from pydantic import ValidationError from omnimarket.nodes.node_delegated_code_edit_orchestrator.models.model_delegated_code_edit import ( MAX_ACTIONS_PER_TURN, MAX_BULK_FILES, + MAX_READ_CHARS_PER_TURN, + MAX_READ_ONLY_TURNS, EnumCodeEditTool, ModelCodeEditAction, ModelDelegatedCodeEditRequest, @@ -246,16 +253,177 @@ def _cap(text: str, limit: int) -> str: return text[:limit] + f"\n... [{len(text) - limit} more characters cut]\n" +@dataclass(frozen=True) +class HistoryAction: + """One applied action as the history shows it.""" + + #: ``> view(path='a.py') -> ok`` + header: str + output: str + #: (path, first line) of a view that succeeded. + view_key: tuple[str, int] | None = None + #: The path a write, edit or format that succeeded changed. + changed_path: str = "" + + +@dataclass(frozen=True) +class HistoryTurn: + """One earlier turn: its applied actions, or one message about it (a failed + delegate run, an unusable reply, a refused finish).""" + + turn: int + actions: tuple[HistoryAction, ...] = () + message: str = "" + + def to_json(self) -> dict[str, object]: + """The turn as a loop receipt's resume block keeps it.""" + return { + "turn": self.turn, + "message": self.message, + "actions": [ + { + "header": a.header, + "output": a.output, + "view_key": list(a.view_key) if a.view_key else None, + "changed_path": a.changed_path, + } + for a in self.actions + ], + } + + @classmethod + def from_json(cls, value: object) -> HistoryTurn: + """A turn from a resume block; a plain string (a receipt written before + turns were structured) is one message.""" + if isinstance(value, str): + return cls(0, message=value) + row = cast("dict[str, object]", value) + actions: list[HistoryAction] = [] + for raw in cast("list[dict[str, object]]", row.get("actions", [])): + key = raw.get("view_key") + pair = cast("list[object]", key) if isinstance(key, list) else None + actions.append( + HistoryAction( + header=str(raw["header"]), + output=str(raw["output"]), + view_key=(str(pair[0]), int(str(pair[1]))) if pair else None, + changed_path=str(raw.get("changed_path", "")), + ) + ) + return cls( + int(str(row["turn"])), + actions=tuple(actions), + message=str(row.get("message", "")), + ) + + +#: Characters of one action's output an elided turn still shows. +_BRIEF_OUTPUT_CHARS = 160 + + +def _brief(output: str, view: bool) -> str: + first = output.split("\n", 1)[0] + whole = first == output + if view and first.startswith("[") and not whole: + return first + " [content elided to fit; view again only what you will edit]" + if len(first) > _BRIEF_OUTPUT_CHARS: + return first[:_BRIEF_OUTPUT_CHARS] + " ... [output elided to fit]" + return first if whole else first + " [output elided to fit]" + + +def _render_turns(history: Sequence[HistoryTurn]) -> list[tuple[str, str]]: + """(full, brief) renderings of each turn, oldest first. + + A view window shown again in a later turn points at that turn instead of + repeating its lines, and a view of a file a later action changed says so, + so an anchor is not copied from content that is no longer there. + """ + shown_later: dict[tuple[str, int], int] = {} + changed_later: dict[str, int] = {} + rendered: list[tuple[str, str]] = [] + for entry in reversed(history): + if not entry.actions: + text = ( + entry.message if entry.message.endswith("\n") else entry.message + "\n" + ) + rendered.append((text, _cap(text, 400))) + continue + full: list[str] = [] + brief: list[str] = [] + for action in reversed(entry.actions): + body = action.output + if action.view_key is not None: + later = shown_later.get(action.view_key) + if later is not None: + body = ( + body.split("\n", 1)[0] + + f" [same window shown again in turn {later}]" + ) + elif action.view_key[0] in changed_later: + window, _, lines = body.partition("\n") + body = ( + f"{window} [note: {action.view_key[0]} changed in turn " + f"{changed_later[action.view_key[0]]} after this view]\n" + + lines + ) + shown_later.setdefault(action.view_key, entry.turn) + if action.changed_path: + changed_later.setdefault(action.changed_path, entry.turn) + full.append(f"{action.header}\n{body}\n") + brief.append( + f"{action.header}\n{_brief(body, action.view_key is not None)}\n" + ) + head = f"TURN {entry.turn}\n" + rendered.append( + (head + "".join(reversed(full)), head + "".join(reversed(brief))) + ) + rendered.reverse() + return rendered + + +def render_history(history: Sequence[HistoryTurn], budget: int) -> str: + """The history in at most about ``budget`` characters. + + Every turn first gets its brief form (action lines and one line of each + output), oldest dropped only when even those do not fit; then the newest + turns are shown in full while the budget lasts. A newest turn too large to + show whole is shown cut, never dropped: its outcomes are the model's only + memory of them. + """ + turns = _render_turns(history) + chosen = [brief for _, brief in turns] + total = sum(len(text) for text in chosen) + first = 0 + while first < len(chosen) and total > budget: + total -= len(chosen[first]) + first += 1 + for index in range(len(turns) - 1, first - 1, -1): + extra = len(turns[index][0]) - len(chosen[index]) + if total + extra <= budget: + chosen[index] = turns[index][0] + total += extra + continue + if index == len(turns) - 1: + room = budget - (total - len(chosen[index])) + if room > _MIN_TURN_CHARS: + chosen[index] = _cap(turns[index][0], room) + break + marker = "[earlier turns cut to fit]\n" if first else "" + return marker + "".join(chosen[first:]) + + def build_turn_prompt( request: ModelDelegatedCodeEditRequest, file_index: Sequence[str], context: Sequence[tuple[str, str]], - history: Sequence[str], + history: Sequence[HistoryTurn], turn: int, *, max_chars: int = 90_000, + reads_paused: bool = False, ) -> str: - """One turn's prompt. ``history`` holds the rendered previous turns, oldest first.""" + """One turn's prompt. ``history`` holds the previous turns, oldest first. + ``reads_paused`` says this turn's reads are refused.""" checks = "\n".join(f"- {c.name}: {' '.join(c.argv)}" for c in request.checks) globs = ", ".join(request.writable_globs) tools = "\n".join( @@ -267,28 +435,32 @@ def build_turn_prompt( "through tools. Reply with ONE JSON object and nothing else:\n" '{"note": "", "actions": [{"tool": "", ...}, ...]}\n' f"At most {MAX_ACTIONS_PER_TURN} actions per turn. Paths are relative " - "to the worktree root. Read before you edit. Results come back next turn.\n\n" + "to the worktree root. Read before you edit. Results come back next turn.\n" + f"One turn's reads (view, grep, ls) show at most {MAX_READ_CHARS_PER_TURN} " + "characters together. HISTORY keeps every earlier turn, older turns " + "without their output: a view whose content is elided there was already " + "read. Do not re-read whole files; view the lines you will change and " + "edit them in the same or the next turn. After " + f"{MAX_READ_ONLY_TURNS} turns in a row that read and change no file, the " + "next turn's reads are refused.\n\n" f"TOOLS\n{tools}\n\n" f"WRITABLE (only these paths may be written): {globs}\n\n" f"CHECKS (run_check by name; finish runs all of them)\n{checks}\n\n" - f"TURN {turn} of {request.max_turns}. Call finish once the checks should pass.\n\n" + f"TURN {turn} of {request.max_turns}. Call finish once the checks should pass.\n" + + ( + f"THIS TURN: {MAX_READ_ONLY_TURNS} or more turns in a row read and changed " + "no file, so reads are refused this turn. Write or edit now with what " + "HISTORY shows; a view row reads 'NNNN| text', copy only the text.\n" + if reads_paused + else "" + ) + + "\n" f"TASK\n{request.task}\n\n" ) index = "FILES\n" + _cap("\n".join(file_index), 12_000) + "\n\n" shown = "".join(f"FILE {path}\n{_cap(body, 20_000)}\n\n" for path, body in context) budget = max_chars - len(head) - len(index) - len(shown) - kept: list[str] = [] - for block in reversed(history): - if budget - len(block) < 0: - # A turn whose results exceed what is left is shown cut, never - # dropped: its actions and outcomes are the model's only memory. - if budget > _MIN_TURN_CHARS: - kept.append(_cap(block, budget)) - kept.append("[earlier turns cut to fit]\n") - break - kept.append(block) - budget -= len(block) - past = "".join(reversed(kept)) + past = render_history(history, budget) if history else "" return head + index + shown + (f"HISTORY\n{past}" if past else "") @@ -299,6 +471,9 @@ def build_turn_prompt( "REQUIRED_ARGUMENTS", "RESPONSE_CONTRACT", "TOOL_SCHEMAS", + "HistoryAction", + "HistoryTurn", "build_turn_prompt", "parse_turn_reply", + "render_history", ] diff --git a/src/omnimarket/nodes/node_delegated_code_edit_orchestrator/models/model_delegated_code_edit.py b/src/omnimarket/nodes/node_delegated_code_edit_orchestrator/models/model_delegated_code_edit.py index 482c3ded6b..b98cbf80bc 100644 --- a/src/omnimarket/nodes/node_delegated_code_edit_orchestrator/models/model_delegated_code_edit.py +++ b/src/omnimarket/nodes/node_delegated_code_edit_orchestrator/models/model_delegated_code_edit.py @@ -38,6 +38,16 @@ VIEW_WINDOW_LINES = 250 #: Bytes one view window may carry. MAX_VIEW_WINDOW_BYTES = 16_000 +#: 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 +#: 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 _CHECK_NAME = re.compile(r"^[a-z0-9][a-z0-9_-]{0,47}$") diff --git a/tests/nodes/node_delegated_code_edit_orchestrator/test_handler_delegated_code_edit_orchestrator.py b/tests/nodes/node_delegated_code_edit_orchestrator/test_handler_delegated_code_edit_orchestrator.py index c220f72e74..cc62e3058e 100644 --- a/tests/nodes/node_delegated_code_edit_orchestrator/test_handler_delegated_code_edit_orchestrator.py +++ b/tests/nodes/node_delegated_code_edit_orchestrator/test_handler_delegated_code_edit_orchestrator.py @@ -648,12 +648,18 @@ def test_offset_must_be_a_positive_integer() -> None: def test_a_turn_whose_results_exceed_the_history_budget_is_cut_not_dropped() -> None: from omnimarket.nodes.node_delegated_code_edit_orchestrator import ( + HistoryAction, + HistoryTurn, build_turn_prompt, ) request = _request() - huge = "TURN 2\n> view(path='src/m.py') -> ok\n" + "x" * 50_000 + "\n" - history = ["TURN 1\n> ls() -> ok\nsrc\n", huge] + history = [ + HistoryTurn(1, actions=(HistoryAction("> ls() -> ok", "src"),)), + HistoryTurn( + 2, actions=(HistoryAction("> view(path='src/m.py') -> ok", "x" * 50_000),) + ), + ] head_only = build_turn_prompt(request, ["src/m.py"], (), [], 3, max_chars=1) prompt = build_turn_prompt( request, ["src/m.py"], (), history, 3, max_chars=len(head_only) + 5_000 @@ -737,9 +743,8 @@ def test_delegate_failed_receipt_preserves_last_good_history() -> None: assert block["max_turns"] == request.max_turns assert block["reason"] == "resumable from turn 3 (4 turns left)" assert len(block["history"]) == 2 - assert block["history"][0].startswith("TURN 1\n") - assert block["history"][1].startswith("TURN 2\n") - assert "delegate run failed" not in "".join(block["history"]) + assert [entry["turn"] for entry in block["history"]] == [1, 2] + assert "delegate run failed" not in json.dumps(block["history"]) assert ( block["diff_sha256"] == hashlib.sha256(ports.diff(request).encode()).hexdigest() ) @@ -771,7 +776,13 @@ def test_resume_restores_history_and_archives_failed_attempts() -> None: assert result.local_tokens_in == 11 assert result.local_tokens_out == 3 assert ports.delegate_turns[-1] == 3 - assert all(block in ports.prompts[-1] for block in history) + assert history + assert all(f"TURN {entry['turn']}\n" in ports.prompts[-1] for entry in history) + assert all( + action["header"] in ports.prompts[-1] + for entry in history + for action in entry["actions"] + ) receipt = ports.receipts[request.correlation_id] assert receipt["resumes"] == 1 assert [t["turn"] for t in cast(list[dict[str, Any]], receipt["turns"])] == [ @@ -869,24 +880,30 @@ def test_unusable_received_reply_is_a_good_resume_turn() -> None: block = cast(dict[str, Any], ports.receipts[request.correlation_id]["resume"]) assert block["last_good_turn"] == 1 assert len(block["history"]) == 1 - assert "your reply was unusable" in block["history"][0] + assert "your reply was unusable" in block["history"][0]["message"] def test_resume_history_keeps_newest_whole_blocks_within_budget() -> None: - actions = tuple(_a("view", path="src/m.py") for _ in range(4)) + # Each turn changes a file, so its reads are never paused, and reads its + # bounded 30,000 characters; eight such turns exceed the 120,000 budget. + actions = ( + _a("write", file_path="src/n.py", content="n = 1\n"), + *(_a("view", path="src/m.py") for _ in range(4)), + ) dead = ModelTurnReply(run_id="dead", ok=False) ports = FakePorts( - [_reply(n, *actions) for n in range(1, 5)] + [dead, dead], + [_reply(n, *actions) for n in range(1, 9)] + [dead, dead], files={"src/m.py": "x" * 10_000}, ) - request = _request(max_turns=10) + request = _request(max_turns=12) HandlerDelegatedCodeEditOrchestrator(ports).run(request) block = cast(dict[str, Any], ports.receipts[request.correlation_id]["resume"]) - assert block["last_good_turn"] == 4 - assert sum(map(len, block["history"])) <= 120_000 - assert len(block["history"]) == 2 - assert block["history"][0].startswith("TURN 3\n") - assert block["history"][1].startswith("TURN 4\n") + assert block["last_good_turn"] == 8 + assert sum(len(json.dumps(entry)) for entry in block["history"]) <= 120_000 + kept = [entry["turn"] for entry in block["history"]] + # The newest turns, whole, oldest dropped first. + assert kept == list(range(9 - len(kept), 9)) + assert 1 <= len(kept) < 8 def test_repeated_resumes_carry_attempts_and_preserve_check_state() -> None: @@ -1263,3 +1280,362 @@ def test_replace_in_files_schemas_declare_arrays_and_keep_the_turn_limit() -> No "minItems": 1, "maxItems": MAX_BULK_FILES, } + + +# -- OMN-20291: the replay's node failure classes ------------------------------ + + +def _calls(ports: FakePorts) -> list[dict[str, Any]]: + return cast("list[dict[str, Any]]", ports.transcript["calls"]) + + +def test_an_absolute_path_inside_the_worktree_is_used_as_its_relative_form() -> None: + """Replay 58744096: the loop prints check argv with the worktree's absolute + PYTHONPATH; the model searched that path and was refused as leaving the + worktree, and the rubric scored the absolute path a phantom path.""" + ports = FakePorts( + [ + _reply( + 1, + _a("view", path="/work/tree/src/m.py"), + _a("grep", pattern="add", path="/work/tree/src"), + _a( + "edit", + file_path="/work/tree/src/m.py", + old_string="return 0", + new_string="return a + b", + ), + _a("view", path="/work/treeX/src/m.py"), + _a("finish", summary="s"), + ) + ], + check_passes=[True], + ) + result = HandlerDelegatedCodeEditOrchestrator(ports).run(_request(max_turns=1)) + assert ports.files["src/m.py"] == FIX + calls = _calls(ports) + assert [c["status"] for c in calls] == ["ok", "ok", "ok", "error", "ok"] + assert '"path": "src/m.py"' in calls[0]["arguments_json"] + assert '"path": "src"' in calls[1]["arguments_json"] + assert '"file_path": "src/m.py"' in calls[2]["arguments_json"] + # A sibling directory that merely shares the prefix still leaves the worktree. + assert "leaves the worktree" in calls[3]["output"] + assert result.refusals == 1 + + +def test_normalise_path_maps_an_absolute_path_under_the_root() -> None: + assert normalise_path("/work/tree/src/m.py", root="/work/tree") == "src/m.py" + assert normalise_path("/work/tree", root="/work/tree") == "." + assert normalise_path("/work/tree/../x", root="/work/tree") is None + assert normalise_path("/tmp/x.py", root="/work/tree") is None + assert normalise_path("/abs") is None + + +DOC = ( + "def f():\n" + ' """Docs.\n' + "\n" + " * Owned: a CLAIM owns a PR by its ticket\n" + " (OMN-1), and no CLAIM owns a PR by a dispatcher. A later TERMINAL ends it.\n" + ' """\n' +) + + +def test_edit_tolerates_a_uniform_indentation_shift() -> None: + """Replay ea3ee264: the view prints `` 96| text`` and the model copied one + space of the ``| `` separator into every line of old_string and new_string.""" + ports = FakePorts( + [ + _reply( + 1, + _a( + "edit", + file_path="src/m.py", + old_string=" (OMN-1), and no CLAIM owns a PR by a dispatcher. A later TERMINAL", + new_string=" (OMN-1), and no CLAIM owns a PR by a dispatcher,\n" + " nor by a shared ticket. A later TERMINAL", + ), + ) + ], + files={"src/m.py": DOC}, + check_passes=[True], + ) + HandlerDelegatedCodeEditOrchestrator(ports).run(_request(max_turns=1)) + assert ports.files["src/m.py"] == DOC.replace( + "a dispatcher. A later", + "a dispatcher,\n nor by a shared ticket. A later", + ) + assert [c["status"] for c in _calls(ports)] == ["ok"] + + +def test_a_multi_line_edit_tolerates_a_uniform_indentation_shift() -> None: + ports = FakePorts( + [ + _reply( + 1, + _a( + "edit", + file_path="src/m.py", + old_string=" * Owned: a CLAIM owns a PR by its ticket\n" + " (OMN-1), and no CLAIM", + new_string=" * Owned: a CLAIM owns a PR by its ticket\n" + " (OMN-2), and no CLAIM", + ), + ) + ], + files={"src/m.py": DOC}, + check_passes=[True], + ) + HandlerDelegatedCodeEditOrchestrator(ports).run(_request(max_turns=1)) + assert ports.files["src/m.py"] == DOC.replace("(OMN-1)", "(OMN-2)") + + +def test_an_indentation_tolerant_match_must_still_be_unique() -> None: + body = "def f():\n x = 1\n\ndef g():\n x = 1\n" + ports = FakePorts( + [ + _reply( + 1, + _a( + "edit", + file_path="src/m.py", + old_string="\tx = 1", + new_string="\tx = 2", + ), + ) + ], + files={"src/m.py": body}, + ) + HandlerDelegatedCodeEditOrchestrator(ports).run(_request(max_turns=1)) + assert ports.files["src/m.py"] == body + assert _calls(ports)[0]["status"] == "error" + + +def test_an_edit_already_applied_is_reported_applied_not_failed() -> None: + """Replay ea3ee264 turn 7: the model re-sent an edit it had applied in turn 3.""" + ports = FakePorts( + [ + _reply( + 1, + _a( + "edit", + file_path="src/m.py", + old_string="return 0", + new_string="return a + b", + ), + ), + _reply( + 2, + _a( + "edit", + file_path="src/m.py", + old_string="return 0", + new_string="return a + b", + ), + ), + ], + check_passes=[True], + ) + HandlerDelegatedCodeEditOrchestrator(ports).run(_request(max_turns=2)) + assert ports.files["src/m.py"] == FIX + calls = _calls(ports) + assert [c["status"] for c in calls] == ["ok", "ok"] + assert "already" in calls[1]["output"] + assert ports.writes == ["src/m.py"] + + +def test_an_insertion_already_applied_is_not_inserted_twice() -> None: + ports = FakePorts( + [ + _reply( + 1, + _a( + "edit", + file_path="src/m.py", + old_string="def add(a, b):", + new_string="def add(a, b):\n # sum", + ), + ), + _reply( + 2, + _a( + "edit", + file_path="src/m.py", + old_string="def add(a, b):", + new_string="def add(a, b):\n # sum", + ), + ), + ], + ) + HandlerDelegatedCodeEditOrchestrator(ports).run(_request(max_turns=2)) + assert ports.files["src/m.py"] == "def add(a, b):\n # sum\n return 0\n" + assert [c["status"] for c in _calls(ports)] == ["ok", "ok"] + + +def _views(n: int, path: str = "src/big.py") -> ModelTurnReply: + return _reply( + n, *(_a("view", path=path, offset=str(1 + 250 * k)) for k in range(4)) + ) + + +def test_every_earlier_turn_stays_in_the_prompt_when_its_output_cannot() -> None: + """Replays a16a3138 and e3a2c923: from turn 3 the prompt sat at its cap and + held only the last one to three turns, so the model forgot what it had read + and re-read the same files for 40 turns without editing.""" + big = "".join(f"line {i} " + "x" * 40 + "\n" for i in range(1, 1001)) + turns = [ + _views(1), + _reply(2, _a("grep", pattern="line 7", path="src")), + *(_views(n) for n in range(3, 9)), + ] + ports = FakePorts(turns, files={"src/m.py": "x\n", "src/big.py": big}) + HandlerDelegatedCodeEditOrchestrator(ports).run(_request(max_turns=9)) + last = ports.prompts[-1] + for n in range(1, 9): + assert f"TURN {n}\n" in last, n + assert "> grep(path='src', pattern='line 7') -> ok" in last + assert "[earlier turns cut to fit]" not in last + # The newest turn's views are shown in full; a window shown again later is + # not repeated, it points at the later turn. + assert last.count("line 250 ") == 1 + # Turns 4, 6 and 8 have their reads paused (three read-only turns in a + # row), so turn 7 holds the last views. + assert "shown again in turn 7" in last + + +def test_a_view_of_a_file_changed_later_is_marked_stale() -> None: + ports = FakePorts( + [ + _reply(1, _a("view", path="src/m.py")), + _reply( + 2, + _a( + "edit", + file_path="src/m.py", + old_string="return 0", + new_string="return a + b", + ), + ), + ], + ) + HandlerDelegatedCodeEditOrchestrator(ports).run(_request(max_turns=3)) + last = ports.prompts[-1] + assert "src/m.py changed in turn 2 after this view" in last + + +def test_one_turns_reads_are_bounded_so_the_newest_turn_is_never_cut() -> None: + """Replays a16a3138 and e3a2c923 again: a turn read 8 to 12 windows of up to + 16 KB, more than the whole history budget, so even the newest turn was shown + cut and the model re-read what it had just read.""" + big = "".join(f"line {i} " + "x" * 40 + "\n" for i in range(1, 1001)) + ports = FakePorts( + [_views(1), _views(2)], files={"src/m.py": "x\n", "src/big.py": big} + ) + HandlerDelegatedCodeEditOrchestrator(ports).run(_request(max_turns=3)) + receipt = next(iter(ports.receipts.values())) + turns = cast("list[dict[str, Any]]", receipt["turns"]) + actions = cast("list[dict[str, Any]]", turns[0]["actions"]) + outputs = [a["output"] for a in actions] + oks = [a["ok"] for a in actions] + assert oks[:2] == [True, True] + assert sum(len(o) for o, ok in zip(outputs, oks, strict=True) if ok) <= 30_000 + assert not oks[-1] + assert "this turn's reads" in outputs[-1] + # The newest turn, and the one before it, are shown whole. + last = ports.prompts[-1] + assert "more characters cut" not in last + assert "line 250 " in last + + +def test_a_view_shrinks_to_what_the_turn_can_still_read() -> None: + from omnimarket.nodes.node_delegated_code_edit_orchestrator.handlers.handler_delegated_code_edit_orchestrator import ( + view_window, + ) + + text = "".join(f"row {i}\n" for i in range(1, 501)) + shown = view_window(text, "a.py", 1, limit=500) + assert len(shown) <= 500 + 200 + assert "[more: view a.py with offset=" in shown + + +def test_reads_pause_after_three_turns_that_read_and_change_nothing() -> None: + """Replays a16a3138 and e3a2c923 a third time: with the history fixed and one + turn's reads bounded, the model still read for 30 turns without editing.""" + view = _a("view", path="src/m.py") + edit = _a( + "edit", file_path="src/m.py", old_string="return 0", new_string="return a + b" + ) + ports = FakePorts( + [ + _reply(1, view), + _reply(2, view), + _reply(3, view), + _reply(4, view, edit), + _reply(5, view), + ], + ) + HandlerDelegatedCodeEditOrchestrator(ports).run(_request(max_turns=5)) + assert "reads are refused this turn" not in ports.prompts[2] + assert "reads are refused this turn" in ports.prompts[3] + receipt = next(iter(ports.receipts.values())) + turns = cast("list[dict[str, Any]]", receipt["turns"]) + fourth = cast("list[dict[str, Any]]", turns[3]["actions"]) + assert [a["ok"] for a in fourth] == [False, True] + assert "change no file" in fourth[0]["output"] + # The edit ended the streak: the next turn reads again. + fifth = cast("list[dict[str, Any]]", turns[4]["actions"]) + assert fifth[0]["ok"] + assert ports.files["src/m.py"] == FIX + + +def test_a_paused_turn_that_still_changes_nothing_allows_one_more_read_turn() -> None: + view = _a("view", path="src/m.py") + ports = FakePorts([_reply(n, view) for n in range(1, 7)]) + HandlerDelegatedCodeEditOrchestrator(ports).run(_request(max_turns=6)) + receipt = next(iter(ports.receipts.values())) + turns = cast("list[dict[str, Any]]", receipt["turns"]) + oks = [cast("list[dict[str, Any]]", t["actions"])[0]["ok"] for t in turns] + assert oks == [True, True, True, False, True, False] + + +def test_an_edit_whose_first_line_alone_carries_an_extra_blank_applies() -> None: + """Replay ea3ee264, re-run 2: copying from ``| def f(`` the model kept the + separator's blank on the first line only; every other line was exact.""" + body = "x = 1\n\n\ndef add(a, b):\n return 0\n" + ports = FakePorts( + [ + _reply( + 1, + _a( + "edit", + file_path="src/m.py", + old_string=" def add(a, b):\n return 0", + new_string=" def add(a, b, c=0):\n return a + b + c", + ), + ) + ], + files={"src/m.py": body}, + ) + HandlerDelegatedCodeEditOrchestrator(ports).run(_request(max_turns=1)) + assert ports.files["src/m.py"] == ( + "x = 1\n\n\ndef add(a, b, c=0):\n return a + b + c\n" + ) + assert [c["status"] for c in _calls(ports)] == ["ok"] + + +def test_replace_in_files_caps_the_files_holding_old_string_not_the_glob() -> None: + """Replay 8e1b5f72 (OMN-20392 AC2, loop 7ea45ffe): the glob + ``src/omnimarket/nodes/*/contract.yaml`` matches more node contracts than + the cap, though fewer hold old_string, and was refused outright; the model + fell back to a /tmp helper script and 117 single edits.""" + files = {f"src/m{n}.py": "other" for n in range(MAX_BULK_FILES + 50)} + files.update({"src/m1.py": "old", "src/m7.py": "old\nold"}) + action = _a("replace_in_files", glob="src/*.py", old_string="old", new_string="new") + ports = FakePorts([_reply(1, action)], files=files, check_passes=[True]) + HandlerDelegatedCodeEditOrchestrator(ports).run(_request(max_turns=1)) + assert sorted(ports.writes) == ["src/m1.py", "src/m7.py"] + assert ports.files["src/m7.py"] == "new\nnew" + call = _calls(ports)[0] + assert call["status"] == "ok" + assert f"2 edited, 0 failed of {MAX_BULK_FILES + 50}" in call["output"] + assert f"{MAX_BULK_FILES + 48} without old_string" in call["output"]