diff --git a/src/renderer/store/slices/sessionDetailSlice.ts b/src/renderer/store/slices/sessionDetailSlice.ts index df6b1727..04cb8980 100644 --- a/src/renderer/store/slices/sessionDetailSlice.ts +++ b/src/renderer/store/slices/sessionDetailSlice.ts @@ -53,9 +53,9 @@ import type { MentionedFileInfo, } from '@renderer/types/contextInjection'; import type { ClaudeMdFileInfo, SessionDetail } from '@renderer/types/data'; -import type { AIGroup, SessionConversation } from '@renderer/types/groups'; +import type { AIGroup, ChatItem, SessionConversation } from '@renderer/types/groups'; import type { AgentConfig } from '@shared/types/api'; -import type { StateCreator } from 'zustand'; +import type { StateCreator, StoreApi } from 'zustand'; // ============================================================================= // Per-tab session data type @@ -89,6 +89,230 @@ function createEmptyTabSessionData(): TabSessionData { }; } +// ============================================================================= +// Phase 2: Deferred context tracking (shared by fetch and refresh paths) +// ============================================================================= + +/** + * Phase 2 of session loading: fire-and-forget computation of per-group context + * stats, CLAUDE.md stats and compaction phase info for the given conversation + * items. Reads CLAUDE.md / mentioned-file token data over IPC (batched), then + * runs `processSessionContextWithPhases`. + * + * Shared by `fetchSessionDetail` (full load) and — since issue #54 — + * `refreshSessionInPlace`: without the recompute the Context pill and per-turn + * cost stay frozen at fetch-time values for the rest of a long session. + * + * Runs are bound to the conversation they compute over by reference identity: + * before every store write the run checks the live conversation still IS the + * items array it was given, and drops its result otherwise. A superseded run + * (newer fetch, newer refresh, another session displayed) therefore never + * writes stale stats — while a no-op refresh that commits nothing does not + * invalidate the pending run. + */ +function runContextPhase2( + set: StoreApi['setState'], + get: StoreApi['getState'], + params: { + items: ChatItem[]; + projectRoot: string; + tabIds: string[]; + /** Optional compute-saving abort for superseded fetches (not writes). */ + isStale?: () => boolean; + } +): void { + const { items, projectRoot, tabIds } = params; + const isStale = () => params.isStale?.() ?? false; + + // CLAUDE.md / mentioned-file reads must hit the local filesystem. + if (get().connectionMode === 'ssh') return; + + void (async () => { + try { + // Fetch real CLAUDE.md token data + let claudeMdTokenData: Record = {}; + try { + claudeMdTokenData = await api.readClaudeMdFiles(projectRoot); + if (isStale()) return; + } catch (err) { + logger.error('Failed to read CLAUDE.md files:', err); + } + + const claudeMdStats = processSessionClaudeMd(items, projectRoot, claudeMdTokenData); + + // Fetch real tokens for directory CLAUDE.md files + const directoryTokenData: Record = {}; + + if (claudeMdStats && claudeMdStats.size > 0) { + const directoryPaths = new Set(); + for (const stats of claudeMdStats.values()) { + for (const injection of stats.accumulatedInjections) { + if (injection.source === 'directory') { + directoryPaths.add(injection.path); + } + } + } + + if (directoryPaths.size > 0) { + const directoryTokens = new Map(); + const nonExistentPaths = new Set(); + + const directoryResults = await batchAsync( + Array.from(directoryPaths), + async (fullPath) => { + try { + const dirPath = fullPath.replace(/[\\/]CLAUDE\.md$/, ''); + const fileInfo = await api.readDirectoryClaudeMd(dirPath); + return { fullPath, fileInfo, error: false }; + } catch (err) { + logger.error('Failed to read directory CLAUDE.md:', fullPath, err); + return { fullPath, fileInfo: null, error: true }; + } + }, + 5 + ); + if (isStale()) return; + + for (const { fullPath, fileInfo, error } of directoryResults) { + if (error || !fileInfo) { + nonExistentPaths.add(fullPath); + } else if (fileInfo.exists && fileInfo.estimatedTokens > 0) { + directoryTokens.set(fullPath, fileInfo.estimatedTokens); + directoryTokenData[fullPath] = fileInfo; + } else { + nonExistentPaths.add(fullPath); + } + } + + // Update stats: set real tokens and REMOVE non-existent files + for (const [, stats] of claudeMdStats.entries()) { + stats.accumulatedInjections = stats.accumulatedInjections.filter( + (inj) => inj.source !== 'directory' || !nonExistentPaths.has(inj.path) + ); + stats.newInjections = stats.newInjections.filter( + (inj) => inj.source !== 'directory' || !nonExistentPaths.has(inj.path) + ); + + for (const injection of stats.accumulatedInjections) { + if (injection.source === 'directory' && directoryTokens.has(injection.path)) { + injection.estimatedTokens = directoryTokens.get(injection.path)!; + } + } + for (const injection of stats.newInjections) { + if (injection.source === 'directory' && directoryTokens.has(injection.path)) { + injection.estimatedTokens = directoryTokens.get(injection.path)!; + } + } + + stats.totalEstimatedTokens = stats.accumulatedInjections.reduce( + (sum, inj) => sum + inj.estimatedTokens, + 0 + ); + stats.accumulatedCount = stats.accumulatedInjections.length; + stats.newCount = stats.newInjections.length; + } + } + } + + // Extract all mentioned file paths from user groups + const mentionedFilePaths = new Set(); + for (const item of items) { + if (item.type === 'user' && item.group.content.fileReferences) { + for (const ref of item.group.content.fileReferences) { + const absolutePath = resolveFilePath(projectRoot, ref.path); + mentionedFilePaths.add(absolutePath); + } + } + } + + // Also collect @-mentions from isMeta:true user messages in AI responses + for (const item of items) { + if (item.type === 'ai') { + for (const msg of item.group.responses) { + if (msg.type !== 'user') continue; + let text = ''; + if (typeof msg.content === 'string') { + text = msg.content; + } else if (Array.isArray(msg.content)) { + for (const block of msg.content) { + if (block.type === 'text' && block.text) text += block.text; + } + } + if (text) { + for (const ref of extractFileReferences(text)) { + const absolutePath = resolveFilePath(projectRoot, ref.path); + mentionedFilePaths.add(absolutePath); + } + } + } + } + } + + // Fetch token data for each mentioned file (throttled IPC calls) + const mentionedFileTokenData = new Map(); + const mentionedFileResults = await batchAsync( + Array.from(mentionedFilePaths), + async (filePath) => { + try { + const fileInfo = await api.readMentionedFile(filePath, projectRoot); + return { filePath, fileInfo }; + } catch (err) { + logger.error('Failed to read mentioned file:', filePath, err); + return { filePath, fileInfo: null }; + } + }, + 5 + ); + if (isStale()) return; + + for (const { filePath, fileInfo } of mentionedFileResults) { + if (fileInfo) { + mentionedFileTokenData.set(filePath, fileInfo); + } + } + + // Process Visible Context with all token data + const phaseResult = processSessionContextWithPhases( + items, + projectRoot, + claudeMdTokenData, + mentionedFileTokenData, + directoryTokenData + ); + + // Write-time identity guards: a run whose conversation was replaced by + // a newer commit (fetch, refresh, another session) drops its result. + if (get().conversation?.items !== items) return; + set({ + sessionClaudeMdStats: claudeMdStats, + sessionContextStats: phaseResult.statsMap, + sessionPhaseInfo: phaseResult.phaseInfo, + }); + + // Per-tab stats: only tabs still showing this exact conversation. + const prev = get().tabSessionData; + const nextTabSessionData = { ...prev }; + let tabsChanged = false; + for (const tabId of tabIds) { + const tabData = prev[tabId]; + if (!tabData || tabData.conversation?.items !== items) continue; + nextTabSessionData[tabId] = { + ...tabData, + sessionClaudeMdStats: claudeMdStats, + sessionContextStats: phaseResult.statsMap, + sessionPhaseInfo: phaseResult.phaseInfo, + }; + tabsChanged = true; + } + if (tabsChanged) { + set({ tabSessionData: nextTabSessionData }); + } + } catch (err) { + logger.error('Phase 2 context tracking error:', err); + } + })(); +} + // ============================================================================= // Slice Interface // ============================================================================= @@ -319,198 +543,17 @@ export const createSessionDetailSlice: StateCreator { - try { - // Fetch real CLAUDE.md token data - let claudeMdTokenData: Record = {}; - try { - claudeMdTokenData = await api.readClaudeMdFiles(projectRoot); - if (requestGeneration !== sessionDetailFetchGeneration) return; - } catch (err) { - logger.error('Failed to read CLAUDE.md files:', err); - } - - const claudeMdStats = processSessionClaudeMd( - conversation.items, - projectRoot, - claudeMdTokenData - ); - - // Fetch real tokens for directory CLAUDE.md files - const directoryTokenData: Record = {}; - - if (claudeMdStats && claudeMdStats.size > 0) { - const directoryPaths = new Set(); - for (const stats of claudeMdStats.values()) { - for (const injection of stats.accumulatedInjections) { - if (injection.source === 'directory') { - directoryPaths.add(injection.path); - } - } - } - - if (directoryPaths.size > 0) { - const directoryTokens = new Map(); - const nonExistentPaths = new Set(); - - const directoryResults = await batchAsync( - Array.from(directoryPaths), - async (fullPath) => { - try { - const dirPath = fullPath.replace(/[\\/]CLAUDE\.md$/, ''); - const fileInfo = await api.readDirectoryClaudeMd(dirPath); - return { fullPath, fileInfo, error: false }; - } catch (err) { - logger.error('Failed to read directory CLAUDE.md:', fullPath, err); - return { fullPath, fileInfo: null, error: true }; - } - }, - 5 - ); - if (requestGeneration !== sessionDetailFetchGeneration) return; - - for (const { fullPath, fileInfo, error } of directoryResults) { - if (error || !fileInfo) { - nonExistentPaths.add(fullPath); - } else if (fileInfo.exists && fileInfo.estimatedTokens > 0) { - directoryTokens.set(fullPath, fileInfo.estimatedTokens); - directoryTokenData[fullPath] = fileInfo; - } else { - nonExistentPaths.add(fullPath); - } - } - - // Update stats: set real tokens and REMOVE non-existent files - for (const [, stats] of claudeMdStats.entries()) { - stats.accumulatedInjections = stats.accumulatedInjections.filter( - (inj) => inj.source !== 'directory' || !nonExistentPaths.has(inj.path) - ); - stats.newInjections = stats.newInjections.filter( - (inj) => inj.source !== 'directory' || !nonExistentPaths.has(inj.path) - ); - - for (const injection of stats.accumulatedInjections) { - if (injection.source === 'directory' && directoryTokens.has(injection.path)) { - injection.estimatedTokens = directoryTokens.get(injection.path)!; - } - } - for (const injection of stats.newInjections) { - if (injection.source === 'directory' && directoryTokens.has(injection.path)) { - injection.estimatedTokens = directoryTokens.get(injection.path)!; - } - } - - stats.totalEstimatedTokens = stats.accumulatedInjections.reduce( - (sum, inj) => sum + inj.estimatedTokens, - 0 - ); - stats.accumulatedCount = stats.accumulatedInjections.length; - stats.newCount = stats.newInjections.length; - } - } - } - - // Extract all mentioned file paths from user groups - const mentionedFilePaths = new Set(); - for (const item of conversation.items) { - if (item.type === 'user' && item.group.content.fileReferences) { - for (const ref of item.group.content.fileReferences) { - const absolutePath = resolveFilePath(projectRoot, ref.path); - mentionedFilePaths.add(absolutePath); - } - } - } - - // Also collect @-mentions from isMeta:true user messages in AI responses - for (const item of conversation.items) { - if (item.type === 'ai') { - for (const msg of item.group.responses) { - if (msg.type !== 'user') continue; - let text = ''; - if (typeof msg.content === 'string') { - text = msg.content; - } else if (Array.isArray(msg.content)) { - for (const block of msg.content) { - if (block.type === 'text' && block.text) text += block.text; - } - } - if (text) { - for (const ref of extractFileReferences(text)) { - const absolutePath = resolveFilePath(projectRoot, ref.path); - mentionedFilePaths.add(absolutePath); - } - } - } - } - } - - // Fetch token data for each mentioned file (throttled IPC calls) - const mentionedFileTokenData = new Map(); - const mentionedFileResults = await batchAsync( - Array.from(mentionedFilePaths), - async (filePath) => { - try { - const fileInfo = await api.readMentionedFile(filePath, projectRoot); - return { filePath, fileInfo }; - } catch (err) { - logger.error('Failed to read mentioned file:', filePath, err); - return { filePath, fileInfo: null }; - } - }, - 5 - ); - if (requestGeneration !== sessionDetailFetchGeneration) return; - - for (const { filePath, fileInfo } of mentionedFileResults) { - if (fileInfo) { - mentionedFileTokenData.set(filePath, fileInfo); - } - } - - // Process Visible Context with all token data - const phaseResult = processSessionContextWithPhases( - conversation.items, - projectRoot, - claudeMdTokenData, - mentionedFileTokenData, - directoryTokenData - ); - - // Phase 2 set: update only the context stats - if (requestGeneration !== sessionDetailFetchGeneration) return; - set({ - sessionClaudeMdStats: claudeMdStats, - sessionContextStats: phaseResult.statsMap, - sessionPhaseInfo: phaseResult.phaseInfo, - }); - - // Update per-tab stats - if (tabId) { - const prev = get().tabSessionData; - const tabData = prev[tabId]; - if (tabData) { - set({ - tabSessionData: { - ...prev, - [tabId]: { - ...tabData, - sessionClaudeMdStats: claudeMdStats, - sessionContextStats: phaseResult.statsMap, - sessionPhaseInfo: phaseResult.phaseInfo, - }, - }, - }); - } - } - } catch (err) { - logger.error('Phase 2 context tracking error:', err); - } - })(); + if (conversation?.items) { + runContextPhase2(set, get, { + items: conversation.items, + projectRoot, + tabIds: tabId ? [tabId] : [], + isStale: () => requestGeneration !== sessionDetailFetchGeneration, + }); } } catch (error) { logger.error('fetchSessionDetail error:', error); @@ -720,8 +763,10 @@ export const createSessionDetailSlice: StateCreator; readDirectoryClaudeMd: ReturnType; readMentionedFile: ReturnType; + readAgentConfigs: ReturnType; validateMentions: ReturnType; openPath: ReturnType; openExternal: ReturnType; @@ -108,6 +109,7 @@ export function createMockElectronAPI(): MockElectronAPI { estimatedTokens: 0, }), readMentionedFile: vi.fn().mockResolvedValue(null), + readAgentConfigs: vi.fn().mockResolvedValue({}), validateMentions: vi.fn().mockResolvedValue({}), openPath: vi.fn().mockResolvedValue({ success: true }), openExternal: vi.fn().mockResolvedValue({ success: true }), diff --git a/test/renderer/store/sessionDetailSlice.test.ts b/test/renderer/store/sessionDetailSlice.test.ts new file mode 100644 index 00000000..6c29f821 --- /dev/null +++ b/test/renderer/store/sessionDetailSlice.test.ts @@ -0,0 +1,266 @@ +/** + * Issue #54: refreshSessionInPlace must recompute Phase-2 context stats + * (sessionContextStats / sessionPhaseInfo), not just swap the conversation. + * Without it the Context pill stays frozen at fetch-time values for the + * whole rest of a long session. + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { installMockElectronAPI, type MockElectronAPI } from '../../mocks/electronAPI'; + +import { createTestStore, type TestStore } from './storeTestUtils'; + +import type { SessionDetail } from '@renderer/types/data'; + +// --- chunk fixtures ------------------------------------------------------- +// Shapes follow EnhancedUserChunk / EnhancedAIChunk (src/main/types/chunks.ts). +// group.id === chunk.id (groupTransformer.createAIGroupFromChunk), and chunk ids +// are the keys of sessionContextStats / aiGroupPhaseMap. + +function makeMetrics(messageCount: number) { + return { + durationMs: 60_000, + totalTokens: 5_000, + inputTokens: 3_000, + outputTokens: 2_000, + cacheReadTokens: 500, + cacheCreationTokens: 100, + messageCount, + costUsd: 0.05, + }; +} + +function userChunk(id: string, uuid: string) { + return { + id, + chunkType: 'user' as const, + startTime: new Date('2026-01-01T00:00:00Z'), + endTime: new Date('2026-01-01T00:00:01Z'), + durationMs: 1_000, + metrics: makeMetrics(1), + userMessage: { + uuid, + parentUuid: null, + type: 'user' as const, + timestamp: new Date('2026-01-01T00:00:00Z'), + content: `message ${uuid}`, + isMeta: false, + isSidechain: false, + toolCalls: [], + toolResults: [], + }, + rawMessages: [], + }; +} + +function aiChunk(id: string) { + return { + id, + chunkType: 'ai' as const, + startTime: new Date('2026-01-01T00:00:01Z'), + endTime: new Date('2026-01-01T00:00:05Z'), + durationMs: 4_000, + metrics: makeMetrics(2), + responses: [ + { + uuid: `resp-${id}`, + parentUuid: null, + type: 'assistant' as const, + timestamp: new Date('2026-01-01T00:00:01Z'), + content: [{ type: 'text', text: `answer ${id}` }], + isMeta: false, + isSidechain: false, + toolCalls: [], + toolResults: [], + model: 'glm-5.3', + usage: { + input_tokens: 3_000, + output_tokens: 2_000, + cache_read_input_tokens: 500, + cache_creation_input_tokens: 100, + }, + }, + ], + processes: [], + sidechainMessages: [], + toolExecutions: [], + semanticSteps: [], + rawMessages: [], + }; +} + +function detail(sessionId: string, chunks: unknown[]): SessionDetail { + return { + session: { + id: sessionId, + projectId: 'project-1', + // unique per session: sessionDetailSlice caches agent-config fetches by + // projectPath at module level, shared across tests in this file + projectPath: `/tmp/proj-54-${sessionId}`, + createdAt: 0, + hasSubagents: false, + messageCount: chunks.length, + isOngoing: false, + name: 'S', + firstMessage: 'hi', + }, + messages: [], + chunks, + processes: [], + metrics: makeMetrics(chunks.length), + } as unknown as SessionDetail; +} + +const TURN1 = [userChunk('c-u1', 'u1'), aiChunk('c-ai-1')]; +const TURN2 = [...TURN1, userChunk('c-u2', 'u2'), aiChunk('c-ai-2')]; + +describe('sessionDetailSlice — Phase 2 on refresh (issue #54)', () => { + let store: TestStore; + let mockAPI: MockElectronAPI; + + beforeEach(() => { + mockAPI = installMockElectronAPI(); + store = createTestStore(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + /** fetchSessionDetail's stillViewingSession guard requires selectedSessionId. */ + function seedAndFetch(sessionId: string, chunks: unknown[]) { + store.setState({ selectedSessionId: sessionId }); + mockAPI.getSessionDetail.mockResolvedValueOnce(detail(sessionId, chunks)); + return store.getState().fetchSessionDetail('project-1', sessionId); + } + + it('fetchSessionDetail computes context stats (Phase 2 baseline)', async () => { + await seedAndFetch('s-base', TURN1); + await vi.waitFor(() => { + expect(store.getState().sessionContextStats?.has('c-ai-1')).toBe(true); + }); + }); + + it('refreshSessionInPlace recomputes stats for appended AI groups', async () => { + await seedAndFetch('s-refresh', TURN1); + await vi.waitFor(() => { + expect(store.getState().sessionContextStats?.has('c-ai-1')).toBe(true); + }); + + // "user appended a turn to the JSONL" — next IPC returns the longer detail + mockAPI.getSessionDetail.mockResolvedValue(detail('s-refresh', TURN2)); + await store.getState().refreshSessionInPlace('project-1', 's-refresh'); + + await vi.waitFor(() => { + // RED pre-fix: stats stay frozen at fetch-time values (only c-ai-1) + expect(store.getState().sessionContextStats?.has('c-ai-2')).toBe(true); + expect(store.getState().sessionPhaseInfo?.aiGroupPhaseMap.has('c-ai-2')).toBe(true); + }); + }); + + it('refreshSessionInPlace updates per-tab stats too', async () => { + store.getState().openTab({ + type: 'session', + sessionId: 's-tab', + projectId: 'project-1', + label: 'S', + }); + const tabId = store.getState().activeTabId; + expect(tabId).toBeTruthy(); + mockAPI.getSessionDetail.mockResolvedValueOnce(detail('s-tab', TURN1)); + await store.getState().fetchSessionDetail('project-1', 's-tab', tabId ?? undefined); + await vi.waitFor(() => { + expect(store.getState().tabSessionData[tabId ?? '']?.sessionContextStats?.has('c-ai-1')).toBe( + true + ); + }); + + mockAPI.getSessionDetail.mockResolvedValue(detail('s-tab', TURN2)); + await store.getState().refreshSessionInPlace('project-1', 's-tab'); + + await vi.waitFor(() => { + // RED pre-fix: per-tab stats frozen as well + expect(store.getState().tabSessionData[tabId ?? '']?.sessionContextStats?.has('c-ai-2')).toBe( + true + ); + }); + }); + + it('a no-op refresh does not abandon the in-flight Phase-2 of the last commit', async () => { + await seedAndFetch('s-noop', TURN1); + await vi.waitFor(() => { + expect(store.getState().sessionContextStats?.has('c-ai-1')).toBe(true); + }); + + // R1 commits TURN2; its Phase-2 stalls on readClaudeMdFiles. + let resolveClaudeMd: (v: object) => void = () => {}; + mockAPI.getSessionDetail.mockResolvedValueOnce(detail('s-noop', TURN2)); + mockAPI.readClaudeMdFiles.mockImplementationOnce( + () => + new Promise((resolve) => { + resolveClaudeMd = resolve; + }) + ); + void store.getState().refreshSessionInPlace('project-1', 's-noop'); + await vi.waitFor(() => { + expect( + store.getState().conversation?.items.some((i) => i.type === 'ai' && i.group.id === 'c-ai-2') + ).toBe(true); + }); + + // R2: a duplicate watcher event — main answers `unchanged`, a pure no-op + // that commits nothing and fires no Phase-2 of its own. + mockAPI.getSessionDetail.mockResolvedValueOnce({ unchanged: true } as unknown as SessionDetail); + await store.getState().refreshSessionInPlace('project-1', 's-noop'); + + // R1's stalled IPC resolves — its Phase-2 must still commit: the + // conversation it computed over is still the current one. RED pre-fix: + // R2's generation bump invalidates R1's run, stats stay at TURN1 forever. + resolveClaudeMd({}); + await vi.waitFor(() => { + expect(store.getState().sessionContextStats?.has('c-ai-2')).toBe(true); + }); + }); + + it('a newer Phase-2 run wins over a still-in-flight older one', async () => { + await seedAndFetch('s-race', TURN1); + await vi.waitFor(() => { + expect(store.getState().sessionContextStats?.has('c-ai-1')).toBe(true); + }); + + // Refresh A of s-race: its Phase-2 stalls in readClaudeMdFiles. + let resolveClaudeMdA: (v: object) => void = () => {}; + mockAPI.getSessionDetail.mockResolvedValueOnce(detail('s-race', TURN2)); + mockAPI.readClaudeMdFiles.mockImplementationOnce( + () => + new Promise((resolve) => { + resolveClaudeMdA = resolve; + }) + ); + void store.getState().refreshSessionInPlace('project-1', 's-race'); + await vi.waitFor(() => { + expect( + store.getState().conversation?.items.some((i) => i.type === 'ai' && i.group.id === 'c-ai-2') + ).toBe(true); + }); + + // A full fetch of ANOTHER session replaces the global conversation — + // A's write-time identity check must drop its result: A computed over + // s-race TURN2, so if A wrote, c-ai-2 would leak into the other + // session's stats map. + store.setState({ selectedSessionId: 's-other' }); + mockAPI.getSessionDetail.mockResolvedValueOnce(detail('s-other', TURN1)); + await store.getState().fetchSessionDetail('project-1', 's-other'); + await vi.waitFor(() => { + expect(store.getState().sessionContextStats?.has('c-ai-1')).toBe(true); + }); + + // A wakes after all that — it must drop its result: A computed over + // s-race TURN2, so if A wrote, c-ai-2 would leak into the other + // session's stats map. + resolveClaudeMdA({}); + await new Promise((r) => setTimeout(r, 50)); + expect(store.getState().sessionContextStats?.has('c-ai-2')).toBe(false); + }); +});