Skip to content

Commit 429bf11

Browse files
authored
feat(pr-history): Report Glitch action and REQ→PR back-link (#573)
## Summary - Added [Report Glitch] button on each merged PR row in the PR History panel - Button fetches PR changed files via new GET /api/v2/pr/{number}/files endpoint - Opens CaptureGlitchModal pre-populated with scope (files), source (production), and PR reference - REQs created from PRs store the GitHub PR URL as source_issue - REQ detail page renders source_issue as a clickable GitHub link with strict URL validation - Full pagination support for PRs with 100+ changed files ## Validation - Review feedback: All addressed (2 rounds — claude-review + CodeRabbit) - Tests: 783 frontend + 32 backend all passing - CI: All checks green - Linting: Clean (ruff + npm build) Closes #573
1 parent 2df7918 commit 429bf11

12 files changed

Lines changed: 425 additions & 11 deletions

File tree

codeframe/git/github_integration.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,36 @@ async def close_pull_request(self, pr_number: int) -> bool:
378378
logger.info(f"Closed PR #{pr_number}")
379379
return data.get("state") == "closed"
380380

381+
async def get_pr_files(self, pr_number: int) -> List[str]:
382+
"""Get the list of files changed in a pull request.
383+
384+
Paginates through all pages (100 per page) to ensure completeness.
385+
386+
Args:
387+
pr_number: PR number
388+
389+
Returns:
390+
List of filenames changed in the PR
391+
392+
Raises:
393+
GitHubAPIError: If API error occurs
394+
"""
395+
files: List[str] = []
396+
page = 1
397+
while True:
398+
endpoint = (
399+
f"/repos/{self.owner}/{self.repo_name}/pulls/{pr_number}/files"
400+
f"?per_page=100&page={page}"
401+
)
402+
data = await self._make_request(method="GET", endpoint=endpoint)
403+
if not isinstance(data, list) or not data:
404+
break
405+
files.extend(f["filename"] for f in data)
406+
if len(data) < 100:
407+
break
408+
page += 1
409+
return files
410+
381411
async def get_pr_ci_checks(
382412
self,
383413
pr_number: int,

codeframe/ui/routers/pr_v2.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,12 @@ class PRHistoryItem(BaseModel):
122122
proof_snapshot: Optional[ProofSnapshotOut]
123123

124124

125+
class PRFilesResponse(BaseModel):
126+
"""Response for PR changed files."""
127+
128+
files: list[str]
129+
130+
125131
class PRHistoryResponse(BaseModel):
126132
"""Response for PR history list."""
127133

@@ -390,6 +396,48 @@ async def get_pr_history(
390396
await client.close()
391397

392398

399+
@router.get("/{pr_number}/files", response_model=PRFilesResponse)
400+
@rate_limit_standard()
401+
async def get_pr_files(
402+
request: Request,
403+
pr_number: int,
404+
workspace: Workspace = Depends(get_v2_workspace),
405+
) -> PRFilesResponse:
406+
"""Get the list of files changed in a pull request.
407+
408+
Args:
409+
pr_number: PR number
410+
workspace: v2 Workspace (for context)
411+
412+
Returns:
413+
List of changed filenames
414+
"""
415+
client = _get_github_client()
416+
try:
417+
files = await client.get_pr_files(pr_number)
418+
return PRFilesResponse(files=files)
419+
except GitHubAPIError as e:
420+
if e.status_code == 404:
421+
raise HTTPException(
422+
status_code=404,
423+
detail=api_error("PR not found", ErrorCodes.NOT_FOUND, f"No PR #{pr_number}"),
424+
)
425+
raise HTTPException(
426+
status_code=e.status_code,
427+
detail=api_error("GitHub API error", ErrorCodes.EXECUTION_FAILED, e.message),
428+
)
429+
except HTTPException:
430+
raise
431+
except Exception as e:
432+
logger.error(f"Failed to get files for PR #{pr_number}: {e}", exc_info=True)
433+
raise HTTPException(
434+
status_code=500,
435+
detail=api_error("Failed to get PR files", ErrorCodes.EXECUTION_FAILED, str(e)),
436+
)
437+
finally:
438+
await client.close()
439+
440+
393441
@router.get("/{pr_number}", response_model=PRResponse)
394442
@rate_limit_standard()
395443
async def get_pull_request(

tests/ui/test_pr_history.py

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
"""Tests for GET /api/v2/pr/history endpoint (pr_v2 router).
1+
"""Tests for PR v2 router endpoints: history and files.
22
3-
These tests verify the PR history endpoint by mocking GitHubIntegration
4-
so no real GitHub API calls are made.
3+
These tests verify the PR history and PR files endpoints by mocking
4+
GitHubIntegration so no real GitHub API calls are made.
55
"""
66

77
import shutil
@@ -254,3 +254,54 @@ def test_author_included(self, test_client):
254254

255255
assert resp.status_code == 200
256256
assert resp.json()["pull_requests"][0]["author"] == "bob"
257+
258+
259+
class TestGetPrFiles:
260+
"""Tests for GET /api/v2/pr/{pr_number}/files."""
261+
262+
def test_returns_file_list(self, test_client):
263+
"""Endpoint returns the list of changed files for a PR."""
264+
mock_client = _make_mock_client()
265+
mock_client.get_pr_files = AsyncMock(return_value=["src/app.py", "tests/test_app.py"])
266+
267+
with patch("codeframe.ui.routers.pr_v2._get_github_client", return_value=mock_client):
268+
resp = test_client.get("/api/v2/pr/42/files?workspace_path=/tmp")
269+
270+
assert resp.status_code == 200
271+
body = resp.json()
272+
assert body["files"] == ["src/app.py", "tests/test_app.py"]
273+
274+
def test_returns_empty_list(self, test_client):
275+
"""Endpoint returns empty list when PR has no file changes."""
276+
mock_client = _make_mock_client()
277+
mock_client.get_pr_files = AsyncMock(return_value=[])
278+
279+
with patch("codeframe.ui.routers.pr_v2._get_github_client", return_value=mock_client):
280+
resp = test_client.get("/api/v2/pr/1/files?workspace_path=/tmp")
281+
282+
assert resp.status_code == 200
283+
assert resp.json()["files"] == []
284+
285+
def test_pr_not_found(self, test_client):
286+
"""Endpoint returns 404 when PR does not exist."""
287+
from codeframe.git.github_integration import GitHubAPIError
288+
289+
mock_client = _make_mock_client()
290+
mock_client.get_pr_files = AsyncMock(
291+
side_effect=GitHubAPIError(404, "Not Found"),
292+
)
293+
294+
with patch("codeframe.ui.routers.pr_v2._get_github_client", return_value=mock_client):
295+
resp = test_client.get("/api/v2/pr/99999/files?workspace_path=/tmp")
296+
297+
assert resp.status_code == 404
298+
299+
def test_client_close_called(self, test_client):
300+
"""Client.close() is always called."""
301+
mock_client = _make_mock_client()
302+
mock_client.get_pr_files = AsyncMock(return_value=[])
303+
304+
with patch("codeframe.ui.routers.pr_v2._get_github_client", return_value=mock_client):
305+
test_client.get("/api/v2/pr/1/files?workspace_path=/tmp")
306+
307+
mock_client.close.assert_awaited_once()

tests/unit/test_github_integration.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,55 @@ async def test_rate_limit_error(self, github):
299299
assert exc_info.value.status_code == 403
300300

301301

302+
@pytest.mark.v2
303+
class TestGetPrFiles:
304+
"""Tests for GitHubIntegration.get_pr_files."""
305+
306+
@pytest.fixture
307+
def github(self):
308+
return GitHubIntegration(
309+
token="ghp_test_token_12345",
310+
repo="owner/test-repo",
311+
)
312+
313+
@pytest.mark.asyncio
314+
async def test_returns_list_of_filenames(self, github):
315+
"""get_pr_files returns a list of filename strings."""
316+
mock_response = [
317+
{"filename": "src/app.py", "status": "modified"},
318+
{"filename": "tests/test_app.py", "status": "added"},
319+
]
320+
321+
with patch.object(github, "_make_request", new_callable=AsyncMock) as mock_request:
322+
mock_request.return_value = mock_response
323+
files = await github.get_pr_files(42)
324+
325+
assert files == ["src/app.py", "tests/test_app.py"]
326+
mock_request.assert_called_once()
327+
call_kwargs = mock_request.call_args.kwargs
328+
assert call_kwargs["method"] == "GET"
329+
assert "/pulls/42/files" in call_kwargs["endpoint"]
330+
assert "per_page=100" in call_kwargs["endpoint"]
331+
332+
@pytest.mark.asyncio
333+
async def test_returns_empty_list_for_no_files(self, github):
334+
"""get_pr_files returns empty list when PR has no file changes."""
335+
with patch.object(github, "_make_request", new_callable=AsyncMock) as mock_request:
336+
mock_request.return_value = []
337+
files = await github.get_pr_files(1)
338+
339+
assert files == []
340+
341+
@pytest.mark.asyncio
342+
async def test_propagates_api_error(self, github):
343+
"""get_pr_files propagates GitHubAPIError."""
344+
with patch.object(github, "_make_request", new_callable=AsyncMock) as mock_request:
345+
mock_request.side_effect = GitHubAPIError(404, "Not Found")
346+
with pytest.raises(GitHubAPIError) as exc_info:
347+
await github.get_pr_files(99999)
348+
assert exc_info.value.status_code == 404
349+
350+
302351
class TestGitHubAPIError:
303352
"""Tests for GitHubAPIError exception."""
304353

web-ui/src/__tests__/components/proof/CaptureGlitchModal.test.tsx

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,4 +234,60 @@ describe('CaptureGlitchModal', () => {
234234
expect((screen.getByLabelText(/Description/i) as HTMLTextAreaElement).value).toBe('');
235235
});
236236
});
237+
238+
describe('pre-population from PR', () => {
239+
const PR_PROPS = {
240+
...DEFAULT_PROPS,
241+
prNumber: 42,
242+
prTitle: 'Fix login timeout',
243+
prUrl: 'https://github.com/owner/repo/pull/42',
244+
initialScope: 'src/auth.py\nsrc/utils.py',
245+
};
246+
247+
it('pre-fills description with PR reference when prNumber is provided', () => {
248+
setup(PR_PROPS);
249+
const textarea = screen.getByLabelText(/Description/i) as HTMLTextAreaElement;
250+
expect(textarea.value).toBe('Reported from PR #42: Fix login timeout');
251+
});
252+
253+
it('pre-fills scope with initialScope', () => {
254+
setup(PR_PROPS);
255+
const textarea = screen.getByLabelText(/Scope/i) as HTMLTextAreaElement;
256+
expect(textarea.value).toBe('src/auth.py\nsrc/utils.py');
257+
});
258+
259+
it('includes source_issue in submission payload', async () => {
260+
mockCapture.mockResolvedValue(MOCK_REQ);
261+
setup(PR_PROPS);
262+
263+
// Fill required fields
264+
fireEvent.click(screen.getByRole('checkbox', { name: 'unit' }));
265+
fireEvent.click(screen.getByRole('button', { name: /Capture Glitch/i }));
266+
267+
await waitFor(() => {
268+
expect(mockCapture).toHaveBeenCalledWith(
269+
WORKSPACE,
270+
expect.objectContaining({
271+
source_issue: 'https://github.com/owner/repo/pull/42',
272+
})
273+
);
274+
});
275+
});
276+
277+
it('does not include source_issue when prUrl is not provided', async () => {
278+
mockCapture.mockResolvedValue(MOCK_REQ);
279+
setup();
280+
281+
fireEvent.change(screen.getByLabelText(/Description/i), {
282+
target: { value: 'Something broke' },
283+
});
284+
fireEvent.click(screen.getByRole('checkbox', { name: 'unit' }));
285+
fireEvent.click(screen.getByRole('button', { name: /Capture Glitch/i }));
286+
287+
await waitFor(() => {
288+
const callArgs = mockCapture.mock.calls[0][1];
289+
expect(callArgs.source_issue).toBeUndefined();
290+
});
291+
});
292+
});
237293
});

web-ui/src/__tests__/components/proof/ProofDetailPage.test.tsx

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,3 +282,42 @@ describe('ProofDetailPage — combined filters', () => {
282282
expect(evidenceRows()).toHaveLength(1);
283283
});
284284
});
285+
286+
describe('ProofDetailPage — source_issue rendering', () => {
287+
beforeEach(() => jest.clearAllMocks());
288+
289+
function setupWithReq(reqOverrides: Partial<ProofRequirement>) {
290+
const req = { ...REQ, ...reqOverrides };
291+
mockGetWorkspace.mockReturnValue(WORKSPACE);
292+
mockUseSWR.mockImplementation((key: unknown) => {
293+
if (!key) {
294+
return { data: undefined, error: undefined, isLoading: false, mutate: jest.fn() } as unknown as ReturnType<typeof useSWR>;
295+
}
296+
const k = String(key);
297+
if (k.includes('/evidence')) {
298+
return { data: [], error: undefined, isLoading: false, mutate: jest.fn() } as unknown as ReturnType<typeof useSWR>;
299+
}
300+
return { data: req, error: undefined, isLoading: false, mutate: jest.fn() } as unknown as ReturnType<typeof useSWR>;
301+
});
302+
render(<ProofDetailPage />);
303+
}
304+
305+
it('renders source_issue as a clickable link when it is a GitHub URL', () => {
306+
setupWithReq({ source_issue: 'https://github.com/owner/repo/pull/42' });
307+
const link = screen.getByRole('link', { name: /github\.com/i });
308+
expect(link).toHaveAttribute('href', 'https://github.com/owner/repo/pull/42');
309+
expect(link).toHaveAttribute('target', '_blank');
310+
});
311+
312+
it('renders source_issue as plain text when it is not a URL', () => {
313+
setupWithReq({ source_issue: 'JIRA-123' });
314+
expect(screen.getByText(/JIRA-123/)).toBeInTheDocument();
315+
expect(screen.queryByRole('link', { name: /JIRA-123/ })).not.toBeInTheDocument();
316+
});
317+
318+
it('does not render source_issue when it is null', () => {
319+
setupWithReq({ source_issue: null });
320+
expect(screen.queryByText(/Source PR/)).not.toBeInTheDocument();
321+
expect(screen.queryByText(/Issue:/)).not.toBeInTheDocument();
322+
});
323+
});

web-ui/src/__tests__/components/review/PRHistoryPanel.test.tsx

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,20 @@
11
import React from 'react';
2-
import { render, screen, fireEvent } from '@testing-library/react';
2+
import { render, screen, fireEvent, waitFor } from '@testing-library/react';
33
import useSWR from 'swr';
44
import { PRHistoryPanel } from '@/components/review/PRHistoryPanel';
5+
import { prApi } from '@/lib/api';
56
import type { PRHistoryResponse } from '@/types';
67

78
// ── Mocks ─────────────────────────────────────────────────────────────────
89

910
jest.mock('swr');
1011
jest.mock('@/lib/api', () => ({
11-
prApi: { getHistory: jest.fn() },
12+
prApi: { getHistory: jest.fn(), getFiles: jest.fn() },
13+
proofApi: { capture: jest.fn() },
1214
}));
1315

16+
const mockGetFiles = prApi.getFiles as jest.MockedFunction<typeof prApi.getFiles>;
17+
1418
const mockUseSWR = useSWR as jest.MockedFunction<typeof useSWR>;
1519

1620
// ── Helpers ───────────────────────────────────────────────────────────────
@@ -233,4 +237,40 @@ describe('PRHistoryPanel', () => {
233237
expect(key).toBeNull();
234238
});
235239
});
240+
241+
describe('Report Glitch button', () => {
242+
it('renders a Report Glitch button for each PR row', () => {
243+
withData(SAMPLE_HISTORY);
244+
render(<PRHistoryPanel workspacePath={WORKSPACE} />);
245+
246+
const buttons = screen.getAllByRole('button', { name: /Report Glitch/i });
247+
expect(buttons).toHaveLength(2);
248+
});
249+
250+
it('fetches PR files when Report Glitch is clicked', async () => {
251+
mockGetFiles.mockResolvedValue(['src/auth.py', 'tests/test_auth.py']);
252+
withData(SAMPLE_HISTORY);
253+
render(<PRHistoryPanel workspacePath={WORKSPACE} />);
254+
255+
const buttons = screen.getAllByRole('button', { name: /Report Glitch/i });
256+
fireEvent.click(buttons[0]);
257+
258+
await waitFor(() => {
259+
expect(mockGetFiles).toHaveBeenCalledWith(WORKSPACE, 10);
260+
});
261+
});
262+
263+
it('opens the capture modal after files are fetched', async () => {
264+
mockGetFiles.mockResolvedValue(['src/auth.py']);
265+
withData(SAMPLE_HISTORY);
266+
render(<PRHistoryPanel workspacePath={WORKSPACE} />);
267+
268+
const buttons = screen.getAllByRole('button', { name: /Report Glitch/i });
269+
fireEvent.click(buttons[0]);
270+
271+
await waitFor(() => {
272+
expect(screen.getByRole('heading', { name: 'Capture Glitch' })).toBeInTheDocument();
273+
});
274+
});
275+
});
236276
});

0 commit comments

Comments
 (0)