fix(webui-v2): remove duplicate chat logs header - #5491
Conversation
italic-jinxin
added
size: M
|
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 (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughChat moves logs navigation into ChangesChat logs navigation update
Message submit retry handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 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 |
italic-jinxin
added
the
skip-regression-check
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the scoped logs link header bar from the active chat thread view, along with its associated imports, mock functions, and helper logic. The corresponding test suite has been updated to assert that this link and its header container are no longer rendered. There are no review comments, so I have no feedback to provide.
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/chat.test.mjs`:
- Around line 383-393: The chat test contains a redundant assertion because
findComponent(tree, components.Link) already searches the full tree recursively,
so the additional check against messageList in the same test does not add
independent coverage. Update the test around the MessageList and Link assertions
by removing the duplicate subtree assertion, keeping only the broader check that
verifies the active chat does not render the scoped logs link.
🪄 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: fd1fbc4a-0b9e-4181-b847-51d2c740021e
📒 Files selected for processing (2)
crates/ironclaw_webui_v2_static/static/js/pages/chat/chat.jscrates/ironclaw_webui_v2_static/static/js/pages/chat/lib/chat.test.mjs
💤 Files with no reviewable changes (1)
- crates/ironclaw_webui_v2_static/static/js/pages/chat/chat.js
512c85c to
adf256e
Compare
|
🚅 Deployed to the ironclaw-pr-5491 environment in ironclaw-ci-preview
|
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
`@crates/ironclaw_webui_v2_static/static/js/pages/chat/components/message-list.js`:
- Line 212: Duplicate URL construction is implemented inline in message-list.js
instead of reusing the shared buildScopedLogsPath helper. Update the
message-list component to call buildScopedLogsPath from logs-data.js for the
logsPath value, passing the current threadId in the same scope shape and
preserving the existing relative/absolute behavior so the path logic stays
centralized and consistent.
🪄 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: 691f921e-20f4-4b0f-a2d9-c2b1da3dc37a
📒 Files selected for processing (4)
crates/ironclaw_webui_v2_static/static/js/design-system/icons.jscrates/ironclaw_webui_v2_static/static/js/pages/chat/components/message-list.jscrates/ironclaw_webui_v2_static/static/js/pages/chat/components/message-list.test.mjscrates/ironclaw_webui_v2_static/static/js/pages/chat/lib/chat.test.mjs
|
@claude review |
This comment was marked as resolved.
This comment was marked as resolved.
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 `@tests/e2e/reborn_webui_harness.py`:
- Around line 469-486: The retry loop in the message submission flow currently
only retries on the "rejected_busy" outcome, so transient httpx.HTTPError
failures from _submit_message will abort the whole attempt budget. Update the
retry logic in the submit-message loop to catch and retry httpx.HTTPError
similarly to the fetch_timeline poll loop, preserving the existing
ACCEPTED_SEND_OUTCOMES and "rejected_busy" handling while allowing brief network
hiccups to continue through the remaining retries.
🪄 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: b17ffc7e-9057-4dba-8484-7e709ceb82ed
📒 Files selected for processing (5)
crates/ironclaw_webui_v2_static/static/js/pages/chat/chat.jscrates/ironclaw_webui_v2_static/static/js/pages/chat/components/message-list.jscrates/ironclaw_webui_v2_static/static/js/pages/chat/components/message-list.test.mjscrates/ironclaw_webui_v2_static/static/js/pages/chat/lib/chat.test.mjstests/e2e/reborn_webui_harness.py
|
|
||
| export const BOTTOM_FOLLOW_THRESHOLD_PX = 100; | ||
| const TOP_LOAD_THRESHOLD_PX = 100; | ||
| const FLOATING_LOGS_BUTTON_CLASS = |
There was a problem hiding this comment.
Why do we need this floating logs button at right bottom corner?
There was a problem hiding this comment.
there were duplicate logs at the top before. That was because the Logs button that only appears during chat was linking to the thread log. Moved it to the bottom and changed it into a floating button.
Another option is to show the Log button under each message, together with the Copy button.
| ) -> None: | ||
| """Send a text turn and wait until ``expected`` assistant replies finalize.""" | ||
| await send_message(client, base_url, thread_id, content) | ||
| submit_body: dict = {} |
There was a problem hiding this comment.
Why do we need to change this function?
There was a problem hiding this comment.
That was changed because the E2E tests were failing.
now wrapped with try catch block,
This branch was successfully deployed
2 participants
Summary
/v2/logs?thread_id=<threadId>.Linked Issue
Closes #5458
Validation
node --test crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/chat.test.mjsnode --test crates/ironclaw_webui_v2_static/static/js/pages/chat/components/message-list.test.mjsnode --test crates/ironclaw_webui_v2_static/static/js/pages/chat/components/message-bubble.test.mjsgit diff --checkSecurity Impact
No. UI-only change; no changes to log APIs, auth, permissions, or data returned.
Database Impact
No schema or migration changes.
Blast Radius
Limited to WebUI v2 Chat message-list presentation and focused tests.
Rollback Plan
Revert this PR to restore the previous active-run Logs shortcut behavior.