feat(daemon,dashboard): snapshot.request endpoint for late-joining clients (#432)#448
Merged
Conversation
…ients (#432) A browser tab that connects mid-execution (first paint after the user clicks a session in the sidebar, refresh, tab switch) used to see only events emitted AFTER it connected. Root nodes, in-flight tools, plans, and assistant text were all missing — the dashboard was silently broken for the most common interaction. Daemon side: - New SessionEventBuffer: per-session ring buffer of broadcast envelopes (capped 5000 per session, drop-oldest with single WARN log on first overflow). Threadsafe via a per-session synchronized ArrayDeque under an outer ConcurrentHashMap. - WebSocketBridge.broadcast appends every envelope to the buffer unconditionally — even with zero connected clients — so a tab that opens AFTER an event was emitted can still replay it. Builds the envelope before the clients-empty short-circuit. - New WS inbound method snapshot.request {sessionId} → snapshot.response {sessionId, lastEventId, events:[envelope...]}. Reply is point-to-point (ctx.send) like sessions.list, not envelope-wrapped — semantically a request-response, not a stream event. - Buffer cleared in setSessionEndCallback after the session_ended broadcast lands (so the snapshot still includes session_ended for the brief race window) and before SessionManager teardown. Dashboard side: - useExecutionTree's onopen now sends snapshot.request for the current sessionId. Reconnects after a drop go through the same path, so a daemon restart or laptop wake doesn't strand the tree state. - onmessage detects snapshot.response (non-enveloped, sessionId-matched) and replays each envelope through the same parseEnvelope gate as live frames. The reducer's eventId<=lastEventId dedup (already in #435) handles overlap with the live stream — works regardless of arrival order between snapshot.response and live broadcasts on the same socket. - parseEnvelope refactored: shared validation extracted to parseEnvelopeFromObject so the snapshot path doesn't pay for a JSON round-trip per event. Bugs this fixes (all reported by user testing #445): - "Click into session, no root node visible" → snapshot includes session_started. - "Switch sessions in sidebar, dashboard goes blank" → new sessionId triggers fresh snapshot.request, tree fills immediately. - "Refresh loses everything" → reconnect re-fetches snapshot. Tests: - SessionEventBufferTest: append/snapshot order, cap-and-evict, clear isolation, concurrent appends + reader. - WebSocketBridgeTest: broadcast populates buffer with zero clients, clearSession isolation, snapshot.request E2E with cross-session filtering, empty-snapshot reply for unknown session. - snapshotReplay.test.ts: replay tree-shape, dedup live=lastEventId, apply live>lastEventId, malformed-envelope rejection, forward-compat unknown method. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThis PR implements a session event buffer and snapshot API for late-joining WebSocket clients. The daemon now buffers broadcast events per session in a bounded deque, handles Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Late-Joining Browser
participant WS as WebSocket Handler
participant Buffer as SessionEventBuffer
participant Reducer as Execution Tree Reducer
Client->>WS: Open connection (onopen)
Client->>WS: snapshot.request {sessionId}
WS->>Buffer: snapshot(sessionId)
Buffer-->>WS: List[envelopes]
WS->>WS: Compute lastEventId from newest
WS-->>Client: snapshot.response {events[], lastEventId}
Client->>Reducer: Load snapshot → build initial tree
Client->>Client: Store lastEventId as watermark
Note over WS,Buffer: Concurrent: daemon continues broadcasting
WS->>Buffer: append(sessionId, envelope)
WS-->>Client: snapshot.response events replay
Client->>Reducer: parseEnvelopeFromObject(event)
Reducer->>Reducer: Dispatch if eventId > watermark
WS-->>Client: Live stream.tool_use {eventId: 42}
Client->>Reducer: Parse & check: 42 > watermark?
Reducer->>Reducer: Apply event, update watermark
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (6 passed)
✨ 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. Review rate limit: 0/1 reviews remaining, refill in 33 minutes and 10 seconds.Comment |
Self-review caught: plain FIFO eviction in SessionEventBuffer would drop the oldest envelope first when the cap is hit. The oldest envelope is always stream.session_started (by daemon construction), so a long session would lose its root node from the snapshot — the dashboard reducer would then rebuild an empty tree on snapshot replay, regressing the exact bug #432 was meant to fix. Fix: pinned-aware eviction. PINNED_METHODS holds the structural events the reducer needs to build the tree skeleton (session, turn, plan, tool, subagent boundaries plus terminal states). On overflow we walk the deque oldest→newest and remove the first non-pinned envelope; high-volume noisy events (text, thinking, heartbeat, usage, compaction) get evicted preferentially while structure survives arbitrary session length. Trade-off documented: long sessions lose some text history from earlier turns, but the tree shape and tool calls remain intact. The "click into a session, see the tree" UX is what the dashboard contract depends on; partial text is recoverable from session history later, lost root is not. All-pinned fallback: degrades to plain FIFO. Practically impossible at the 5000 default cap (real sessions generate far more text than structural events) but keeps the append path deterministic. Tests: - evictionPreservesStructuralEventsLikeSessionStarted: cap=3, with session_started + 3 text deltas, the 4th append evicts the oldest TEXT (eventId=2), not session_started (eventId=1). - evictionFallsBackToFifoWhenEveryEnvelopeIsPinned: pathological buffer of all tool_use, cap=2, third append evicts oldest tool_use. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
aceclaw-dashboard/src/hooks/useExecutionTree.ts (1)
182-245:⚠️ Potential issue | 🔴 CriticalBuffer same-session live frames until snapshot replay finishes.
A live frame for this session can arrive after
snapshot.requestis sent but beforesnapshot.responseis processed. In that ordering, the reducer advanceslastEventIdfrom the live frame first, and the later snapshot replay gets dropped by the existingeventId <= lastEventIddedup gate. The tab then never reconstructs the missing root/turn/tool history.Possible fix
useEffect(() => { let ws: WebSocket | null = null; let reconnectTimer: ReturnType<typeof setTimeout> | null = null; let backoffMs = RECONNECT_INITIAL_MS; let cancelled = false; + let snapshotPending = true; + const queuedLive: ParseResult[] = []; + + const applyParsed = (result: ParseResult) => { + if (!result || result.sessionId !== sessionId) return; + if (result.kind === 'unknown') { + setTree((prev) => + prev.lastEventId >= result.eventId + ? prev + : { ...prev, lastEventId: result.eventId }, + ); + return; + } + dispatch(result.envelope); + }; const connect = (): void => { if (cancelled) return; setStatus('connecting'); ws = new WebSocket(wsUrl); @@ if ( isPlainObject(data) && data['method'] === 'snapshot.response' && data['sessionId'] === sessionId && Array.isArray(data['events']) ) { for (const env of data['events']) { const parsed = parseEnvelopeFromObject(env); - if (!parsed) continue; - if (parsed.sessionId !== sessionId) continue; - if (parsed.kind === 'unknown') { - setTree((prev) => - prev.lastEventId >= parsed.eventId - ? prev - : { ...prev, lastEventId: parsed.eventId }, - ); - continue; - } - dispatch(parsed.envelope); + applyParsed(parsed); } + snapshotPending = false; + for (const queued of queuedLive) applyParsed(queued); + queuedLive.length = 0; return; } const result = parseEnvelopeFromObject(data); - if (!result) return; - if (result.sessionId !== sessionId) return; - if (result.kind === 'unknown') { - setTree((prev) => - prev.lastEventId >= result.eventId - ? prev - : { ...prev, lastEventId: result.eventId }, - ); + if (!result || result.sessionId !== sessionId) return; + if (snapshotPending) { + queuedLive.push(result); return; } - dispatch(result.envelope); + applyParsed(result); };Please add a regression test for the
live frame arrives before snapshot.responseordering as well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@aceclaw-dashboard/src/hooks/useExecutionTree.ts` around lines 182 - 245, Live envelopes for the same session can arrive after sending snapshot.request but before processing snapshot.response, causing live frames to advance lastEventId and then drop replayed snapshot events; fix by introducing a "snapshotPending" flag (e.g., a ref set when sending snapshot.request) and a small buffer (e.g., pendingLiveEnvelopes ref) referenced from ws.onmessage: if a parsed envelope (from parseEnvelopeFromObject) has result.sessionId === sessionId and snapshotPending is true and the message is a live envelope (not snapshot.response), push it into pendingLiveEnvelopes instead of dispatching or bumping lastEventId; when snapshot.response handling completes (after replaying/parsing its events and updating setTree/dispatch), clear snapshotPending and flush pendingLiveEnvelopes in arrival order applying the same dedup logic (setTree lastEventId checks and dispatch) so live frames are processed only after snapshot replay; add a regression test that simulates receiving a live envelope before snapshot.response to verify the buffer and flush behavior.
🧹 Nitpick comments (1)
aceclaw-daemon/src/main/java/dev/aceclaw/daemon/WebSocketBridge.java (1)
322-323: Null-guardsessionIdat the public boundary.
clearSession()currently relies onSessionEventBuffer.clear()to fail on null. Guarding here keeps this public API consistent with the rest of the class and makes the contract explicit.Suggested change
public void clearSession(String sessionId) { + Objects.requireNonNull(sessionId, "sessionId"); eventBuffer.clear(sessionId); }As per coding guidelines "Method parameters: Use
Objects.requireNonNull(param, "param")on parameters used in.equals()or passed to downstream calls".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@aceclaw-daemon/src/main/java/dev/aceclaw/daemon/WebSocketBridge.java` around lines 322 - 323, Add a null-check on the public parameter sessionId in clearSession by calling Objects.requireNonNull(sessionId, "sessionId") before delegating to eventBuffer.clear(sessionId); this ensures the method enforces the non-null contract consistently and prevents relying on SessionEventBuffer.clear to throw on null.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@aceclaw-dashboard/src/hooks/useExecutionTree.ts`:
- Around line 182-245: Live envelopes for the same session can arrive after
sending snapshot.request but before processing snapshot.response, causing live
frames to advance lastEventId and then drop replayed snapshot events; fix by
introducing a "snapshotPending" flag (e.g., a ref set when sending
snapshot.request) and a small buffer (e.g., pendingLiveEnvelopes ref) referenced
from ws.onmessage: if a parsed envelope (from parseEnvelopeFromObject) has
result.sessionId === sessionId and snapshotPending is true and the message is a
live envelope (not snapshot.response), push it into pendingLiveEnvelopes instead
of dispatching or bumping lastEventId; when snapshot.response handling completes
(after replaying/parsing its events and updating setTree/dispatch), clear
snapshotPending and flush pendingLiveEnvelopes in arrival order applying the
same dedup logic (setTree lastEventId checks and dispatch) so live frames are
processed only after snapshot replay; add a regression test that simulates
receiving a live envelope before snapshot.response to verify the buffer and
flush behavior.
---
Nitpick comments:
In `@aceclaw-daemon/src/main/java/dev/aceclaw/daemon/WebSocketBridge.java`:
- Around line 322-323: Add a null-check on the public parameter sessionId in
clearSession by calling Objects.requireNonNull(sessionId, "sessionId") before
delegating to eventBuffer.clear(sessionId); this ensures the method enforces the
non-null contract consistently and prevents relying on SessionEventBuffer.clear
to throw on null.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0958088f-33fb-435f-b37b-b64aca8bf700
📒 Files selected for processing (7)
aceclaw-daemon/src/main/java/dev/aceclaw/daemon/AceClawDaemon.javaaceclaw-daemon/src/main/java/dev/aceclaw/daemon/SessionEventBuffer.javaaceclaw-daemon/src/main/java/dev/aceclaw/daemon/WebSocketBridge.javaaceclaw-daemon/src/test/java/dev/aceclaw/daemon/SessionEventBufferTest.javaaceclaw-daemon/src/test/java/dev/aceclaw/daemon/WebSocketBridgeTest.javaaceclaw-dashboard/src/hooks/useExecutionTree.tsaceclaw-dashboard/tests/snapshotReplay.test.ts
User testing #432 found a missing case the existing #435 reducer doesn't cover: a turn shows tool nodes but no reasoning step. Daemon emits stream.thinking with {delta} (StreamingAgentHandler#3066) and useExecutionTree validates it (it's in KNOWN_METHODS), but the treeReducer switch had no case for it — it fell through default and got dropped. The user's "看不到 reasoning 这步" was the fact that thinking blocks left no node behind. Mirror the text path: - Add 'thinking' to ExecutionNodeType. - thinkingNodeId(turnId) — one thinking child per turn, like text. - appendThinkingToCurrentTurn — same shape as appendTextToCurrentTurn (find running turn, append-or-create child, concat deltas into one node so the renderer shows a single bubble). - Reducer switch case routes stream.thinking to the new function. Visual contract: thinking node renders identically to text/tool/etc through GrowingNode (no type-based switch in the renderer), with label="thinking" so the user can distinguish it from "response". Layout-wise it sits under the same turn as text and tools — dagre LR places siblings vertically. Tests: - rolls thinking deltas into a single thinking child of the running turn, deltas concatenate. - thinking + text coexist as siblings under the same turn (real shape: a turn typically thinks, then responds). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ations) (#432) User caught the right semantic: thinking is the cause, tool calls are the effects. Previously tools attached to the running turn directly, making thinking and tools look like unrelated siblings — but in a ReAct loop, the model thinks, then chooses tools (potentially in parallel within one LLM response), then receives results, then thinks again. The tree should mirror that: Turn ├── thinking (iteration 1) │ ├── tool A ← parallel from one LLM response │ └── tool B ├── thinking (iteration 2) ← new LLM call │ └── tool C └── response (text) Implementation: reducer state tracks a thinking anchor. - ExecutionTree gains currentThinkingId + thinkingSealed. - A new turn resets both (fresh ReAct loop). - A thinking delta with anchor empty OR sealed → mints a new thinking node; otherwise appends to the current one. So a thinking delta arriving after a tool_use creates a fresh iteration. - A tool_use attaches under currentThinkingId when present, else falls back to the running turn (extended thinking disabled). It seals the anchor so the NEXT thinking delta starts a new node — but parallel tool_uses from one LLM response don't get a thinking delta between them, so they correctly attach to the same parent. - A text delta also seals (response signals end of current iteration). - Multiple thinking nodes per turn get unique ids by counting existing thinking descendants — first uses the original `${turnId}:thinking`, later iterations append a counter. - Parallel detection still flags the turn (sidebar stats consistency) but checks the anchor's children, whether that's the turn or the thinking node. Tests (5 new cases on top of the existing 2): - tool_use under most-recent thinking, not the turn - parallel tools share one thinking parent (one LLM response) - new thinking after tool_use mints a new node (iteration boundary) - no thinking → tools fall back to attaching to the turn - turn boundary resets the anchor 71/71 dashboard tests green; existing thinking concatenation test still passes (no tool_use between deltas, anchor not sealed). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ings (#432) User caught: solid edges should mean containment ("属于"), but the same solid edges currently double as time-order between turns. Splitting the visual semantic: - Solid bezier (containment): parent-child relationship from the reducer's tree shape — session→turn, turn→thinking, thinking→tool, etc. Status-coloured. - Dashed straight line (sequence): temporal order between same-rank siblings whose ordering is meaningful — turns under a session, thinking nodes under a turn (one per ReAct iteration). Muted neutral so it recedes behind containment edges. Tools intentionally don't get sequence edges. Within one thinking parent, parallel tool_uses come from a single LLM response and are parallel by construction (no edge between them = parallel). Outside a thinking parent, we can't tell parallel from sequential without execution-order metadata, and a wrong sequence edge is worse than no edge. Implementation: - LayoutEdge gains a `kind: 'containment' | 'sequence'` discriminator. - useTreeLayout: dagre handles containment as before. After layout, a pre-order tree walk pushes a sequence edge between every pair of consecutive same-type siblings whose type is in SEQUENCE_TYPES (turn, thinking). Edge geometry is a vertical straight line from the bottom of `prev` to the top of `curr` (LR layout siblings stack vertically at the same x). - GrowingEdge: branch on kind. Sequence variant is a straight line, 4-4 dash, 1.25 stroke, 0.5 opacity (vs containment's bezier, solid, 1.75 stroke, 0.85 opacity). - Sequence edges are NOT added to dagre's input graph — that would push siblings into different ranks and break the LR "siblings stack vertically" contract. Tests: - useTreeLayout: dashed sequence edge between consecutive sibling turns; no sequence edges between sibling tools. - treeReducer test fixup: TurnCompletedParams requires toolCount. 73/73 dashboard tests pass; typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…#432) User pushed back on sibling-stacked thinkings: they wanted the actual ReAct pipeline visible — thinking spawns tools, tool results merge into the next thinking, last tools merge into the response. The previous round drew thinking siblings vertically with a dashed connector between them; this expresses ReAct as the linear pipeline it actually is. Layout per turn now: turn ── thinking1 ─┬─ toolA ─ ─ ─┐ └─ toolB ─ ─ ─┴─ thinking2 ─── toolC ─ ─ ─ response Implementation in useTreeLayout: - populateGraph skips redundant turn→thinking[i] containment edges for i > 0 and turn→response when any thinking exists; the flow chain reaches them. Without the skip, dagre would draw a long containment line from turn to thinking[i+1] crossing the chain visually for no semantic gain. - New addReActFlowEdges adds dagre flow edges with { flow: true } label: - For each consecutive thinking pair under a turn: every tool of thinking[i] gets an edge to thinking[i+1]. Multiple parallel tools all merge into the next thinking, mirroring "tool results feed the next LLM call". - For the response: every tool of the final thinking gets an edge to response (or the final thinking itself when it had no tools). - When iterating dagre.edges() we read the edge label and mark kind='sequence' for flow edges so GrowingEdge renders dashed. Sibling-sequence (vertical dashed) now applies only to turns under a session/step. Thinking-sibling sequence is replaced by the horizontal flow chain. Tests: - chains thinking → tools → next thinking via flow edges: pins the 3 flow edges (toolA→th2, toolB→th2, toolC→resp) and confirms the redundant turn→th2 and turn→resp containment edges are absent. - existing "dashed sequence between sibling turns" test still passes (vertical sibling sequence preserved for turns). 74/74 dashboard tests green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![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: 4417a48a19
ℹ️ 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".
Comment on lines
+227
to
230
| const result = parseEnvelopeFromObject(data); | ||
| if (!result) return; | ||
| // Cross-session filter: the bridge broadcasts every session's events | ||
| // to every connected client. A multi-tenant browser tab on its own |
There was a problem hiding this comment.
This path dispatches live envelopes immediately, even before the first snapshot.response has been applied. If a newly-connected tab receives a newer live event first (for example eventId=120) and then receives snapshot events 1..120, the reducer watermark will already be 120, so replayed historical events are dropped by dedup and the tree can never reconstruct missing structure (session/turn roots, prior tools/text). Add a handshake state that buffers or ignores live events until initial snapshot replay for that session is finished, then apply queued live events in order.
Useful? React with 👍 / 👎.
Two UX nits the user caught while watching live execution: 1. Dashed flow/sequence edges had no direction indicator. With parallel tools merging into the next thinking, multiple converging dashes look ambiguous — readers couldn't tell which way the data flowed at a glance. Added an SVG <marker> with a small triangle (orient=auto-start-reverse so it follows the path tangent at the endpoint). markerEnd applied only to sequence edges; containment edges keep clean — the bezier shape plus left-to-right convention already implies direction, and arrows on every edge would be noise. 2. Auto-fit only re-ran when dagre's bounding box changed. Status-only updates (tool_completed, plan_step_completed) leave width×height alone, so the diagram could grow visible-content-wise without retriggering the fit. Switch the layoutSignature key from "WxH" to "WxH#nodeCount" so every new node refits — userControlledRef still gates this so manual pan/zoom locks autopilot out. 74/74 dashboard tests green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Live testing exposed the bug: userControlledRef was set to true on the first pointer/wheel event and never reset, so a tab that had ever been panned silently stopped auto-fitting forever — the user typed a new prompt and watched the tree grow off-screen with no feedback. The original gate was meant to "respect operator intent". For a static diagram that's right; for a streaming execution tree it's wrong — the user explicitly wants the view to follow new nodes. Drop the ref entirely. The new gate is simply "are we mid-drag?" — checked via dragRef (already tracked for pan). A wheel zoom is instantaneous, so there's nothing to interrupt; an active drag is the only state where yanking the view would visibly fight the user's gesture. After drag release, the next event re-fits. 74/74 dashboard tests green; typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Live testing turned up that the previous behaviour was re-fitting the WHOLE bounding box on every new node — so as the tree grew, the scale shrank and the final response landed in a zoomed-out fit-everything view. The user's feedback: "after response don't jump back, just stop where you are". They want the camera to follow the action at a fixed scale, not zoom out to keep fitting more. Restructure: - Initial auto-fit: runs ONCE per session on the first render with content. Picks a scale that shows the whole tree at that moment. Reset on sessionId change so picking a new session re-fits to its dimensions (lastFitRef tracks per-session state). - Auto-scroll: pans to the active node whenever activeNodeId or the layout.nodes array identity changes. Scale stays where the initial fit left it — no rescale per event. Once the turn completes and activeNodeId stops moving, the camera sits on the last active node and stays put. That's the "stop after response" behaviour. The dragRef gate stays — pan-mid-gesture still skips both effects so the view doesn't fight the user's drag. 74/74 dashboard tests green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…432) Two visual fixes the user caught while watching live execution: 1. Thinking node never stopped pulsing. The reducer minted thinking nodes with status='running' but had no transition to 'completed' — tools manage their own status via stream.tool_completed, but thinking is delta-only with no explicit "ended" event. The pulse glow stayed on forever, even after the turn moved to tool execution or response. Fix: thinking transitions to 'completed' the moment something seals the anchor. addToolNode and appendTextToCurrentTurn now flip currentThinkingId.status from running to completed before returning the new state. completeTurnNode also sweeps any STILL-running thinking/text descendants on turn close (covers the rare case where the turn ends without ever firing a tool_use or text after a thinking — the sweep makes sure no node is stuck pulsing past the turn boundary). 2. Thinking node looked the same as tools at a glance — both share the default blue/green status palette, so the eye couldn't tell model reasoning from tool work. Added a TYPE_BG / TYPE_BORDER override for thinking: indigo/violet palette (running=#5b21b6 violet-800, completed=#1e1b4b indigo-950 muted) with matching borders. Other types fall through to the existing status palette unchanged. Tests: - thinking marked completed on tool_use seal - thinking marked completed on text seal - remaining running thinking/text swept on turn_completed (the no-tool, no-text edge case) 77/77 dashboard tests green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Response (text) nodes shared the default green completed-state colour with finished tools, so the eye couldn't tell where in the diagram the model's actual answer landed. Rose/pink palette gives them their own visual identity (pink-700 while streaming, pink-900 muted at rest) — distinct from tools (blue/green) and thinking (purple). Now the three node-type cues read at a glance: - tool: blue → green (the workhorse default) - thinking: indigo/violet (model reasoning) - response: rose/pink (final answer) 77/77 dashboard tests green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pink response was a clash, not a system. Replace with amber so all three node types form one harmonious jewel-tone family at matched depth (Tailwind 700/800 running, 900/950 completed): - tool: blue → green (workhorse default) - thinking: indigo/violet (cool jewel — model reasoning) - response: amber/gold (warm jewel — final answer) Hue separates type; brightness separates running vs done. The pink broke that system because it was at the same warm pole as amber would be — picking one warm hue (amber) and one cool (indigo/violet) makes the contrast intentional rather than noisy. 77/77 dashboard tests green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Dark jewel-tone backgrounds (700-800 fills) read heavy and clashy.
Switch to the modern dashboard idiom: every node is a tinted-glass
card — accent colour at low fill opacity, full-opacity accent border,
running pops via higher alpha. Looks closer to Linear/Vercel/Notion
dark mode — light, airy, unified.
Palette is now Tailwind 200-400 range (pastel) instead of 700-900
(jewel):
- tool: blue-400 / green-400 (sky → fresh)
- thinking: violet-400 / 300 (lavender, paler when sealed)
- response: amber-400 / 200 (gold, paler when done)
Status now reads via TWO contrasts on the same dark base:
1. hue family (which TYPE)
2. fill alpha (running 0.22 vs completed 0.12) AND palette shift
(running brighter, completed muted)
Both signals pull in the same direction so a quick glance can tell
type AND state. Border stays full-opacity accent so node outlines
remain crisp regardless of fill alpha.
77/77 dashboard tests green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The completed-state checkmark was hardcoded to a single green regardless of node type, so a finished thinking and a finished response both showed the same green ✓ — fighting the type-coloured border the rest of the node carries. Pull the icon colour off the node's accent palette via a new optional `color` prop on StatusIcon that overrides the default per-status colour. GrowingNode now passes its computed accent down, so: - completed tool → green ✓ (unchanged — its accent IS green) - completed thinking → violet ✓ - completed response → amber ✓ The icon and the border now read as one piece, and the eye sees "this whole box is type X, in state Y" rather than two competing colour languages. 77/77 dashboard tests green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3748cd4def
ℹ️ 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".
Comment on lines
+807
to
+813
| // Drop the snapshot buffer for this session (issue #432). The | ||
| // session_ended event was just buffered above, so a tab that | ||
| // opens between this point and the buffer being cleared still | ||
| // gets a snapshot containing session_ended; afterward the | ||
| // session is gone from sessionManager, so sessions.list won't | ||
| // surface it and snapshot.request returns an empty list. | ||
| bridge.clearSession(session.id()); |
There was a problem hiding this comment.
Clearing the session buffer immediately after broadcasting stream.session_ended makes snapshot.request effectively return empty for that session in almost all reconnect/refresh cases, because the clear happens synchronously right after broadcast. If a tab disconnects around session shutdown and misses the live stream.session_ended, it cannot recover that terminal event via snapshot replay and may keep rendering the session as still running indefinitely. Retain the buffer at least until clients have a chance to reconnect (e.g., TTL or delayed clear) so the end-state can be replayed.
Useful? React with 👍 / 👎.
Two activeNodeId problems chained into one bad UX after turn close: 1. appendTextToCurrentTurn never updated activeNodeId, so the camera stayed centred on the LAST tool while the model was streaming the response — user couldn't see the response form. 2. completeTurnNode bubbled activeNodeId up to the turn's parent (session/step) on close, so the moment the turn ended the auto- scroll yanked the camera to the session box. The response had just finished streaming, then the user lost it. The complaint was "response 结束就结束, don't jump back". Fix: - appendTextToCurrentTurn sets activeNodeId to the response node id (newly minted on first delta, or the existing one on subsequent deltas). Camera now follows the response while it streams; once the response stops growing, activeNodeId stops changing and the camera sits there. - completeTurnNode no longer touches activeNodeId. The leaf the user was watching (last tool, or response) stays the focus point through and after the turn close. The auto-scroll re-fires on the layout-change tick from the turn flipping to completed, but its target is the same leaf so the pan is a no-op. 77/77 dashboard tests green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codex P1 on PR #448: a live envelope arriving on the socket BEFORE the snapshot.response advances the reducer's lastEventId watermark past every snapshot envelope. The snapshot replay then gets entirely dedup'd by that watermark, leaving the tree with no structural roots (session/turn/tool) and the user staring at an empty diagram even though the daemon shipped the snapshot correctly. Race window: client connects, sends snapshot.request; daemon's broadcast (in another Jetty thread) calls client.send for an unrelated live event before the inbound handler's ctx.send for the snapshot reply. WS-level write ordering is by send-call order, so either can win — small window but real. Fix: snapshot handshake gate. - Two ref-scoped state vars in the WS effect: snapshotPending: bool and pendingLive: envelope[]. Reset on every reconnect (fresh socket = fresh handshake). - onopen sets snapshotPending=true and clears the queue, then sends snapshot.request. - onmessage: live envelopes during pending are pushed to the queue instead of dispatched. snapshot.response handler runs the snapshot events through the reducer, then flushes the queue in arrival order — the reducer's existing eventId<=lastEventId dedup correctly drops queued events that fell inside the snapshot's range, while newer ones apply. - Unknown (forward-compat) live events during pending are skipped outright — they'd otherwise advance lastEventId via the watermark shortcut and poison the snapshot replay. Test (useExecutionTreeSnapshotGate.test.tsx): mocks WebSocket via vi.stubGlobal so the test drives message ordering directly. Three cases: queued live event with eventId>snapshot's range survives flush; queued live event whose eventId is inside snapshot range gets dedup'd; post-snapshot live events flow without re-gating. 80/80 dashboard tests green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57b5e2cc1c
ℹ️ 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".
Comment on lines
+282
to
+284
| if (snapshotPending) { | ||
| pendingLive.push(result.envelope); | ||
| return; |
There was a problem hiding this comment.
When the socket opens, snapshotPending is set to true and only cleared inside the snapshot.response handler. If that response is never delivered (e.g. daemon error path, mixed-version daemon without snapshot.request, or a malformed reply), every known live envelope is appended to pendingLive and never dispatched, so the tree stays blank while the queue grows unbounded. Please add a bounded fallback (timeout or max queue + fail-open) so live events can resume even if snapshot replay fails.
Useful? React with 👍 / 👎.
6 tasks
This was referenced Apr 30, 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
Closes #432.
A browser tab connecting mid-execution (clicking a session in the sidebar, refresh, tab switch) used to see only events emitted after it connected — root nodes, in-flight tools, and assistant text were all missing. This adds a snapshot endpoint so the dashboard can rebuild its tree from history on first paint.
Architecture decision (deviation from issue): events-list snapshot, not state-object snapshot. The reducer is the source of truth for "events → tree"; reproducing that logic in Java would double the surface area. Shipping the buffered envelopes lets the existing reducer build state once. Cost: snapshot 10-100KB per Tier 1 session vs <10KB state object — fine for interactive sessions, capped at 5000 envelopes per session with drop-oldest eviction.
Changes
Daemon
SessionEventBuffer: per-session capped ring buffer of broadcast envelopesWebSocketBridge.broadcastappends to buffer unconditionally (even with zero clients) so tabs that open AFTER an event was emitted still see it on replaysnapshot.request {sessionId}→snapshot.response {sessionId, lastEventId, events:[]}. Point-to-point reply (sibling ofsessions.listfrom feat(dashboard): multi-session discovery — sessions.list endpoint + SessionList sidebar #445)setSessionEndCallbackaftersession_endedbroadcast lands so the brief race window still includessession_endedin the snapshotDashboard
useExecutionTree.onopensendssnapshot.request; reconnects re-issue automaticallyonmessagedetectssnapshot.responseand replays each envelope through the same validator as live frameseventId <= state.lastEventId) handles snapshot/live overlap, regardless of arrival orderBugs this fixes (all from #445 testing)
session_startedTest plan
./gradlew :aceclaw-daemon:test --rerun-tasks)npm test -- --run→ 64/64)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests