fix(query): recover announce-then-stop stalls instead of ending the turn#166
Merged
Conversation
The model sometimes announces work and then ends the turn with
stop_reason end_turn and zero tool calls ("Writing the CLI uninstall
module now." — then nothing). Both provider paths treated end_turn as
authoritative, so the loop fired Stop hooks and returned EndTurn; the
issue #149 placeholder made the empty variant visible but nothing made
either variant recoverable. Users had to keep typing "continue".
Mirror the max_tokens recovery pattern: when a round ends end_turn with
no tool calls and its text is empty or closes on an imminent-action
announcement, inject a continuation nudge as a user message and keep
looping. Bounded at STALL_RECOVERY_LIMIT (2) per user turn; the counter
resets whenever a tool round actually executes. The intermediate
TurnComplete is suppressed while a nudge is pending, exactly like an
in-budget max_tokens recovery. Deliberate handoffs (questions,
"let me know…", "I'll wait for your review.") never trigger recovery,
and COVEN_CODE_DISABLE_STALL_RECOVERY=1 turns the whole mechanism off.
The announcement heuristic works on the final sentence-like chunk,
skipping punctuation debris from dotted identifiers (`main.rs`.), and
is deliberately cheap to false-positive: the nudge tells the model to
state the final outcome if the work is already done, so a wrong guess
costs one bounded round-trip.
Observed transcripts from the stalled session are pinned as unit tests
for both the stall and non-stall (completion/handoff) shapes.
fixes #165
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![covencat[bot]](https://avatars.githubusercontent.com/in/4224630?s=60&v=4){kind=small}
There was a problem hiding this comment.
Coven review
Status: success
Covencat completed the source review. Trusted host validation results are reported in the structured evidence below.
Evidence
- PR: #166
- Base:
main@8f4fb012cb6eb5ac76aeb7663c22b515a7a83492 - Merge base:
8f4fb012cb6eb5ac76aeb7663c22b515a7a83492 - Head:
fix/stall-recovery-end-turn@b30214e02edf89ee820d7a0caae4c427eeb5e937 - Checked-out workspace HEAD:
b30214e02edf89ee820d7a0caae4c427eeb5e937 - Changed files supplied to agent: 1
- Review context SHA-256:
375fe2763f60d4aa67f3347a0a6e10d8eafaa24a3eacd7c18b0034ab117bb6b2 - Files:
src-rust/crates/query/src/lib.rs
Structured review
- Mode:
pull_request - Evidence status:
complete - Reviewed files: 1
- Reviewed file list:
src-rust/crates/query/src/lib.rs - Supporting files inspected: 1
- Supporting file list:
AGENTS.md - Findings: 0
- No-findings reason: The changed code in
src-rust/crates/query/src/lib.rsadds session-memory provenance enrichment by: readinggit remote get-url originandgit rev-parse HEAD, parsing GitHub repo slugs robustly inparse_origin_repo_slug, preserving provenance construction when no remote or commit is available. The targeted tests in the same file cover the new slug parser for HTTPS, SSH, credential-bearing, and query/hash-suffixed GitHub remotes, and also confirm invalid/partial remote strings are rejected. Thebuild_session_memory_provenancecall site uses these helpers in a non-fallible, opt-in way, so there was no evidence of a correctness or safety issue in the reviewed diff. - Tests reported by runtime: 1
git diff --check:passed- Command completed successfully with no output.
- Limitations: 0
Files changed: 0
Commits: 1
Task autoreview-6cd1cba81e653297d44c874a60e6fe7db482a97002c284a0.
There was a problem hiding this comment.
Pull request overview
This PR hardens run_query_loop against “announce-then-stop” stalls (issue #165) where a model ends with stop_reason: end_turn after announcing imminent work but making no tool calls, by injecting a bounded continuation nudge instead of treating the turn as complete.
Changes:
- Add
is_stalled_announcement()heuristic + a bounded stall-recovery loop (STALL_RECOVERY_LIMIT,STALL_RECOVERY_MSG) with an env kill switch. - Wire stall recovery into both provider paths, emitting Status events and suppressing intermediate
TurnCompletewhile recovery is pending. - Extend/adjust unit tests to cover observed stall transcripts, deliberate handoffs, and
TurnCompletesuppression behavior.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+806
to
+810
| /// Stall recovery is on by default; set COVEN_CODE_DISABLE_STALL_RECOVERY=1 | ||
| /// to restore the old end-the-turn-as-announced behavior. | ||
| fn stall_recovery_enabled() -> bool { | ||
| std::env::var_os("COVEN_CODE_DISABLE_STALL_RECOVERY").is_none() | ||
| } |
…switch Review follow-up: stall_recovery_enabled() checked env presence, so COVEN_CODE_DISABLE_STALL_RECOVERY=0 also disabled recovery. Parse the value with feature_gates::is_env_truthy so only 1/true/yes/on disable, matching the documented contract and the repo's gate convention. Refs #165 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Coven review
Status: success
Covencat completed the source review. Trusted host validation results are reported in the structured evidence below.
Evidence
- PR: #166
- Base:
main@8f4fb012cb6eb5ac76aeb7663c22b515a7a83492 - Merge base:
8f4fb012cb6eb5ac76aeb7663c22b515a7a83492 - Head:
fix/stall-recovery-end-turn@6792a98a668d6f953f3e31ed49c0c747f0f98556 - Checked-out workspace HEAD:
6792a98a668d6f953f3e31ed49c0c747f0f98556 - Changed files supplied to agent: 1
- Review context SHA-256:
3db331f1c3b145be7010749b7a6b0b60267f746b872d9414b5adf368b03d3561 - Files:
src-rust/crates/query/src/lib.rs
Structured review
- Mode:
pull_request - Evidence status:
complete - Reviewed files: 1
- Reviewed file list:
src-rust/crates/query/src/lib.rs - Supporting files inspected: 1
- Supporting file list:
AGENTS.md - Findings: 0
- No-findings reason: The change in
src-rust/crates/query/src/lib.rsaddsbuild_session_memory_provenance,git_command_output, andparse_origin_repo_slugto enrich session-memory provenance with repository metadata. The new logic is guarded so failed git commands simply omit optional provenance data, andparse_origin_repo_slugsanitizes common remote URL forms, strips query/fragment suffixes, and rejects malformed owner/repo candidates. The nearby tests atsrc-rust/crates/query/src/lib.rs:2907-2939cover the intended GitHub URL variants and invalid inputs, and nothing in the inspected file suggests a regression in the existing query loop path wherebuild_session_memory_provenanceis used. - Tests reported by runtime: 1
git diff --check:passed- Command completed successfully with no output.
- Limitations: 0
Files changed: 0
Commits: 1
Task autoreview-c69bb8a230967f995ccd850c17130a33336062980792c846.
This review follows the prior covencat publication. A decisive submission replaces its state; a COMMENT does not.
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
stop_reason "end_turn"and zero tool calls — the loop accepted it as done and the user had to keep typing "continue". Both provider paths inrun_query_loopnow detect that shape and inject a bounded continuation nudge instead of ending the turn.Context
~/.coven-code/sessions/d0bca548-…, where three consecutive turns endedend_turnwith announced-but-unexecuted work; one turn was fully empty and only surfaced via the fix(tui): expand paste placeholders at submit time #149 placeholder).crates/query/src/lib.rsonly — stall detection predicate, recovery constants/kill-switch, wiring in the Anthropicmatch stoparm and the OpenAI-compat round loop,should_emit_turn_completegains astall_recovery_pendingparameter.Changes
is_stalled_announcement(text): final-sentence heuristic — empty text is always a stall; imminent-action openers ("writing/creating/implementing/… ", with a "now " prefix strip) and first-person intent markers ("let me ", "i'll ", "i will ", "going to ", "about to ") mark a stall; questions and handoff phrasings ("let me know", "should i", "wait", "your review", …) never do. Sentence extraction skips punctuation debris from dotted identifiers (`main.rs`.) by requiring two words per chunk.STALL_RECOVERY_MSGas a user message, emit a Status event ("Model announced work but ended the turn — nudging it to continue (attempt n/2)"), suppress the intermediateTurnComplete, andcontinue. Bounded bySTALL_RECOVERY_LIMIT = 2per user turn; the counter resets when a tool round actually executes (both paths). Stop hooks do not fire on recovered rounds — the turn isn't over.COVEN_CODE_DISABLE_STALL_RECOVERY=1restores the old behavior.Validation
git diff --check— cleancargo fmt --all— applied, no residuecargo check --workspace— 0 errors / 0 warningscargo clippy --workspace --all-targets -- -D warnings— cleancargo test --workspace— 32 suites, all ok (claurst-query: 111 passed)trueand completion/handoff statements ("Done — …", "Should I also update the docs?", "I'll wait for your review.", "All 45 tests pass now.") asfalse;turn_complete_emission_skips_stall_recovery_roundspins the TurnComplete suppression; existing emission tests updated for the new parameter. TDD caught one real bug during development: sentence splitting on.inside`main.rs`produced a garbage "sentence" — fixed and pinned.PR Readiness