harness(agent_loop): let a model response keep the floor#61
Open
yh928 wants to merge 1 commit into
Open
Conversation
A response carrying tool calls continues the turn; one without them is taken as the turn's final answer. That leaves no way for a model to say something *before* it acts — a text-mode protocol, where tool calls are encoded as prose, has no tool call to ride and so ends its turn on the first sentence. Working around it means faking a tool call purely to keep the loop going. Add `ModelResponse::continue_turn: Option<String>`. When set, the loop appends the carried string as the next user turn and asks for another reply instead of finalizing. `None` is the default, so existing callers are unaffected. It carries a string rather than a bool because most chat APIs reject two assistant turns in a row — something must be appended — and letting the adapter choose keeps the protocol's vocabulary out of the harness. Checked after truncated-empty recovery (a truncated response is broken, not a deliberate continue) and before structured extraction (which would treat it as terminal). No new cap: each continue costs one model call, so `max_model_calls` already bounds a model that never stops.
📝 WalkthroughWalkthroughAdds an optional ChangesTurn continuation
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
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
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.
Summary
A response carrying tool calls continues the turn; one without them is taken as the turn's final answer. That leaves no way for a model to say something before it acts.
The motivating case: a text-mode tool protocol
Some models can't use native tool calling — a large toolkit compiles to a provider grammar that blows the rule ceiling and gets rejected with a 400 before any generation, and local runtimes often reject the
toolsparameter outright. Those turns fall back to encoding tool calls as prose.The one we drive looks like this. A reply is exactly one tagged block:
T— say something to the userC— call a tool (positional or JSON body)E— doneThe tag sits only at the very start of a reply and everything after it is payload verbatim, so a
Tblock containing a line that begins withCstays literal text. That property is the whole point of a distinct format: the parser never scans for tags, so it can't false-positive on prose.The model adapter parses each reply and hands the loop an ordinary
ModelResponse, which lines up with the existing loop almost exactly:ModelResponseCtool_callsETTis the one block with no home. It means "I have something to say and I am not finished", but it carries no tool call, so the loop ends the turn on it. A model can therefore never narrate before doing slow work — it must stay silent until the work is done, which is exactly the moment a user most wants to hear something.The only workaround is to synthesise a fake tool call — register a no-op tool, attach a call to it on every
T, and let the loop "execute" nothing — purely to buy one more reply. That is a real no-op round-trip in the transcript, and a tool in the registry that exists to lie to the loop. This field is that workaround, done honestly.The change
Adds
ModelResponse::continue_turn: Option<String>. When set, the loop appends the carried string as the next user turn and asks for another reply instead of finalizing:Two placement details are deliberate:
Nothing in the harness knows what a
Tblock is: it only learns that a given response does not end the turn. The protocol stays entirely in the adapter.Why a
Stringand not aboolMost chat APIs reject two assistant turns in a row, so something must be appended to continue. Letting the adapter supply that string keeps the calling protocol's vocabulary (ours is
"(next)") out of the harness — and it is the same token the protocol's system prompt teaches the model to expect, so the two can't drift.Why no new limit
Each continue costs exactly one model call, so
RunLimits::max_model_callsalready bounds a model that never stops. An earlier draft of this grew a second, protocol-specific block cap; having two independent ceilings on one turn made "how long is a turn?" unanswerable, and this needs none.API Or Behavior Changes
ModelResponse::continue_turn: Option<String>(#[serde(default)]).Noneis the default and preserves the existing loop exactly — covered by a test that pins it.ModelResponse { .. }literal must name it; the in-crate literals (tests, examples, providers) are updated here. Callers usingModelResponse::assistant(..)are unaffected.Tests
Three new tests in
src/harness/agent_loop/test.rs:continue_turn_keeps_the_floor_and_appends_the_nudge— a continuing reply does not end the turn (model_calls == 2) and the nudge lands in the transcript.no_continue_turn_ends_on_the_first_tool_less_reply— pins the defaultNonebehavior.an_endless_continue_is_bounded_by_max_model_calls— a model that never stops trips the model-call cap.All commands run locally on this branch:
cargo fmt --checkcargo clippy --all-targets -- -D warningscargo clippy --all-targets --all-features -- -D warningscargo build --all-targetscargo build --all-targets --all-featurescargo testcargo test --all-featuresDocumentation
The field carries its own rustdoc: what it does, why it exists, why it's a
String, and why it needs no cap of its own. The loop's branch carries the placement rationale. No architecture or example docs describe turn termination, so nothing else needed updating.