Repository navigation
fix(chat): heartbeat keeps long reasoning turns alive past 2-min timeout (#4270) - #4282
Conversation
… turn (tinyhumansai#4270) The frontend silence timer fires after ~120s with no progress signal of any kind. A long prefill on a large context, or a reasoning-tier model that buffers reasoning_content server-side, can legitimately stream nothing for minutes, tripping a false "no response after 2 minutes" timeout that discards the live turn. Add a 20s wall-clock liveness beat ("inference_heartbeat") emitted by the progress bridge for the whole duration of an in-flight turn (gated on TurnStarted, stopped on TurnCompleted). It rides the same socket as real progress events, so it keeps the turn alive while work progresses yet stops the instant the socket/core dies — preserving the genuine-disconnect error path. Tokio test-util dev-dependency added so the paused-clock heartbeat tests assert the beat without real-time waits.
…4270) Subscribe to the new inference_heartbeat socket event, bump a per-thread liveness counter in chatRuntime, and add that counter to the Conversations silence-timer rearm signature. A long buffered-reasoning phase that emits no status/stream/tool/board signal now keeps the 120s timer armed via the beat, while a real disconnect (beats stop) still fails fast.
…inyhumansai#4270) - chatService: inference_heartbeat forwards through onInferenceHeartbeat - chatRuntimeSlice: heartbeat counter bumps per thread, cleared on runtime reset - Conversations: beats during a silent reasoning phase keep the turn active past the 120s window (repro gone); when beats stop the silence timer still fires (genuine-disconnect regression safety)
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds periodic ChangesInference Heartbeat End-to-End
Sequence Diagram(s)sequenceDiagram
participant progress_bridge
participant socket
participant provider as ChatRuntimeProvider
participant store as Redux Store
participant conv as Conversations.tsx
progress_bridge->>socket: inference_heartbeat
socket->>provider: onInferenceHeartbeat(event)
provider->>store: dispatch bumpInferenceHeartbeatForThread(thread_id)
store-->>conv: inferenceHeartbeatByThread updated
conv->>conv: rearm 120s silence timer
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
Comment |
…nsai#4270) Lifecycle test: turn beats while in flight, then stops once TurnCompleted lands and the channel closes — exercises the turn_active=false arm and the loop break so diff coverage holds on the changed bridge lines.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/providers/ChatRuntimeProvider.tsx`:
- Around line 478-484: The heartbeat handling in ChatRuntimeProvider is
collapsing liveness to thread scope, which lets parallel requests on the same
thread incorrectly keep the primary turn alive. Update the onInferenceHeartbeat
path to preserve request scoping using ChatInferenceHeartbeatEvent.request_id,
and make bumpInferenceHeartbeatForThread or the surrounding runtime state filter
by the owning request before dispatching the keepalive. Ensure the primary
silence timer attached by handleSendParallel only gets bumped by its own
request’s heartbeat, not sibling branches.
In `@app/src/store/chatRuntimeSlice.ts`:
- Around line 414-422: Add `inferenceHeartbeatByThread` to the state reset
performed by `clearAllChatRuntime()` in `chatRuntimeSlice`, so the slice is
fully cleared and no stale per-thread heartbeat counters survive. Use the
existing `clearAllChatRuntime` reducer and the new `inferenceHeartbeatByThread`
bucket to locate the change, and update the slice test to assert this map is
reset along with the other chat runtime fields.
🪄 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: 5e358638-415b-4062-bb6e-1a8d32560b02
📒 Files selected for processing (9)
Cargo.tomlapp/src/pages/Conversations.tsxapp/src/pages/__tests__/Conversations.render.test.tsxapp/src/providers/ChatRuntimeProvider.tsxapp/src/services/__tests__/chatService.test.tsapp/src/services/chatService.tsapp/src/store/__tests__/chatRuntimeSlice.test.tsapp/src/store/chatRuntimeSlice.tssrc/openhuman/channels/providers/web/progress_bridge.rs
…inyhumansai#4270) CodeRabbit: a parallel (forked) turn shares the thread but streams into its own lane; its heartbeat must not keep the thread's primary silence timer alive, or a sibling branch would mask a stalled primary turn. Mirror the text/thinking-delta routing — skip the bump when the heartbeat's request_id is a registered parallel request.
…inyhumansai#4270) CodeRabbit: the slice-wide reset left the new heartbeat bucket behind, so stale per-thread counters survived a full chat-runtime clear. Wipe it alongside the other runtime maps + assert in the slice test.
Conversations.render.test.tsx: both sides added named imports from chatRuntimeSlice — this PR's bumpInferenceHeartbeatForThread and main's clearFollowupsForThread/enqueueFollowup. Kept all three (alphabetical); each is used by its own test block, both of which are present in the merged file.
M3gA-Mind
left a comment
There was a problem hiding this comment.
Reviewed and verified: correct, well-tested, CI green. Approving. Thanks for the contribution!
Summary
inference_heartbeat(every 20s) for the whole duration of an in-flight turn.chat.safetyTimeouterror for true connectivity failures.Problem
The 2-minute timeout is a purely frontend silence timer in
app/src/pages/Conversations.tsx(armSilenceTimer, 120s). It only rearms when one of four redux slices changes (inferenceStatusByThread,streamingAssistantByThread,toolTimelineByThread,taskBoardByThread).inference_startarms it once at turn start; after that, two legitimate cases produce zero progress events for >120s while the turn is genuinely working:reasoning_contentserver-side and streams no intermediate delta.In both, the silence timer fires, surfaces
chat.safetyTimeout, clears the runtime, and the user loses the turn with no output. No core-side liveness signal existed (theheartbeatmodule is the unrelated subconscious loop).Solution
channels/providers/web/progress_bridge.rs): convert the bridge'swhile letevent loop to aloop { tokio::select! { … } }with abiasedinterval branch. EveryINFERENCE_HEARTBEAT_SECS(20s) it publishes aWebChannelEvent { event: "inference_heartbeat", … }, gated onturn_active(set onTurnStarted, cleared onTurnCompleted). The beat rides the same socket as real progress, so it stops the instant the socket/core dies.inference_heartbeat(chatService.ts), bump a per-threadinferenceHeartbeatByThreadcounter (chatRuntimeSlice.ts) fromChatRuntimeProvider.tsx, and add that counter to the silence-timer rearm signature inConversations.tsx(kept last so the done-transition probe oncurrent[0]is unaffected).Submission Checklist
## Relatedtokiotest-utilis a test-only dev-dependency)Closes #NNNin the## RelatedsectionImpact
chat.safetyTimeoutwithin ~120s of the last beat.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/4270-inference-heartbeat-timeout5e45cdb6fValidation Run
pnpm --filter openhuman-app format:checkpnpm typecheckvitest runConversations.render / chatService / chatRuntimeSlice (110 passed);cargo test --lib …web::progress_bridge(6 passed)cargo fmt --check,cargo check,cargo clippy --lib(clean for changed file)Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
chat.safetyTimeout.clearRuntimeForThread.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Bug Fixes
Tests