Add streaming retry resilience coverage - #6376
Conversation
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted reviewer state before this projection. |
There was a problem hiding this comment.
Code Review
This pull request introduces streaming support for Rig-backed model calls in WebUI v2 runs, adds robust E2E tests for delayed, broken, cancelled, and transient mock LLM behaviors, and enables retrying failed runs after transient checkpoint-state outages, including those failing before the first checkpoint. The review feedback identifies an issue in complete_streaming where reasoning content is discarded and hardcoded to None in CompletionResponse, and suggests propagating the extracted reasoning value to support reasoning-focused models.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
⚠️ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| 0 | 0 | 0 | ab90450e3a7d |
Head: ab90450e3a7d442b3bffbe50eaceb02e1aad63e3
Next: Human review or validation is required before merging.
Run details
Status: Current
Needs human: no
Needs validation: yes
Summary
Static review found no blocking correctness or security issue. Targeted Rust and Python validation could not run because this reviewer environment lacks cargo and python3.
Findings
None.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas.
|
🚅 Deployed to the ironclaw-pr-6376 environment in ironclaw-ci-preview
|
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 86.41% — 321234 / 371765 lines Per-crate breakdown (65 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (3 entry/entries excluded from the accounting above)
|
ab90450 to
0aa0c25
Compare
📝 WalkthroughWalkthroughRigAdapter now preserves reasoning during plain and tool streaming. Turn retries support failed runs without checkpoints, including transient checkpoint-store recovery. WebUI v2 end-to-end coverage adds scripted mock LLM faults for retries, delays, broken streams, and cancellation. ChangesStreaming reasoning propagation
Checkpointless failed-run retry
Mock LLM fault-driven run control
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant StreamingCompletionModel
participant RigAdapter
participant CompletionStreamSink
StreamingCompletionModel->>RigAdapter: emit text and reasoning chunks
RigAdapter->>CompletionStreamSink: forward text deltas
RigAdapter->>RigAdapter: accumulate reasoning details
RigAdapter-->>StreamingCompletionModel: return final response with reasoning
sequenceDiagram
participant WebChatClient
participant RebornServe
participant MockLLM
WebChatClient->>RebornServe: submit message
RebornServe->>MockLLM: request streaming completion
MockLLM-->>RebornServe: fault, delay, or broken stream
RebornServe->>MockLLM: retry request when applicable
MockLLM-->>RebornServe: completed assistant stream
RebornServe-->>WebChatClient: completed SSE event
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_llm/src/rig_adapter.rs`:
- Around line 1072-1105: The streaming response drain logic is duplicated
between the current streaming method and complete_with_tools_streaming. Extract
the shared stream.next() processing, usage/cache extraction, extract_response
call, and StreamedReasoningAccumulator::finish() handling into a helper such as
drain_reasoning_stream, then have both callers use it while preserving their
existing sink behavior and returned values.
In `@crates/ironclaw_turns/src/filesystem_store/turn_state_engine.rs`:
- Around line 3645-3656: Remove the duplicate has_loop_checkpoints_for_run
helper and reuse run_has_loop_checkpoint instead. At
crates/ironclaw_turns/src/filesystem_store/turn_state_engine.rs:3645-3656,
update failed_run_retryable to call run_has_loop_checkpoint with record.scope,
record.turn_id, and record.run_id; at :2961-2979, replace the
has_loop_checkpoints_for_run call on failed with the equivalent
run_has_loop_checkpoint arguments.
- Around line 492-494: Remove the unconditional #[allow(dead_code)] from
with_block_persistence and gate this crate-private test seam with the
repository’s appropriate test configuration, such as #[cfg(any(test, feature =
"test-support"))]. Preserve its availability for tests and test-support builds
without suppressing the dead-code lint.
In `@tests/e2e/mock_llm.py`:
- Around line 3358-3397: Update set_llm_faults to validate and construct the
complete fault-script list in a local collection before changing
_llm_fault_scripts. Only after every fault passes validation should the handler
reset and atomically commit the validated list; any 400 response must leave the
existing shared fault scripts unchanged.
In `@tests/e2e/scenarios/test_reborn_webui_v2_streaming_run_control_api.py`:
- Around line 288-442: Extract the repeated submit, SSE completion wait,
assistant-content wait, request-count wait, and assertions from the three tests
into a shared async helper named _run_fault_scenario. Parameterize it with
marker, actions, and expected_request_count, then update
test_reborn_v2_retries_mock_llm_http_error_then_finalizes,
test_reborn_v2_retries_mock_llm_broken_sse_stream_then_finalizes, and
test_reborn_v2_delayed_mock_llm_response_finalizes to configure their fault
payloads and call the helper while preserving the stream=True assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 918f0bb9-cc77-47f2-b6b8-4e0b8f061478
⛔ Files ignored due to path filters (1)
CHANGELOG.mdis excluded by!CHANGELOG.md
📒 Files selected for processing (7)
crates/ironclaw_llm/src/rig_adapter.rscrates/ironclaw_runner/tests/loop_driver_host.rscrates/ironclaw_turns/src/filesystem_store/turn_state_engine.rscrates/ironclaw_turns/tests/retry_failed_turn_store_contract.rstests/e2e/conftest.pytests/e2e/mock_llm.pytests/e2e/scenarios/test_reborn_webui_v2_streaming_run_control_api.py
0aa0c25 to
51a2ab8
Compare
Summary
stream: truerequest assertions and lifecycle SSE completion.Change Type
Linked Issue
None.
Validation
cargo fmt --all -- --checkequivalent:cargo fmt -p ironclaw_llm -p ironclaw_turns -p ironclaw_runnercargo clippy --all --benches --tests --examples --all-features -- -D warningsequivalent:cargo clippy --workspace --all-targets --all-features -- -D warningscargo buildpython3 -m py_compile tests/e2e/mock_llm.py tests/e2e/conftest.py tests/e2e/scenarios/test_reborn_webui_v2_streaming_run_control_api.pycargo test -p ironclaw_llm rig_adaptercargo test -p ironclaw_turns --test retry_failed_turn_store_contractcargo test -p ironclaw_runner --all-features --test loop_driver_hostpython3 -m pytest -q tests/e2e/scenarios/test_reborn_webui_v2_streaming_run_control_api.pycargo test --features integrationif database-backed or integration behavior changedreview-prorpr-shepherd --fixwas run before requesting review: addressed unresolved review threads and reran local validation.Security Impact
No new credentials, permissions, sandbox policy, or file access. This changes provider request shape to use streaming for Rig-backed model calls and adds hermetic mock-server fault controls under E2E tests only.
Reborn Trust-Boundary Checklist
host_stage_unavailable_checkpointand retryable lifecycle flag are reused. Audited throughrg -n "host_stage_unavailable_checkpoint|RunNotRetryable|retry_turn_once|run_has_loop_checkpoint" crates/ironclaw_turns crates/ironclaw_runner.serde(default)fields fail closed or have migration tests: no new persisted fields or schema changes.Transient,Permanent,Misconfigured,PolicyDeniedor equivalent): existing checkpoint unavailable category remains stable and retryable when safe.Database Impact
None. No migrations or schema changes. Turn-state retry semantics are covered against the filesystem-backed row store contract.
Blast Radius
Touches Rig-backed LLM streaming, Reborn turn retry classification, runner checkpoint-outage behavior, and WebUI v2 served E2E mock LLM scenarios. Regressions would most likely appear as provider streaming request-shape issues, missing reasoning metadata in streamed completions, or incorrect retry affordances for failed runs.
Rollback Plan
Revert this PR commit. That returns Rig-backed WebUI runs to the previous non-streaming provider path and restores the old checkpoint-only retry behavior. No data migration rollback is required.
Review Follow-Through
Addressed review feedback for reasoning propagation, duplicated stream-drain logic, dead test seam shape after turn-state decomposition, duplicate checkpoint helper use, atomic mock LLM fault validation, and duplicated E2E fault scenario structure. Existing IronLoop reviewer could not run validation in its environment; local validation above was run after the latest rebase.
Review track: C (runtime/turn retry and provider streaming behavior)