test: MessageProcessor + fallback-routing tests (slice 12, last task)#125
Merged
Conversation
- MessageProcessor captures ResultMessage.is_error/subtype (resettable per query) so callers can derive session success instead of assuming it - run_work_session derives success from is_error (was hardcoded True) and exposes subtype; budget/turn-capped or errored sessions now report failure instead of being silently marked successful - Skip subagent messages (parent_tool_use_id != None) in end-of-turn detection: a Task-subagent's end_turn no longer arms the short post-completion idle timeout and truncates the still-working parent - Guard _default_process_message against a None result overwriting accumulated text (error ResultMessages carry result=None) Co-authored-by: Claude <noreply@anthropic.com>
- shutdown handler now ONLY sets the Event; running callbacks under the non-reentrant lock could self-deadlock if a signal lands mid add_callback - unregister restores SIG_DFL when the original handler was None (was leaving our handler wedged in place) - interruptible_sleep blocks on Event.wait → instant in-process wake, still polls the durable cross-process stop each interval - _drive_coroutine_on_new_loop spawns a shutdown watcher that cancels the in-flight task via call_soon_threadsafe and surfaces KeyboardInterrupt, so Ctrl+C during a long streaming turn exits 2 promptly instead of waiting out the ~30-min SDK idle ceiling Co-Authored-By: Claude <noreply@anthropic.com>
Model routing (slice 12): - MODEL_EFFORT_MAP: direct model→effort lookup (fable→"max"). Replaces the reverse-lookup-by-complexity that left FABLE with no effort → the premium tier ran without extended thinking while Opus got "max". - get_fallback_chain(): cycle-guarded multi-hop chain (guards the HAIKU↔SONNET 2-cycle). Wired into _run_query_with_retry via a new ModelUnavailableError so the fallback walks the full documented chain (Fable→Opus→Sonnet→Haiku), not a single SDK hop; model-unavailability does not consume the transient-failure budget. - parse_task_complexity: anchored/last-match tag selection with count=1 strip — a quoted prose tag no longer beats the real leading/trailing routing tag, and only the winning tag is removed. Circuit breaker (decision b: FIX — it is live, not dead): - _check_state_timeout: HALF_OPEN → OPEN after timeout so a wedged HALF_OPEN restarts the recovery clock instead of failing fast forever. - __exit__: CancelledError/KeyboardInterrupt recorded as neither success nor failure — user interrupts must not trip the breaker. - CircuitBreakerConfig.__post_init__ validates success_threshold <= half_open_max_calls (and thresholds >= 1); aggressive() fixed (half_open_max_calls 1→3) so it can actually close. Tests: effort map, fallback chain, tag parsing, breaker invariant/timeout/ interrupt handling, model-unavailable classification + chain retry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…stener, cost tracking - agent_query.py: drop os.chdir (replaced by cwd= in SDK options); add StreamStallError subclass so stall detection is isinstance-checked instead of float-equality; cap retry_after at max_backoff to prevent pathological servers from stalling retries - rate_limit.py: add decorrelated jitter [0.5×, 1.0×] to calculate_backoff(); _base_backoff() is deterministic and used by get_total_max_time() for planning - key_listener.py: add _settings_lock around _original_settings; join-before-restore in stop() to prevent raw-mode race; reset _running=False in _listen finally so restart works after Escape exits the loop without stop() being called - agent_message.py: extract ResultMessage.total_cost_usd/usage into last_total_cost_usd, last_input_tokens, last_output_tokens (reset in reset_result_state) - progress_tracker.py: add actual_cost_usd to SessionMetrics (preferred over estimate); add record_cost() method for callers to feed real API charges - orchestrator.py: feed MessageProcessor cost data to tracker before end_session so cost report shows real API charges instead of $0.00 - tests: update backoff tests to patch random.random=1.0 for determinism; update key_listener timeout from 0.1→1.0; update working-directory tests to use isdir mock Co-Authored-By: Claude <noreply@anthropic.com>
- tests/core/test_agent_message.py: 45 tests covering MessageProcessor directly — is_error propagation, subagent end_turn stop_reason suppression, None-guard on accumulated text, reset_result_state, cost/token extraction, TextBlock/ToolUseBlock/ToolResultBlock routing, RateLimitEvent warning, and format_tool_detail per-tool dispatch. - tests/core/test_agent_models_fallback.py: 30 tests covering routing integration — tag→complexity→model→effort chain for all 4 tags, FABLE effort="max" (not a complexity but in MODEL_EFFORT_MAP), fallback chain cycle-guard invariants, HAIKU↔SONNET cycle cut, multi-hop retry on ModelUnavailableError (FABLE→SONNET skipping OPUS pre-seeded), and SIGINT→is_cancellation_requested→orchestrator exit 2 end-to-end. Co-Authored-By: Claude <noreply@anthropic.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 51 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (30)
✨ Finishing Touches🧪 Generate unit tests (beta)
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
1 participant
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
tests/core/test_agent_message.py(45 tests): Directly testsMessageProcessorfromagent_message.py—is_errorpropagation intolast_result_is_error,stop_reason="end_turn"/"tool_use"suppression (subagent end_turn ignored),None-guard preventing accumulated text loss on error ResultMessages,reset_result_state(), cost/token extraction,TextBlock/ToolUseBlock/ToolResultBlockrouting,RateLimitEventwarning, andformat_tool_detailper-tool dispatch.tests/core/test_agent_models_fallback.py(30 tests): Integration coverage for the routing pipeline — tag→complexity→model→effort chain (all 4 tags), FABLEeffort="max"viaMODEL_EFFORT_MAP(not routed by any complexity), cycle-guarded fallback chain invariants (HAIKU↔SONNET cycle cut), multi-hop retry skipping pre-seeded SDK hop, and SIGINT→is_cancellation_requested()→orchestrator.run()==2end-to-end path.Test plan
uv run pytest tests/core/test_agent_message.py tests/core/test_agent_models_fallback.py— 75 passedruff check+ruff format --check— cleanmypy .— no issues🤖 Generated with Claude Code