feat: implement decision persistence for review process

- Added ReviewDecisionStore to manage loading, saving, and clearing review decisions from disk.
- Introduced new IPC channels for loading, saving, and clearing decisions, enhancing the review process.
- Updated ChangeReviewDialog and related components to integrate decision persistence, improving user experience during reviews.
- Implemented debounced saving of decisions to optimize performance and reduce unnecessary writes.
This commit is contained in:
iliya 2026-02-25 19:53:14 +02:00
parent a5da786343
commit a02f46c3aa
8 changed files with 342 additions and 6 deletions

View file

@ -4,17 +4,21 @@
* Паттерн: module-level state + guard + wrapReviewHandler (как teams.ts)
*/
import { ReviewDecisionStore } from '@main/services/team/ReviewDecisionStore';
import {
REVIEW_APPLY_DECISIONS,
REVIEW_CHECK_CONFLICT,
REVIEW_CLEAR_DECISIONS,
REVIEW_GET_AGENT_CHANGES,
REVIEW_GET_CHANGE_STATS,
REVIEW_GET_FILE_CONTENT,
REVIEW_GET_GIT_FILE_LOG,
REVIEW_GET_TASK_CHANGES,
REVIEW_LOAD_DECISIONS,
REVIEW_PREVIEW_REJECT,
REVIEW_REJECT_FILE,
REVIEW_REJECT_HUNKS,
REVIEW_SAVE_DECISIONS,
REVIEW_SAVE_EDITED_FILE,
// eslint-disable-next-line boundaries/element-types -- IPC channel constants are shared between main and preload by design
} from '@preload/constants/ipcChannels';
@ -32,6 +36,7 @@ import type {
ChangeStats,
ConflictCheckResult,
FileChangeWithContent,
HunkDecision,
RejectResult,
SnippetDiff,
TaskChangeSetV2,
@ -46,6 +51,7 @@ let changeExtractor: ChangeExtractorService | null = null;
let reviewApplier: ReviewApplierService | null = null;
let fileContentResolver: FileContentResolver | null = null;
let gitDiffFallback: GitDiffFallback | null = null;
const reviewDecisionStore = new ReviewDecisionStore();
function getChangeExtractor(): ChangeExtractorService {
if (!changeExtractor) throw new Error('Review handlers not initialized');
@ -94,6 +100,10 @@ export function registerReviewHandlers(ipcMain: IpcMain): void {
ipcMain.handle(REVIEW_SAVE_EDITED_FILE, handleSaveEditedFile);
// Phase 4
ipcMain.handle(REVIEW_GET_GIT_FILE_LOG, handleGetGitFileLog);
// Decision persistence
ipcMain.handle(REVIEW_LOAD_DECISIONS, handleLoadDecisions);
ipcMain.handle(REVIEW_SAVE_DECISIONS, handleSaveDecisions);
ipcMain.handle(REVIEW_CLEAR_DECISIONS, handleClearDecisions);
}
export function removeReviewHandlers(ipcMain: IpcMain): void {
@ -112,6 +122,10 @@ export function removeReviewHandlers(ipcMain: IpcMain): void {
ipcMain.removeHandler(REVIEW_SAVE_EDITED_FILE);
// Phase 4
ipcMain.removeHandler(REVIEW_GET_GIT_FILE_LOG);
// Decision persistence
ipcMain.removeHandler(REVIEW_LOAD_DECISIONS);
ipcMain.removeHandler(REVIEW_SAVE_DECISIONS);
ipcMain.removeHandler(REVIEW_CLEAR_DECISIONS);
}
// --- Локальный wrapReviewHandler ---
@ -261,3 +275,38 @@ async function handleGetGitFileLog(
return gitDiffFallback.getFileLog(projectPath, filePath);
});
}
// --- Decision Persistence Handlers ---
async function handleLoadDecisions(
_event: IpcMainInvokeEvent,
teamName: string,
scopeKey: string
): Promise<
IpcResult<{
hunkDecisions: Record<string, HunkDecision>;
fileDecisions: Record<string, HunkDecision>;
} | null>
> {
return wrapReviewHandler('loadDecisions', () => reviewDecisionStore.load(teamName, scopeKey));
}
async function handleSaveDecisions(
_event: IpcMainInvokeEvent,
teamName: string,
scopeKey: string,
hunkDecisions: Record<string, HunkDecision>,
fileDecisions: Record<string, HunkDecision>
): Promise<IpcResult<void>> {
return wrapReviewHandler('saveDecisions', () =>
reviewDecisionStore.save(teamName, scopeKey, { hunkDecisions, fileDecisions })
);
}
async function handleClearDecisions(
_event: IpcMainInvokeEvent,
teamName: string,
scopeKey: string
): Promise<IpcResult<void>> {
return wrapReviewHandler('clearDecisions', () => reviewDecisionStore.clear(teamName, scopeKey));
}

View file

@ -0,0 +1,103 @@
import { getTeamsBasePath } from '@main/utils/pathDecoder';
import { createLogger } from '@shared/utils/logger';
import * as fs from 'fs';
import * as path from 'path';
import { atomicWriteAsync } from './atomicWrite';
import type { HunkDecision } from '@shared/types';
const logger = createLogger('ReviewDecisionStore');
export interface ReviewDecisionsData {
hunkDecisions: Record<string, HunkDecision>;
fileDecisions: Record<string, HunkDecision>;
updatedAt: string;
}
export class ReviewDecisionStore {
private getDirPath(teamName: string): string {
return path.join(getTeamsBasePath(), teamName, 'review-decisions');
}
private getFilePath(teamName: string, scopeKey: string): string {
return path.join(this.getDirPath(teamName), `${scopeKey}.json`);
}
async load(
teamName: string,
scopeKey: string
): Promise<{
hunkDecisions: Record<string, HunkDecision>;
fileDecisions: Record<string, HunkDecision>;
} | null> {
const filePath = this.getFilePath(teamName, scopeKey);
let raw: string;
try {
raw = await fs.promises.readFile(filePath, 'utf8');
} catch (error) {
if ((error as NodeJS.ErrnoException).code === 'ENOENT') {
return null;
}
logger.error(`Failed to read review decisions for ${teamName}/${scopeKey}: ${String(error)}`);
return null;
}
let parsed: unknown;
try {
parsed = JSON.parse(raw) as unknown;
} catch {
logger.error(`Corrupted review decisions file for ${teamName}/${scopeKey}`);
return null;
}
if (!parsed || typeof parsed !== 'object') {
return null;
}
const data = parsed as Partial<ReviewDecisionsData>;
const hunkDecisions: Record<string, HunkDecision> =
data.hunkDecisions && typeof data.hunkDecisions === 'object' ? data.hunkDecisions : {};
const fileDecisions: Record<string, HunkDecision> =
data.fileDecisions && typeof data.fileDecisions === 'object' ? data.fileDecisions : {};
return { hunkDecisions, fileDecisions };
}
async save(
teamName: string,
scopeKey: string,
data: {
hunkDecisions: Record<string, HunkDecision>;
fileDecisions: Record<string, HunkDecision>;
}
): Promise<void> {
try {
const payload: ReviewDecisionsData = {
hunkDecisions: data.hunkDecisions,
fileDecisions: data.fileDecisions,
updatedAt: new Date().toISOString(),
};
await atomicWriteAsync(
this.getFilePath(teamName, scopeKey),
JSON.stringify(payload, null, 2)
);
} catch (error) {
logger.error(`Failed to save review decisions for ${teamName}/${scopeKey}: ${String(error)}`);
}
}
async clear(teamName: string, scopeKey: string): Promise<void> {
try {
await fs.promises.unlink(this.getFilePath(teamName, scopeKey));
} catch (error) {
if ((error as NodeJS.ErrnoException).code !== 'ENOENT') {
logger.error(
`Failed to clear review decisions for ${teamName}/${scopeKey}: ${String(error)}`
);
}
}
}
}

View file

@ -337,3 +337,12 @@ export const REVIEW_SAVE_EDITED_FILE = 'review:saveEditedFile';
/** Get git file change log */
export const REVIEW_GET_GIT_FILE_LOG = 'review:getGitFileLog';
/** Load persisted review decisions from disk */
export const REVIEW_LOAD_DECISIONS = 'review:loadDecisions';
/** Save review decisions to disk */
export const REVIEW_SAVE_DECISIONS = 'review:saveDecisions';
/** Clear review decisions from disk */
export const REVIEW_CLEAR_DECISIONS = 'review:clearDecisions';

View file

@ -12,14 +12,17 @@ import {
HTTP_SERVER_STOP,
REVIEW_APPLY_DECISIONS,
REVIEW_CHECK_CONFLICT,
REVIEW_CLEAR_DECISIONS,
REVIEW_GET_AGENT_CHANGES,
REVIEW_GET_CHANGE_STATS,
REVIEW_GET_FILE_CONTENT,
REVIEW_GET_GIT_FILE_LOG,
REVIEW_GET_TASK_CHANGES,
REVIEW_LOAD_DECISIONS,
REVIEW_PREVIEW_REJECT,
REVIEW_REJECT_FILE,
REVIEW_REJECT_HUNKS,
REVIEW_SAVE_DECISIONS,
REVIEW_SAVE_EDITED_FILE,
SSH_CONNECT,
SSH_DISCONNECT,
@ -123,6 +126,7 @@ import type {
FileChangeWithContent,
GlobalTask,
HttpServerStatus,
HunkDecision,
IpcResult,
KanbanColumnId,
MemberFullStats,
@ -777,6 +781,30 @@ const electronAPI: ElectronAPI = {
saveEditedFile: async (filePath: string, content: string) => {
return invokeIpcWithResult<{ success: boolean }>(REVIEW_SAVE_EDITED_FILE, filePath, content);
},
// Decision persistence
loadDecisions: async (teamName: string, scopeKey: string) => {
return invokeIpcWithResult<{
hunkDecisions: Record<string, HunkDecision>;
fileDecisions: Record<string, HunkDecision>;
} | null>(REVIEW_LOAD_DECISIONS, teamName, scopeKey);
},
saveDecisions: async (
teamName: string,
scopeKey: string,
hunkDecisions: Record<string, HunkDecision>,
fileDecisions: Record<string, HunkDecision>
) => {
return invokeIpcWithResult<void>(
REVIEW_SAVE_DECISIONS,
teamName,
scopeKey,
hunkDecisions,
fileDecisions
);
},
clearDecisions: async (teamName: string, scopeKey: string) => {
return invokeIpcWithResult<void>(REVIEW_CLEAR_DECISIONS, teamName, scopeKey);
},
onCmdN: (callback: () => void): (() => void) => {
const handler = (): void => callback();
ipcRenderer.on('review:cmdN', handler);

View file

@ -823,6 +823,16 @@ export class HttpAPIClient implements ElectronAPI {
saveEditedFile: async (): Promise<never> => {
throw new Error('Review is not available in browser mode');
},
// Decision persistence stubs
loadDecisions: async (): Promise<never> => {
throw new Error('Review is not available in browser mode');
},
saveDecisions: async (): Promise<never> => {
throw new Error('Review is not available in browser mode');
},
clearDecisions: async (): Promise<never> => {
throw new Error('Review is not available in browser mode');
},
// Phase 4 stubs
getGitFileLog: async (): Promise<never> => {
throw new Error('Review is not available in browser mode');

View file

@ -51,7 +51,7 @@ export const ChangeReviewDialog = ({
changeSetError,
fetchAgentChanges,
fetchTaskChanges,
clearChangeReview,
clearChangeReviewCache,
hunkDecisions,
fileDecisions,
fileContents,
@ -70,6 +70,10 @@ export const ChangeReviewDialog = ({
updateEditedContent,
discardFileEdits,
saveEditedFile,
loadDecisionsFromDisk,
persistDecisions,
clearDecisionsFromDisk,
resetAllReviewState,
} = useStore();
// Active file from scroll-spy (replaces selectedReviewFilePath for continuous scroll)
@ -105,6 +109,9 @@ export const ChangeReviewDialog = ({
// Build scope key for viewed storage
const scopeKey = mode === 'task' ? `task:${taskId ?? ''}` : `agent:${memberName ?? ''}`;
// Build scope key for decision persistence (filesystem-safe: use `-` instead of `:`)
const decisionScopeKey = mode === 'task' ? `task-${taskId ?? ''}` : `agent-${memberName ?? ''}`;
// File paths for viewed tracking
const allFilePaths = useMemo(
() => (activeChangeSet?.files ?? []).map((f) => f.filePath),
@ -243,21 +250,46 @@ export const ChangeReviewDialog = ({
// Load data on open
useEffect(() => {
if (!open) return;
// Load persisted decisions from disk
void loadDecisionsFromDisk(teamName, decisionScopeKey);
// Fetch changeSet
if (mode === 'agent' && memberName) {
void fetchAgentChanges(teamName, memberName);
} else if (mode === 'task' && taskId) {
void fetchTaskChanges(teamName, taskId);
}
return () => clearChangeReview();
// On close — clear only volatile cache, keep decisions in store
return () => clearChangeReviewCache();
}, [
open,
mode,
teamName,
memberName,
taskId,
decisionScopeKey,
fetchAgentChanges,
fetchTaskChanges,
clearChangeReview,
clearChangeReviewCache,
loadDecisionsFromDisk,
]);
// Persist decisions to disk on change (debounced via store action)
const hasDecisions =
Object.keys(hunkDecisions).length > 0 || Object.keys(fileDecisions).length > 0;
useEffect(() => {
if (!open || !hasDecisions) return;
persistDecisions(teamName, decisionScopeKey);
}, [
open,
hasDecisions,
hunkDecisions,
fileDecisions,
teamName,
decisionScopeKey,
persistDecisions,
]);
// Reset initial scroll flag when initialFilePath changes
@ -336,9 +368,23 @@ export const ChangeReviewDialog = ({
};
}, [activeChangeSet]);
const handleApply = useCallback(() => {
void applyReview(teamName, taskId, memberName);
}, [applyReview, teamName, taskId, memberName]);
const handleApply = useCallback(async () => {
await applyReview(teamName, taskId, memberName);
// Only cleanup if apply succeeded (no error in store)
const state = useStore.getState();
if (!state.applyError) {
void clearDecisionsFromDisk(teamName, decisionScopeKey);
resetAllReviewState();
}
}, [
applyReview,
teamName,
taskId,
memberName,
clearDecisionsFromDisk,
decisionScopeKey,
resetAllReviewState,
]);
// Active file for timeline (derived from scroll-spy)
const activeFile = useMemo(() => {

View file

@ -7,6 +7,10 @@ const taskChangesCheckInFlight = new Set<string>();
const taskChangesNegativeCache = new Map<string, number>();
const NEGATIVE_CACHE_TTL = 30_000;
/** Debounce timer for persisting decisions to disk */
let persistDebounceTimer: ReturnType<typeof setTimeout> | null = null;
const PERSIST_DEBOUNCE_MS = 500;
import type { AppState } from '../types';
import type {
AgentChangeSet,
@ -64,8 +68,15 @@ export interface ChangeReviewSlice {
fetchTaskChanges: (teamName: string, taskId: string) => Promise<void>;
selectReviewFile: (filePath: string | null) => void;
clearChangeReview: () => void;
clearChangeReviewCache: () => void;
resetAllReviewState: () => void;
fetchChangeStats: (teamName: string, memberName: string) => Promise<void>;
// Decision persistence actions
loadDecisionsFromDisk: (teamName: string, scopeKey: string) => Promise<void>;
persistDecisions: (teamName: string, scopeKey: string) => void;
clearDecisionsFromDisk: (teamName: string, scopeKey: string) => Promise<void>;
// Phase 2 actions
setHunkDecision: (filePath: string, hunkIndex: number, decision: HunkDecision) => void;
setFileDecision: (filePath: string, decision: HunkDecision) => void;
@ -177,6 +188,70 @@ export const createChangeReviewSlice: StateCreator<AppState, [], [], ChangeRevie
});
},
clearChangeReviewCache: () => {
set({
activeChangeSet: null,
changeSetLoading: false,
changeSetError: null,
selectedReviewFilePath: null,
fileContents: {},
fileContentsLoading: {},
applyError: null,
applying: false,
editedContents: {},
});
},
resetAllReviewState: () => {
set({
activeChangeSet: null,
changeSetLoading: false,
changeSetError: null,
selectedReviewFilePath: null,
hunkDecisions: {},
fileDecisions: {},
fileContents: {},
fileContentsLoading: {},
applyError: null,
applying: false,
editedContents: {},
});
},
// ── Decision persistence ──
loadDecisionsFromDisk: async (teamName: string, scopeKey: string) => {
try {
const data = await api.review.loadDecisions(teamName, scopeKey);
// Always set decisions — even to empty if no saved file exists.
// This prevents stale decisions from a previous scope leaking through.
set({
hunkDecisions: data?.hunkDecisions ?? {},
fileDecisions: data?.fileDecisions ?? {},
});
} catch (error) {
logger.error('loadDecisionsFromDisk error:', error);
}
},
persistDecisions: (teamName: string, scopeKey: string) => {
if (persistDebounceTimer) {
clearTimeout(persistDebounceTimer);
}
persistDebounceTimer = setTimeout(() => {
const { hunkDecisions, fileDecisions } = get();
void api.review.saveDecisions(teamName, scopeKey, hunkDecisions, fileDecisions);
}, PERSIST_DEBOUNCE_MS);
},
clearDecisionsFromDisk: async (teamName: string, scopeKey: string) => {
try {
await api.review.clearDecisions(teamName, scopeKey);
} catch (error) {
logger.error('clearDecisionsFromDisk error:', error);
}
},
fetchChangeStats: async (teamName: string, memberName: string) => {
try {
const stats = await api.review.getChangeStats(teamName, memberName);

View file

@ -20,6 +20,7 @@ import type {
ChangeStats,
ConflictCheckResult,
FileChangeWithContent,
HunkDecision,
RejectResult,
SnippetDiff,
TaskChangeSetV2,
@ -470,6 +471,21 @@ export interface ReviewAPI {
) => Promise<{ preview: string; hasConflicts: boolean }>;
// Editable diff
saveEditedFile: (filePath: string, content: string) => Promise<{ success: boolean }>;
// Decision persistence
loadDecisions: (
teamName: string,
scopeKey: string
) => Promise<{
hunkDecisions: Record<string, HunkDecision>;
fileDecisions: Record<string, HunkDecision>;
} | null>;
saveDecisions: (
teamName: string,
scopeKey: string,
hunkDecisions: Record<string, HunkDecision>,
fileDecisions: Record<string, HunkDecision>
) => Promise<void>;
clearDecisions: (teamName: string, scopeKey: string) => Promise<void>;
onCmdN?: (callback: () => void) => (() => void) | undefined;
// Phase 4
getGitFileLog: (