Fix WebUI v2 live canary final-response waits - #5894
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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds ChangesFrontend final reply marker
Live QA assistant reply and trigger record waiting
Sequence Diagram(s)sequenceDiagram
participant ChatPage
participant MessageBubble
participant LiveQARunner
participant PlaywrightPage
ChatPage->>MessageBubble: render assistant message
MessageBubble->>PlaywrightPage: expose data-final-reply
LiveQARunner->>PlaywrightPage: poll assistant text and data-final-reply
PlaywrightPage-->>LiveQARunner: final-reply state and text
sequenceDiagram
participant LiveQARunner
participant PlaywrightPage
participant TriggerStore
LiveQARunner->>PlaywrightPage: run routine creation case
LiveQARunner->>TriggerStore: read trigger record count
TriggerStore-->>LiveQARunner: count before success
LiveQARunner->>TriggerStore: poll until count increases
TriggerStore-->>LiveQARunner: updated count and wait time
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 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.
Code Review
This pull request introduces asynchronous polling to wait for a trigger record to be added after a routine is created, replacing a single immediate check. It adds the _wait_for_trigger_record_after_count helper function, integrates it into _routine_creation_case, and updates the corresponding unit tests. Feedback suggests wrapping the database count check in a try-except block to handle potential sqlite3.Error exceptions due to transient database locks during concurrent execution.
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.
| last_count = _trigger_record_count(reborn_home, routine_name) | ||
| while last_count <= before_count: | ||
| now = time.monotonic() | ||
| if now >= deadline: | ||
| break | ||
| await asyncio.sleep(min(poll_interval, deadline - now)) | ||
| last_count = _trigger_record_count(reborn_home, routine_name) |
There was a problem hiding this comment.
Since the ironclaw-reborn serve process runs concurrently and writes to the SQLite database, transient database locks (sqlite3.OperationalError: database is locked) can occur. Wrapping the _trigger_record_count call in a helper that catches sqlite3.Error and defaults to before_count (to keep polling) would make the helper much more resilient to these transient errors and prevent flaky test failures.
| last_count = _trigger_record_count(reborn_home, routine_name) | |
| while last_count <= before_count: | |
| now = time.monotonic() | |
| if now >= deadline: | |
| break | |
| await asyncio.sleep(min(poll_interval, deadline - now)) | |
| last_count = _trigger_record_count(reborn_home, routine_name) | |
| def get_count() -> int: | |
| try: | |
| return _trigger_record_count(reborn_home, routine_name) | |
| except sqlite3.Error: | |
| return before_count | |
| last_count = get_count() | |
| while last_count <= before_count: | |
| now = time.monotonic() | |
| if now >= deadline: | |
| break | |
| await asyncio.sleep(min(poll_interval, deadline - now)) | |
| last_count = get_count() |
References
- To prevent flaky tests, avoid assertions based on wall-clock time. Instead, verify state changes by comparing values (e.g., counts) before and after an action.
There was a problem hiding this comment.
✅ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| ✅ Approved | 0 | 0 | 0 | e4938afd9908 |
Head: e4938afd9908dbb3ba1178da6a2e03241adaa330
Next: No reviewer action needed.
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
No concrete actionable issues found in the routine trigger-record wait change or its focused unit coverage.
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. - Use
@ironloopai statusto check queued/running/completed/failed/superseded state while reviewers run.
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.13% — 283982 / 333600 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 (4 entry/entries excluded from the accounting above)
|
There was a problem hiding this comment.
✅ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| ✅ Approved | 0 | 0 | 0 | c61a2fb70a10 |
Head: c61a2fb70a10a2668d3265799ed8d2bc3692cda3
Next: No reviewer action needed.
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
No concrete blocking issues found in the PR changes. The diff adds a DOM-visible assistant final-reply state and updates the live QA waits/tests consistently.
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. - Use
@ironloopai statusto check queued/running/completed/failed/superseded state while reviewers run.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@scripts/reborn_webui_v2_live_qa/run_live_qa.py`:
- Around line 1451-1457: The attribute read fallback in run_live_qa.py is
clearing a previously known non-final state by setting last_final_reply_state to
None on exception, which later allows the quiet-period and semantic fallback
paths to run incorrectly. Update the try/except around
assistant.get_attribute("data-final-reply", ...) so a transient read failure
preserves the prior explicit value of last_final_reply_state rather than
overwriting it, and ensure the downstream fallback checks continue to respect
that state.
🪄 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: e096609e-bc09-4684-b80b-03a0af191dff
📒 Files selected for processing (4)
crates/ironclaw_webui_v2/frontend/src/pages/chat/components/message-bubble.test.tscrates/ironclaw_webui_v2/frontend/src/pages/chat/components/message-bubble.tsxscripts/reborn_webui_v2_live_qa/run_live_qa.pyscripts/reborn_webui_v2_live_qa/test_run_live_qa.py
|
🚅 Deployed to the ironclaw-pr-5894 environment in ironclaw-ci-preview
|
Summary
Validation