feat(dashboard): align chat bubbles to request/response sides and add…#135
feat(dashboard): align chat bubbles to request/response sides and add…#135SantiagoDePolonia merged 4 commits intomainfrom
Conversation
… collapsible function notes User/request messages positioned left, agent/response messages right to reflect audit log request-response flow. Function calls (from AI) align right, function results (from client) align left. Function notes are now collapsible via <details> — showing one-line summary by default with expandable full content. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughRenames the admin "conversation" UI to "interactions", adds client-side mapping/attachment of tool call IDs to messages, and introduces UI and CSS to render collapsible function-call/function-result notes and a tool-call list in the audit drawer. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser
participant Template as Server Template
participant DrawerJS as conversation-drawer.js
participant AuditAPI as Audit Response
Browser->>Template: open admin page / click audit trigger
Browser->>DrawerJS: request interactions for entry
DrawerJS->>AuditAPI: fetch audit entry (request/response bodies)
AuditAPI-->>DrawerJS: return entry (includes tool call ids)
DrawerJS->>DrawerJS: _collectCallIds(map, requestBody, responseBody)
DrawerJS->>DrawerJS: _extractToolCalls(collectedCallIds)
DrawerJS->>Browser: emit messages enriched with toolCalls + functionName
Browser->>Template: render messages (chat bubbles, .chat-function-note, tool-calls footer)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/admin/dashboard/static/js/modules/conversation-drawer.js (1)
183-193:⚠️ Potential issue | 🟠 MajorPreserve
requestBody.inputorder in a single pass.This block emits all messages from
extractResponsesInputMessages(...)first and then appends everyfunction_call_output. IfrequestBody.inputmixes normal messages and function outputs, the reconstructed interaction order is wrong.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/admin/dashboard/static/js/modules/conversation-drawer.js` around lines 183 - 193, The code currently calls extractResponsesInputMessages(requestBody.input) first and then separately iterates requestBody.input to append function_call_output items, which breaks original ordering when inputs mix types; change the logic to do a single pass when requestBody.input is an array: iterate requestBody.input once and for each item, if item is a function_call_output produce the function_result message (use extractText for non-string outputs and use item.call_id or '' for call id) and otherwise pass the item into extractResponsesInputMessages (or extract its messages) and push those messages via this._conversationMessage to the messages array; keep the existing behavior for non-array requestBody.input by still calling extractResponsesInputMessages(requestBody.input). Ensure you still increment idx for every pushed message and use the same fields (m.role, m.text, ts, entry.id, isAnchor) and the same creation of function_result messages (role 'function_result', text, ts, entry.id, isAnchor, ++idx, [], item.call_id || '').
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/admin/dashboard/static/css/dashboard.css`:
- Around line 1421-1459: The anchor border-color is being overridden because
.chat-function-note.is-anchor and .chat-function-note.role-function-* share
equal specificity; add more specific rules that target both states together
(e.g. .chat-function-note.is-anchor.role-function-call and
.chat-function-note.is-anchor.role-function-result) and set the desired
border-color (the same color-mix used in .chat-function-note.is-anchor) so
anchored function notes retain the anchor border color even when role classes
are present.
In `@internal/admin/dashboard/static/js/modules/conversation-drawer.js`:
- Around line 165-167: The UI is using m.name (which is not provided) to label
tool results, so change the logic that builds the message label in
conversation-drawer.js (where this._conversationMessage is called for role ===
'tool' and in the other affected blocks) to resolve the function name by
matching m.tool_call_id or m.call_id against the original call records (the
tool_calls[] entries or the function_call item on the corresponding response
choice) instead of using m.name; implement a small helper (e.g.,
getFunctionNameFromCallId(callId)) that searches the conversation/response
metadata for the matching tool_call entry or function_call and returns its name,
then pass that name into this._conversationMessage (falling back to an empty
string only if no match is found).
In `@internal/admin/dashboard/templates/index.html`:
- Line 492: The Close button lacks an explicit type which makes it default to
"submit" and may inadvertently submit a surrounding form; update the button
element (x-ref="conversationCloseBtn", `@click`="closeConversation()") to include
type="button" so it won't trigger form submission when clicked.
- Around line 515-520: The x-for loop uses a non-unique key (:key="tc.name")
which can break Alpine's DOM diffing when the same tool is called multiple
times; update the template to include the loop index (e.g., change x-for to "
(tc, idx) in (msg.toolCalls || [])" ) and make the key unique (e.g.,
:key="tc.name + '-' + idx" ), or use a unique identifier on the tool-call
objects (tc.id) if available, ensuring each chat-tool-call row has a truly
unique :key.
---
Outside diff comments:
In `@internal/admin/dashboard/static/js/modules/conversation-drawer.js`:
- Around line 183-193: The code currently calls
extractResponsesInputMessages(requestBody.input) first and then separately
iterates requestBody.input to append function_call_output items, which breaks
original ordering when inputs mix types; change the logic to do a single pass
when requestBody.input is an array: iterate requestBody.input once and for each
item, if item is a function_call_output produce the function_result message (use
extractText for non-string outputs and use item.call_id or '' for call id) and
otherwise pass the item into extractResponsesInputMessages (or extract its
messages) and push those messages via this._conversationMessage to the messages
array; keep the existing behavior for non-array requestBody.input by still
calling extractResponsesInputMessages(requestBody.input). Ensure you still
increment idx for every pushed message and use the same fields (m.role, m.text,
ts, entry.id, isAnchor) and the same creation of function_result messages (role
'function_result', text, ts, entry.id, isAnchor, ++idx, [], item.call_id || '').
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 561ef3ba-efbb-416e-bf46-147351cdeb8a
📒 Files selected for processing (3)
internal/admin/dashboard/static/css/dashboard.cssinternal/admin/dashboard/static/js/modules/conversation-drawer.jsinternal/admin/dashboard/templates/index.html
internal/admin/dashboard/static/js/modules/conversation-drawer.js
Outdated
Show resolved
Hide resolved
- Fix CSS specificity so anchor border-color is not overridden by role-specific function note styles - Resolve tool result function names by matching tool_call_id against call records instead of relying on m.name - Add type="button" to Close button to prevent implicit form submission - Use indexed key in tool-calls x-for loop to avoid duplicate keys - Process requestBody.input in a single pass to preserve original ordering of messages and function_call_output items Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/admin/dashboard/templates/index.html`:
- Line 502: The article element that renders function notes (conditional classes
using msg.role === 'function_call' || msg.role === 'function_result' and
classNames msg.roleClass / msg.isAnchor) lacks a screen-reader label; update the
<article> rendered for 'chat-function-note' to include an appropriate ARIA
attribute (e.g., aria-label or aria-labelledby, or role="region" + aria-label)
that describes it as a function call/result (for example "Function call details"
or include the function name from the message data) so assistive tech can
announce the purpose of the collapsible <details> content; keep the attribute
conditional only for messages with msg.role === 'function_call' || msg.role ===
'function_result'.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 66c3216f-3532-4a19-8215-21f9b10d41d7
📒 Files selected for processing (3)
internal/admin/dashboard/static/css/dashboard.cssinternal/admin/dashboard/static/js/modules/conversation-drawer.jsinternal/admin/dashboard/templates/index.html
| <template x-for="msg in conversationMessages" :key="msg.uid"> | ||
| <article class="chat-message" :class="[msg.roleClass, msg.isAnchor ? 'is-anchor' : '']"> | ||
| <header class="chat-message-meta"> | ||
| <article :class="[msg.role === 'function_call' || msg.role === 'function_result' ? 'chat-function-note' : 'chat-message', msg.roleClass, msg.isAnchor ? 'is-anchor' : '']"> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider accessibility: missing role/ARIA attributes on function note article.
The article element conditionally becomes either a .chat-function-note or .chat-message based on role. For screen reader users, the nested <details> inside .chat-function-note is accessible, but consider adding aria-label to clarify the function note's purpose since it lacks a visible header like regular messages.
♿ Optional accessibility improvement
- <article :class="[msg.role === 'function_call' || msg.role === 'function_result' ? 'chat-function-note' : 'chat-message', msg.roleClass, msg.isAnchor ? 'is-anchor' : '']">
+ <article :class="[msg.role === 'function_call' || msg.role === 'function_result' ? 'chat-function-note' : 'chat-message', msg.roleClass, msg.isAnchor ? 'is-anchor' : '']"
+ :aria-label="(msg.role === 'function_call' || msg.role === 'function_result') ? msg.roleLabel : null">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <article :class="[msg.role === 'function_call' || msg.role === 'function_result' ? 'chat-function-note' : 'chat-message', msg.roleClass, msg.isAnchor ? 'is-anchor' : '']"> | |
| <article :class="[msg.role === 'function_call' || msg.role === 'function_result' ? 'chat-function-note' : 'chat-message', msg.roleClass, msg.isAnchor ? 'is-anchor' : '']" | |
| :aria-label="(msg.role === 'function_call' || msg.role === 'function_result') ? msg.roleLabel : null"> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/admin/dashboard/templates/index.html` at line 502, The article
element that renders function notes (conditional classes using msg.role ===
'function_call' || msg.role === 'function_result' and classNames msg.roleClass /
msg.isAnchor) lacks a screen-reader label; update the <article> rendered for
'chat-function-note' to include an appropriate ARIA attribute (e.g., aria-label
or aria-labelledby, or role="region" + aria-label) that describes it as a
function call/result (for example "Function call details" or include the
function name from the message data) so assistive tech can announce the purpose
of the collapsible <details> content; keep the attribute conditional only for
messages with msg.role === 'function_call' || msg.role === 'function_result'.
…delegating to helper extractResponsesInputMessages was designed for the full input, not individual items. Process function_call, function_call_output, and role-based message items explicitly in the single-pass loop, falling back to the helper only for non-array inputs (plain strings). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add overflow:hidden to body via conversation-drawer-open class toggled on open/close. Make #interactions-drawer-content the scrollable region with flex:1 and overflow-y:auto so the drawer scrolls independently. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/admin/dashboard/static/js/modules/conversation-drawer.js (1)
303-316: 🧹 Nitpick | 🔵 TrivialConsider using an options object for optional parameters.
The signature now has 8 positional parameters, with the last two (
toolCalls,functionName) being optional. This works correctly but may become harder to maintain as more fields are added.♻️ Optional refactor using options object
- _conversationMessage(role, text, timestamp, entryID, isAnchor, idx, toolCalls, functionName) { + _conversationMessage(role, text, timestamp, entryID, isAnchor, idx, opts = {}) { + const { toolCalls, functionName } = opts; const normalized = this._roleMeta(role); return { uid: entryID + '-' + idx,Then callers would use:
this._conversationMessage('function_result', text, ts, entry.id, isAnchor, ++idx, { functionName: fnName }) this._conversationMessage('function_call', '', ts, entry.id, isAnchor, ++idx, { toolCalls: [...] })
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d55ff2fd-d4f1-410e-b0ab-b1038993df4e
📒 Files selected for processing (1)
internal/admin/dashboard/static/js/modules/conversation-drawer.js
… collapsible function notes
User/request messages positioned left, agent/response messages right to reflect audit log request-response flow. Function calls (from AI) align right, function results (from client) align left. Function notes are now collapsible via
Details
— showing one-line summary by default with expandable full content.Summary by CodeRabbit
New Features
Style
Chores