Studio: Let the agent run a command again after it edits a file - #10810
Studio: Let the agent run a command again after it edits a file#10810NilayYadav wants to merge 3 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 02682d496c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) | ||
| ) | ||
| if not failed: | ||
| if decision.tool_name in _WORKSPACE_TOOLS: |
There was a problem hiding this comment.
When a Python or terminal invocation modifies a file and then exits nonzero, is_tool_error() marks the result as failed, so this invalidation never runs. For example, after a successful cat x, a Python call can write new contents to x and then raise an exception; the next identical cat x remains in _successful_keys and is incorrectly suppressed, potentially forcing the reply to finish without verifying the changed file. Clear stale workspace keys whenever one of these tools actually executes, while retaining the current success check only for caching the just-completed call.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 56ccabc: python, terminal and edit_file now clear earlier workspace results whether the call succeeds or fails, so a read after a failed write runs again. Covered by test_failed_workspace_execution_invalidates_previous_reads.
| ) | ||
| ) | ||
| if not failed: | ||
| if decision.tool_name in _WORKSPACE_TOOLS: |
There was a problem hiding this comment.
When a model emits terminal(cat x), edit_file(x, ...), and the same terminal(cat x) in one turn, this invalidation cannot take effect in the safetensors loop or the GGUF textual-fallback path: those paths remove repeated (tool name, arguments) pairs before calls reach ToolLoopController (safetensors_agentic.py:1305-1324 and llama_cpp.py:32356-32377). Consequently the second read is still discarded even though the intervening edit makes its result stale, so the fix only works when each call arrives in a separate model turn. Those prefilters need to retain non-adjacent workspace repeats or defer their deduplication to this controller.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 56ccabc: the safetensors and GGUF text-fallback prefilters no longer drop repeated workspace calls within a batch, so the controller decides them and a read after an edit in the same turn runs. Covered by test_workspace_read_edit_read_in_one_turn and test_textual_workspace_read_edit_read_in_one_turn.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56ccabc127
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if _key in seen_keys and _fn.get("name") not in _WORKSPACE_TOOLS: | ||
| continue | ||
| seen_keys.add(_key) | ||
| deduped.append(_tc) |
There was a problem hiding this comment.
When a textual turn contains repeated workspace calls before a later useful call—for example, eight identical terminal(cat x) calls followed by edit_file—this exception retains every duplicate until _MAX_TOOL_CALLS_PER_TURN is reached, so the edit is discarded even though seven reads become controller no-ops. Collapse workspace duplicates that have no intervening workspace execution, or apply the cap after no-op classification; the mirrored condition in llama_cpp.py has the same failure.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8da4fc4: both text-parsed prefilters now collapse a workspace call that repeats the previous workspace call in the batch, since the controller would no-op it anyway, so repeats no longer fill the batch cap and drop a later edit. A repeat after a different workspace call is still kept. Covered by test_repeated_workspace_reads_do_not_crowd_out_a_later_edit and test_textual_repeated_workspace_reads_do_not_crowd_out_a_later_edit.
| self._successful_keys -= stale | ||
| for key in stale: | ||
| self._duplicate_noop_counts.pop(key, None) |
There was a problem hiding this comment.
If a successful workspace read is repeated twice, record_noop sets _force_final_answer; when an edit_file call later executes in the same batch, this block removes the now-stale duplicate counter but leaves that latch set. Both local loops then enter their no-tools final pass (safetensors_agentic.py:1586 and llama_cpp.py:33317), so the agent still cannot rerun the read to verify the edit. The duplicate-driven latch must be recomputed when these workspace entries are invalidated without clearing terminal states caused by unrelated disabled or one-shot calls.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not changing this one. The latch is only set after the model repeats an identical call that already ran, twice in one reply despite the duplicate nudge, which is the #5962 loop guard working as intended. On main the same sequence also ends the reply and additionally skips the read after the edit; with this PR a read after the edit in the same batch still runs, so this is not a regression, and clearing the latch on an edit would weaken that guard.
Reproduction on GitHub Actions: before and afterI ran the same flow in a real Studio install on A scripted OpenAI-compatible connection sends four real calls through the Studio tool loop with Code enabled:
Unit tests, same two runs:
Review:
The two red upstream Backend CI jobs fail the same tests on |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8da4fc4dfc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _key = (_fn.get("name", ""), str(_fn.get("arguments", ""))) | ||
| if _key in seen_keys: | ||
| if _fn.get("name") in _WORKSPACE_TOOLS: | ||
| # A workspace repeat only matters after a different workspace call. | ||
| if _key == last_workspace_key: |
There was a problem hiding this comment.
When textual output alternates equivalent edit_file arguments whose JSON object keys have different orders, str(arguments) gives different _key values even though ToolLoopController canonicalizes them to the same call. Consequently eight alternating serializations all survive this filter, the first executes and the rest become controller no-ops, and _MAX_TOOL_CALLS_PER_TURN can discard a later verification command. Fresh evidence after 8da4fc4 is that its back-to-back check compares raw serialization rather than the controller's canonical key; the mirrored GGUF filter has the same issue.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not changing this one. It needs a single turn with eight or more equivalent calls whose JSON keys alternate order before a later call, which is degenerate model output rather than a normal session. The prefilter on main already compares the same raw argument string for every tool, so this PR does not introduce it, and the controller still turns those repeats into no-ops.
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Confirmed this hits tool_loop_controller.py where record_result keeps every successful key for the whole reply, so a command after an edit_file comes back as a duplicate and the model never sees its own change. Will get this reviewed. |
2 participants
Fixes #10792. When the agent ran a command, edited a file, and then ran the same command again, the second run was skipped. Studio remembered every successful call for the whole reply and treated any repeat as a duplicate, even when a file had changed in between. The agent was told the command had already run, so it could not check its own change, and trying again removed its tools and ended the reply.
Now when a python, terminal, or edit_file call succeeds, Studio forgets the earlier calls to those three tools, so a later identical command runs again. Running the exact same call twice in a row is still skipped, the limit on repeated duplicates from #5962 is kept, and web search still skips repeats.
Tested in the Studio chat with an agent that runs cat notes.txt, edits the file, and runs cat notes.txt again. Before, the second cat was skipped and the reply never finished. After, it ran and showed the edited file. This is separate from #10658, which only lets its own project task tools repeat.