Skip to content

Commit 9c8a738

Browse files
dimakisclaude
andcommitted
fix: address review — preview cache wiring + missing tests
- Move preview DB write before forceFlush so turn_end handler can read it and populate the in-memory cache for real-time SSE - Wire overviewEmitter.updatePreview() in the turn_end handler - Add EventStore preview round-trip tests (3 tests) - Add SessionFeed component tests (8 tests) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 66e75d4 commit 9c8a738

4 files changed

Lines changed: 152 additions & 208 deletions

File tree

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
// @vitest-environment jsdom
2+
import { describe, it, expect, vi, afterEach } from 'vitest';
3+
import { render, screen, cleanup, fireEvent } from '@testing-library/react';
4+
import { MemoryRouter } from 'react-router-dom';
5+
import type { SessionActivity } from '@mitzo/protocol';
6+
7+
const mockNavigate = vi.fn();
8+
vi.mock('react-router-dom', async () => {
9+
const actual = await vi.importActual('react-router-dom');
10+
return { ...actual, useNavigate: () => mockNavigate };
11+
});
12+
13+
const mockFeed = {
14+
items: [] as SessionActivity[],
15+
counts: { all: 0, needsMe: 0, inProgress: 0, done: 0 },
16+
filter: 'needs_me' as const,
17+
setFilter: vi.fn(),
18+
connected: true,
19+
};
20+
21+
vi.mock('../../hooks/useSessionFeed', () => ({
22+
useSessionFeed: () => mockFeed,
23+
}));
24+
25+
import { SessionFeed } from '../SessionFeed';
26+
27+
function renderFeed() {
28+
return render(
29+
<MemoryRouter>
30+
<SessionFeed />
31+
</MemoryRouter>,
32+
);
33+
}
34+
35+
afterEach(() => {
36+
cleanup();
37+
mockNavigate.mockClear();
38+
mockFeed.setFilter.mockClear();
39+
});
40+
41+
function makeActivity(overrides: Partial<SessionActivity> = {}): SessionActivity {
42+
return {
43+
sessionId: 'sess-1',
44+
clientId: 'client-1',
45+
title: 'Test Session',
46+
state: 'working',
47+
flags: [],
48+
lastEventAt: Date.now(),
49+
...overrides,
50+
};
51+
}
52+
53+
describe('SessionFeed', () => {
54+
it('renders nothing when counts.all is 0', () => {
55+
mockFeed.counts = { all: 0, needsMe: 0, inProgress: 0, done: 0 };
56+
mockFeed.items = [];
57+
const { container } = renderFeed();
58+
expect(container.querySelector('.feed-section')).toBeNull();
59+
});
60+
61+
it('renders filter chips with counts', () => {
62+
mockFeed.counts = { all: 5, needsMe: 2, inProgress: 2, done: 1 };
63+
mockFeed.items = [makeActivity()];
64+
renderFeed();
65+
expect(screen.getByText('All')).toBeTruthy();
66+
expect(screen.getByText('Needs me')).toBeTruthy();
67+
expect(screen.getByText('5')).toBeTruthy();
68+
const countBadges = screen.getAllByText('2');
69+
expect(countBadges.length).toBeGreaterThanOrEqual(1);
70+
});
71+
72+
it('calls setFilter when a chip is tapped', () => {
73+
mockFeed.counts = { all: 3, needsMe: 1, inProgress: 1, done: 1 };
74+
mockFeed.items = [makeActivity()];
75+
renderFeed();
76+
fireEvent.click(screen.getByText('All'));
77+
expect(mockFeed.setFilter).toHaveBeenCalledWith('all');
78+
});
79+
80+
it('navigates to session on card tap', () => {
81+
mockFeed.counts = { all: 1, needsMe: 0, inProgress: 1, done: 0 };
82+
mockFeed.items = [makeActivity({ sessionId: 'sess-abc' })];
83+
renderFeed();
84+
fireEvent.click(screen.getByText('Test Session'));
85+
expect(mockNavigate).toHaveBeenCalledWith('/chat/sess-abc');
86+
});
87+
88+
it('shows message preview when available', () => {
89+
mockFeed.counts = { all: 1, needsMe: 0, inProgress: 1, done: 0 };
90+
mockFeed.items = [makeActivity({ lastMessagePreview: 'Here is the implementation plan...' })];
91+
renderFeed();
92+
expect(screen.getByText('Here is the implementation plan...')).toBeTruthy();
93+
});
94+
95+
it('renders without preview gracefully', () => {
96+
mockFeed.counts = { all: 1, needsMe: 0, inProgress: 1, done: 0 };
97+
mockFeed.items = [makeActivity()];
98+
const { container } = renderFeed();
99+
expect(container.querySelector('.feed-card-preview')).toBeNull();
100+
expect(screen.getByText('Test Session')).toBeTruthy();
101+
});
102+
103+
it('shows empty state text for needs_me filter', () => {
104+
mockFeed.counts = { all: 1, needsMe: 0, inProgress: 1, done: 0 };
105+
mockFeed.items = [];
106+
mockFeed.filter = 'needs_me';
107+
renderFeed();
108+
expect(screen.getByText('All clear')).toBeTruthy();
109+
});
110+
111+
it('shows repo tag when present', () => {
112+
mockFeed.counts = { all: 1, needsMe: 0, inProgress: 1, done: 0 };
113+
mockFeed.items = [makeActivity({ repo: 'mitzo' })];
114+
renderFeed();
115+
expect(screen.getByText('mitzo:')).toBeTruthy();
116+
});
117+
});

packages/protocol/__tests__/event-store.test.ts

Lines changed: 14 additions & 197 deletions
Original file line numberDiff line numberDiff line change
@@ -642,212 +642,29 @@ describe('EventStore', () => {
642642
});
643643
});
644644

645-
describe('toClientState mapping (via session_state_changed events)', () => {
646-
const sid = 'client-state-test';
645+
describe('lastAssistantPreview', () => {
646+
const sid = 'preview-session';
647647

648648
beforeEach(() => {
649649
store.upsertSession({ sessionId: sid });
650650
});
651651

652-
it('maps CREATED to idle', () => {
653-
store.setSessionState(sid, 'CREATED', { force: true });
654-
const events = store.getSessionEvents(sid);
655-
const stateEvent = events.find((e) => e.type === 'session_state_changed');
656-
expect(stateEvent?.payload.state).toBe('idle');
657-
expect(stateEvent?.payload.internalState).toBe('CREATED');
658-
});
659-
660-
it('maps STARTING to running', () => {
661-
store.setSessionState(sid, 'STARTING', { force: true });
662-
const events = store.getSessionEvents(sid);
663-
const stateEvent = events.find((e) => e.type === 'session_state_changed');
664-
expect(stateEvent?.payload.state).toBe('running');
665-
});
666-
667-
it('maps ACTIVE to running', () => {
668-
store.setSessionState(sid, 'ACTIVE', { force: true });
669-
const events = store.getSessionEvents(sid);
670-
const stateEvent = events.find((e) => e.type === 'session_state_changed');
671-
expect(stateEvent?.payload.state).toBe('running');
672-
});
673-
674-
it('maps DETACHED to idle', () => {
675-
store.setSessionState(sid, 'DETACHED', { force: true });
676-
const events = store.getSessionEvents(sid);
677-
const stateEvent = events.find((e) => e.type === 'session_state_changed');
678-
expect(stateEvent?.payload.state).toBe('idle');
679-
});
680-
681-
it('maps SUSPENDED to idle', () => {
682-
store.setSessionState(sid, 'SUSPENDED', { force: true });
683-
const events = store.getSessionEvents(sid);
684-
const stateEvent = events.find((e) => e.type === 'session_state_changed');
685-
expect(stateEvent?.payload.state).toBe('idle');
686-
});
687-
688-
it('maps CLOSING to idle', () => {
689-
store.setSessionState(sid, 'CLOSING', { force: true });
690-
const events = store.getSessionEvents(sid);
691-
const stateEvent = events.find((e) => e.type === 'session_state_changed');
692-
expect(stateEvent?.payload.state).toBe('idle');
693-
});
694-
695-
it('maps ENDED to idle', () => {
696-
store.setSessionState(sid, 'ENDED', { force: true });
697-
const events = store.getSessionEvents(sid);
698-
const stateEvent = events.find((e) => e.type === 'session_state_changed');
699-
expect(stateEvent?.payload.state).toBe('idle');
700-
});
701-
702-
it('includes timestamp in event payload', () => {
703-
const before = Date.now();
704-
store.setSessionState(sid, 'ACTIVE', { force: true });
705-
const events = store.getSessionEvents(sid);
706-
const stateEvent = events.find((e) => e.type === 'session_state_changed');
707-
expect(stateEvent?.payload.timestamp).toBeGreaterThanOrEqual(before);
708-
});
709-
});
710-
711-
describe('setSessionState syncs is_active', () => {
712-
const sid = 'is-active-sync-test';
713-
714-
beforeEach(() => {
715-
store.upsertSession({ sessionId: sid });
716-
});
717-
718-
it('sets is_active=true for ACTIVE', () => {
719-
store.setSessionState(sid, 'ACTIVE', { force: true });
720-
expect(store.getSession(sid)!.isActive).toBe(true);
721-
});
722-
723-
it('sets is_active=true for DETACHED', () => {
724-
store.setSessionState(sid, 'DETACHED', { force: true });
725-
expect(store.getSession(sid)!.isActive).toBe(true);
726-
});
727-
728-
it('sets is_active=true for SUSPENDED', () => {
729-
store.setSessionState(sid, 'SUSPENDED', { force: true });
730-
expect(store.getSession(sid)!.isActive).toBe(true);
731-
});
732-
733-
it('sets is_active=false for ENDED', () => {
734-
store.setSessionState(sid, 'ENDED', { force: true });
735-
expect(store.getSession(sid)!.isActive).toBe(false);
736-
});
737-
738-
it('sets is_active=false for CLOSING', () => {
739-
store.setSessionState(sid, 'CLOSING', { force: true });
740-
expect(store.getSession(sid)!.isActive).toBe(false);
741-
});
742-
743-
it('sets is_active=true for CREATED', () => {
744-
store.setSessionState(sid, 'CREATED', { force: true });
745-
expect(store.getSession(sid)!.isActive).toBe(true);
746-
});
747-
748-
it('sets is_active=true for STARTING', () => {
749-
store.setSessionState(sid, 'STARTING', { force: true });
750-
expect(store.getSession(sid)!.isActive).toBe(true);
751-
});
752-
});
753-
754-
describe('recoverStaleSessions', () => {
755-
it('transitions ACTIVE sessions to ENDED', () => {
756-
store.upsertSession({ sessionId: 'active-1' });
757-
store.setSessionState('active-1', 'ACTIVE', { force: true });
758-
759-
const count = store.recoverStaleSessions();
760-
761-
expect(count).toBe(1);
762-
expect(store.getSessionState('active-1')).toBe('ENDED');
763-
});
764-
765-
it('transitions STARTING sessions to ENDED', () => {
766-
store.upsertSession({ sessionId: 'starting-1' });
767-
store.setSessionState('starting-1', 'STARTING', { force: true });
768-
769-
store.recoverStaleSessions();
770-
771-
expect(store.getSessionState('starting-1')).toBe('ENDED');
772-
});
773-
774-
it('transitions DETACHED sessions to ENDED', () => {
775-
store.upsertSession({ sessionId: 'detached-1' });
776-
store.setSessionState('detached-1', 'DETACHED', { force: true });
777-
778-
store.recoverStaleSessions();
779-
780-
expect(store.getSessionState('detached-1')).toBe('ENDED');
781-
});
782-
783-
it('transitions SUSPENDED sessions to ENDED', () => {
784-
store.upsertSession({ sessionId: 'suspended-1' });
785-
store.setSessionState('suspended-1', 'SUSPENDED', { force: true });
786-
787-
store.recoverStaleSessions();
788-
789-
expect(store.getSessionState('suspended-1')).toBe('ENDED');
790-
});
791-
792-
it('transitions CLOSING sessions to ENDED', () => {
793-
store.upsertSession({ sessionId: 'closing-1' });
794-
store.setSessionState('closing-1', 'CLOSING', { force: true });
795-
796-
store.recoverStaleSessions();
797-
798-
expect(store.getSessionState('closing-1')).toBe('ENDED');
799-
});
800-
801-
it('does not touch ENDED sessions', () => {
802-
store.upsertSession({ sessionId: 'ended-1' });
803-
store.setSessionState('ended-1', 'ENDED', { force: true });
804-
805-
const count = store.recoverStaleSessions();
806-
807-
expect(count).toBe(0);
808-
expect(store.getSessionState('ended-1')).toBe('ENDED');
809-
});
810-
811-
it('does not touch CREATED sessions', () => {
812-
store.upsertSession({ sessionId: 'created-1' });
813-
store.setSessionState('created-1', 'CREATED', { force: true });
814-
815-
const count = store.recoverStaleSessions();
816-
817-
expect(count).toBe(0);
818-
expect(store.getSessionState('created-1')).toBe('CREATED');
652+
it('stores and retrieves preview text', () => {
653+
store.updateLastAssistantPreview(sid, 'Here is the plan...');
654+
const meta = store.getSession(sid);
655+
expect(meta?.lastAssistantPreview).toBe('Here is the plan...');
819656
});
820657

821-
it('returns correct count for multiple stale sessions', () => {
822-
store.upsertSession({ sessionId: 'stale-1' });
823-
store.upsertSession({ sessionId: 'stale-2' });
824-
store.upsertSession({ sessionId: 'ok-1' });
825-
store.setSessionState('stale-1', 'ACTIVE', { force: true });
826-
store.setSessionState('stale-2', 'DETACHED', { force: true });
827-
store.setSessionState('ok-1', 'ENDED', { force: true });
828-
829-
const count = store.recoverStaleSessions();
830-
831-
expect(count).toBe(2);
658+
it('overwrites previous preview on update', () => {
659+
store.updateLastAssistantPreview(sid, 'First response');
660+
store.updateLastAssistantPreview(sid, 'Second response');
661+
const meta = store.getSession(sid);
662+
expect(meta?.lastAssistantPreview).toBe('Second response');
832663
});
833664

834-
it('emits session_state_changed events for recovered sessions', () => {
835-
store.upsertSession({ sessionId: 'recover-1' });
836-
store.setSessionState('recover-1', 'ACTIVE', { force: true });
837-
838-
// Clear events from setup
839-
const beforeCount = store.getSessionEvents('recover-1').length;
840-
841-
store.recoverStaleSessions();
842-
843-
const events = store.getSessionEvents('recover-1');
844-
// Should have new session_state_changed event from recovery
845-
const recoveryEvent = events
846-
.slice(beforeCount)
847-
.find((e) => e.type === 'session_state_changed');
848-
expect(recoveryEvent).toBeDefined();
849-
expect(recoveryEvent?.payload.state).toBe('idle');
850-
expect(recoveryEvent?.payload.internalState).toBe('ENDED');
665+
it('returns null when no preview has been set', () => {
666+
const meta = store.getSession(sid);
667+
expect(meta?.lastAssistantPreview).toBeNull();
851668
});
852669
});
853670

server/index.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,11 @@ setSessionChangeCallback((clientId, event, sessionId) => {
315315
if (session?.sessionId) {
316316
eventStore.updateLastSpeaker(session.sessionId, 'assistant');
317317
overviewEmitter.updateSpeaker(session.sessionId, 'assistant');
318+
// Update preview cache from DB (preview was persisted before flush fired turn_end)
319+
const meta = eventStore.getSession(session.sessionId);
320+
if (meta?.lastAssistantPreview) {
321+
overviewEmitter.updatePreview(session.sessionId, meta.lastAssistantPreview);
322+
}
318323
}
319324
}
320325
overviewEmitter.scheduleBroadcast();

0 commit comments

Comments
 (0)