diff --git a/frontend/index.html b/frontend/index.html index 4de7e07..2308b95 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -71,6 +71,14 @@ textarea { resize: vertical; min-height: 120px; } .review-sheet-header { display:flex; align-items:center; justify-content:space-between; gap:10px; flex-wrap:wrap; } .review-sheet-body { white-space:pre-wrap; overflow-wrap:anywhere; } .review-file, .review-history { padding:8px 0; border-bottom:1px solid #1b2d45; overflow-wrap:anywhere; } +.review-file-toggle { min-height:44px; width:100%; display:flex; align-items:flex-start; justify-content:space-between; gap:8px; text-align:left; } +.review-file-toggle strong { overflow-wrap:anywhere; } +.review-diff { overflow-x:auto; max-width:100%; margin-top:8px; white-space:pre; } +.review-diff-line { display:block; min-width:max-content; } +.review-diff-line.hunk { color:#93c5fd; } +.review-diff-line.added { color:#86efac; background:rgba(34,197,94,.09); } +.review-diff-line.removed { color:#fca5a5; background:rgba(239,68,68,.09); } +.review-diff-note, .review-diff-empty { display:block; padding:8px; color:#fcd34d; white-space:normal; } .review-action { min-height:44px; } @media (max-width: 600px) { header { align-items:flex-start; } @@ -373,10 +381,15 @@ textarea { resize: vertical; min-height: 120px; } if (selectedReview !== item) return; qs('#review-sheet-body').textContent = detail.body || 'No description provided.'; qs('#review-ci-state').textContent = 'CI ' + (detail.ci_state || 'unknown'); - qs('#review-files').innerHTML = (detail.files || []).length ? detail.files.map(file => - '
' + escapeHtml(file.filename || 'Unknown file') + '
' + - escapeHtml(file.status || 'changed') + ' · +' + Number(file.additions || 0) + ' / −' + Number(file.deletions || 0) + '
' + qs('#review-files').innerHTML = (detail.files || []).length ? detail.files.map((file, index) => + createReviewController.renderDiffFile(file, index, escapeHtml) ).join('') : '
No changed files reported.
'; + document.querySelectorAll('.review-file-toggle').forEach(button => { + button.addEventListener('click', () => { + const panel = document.getElementById(button.getAttribute('aria-controls')); + if (panel) createReviewController.toggleDiff(button, panel); + }); + }); qs('#review-history').innerHTML = (detail.reviews || []).length ? detail.reviews.map(review => '
' + escapeHtml(review.user?.login || 'Reviewer') + ' · ' + escapeHtml(review.state || 'commented') + (review.body ? '
' + escapeHtml(review.body) + '
' : '') + '
' diff --git a/frontend/review-sheet.js b/frontend/review-sheet.js index 3ece213..cbc853c 100644 --- a/frontend/review-sheet.js +++ b/frontend/review-sheet.js @@ -15,6 +15,44 @@ function createReviewController({ fetchJson }) { return { load }; } +function diffLineClass(line) { + if (line.startsWith('@@')) return 'hunk'; + if (line.startsWith('+')) return 'added'; + if (line.startsWith('-')) return 'removed'; + return 'context'; +} + +function renderDiffFile(file, index, escapeHtml) { + const panelId = 'review-diff-' + index; + let preview; + if (file.diff_available) { + const lines = (file.diff_lines || []).map(line => + '' + + escapeHtml(String(line)) + '' + ).join(''); + preview = ''; + } else { + const message = file.diff_binary ? 'Binary file · preview unavailable.' : 'Diff preview unavailable.'; + preview = ''; + } + return '
' + preview + '
'; +} + +function toggleDiff(button, panel) { + const expanded = button.getAttribute('aria-expanded') === 'true'; + button.setAttribute('aria-expanded', String(!expanded)); + panel.hidden = expanded; +} + +createReviewController.renderDiffFile = renderDiffFile; +createReviewController.toggleDiff = toggleDiff; + if (typeof module !== 'undefined' && module.exports) { module.exports = createReviewController; } diff --git a/src/gitea_proxy.py b/src/gitea_proxy.py index 1313f9b..7c41807 100644 --- a/src/gitea_proxy.py +++ b/src/gitea_proxy.py @@ -1,10 +1,13 @@ import os +import shlex from typing import Any import httpx GITEA_URL = os.getenv("GITEA_URL", "http://127.0.0.1:3000").rstrip("/") GITEA_TOKEN = os.getenv("GITEA_TOKEN", "") +REVIEW_DIFF_MAX_BYTES = 64 * 1024 +REVIEW_DIFF_MAX_LINES = 400 def _auth() -> dict[str, str]: @@ -21,6 +24,66 @@ async def fetch(path: str) -> Any: return r.json() +async def fetch_text(path: str, max_bytes: int) -> tuple[str, bool]: + chunks: list[bytes] = [] + size = 0 + truncated = False + async with httpx.AsyncClient(base_url=GITEA_URL, timeout=10) as client: + async with client.stream( + "GET", f"/api/v1/{path}", headers={**_auth(), "Accept": "text/plain"} + ) as response: + response.raise_for_status() + async for chunk in response.aiter_bytes(): + remaining = max_bytes - size + if len(chunk) > remaining: + chunks.append(chunk[:remaining]) + truncated = True + break + chunks.append(chunk) + size += len(chunk) + return b"".join(chunks).decode("utf-8", errors="replace"), truncated + + +def _diff_previews(diff: str, stream_truncated: bool) -> dict[str, dict]: + previews: dict[str, dict] = {} + current: dict | None = None + in_hunk = False + remaining = REVIEW_DIFF_MAX_LINES + for line in diff.splitlines(): + if line.startswith("diff --git "): + try: + target = shlex.split(line)[3] + filename = target[2:] if target.startswith("b/") else target + except (IndexError, ValueError): + current = None + continue + current = { + "diff_lines": [], + "diff_available": False, + "diff_binary": False, + "diff_truncated": stream_truncated, + } + previews[filename] = current + in_hunk = False + continue + if current is None: + continue + if line.startswith("Binary files ") or line == "GIT binary patch": + current["diff_binary"] = True + in_hunk = False + continue + if line.startswith("@@"): + in_hunk = True + if in_hunk and not line.startswith("\\ No newline at end of file"): + if remaining: + current["diff_lines"].append(line) + current["diff_available"] = True + remaining -= 1 + else: + current["diff_truncated"] = True + return previews + + async def current_user() -> dict: return await fetch("user") @@ -79,6 +142,10 @@ async def pull_review_detail(repository: str, number: int) -> dict: sha = sha_value if isinstance(sha_value, str) else "" status = await fetch(f"repos/{repository}/commits/{sha}/status") reviews = await fetch(f"{base}/reviews") + diff, diff_truncated = await fetch_text( + f"repos/{repository}/pulls/{number}.diff", REVIEW_DIFF_MAX_BYTES + ) + previews = _diff_previews(diff, diff_truncated) user_value = pull.get("user") user: dict = user_value if isinstance(user_value, dict) else {} normalized_files = [ @@ -87,6 +154,15 @@ async def pull_review_detail(repository: str, number: int) -> dict: "status": file.get("status") or "changed", "additions": file.get("additions") or 0, "deletions": file.get("deletions") or 0, + **previews.get( + file["filename"], + { + "diff_lines": [], + "diff_available": False, + "diff_binary": False, + "diff_truncated": diff_truncated, + }, + ), } for file in (files if isinstance(files, list) else [])[:100] if isinstance(file, dict) and isinstance(file.get("filename"), str) diff --git a/tests/test_gitea_work_search.py b/tests/test_gitea_work_search.py index 1c718d4..df02f16 100644 --- a/tests/test_gitea_work_search.py +++ b/tests/test_gitea_work_search.py @@ -159,6 +159,13 @@ async def test_pull_review_detail_combines_pr_files_status_and_reviews(monkeypat monkeypatch.setattr(gitea_proxy, "fetch", fake_fetch) + async def fake_fetch_text(path, max_bytes): + assert path == "repos/stackchain/api/pulls/7.diff" + assert max_bytes == gitea_proxy.REVIEW_DIFF_MAX_BYTES + return "", False + + monkeypatch.setattr(gitea_proxy, "fetch_text", fake_fetch_text) + detail = await gitea_proxy.pull_review_detail("stackchain/api", 7) assert requested_paths == [ @@ -173,3 +180,55 @@ async def test_pull_review_detail_combines_pr_files_status_and_reviews(monkeypat assert detail["reviews"][0]["state"] == "APPROVED" assert len(detail["files"]) == 1 assert len(detail["reviews"]) == 1 + + +@pytest.mark.anyio +async def test_pull_review_detail_attaches_bounded_per_file_diff_previews(monkeypatch): + async def fake_fetch(path): + if path.endswith("/pulls/7"): + return { + "title": "Review API", + "head": {"sha": "abc123"}, + "user": {"login": "alex"}, + } + if path.endswith("/files"): + return [ + {"filename": "src/api.py", "status": "modified"}, + {"filename": "assets/logo.png", "status": "modified"}, + ] + if path.endswith("/reviews"): + return [] + return {"state": "success"} + + diff = """diff --git a/src/api.py b/src/api.py +index 123..456 100644 +--- a/src/api.py ++++ b/src/api.py +@@ -1,2 +1,3 @@ + context +-old ++new +diff --git a/assets/logo.png b/assets/logo.png +Binary files a/assets/logo.png and b/assets/logo.png differ +""" + + async def fake_fetch_text(path, max_bytes): + assert path == "repos/stackchain/api/pulls/7.diff" + return diff, True + + monkeypatch.setattr(gitea_proxy, "fetch", fake_fetch) + monkeypatch.setattr(gitea_proxy, "fetch_text", fake_fetch_text) + + detail = await gitea_proxy.pull_review_detail("stackchain/api", 7) + + source, binary = detail["files"] + assert source["diff_lines"] == [ + "@@ -1,2 +1,3 @@", + " context", + "-old ", + "+new ", + ] + assert source["diff_truncated"] is True + assert source["diff_available"] is True + assert binary["diff_available"] is False + assert binary["diff_binary"] is True diff --git a/tests/test_my_work.py b/tests/test_my_work.py index ff95ba9..f99dfd9 100644 --- a/tests/test_my_work.py +++ b/tests/test_my_work.py @@ -172,6 +172,35 @@ controller.load({{ repository: 'stackchain/api', number: 7 }}).then(detail => } +def test_review_diff_rows_escape_content_and_toggle_accessibly(): + script = f""" +const reviewSheet = require({json.dumps(str(REVIEW_SHEET))}); +const escapeHtml = value => String(value) + .replaceAll('&', '&').replaceAll('<', '<').replaceAll('>', '>'); +const html = reviewSheet.renderDiffFile({{ + filename: 'src/.py', status: 'modified', additions: 1, deletions: 1, + diff_available: true, diff_truncated: true, + diff_lines: ['@@ -1 +1 @@', '-old ', '+new & safe'] +}}, 2, escapeHtml); +const button = {{ attrs: {{ 'aria-expanded': 'false' }}, getAttribute(k) {{ return this.attrs[k]; }}, setAttribute(k,v) {{ this.attrs[k]=v; }} }}; +const panel = {{ hidden: true }}; +reviewSheet.toggleDiff(button, panel); +process.stdout.write(JSON.stringify({{ html, expanded: button.attrs['aria-expanded'], hidden: panel.hidden }})); +""" + result = subprocess.run( + ["node", "-e", script], check=True, capture_output=True, text=True + ) + output = json.loads(result.stdout) + + assert 'aria-expanded="false"' in output["html"] + assert 'src/<api>.py' in output["html"] + assert '-old <token>' in output["html"] + assert '+new & safe' in output["html"] + assert 'Preview truncated' in output["html"] + assert output["expanded"] == "true" + assert output["hidden"] is False + + @pytest.mark.anyio async def test_review_requests_open_an_accessible_mobile_detail_sheet(): html = await dashboard() @@ -184,6 +213,8 @@ async def test_review_requests_open_an_accessible_mobile_detail_sheet(): assert '@media (max-width: 600px)' in html assert '.review-sheet-panel' in html and 'width:100%' in html assert '.review-action' in html and 'min-height:44px' in html + assert '.review-file-toggle' in html and 'min-height:44px' in html + assert '.review-diff' in html and 'overflow-x:auto' in html @pytest.mark.anyio