feat(orchestration): chat-UI redesign — agent chat as normal chat window + connections sessions#4713
Conversation
…t-window session history Redesign the Connections sub-tab (only) into a list of connections WITH their sessions: each contact expands to its sessions (status · preview · message count), and opening a session shows its conversation history in place — rendered in the app's normal chat-window style — with an inline reply composer. Stays in the same OrchestrationPage shell + max-w-3xl column. - New shared SessionTranscript renderer: chat-window bubbles (blue user / neutral agent) + inline v2 activity (merged tool call+result via mergeToolActivity, red on failure, thinking, error, approval). Reusable by the Agent chat next. - New hooks useContactSessions (sessions grouped by contact agentId) and useSessionTranscript (per-session messages), socket-refreshed. - Client types gain the tool_result outcome (ok/isError/exitCode) + messageCount, consumed from the core change in tinyhumansai#4712. - Full Vitest coverage for the panel, renderer, hooks, and merge helper; i18n keys added across all 14 locales. Agent-chat redesign (thread rail + session drawer) follows in a separate PR that reuses SessionTranscript.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 53 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: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR reworks orchestration UI panels to support inline session viewing/replying. It adds a ChangesSession UI, hooks, and transcript rendering
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa028ea1b2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| export function toolResultFailed( | ||
| result: Pick<ChatMessage, 'ok' | 'isError' | 'exitCode'> | ||
| ): boolean { | ||
| return result.isError === true || (result.exitCode ?? 0) > 0 || result.ok === false; |
There was a problem hiding this comment.
In the actual RPC path these fields never arrive: the core OrchestrationMessage DTO only serializes eventKind/toolName/callId, and ingest does not copy ToolResultPayload.ok/is_error/exit_code into the stored message. For any failed tool_result from a real harness, result.isError, result.exitCode, and result.ok are therefore all undefined here, so failed tools render with the success styling instead of the red failure state. Please persist/return these outcome fields, or derive failure before storing, before relying on them in the transcript.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct that main doesn't carry these yet — the core side is added in the companion PR #4712 (adds ok/is_error/exit_code to OrchestrationMessage, decoded from ToolResultPayload in ingest and persisted via additive migration-guarded columns). This frontend PR stacks on it; the client fields are optional so until #4712 merges a failed tool_result simply renders with the neutral (non-red) styling rather than crashing. mergeToolActivity deliberately reads the fields defensively for that reason. Once #4712 lands the red failure state activates end-to-end.
| return <ApprovalRow key={message.id} message={message} onDecide={onDecide} />; | ||
| default: | ||
| // agent_message + legacy v1 rows (from user/agent) → bubble by sender. | ||
| return message.from === 'you' ? ( |
There was a problem hiding this comment.
When the new reply composer sends into a session, the core mirrors that message back with role: "owner" and no eventKind; mapTranscriptMessage then sets from to "owner". After the refresh, this strict message.from === 'you' check is false, so the user's own replies in the session history move to the left-side agent bubble. Please map owner/user-authored rows to the localized “you” value or include owner in the user-bubble condition.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in ce0b56a. SessionTranscript now treats owner/user-authored rows (from ∈ {you, owner, user}) as right-side user bubbles, so a composer reply mirrored back with role: "owner" stays on the right after refresh. Added a regression test.
…sion side-tab Rebuild the Agent sub-tab to match the app's normal chat window: a ThreadList-style rail (Main agent / Subconscious) beside a centered pane rendered with the shared SessionTranscript (chat-window bubbles + inline harness activity), keeping the subconscious steering header + Master composer. Add the session side-tab: when the agent engages a fleet session parked on an approval, an inline 'View session' card surfaces below the thread; clicking it opens a right-hand drawer with that session's live chat + a reply composer. It never auto-opens — the user chooses when. Replaces the OrchestrationFocusPane transcript wiring; adds the orchPage.agent.viewSession key across all 14 locales.
sanil-23
changed the title
…bles A composer reply is mirrored back with role "owner" (no eventKind), so the strict from === 'you' check pushed the user's own replies to the left agent bubble after refresh. Treat owner/user-authored rows as right-side user bubbles. (CodeRabbit/Codex P2.)
|
@coderabbitai review |
✅ Action performedReview finished.
|
coderabbitai
Bot
added
agent
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
app/src/lib/orchestration/mergeToolActivity.ts (1)
39-43: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider negative exit codes as failures too.
toolResultFailedonly flagsexitCode > 0. Some process/tool wrappers report negative exit codes for signal termination (e.g. Node'schild_processconvention of negative codes for signals). Those would currently render as "success" unlessisError/okalso happen to be set.♻️ Possible tweak
- return result.isError === true || (result.exitCode ?? 0) > 0 || result.ok === false; + return result.isError === true || (result.exitCode ?? 0) !== 0 || result.ok === false;🤖 Prompt for 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. In `@app/src/lib/orchestration/mergeToolActivity.ts` around lines 39 - 43, Update toolResultFailed so it treats negative exit codes as failures too; currently the exitCode check only catches values greater than zero, so adjust the logic in toolResultFailed to consider any non-zero exitCode (including negatives) alongside isError and ok when determining failure.
🤖 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 `@app/src/components/orchestration/AgentChatPanel.tsx`:
- Around line 273-296: The main pane in AgentChatPanel only handles
sessionsState.status === 'error', so messagesState.status === 'error' falls
through to the empty-state UI. Update the conditional rendering around the
transcript area to check messagesState.status for error as well, and show a
dedicated error message with the existing retry action instead of rendering the
generic “no messages” state when message loading fails.
- Around line 91-103: The transcript panel in AgentChatPanel is treating fetch
failures the same as empty sessions because it only checks loading and
messages.length; update the render logic to handle useSessionTranscript’s error
state explicitly. Add a branch for status === 'error' in AgentChatPanel (near
SessionTranscript and the loading/noMessages conditions) and show the returned
error message instead of the noMessages text, while keeping the empty-state copy
only for truly empty successful loads.
- Around line 358-360: The conditional rendering in AgentChatPanel for
SessionDrawer is reusing the same component instance across different sessions,
which can carry over local composer state like body and sending. Update the
SessionDrawer usage so each openSession gets a stable unique key derived from
the session identity, ensuring React remounts it when switching sessions. Keep
the fix localized to the openSession rendering block and preserve the existing
onClose behavior.
- Around line 48-67: The session reply submit flow in AgentChatPanel’s submit
handler currently sends sendMasterMessage without any rejection handling, so add
explicit failure handling to avoid unhandled promise rejections and surface an
error state to the user similar to the master composer’s masterError behavior.
Update the submit callback to catch sendMasterMessage failures, clear sending in
all cases, and store/display a reply-send error so the drawer doesn’t fail
silently. Also add debug logging for this new flow, following the
runSteeringReview pattern in the same component, with logs for both send start
and failure to aid troubleshooting.
In `@app/src/components/orchestration/ConnectionsPanel.tsx`:
- Around line 370-379: The SessionView is rendering a stale snapshot because
`open.session` is stored once in `newSession`/`onOpenSession` and never
refreshed. Update `ConnectionsPanel` so the open session is derived from the
live `sessions.byContact` data (or re-resolved from `open.address`) right before
rendering `SessionView`, and pass that live session instead of the captured
snapshot. Keep the existing `open` state for the selected address, but make
`SessionView` consume the current session data so `status`, `messageCount`, and
`label` stay in sync.
- Around line 98-113: The inline reply flow in ConnectionsPanel’s submit handler
currently swallows sendMasterMessage failures and has no debug logging. Update
the submit callback to handle the promise rejection with a catch path, emit a
debug log with a stable prefix and correlation fields (without secrets/PII), and
surface the failure to the user using the existing
tinyplaceOrchestration.composer.sendFailed translation key. Keep the existing
success path intact in submit and ensure setSending is cleared on both success
and failure.
In `@app/src/lib/orchestration/useOrchestrationSessions.ts`:
- Around line 65-75: Add verbose debug logging to the new session-fetch and
error paths in useOrchestrationSessions, including the socket-triggered refresh
and the catch blocks for useContactSessions and useSessionTranscript. Use stable
log prefixes and correlation fields (for example session or conversation
identifiers) and log the error branch before setting state, while avoiding
secrets or full PII. Keep the logging consistent with the existing orchestration
flow so these paths can be traced in debug output.
- Around line 126-174: The `useSessionTranscript` hook in
`useOrchestrationSessions.ts` can apply stale `messagesList()` results after
`sessionId` changes because `mountedRef` only tracks unmount, not request
freshness. Add a per-request guard inside `refresh` (for example a monotonically
increasing request/version token stored in a ref) and only call
`setMessages`/`setState` when the response matches the latest token. Update the
`useEffect`/`refresh` flow so switching sessions or out-of-order responses from
`orchestrationClient.messagesList` cannot overwrite the current session
transcript. Add a regression test covering a mid-flight `sessionId` swap where
the older request resolves after the newer one.
---
Nitpick comments:
In `@app/src/lib/orchestration/mergeToolActivity.ts`:
- Around line 39-43: Update toolResultFailed so it treats negative exit codes as
failures too; currently the exitCode check only catches values greater than
zero, so adjust the logic in toolResultFailed to consider any non-zero exitCode
(including negatives) alongside isError and ok when determining failure.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1c9534d0-0683-43f5-9938-71d4681fa0cc
📒 Files selected for processing (26)
app/src/components/orchestration/AgentChatPanel.tsxapp/src/components/orchestration/ConnectionsPanel.tsxapp/src/components/orchestration/SessionTranscript.tsxapp/src/components/orchestration/__tests__/AgentChatPanel.test.tsxapp/src/components/orchestration/__tests__/ConnectionsPanel.test.tsxapp/src/components/orchestration/__tests__/SessionTranscript.test.tsxapp/src/lib/i18n/ar.tsapp/src/lib/i18n/bn.tsapp/src/lib/i18n/de.tsapp/src/lib/i18n/en.tsapp/src/lib/i18n/es.tsapp/src/lib/i18n/fr.tsapp/src/lib/i18n/hi.tsapp/src/lib/i18n/id.tsapp/src/lib/i18n/it.tsapp/src/lib/i18n/ko.tsapp/src/lib/i18n/pl.tsapp/src/lib/i18n/pt.tsapp/src/lib/i18n/ru.tsapp/src/lib/i18n/zh-CN.tsapp/src/lib/orchestration/mergeToolActivity.test.tsapp/src/lib/orchestration/mergeToolActivity.tsapp/src/lib/orchestration/orchestrationClient.tsapp/src/lib/orchestration/useOrchestrationChats.tsapp/src/lib/orchestration/useOrchestrationSessions.test.tsapp/src/lib/orchestration/useOrchestrationSessions.ts
- useSessionTranscript: guard against a stale-session race with a monotonic
request token so a slow response for a previous sessionId can't overwrite the
current transcript (shared mountedRef alone couldn't guard it). [Critical]
- Surface + log reply-send failures in both session composers (Connections
SessionView + Agent SessionDrawer) via .catch() + the existing
composer.sendFailed key, instead of swallowing the rejection. [Major]
- key={sessionId} on the Agent SessionDrawer so a draft reply never leaks across
sessions. [Major]
- Add verbose debug logging to the new session fetch / transcript / reply flows
(stable prefixes, no PII). [Major]
- Handle transcript-load error state in the Agent drawer, and messagesState error
in the Agent main pane (no longer masked as 'no messages'). [Minor]
- Connections SessionView header reads the live (socket-refreshed) session row so
status / message count / label stay current. [Minor]
- Tests for the new reply-failure + transcript-error paths.
2 participants
Summary
SessionTranscriptrenderer (chat-window bubbles + inline v2 activity: mergedtool_call+tool_resultred on failure, thinking, error, approval) used by both surfaces, plusmergeToolActivityand theuseContactSessions/useSessionTranscripthooks.tool_resultoutcome (ok/isError/exitCode) +messageCountfrom the core change in feat(orchestration): surface tool_result outcome + per-session message_count #4712.Problem
The orchestration Agent tab rendered a glyph-style activity log rather than the app's chat window, and Connections was a flat contact list that never surfaced the sessions the agent runs with each peer or let you read/reply to one. Failed tool results were also indistinguishable from successes.
Solution
ConnectionsPanel→ accordion (contact → its sessions) + in-panel session view (transcript + reply viasendMasterMessage({ recipient, sessionId })); sessions grouped bySessionSummary.agentId.AgentChatPanel→ thread rail +SessionTranscriptpane + composer + steering header; on-demand session drawer driven by a "View session" card (sessions parked onwaiting-approval).mergeToolActivitypairstool_call/tool_resultbycallIdand derivesfailed(isError/ non-zeroexitCode/ok === false). The renderer gates activity oneventKind, so a v1 text session degrades to plain bubbles.Submission Checklist
mergeToolActivity,SessionTranscript(bubbles / failed-tool / read-only vs actionable approval),useOrchestrationSessions,ConnectionsPanel(expand→open→reply, requests, new-session),AgentChatPanel(rail switch, composer, steering, View-session→drawer→reply).docs/TEST-COVERAGE-MATRIX.md.Impact
pnpm i18n:checkclean).Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
pnpm --filter openhuman-app format:check— Prettier clean on changed files.pnpm typecheck— 0 errors.vitest run src/components/orchestration src/lib/orchestration— 8 files, 35 passed.app/src-taurinot touched.Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
ConnectionsPanel({ onDiscover })prop + shell/container kept; Master composer + subconscious steering-review path kept; optional wire fields default absent.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Bug Fixes