refactor(team): narrow PR to progress payload cap only
Remove the SessionDetail.messages stripping and related cache-size change per maintainer feedback. The session-detail optimization will follow separately after PR #58 lands with the right architectural pattern (lightweight snapshot + separate endpoints). This PR now contains only: - progressPayload helpers (buildProgressLogsTail, buildProgressAssistantOutput) - cap applied to emitLogsProgress, updateProgress, stall warning, retry error - throttle raised 300ms -> 1000ms - tests for the progress payload behavior
This commit is contained in:
parent
ca5791b801
commit
fb474af2a7
5 changed files with 13 additions and 118 deletions
|
|
@ -13,7 +13,6 @@
|
|||
import { createLogger } from '@shared/utils/logger';
|
||||
|
||||
import { coercePageLimit, validateProjectId, validateSessionId } from '../ipc/guards';
|
||||
import { stripSessionDetailMessages } from '../services/analysis/sessionDetailPayload';
|
||||
import { DataCache } from '../services/infrastructure/DataCache';
|
||||
|
||||
import type { SessionsByIdsOptions, SessionsPaginationOptions } from '../types';
|
||||
|
|
@ -189,11 +188,11 @@ export function registerSessionRoutes(app: FastifyInstance, services: HttpServic
|
|||
);
|
||||
session.hasSubagents = subagents.length > 0;
|
||||
|
||||
// Build session detail with chunks. Strip the raw `messages` array before
|
||||
// caching/returning — the renderer only consumes chunks/processes/session,
|
||||
// and including messages doubled IPC payload size and cache footprint.
|
||||
sessionDetail = stripSessionDetailMessages(
|
||||
services.chunkBuilder.buildSessionDetail(session, parsedSession.messages, subagents)
|
||||
// Build session detail with chunks
|
||||
sessionDetail = services.chunkBuilder.buildSessionDetail(
|
||||
session,
|
||||
parsedSession.messages,
|
||||
subagents
|
||||
);
|
||||
|
||||
// Cache the result
|
||||
|
|
@ -319,10 +318,10 @@ export function registerSessionRoutes(app: FastifyInstance, services: HttpServic
|
|||
parsedSession.messages
|
||||
);
|
||||
|
||||
// Strip the raw `messages` array before caching so the waterfall path
|
||||
// cannot "poison" the cache entry reused by the session-detail route.
|
||||
detail = stripSessionDetailMessages(
|
||||
services.chunkBuilder.buildSessionDetail(session, parsedSession.messages, subagents)
|
||||
detail = services.chunkBuilder.buildSessionDetail(
|
||||
session,
|
||||
parsedSession.messages,
|
||||
subagents
|
||||
);
|
||||
services.dataCache.set(cacheKey, detail);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -13,7 +13,6 @@ import { createLogger } from '@shared/utils/logger';
|
|||
import { type IpcMain, type IpcMainInvokeEvent } from 'electron';
|
||||
|
||||
import { DataCache } from '../services';
|
||||
import { stripSessionDetailMessages } from '../services/analysis/sessionDetailPayload';
|
||||
import {
|
||||
type ConversationGroup,
|
||||
type PaginatedSessionsResult,
|
||||
|
|
@ -248,12 +247,8 @@ async function handleGetSessionDetail(
|
|||
);
|
||||
session.hasSubagents = subagents.length > 0;
|
||||
|
||||
// Build session detail with chunks. Strip the raw `messages` array before
|
||||
// caching/returning — the renderer only consumes chunks/processes/session,
|
||||
// and including messages doubled IPC payload size and cache footprint.
|
||||
sessionDetail = stripSessionDetailMessages(
|
||||
chunkBuilder.buildSessionDetail(session, parsedSession.messages, subagents)
|
||||
);
|
||||
// Build session detail with chunks
|
||||
sessionDetail = chunkBuilder.buildSessionDetail(session, parsedSession.messages, subagents);
|
||||
|
||||
// Cache the result
|
||||
dataCache.set(cacheKey, sessionDetail);
|
||||
|
|
|
|||
|
|
@ -1,24 +0,0 @@
|
|||
import type { SessionDetail } from '../../types';
|
||||
|
||||
/**
|
||||
* Strip the raw `messages` array from a `SessionDetail` before it crosses the
|
||||
* IPC / HTTP boundary to the renderer.
|
||||
*
|
||||
* The renderer consumes `session`, `chunks`, `processes`, and `metrics` only —
|
||||
* `messages` is an implementation detail that `ChunkBuilder` retains for
|
||||
* internal callers. Including it in the serialized payload roughly doubled
|
||||
* the IPC cost for sessions with large JSONL files (tens of MB per response)
|
||||
* while also inflating the in-memory `DataCache` footprint. The field is
|
||||
* preserved (as an empty array) so the shared `SessionDetail` type stays
|
||||
* satisfied and downstream code can still observe `.messages.length === 0`
|
||||
* without runtime type narrowing.
|
||||
*/
|
||||
export function stripSessionDetailMessages(detail: SessionDetail): SessionDetail {
|
||||
if (detail.messages.length === 0) {
|
||||
return detail;
|
||||
}
|
||||
return {
|
||||
...detail,
|
||||
messages: [],
|
||||
};
|
||||
}
|
||||
|
|
@ -2,15 +2,8 @@
|
|||
* Cache-related constants.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Maximum number of session details retained in the in-memory LRU cache.
|
||||
*
|
||||
* Users regularly juggle dozens of sessions — the previous cap of 5 caused
|
||||
* constant re-parsing of large JSONL files on every session switch. Raised
|
||||
* now that the per-entry footprint is bounded (the raw `messages` array is
|
||||
* stripped before caching; see `stripSessionDetailMessages`).
|
||||
*/
|
||||
export const MAX_CACHE_SESSIONS = 20;
|
||||
/** Maximum number of sessions to cache */
|
||||
export const MAX_CACHE_SESSIONS = 5;
|
||||
|
||||
/** Cache TTL in minutes */
|
||||
export const CACHE_TTL_MINUTES = 5;
|
||||
|
|
|
|||
|
|
@ -1,68 +0,0 @@
|
|||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import { stripSessionDetailMessages } from '../../../../src/main/services/analysis/sessionDetailPayload';
|
||||
import type { ParsedMessage, SessionDetail } from '../../../../src/main/types';
|
||||
|
||||
function createDetail(overrides: Partial<SessionDetail> = {}): SessionDetail {
|
||||
return {
|
||||
session: {
|
||||
id: 'session-1',
|
||||
projectId: 'project-1',
|
||||
projectPath: '/tmp/project',
|
||||
isOngoing: false,
|
||||
hasSubagents: false,
|
||||
messageCount: 0,
|
||||
createdAt: 0,
|
||||
},
|
||||
messages: [],
|
||||
chunks: [],
|
||||
processes: [],
|
||||
metrics: {
|
||||
durationMs: 0,
|
||||
totalTokens: 0,
|
||||
inputTokens: 0,
|
||||
outputTokens: 0,
|
||||
cacheReadTokens: 0,
|
||||
cacheCreationTokens: 0,
|
||||
messageCount: 0,
|
||||
},
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
describe('stripSessionDetailMessages', () => {
|
||||
it('returns the same reference when messages is already empty', () => {
|
||||
const detail = createDetail();
|
||||
const result = stripSessionDetailMessages(detail);
|
||||
expect(result).toBe(detail);
|
||||
});
|
||||
|
||||
it('drops the messages array when it is non-empty', () => {
|
||||
const messages = [{ uuid: 'm-1' } as unknown as ParsedMessage];
|
||||
const detail = createDetail({ messages });
|
||||
const result = stripSessionDetailMessages(detail);
|
||||
expect(result).not.toBe(detail);
|
||||
expect(result.messages).toEqual([]);
|
||||
});
|
||||
|
||||
it('preserves every other field (session, chunks, processes, metrics)', () => {
|
||||
const messages = Array.from(
|
||||
{ length: 3 },
|
||||
(_, i) => ({ uuid: `m-${i}` }) as unknown as ParsedMessage
|
||||
);
|
||||
const detail = createDetail({ messages });
|
||||
const result = stripSessionDetailMessages(detail);
|
||||
expect(result.session).toBe(detail.session);
|
||||
expect(result.chunks).toBe(detail.chunks);
|
||||
expect(result.processes).toBe(detail.processes);
|
||||
expect(result.metrics).toBe(detail.metrics);
|
||||
});
|
||||
|
||||
it('does not mutate the input detail', () => {
|
||||
const messages = [{ uuid: 'm-1' } as unknown as ParsedMessage];
|
||||
const detail = createDetail({ messages });
|
||||
stripSessionDetailMessages(detail);
|
||||
expect(detail.messages).toBe(messages);
|
||||
expect(detail.messages).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
Loading…
Reference in a new issue