feat(conversations): per-turn process history (timeline refactor Phases 1–2, 4–5)#4612
Merged
senamakel merged 14 commits intoJul 7, 2026
Merged
Conversation
Audit of Conversations.tsx (rendering of tool calls/thoughts/subagents, ordering, storage/streaming, tab-switch context loss) and a 6-phase refactor plan: unified TimelineItem projection, component decomposition into features/conversations/, reducer-side merge/dedup consolidation, and the per-turn turn-state ring store (adopts per-turn-tool-timeline-history.md as the backend track). Claude-Session: https://claude.ai/code/session_01NsLNxq4vPbhverA3aCDtzs
…Phase 1a)
Mechanical move of `app/src/pages/conversations/{components,hooks,utils,
composerSendDecision,taskPlanActions}` to `app/src/features/conversations/`,
matching the `features/human/` layout per the timeline-refactor plan. Pure
relocation — no behavior change. `pages/` and `features/` sit at the same depth
under `app/src`, so the moved files' own imports are unchanged; only references
*to* the subtree were repointed (Conversations.tsx, pages/dev preview, and 9
external importers under components/{chat,intelligence,flows,notifications,
channels,layout}).
Typecheck + lint clean; 337 tests across the moved subtree and all consumers
(Conversations render suite, HumanPage, Accounts, IntelligenceTasksTab) pass.
Refs: docs/plans/conversations-timeline-refactor.md Phase 1
Claude-Session: https://claude.ai/code/session_01MVwyyQVNm8UZpvJyWrf2yo
…se 1b) Relocate the monolithic `Conversations` component to `app/src/features/conversations/Conversations.tsx`; `pages/Conversations.tsx` becomes a thin re-export shim so `HumanPage` and the co-located test suites keep their current import paths during migration (removed in Phase 6). Add `features/conversations/index.ts` as the public reusability contract. Resolve the `AgentChatPanel` name collision with the unrelated `components/settings/panels/AgentChatPanel.tsx`: the page-variant alias is renamed `ConversationsPage`; `Accounts.tsx` and its webview-selection test mock updated to match. All relative imports in the moved file deepened one level (`../` → `../../`); no behavior change. Typecheck + lint clean; 118 tests (Conversations render/attachments/overflow/ welcomeLock/helpers, HumanPage, Accounts) pass. Refs: docs/plans/conversations-timeline-refactor.md Phase 1 Claude-Session: https://claude.ai/code/session_01MVwyyQVNm8UZpvJyWrf2yo
Additive data model + memoized selector that project threadSlice messages and chatRuntimeSlice's tool timeline / streaming previews into one ordered `TimelineItem[]`. No renderer change yet — nothing consumes it, so zero regression risk; this locks the ordering/anchoring contract with tests ahead of the render-loop swap. - `timeline/types.ts`: the discriminated `TimelineItem` union (userMessage, assistantMessage, streamingText, reasoning, toolCall, subagentActivity), `TimelineTurn` grouping, `LEGACY_TURN_ID`, and status/subagent helpers. - `timeline/selectors.ts`: pure `buildThreadTimeline` + memoized `selectTimelineForThread`. Reproduces today's behavior exactly — messages in append order (hidden dropped), the single tool timeline anchored after the last user message, the proactive/no-user-message fallback trailing the messages, streaming previews (primary + forked branches) trailing durable items, and `hideAgentInsights` omitting the process kinds. Until Phase 4 stamps requestId, everything is one `legacy` turn. 17 unit tests (ordering, anchor splice, success→ok mapping, subagent classification, empty/proactive/hidden fallbacks, hideAgentInsights, streaming trailing, grouping, memoization stability). Typecheck + lint clean. Refs: docs/plans/conversations-timeline-refactor.md Phase 2 Claude-Session: https://claude.ai/code/session_01MVwyyQVNm8UZpvJyWrf2yo
… additive)
Pure presentational renderer driving off the `TimelineItem[]` projection: one
element per kind, wrapping the existing components. Consecutive process items
(toolCall/subagentActivity) coalesce into a single `ToolTimelineBlock`, matching
today's grouped `agentInsights` anchor; ordering/anchoring/`hideAgentInsights`
filtering stay upstream in the selector. Additive — not yet wired into the panel
(the live render-loop swap is the next, higher-risk step), so no behavior change.
- `timeline/ConversationTimeline.tsx` — kind dispatch + process-run coalescing +
`onOpenSubagent`/`onViewWholeRun` handler seam.
- `timeline/items/{UserMessageItem,AssistantMessageItem,StreamingTailItem}.tsx`
— assistant bubbles reuse `splitAgentMessageIntoBubbles` + positions; the
streaming tail reuses the 120-char ticker slice.
6 renderer tests (message order, process coalescing into one block, subagent
handler wiring, 120-char truncation + thinking details, forked branch tails,
empty timeline). Typecheck + lint clean.
Refs: docs/plans/conversations-timeline-refactor.md Phase 2
Claude-Session: https://claude.ai/code/session_01MVwyyQVNm8UZpvJyWrf2yo
…jection (Phase 2b swap) Route the live message list through `buildThreadTimeline` — the unified projection is now the single source of render order. With the streaming/tool inputs omitted the projection yields exactly the visible messages in order, so the rendered DOM is unchanged; the tool-timeline block and streaming previews stay anchored inline below as before. This wires the projection into the shipping render path ahead of the per-turn grouping in Phase 5. DOM-identical and verified: the full Conversations render suite + attachments/ overflow/welcomeLock + HumanPage + Accounts (141 tests) stay green, and three chat Playwright web specs (send+stream, scroll-render, multi-turn history persistence) pass in a real browser. Typecheck + lint clean. Note: deeper componentization — rendering messages via <ConversationTimeline> with the message chrome (attachments/reactions/citations/copy, hideAgentInsights variants, inference status) ported into item components — remains as follow-up; it is deferred because a byte-identical port is higher-risk and delivers no behavior change until Phase 4 stamps real per-turn requestIds. Refs: docs/plans/conversations-timeline-refactor.md Phase 2 Claude-Session: https://claude.ai/code/session_01MVwyyQVNm8UZpvJyWrf2yo
…timeline by turn (Phase 4) Rollout step 2 of the per-turn history plan (additive, frontend-only): stamp `extraMetadata.requestId` on every persisted assistant message so a turn's answer can be grouped with the process trail that produced it (Option B anchoring). Covered at all append sites in ChatRuntimeProvider — `chat_done` (via chatDoneExtraMetadata, incl. the parallel + segment-reconcile paths), `chat_segment` (merged alongside citations), `chat_interim` narration, and proactive messages. `addInferenceResponse` already threads extraMetadata to the persisted message, so this is a caller-side change only. The timeline projection now derives `turnId` from `extraMetadata.requestId`, falling back to the `legacy` turn for pre-migration messages — so `groupTimelineIntoTurns` yields real per-request turns once messages are stamped, while old threads keep today's single-anchor behavior. Tests: provider stamps requestId on chat_done; selector derives turnId from requestId, legacy fallback, and groups a mixed thread into per-request turns (20 selector + 59 provider tests green). Full Conversations render suite + typecheck + lint clean. Refs: docs/plans/conversations-timeline-refactor.md Phase 4; docs/plans/per-turn-tool-timeline-history.md step 2 Claude-Session: https://claude.ai/code/session_01MVwyyQVNm8UZpvJyWrf2yo
…d, step 1) Replace the single-snapshot-per-thread layout with a bounded per-turn ring: `turn_states/<hex(thread_id)>/<hex(request_id)>.json`, one file per turn, so a multi-turn thread retains every turn's tool timeline instead of only the latest (the root cause of tab-switch context loss). Completed turns prune to the newest 20 per thread; a live/non-completed turn is never pruned. Back-compat is preserved so no caller changes are needed: `get(thread_id)` and `list()` resolve the *latest* turn per thread, `delete`/`clear_all`/ `mark_all_interrupted` operate over the per-thread directory. New `get_turn(thread_id, request_id)` and `list_thread(thread_id)` expose the per-turn history. Old flat `<hex(thread_id)>.json` snapshots are migrated in place on first access (idempotent). Tests: 14 store tests pass — all 9 preserved (roundtrip, latest wins, delete, list one-per-thread, mark-interrupted, clear corrupted) plus 5 new (per-turn retention, get_turn/list_thread, retention prunes completed to N while keeping a live turn, idempotent legacy migration). Mirror (12) + threads::ops (34) tests still green. Refs: docs/plans/conversations-timeline-refactor.md Phase 4; docs/plans/per-turn-tool-timeline-history.md step 1 Claude-Session: https://claude.ai/code/session_01MVwyyQVNm8UZpvJyWrf2yo
…get_turn (Phase 4, step 3) Expose the per-turn ring store over JSON-RPC without disturbing the existing no-arg cold-boot `turn_state_list` or single-turn `turn_state_get`: - `threads.turn_state_history(threadId)` → every turn snapshot for a thread, newest first (the per-turn process history the UI groups by requestId). - `threads.turn_state_get_turn(threadId, requestId)` → one specific past turn, for lazily loading a collapsed turn's full timeline on first expand. Wires the new `GetTurnStateForRequestRequest` type, ops handlers, controller schemas (+ registry + parity test), and the `threadApi.getTurnStateHistory` / `getTurnStateForRequest` clients. Reuses the existing `ListTurnStatesResponse` / `GetTurnStateResponse` envelopes. Tests: threads schema parity (17) updated for the two new functions; json_rpc E2E extended — seeds two turns, asserts history returns both newest-first, get_turn fetches a specific turn, and an unknown request id returns null. TS typecheck + threadApi suite (528) green. Refs: docs/plans/conversations-timeline-refactor.md Phase 4; docs/plans/per-turn-tool-timeline-history.md step 3 Claude-Session: https://claude.ai/code/session_01MVwyyQVNm8UZpvJyWrf2yo
…se 5) Each past answer now keeps its own "agent insights" trail instead of losing it on the next send — the tab-switch context-loss fix made visible. - New `chatRuntimeSlice.turnTimelinesByThread` (threadId → requestId → entries) holds the *settled* turns' timelines, hydrated by `fetchAndHydrateTurnHistory` from the `turn_state_history` RPC on thread open. It deliberately survives a new send (unlike `toolTimelineByThread`), and excludes the newest turn (that one still renders as the live/anchored insights via `getTurnState`), so no turn double-renders. - The render loop maps each older turn's timeline to the first assistant message of its `requestId` group and renders a collapsed `ToolTimelineBlock` above it. Purely additive: threads without hydrated history (legacy messages, existing tests) render exactly as before. Tests: thunk stores older settled turns by requestId while skipping the latest and filtering non-settled lifecycles, and swallows transport errors; render test asserts a past turn's trail appears once above its answer only after hydration. Full Conversations render suite (68) + slice/provider/thunk suites (120) green; typecheck + lint clean. Refs: docs/plans/conversations-timeline-refactor.md Phase 5; docs/plans/per-turn-tool-timeline-history.md step 4 Claude-Session: https://claude.ai/code/session_01MVwyyQVNm8UZpvJyWrf2yo
Apply Prettier and cargo fmt to the files touched across Phases 1–5 so the CI format gate (pnpm format:check) passes. No behavior change. Claude-Session: https://claude.ai/code/session_01MVwyyQVNm8UZpvJyWrf2yo
Contributor
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 40 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 (11)
📝 WalkthroughWalkthroughThis PR relocates conversation-related components/utilities from ChangesFrontend Conversations Timeline Feature
Backend Per-Turn State Persistence
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 |
coderabbitai
Bot
added
feature
![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: dbe4e326de
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (const turn of history.slice(1)) { | ||
| if (turn.lifecycle !== 'completed' && turn.lifecycle !== 'interrupted') continue; | ||
| if (!turn.requestId || turn.toolTimeline.length === 0) continue; | ||
| timelines[turn.requestId] = turn.toolTimeline.map(toolTimelineFromPersisted); |
There was a problem hiding this comment.
When an older turn is interrupted (which this history path explicitly includes), its persisted snapshot can still contain tool/subagent rows with status: 'running' because the live driver died before emitting completion events. Hydrating those rows directly makes a reopened thread render stale past-turn insights as still running/pulsing, whereas the latest-snapshot path already applies settleOrphanedTimelineEntry; apply the same settling for interrupted history before storing these timelines.
Useful? React with 👍 / 👎.
8 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/store/chatRuntimeSlice.ts (1)
1408-1426: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
clearRuntimeForThreaddoesn't clearturnTimelinesByThread, leaking per-thread state on delete/reset.
clearAllChatRuntimeresetsturnTimelinesByThreadglobally, but the per-threadclearRuntimeForThreadreducer (which clears every other per-thread map, e.g.toolTimelineByThread,processingByThread,taskBoardByThread) omits it. UnlikeartifactsByThread, which has an explicit comment justifying its exclusion, this omission appears unintentional. Once a thread is deleted, itsturnTimelinesByThread[threadId]entry is never freed.🛠️ Proposed fix
clearRuntimeForThread: (state, action: PayloadAction<{ threadId: string }>) => { delete state.inferenceStatusByThread[action.payload.threadId]; delete state.streamingAssistantByThread[action.payload.threadId]; delete state.inferenceHeartbeatByThread[action.payload.threadId]; ... delete state.toolTimelineByThread[action.payload.threadId]; + delete state.turnTimelinesByThread[action.payload.threadId]; delete state.processingByThread[action.payload.threadId];🤖 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/store/chatRuntimeSlice.ts` around lines 1408 - 1426, clearRuntimeForThread is missing cleanup for turnTimelinesByThread, leaving per-thread timeline state behind after thread delete/reset. Update the clearRuntimeForThread reducer in chatRuntimeSlice to delete the matching turnTimelinesByThread[threadId] entry alongside the other per-thread maps already being cleared, following the same pattern used for toolTimelineByThread, processingByThread, and taskBoardByThread; keep any intentional exclusions like artifactsByThread unchanged.
🧹 Nitpick comments (7)
app/src/features/conversations/timeline/items/AssistantMessageItem.tsx (1)
34-38: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnnecessary
Fragmentwrapping a single child.
<Fragment key={index}>wrapping one<AgentMessageBubble>adds no value here — thekeycan go directly onAgentMessageBubble.♻️ Proposed simplification
- return ( - <Fragment key={index}> - <AgentMessageBubble content={segment} position={position} /> - </Fragment> - ); + return <AgentMessageBubble key={index} content={segment} position={position} />;🤖 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/features/conversations/timeline/items/AssistantMessageItem.tsx` around lines 34 - 38, The map render in AssistantMessageItem is wrapping a single AgentMessageBubble in a Fragment just to carry the key, which is unnecessary. Remove the Fragment and put the key directly on AgentMessageBubble inside the segment rendering logic so the structure is simpler while preserving list identity.app/src/features/conversations/timeline/ConversationTimeline.test.tsx (2)
94-108: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFragile CSS-class selector for asserting the content bubble.
container.querySelector('.font-mono.text-sm')(Line 106) couples the test toStreamingTailItem's exact Tailwind classes; a future style tweak would silently break this assertion without indicating an actual behavior regression. Consider adding adata-testidon the content wrapper instead.🤖 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/features/conversations/timeline/ConversationTimeline.test.tsx` around lines 94 - 108, The test in ConversationTimeline.test.tsx is using a fragile CSS-class query to locate the streaming content bubble, which ties it to StreamingTailItem’s Tailwind styling instead of behavior. Update the component to expose a stable test hook such as a data-testid on the content wrapper, then change the assertion in the ConversationTimeline test to target that identifier rather than .font-mono.text-sm.
1-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTests use the real production
storeinstead of a test-scoped store.
renderInStorewraps components with the actual appstore(Line 5, 12) rather than a minimal test store built fromtestRootReducer. This works for now since no state is read, but it couples this test's stability to whatever middleware/side effects the real store initializes (sockets, persistence, etc.), and any future selector usage inside these components would read live default state rather than controlled fixtures.🤖 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/features/conversations/timeline/ConversationTimeline.test.tsx` around lines 1 - 13, The test helper renderInStore is using the real app store instead of an isolated test store, which makes ConversationTimeline.test.tsx depend on production middleware and default state. Replace the imported store with a test-scoped store created from testRootReducer (or an equivalent minimal test setup) inside renderInStore, and keep ConversationTimeline and buildThreadTimeline tests wired to controlled fixture state so the test remains deterministic.app/src/features/conversations/timeline/ConversationTimeline.tsx (1)
41-63: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCoalescing loop and process-item mapping look correct.
Anchoring/ordering logic is delegated upstream as documented, and this component purely projects; the cast on Line 50 is redundant given the
isProcessItemguard on thewhilecondition but harmless.🤖 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/features/conversations/timeline/ConversationTimeline.tsx` around lines 41 - 63, The coalescing logic in ConversationTimeline and the ToolTimelineBlock rendering are already correct, so no behavioral change is needed; the only cleanup is to remove the redundant type cast inside the while loop where isProcessItem already narrows items[j]. Keep the existing run collection, keying by startId, and upstream ordering assumptions intact.app/src/providers/ChatRuntimeProvider.tsx (1)
1046-1053: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd debug logging for the new requestId stamping.
onSegment,onInterim, andonProactiveMessagenow stamprequestIdonto persisted messages — the core signal the new per-turn timeline grouping relies on — but none of them log it. Ifrequest_idis ever missing/empty here, there's currently no diagnostic trail to explain a turn silently failing to group with its process trail.As per path instructions: "On new or changed flows, add verbose debug logging with stable prefixes, correlation fields, and no secrets or full PII; incomplete changes are those lacking logging."
Also applies to: 1068-1076, 1238-1245
🤖 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/providers/ChatRuntimeProvider.tsx` around lines 1046 - 1053, Add verbose debug logging around the new requestId stamping in ChatRuntimeProvider so missing or empty request_id values can be diagnosed; update the onSegment, onInterim, and onProactiveMessage flows that build extraMetadata to log a stable debug prefix plus correlation fields (such as turn/message identifiers and whether requestId was stamped) without secrets or full PII. Use the existing requestId/request_id handling in ChatRuntimeProvider to log both the success path when extraMetadata includes requestId and the fallback path when it is absent, so failures to group a turn with its process trail are traceable.Source: Path instructions
src/openhuman/threads/turn_state/store.rs (2)
261-434: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
migrate_all_legacy_lockedsilently drops write failures.
migrate_thread_lockedlogs awarn!whenwrite_turn_filefails (line 376-379).migrate_all_legacy_locked's equivalent branch (if self.write_turn_file(&state).is_ok() { ... }) does nothing at all when it returnsErr— no log, no trace. A legacy file that repeatedly fails to migrate during a bulk scan will be silently retried forever with zero diagnostic signal, unlike the per-thread path.🔧 Proposed fix for parity with `migrate_thread_locked`
match read_snapshot(&path) { Ok(state) => { - if self.write_turn_file(&state).is_ok() { - let _ = fs::remove_file(&path); - debug!( - "{LOG_PREFIX} migrated legacy snapshot thread={} request={}", - state.thread_id, state.request_id - ); - } + match self.write_turn_file(&state) { + Ok(()) => { + if let Err(err) = fs::remove_file(&path) { + warn!( + "{LOG_PREFIX} migrate: removed-into-dir but flat delete failed {}: {err}", + path.display() + ); + } else { + debug!( + "{LOG_PREFIX} migrated legacy snapshot thread={} request={}", + state.thread_id, state.request_id + ); + } + } + Err(err) => warn!( + "{LOG_PREFIX} legacy migrate write failed {}: {err} (flat file kept)", + path.display() + ), + } }🤖 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 `@src/openhuman/threads/turn_state/store.rs` around lines 261 - 434, The bulk migration path in migrate_all_legacy_locked is swallowing write errors when write_turn_file fails, unlike migrate_thread_locked which logs a warn! for the same failure. Update migrate_all_legacy_locked to handle the Err case from write_turn_file explicitly and emit a warning with the legacy file path and error details, keeping behavior and diagnostics consistent with migrate_thread_locked.
143-173: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftCold-boot
list()now reads/parses every retained turn per thread just to pick the latest.Previously each thread had exactly one flat snapshot file, so
list()was O(threads). Nowread_thread_turns(called fromlist()andmark_all_interrupted()) opens and fully deserializes up toCOMPLETED_RETENTION + 1(21) turn files per thread just to computelatest_turn, making both cold-boot paths O(threads × 21). For workspaces with many long-lived threads this meaningfully increases startup I/O/CPU versus the prior single-file-per-thread design, and both of these cold-boot paths (list()andmark_all_interrupted()) do this scan independently, doubling the cost.A cheaper design (e.g., a small per-thread "latest pointer" file, or filename-embedded timestamps enabling a metadata-only comparison before parsing) would let
get/listavoid parsing every retained completed turn just to find the newest one.🤖 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 `@src/openhuman/threads/turn_state/store.rs` around lines 143 - 173, The cold-boot path in TurnStateStore::list is fully deserializing every retained turn via read_thread_turns just to compute latest_turn, which makes startup unnecessarily O(threads × retention). Update the lookup so list() and mark_all_interrupted() can identify the newest turn without parsing every file, for example by using a per-thread latest pointer or metadata-based ordering, while keeping list_thread() as the full sorted read for one thread.
🤖 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/features/conversations/timeline/items/StreamingTailItem.tsx`:
- Line 24: The StreamingTailItem component is rendering a hardcoded English
label instead of using i18n. Update the summary text in StreamingTailItem to
come from useT() in app/src/lib/i18n/I18nContext, add a new translation key in
the English locale, and mirror that key across all locale files so the
"Thinking…" label is translated consistently.
In `@docs/plans/conversations-timeline-refactor.md`:
- Around line 56-75: The markdown fence for the directory-tree block is missing
a language label, causing lint issues; update the fenced block in the
conversations timeline refactor plan to use a text/plaintext tag so it remains a
valid documentation tree snippet. Make the change directly in the directory-tree
section near the `app/src/features/conversations/` and
`app/src/pages/Conversations.tsx` listing, keeping the content unchanged aside
from the fence label.
In `@src/openhuman/threads/turn_state/store.rs`:
- Around line 100-118: get_turn currently treats a bad turn snapshot as a hard
error, unlike get/list/list_thread which skip unreadable files. Update
Store::get_turn to mirror read_thread_turns by catching read_snapshot failures,
logging a warning, and returning Ok(None) for corrupted/legacy snapshots so
ops::turn_state_get_turn does not bubble an RPC error. Add or update a
store_tests.rs case that writes an invalid turn file and verifies get_turn
returns None instead of Err.
---
Outside diff comments:
In `@app/src/store/chatRuntimeSlice.ts`:
- Around line 1408-1426: clearRuntimeForThread is missing cleanup for
turnTimelinesByThread, leaving per-thread timeline state behind after thread
delete/reset. Update the clearRuntimeForThread reducer in chatRuntimeSlice to
delete the matching turnTimelinesByThread[threadId] entry alongside the other
per-thread maps already being cleared, following the same pattern used for
toolTimelineByThread, processingByThread, and taskBoardByThread; keep any
intentional exclusions like artifactsByThread unchanged.
---
Nitpick comments:
In `@app/src/features/conversations/timeline/ConversationTimeline.test.tsx`:
- Around line 94-108: The test in ConversationTimeline.test.tsx is using a
fragile CSS-class query to locate the streaming content bubble, which ties it to
StreamingTailItem’s Tailwind styling instead of behavior. Update the component
to expose a stable test hook such as a data-testid on the content wrapper, then
change the assertion in the ConversationTimeline test to target that identifier
rather than .font-mono.text-sm.
- Around line 1-13: The test helper renderInStore is using the real app store
instead of an isolated test store, which makes ConversationTimeline.test.tsx
depend on production middleware and default state. Replace the imported store
with a test-scoped store created from testRootReducer (or an equivalent minimal
test setup) inside renderInStore, and keep ConversationTimeline and
buildThreadTimeline tests wired to controlled fixture state so the test remains
deterministic.
In `@app/src/features/conversations/timeline/ConversationTimeline.tsx`:
- Around line 41-63: The coalescing logic in ConversationTimeline and the
ToolTimelineBlock rendering are already correct, so no behavioral change is
needed; the only cleanup is to remove the redundant type cast inside the while
loop where isProcessItem already narrows items[j]. Keep the existing run
collection, keying by startId, and upstream ordering assumptions intact.
In `@app/src/features/conversations/timeline/items/AssistantMessageItem.tsx`:
- Around line 34-38: The map render in AssistantMessageItem is wrapping a single
AgentMessageBubble in a Fragment just to carry the key, which is unnecessary.
Remove the Fragment and put the key directly on AgentMessageBubble inside the
segment rendering logic so the structure is simpler while preserving list
identity.
In `@app/src/providers/ChatRuntimeProvider.tsx`:
- Around line 1046-1053: Add verbose debug logging around the new requestId
stamping in ChatRuntimeProvider so missing or empty request_id values can be
diagnosed; update the onSegment, onInterim, and onProactiveMessage flows that
build extraMetadata to log a stable debug prefix plus correlation fields (such
as turn/message identifiers and whether requestId was stamped) without secrets
or full PII. Use the existing requestId/request_id handling in
ChatRuntimeProvider to log both the success path when extraMetadata includes
requestId and the fallback path when it is absent, so failures to group a turn
with its process trail are traceable.
In `@src/openhuman/threads/turn_state/store.rs`:
- Around line 261-434: The bulk migration path in migrate_all_legacy_locked is
swallowing write errors when write_turn_file fails, unlike migrate_thread_locked
which logs a warn! for the same failure. Update migrate_all_legacy_locked to
handle the Err case from write_turn_file explicitly and emit a warning with the
legacy file path and error details, keeping behavior and diagnostics consistent
with migrate_thread_locked.
- Around line 143-173: The cold-boot path in TurnStateStore::list is fully
deserializing every retained turn via read_thread_turns just to compute
latest_turn, which makes startup unnecessarily O(threads × retention). Update
the lookup so list() and mark_all_interrupted() can identify the newest turn
without parsing every file, for example by using a per-thread latest pointer or
metadata-based ordering, while keeping list_thread() as the full sorted read for
one thread.
🪄 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: 4b1866ef-a53c-44d5-8433-eeae25f2dacf
📒 Files selected for processing (79)
app/src/components/channels/mcp/ConfigAssistantPanel.tsxapp/src/components/chat/CycleUsagePill.tsxapp/src/components/flows/FlowRunInspectorDrawer.tsxapp/src/components/flows/WorkflowCopilotPanel.tsxapp/src/components/intelligence/IntelligenceTasksTab.tsxapp/src/components/intelligence/ModelCouncilTab.tsxapp/src/components/intelligence/__tests__/IntelligenceTasksTab.test.tsxapp/src/components/layout/shell/useNewChat.tsapp/src/components/notifications/NotificationBody.tsxapp/src/features/conversations/Conversations.tsxapp/src/features/conversations/components/AgentMessageBubble.test.tsxapp/src/features/conversations/components/AgentMessageBubble.tsxapp/src/features/conversations/components/AgentProcessSourcePanel.tsxapp/src/features/conversations/components/AgentTimelineRail.tsxapp/src/features/conversations/components/BackgroundActivityRows.tsxapp/src/features/conversations/components/BackgroundProcessesPanel.tsxapp/src/features/conversations/components/CitationChips.tsxapp/src/features/conversations/components/LimitPill.tsxapp/src/features/conversations/components/PlanReviewCard.test.tsxapp/src/features/conversations/components/PlanReviewCard.tsxapp/src/features/conversations/components/ProcessingTranscriptView.test.tsxapp/src/features/conversations/components/ProcessingTranscriptView.tsxapp/src/features/conversations/components/SubagentDrawer.tsxapp/src/features/conversations/components/TaskKanbanBoard.test.tsxapp/src/features/conversations/components/TaskKanbanBoard.tsxapp/src/features/conversations/components/ThreadGoalChip.test.tsxapp/src/features/conversations/components/ThreadGoalChip.tsxapp/src/features/conversations/components/ThreadTodoStrip.test.tsxapp/src/features/conversations/components/ThreadTodoStrip.tsxapp/src/features/conversations/components/ToolFailureLines.tsxapp/src/features/conversations/components/ToolTimelineBlock.tsxapp/src/features/conversations/components/WorkerThreadRefCard.tsxapp/src/features/conversations/components/__tests__/AgentProcessSourcePanel.test.tsxapp/src/features/conversations/components/__tests__/AgentTimelineRail.test.tsxapp/src/features/conversations/components/__tests__/BackgroundActivityRows.test.tsxapp/src/features/conversations/components/__tests__/BackgroundProcessesPanel.test.tsxapp/src/features/conversations/components/__tests__/SubagentDrawer.test.tsxapp/src/features/conversations/components/__tests__/TaskKanbanBoard.test.tsxapp/src/features/conversations/components/__tests__/ToolTimelineBlock.test.tsxapp/src/features/conversations/components/__tests__/WorkerThreadRefCard.test.tsxapp/src/features/conversations/composerSendDecision.test.tsapp/src/features/conversations/composerSendDecision.tsapp/src/features/conversations/hooks/useBackgroundActivity.test.tsapp/src/features/conversations/hooks/useBackgroundActivity.tsapp/src/features/conversations/index.tsapp/src/features/conversations/taskPlanActions.test.tsapp/src/features/conversations/taskPlanActions.tsapp/src/features/conversations/timeline/ConversationTimeline.test.tsxapp/src/features/conversations/timeline/ConversationTimeline.tsxapp/src/features/conversations/timeline/items/AssistantMessageItem.tsxapp/src/features/conversations/timeline/items/StreamingTailItem.tsxapp/src/features/conversations/timeline/items/UserMessageItem.tsxapp/src/features/conversations/timeline/selectors.test.tsapp/src/features/conversations/timeline/selectors.tsapp/src/features/conversations/timeline/types.tsapp/src/features/conversations/utils/format.tsapp/src/features/conversations/utils/threadFilter.test.tsapp/src/features/conversations/utils/threadFilter.tsapp/src/features/conversations/utils/workerThreadRef.test.tsapp/src/features/conversations/utils/workerThreadRef.tsapp/src/pages/Accounts.tsxapp/src/pages/Conversations.tsxapp/src/pages/__tests__/Accounts.webviewSelection.test.tsxapp/src/pages/__tests__/Conversations.render.test.tsxapp/src/pages/dev/AgentInsightsPreview.tsxapp/src/providers/ChatRuntimeProvider.tsxapp/src/providers/__tests__/ChatRuntimeProvider.test.tsxapp/src/services/api/threadApi.tsapp/src/store/__tests__/chatRuntimeSlice.thunk.test.tsapp/src/store/chatRuntimeSlice.tsdocs/plans/conversations-timeline-refactor.mdsrc/openhuman/threads/ops.rssrc/openhuman/threads/schemas.rssrc/openhuman/threads/schemas_tests.rssrc/openhuman/threads/turn_state/mod.rssrc/openhuman/threads/turn_state/store.rssrc/openhuman/threads/turn_state/store_tests.rssrc/openhuman/threads/turn_state/types.rstests/json_rpc_e2e.rs
| <div className="max-w-[80%] space-y-1"> | ||
| {thinking && thinking.length > 0 ? ( | ||
| <details className="text-xs text-content-muted"> | ||
| <summary className="cursor-pointer select-none">Thinking…</summary> |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Hardcoded English string bypasses i18n.
"Thinking…" is rendered directly instead of going through useT(). As per coding guidelines, "All UI text must go through useT() from app/src/lib/i18n/I18nContext, and new keys must be added to en.ts plus all locale files (ar, bn, de, es, fr, hi, id, it, ko, pl, pt, ru, zh-CN)."
🌐 Proposed fix
+import { useT } from '../../../../lib/i18n/I18nContext';
+
export function StreamingTailItem({
text,
thinking,
branch = false,
}: {
text: string;
thinking?: string;
branch?: boolean;
}) {
+ const { t } = useT();
const tail = text.slice(-STREAMING_PREVIEW_CHARS);
@@
- <summary className="cursor-pointer select-none">Thinking…</summary>
+ <summary className="cursor-pointer select-none">{t('conversations.streamingTail.thinking')}</summary>Add the new key to en.ts and all other locale files.
📝 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.
Suggested change
| <summary className="cursor-pointer select-none">Thinking…</summary> | |
| import { useT } from '../../../../lib/i18n/I18nContext'; | |
| export function StreamingTailItem({ | |
| text, | |
| thinking, | |
| branch = false, | |
| }: { | |
| text: string; | |
| thinking?: string; | |
| branch?: boolean; | |
| }) { | |
| const { t } = useT(); | |
| const tail = text.slice(-STREAMING_PREVIEW_CHARS); | |
| ... | |
| <summary className="cursor-pointer select-none">{t('conversations.streamingTail.thinking')}</summary> | |
| ... | |
| } |
🤖 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/features/conversations/timeline/items/StreamingTailItem.tsx` at line
24, The StreamingTailItem component is rendering a hardcoded English label
instead of using i18n. Update the summary text in StreamingTailItem to come from
useT() in app/src/lib/i18n/I18nContext, add a new translation key in the English
locale, and mirror that key across all locale files so the "Thinking…" label is
translated consistently.
Source: Path instructions
Comment on lines
+56
to
+75
| ``` | ||
| app/src/features/conversations/ | ||
| ConversationsPage.tsx # page-shell variant | ||
| ConversationsSidebar.tsx # sidebar variant (Accounts, HumanPage) | ||
| index.ts # public exports = reusability contract | ||
| timeline/ | ||
| types.ts # TimelineItem union | ||
| selectors.ts # projection + turn grouping (reselect-memoized) | ||
| ConversationTimeline.tsx # pure renderer: TimelineItem[] → one component per kind | ||
| items/ # UserMessageItem, AssistantMessageItem, ToolCallItem, | ||
| # ReasoningItem, SubagentActivityItem, StreamingTailItem | ||
| TurnInsightsGroup.tsx # collapsed-header/expanded-body per-turn wrapper (lazy-load) | ||
| composer/ | ||
| Composer.tsx / MicCloudComposer.tsx / useComposerState.ts / composerSendDecision.ts | ||
| threadList/ | ||
| ThreadList.tsx / threadFilter.ts | ||
| components/ # existing subcomponents relocated as-is | ||
| (ToolTimelineBlock, SubagentDrawer, TaskKanbanBoard, AgentProcessSourcePanel, …) | ||
| app/src/pages/Conversations.tsx # thin re-export shim during migration; deleted in Phase 6 | ||
| ``` |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Label the directory-tree fence.
markdownlint flags this block because it has no language tag. Adding text/plaintext keeps the doc lint-clean and renders more consistently.
Suggested fix
-```
+```text
app/src/features/conversations/
ConversationsPage.tsx # page-shell variant
ConversationsSidebar.tsx # sidebar variant (Accounts, HumanPage)
@@
app/src/pages/Conversations.tsx # thin re-export shim during migration; deleted in Phase 6
</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 56-56: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@docs/plans/conversations-timeline-refactor.md` around lines 56 - 75, The
markdown fence for the directory-tree block is missing a language label, causing
lint issues; update the fenced block in the conversations timeline refactor plan
to use a text/plaintext tag so it remains a valid documentation tree snippet.
Make the change directly in the directory-tree section near the
`app/src/features/conversations/` and `app/src/pages/Conversations.tsx` listing,
keeping the content unchanged aside from the fence label.
Source: Linters/SAST tools
Comment on lines
+100
to
118
| /// Return the latest turn for `thread_id`, or `None` if none exists. | ||
| /// "Latest" is the turn with the greatest `started_at` (ties broken by | ||
| /// `updated_at`) — the in-flight or most-recent turn. | ||
| pub fn get(&self, thread_id: &str) -> Result<Option<TurnState>, String> { | ||
| let _guard = TURN_STATE_LOCK.lock(); | ||
| let path = self.snapshot_path(thread_id); | ||
| self.migrate_thread_locked(thread_id); | ||
| Ok(latest_turn(self.read_thread_turns(thread_id)?)) | ||
| } | ||
|
|
||
| /// Return a specific turn by `request_id`, or `None` if absent. | ||
| pub fn get_turn(&self, thread_id: &str, request_id: &str) -> Result<Option<TurnState>, String> { | ||
| let _guard = TURN_STATE_LOCK.lock(); | ||
| self.migrate_thread_locked(thread_id); | ||
| let path = self.turn_path(thread_id, request_id); | ||
| if !path.exists() { | ||
| return Ok(None); | ||
| } | ||
| read_snapshot(&path).map(Some) | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
get_turn fails hard on an unreadable snapshot instead of degrading gracefully.
Every other read path (get, list, list_thread, via read_thread_turns) logs a warning and skips an unparsable turn file. get_turn instead calls read_snapshot(&path) directly and propagates any parse error as Err(String), which ops::turn_state_get_turn bubbles up via ? into a JSON-RPC error — turning a single corrupted/legacy turn file into a hard RPC failure for the new "lazily load a past turn" UI flow, instead of the None/null the UI already handles.
🐛 Proposed fix to align with sibling read paths
pub fn get_turn(&self, thread_id: &str, request_id: &str) -> Result<Option<TurnState>, String> {
let _guard = TURN_STATE_LOCK.lock();
self.migrate_thread_locked(thread_id);
let path = self.turn_path(thread_id, request_id);
if !path.exists() {
return Ok(None);
}
- read_snapshot(&path).map(Some)
+ match read_snapshot(&path) {
+ Ok(snapshot) => Ok(Some(snapshot)),
+ Err(err) => {
+ warn!(
+ "{LOG_PREFIX} skip unreadable snapshot {}: {err}",
+ path.display()
+ );
+ Ok(None)
+ }
+ }Consider adding a store_tests.rs case that hand-writes a corrupted turn file and asserts get_turn returns Ok(None) rather than Err.
📝 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.
Suggested change
| /// Return the latest turn for `thread_id`, or `None` if none exists. | |
| /// "Latest" is the turn with the greatest `started_at` (ties broken by | |
| /// `updated_at`) — the in-flight or most-recent turn. | |
| pub fn get(&self, thread_id: &str) -> Result<Option<TurnState>, String> { | |
| let _guard = TURN_STATE_LOCK.lock(); | |
| let path = self.snapshot_path(thread_id); | |
| self.migrate_thread_locked(thread_id); | |
| Ok(latest_turn(self.read_thread_turns(thread_id)?)) | |
| } | |
| /// Return a specific turn by `request_id`, or `None` if absent. | |
| pub fn get_turn(&self, thread_id: &str, request_id: &str) -> Result<Option<TurnState>, String> { | |
| let _guard = TURN_STATE_LOCK.lock(); | |
| self.migrate_thread_locked(thread_id); | |
| let path = self.turn_path(thread_id, request_id); | |
| if !path.exists() { | |
| return Ok(None); | |
| } | |
| read_snapshot(&path).map(Some) | |
| } | |
| /// Return the latest turn for `thread_id`, or `None` if none exists. | |
| /// "Latest" is the turn with the greatest `started_at` (ties broken by | |
| /// `updated_at`) — the in-flight or most-recent turn. | |
| pub fn get(&self, thread_id: &str) -> Result<Option<TurnState>, String> { | |
| let _guard = TURN_STATE_LOCK.lock(); | |
| self.migrate_thread_locked(thread_id); | |
| Ok(latest_turn(self.read_thread_turns(thread_id)?)) | |
| } | |
| /// Return a specific turn by `request_id`, or `None` if absent. | |
| pub fn get_turn(&self, thread_id: &str, request_id: &str) -> Result<Option<TurnState>, String> { | |
| let _guard = TURN_STATE_LOCK.lock(); | |
| self.migrate_thread_locked(thread_id); | |
| let path = self.turn_path(thread_id, request_id); | |
| if !path.exists() { | |
| return Ok(None); | |
| } | |
| match read_snapshot(&path) { | |
| Ok(snapshot) => Ok(Some(snapshot)), | |
| Err(err) => { | |
| warn!( | |
| "{LOG_PREFIX} skip unreadable snapshot {}: {err}", | |
| path.display() | |
| ); | |
| Ok(None) | |
| } | |
| } | |
| } |
🤖 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 `@src/openhuman/threads/turn_state/store.rs` around lines 100 - 118, get_turn
currently treats a bad turn snapshot as a hard error, unlike
get/list/list_thread which skip unreadable files. Update Store::get_turn to
mirror read_thread_turns by catching read_snapshot failures, logging a warning,
and returning Ok(None) for corrupted/legacy snapshots so
ops::turn_state_get_turn does not bubble an RPC error. Add or update a
store_tests.rs case that writes an invalid turn file and verifies get_turn
returns None instead of Err.
…, partial) The panel has lived at `features/conversations/Conversations.tsx` since Phase 1b; remove the temporary `pages/Conversations.tsx` re-export shim and point every consumer at the real module: `HumanPage`, `Accounts` (+ its `ConversationsPage` alias), and the co-located Conversations/Accounts test suites and mocks. Scope note: Phase 6's dead-state removal (`parallelStreamsByThread`, `preserveLiveSubagentProse`, provider refs) is coupled to Phase 3 and is tracked in the follow-up issue, not this PR. Typecheck + lint clean; 119 tests across the Conversations render suite, Accounts, and HumanPage pass with the repointed imports. Refs: docs/plans/conversations-timeline-refactor.md Phase 6 Claude-Session: https://claude.ai/code/session_01MVwyyQVNm8UZpvJyWrf2yo
…hells) Move the left-rail thread list (search, new-conversation row, thread rows with inline rename + delete) out of the 3.3k-line panel into a self-contained presentational `features/conversations/threadList/ThreadList.tsx`. Driven by a clean callback interface — `onSelectThread` / `onRequestDelete` / `onCommitTitle` / `onBlurTitle` etc. encapsulate the multi-step dispatch+route logic in the parent, so the component takes ~14 typed props instead of drilling dispatch/navigate/refs. JSX moved verbatim; no behavior change. The panel shrinks by ~165 lines. Guardrails: full Conversations render suite + sidebarOverflow (thread search, row select, inline rename, delete modal via existing data-testids), Accounts, and HumanPage all pass. Typecheck + lint clean. Refs: docs/plans/conversations-timeline-refactor.md Phase 1 (shell extraction) Claude-Session: https://claude.ai/code/session_01MVwyyQVNm8UZpvJyWrf2yo
eslint import/order auto-fix for the HumanPage/Accounts imports repointed to features/conversations in the shim-deletion commit. No behavior change. Claude-Session: https://claude.ai/code/session_01MVwyyQVNm8UZpvJyWrf2yo
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ec5dee94c
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // hydrates into `toolTimelineByThread` (rendered as the live/anchored | ||
| // "agent insights"), so skip it here to avoid rendering it twice — this | ||
| // field holds only the *older* settled turns. | ||
| for (const turn of history.slice(1)) { |
There was a problem hiding this comment.
When the user keeps a thread open, the newest completed turn is skipped here and exists only in toolTimelineByThread; the next send immediately clears that live timeline with setToolTimelineForThread([]) before this history thunk runs again. That means the just-finished answer loses its process trail as soon as the user starts another turn, and it only comes back after switching/reopening the thread. Stash the skipped completed turn when a newer request starts/completes, or refresh history at turn end, instead of permanently leaving it out of turnTimelinesByThread.
Useful? React with 👍 / 👎.
This was referenced Jul 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
turn_states/<hex(thread)>/<hex(request)>.json, retention N=20), with idempotent legacy migration and unchanged public signatures (zero caller churn).threads.turn_state_history/turn_state_get_turnexpose the per-turn history without disturbing the cold-bootturn_state_list.app/src/features/conversations/; a memoizedTimelineItemprojection (selectTimelineForThread) is now the single source of render order, and assistant messages are stamped withextraMetadata.requestIdso turns group deterministically.Problem
app/src/pages/Conversations.tsx(3,302 lines) held one tool-timeline array per thread and the Rust core persisted one turn snapshot per thread (whole-file overwrite). So a multi-turn thread lost every turn's process trail but the latest, and each send wiped the live timeline. Seedocs/plans/conversations-timeline-refactor.mdanddocs/plans/per-turn-tool-timeline-history.md(both landed on this branch).Solution
Frontend-first, then the backend slots in underneath:
features/conversations/behind a thinpages/Conversations.tsxshim; rename theAgentChatPanelpage-alias toConversationsPage(resolves the collision with the settings panel).TimelineItemdiscriminated union + memoized projection that reproduces today's anchoring exactly (single tool timeline after the last user message; proactive/no-user-message fallback;hideAgentInsightsfiltering). The live render loop is then sourced from the projection DOM-identically.extraMetadata.requestIdon every assistant append (chat_done incl. parallel/segment-reconcile paths, chat_segment, chat_interim, proactive); rebuild the store as a per-turn ring with retention + legacy migration; add the history RPCs +threadApiclients.turn_state_history) on thread open into an additiveturnTimelinesByThreadslice field (which survives a new send), and render each older turn's collapsedToolTimelineBlockabove the answer it produced. The newest turn is excluded (it still renders as the live anchor), so nothing double-renders.Deliberately deferred (own follow-up PR): Phase 3 (ChatRuntimeProvider → reducer consolidation + dedup unification +
preserveLiveSubagentProseremoval) — a large refactor of the live streaming path whose dedup changes the plan flags as risk-sensitive and which needs manual live-streaming QA; Phase 6 cleanup (shim deletion / dead-state removal, mostly coupled to Phase 3); and the Phase 1 shell extraction.Submission Checklist
N/A: no feature IDs added/removed/renamed indocs/TEST-COVERAGE-MATRIX.md; behaviour is additive within the existing conversations/threads features.## Related—N/A(see above).N/A: no new release-cut surface; recommend a manual eyeball of a multi-turn thread (per the plan's Phase 5 QA note) before merge.N/A: no tracking issue.Impact
<hex(thread)>.jsonsnapshots are read once and migrated to the per-turn layout on first access (idempotent); the read path is kept one release.get/list/delete/clear_all/mark_all_interruptedsemantics are preserved, so all existing callers and the cold-boot path are unaffected.get/listscan a small per-thread directory (≤ retention window); completed turns prune to N=20 per thread so history stays bounded. Selector is reselect-memoized per thread.Related
AI Authored PR Metadata
Linear Issue
Commit & Branch
feat/conversations-per-turn-historyValidation Run
pnpm --filter openhuman-app format:checkpnpm typecheckturn_state::store(14),turn_state::mirror(12),threads::ops(34),threads::schemas(17),json_rpc_thread_turn_state_lifecycleE2E; chat Playwright web specs (send-stream, scroll-render, multi-turn history).cargo fmt --checkclean;pnpm rust:check(Tauri crate) passed via pre-push hook.app/src-taurichanges.Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
requestIdfall back to the single-anchor timeline; the render-loop swap is DOM-identical (full Conversations render suite green); storeget/list/delete/clear_all/mark_all_interruptedsignatures + semantics unchanged.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Bug Fixes