Skip to content

Commit 2b81a71

Browse files
frankbriaTest User
frankbria
and
Test User
authored
feat(review): add business context (task/requirement) to diff viewer file tree (#480)
* feat(review): extend FileChange type + wire task context to review page (#480) * feat(review): task grouping in FileTreePanel + task context in DiffViewer (#480) * fix(review): use explicit task props instead of spread in review page (#480) * fix(review): use contextTask as fallback in FileTreePanel grouping and badges (#480) * test(review): add missing test for task without requirement_ids (#480) * fix(review): per-file task resolution in DiffViewer, O(n) grouping, error handling (#480) * fix(review): only show contextTask badge on files without explicit task_id (#480) --------- Co-authored-by: Test User <test@example.com>
1 parent 10ec571 commit 2b81a71

7 files changed

Lines changed: 597 additions & 97 deletions

File tree

‎web-ui/__mocks__/@hugeicons/react.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,4 +62,7 @@ module.exports = {
6262
ArrowLeft01Icon: createIconMock('ArrowLeft01Icon'),
6363
// Proof page
6464
InformationCircleIcon: createIconMock('InformationCircleIcon'),
65+
// FileTreePanel / DiffViewer
66+
FileAddIcon: createIconMock('FileAddIcon'),
67+
FileRemoveIcon: createIconMock('FileRemoveIcon'),
6568
};

‎web-ui/__tests__/components/review/DiffViewer.test.tsx‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
import { render, screen } from '@testing-library/react';
2+
import userEvent from '@testing-library/user-event';
3+
import { DiffViewer } from '@/components/review/DiffViewer';
4+
import type { FileChange, Task } from '@/types';
5+
import type { DiffFile } from '@/lib/diffParser';
6+
7+
// jsdom doesn't provide ResizeObserver (needed by radix ScrollArea)
8+
global.ResizeObserver = class {
9+
observe() {}
10+
unobserve() {}
11+
disconnect() {}
12+
} as unknown as typeof ResizeObserver;
13+
14+
// Mock scrollIntoView (not available in jsdom)
15+
Element.prototype.scrollIntoView = jest.fn();
16+
17+
// ─── Fixtures ───────────────────────────────────────────────────────
18+
19+
const mockDiffFiles: DiffFile[] = [
20+
{
21+
oldPath: 'src/foo.ts',
22+
newPath: 'src/foo.ts',
23+
hunks: [
24+
{
25+
header: '@@ -1,3 +1,4 @@',
26+
oldStart: 1,
27+
oldCount: 3,
28+
newStart: 1,
29+
newCount: 4,
30+
lines: [
31+
{ type: 'context', content: 'const a = 1;', oldLineNumber: 1, newLineNumber: 1 },
32+
{ type: 'addition', content: 'const b = 2;', oldLineNumber: null, newLineNumber: 2 },
33+
],
34+
},
35+
],
36+
insertions: 1,
37+
deletions: 0,
38+
isNew: false,
39+
isDeleted: false,
40+
isRenamed: false,
41+
},
42+
];
43+
44+
const mockTasks: Task[] = [
45+
{
46+
id: 'task-1',
47+
title: 'Add login',
48+
description: '',
49+
status: 'IN_PROGRESS',
50+
priority: 1,
51+
depends_on: [],
52+
requirement_ids: ['REQ-42'],
53+
},
54+
];
55+
56+
const mockTaskNoReqs: Task[] = [
57+
{
58+
id: 'task-1',
59+
title: 'Add login',
60+
description: '',
61+
status: 'IN_PROGRESS',
62+
priority: 1,
63+
depends_on: [],
64+
},
65+
];
66+
67+
// getFilePath returns newPath for non-deleted/non-renamed files
68+
const mockChangedFiles: FileChange[] = [
69+
{ path: 'src/foo.ts', change_type: 'modified', insertions: 1, deletions: 0, task_id: 'task-1' },
70+
];
71+
72+
// ─── Tests ──────────────────────────────────────────────────────────
73+
74+
describe('DiffViewer', () => {
75+
it('renders "No changes to display" when diffFiles is empty', () => {
76+
render(<DiffViewer diffFiles={[]} selectedFile={null} />);
77+
expect(screen.getByText('No changes to display')).toBeInTheDocument();
78+
});
79+
80+
it('does not render task chip when no tasks or contextTask', () => {
81+
render(<DiffViewer diffFiles={mockDiffFiles} selectedFile={null} />);
82+
expect(screen.queryByText('Add login')).not.toBeInTheDocument();
83+
expect(screen.queryByText(/View Task/)).not.toBeInTheDocument();
84+
});
85+
86+
it('does not render task chip when tasks is empty and no contextTask', () => {
87+
render(<DiffViewer diffFiles={mockDiffFiles} selectedFile={null} tasks={[]} />);
88+
expect(screen.queryByText(/View Task/)).not.toBeInTheDocument();
89+
});
90+
91+
it('renders task chip via contextTask fallback when no per-file mapping', () => {
92+
render(
93+
<DiffViewer
94+
diffFiles={mockDiffFiles}
95+
selectedFile={null}
96+
tasks={mockTasks}
97+
contextTask={mockTasks[0]}
98+
/>
99+
);
100+
expect(screen.getByText('Add login')).toBeInTheDocument();
101+
});
102+
103+
it('renders task chip via per-file changedFiles mapping', () => {
104+
render(
105+
<DiffViewer
106+
diffFiles={mockDiffFiles}
107+
selectedFile={null}
108+
tasks={mockTasks}
109+
changedFiles={mockChangedFiles}
110+
/>
111+
);
112+
expect(screen.getByText('Add login')).toBeInTheDocument();
113+
});
114+
115+
it('renders requirement ID when task has requirement_ids', () => {
116+
render(
117+
<DiffViewer
118+
diffFiles={mockDiffFiles}
119+
selectedFile={null}
120+
tasks={mockTasks}
121+
contextTask={mockTasks[0]}
122+
/>
123+
);
124+
expect(screen.getByText('REQ: REQ-42')).toBeInTheDocument();
125+
});
126+
127+
it('does not render requirement ID when task has no requirement_ids', () => {
128+
render(
129+
<DiffViewer
130+
diffFiles={mockDiffFiles}
131+
selectedFile={null}
132+
tasks={mockTaskNoReqs}
133+
contextTask={mockTaskNoReqs[0]}
134+
/>
135+
);
136+
expect(screen.queryByText(/REQ:/)).not.toBeInTheDocument();
137+
});
138+
139+
it('renders "View Task" link pointing to /tasks', () => {
140+
render(
141+
<DiffViewer
142+
diffFiles={mockDiffFiles}
143+
selectedFile={null}
144+
tasks={mockTasks}
145+
contextTask={mockTasks[0]}
146+
/>
147+
);
148+
const link = screen.getByText(/View Task/);
149+
expect(link.closest('a')).toHaveAttribute('href', '/tasks');
150+
});
151+
152+
it('clicking "View Task" link does not collapse the file', async () => {
153+
const user = userEvent.setup();
154+
render(
155+
<DiffViewer
156+
diffFiles={mockDiffFiles}
157+
selectedFile={null}
158+
tasks={mockTasks}
159+
contextTask={mockTasks[0]}
160+
/>
161+
);
162+
163+
expect(screen.getByText('const a = 1;')).toBeInTheDocument();
164+
const link = screen.getByText(/View Task/);
165+
await user.click(link);
166+
expect(screen.getByText('const a = 1;')).toBeInTheDocument();
167+
});
168+
});

‎web-ui/__tests__/components/review/FileTreePanel.test.tsx‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
import { render, screen } from '@testing-library/react';
2+
import userEvent from '@testing-library/user-event';
3+
import { FileTreePanel } from '@/components/review/FileTreePanel';
4+
import type { FileChange, Task } from '@/types';
5+
6+
// jsdom doesn't provide ResizeObserver (needed by radix ScrollArea)
7+
global.ResizeObserver = class {
8+
observe() {}
9+
unobserve() {}
10+
disconnect() {}
11+
} as unknown as typeof ResizeObserver;
12+
13+
// ─── Fixtures ───────────────────────────────────────────────────────
14+
15+
const mockFiles: FileChange[] = [
16+
{ path: 'src/foo.ts', change_type: 'modified', insertions: 5, deletions: 2, task_id: 'task-1', task_title: 'Add login' },
17+
{ path: 'src/bar.ts', change_type: 'added', insertions: 10, deletions: 0 },
18+
{ path: 'lib/utils.ts', change_type: 'modified', insertions: 3, deletions: 1, task_id: 'task-2', task_title: 'Fix utils' },
19+
];
20+
21+
const mockTasks: Task[] = [
22+
{ id: 'task-1', title: 'Add login', description: '', status: 'IN_PROGRESS', priority: 1, depends_on: [] },
23+
{ id: 'task-2', title: 'Fix utils', description: '', status: 'READY', priority: 2, depends_on: [] },
24+
];
25+
26+
const defaultProps = {
27+
files: mockFiles,
28+
selectedFile: null,
29+
onFileSelect: jest.fn(),
30+
};
31+
32+
beforeEach(() => {
33+
jest.clearAllMocks();
34+
});
35+
36+
// ─── Tests ──────────────────────────────────────────────────────────
37+
38+
describe('FileTreePanel', () => {
39+
it('renders file list grouped by directory', () => {
40+
render(<FileTreePanel {...defaultProps} />);
41+
expect(screen.getByText('src')).toBeInTheDocument();
42+
expect(screen.getByText('lib')).toBeInTheDocument();
43+
expect(screen.getByText('foo.ts')).toBeInTheDocument();
44+
expect(screen.getByText('bar.ts')).toBeInTheDocument();
45+
expect(screen.getByText('utils.ts')).toBeInTheDocument();
46+
});
47+
48+
it('renders a grouping toggle button when tasks prop has items', () => {
49+
render(<FileTreePanel {...defaultProps} tasks={mockTasks} />);
50+
expect(screen.getByRole('button', { name: /task/i })).toBeInTheDocument();
51+
});
52+
53+
it('does not render a grouping toggle when tasks is empty', () => {
54+
render(<FileTreePanel {...defaultProps} tasks={[]} />);
55+
expect(screen.queryByRole('button', { name: /task/i })).not.toBeInTheDocument();
56+
});
57+
58+
it('does not render a grouping toggle when tasks is undefined', () => {
59+
render(<FileTreePanel {...defaultProps} />);
60+
expect(screen.queryByRole('button', { name: /task/i })).not.toBeInTheDocument();
61+
});
62+
63+
it('groups files under task headers when toggled to task mode', async () => {
64+
const user = userEvent.setup();
65+
render(<FileTreePanel {...defaultProps} tasks={mockTasks} />);
66+
67+
await user.click(screen.getByRole('button', { name: /task/i }));
68+
69+
// Task group headers should appear
70+
expect(screen.getByText('Add login')).toBeInTheDocument();
71+
expect(screen.getByText('Fix utils')).toBeInTheDocument();
72+
expect(screen.getByText('Unassigned')).toBeInTheDocument();
73+
});
74+
75+
it('shows task title badge next to filename in dir mode when file has task_title', () => {
76+
render(<FileTreePanel {...defaultProps} tasks={mockTasks} />);
77+
// In dir mode (default), files with task_title should show a badge
78+
const badges = screen.getAllByText('Add login');
79+
expect(badges.length).toBeGreaterThanOrEqual(1);
80+
});
81+
82+
it('groups untagged files under contextTask when contextTask is provided', async () => {
83+
const user = userEvent.setup();
84+
const contextTask = mockTasks[0]; // 'Add login'
85+
const untaggedFiles: FileChange[] = [
86+
{ path: 'src/untagged.ts', change_type: 'modified', insertions: 1, deletions: 0 },
87+
];
88+
render(
89+
<FileTreePanel
90+
files={untaggedFiles}
91+
selectedFile={null}
92+
onFileSelect={jest.fn()}
93+
tasks={mockTasks}
94+
contextTask={contextTask}
95+
/>
96+
);
97+
98+
await user.click(screen.getByRole('button', { name: /group by task/i }));
99+
100+
// Untagged file should appear under the contextTask group, not 'Unassigned'
101+
expect(screen.getByText('Add login')).toBeInTheDocument();
102+
expect(screen.queryByText('Unassigned')).not.toBeInTheDocument();
103+
});
104+
105+
it('shows contextTask title as badge in dir mode for files without task_title', () => {
106+
const contextTask = mockTasks[0]; // 'Add login'
107+
const untaggedFiles: FileChange[] = [
108+
{ path: 'src/untagged.ts', change_type: 'modified', insertions: 1, deletions: 0 },
109+
];
110+
render(
111+
<FileTreePanel
112+
files={untaggedFiles}
113+
selectedFile={null}
114+
onFileSelect={jest.fn()}
115+
tasks={mockTasks}
116+
contextTask={contextTask}
117+
/>
118+
);
119+
120+
expect(screen.getByText('Add login')).toBeInTheDocument();
121+
});
122+
123+
it('task groups are collapsible', async () => {
124+
const user = userEvent.setup();
125+
render(<FileTreePanel {...defaultProps} tasks={mockTasks} />);
126+
127+
// Switch to task mode
128+
await user.click(screen.getByRole('button', { name: /task/i }));
129+
130+
// Files should be visible
131+
expect(screen.getByText('foo.ts')).toBeInTheDocument();
132+
133+
// Click on task group header to collapse
134+
const addLoginHeader = screen.getByRole('button', { name: /collapse add login/i });
135+
await user.click(addLoginHeader);
136+
137+
// foo.ts should no longer be visible
138+
expect(screen.queryByText('foo.ts')).not.toBeInTheDocument();
139+
});
140+
});

‎web-ui/src/app/review/page.tsx‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@ import { useState, useEffect, useCallback, useMemo, useRef } from 'react';
44
import Link from 'next/link';
55
import useSWR from 'swr';
66
import { getSelectedWorkspacePath } from '@/lib/workspace-storage';
7-
import { reviewApi, gatesApi, gitApi, prApi } from '@/lib/api';
7+
import { reviewApi, gatesApi, gitApi, prApi, tasksApi } from '@/lib/api';
88
import { parseDiff, getFilePath } from '@/lib/diffParser';
99
import type {
1010
DiffStatsResponse,
11+
TaskListResponse,
12+
Task,
1113
GateResult,
1214
ApiError,
1315
} from '@/types';
@@ -62,6 +64,20 @@ export default function ReviewPage() {
6264
() => reviewApi.getDiff(workspacePath!)
6365
);
6466

67+
// Fetch tasks for context (optional — errors degrade gracefully, no banner)
68+
const { data: tasksData } = useSWR<TaskListResponse>(
69+
workspacePath ? `/api/v2/tasks?workspace_path=${workspacePath}` : null,
70+
() => tasksApi.getAll(workspacePath!),
71+
{ onError: () => {} }
72+
);
73+
74+
// Auto-select single IN_PROGRESS task as context
75+
const inProgressTasks = useMemo(
76+
() => (tasksData?.tasks ?? []).filter((t: Task) => t.status === 'IN_PROGRESS'),
77+
[tasksData?.tasks]
78+
);
79+
const contextTask = inProgressTasks.length === 1 ? inProgressTasks[0] : null;
80+
6581
// Parse diff into structured files
6682
const diffFiles = useMemo(
6783
() => (diffData?.diff ? parseDiff(diffData.diff) : []),
@@ -280,12 +296,17 @@ export default function ReviewPage() {
280296
files={diffData?.changed_files ?? []}
281297
selectedFile={selectedFile}
282298
onFileSelect={handleFileSelect}
299+
tasks={tasksData?.tasks ?? []}
300+
contextTask={contextTask}
283301
/>
284302

285303
{/* Diff viewer (center) */}
286304
<DiffViewer
287305
diffFiles={diffFiles}
288306
selectedFile={selectedFile}
307+
tasks={tasksData?.tasks ?? []}
308+
contextTask={contextTask}
309+
changedFiles={diffData?.changed_files ?? []}
289310
/>
290311

291312
{/* Commit panel (right sidebar) */}

0 commit comments

Comments
 (0)