fix: fall back when provider result is empty - #240
Open
FrundlesTian wants to merge 12 commits into
Open
FrundlesTian wants to merge 12 commits into
FrundlesTian wants to merge 12 commits into
Conversation
…, overwirehq#172) Before: a run the CLI killed at the turn limit produced no final text, so the bot fell through to its "Task completed" placeholder. A run that died at turn 10 mid-task and a run that finished looked identical in Telegram. After: the reply carries a footer naming the reason — turn limit, cost budget, cancellation, an execution error, or an unrecognised reason named by its raw subtype — with the turn count and a prompt to send another message to continue. Tool calls the run had blocked are listed too, with their most identifying argument, on successful runs as well as stopped ones; this bot generates those denials itself and the user previously saw only Claude's own narration of them. How: ClaudeResponse gains result_subtype, stop_reason, terminal_reason, errors and permission_denials, all defaulted, read off ResultMessage with getattr so an older CLI that omits them still works. The "Task completed" placeholder is gated on subtype == "success". format_stop_reason() and format_permission_denials() build the footer, keyed off subtype with terminal_reason preferred where it is a value we recognise; the SDK types both as bare str, so unknown values fall back to naming the raw subtype rather than guessing. Both agentic and classic mode render it. The fields are written to the structlog event for any run that did not end cleanly, but not persisted — that is a claude_interactions schema change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JkxpkyVHZACpjisj3C6wtP
The first cut covered agentic_text and the four call sites in handlers/message.py and missed five more, so a run truncated at the turn limit still reported success whenever the user sent a document, photo or voice note in agentic mode, or used /continue, the Continue Session button or a quick action in classic mode. The quick-action heading was the plainest case: it said "Complete" whatever the run did. All nine now go through one helper, with_stop_reason(), and a test walks the source for format_claude_response() calls that are not given it, so a new entry point cannot quietly skip the footer again. The two callbacks that build their HTML by hand convert the footer themselves. The quick-action heading reads "Stopped" for a run that was cut short. The footer's dynamic parts are now wrapped as inline code. It renders with Claude's reply, so the Markdown pass ran over it: a path like /tmp/_a_b_ came out italicised, and on a line listing two denials the italics bleed from one entry into the next. Inline code is extracted before any Markdown conversion, so it survives verbatim. The interruption note moves into the same footer, which keeps its wording byte-identical and lets every site handle an interrupted run alike. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JkxpkyVHZACpjisj3C6wtP
The stop-reason footer was appended after the body had already been clipped to 4000 characters, so a long reply with a footer could land past Telegram's 4096-character cap. reply_text does not split, so the send raised, the handler's except reported "Action Error", and the run that most needed its stop reason — a stopped one with denials — was the one that lost it. A 10,000-character stopped reply with eight denials came to 4343 characters. The heading and footer are now built first and Claude's own text is clipped to whatever room is left, against the same 4000-character budget ResponseFormatter works to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JkxpkyVHZACpjisj3C6wtP
A run killed at the turn limit mid-tool-use is the case this change is mostly for, and it stacked two warnings: the placeholder that stands in for the missing reply said "⚠️ Run stopped before finishing", then the footer said "⚠️ Stopped: turn limit reached after 10 turns". The placeholder now only reports what the run got done ("No final response. Tools used: ..."), and the footer carries the warning and the reason. _handle_continue_action was the one hand-built site without a length clamp. Its body looked safely bounded at 500 characters, but that clip happens before HTML escaping: 500 characters of "&" escape to 2500, and with a worst-case footer the message composed to 4764, past Telegram's 4096 cap. Both callbacks now compose through one helper that sizes the heading and footer first and clips the body to what is left. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JkxpkyVHZACpjisj3C6wtP
Both from review. num_turns was derived by counting UserMessage and AssistantMessage objects, which over-reports: every tool result arrives as another user message, so a run stopped at turn 10 could be recorded as roughly twice that. The number only reached the logs and the session store before this branch; the stop-reason footer now shows it to the user, which is what makes the approximation worth removing. ResultMessage.num_turns is used where the CLI supplies it, with the message count as the fallback. _inline_code deleted backticks from a denied tool argument so they could not close the code span early. That silently rewrote the thing being reported: `whoami` is command substitution, whoami is an argument, and the reader cannot tell a rewrite from the real value. Clipping for length is visible because it leaves an ellipsis; this was not. markdown_to_telegram_html now matches a code span between backtick runs of any length, closing only on a run of the same length, and strips one space from each end when both are present, as CommonMark specifies. _inline_code picks a delimiter one longer than the longest run in the value and pads a value that begins or ends with a backtick. Fenced blocks are still extracted first, so they keep precedence. Single- backtick spans are unaffected; the only other behaviour change is that ``a`b`` now renders as one span rather than the span a followed by loose text. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JkxpkyVHZACpjisj3C6wtP
A run stopped on its first turn is reachable — a low CLAUDE_MAX_TURNS, or an error before the second turn — and this is the sentence the whole change exists to produce. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JkxpkyVHZACpjisj3C6wtP
_handle_continue_action sent "✅ Session Continued" whatever happened, directly above a footer that can read "⚠️ Stopped: turn limit reached after N turns" — the contradiction this branch had just removed from the quick-action heading three functions away. The words stay: the session did continue, unlike the quick action's "Complete", which claimed the work was finished. It is the tick that would be the false report, so the tick is what changes. The length test already composed this heading with a stopped response but asserted only on the footer, so it never saw the contradiction; it now asserts the heading too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JkxpkyVHZACpjisj3C6wtP
_compose_reply escaped the body and then clipped it, so the cut could land inside & and leave &am. Telegram rejects that with "can't parse entities", the handler's broad except turns the rejection into a generic failure message, and the reply is lost for exactly the stopped run the footer exists to explain. Reproduced first: with a body of ampersands, offsets 0, 1 and 2 ended in &am, &a and a bare &. The clip still has to happen after escaping, because escaping is what can quintuple the length and blow the budget, so it now trims back to the last complete entity. Every & in the escaped body opens one, the literal ones having already become &, so a trailing & with no ; after it is a cut one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JkxpkyVHZACpjisj3C6wtP
Three findings from review. The webhook and scheduler paths published response.content straight to AgentResponseEvent, so a nightly job or a webhook run that died at the turn limit sent "No final response. Tools used: ..." and nothing about why -- the overwirehq#172 ambiguity on the one path where no user was watching to notice the run had been cut short. Both now publish with_stop_reason(). Pairing equal-length backtick runs with a backreference inside a lazy middle re-scans the rest of the line for every opener that never closes. On a reply whose backtick runs are all of different lengths that is superlinear: 313ms at 64KB and 2.5s at 256KB, synchronously on the event loop, against 1-4ms for the single-backtick pattern it replaced. The pass now scans the runs and pre-computes each one's next same-length partner, which is linear -- 0.76ms at 256KB. Output is unchanged: the scanner and the regex agree on all 31,120 cases of a corpus that enumerates every sequence of up to four backtick, space, newline and text tokens. format_permission_denials sliced to DENIAL_LIST_MAX before dropping non-dict entries, so five malformed entries at the front of a longer list reported that nothing had been blocked. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JkxpkyVHZACpjisj3C6wtP
The code-span scanner paired runs in O(1) but decided "is there a line break between them" with `"\n" in text[a:b]`, whose cost is the distance between the two runs and which is paid again for every opener that is then rejected. That reproduces the shape the scanner was meant to remove: runs of lengths 1..K, a newline, then the same lengths mirrored, puts every opener's only partner at the far end of the text and across the break, so each of the K lookups pays for the whole string. K distinct lengths need O(K^2) characters, so that is O(n^1.5) again -- measured at 97ms on 1.6MB and 1.07s on 6.5MB. The newline offsets are now collected once and the gap is checked by binary search: 3.3ms and 10.4ms on the same two inputs. The existing budget test could not have caught this. It uses runs of every distinct length, which have no partner at all, so `next_same` is -1 and the check is short-circuited before the gap is ever looked at. The new case pairs them across a newline so every lookup reaches it, and it fails at 1.10s against the implementation this replaces. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JkxpkyVHZACpjisj3C6wtP
…p-reason-zfw34i # Conflicts: # CHANGELOG.md
|
No blocking issues found. |
This was referenced Sep 23, 2026
RichardAtCT
changed the base branch from
claude/report-run-stop-reason-zfw34i
to
main
October 1, 2026 07:44
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
2 participants
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.
Description
Fixes the empty response shown with OpenRouter and other Anthropic-compatible providers that return
ResultMessage.result == ""while still emitting the real response in anAssistantMessage.The result is now stripped once and used only when it contains text. Empty and whitespace-only provider results take the existing AssistantMessage extraction path; non-empty ResultMessage content keeps its current priority.
This is stacked on #236 because that PR currently owns the same result-extraction block. The only diff relative to #236 is this fix, its regression tests, and the changelog entry. After #236 merges, this PR can be retargeted to
mainwithout carrying unrelated commits.Related issue
Closes #171
Type of change
How it was tested
Tests added or updated
Equivalent
make testand formatting/lint commands pass locallyTested by hand against a running bot
pytest --no-cov— 685 passedblack --check src testsisort --check-only src testsflake8 src testsThe regression test covers both an empty string and a whitespace-only result while an AssistantMessage contains the OpenRouter response.
Checklist
CHANGELOG.mdhas an entry under[Unreleased]pyproject.tomldependencies changed,poetry lockwas run and the updatedpoetry.lockis committed (not applicable)README.md,docs/,.env.example,CLAUDE.md) where settings or commands changed (not applicable)