Repository navigation
Conversation
ReplayModel only advanced to the next replay after an action that was
exactly the string "submit". run-replay writes actions as dicts
(`{"message": ...}` with optional `tool_calls`), so `_replay_idx` never
advanced: a multi-instance replay file stayed on its first line forever
and queries past the end of a line auto-submitted endlessly.
Detect the submit command in all action formats: as a tool call, as a
standalone line of the message (also covering the auto-generated
submission), and as a legacy plain string action.
chessing234
left a comment
chessing234
left a comment
There was a problem hiding this comment.
The nine model tests pass, but an additional replay check reproduces premature instance advancement when submit is mentioned outside the executable action. Please address the inline finding and add a regression test for discussion containing a standalone submit line.
| if tool_calls := action.get("tool_calls"): | ||
| return any(tool_call["function"]["name"] == self.submit_command for tool_call in tool_calls) | ||
| action = action.get("message", "") | ||
| return any(line.strip() == self.submit_command for line in action.splitlines()) |
There was a problem hiding this comment.
[P2] Detect submission from the executable action, not every message line.
A valid ThoughtActionParser response can mention submit in discussion:
The final command will be:
submit
First inspect the repository.
```
echo first
```
The configured parser extracts echo first, yet this helper returns true because the discussion contains a standalone submit. I reproduced that the next query then returns the next instance's first action, skipping the current instance's remaining actions and actual submission. Extract the action using the configured parser (or otherwise restrict matching to executable submit syntax) and add a regression for submit mentioned in discussion.
There was a problem hiding this comment.
Addressed in a42fbb4: _is_submit_action now extracts the executable action with the configured parser — parse_function(model_response, commands), the same call the live loop makes via ToolHandler.parse_actions — and compares that to the submit command. The discussion-only submit from your repro no longer advances the replay; added test_replay_model_ignores_submit_mentioned_in_discussion with that exact message.
An action the parser cannot extract counts as not-submit and logs a warning; the replay continues and the end-of-trajectory fallback submits, so a missed submit degrades to the auto-submit path instead of skipping actions. That fallback now advances unconditionally (it is by definition the end of the instance) instead of relying on the parser. The fenced-block tests now run with ThoughtActionParser to match the format those messages were written in; the legacy bare-string test keeps Identity.
chessing234
left a comment
chessing234
left a comment
There was a problem hiding this comment.
Found a reproducible issue in the current PR head; see the inline change request.
All 9 model tests passed, but an additional replay/parser check reproduced the skipped-instance problem. The full integration suite was not run.
| if tool_calls := action.get("tool_calls"): | ||
| return any(tool_call["function"]["name"] == self.submit_command for tool_call in tool_calls) | ||
| action = action.get("message", "") | ||
| return any(line.strip() == self.submit_command for line in action.splitlines()) |
There was a problem hiding this comment.
[P1] Detect submit in the executable action, not the entire message
A standalone submit line in the explanatory text now advances _replay_idx even when ThoughtActionParser extracts a different command. Reproduced with the message `The final command will be:
submit
First inspect the repository.
echo first
: the parser returns echo first`, but after replaying it the next query returns the second instance's first action, skipping the remaining commands and actual submission from instance one. Please parse the executable action with the configured parser (or otherwise distinguish thought from action) before advancing, and add a regression for a submit mention outside the selected command block.
There was a problem hiding this comment.
Addressed in a42fbb4: _is_submit_action now extracts the executable action with the configured parser — parse_function(model_response, commands), the same call the live loop makes via ToolHandler.parse_actions — and compares that to the submit command. The discussion-only submit from your repro no longer advances the replay; added test_replay_model_ignores_submit_mentioned_in_discussion with that exact message.
An action the parser cannot extract counts as not-submit and logs a warning; the replay continues and the end-of-trajectory fallback submits, so a missed submit degrades to the auto-submit path instead of skipping actions. That fallback now advances unconditionally (it is by definition the end of the instance) instead of relying on the parser. The fenced-block tests now run with ThoughtActionParser to match the format those messages were written in; the legacy bare-string test keeps Identity.
Submit detection now extracts the action with the configured parser and compares that to the submit command, so a standalone submit line in the discussion outside the command block no longer advances the replay and skips the remaining actions of the instance. The auto-generated submission at the end of a trajectory advances unconditionally instead of relying on the parser.
|
Thanks for the review! Both points are addressed on the branch (submit detection now works on the executable action extracted with the configured parser, so |
Reference Issues/PRs
None — found while reading the replay flow; no existing issue or PR covers this.
What does this implement/fix? Explain your changes.
Summary
ReplayModelnever advanced to the next entry of a replay file:_next_replay()was only called when the replayed action was exactly the string"submit".run-replay's own writer (_create_actions_file,sweagent/run/run_replay.py:160-167) writes every action as a dict ({"message": ...}, plustool_callsfor function-calling configs), so that exact-string comparison could never match a file produced by the repo's own tooling.Why (root cause)
sweagent/agent/models.py:519readif isinstance(action, str) and action == "submit": self._next_replay()— only a legacy plain-string"submit"action advanced_replay_idx._replay_idxnever advanced. After the last replayed action, every further query fell into theIndexErrorfallback ("Reached end of replay trajectory without submitting. Submitting now.") and auto-submitted, without ever moving on to the next replay entry.Changes (
sweagent/agent/models.py)ReplayModel._is_submit_action(), which recognizes the submit command in all action formats: as a submit tool call (dict actions from function-calling configs), or by extracting the executable action with the configured parser and comparing it to the submit command - so asubmitmentioned in the surrounding discussion does not advance the replay (an action that cannot be parsed also does not advance it; the end-of-trajectory fallback submits if that was the missing submission). This also covers the auto-generated submission built in theIndexErrorfallback and the legacy plain string"submit".query()instead of the exact-string comparison.self._replays[self._replay_idx]lookup inside thetryblock so a query after the last entry's submit lands in the auto-submit fallback instead of raising an uncaughtIndexError(previously reachable with legacy string files whose entry ends in"submit").Note:
run-batchre-creates the agent (and with it the model) per instance (sweagent/run/run_batch.py:337), so selecting a replay entry by instance id would require threading the instance id into the model; that is a larger interface change and out of scope here.Testing
New regression tests in
tests/test_models.py, using the replay-file format written by_create_actions_file(one{instance_id: [actions]}object per line):test_replay_model_advances_after_dict_submit_actiontest_replay_model_advances_after_function_calling_submit_actiontest_replay_model_advances_after_string_submit_action(legacy format; guards existing behavior)test_replay_model_advances_after_auto_submissiontest_replay_model_auto_submits_when_file_exhaustedCommands and results (on
mainthe four advancement tests fail — the model returns the auto-generated{"message": "```\nsubmit\n```"}instead of the next instance's action — and pass on this branch):The 11 failures pre-exist identically on
mainand are unrelated to this change: 7 are GitHub API 403 rate-limit errors (tests/test_env_utils.py,tests/test_run_hooks.py), and 4 are subprocess-based CLI help tests that need thesweagentconsole script onPATH(they pass when it is:4 passed).tests/test_run_replay.py::test_replayand theslow-marked tests need Docker and were not run here.