feat(home): unified session feed with message preview#409
Conversation
dimakis
left a comment
dimakis
left a comment
There was a problem hiding this comment.
Centaur Review
Found 6 issue(s) (3 warning).
server/query-loop.ts
Solid refactor that simplifies the home feed, but the preview cache isn't wired up for real-time updates (query-loop doesn't call emitter.updatePreview), and the removal of Telos/ATB attention items from the home page is a notable behavioral regression that should be intentional.
- 🟡 bugs (L569): Preview is persisted to the EventStore but
SessionOverviewEmitter.updatePreview()is never called, so the in-memory preview cache stays stale. Live SSE broadcasts viagetCachedPreview()will miss the latest preview until the cache is opportunistically populated bygetCachedSpeaker()on the next tick. TheupdatePreview()method exists on the emitter but is unreachable from query-loop (no reference passed in). Preview will eventually appear after a page refresh or cache miss, but real-time updates will lag.[fixable] - 🔵 bugs (L563):
previewText.slice(-300)takes the last 300 characters. For a user-facing preview, taking the first 300 characters (.slice(0, 300)) would be more useful — the beginning of a response typically has the most context (greeting, summary, key decision). The trailing 300 chars might be mid-sentence code or boilerplate.[fixable]
frontend/src/pages/SessionList.tsx
Solid refactor that simplifies the home feed, but the preview cache isn't wired up for real-time updates (query-loop doesn't call emitter.updatePreview), and the removal of Telos/ATB attention items from the home page is a notable behavioral regression that should be intentional.
- 🟡 regressions (L312): Telos (todo) and ATB (task board) attention items are no longer surfaced on the home page. The deleted
useAttentionFeedaggregated items from three sources (telos, ATB, sessions); the replacementuseSessionFeedonly shows sessions. Starred/urgent todos and pending_review/blocked/failed tasks previously appeared as tier-1 attention items on the home screen and now only appear in the CommandCenter sidebar (visible only inside a chat session). This may be intentional but is a significant behavioral change worth calling out.
frontend/src/components/SessionFeed.tsx
Solid refactor that simplifies the home feed, but the preview cache isn't wired up for real-time updates (query-loop doesn't call emitter.updatePreview), and the removal of Telos/ATB attention items from the home page is a notable behavioral regression that should be intentional.
- 🟡 missing_tests: No component test for
SessionFeed.tsx. The deletedAttentionFeed.tsxandSessionOverview.tsxboth had comprehensive component tests (rendering, navigation, state transitions). The replacement has hook-level tests but no component-level tests for rendering, filter chip interaction, card tap navigation, empty state display, or thecounts.all === 0early return.[fixable]
packages/protocol/src/event-store.ts
Solid refactor that simplifies the home feed, but the preview cache isn't wired up for real-time updates (query-loop doesn't call emitter.updatePreview), and the removal of Telos/ATB attention items from the home page is a notable behavioral regression that should be intentional.
- 🔵 missing_tests (L589): No test coverage for the new
updateLastAssistantPreview()method, themigratePreviewColumn()migration, or thelastAssistantPreviewfield inrowToSession(). The existing EventStore test file has no assertions for preview persistence or retrieval.[fixable]
frontend/src/hooks/useSessionFeed.ts
Solid refactor that simplifies the home feed, but the preview cache isn't wired up for real-time updates (query-loop doesn't call emitter.updatePreview), and the removal of Telos/ATB attention items from the home page is a notable behavioral regression that should be intentional.
- 🔵 unsafe_assumptions (L96):
workingBatchusesDate.now()insideuseMemobut only recomputes whenactivitieschanges. If no new SSE events arrive, expired done sessions (past the 48h window) won't be evicted from the batch until the next activity update. This is minor — SSE events are frequent — but could show stale sessions on a quiet day.
| if (store && resolvedSessionId) { | ||
| store.recordUsage(resolvedSessionId, usageData); | ||
| if (lastPreview) { | ||
| store.updateLastAssistantPreview(resolvedSessionId, lastPreview); |
There was a problem hiding this comment.
🟡 bugs: Preview is persisted to the EventStore but SessionOverviewEmitter.updatePreview() is never called, so the in-memory preview cache stays stale. Live SSE broadcasts via getCachedPreview() will miss the latest preview until the cache is opportunistically populated by getCachedSpeaker() on the next tick. The updatePreview() method exists on the emitter but is unreachable from query-loop (no reference passed in). Preview will eventually appear after a page refresh or cache miss, but real-time updates will lag. [fixable]
| .map((b) => b.content) | ||
| .join('\n') | ||
| .trim(); | ||
| const lastPreview = previewText.length > 300 ? previewText.slice(-300) : previewText; |
There was a problem hiding this comment.
🔵 bugs: previewText.slice(-300) takes the last 300 characters. For a user-facing preview, taking the first 300 characters (.slice(0, 300)) would be more useful — the beginning of a response typically has the most context (greeting, summary, key decision). The trailing 300 chars might be mid-sentence code or boilerplate. [fixable]
| @@ -311,9 +310,7 @@ export function SessionList() { | |||
|
|
|||
| <BriefingCard /> | |||
|
|
|||
There was a problem hiding this comment.
🟡 regressions: Telos (todo) and ATB (task board) attention items are no longer surfaced on the home page. The deleted useAttentionFeed aggregated items from three sources (telos, ATB, sessions); the replacement useSessionFeed only shows sessions. Starred/urgent todos and pending_review/blocked/failed tasks previously appeared as tier-1 attention items on the home screen and now only appear in the CommandCenter sidebar (visible only inside a chat session). This may be intentional but is a significant behavioral change worth calling out.
| this.stmts.updateLastSpeaker.run(speaker, sessionId); | ||
| } | ||
|
|
||
| updateLastAssistantPreview(sessionId: string, preview: string): void { |
There was a problem hiding this comment.
🔵 missing_tests: No test coverage for the new updateLastAssistantPreview() method, the migratePreviewColumn() migration, or the lastAssistantPreview field in rowToSession(). The existing EventStore test file has no assertions for preview persistence or retrieval. [fixable]
| }, []); | ||
|
|
||
| // Working batch: all sessions in the user's current active set | ||
| const workingBatch = useMemo(() => { |
There was a problem hiding this comment.
🔵 unsafe_assumptions: workingBatch uses Date.now() inside useMemo but only recomputes when activities changes. If no new SSE events arrive, expired done sessions (past the 48h window) won't be evicted from the batch until the next activity update. This is minor — SSE events are frequent — but could show stale sessions on a quiet day.
Review responseFixed in 7264748:
Design decisions (intentional, no change):
|
dimakis
left a comment
dimakis
left a comment
There was a problem hiding this comment.
Centaur Review
Found 7 issue(s) (2 warning).
server/session-overview.ts
Solid replacement of two overlapping home-screen sections with a cleaner unified feed. Main concerns: previewCache not cleared in destroy(), loss of Telos/ATB attention items with no obvious replacement, and paused-session visibility change.
- 🟡 bugs: The new
previewCacheMap is not cleared indestroy(). The diff addspreviewCache(line 82 in diff) and cleans it inforget()(line 140), butdestroy()(lines 457-473, unchanged) only clearslastEventTimes,uncommittedCache, andspeakerCache. Addthis.previewCache.clear()alongside the other cache clears indestroy().[fixable]
frontend/src/hooks/useSessionFeed.ts
Solid replacement of two overlapping home-screen sections with a cleaner unified feed. Main concerns: previewCache not cleared in destroy(), loss of Telos/ATB attention items with no obvious replacement, and paused-session visibility change.
- 🟡 regressions: The old
AttentionFeedaggregated items from three sources — Telos todos, ATB tasks, and sessions — with tiered priority sorting. The newSessionFeedonly shows sessions. Telos starred/urgent items and ATBpending_review/blocked/failedtasks no longer surface on the home screen. If this is intentional, consider whether those signals need a new home or if removing them is the desired UX change. - 🔵 regressions (L33): The old
SessionOverviewdisplayedpausedsessions (it only excludedidleandinit). The newisInWorkingBatch()excludespausedsessions — they won't appear in any filter. TheSTATE_CONFIGinSessionFeed.tsxdefines apausedentry that can never be reached. If paused sessions should be visible, adda.state === 'paused'toisInWorkingBatch().[fixable] - 🔵 unsafe_assumptions (L86): The SSE callback casts
data as SessionActivity[]without runtime validation. The olduseAttentionFeedfiltered withtypeof d.sessionId === 'string' && typeof d.state === 'string'. While this matches the pattern used byuseSessionOverviewanduseSessionList, the lack of validation means a malformed SSE payload could propagate undefined fields into the filter/sort logic. Low risk since the server controls the data, but worth noting.[fixable]
frontend/src/components/SessionFeed.tsx
Solid replacement of two overlapping home-screen sections with a cleaner unified feed. Main concerns: previewCache not cleared in destroy(), loss of Telos/ATB attention items with no obvious replacement, and paused-session visibility change.
- 🔵 regressions (L63): The old
SessionOverviewandAttentionFeedcalledselectionChanged()(haptic feedback) on card tap. The newSessionFeed.handleTapnavigates without haptics. If this is a mobile-first app, users may notice the missing tactile feedback on session card taps.[fixable] - 🔵 style (L13): Dead config entry:
STATE_CONFIGincludespausedandinitentries, butisInWorkingBatch()in the hook excludes both states from ever reaching the feed. These entries can never be rendered. Consider removing them or adding a comment explaining they're kept for completeness.[fixable]
frontend/src/hooks/__tests__/useSessionFeed.test.ts
Solid replacement of two overlapping home-screen sections with a cleaner unified feed. Main concerns: previewCache not cleared in destroy(), loss of Telos/ATB attention items with no obvious replacement, and paused-session visibility change.
- 🔵 missing_tests: No test covers
pausedsession exclusion from the working batch. The test at line 62 verifiesidleandinitare excluded but doesn't assert thatpausedis also filtered out. Add apausedentry to that test's activity list to prevent accidental inclusion in the future.[fixable]
| if (a.state === 'working' || a.state === 'waiting') return true; | ||
| // Awaiting reply or uncommitted work — always relevant | ||
| if (a.awaitingReply || a.uncommittedWork) return true; | ||
| // Done sessions within the time window |
There was a problem hiding this comment.
🔵 regressions: The old SessionOverview displayed paused sessions (it only excluded idle and init). The new isInWorkingBatch() excludes paused sessions — they won't appear in any filter. The STATE_CONFIG in SessionFeed.tsx defines a paused entry that can never be reached. If paused sessions should be visible, add a.state === 'paused' to isInWorkingBatch(). [fixable]
|
|
||
| useEffect(() => { | ||
| const unsubActivity = eventBus.on('session_activity', (data) => { | ||
| setActivities(data as SessionActivity[]); |
There was a problem hiding this comment.
🔵 unsafe_assumptions: The SSE callback casts data as SessionActivity[] without runtime validation. The old useAttentionFeed filtered with typeof d.sessionId === 'string' && typeof d.state === 'string'. While this matches the pattern used by useSessionOverview and useSessionList, the lack of validation means a malformed SSE payload could propagate undefined fields into the filter/sort logic. Low risk since the server controls the data, but worth noting. [fixable]
| <button | ||
| className="feed-card" | ||
| onClick={() => onTap(activity.sessionId)} | ||
| style={{ '--card-accent': iconColor } as React.CSSProperties} |
There was a problem hiding this comment.
🔵 regressions: The old SessionOverview and AttentionFeed called selectionChanged() (haptic feedback) on card tap. The new SessionFeed.handleTap navigates without haptics. If this is a mobile-first app, users may notice the missing tactile feedback on session card taps. [fixable]
| waiting: { icon: '⚠', color: '#ff6d6d', label: 'waiting' }, | ||
| done: { icon: '✓', color: '#4ade80', label: 'done' }, | ||
| idle: { icon: '○', color: 'var(--text-dim)', label: 'idle' }, | ||
| init: { icon: '○', color: 'var(--text-dim)', label: 'starting' }, |
There was a problem hiding this comment.
🔵 style: Dead config entry: STATE_CONFIG includes paused and init entries, but isInWorkingBatch() in the hook excludes both states from ever reaching the feed. These entries can never be rendered. Consider removing them or adding a comment explaining they're kept for completeness. [fixable]
Replace separate Active Sessions + What's Next sections with a single SessionFeed component. Adds chip-based filtering (All / Needs me / In progress / Done), message preview capture from last assistant turn, and oldest-first sort within the working batch (48h window). Backend: preview column in EventStore, capture at turn-end in query-loop, in-memory cache in SessionOverviewEmitter, REST + SSE exposure. Frontend: useSessionFeed hook with working-batch logic, SessionFeed component with filter chips and 5-item scrollable viewport. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- 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>
dimakis
force-pushed
the
session/2026-06-25-22a9ab867321
branch
from
7264748 to
9c8a738
Compare
dimakis
left a comment
dimakis
left a comment
There was a problem hiding this comment.
Centaur Review
Found 7 issue(s) (1 critical) (4 warning).
packages/protocol/__tests__/event-store.test.ts
Solid session feed unification with good backend preview caching, but deletes 212 lines of unrelated event-store tests (critical coverage regression), drops runtime SSE validation, and removes telos/ATB from the mobile home page.
- 🔴 regressions: 212 lines of tests for
toClientStatemapping,setSessionStateis_active syncing, andrecoverStaleSessionswere deleted. These functions still exist inpackages/protocol/src/event-store.ts(lines 27, 632, 666) and are actively used at runtime. This is a significant test coverage regression — especiallyrecoverStaleSessionswhich is critical startup safety logic. These test deletions appear unrelated to the session feed feature and should be restored.[fixable]
frontend/src/hooks/useSessionFeed.ts
Solid session feed unification with good backend preview caching, but deletes 212 lines of unrelated event-store tests (critical coverage regression), drops runtime SSE validation, and removes telos/ATB from the mobile home page.
- 🟡 unsafe_assumptions (L86): Bare
as SessionActivity[]cast on SSE event data without runtime validation. The EventBus callback parameter is typedunknown. The deleteduseAttentionFeedhad defensive validation (Array.isArraycheck + field type guards forsessionIdandstate). If SSE data is malformed or schema drifts,isInWorkingBatch()andneedsMe()will crash accessing.state,.lastEventAt, etc. Add at minimum anArray.isArray(data)guard before the cast.[fixable]
frontend/src/pages/SessionList.tsx
Solid session feed unification with good backend preview caching, but deletes 212 lines of unrelated event-store tests (critical coverage regression), drops runtime SSE validation, and removes telos/ATB from the mobile home page.
- 🟡 regressions: The deleted
AttentionFeedaggregated items from three sources: telos (starred/urgent todos), ATB (pending_review/blocked/failed tasks), and sessions. The replacementSessionFeedonly shows sessions. On mobile, the home page no longer surfaces urgent todos or failing tasks inline — users must navigate to separate tabs. This is a functional regression for mobile users who relied on the unified 'What's Next' feed for triage. If intentional, consider adding a brief note or linking to the telos/tasks tabs.
frontend/src/hooks/__tests__/useSessionFeed.test.ts
Solid session feed unification with good backend preview caching, but deletes 212 lines of unrelated event-store tests (critical coverage regression), drops runtime SSE validation, and removes telos/ATB from the mobile home page.
- 🟡 missing_tests: No test covers the
connectedstate tracking. The hook subscribes toeventBus.onConnectionChangeand exposes aconnectedfield, but no test verifies it updates when connection state changes. Also, no test covers the cleanup (unsubscribe) behavior of theuseEffectthat sets up SSE listeners.[fixable] - 🔵 missing_tests: No test verifies that
uncommittedWorksessions with states other thanworking/waiting(e.g.,done+uncommittedWork: true) are included in the working batch. TheisInWorkingBatchhelper includes them viaa.uncommittedWorkregardless of state, but this case isn't explicitly tested.[fixable]
server/query-loop.ts
Solid session feed unification with good backend preview caching, but deletes 212 lines of unrelated event-store tests (critical coverage regression), drops runtime SSE validation, and removes telos/ATB from the mobile home page.
- 🟡 bugs (L544): Preview extraction joins ALL text blocks in the snapshot with
\n, then takes the last 300 chars. For multi-turn conversations,snapshotBlocksmay contain text from earlier blocks in the same streaming message. The intent is 'last assistant message preview' but the code captures all text blocks from the current snapshot, which could include content from a previous text block if the assistant produced multiple text blocks in one message. Consider taking only the last text block's content.[fixable]
frontend/src/components/SessionFeed.tsx
Solid session feed unification with good backend preview caching, but deletes 212 lines of unrelated event-store tests (critical coverage regression), drops runtime SSE validation, and removes telos/ATB from the mobile home page.
- 🔵 style (L90): The
handleTapcallback is defined inline withoutuseCallback, unlike the deletedSessionOverviewwhich useduseCallbackwith a[navigate]dependency. While functional, this creates a new function on every render. For consistency with the codebase pattern and to prevent unnecessary re-renders ofFeedItemchildren, wrap inuseCallback.[fixable]
|
|
||
| useEffect(() => { | ||
| const unsubActivity = eventBus.on('session_activity', (data) => { | ||
| setActivities(data as SessionActivity[]); |
There was a problem hiding this comment.
🟡 unsafe_assumptions: Bare as SessionActivity[] cast on SSE event data without runtime validation. The EventBus callback parameter is typed unknown. The deleted useAttentionFeed had defensive validation (Array.isArray check + field type guards for sessionId and state). If SSE data is malformed or schema drifts, isInWorkingBatch() and needsMe() will crash accessing .state, .lastEventAt, etc. Add at minimum an Array.isArray(data) guard before the cast. [fixable]
| const snapshotBlocks = currentSession.currentSnapshot?.blocks ?? []; | ||
|
|
||
| // Extract preview from last assistant message (before snapshot is discarded) | ||
| const previewText = snapshotBlocks |
There was a problem hiding this comment.
🟡 bugs: Preview extraction joins ALL text blocks in the snapshot with \n, then takes the last 300 chars. For multi-turn conversations, snapshotBlocks may contain text from earlier blocks in the same streaming message. The intent is 'last assistant message preview' but the code captures all text blocks from the current snapshot, which could include content from a previous text block if the assistant produced multiple text blocks in one message. Consider taking only the last text block's content. [fixable]
|
|
||
| const handleTap = (sessionId: string) => { | ||
| navigate(`/chat/${sessionId}`); | ||
| }; |
There was a problem hiding this comment.
🔵 style: The handleTap callback is defined inline without useCallback, unlike the deleted SessionOverview which used useCallback with a [navigate] dependency. While functional, this creates a new function on every render. For consistency with the codebase pattern and to prevent unnecessary re-renders of FeedItem children, wrap in useCallback. [fixable]
dimakis
left a comment
dimakis
left a comment
There was a problem hiding this comment.
Centaur Review
Found 8 issue(s) (2 critical) (4 warning).
packages/protocol/__tests__/event-store.test.ts
The PR has two critical issues: it deletes ~183 lines of unrelated event-store test coverage (for methods still in production), and it removes Telos/ATB attention items from the home screen without an alternative. The session feed itself is well-structured but needs SSE data validation and haptic feedback restored.
- 🔴 regressions (L645): The PR deletes ~183 lines of tests covering
toClientState mapping,setSessionState syncs is_active, andrecoverStaleSessions. All three methods still exist inevent-store.tsand are actively used at runtime. These deleted tests are the only coverage for those methods — this is a significant test coverage regression. The 29-linelastAssistantPreviewdescribe block that replaces them is unrelated and does not compensate.[fixable]
frontend/src/hooks/useAttentionFeed.ts
The PR has two critical issues: it deletes ~183 lines of unrelated event-store test coverage (for methods still in production), and it removes Telos/ATB attention items from the home screen without an alternative. The session feed itself is well-structured but needs SSE data validation and haptic feedback restored.
- 🔴 regressions: Deleting
useAttentionFeedremoves home-screen visibility for Telos (starred/urgent todos) and ATB (pending_review/blocked/failed tasks). The replacementuseSessionFeedonly surfaces sessions. Users lose the unified attention signal for todos and task board items that need action. If this is intentional, it should be called out explicitly and the Telos/ATB attention items need an alternative home.
frontend/src/hooks/useSessionFeed.ts
The PR has two critical issues: it deletes ~183 lines of unrelated event-store test coverage (for methods still in production), and it removes Telos/ATB attention items from the home screen without an alternative. The session feed itself is well-structured but needs SSE data validation and haptic feedback restored.
- 🟡 bugs (L85): SSE data is cast without runtime validation:
setActivities(data as SessionActivity[]). The olduseAttentionFeedvalidated each record with type guards (Array.isArray(data)+ property checks forsessionIdandstate). Malformed SSE payloads will cause rendering errors instead of being filtered out.[fixable]
frontend/src/components/SessionFeed.tsx
The PR has two critical issues: it deletes ~183 lines of unrelated event-store test coverage (for methods still in production), and it removes Telos/ATB attention items from the home screen without an alternative. The session feed itself is well-structured but needs SSE data validation and haptic feedback restored.
- 🟡 regressions (L89): The old
AttentionFeedandSessionOverviewcomponents calledselectionChanged()from haptics on card tap (iOS haptic feedback). The newSessionFeedomits this, which is a UX regression on mobile/iOS where users expect tactile feedback on interactive elements.[fixable]
server/query-loop.ts
The PR has two critical issues: it deletes ~183 lines of unrelated event-store test coverage (for methods still in production), and it removes Telos/ATB attention items from the home screen without an alternative. The session feed itself is well-structured but needs SSE data validation and haptic feedback restored.
- 🟡 unsafe_assumptions (L544): Preview extraction joins ALL text blocks in
snapshotBlocksand then takesslice(-300)(the tail). For multi-turn or long responses, this surfaces the end of the message rather than the beginning. Users typically scan the start of a message for context. Considerslice(0, 300)or a smarter truncation that captures the first sentence/paragraph.[fixable] - 🔵 missing_tests (L539): The preview extraction logic (filter text blocks, join, trim, truncate to 300 chars, persist via
store.updateLastAssistantPreview) has no unit test. Edge cases worth covering: no text blocks (empty preview), blocks with only whitespace, preview exactly at 300 chars.[fixable]
server/session-overview.ts
The PR has two critical issues: it deletes ~183 lines of unrelated event-store test coverage (for methods still in production), and it removes Telos/ATB attention items from the home screen without an alternative. The session feed itself is well-structured but needs SSE data validation and haptic feedback restored.
- 🟡 missing_tests (L297): The
previewCacheandupdatePreview()additions toSessionOverviewEmitterhave no test coverage. The cache population path (viagetCachedSpeakeropportunistic fill and explicitupdatePreviewcalls) and cleanup path (viaforget) should be tested to ensure previews are populated and evicted correctly.[fixable]
frontend/src/styles/global.css
The PR has two critical issues: it deletes ~183 lines of unrelated event-store test coverage (for methods still in production), and it removes Telos/ATB attention items from the home screen without an alternative. The session feed itself is well-structured but needs SSE data validation and haptic feedback restored.
- 🔵 style (L707):
.feed-listhas a hardcodedmax-height: 380px. This may clip content awkwardly on short screens or waste space on tall ones. Consider using a viewport-relative unit or removing the constraint in favor of the page's natural scroll.[fixable]
|
|
||
| describe('toClientState mapping (via session_state_changed events)', () => { | ||
| const sid = 'client-state-test'; | ||
| describe('lastAssistantPreview', () => { |
There was a problem hiding this comment.
🔴 regressions: The PR deletes ~183 lines of tests covering toClientState mapping, setSessionState syncs is_active, and recoverStaleSessions. All three methods still exist in event-store.ts and are actively used at runtime. These deleted tests are the only coverage for those methods — this is a significant test coverage regression. The 29-line lastAssistantPreview describe block that replaces them is unrelated and does not compensate. [fixable]
| }, []); | ||
|
|
||
| useEffect(() => { | ||
| const unsubActivity = eventBus.on('session_activity', (data) => { |
There was a problem hiding this comment.
🟡 bugs: SSE data is cast without runtime validation: setActivities(data as SessionActivity[]). The old useAttentionFeed validated each record with type guards (Array.isArray(data) + property checks for sessionId and state). Malformed SSE payloads will cause rendering errors instead of being filtered out. [fixable]
| const navigate = useNavigate(); | ||
|
|
||
| const handleTap = (sessionId: string) => { | ||
| navigate(`/chat/${sessionId}`); |
There was a problem hiding this comment.
🟡 regressions: The old AttentionFeed and SessionOverview components called selectionChanged() from haptics on card tap (iOS haptic feedback). The new SessionFeed omits this, which is a UX regression on mobile/iOS where users expect tactile feedback on interactive elements. [fixable]
| const snapshotBlocks = currentSession.currentSnapshot?.blocks ?? []; | ||
|
|
||
| // Extract preview from last assistant message (before snapshot is discarded) | ||
| const previewText = snapshotBlocks |
There was a problem hiding this comment.
🟡 unsafe_assumptions: Preview extraction joins ALL text blocks in snapshotBlocks and then takes slice(-300) (the tail). For multi-turn or long responses, this surfaces the end of the message rather than the beginning. Users typically scan the start of a message for context. Consider slice(0, 300) or a smarter truncation that captures the first sentence/paragraph. [fixable]
| @@ -539,6 +539,22 @@ async function _runQueryLoopInner( | |||
| log.info('result received', { clientId, sessionId: msg.session_id }); | |||
There was a problem hiding this comment.
🔵 missing_tests: The preview extraction logic (filter text blocks, join, trim, truncate to 300 chars, persist via store.updateLastAssistantPreview) has no unit test. Edge cases worth covering: no text blocks (empty preview), blocks with only whitespace, preview exactly at 300 chars. [fixable]
| @@ -294,6 +297,8 @@ export class SessionOverviewEmitter { | |||
| const speakerAt = speaker.at ?? lastEventAt; | |||
There was a problem hiding this comment.
🟡 missing_tests: The previewCache and updatePreview() additions to SessionOverviewEmitter have no test coverage. The cache population path (via getCachedSpeaker opportunistic fill and explicit updatePreview calls) and cleanup path (via forget) should be tested to ensure previews are populated and evicted correctly. [fixable]
| @@ -719,138 +707,61 @@ textarea:focus { | |||
| border-radius: var(--radius-md); | |||
There was a problem hiding this comment.
🔵 style: .feed-list has a hardcoded max-height: 380px. This may clip content awkwardly on short screens or waste space on tall ones. Consider using a viewport-relative unit or removing the constraint in favor of the page's natural scroll. [fixable]
1 participant
Summary
Changes
Backend:
event-store.ts:last_assistant_previewcolumn + migration + prepared statementquery-loop.ts: extract preview from snapshot blocks at turn-endsession-overview.ts: preview cache (avoids per-broadcast DB lookups), expose in activity outputapp.ts: include preview in REST/api/sessionsresponseFrontend:
useSessionFeed.ts(new): working-batch filter, chip filters, oldest-first sort, localStorage persistenceSessionFeed.tsx(new): filter chips + feed cards with status icon, repo tag, title, preview, metaSessionList.tsx: swap old components for unified<SessionFeed />global.css: remove old overview/attention CSS (~210 lines), add feed stylesRemoved:
SessionOverview.tsx,AttentionFeed.tsx,useAttentionFeed.ts+ their testsTest plan
useSessionFeedhook (working batch, filters, sort, persistence)🤖 Generated with Claude Code