feat: enhance diff view and review process with new features
- Updated SnippetDiff to include replaceAll for handling multiple replacements in edits. - Improved rejectHunks logic to support delete operations and handle replaceAll scenarios. - Added validation for path traversal in rejectHunks to enhance security. - Enhanced previewReject to ensure consistency with rejectHunks and prevent conflicts. - Updated ReviewAPI and IPC handlers to accommodate new features and ensure backward compatibility. - Improved documentation to reflect changes in diff view and review processes.
This commit is contained in:
parent
61c62741c3
commit
545fa2cfb8
4 changed files with 328 additions and 75 deletions
|
|
@ -20,10 +20,11 @@ pnpm add diff # jsdiff v8 — structuredPatch, createPatch для вычис
|
||||||
export interface SnippetDiff {
|
export interface SnippetDiff {
|
||||||
toolUseId: string;
|
toolUseId: string;
|
||||||
filePath: string;
|
filePath: string;
|
||||||
toolName: 'Edit' | 'Write' | 'MultiEdit' | 'NotebookEdit';
|
toolName: 'Edit' | 'Write' | 'MultiEdit';
|
||||||
type: 'edit' | 'write-new' | 'write-update' | 'multi-edit';
|
type: 'edit' | 'write-new' | 'write-update' | 'multi-edit';
|
||||||
oldString: string; // пустая строка для Write (create)
|
oldString: string; // пустая строка для Write (create)
|
||||||
newString: string;
|
newString: string;
|
||||||
|
replaceAll: boolean; // Edit с replace_all: true → все вхождения old_string заменяются
|
||||||
timestamp: string; // ISO timestamp из JSONL
|
timestamp: string; // ISO timestamp из JSONL
|
||||||
isError: boolean; // пропускаем если true
|
isError: boolean; // пропускаем если true
|
||||||
}
|
}
|
||||||
|
|
@ -108,8 +109,17 @@ export class ChangeExtractorService {
|
||||||
```json
|
```json
|
||||||
{ "file_path": "/abs/path", "edits": [{ "old_string": "...", "new_string": "..." }, ...] }
|
{ "file_path": "/abs/path", "edits": [{ "old_string": "...", "new_string": "..." }, ...] }
|
||||||
```
|
```
|
||||||
5. **Пропуск ошибок**: Следующий за tool_use блок `tool_result` с `is_error: true` → пропускаем этот tool_use
|
5. **NotebookEdit — SKIP**: `NotebookEdit` имеет другую структуру input (`notebook_path`, `cell_number`, `new_source`) — **нет** `file_path`, `old_string`, `new_string`. Пропускаем при парсинге (`toolName !== 'NotebookEdit'` guard). НЕ включаем в SnippetDiff.
|
||||||
6. **Фильтрация proxy_ префикса**: Имена инструментов приходят как `proxy_Edit` — нужно strip prefix (паттерн из MemberStatsComputer)
|
6. **replace_all** — при `replace_all: true` в Edit input:
|
||||||
|
- В SnippetDiff записываем `replaceAll: true`
|
||||||
|
- При snippet chain reconstruction используем `content.replaceAll(oldString, newString)` вместо `content.replace()`
|
||||||
|
- При reject — нужно откатить ВСЕ вхождения, не только первое (см. Phase 2)
|
||||||
|
7. **Пропуск ошибок** — `tool_result` с `is_error: true` находится в **ДРУГОМ JSONL entry** (следующий user/isMeta entry), а НЕ в том же content массиве:
|
||||||
|
- Парсить JSONL попарно: assistant entry (с tool_use) → user entry (с tool_result)
|
||||||
|
- Маппить `tool_use.id` → `tool_result.tool_use_id`
|
||||||
|
- Если `is_error: true` → пропускаем соответствующий tool_use
|
||||||
|
- **Простой подход**: первый pass — собрать `Set<string>` errored tool_use_id из всех tool_result блоков. Второй pass — фильтровать tool_use по этому set.
|
||||||
|
8. **Фильтрация proxy_ префикса**: Имена инструментов приходят как `proxy_Edit` — нужно strip prefix (паттерн из MemberStatsComputer)
|
||||||
7. **Подсчёт строк** (через `jsdiff.diffLines`):
|
7. **Подсчёт строк** (через `jsdiff.diffLines`):
|
||||||
```typescript
|
```typescript
|
||||||
import { diffLines } from 'diff';
|
import { diffLines } from 'diff';
|
||||||
|
|
@ -423,6 +433,7 @@ export type AppState = ProjectSlice &
|
||||||
| Файл | Тип | ~LOC |
|
| Файл | Тип | ~LOC |
|
||||||
|------|-----|---:|
|
|------|-----|---:|
|
||||||
| `src/shared/types/review.ts` | NEW | 80 |
|
| `src/shared/types/review.ts` | NEW | 80 |
|
||||||
|
| `src/shared/types/index.ts` | MODIFY | +1 (re-export review types из barrel) |
|
||||||
| `src/shared/types/api.ts` | MODIFY | +15 (ReviewAPI interface + ElectronAPI field) |
|
| `src/shared/types/api.ts` | MODIFY | +15 (ReviewAPI interface + ElectronAPI field) |
|
||||||
| `src/main/services/team/ChangeExtractorService.ts` | NEW | 350 |
|
| `src/main/services/team/ChangeExtractorService.ts` | NEW | 350 |
|
||||||
| `src/main/ipc/review.ts` | NEW | 100 (с wrapReviewHandler) |
|
| `src/main/ipc/review.ts` | NEW | 100 (с wrapReviewHandler) |
|
||||||
|
|
|
||||||
|
|
@ -216,12 +216,14 @@ export class ReviewApplierService {
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Preview reject без записи на диск.
|
* Preview reject без записи на диск.
|
||||||
|
* Принимает snippets для consistency с rejectHunks (иначе preview и actual reject дадут разные результаты).
|
||||||
*/
|
*/
|
||||||
async previewReject(
|
async previewReject(
|
||||||
filePath: string,
|
filePath: string,
|
||||||
original: string,
|
original: string,
|
||||||
modified: string,
|
modified: string,
|
||||||
hunkIndicesToReject: number[]
|
hunkIndicesToReject: number[],
|
||||||
|
snippets: SnippetDiff[]
|
||||||
): Promise<{ preview: string; hasConflicts: boolean }>;
|
): Promise<{ preview: string; hasConflicts: boolean }>;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
@ -278,6 +280,19 @@ async rejectHunks(
|
||||||
for (const snippet of rejectedSnippets) {
|
for (const snippet of rejectedSnippets) {
|
||||||
if (snippet.type === 'write-new') continue; // Обрабатывается через rejectFile()
|
if (snippet.type === 'write-new') continue; // Обрабатывается через rejectFile()
|
||||||
|
|
||||||
|
// Guard: пустой newString (delete operation) — indexOf('') вернёт 0, сломает файл
|
||||||
|
if (!snippet.newString) {
|
||||||
|
// Delete reject = вставить oldString обратно. Требует позиционный контекст.
|
||||||
|
// Fallback на hunk-level для таких случаев.
|
||||||
|
return this.rejectHunksFallback(filePath, original, modified, hunkIndicesToReject);
|
||||||
|
}
|
||||||
|
|
||||||
|
// replaceAll: true — все вхождения были заменены, нужно откатить все
|
||||||
|
if (snippet.replaceAll) {
|
||||||
|
content = content.replaceAll(snippet.newString, snippet.oldString);
|
||||||
|
continue; // Не добавляем в positioned — уже обработано
|
||||||
|
}
|
||||||
|
|
||||||
const offset = content.indexOf(snippet.newString);
|
const offset = content.indexOf(snippet.newString);
|
||||||
if (offset === -1) {
|
if (offset === -1) {
|
||||||
// Snippet не найден — fallback на hunk-level
|
// Snippet не найден — fallback на hunk-level
|
||||||
|
|
@ -443,6 +458,23 @@ initializeReviewHandlers({
|
||||||
```
|
```
|
||||||
`registerReviewHandlers()` и `removeReviewHandlers()` уже зарегистрированы в Phase 1.
|
`registerReviewHandlers()` и `removeReviewHandlers()` уже зарегистрированы в Phase 1.
|
||||||
|
|
||||||
|
**ВАЖНО**: `removeReviewHandlers()` нужно обновить — добавить Phase 2 каналы:
|
||||||
|
```typescript
|
||||||
|
export function removeReviewHandlers(ipcMain: IpcMain): void {
|
||||||
|
// Phase 1
|
||||||
|
ipcMain.removeHandler(REVIEW_GET_AGENT_CHANGES);
|
||||||
|
ipcMain.removeHandler(REVIEW_GET_TASK_CHANGES);
|
||||||
|
ipcMain.removeHandler(REVIEW_GET_CHANGE_STATS);
|
||||||
|
// Phase 2
|
||||||
|
ipcMain.removeHandler(REVIEW_CHECK_CONFLICT);
|
||||||
|
ipcMain.removeHandler(REVIEW_REJECT_HUNKS);
|
||||||
|
ipcMain.removeHandler(REVIEW_REJECT_FILE);
|
||||||
|
ipcMain.removeHandler(REVIEW_PREVIEW_REJECT);
|
||||||
|
ipcMain.removeHandler(REVIEW_APPLY_DECISIONS);
|
||||||
|
ipcMain.removeHandler(REVIEW_GET_FILE_CONTENT);
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
**ВАЖНО**: Обновить `ReviewAPI` в `src/shared/types/api.ts` — добавить Phase 2 методы:
|
**ВАЖНО**: Обновить `ReviewAPI` в `src/shared/types/api.ts` — добавить Phase 2 методы:
|
||||||
```typescript
|
```typescript
|
||||||
export interface ReviewAPI {
|
export interface ReviewAPI {
|
||||||
|
|
@ -452,9 +484,9 @@ export interface ReviewAPI {
|
||||||
getChangeStats: (...) => Promise<ChangeStats>;
|
getChangeStats: (...) => Promise<ChangeStats>;
|
||||||
// Phase 2
|
// Phase 2
|
||||||
checkConflict: (filePath: string, expectedModified: string) => Promise<ConflictCheckResult>;
|
checkConflict: (filePath: string, expectedModified: string) => Promise<ConflictCheckResult>;
|
||||||
rejectHunks: (filePath: string, original: string, modified: string, hunkIndices: number[], snippets: SnippetDiff[]) => Promise<RejectResult>;
|
rejectHunks: (teamName: string, filePath: string, original: string, modified: string, hunkIndices: number[], snippets: SnippetDiff[]) => Promise<RejectResult>;
|
||||||
rejectFile: (filePath: string, original: string, modified: string) => Promise<RejectResult>;
|
rejectFile: (teamName: string, filePath: string, original: string, modified: string) => Promise<RejectResult>;
|
||||||
previewReject: (...) => Promise<{ preview: string; hasConflicts: boolean }>;
|
previewReject: (filePath: string, original: string, modified: string, hunkIndices: number[], snippets: SnippetDiff[]) => Promise<{ preview: string; hasConflicts: boolean }>;
|
||||||
applyDecisions: (request: ApplyReviewRequest) => Promise<ApplyReviewResult>;
|
applyDecisions: (request: ApplyReviewRequest) => Promise<ApplyReviewResult>;
|
||||||
getFileContent: (teamName: string, memberName: string, filePath: string) => Promise<FileChangeWithContent>;
|
getFileContent: (teamName: string, memberName: string, filePath: string) => Promise<FileChangeWithContent>;
|
||||||
}
|
}
|
||||||
|
|
@ -518,22 +550,49 @@ async function handleGetFileContent(
|
||||||
): Promise<IpcResult<FileChangeWithContent>> {
|
): Promise<IpcResult<FileChangeWithContent>> {
|
||||||
return wrapReviewHandler('review:getFileContent', async () => {
|
return wrapReviewHandler('review:getFileContent', async () => {
|
||||||
const resolver = getContentResolver();
|
const resolver = getContentResolver();
|
||||||
const result = await resolver.resolveFileContent(teamName, memberName, filePath);
|
const resolved = await resolver.resolveFileContent(teamName, memberName, filePath);
|
||||||
return result;
|
|
||||||
|
// ВАЖНО: resolver возвращает { original, modified, source },
|
||||||
|
// но ReviewAPI.getFileContent обещает FileChangeWithContent.
|
||||||
|
// Нужно маппить в полный тип:
|
||||||
|
const extractor = getChangeExtractor();
|
||||||
|
const changeSet = await extractor.getAgentChanges(teamName, memberName);
|
||||||
|
const fileSummary = changeSet.files.find(f => f.filePath === filePath);
|
||||||
|
|
||||||
|
return {
|
||||||
|
filePath,
|
||||||
|
relativePath: fileSummary?.relativePath ?? filePath.split('/').pop() ?? filePath,
|
||||||
|
snippets: fileSummary?.snippets ?? [],
|
||||||
|
linesAdded: fileSummary?.linesAdded ?? 0,
|
||||||
|
linesRemoved: fileSummary?.linesRemoved ?? 0,
|
||||||
|
isNewFile: fileSummary?.isNewFile ?? false,
|
||||||
|
originalFullContent: resolved.original,
|
||||||
|
modifiedFullContent: resolved.modified,
|
||||||
|
contentSource: resolved.source,
|
||||||
|
};
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
async function handleRejectHunks(
|
async function handleRejectHunks(
|
||||||
_event: IpcMainInvokeEvent,
|
_event: IpcMainInvokeEvent,
|
||||||
|
teamName: string, // для path traversal validation
|
||||||
filePath: string,
|
filePath: string,
|
||||||
original: string,
|
original: string,
|
||||||
modified: string,
|
modified: string,
|
||||||
hunkIndices: number[],
|
hunkIndices: number[],
|
||||||
snippets: SnippetDiff[] // R1 fix: renderer MUST передать snippets
|
snippets: SnippetDiff[] // R1 fix: renderer MUST передать snippets
|
||||||
): Promise<IpcResult<RejectResult>> {
|
): Promise<IpcResult<RejectResult>> {
|
||||||
// Security fix: валидация что filePath внутри проекта
|
|
||||||
// TODO: получить projectPath из team config и проверить path.resolve(filePath).startsWith(projectPath)
|
|
||||||
return wrapReviewHandler('review:rejectHunks', async () => {
|
return wrapReviewHandler('review:rejectHunks', async () => {
|
||||||
|
// Security: path traversal protection — ОБЯЗАТЕЛЬНО перед writeFile!
|
||||||
|
// Получаем projectPath из team config через TeamDataService
|
||||||
|
const teamData = getTeamDataService(); // добавить в ReviewHandlerDeps
|
||||||
|
const team = await teamData.getTeam(teamName); // teamName из IPC args
|
||||||
|
if (team?.projectPath) {
|
||||||
|
const resolved = require('path').resolve(filePath);
|
||||||
|
if (!resolved.startsWith(team.projectPath)) {
|
||||||
|
throw new Error('File path outside project directory');
|
||||||
|
}
|
||||||
|
}
|
||||||
const applier = getApplier();
|
const applier = getApplier();
|
||||||
return await applier.rejectHunks(filePath, original, modified, hunkIndices, snippets);
|
return await applier.rejectHunks(filePath, original, modified, hunkIndices, snippets);
|
||||||
});
|
});
|
||||||
|
|
@ -544,17 +603,44 @@ async function handleApplyDecisions(
|
||||||
request: ApplyReviewRequest
|
request: ApplyReviewRequest
|
||||||
): Promise<IpcResult<ApplyReviewResult>> {
|
): Promise<IpcResult<ApplyReviewResult>> {
|
||||||
return wrapReviewHandler('review:applyDecisions', async () => {
|
return wrapReviewHandler('review:applyDecisions', async () => {
|
||||||
|
// Validation: хотя бы один из taskId/memberName обязателен
|
||||||
|
if (!request.taskId && !request.memberName) {
|
||||||
|
throw new Error('Either taskId or memberName must be provided');
|
||||||
|
}
|
||||||
|
|
||||||
const applier = getApplier();
|
const applier = getApplier();
|
||||||
const resolver = getContentResolver();
|
const resolver = getContentResolver();
|
||||||
|
|
||||||
// Resolve all file contents first
|
// Resolve all file contents first
|
||||||
const filePaths = request.decisions.map(d => d.filePath);
|
const filePaths = request.decisions.map(d => d.filePath);
|
||||||
|
|
||||||
|
// В task mode memberName может быть undefined — resolver должен определить
|
||||||
|
// member из task scope. В agent mode memberName обязательно задан.
|
||||||
|
const memberName = request.memberName ?? '';
|
||||||
const contents = await resolver.resolveAllFileContents(
|
const contents = await resolver.resolveAllFileContents(
|
||||||
request.teamName,
|
request.teamName,
|
||||||
request.memberName ?? '',
|
memberName,
|
||||||
filePaths
|
filePaths
|
||||||
);
|
);
|
||||||
|
|
||||||
|
// Dry-run: сначала previewReject для всех файлов, чтобы обнаружить ошибки ДО записи
|
||||||
|
const rejectedDecisions = request.decisions.filter(d =>
|
||||||
|
Object.values(d.hunkDecisions).some(v => v === 'rejected')
|
||||||
|
);
|
||||||
|
for (const decision of rejectedDecisions) {
|
||||||
|
const fc = contents.get(decision.filePath);
|
||||||
|
if (!fc?.originalFullContent || !fc?.modifiedFullContent) continue;
|
||||||
|
const preview = await applier.previewReject(
|
||||||
|
decision.filePath, fc.originalFullContent, fc.modifiedFullContent,
|
||||||
|
Object.entries(decision.hunkDecisions)
|
||||||
|
.filter(([, v]) => v === 'rejected')
|
||||||
|
.map(([k]) => Number(k))
|
||||||
|
);
|
||||||
|
if (preview.hasConflicts) {
|
||||||
|
throw new Error(`Conflict detected in ${decision.filePath}. Resolve before applying.`);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
return await applier.applyReviewDecisions(request, contents);
|
return await applier.applyReviewDecisions(request, contents);
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
@ -579,9 +665,9 @@ review: {
|
||||||
invokeIpcWithResult<RejectResult>(REVIEW_REJECT_HUNKS, filePath, original, modified, hunkIndices, snippets),
|
invokeIpcWithResult<RejectResult>(REVIEW_REJECT_HUNKS, filePath, original, modified, hunkIndices, snippets),
|
||||||
rejectFile: (filePath: string, original: string, modified: string) =>
|
rejectFile: (filePath: string, original: string, modified: string) =>
|
||||||
invokeIpcWithResult<RejectResult>(REVIEW_REJECT_FILE, filePath, original, modified),
|
invokeIpcWithResult<RejectResult>(REVIEW_REJECT_FILE, filePath, original, modified),
|
||||||
previewReject: (filePath: string, original: string, modified: string, hunkIndices: number[]) =>
|
previewReject: (filePath: string, original: string, modified: string, hunkIndices: number[], snippets: SnippetDiff[]) =>
|
||||||
invokeIpcWithResult<{ preview: string; hasConflicts: boolean }>(
|
invokeIpcWithResult<{ preview: string; hasConflicts: boolean }>(
|
||||||
REVIEW_PREVIEW_REJECT, filePath, original, modified, hunkIndices
|
REVIEW_PREVIEW_REJECT, filePath, original, modified, hunkIndices, snippets
|
||||||
),
|
),
|
||||||
applyDecisions: (request: ApplyReviewRequest) =>
|
applyDecisions: (request: ApplyReviewRequest) =>
|
||||||
invokeIpcWithResult<ApplyReviewResult>(REVIEW_APPLY_DECISIONS, request),
|
invokeIpcWithResult<ApplyReviewResult>(REVIEW_APPLY_DECISIONS, request),
|
||||||
|
|
@ -622,6 +708,12 @@ export interface ChangeReviewSlice {
|
||||||
/** В процессе apply */
|
/** В процессе apply */
|
||||||
applying: boolean;
|
applying: boolean;
|
||||||
|
|
||||||
|
// Phase 1 actions (MUST be included — Phase 2 interface is full superset)
|
||||||
|
fetchAgentChanges: (teamName: string, memberName: string) => Promise<void>;
|
||||||
|
fetchTaskChanges: (teamName: string, taskId: string) => Promise<void>;
|
||||||
|
selectReviewFile: (filePath: string | null) => void;
|
||||||
|
fetchChangeStats: (teamName: string, memberName: string) => Promise<void>;
|
||||||
|
|
||||||
// Phase 2 actions
|
// Phase 2 actions
|
||||||
setHunkDecision: (filePath: string, hunkIndex: number, decision: HunkDecision) => void;
|
setHunkDecision: (filePath: string, hunkIndex: number, decision: HunkDecision) => void;
|
||||||
setFileDecision: (filePath: string, decision: HunkDecision) => void;
|
setFileDecision: (filePath: string, decision: HunkDecision) => void;
|
||||||
|
|
@ -631,10 +723,13 @@ export interface ChangeReviewSlice {
|
||||||
rejectAll: () => void;
|
rejectAll: () => void;
|
||||||
setDiffViewMode: (mode: 'unified' | 'split') => void;
|
setDiffViewMode: (mode: 'unified' | 'split') => void;
|
||||||
setCollapseUnchanged: (collapse: boolean) => void;
|
setCollapseUnchanged: (collapse: boolean) => void;
|
||||||
fetchFileContent: (teamName: string, memberName: string, filePath: string) => Promise<void>;
|
/** memberName optional — в task mode определяется из changeSet */
|
||||||
|
fetchFileContent: (teamName: string, memberName: string | undefined, filePath: string) => Promise<void>;
|
||||||
previewReject: (filePath: string) => Promise<{ preview: string; hasConflicts: boolean }>;
|
previewReject: (filePath: string) => Promise<{ preview: string; hasConflicts: boolean }>;
|
||||||
applyReview: (teamName: string, taskId?: string, memberName?: string) => Promise<void>;
|
applyReview: (teamName: string, taskId?: string, memberName?: string) => Promise<void>;
|
||||||
clearChangeReview: () => void;
|
clearChangeReview: () => void;
|
||||||
|
/** Инвалидировать changeStatsCache при team data refresh */
|
||||||
|
invalidateChangeStats: (teamName: string) => void;
|
||||||
}
|
}
|
||||||
```
|
```
|
||||||
|
|
||||||
|
|
@ -671,6 +766,22 @@ applyReview: async (teamName, taskId, memberName) => {
|
||||||
const { hunkDecisions, fileDecisions, activeChangeSet } = get();
|
const { hunkDecisions, fileDecisions, activeChangeSet } = get();
|
||||||
if (!activeChangeSet) throw new Error('No active change set');
|
if (!activeChangeSet) throw new Error('No active change set');
|
||||||
|
|
||||||
|
// Stale check: пересчитать computedAt и сравнить с текущим
|
||||||
|
// Если не совпадает — данные устарели (file watcher мог обновить между review и apply)
|
||||||
|
const freshSet = taskId
|
||||||
|
? await api.review.getTaskChanges(teamName, taskId)
|
||||||
|
: await api.review.getAgentChanges(teamName, memberName!);
|
||||||
|
if (freshSet.computedAt !== activeChangeSet.computedAt) {
|
||||||
|
set({
|
||||||
|
applying: false,
|
||||||
|
applyError: 'Changes have been updated since you started reviewing. Please review again.',
|
||||||
|
activeChangeSet: freshSet, // обновляем данные
|
||||||
|
hunkDecisions: {}, // сбрасываем decisions
|
||||||
|
fileDecisions: {},
|
||||||
|
});
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
// Собрать decisions
|
// Собрать decisions
|
||||||
const decisions: FileReviewDecision[] = activeChangeSet.files.map(file => {
|
const decisions: FileReviewDecision[] = activeChangeSet.files.map(file => {
|
||||||
const perHunk: Record<number, HunkDecision> = {};
|
const perHunk: Record<number, HunkDecision> = {};
|
||||||
|
|
@ -752,7 +863,7 @@ import { json } from '@codemirror/lang-json';
|
||||||
import { css } from '@codemirror/lang-css';
|
import { css } from '@codemirror/lang-css';
|
||||||
import { html } from '@codemirror/lang-html';
|
import { html } from '@codemirror/lang-html';
|
||||||
import { xml } from '@codemirror/lang-xml';
|
import { xml } from '@codemirror/lang-xml';
|
||||||
import { oneDark } from '@codemirror/theme-one-dark';
|
// НЕ используем @codemirror/theme-one-dark — вместо этого CSS variables
|
||||||
|
|
||||||
interface CodeMirrorDiffViewProps {
|
interface CodeMirrorDiffViewProps {
|
||||||
/** Полное содержимое файла ДО изменений */
|
/** Полное содержимое файла ДО изменений */
|
||||||
|
|
@ -857,77 +968,110 @@ export function CodeMirrorDiffView({
|
||||||
: undefined,
|
: undefined,
|
||||||
```
|
```
|
||||||
|
|
||||||
4. **Event tracking для accept/reject — VERIFIED API:**
|
4. **Event tracking для accept/reject — через mergeControls callback (НЕ Transaction аннотации!):**
|
||||||
|
|
||||||
|
**ВАЖНО**: `Transaction.userEvent` значения `"accept"`/`"revert"` — это internal implementation detail
|
||||||
|
`@codemirror/merge`, **не документированные публично**. Могут измениться без предупреждения.
|
||||||
|
Вместо перехвата аннотаций — используем `mergeControls` callback:
|
||||||
|
|
||||||
```typescript
|
```typescript
|
||||||
// VERIFIED: CodeMirror merge помечает transactions через annotation:
|
// mergeControls callback уже вызывается при клике accept/reject.
|
||||||
// - Accept: Transaction.userEvent = "accept"
|
// Вычисляем hunk index ВНУТРИ callback через getChunks():
|
||||||
// - Reject: Transaction.userEvent = "revert"
|
import { getChunks } from '@codemirror/merge';
|
||||||
//
|
|
||||||
// НЕ используем tr.isUserEvent() — используем tr.annotation()!
|
|
||||||
|
|
||||||
EditorView.updateListener.of((update) => {
|
mergeControls: showMergeControls
|
||||||
for (const tr of update.transactions) {
|
? (type: 'reject' | 'accept', action: (e: MouseEvent) => void) => {
|
||||||
const userEvent = tr.annotation(Transaction.userEvent);
|
const btn = document.createElement('button');
|
||||||
if (userEvent === 'accept') {
|
btn.className = type === 'accept' ? 'cm-merge-accept-btn' : 'cm-merge-reject-btn';
|
||||||
// Определяем hunk index по позиции cursor ПЕРЕД транзакцией
|
btn.textContent = type === 'accept' ? 'Accept' : 'Reject';
|
||||||
const pos = tr.startState.selection.main.head;
|
btn.onmousedown = (e) => {
|
||||||
const hunkIndex = computeHunkIndexAtPos(tr.startState, pos);
|
// 1. Вычисляем hunk index ДО action (action изменит state)
|
||||||
onHunkAccepted?.(hunkIndex);
|
const view = editorRef.current;
|
||||||
|
if (view) {
|
||||||
|
const pos = view.state.selection.main.head;
|
||||||
|
const hunkIndex = computeHunkIndexAtPos(view.state, pos);
|
||||||
|
// 2. Выполняем оригинальное CM action
|
||||||
|
action(e);
|
||||||
|
// 3. Callback в React
|
||||||
|
if (type === 'accept') onHunkAccepted?.(hunkIndex);
|
||||||
|
else onHunkRejected?.(hunkIndex);
|
||||||
|
} else {
|
||||||
|
action(e);
|
||||||
|
}
|
||||||
|
};
|
||||||
|
return btn;
|
||||||
}
|
}
|
||||||
if (userEvent === 'revert') {
|
: undefined,
|
||||||
const pos = tr.startState.selection.main.head;
|
|
||||||
const hunkIndex = computeHunkIndexAtPos(tr.startState, pos);
|
|
||||||
onHunkRejected?.(hunkIndex);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
});
|
|
||||||
```
|
```
|
||||||
|
|
||||||
**ВАЖНО: Chunk positions — NO PUBLIC API!**
|
Для **keyboard shortcuts** (Phase 4) — используем `acceptChunk(view, pos)` / `rejectChunk(view, pos)` программно и вызываем callback напрямую.
|
||||||
|
|
||||||
`@codemirror/merge` НЕ экспортирует публичный API для получения позиций chunks.
|
**Chunk positions через `getChunks()` — PUBLIC API:**
|
||||||
Нужно вычислять hunk index самостоятельно через diff algorithm:
|
|
||||||
|
`@codemirror/merge` экспортирует `getChunks(state)` для получения позиций chunks.
|
||||||
|
**НЕ используем jsdiff** для вычисления hunk позиций — jsdiff и CM используют РАЗНЫЕ diff алгоритмы, границы hunks могут не совпадать!
|
||||||
|
|
||||||
```typescript
|
```typescript
|
||||||
// Вычисляем позиции hunks через jsdiff (уже установлен)
|
import { getChunks, acceptChunk, rejectChunk } from '@codemirror/merge';
|
||||||
import { structuredPatch } from 'diff';
|
|
||||||
|
|
||||||
function computeHunkRanges(original: string, modified: string) {
|
|
||||||
const patch = structuredPatch('', '', original, modified);
|
|
||||||
// patch.hunks содержит newStart/newLines для каждого hunk
|
|
||||||
return patch.hunks.map((h, idx) => ({
|
|
||||||
index: idx,
|
|
||||||
fromLine: h.newStart,
|
|
||||||
toLine: h.newStart + h.newLines,
|
|
||||||
}));
|
|
||||||
}
|
|
||||||
|
|
||||||
function computeHunkIndexAtPos(state: EditorState, pos: number): number {
|
function computeHunkIndexAtPos(state: EditorState, pos: number): number {
|
||||||
|
const result = getChunks(state);
|
||||||
|
if (!result) return -1;
|
||||||
|
const { chunks } = result;
|
||||||
const line = state.doc.lineAt(pos).number;
|
const line = state.doc.lineAt(pos).number;
|
||||||
const ranges = hunkRangesRef.current; // Кешируем при создании editor
|
return chunks.findIndex(c => line >= c.fromB && line < c.toB);
|
||||||
for (const r of ranges) {
|
}
|
||||||
if (line >= r.fromLine && line <= r.toLine) return r.index;
|
|
||||||
}
|
// Программный accept/reject по позиции (вместо перехвата Transaction аннотаций):
|
||||||
return -1; // Not in a hunk
|
function acceptHunkAtPos(view: EditorView, pos: number): boolean {
|
||||||
|
return acceptChunk(view, pos);
|
||||||
|
}
|
||||||
|
function rejectHunkAtPos(view: EditorView, pos: number): boolean {
|
||||||
|
return rejectChunk(view, pos);
|
||||||
}
|
}
|
||||||
```
|
```
|
||||||
|
|
||||||
5. **Keyboard navigation через StateCommand — VERIFIED API:**
|
**ВАЖНО**: `acceptChunk`/`rejectChunk` — публичные функции из `@codemirror/merge`.
|
||||||
|
Принимают `(view: EditorView, pos?: number)`, возвращают `boolean`.
|
||||||
|
Если `pos` не указан — работают с chunk под курсором.
|
||||||
|
|
||||||
|
5. **Keyboard navigation — прямой вызов (НЕ .run()):**
|
||||||
|
|
||||||
```typescript
|
```typescript
|
||||||
// goToNextChunk и goToPreviousChunk — это StateCommand объекты.
|
// goToNextChunk и goToPreviousChunk — это функции (Command type).
|
||||||
// Используются через keymap ИЛИ вызов .run(view):
|
// Используются через keymap ИЛИ прямой вызов:
|
||||||
|
|
||||||
keymap.of([
|
keymap.of([
|
||||||
{ key: 'Ctrl-Alt-ArrowDown', run: goToNextChunk },
|
{ key: 'Ctrl-Alt-ArrowDown', run: goToNextChunk },
|
||||||
{ key: 'Ctrl-Alt-ArrowUp', run: goToPreviousChunk },
|
{ key: 'Ctrl-Alt-ArrowUp', run: goToPreviousChunk },
|
||||||
]),
|
]),
|
||||||
|
|
||||||
// Программный вызов:
|
// Программный вызов (прямой, НЕ через .run()!):
|
||||||
// goToNextChunk(editorRef.current!) // returns boolean
|
// goToNextChunk(editorRef.current!) // returns boolean
|
||||||
```
|
```
|
||||||
|
|
||||||
|
6. **Unified vs Split — РАЗНЫЕ классы!**
|
||||||
|
|
||||||
|
Toggle unified ↔ split требует **полного пересоздания**:
|
||||||
|
- **Unified**: `new EditorView({ extensions: [unifiedMergeView({...})] })`
|
||||||
|
- **Split**: `new MergeView({ a: {...}, b: {...}, parent, revertControls: 'a-to-b' })`
|
||||||
|
|
||||||
|
Это разные DOM-структуры и разные lifecycle. При переключении — `destroy()` старый + создать новый.
|
||||||
|
В split mode: `MergeView` имеет `.a` и `.b` EditorView, accept/reject через `revertControls` (не `mergeControls`).
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// Ref должен быть union:
|
||||||
|
const viewRef = useRef<EditorView | MergeView | null>(null);
|
||||||
|
|
||||||
|
// Helper для получения активного EditorView:
|
||||||
|
function getActiveEditorView(): EditorView | null {
|
||||||
|
const ref = viewRef.current;
|
||||||
|
if (!ref) return null;
|
||||||
|
if ('b' in ref) return ref.b; // MergeView → use "modified" side
|
||||||
|
return ref; // EditorView (unified)
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
5. **Тема (CSS variables integration)**:
|
5. **Тема (CSS variables integration)**:
|
||||||
```typescript
|
```typescript
|
||||||
const customTheme = EditorView.theme({
|
const customTheme = EditorView.theme({
|
||||||
|
|
@ -982,7 +1126,7 @@ export function CodeMirrorDiffView({
|
||||||
'.cm-merge-reject-btn:hover': {
|
'.cm-merge-reject-btn:hover': {
|
||||||
backgroundColor: 'rgba(239, 68, 68, 0.35)',
|
backgroundColor: 'rgba(239, 68, 68, 0.35)',
|
||||||
},
|
},
|
||||||
}, { dark: true });
|
}); // БЕЗ { dark: true } — CSS variables адаптируются к теме автоматически
|
||||||
```
|
```
|
||||||
|
|
||||||
6. **Extensions assembly — VERIFIED unifiedMergeView config:**
|
6. **Extensions assembly — VERIFIED unifiedMergeView config:**
|
||||||
|
|
|
||||||
|
|
@ -565,8 +565,10 @@ async getTaskChanges(teamName: string, taskId: string): Promise<TaskChangeSetV2>
|
||||||
const boundaries = await this.boundaryParser.parseBoundaries(ref.filePath);
|
const boundaries = await this.boundaryParser.parseBoundaries(ref.filePath);
|
||||||
const scope = boundaries.scopes.find(s => s.taskId === taskId);
|
const scope = boundaries.scopes.find(s => s.taskId === taskId);
|
||||||
if (scope) {
|
if (scope) {
|
||||||
scope.memberName = ref.memberName;
|
// CRITICAL: НЕ мутируем scope напрямую — он из кеша TaskBoundaryParser!
|
||||||
allScopes.push(scope);
|
// Мутация scope.memberName = ... портит кешированный объект при повторных вызовах.
|
||||||
|
const scopeCopy = { ...scope, memberName: ref.memberName };
|
||||||
|
allScopes.push(scopeCopy);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -735,14 +737,63 @@ async hasTaskUpdateMarker(filePath: string, taskId: string): Promise<boolean> {
|
||||||
// Но findLogsForTask() НЕ МЕНЯЕТСЯ.
|
// Но findLogsForTask() НЕ МЕНЯЕТСЯ.
|
||||||
```
|
```
|
||||||
|
|
||||||
### 5. IPC (без изменений)
|
### 5. Обновление `src/main/index.ts` (MODIFY)
|
||||||
|
|
||||||
|
Phase 3 создаёт `TaskBoundaryParser` и передаёт в `ChangeExtractorService`:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// В initializeIpcHandlers() или рядом с ним:
|
||||||
|
import { TaskBoundaryParser } from '@main/services/team/TaskBoundaryParser';
|
||||||
|
|
||||||
|
// Phase 1 было:
|
||||||
|
// const changeExtractor = new ChangeExtractorService(teamMemberLogsFinder);
|
||||||
|
|
||||||
|
// Phase 3 →:
|
||||||
|
const taskBoundaryParser = new TaskBoundaryParser();
|
||||||
|
const changeExtractor = new ChangeExtractorService(teamMemberLogsFinder, taskBoundaryParser);
|
||||||
|
|
||||||
|
// ReviewHandlerDeps не меняется (Phase 3 не добавляет новых deps в review.ts).
|
||||||
|
// TaskBoundaryParser используется ВНУТРИ ChangeExtractorService.
|
||||||
|
```
|
||||||
|
|
||||||
|
### 6. IPC (без изменений)
|
||||||
|
|
||||||
Phase 1 уже определил `REVIEW_GET_TASK_CHANGES`. Phase 3 не добавляет новых каналов — только улучшает backend точность.
|
Phase 1 уже определил `REVIEW_GET_TASK_CHANGES`. Phase 3 не добавляет новых каналов — только улучшает backend точность.
|
||||||
|
|
||||||
### 6. Preload bridge (без изменений)
|
### 6. Preload bridge и Store — обновление типов
|
||||||
|
|
||||||
Тип `TaskChangeSet` расширяется до `TaskChangeSetV2` (backwards compatible через extends).
|
Тип `TaskChangeSet` расширяется до `TaskChangeSetV2` (backwards compatible через extends).
|
||||||
|
|
||||||
|
**IMPORTANT: Обновить типы в 3 местах:**
|
||||||
|
|
||||||
|
1. **Preload bridge** (`src/preload/index.ts`): generic тип IPC-вызова обновить:
|
||||||
|
```typescript
|
||||||
|
// Phase 1:
|
||||||
|
getTaskChanges: (teamName: string, taskId: string) =>
|
||||||
|
invokeIpcWithResult<TaskChangeSet>(REVIEW_GET_TASK_CHANGES, teamName, taskId),
|
||||||
|
// Phase 3 → заменить на:
|
||||||
|
getTaskChanges: (teamName: string, taskId: string) =>
|
||||||
|
invokeIpcWithResult<TaskChangeSetV2>(REVIEW_GET_TASK_CHANGES, teamName, taskId),
|
||||||
|
```
|
||||||
|
|
||||||
|
2. **Store type** (`src/renderer/store/slices/changeReviewSlice.ts`):
|
||||||
|
```typescript
|
||||||
|
// Phase 2 тип:
|
||||||
|
activeChangeSet: AgentChangeSet | TaskChangeSet | null;
|
||||||
|
// Phase 3 → расширить:
|
||||||
|
activeChangeSet: AgentChangeSet | TaskChangeSet | TaskChangeSetV2 | null;
|
||||||
|
```
|
||||||
|
|
||||||
|
3. **ReviewAPI** (`src/shared/types/api.ts`): return type обновить:
|
||||||
|
```typescript
|
||||||
|
// Phase 1:
|
||||||
|
getTaskChanges: (teamName: string, taskId: string) => Promise<TaskChangeSet>;
|
||||||
|
// Phase 3 →:
|
||||||
|
getTaskChanges: (teamName: string, taskId: string) => Promise<TaskChangeSetV2>;
|
||||||
|
```
|
||||||
|
|
||||||
|
Все три изменения backwards compatible: `TaskChangeSetV2 extends TaskChangeSet`, поэтому все Phase 2 компоненты продолжают работать. Phase 3 компоненты используют `isTaskChangeSetV2()` type guard для доступа к `.scope` и `.warnings`.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Frontend
|
## Frontend
|
||||||
|
|
@ -959,12 +1010,15 @@ JSONL Timeline:
|
||||||
| `src/main/services/team/ChangeExtractorService.ts` | MODIFY | +150 |
|
| `src/main/services/team/ChangeExtractorService.ts` | MODIFY | +150 |
|
||||||
| `src/main/services/team/TeamMemberLogsFinder.ts` | MODIFY | +40 |
|
| `src/main/services/team/TeamMemberLogsFinder.ts` | MODIFY | +40 |
|
||||||
| `src/main/services/team/index.ts` | MODIFY | +1 |
|
| `src/main/services/team/index.ts` | MODIFY | +1 |
|
||||||
| `src/main/index.ts` | MODIFY | +5 (создать TaskBoundaryParser, передать в ChangeExtractorService) |
|
| `src/main/index.ts` | MODIFY | +5 (см. ниже) |
|
||||||
|
| `src/preload/index.ts` | MODIFY | +1 (generic `<TaskChangeSetV2>`) |
|
||||||
|
| `src/shared/types/api.ts` | MODIFY | +1 (return type `TaskChangeSetV2`) |
|
||||||
|
| `src/renderer/store/slices/changeReviewSlice.ts` | MODIFY | +1 (union type) |
|
||||||
| `src/renderer/components/team/review/ConfidenceBadge.tsx` | NEW | 45 |
|
| `src/renderer/components/team/review/ConfidenceBadge.tsx` | NEW | 45 |
|
||||||
| `src/renderer/components/team/review/ScopeWarningBanner.tsx` | NEW | 50 |
|
| `src/renderer/components/team/review/ScopeWarningBanner.tsx` | NEW | 50 |
|
||||||
| `src/renderer/components/team/review/ChangeReviewDialog.tsx` | MODIFY | +20 |
|
| `src/renderer/components/team/review/ChangeReviewDialog.tsx` | MODIFY | +20 |
|
||||||
| `src/renderer/components/team/kanban/KanbanTaskCard.tsx` | MODIFY | +15 |
|
| `src/renderer/components/team/kanban/KanbanTaskCard.tsx` | MODIFY | +15 |
|
||||||
| **Итого** | 3 NEW + 6 MODIFY | ~750 |
|
| **Итого** | 3 NEW + 9 MODIFY | ~760 |
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -135,17 +135,17 @@ useEffect(() => {
|
||||||
**Scroll-to-hunk через CodeMirror API — VERIFIED:**
|
**Scroll-to-hunk через CodeMirror API — VERIFIED:**
|
||||||
|
|
||||||
```typescript
|
```typescript
|
||||||
// VERIFIED: goToNextChunk и goToPreviousChunk — это StateCommand объекты.
|
// VERIFIED: goToNextChunk и goToPreviousChunk — это (view: EditorView) => boolean функции.
|
||||||
// Вызываются через .run(view) или через keymap:
|
// Вызываются НАПРЯМУЮ, НЕ через .run():
|
||||||
import { goToNextChunk, goToPreviousChunk } from '@codemirror/merge';
|
import { goToNextChunk, goToPreviousChunk } from '@codemirror/merge';
|
||||||
|
|
||||||
function scrollToHunk(editorView: EditorView, direction: 'next' | 'prev') {
|
function scrollToHunk(editorView: EditorView, direction: 'next' | 'prev'): boolean {
|
||||||
if (direction === 'next') {
|
if (direction === 'next') {
|
||||||
goToNextChunk.run(editorView); // StateCommand.run() — НЕ прямой вызов!
|
return goToNextChunk(editorView); // Прямой вызов! Возвращает boolean.
|
||||||
} else {
|
} else {
|
||||||
goToPreviousChunk.run(editorView);
|
return goToPreviousChunk(editorView);
|
||||||
}
|
}
|
||||||
// Возвращает boolean: true если нашёл chunk, false если конец/начало
|
// true = нашёл chunk и перешёл, false = конец/начало (нет больше chunks)
|
||||||
}
|
}
|
||||||
```
|
```
|
||||||
|
|
||||||
|
|
@ -189,6 +189,23 @@ const MAX_TOTAL_ENTRIES = 50; // M2 fix: max number of scope keys in stora
|
||||||
*
|
*
|
||||||
* M2 fix: scopeKey включает version hash (computedAt) для инвалидации
|
* M2 fix: scopeKey включает version hash (computedAt) для инвалидации
|
||||||
* при перевычислении changeSet.
|
* при перевычислении changeSet.
|
||||||
|
*
|
||||||
|
* ФОРМАТ scopeKey:
|
||||||
|
* - Task mode: `task:{taskId}` (пример: `task:42`)
|
||||||
|
* - Agent mode: `agent:{memberName}` (пример: `agent:researcher`)
|
||||||
|
* - Full team: `team` (для полного team review без фильтрации)
|
||||||
|
*
|
||||||
|
* Вызывающий код генерирует scopeKey:
|
||||||
|
* ```typescript
|
||||||
|
* function buildScopeKey(mode: 'task' | 'agent' | 'team', id?: string): string {
|
||||||
|
* if (mode === 'task') return `task:${id}`;
|
||||||
|
* if (mode === 'agent') return `agent:${id}`;
|
||||||
|
* return 'team';
|
||||||
|
* }
|
||||||
|
* ```
|
||||||
|
*
|
||||||
|
* Инвалидация: При изменении computedAt в activeChangeSet, viewed state
|
||||||
|
* сбрасывается через useEffect в useViewedFiles (version bump → re-read).
|
||||||
*/
|
*/
|
||||||
|
|
||||||
interface ViewedStorageEntry {
|
interface ViewedStorageEntry {
|
||||||
|
|
@ -759,13 +776,40 @@ getGitFileLog: async (projectPath: string, filePath: string) =>
|
||||||
window.electronAPI.review.getGitFileLog(projectPath, filePath),
|
window.electronAPI.review.getGitFileLog(projectPath, filePath),
|
||||||
```
|
```
|
||||||
|
|
||||||
#### IPC: `src/preload/constants/ipcChannels.ts` (MODIFY)
|
#### IPC channel: `src/preload/constants/ipcChannels.ts` (MODIFY)
|
||||||
|
|
||||||
```typescript
|
```typescript
|
||||||
// Phase 4 additions
|
// Phase 4 additions
|
||||||
export const REVIEW_GET_GIT_FILE_LOG = 'review:getGitFileLog';
|
export const REVIEW_GET_GIT_FILE_LOG = 'review:getGitFileLog';
|
||||||
```
|
```
|
||||||
|
|
||||||
|
#### IPC handler: `src/main/ipc/review.ts` (MODIFY)
|
||||||
|
|
||||||
|
Добавить handler и регистрацию в `registerReviewHandlers()`:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// Handler
|
||||||
|
async function handleGetGitFileLog(
|
||||||
|
_event: IpcMainInvokeEvent,
|
||||||
|
projectPath: string,
|
||||||
|
filePath: string
|
||||||
|
): Promise<IpcResult<Array<{ hash: string; timestamp: string; message: string }>>> {
|
||||||
|
return wrapReviewHandler(async () => {
|
||||||
|
const deps = getReviewDeps();
|
||||||
|
if (!deps.gitFallback) {
|
||||||
|
return [];
|
||||||
|
}
|
||||||
|
return deps.gitFallback.getFileLog(projectPath, filePath);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
// В registerReviewHandlers():
|
||||||
|
ipcMain.handle(REVIEW_GET_GIT_FILE_LOG, handleGetGitFileLog);
|
||||||
|
|
||||||
|
// В removeReviewHandlers():
|
||||||
|
ipcMain.removeHandler(REVIEW_GET_GIT_FILE_LOG);
|
||||||
|
```
|
||||||
|
|
||||||
#### Preload: `src/preload/index.ts` (MODIFY)
|
#### Preload: `src/preload/index.ts` (MODIFY)
|
||||||
|
|
||||||
```typescript
|
```typescript
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue