Merge pull request 'Preview requested pull request diffs on mobile' (#124)

Closes #123

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 =>
-        '<div class="review-file"><strong>' + escapeHtml(file.filename || 'Unknown file') + '</strong><div class="small">' +
-        escapeHtml(file.status || 'changed') + ' · +' + Number(file.additions || 0) + ' / −' + Number(file.deletions || 0) + '</div></div>'
+      qs('#review-files').innerHTML = (detail.files || []).length ? detail.files.map((file, index) =>
+        createReviewController.renderDiffFile(file, index, escapeHtml)
       ).join('') : '<div>No changed files reported.</div>';
+      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 =>
         '<div class="review-history"><strong>' + escapeHtml(review.user?.login || 'Reviewer') + '</strong> · ' +
         escapeHtml(review.state || 'commented') + (review.body ? '<div class="small">' + escapeHtml(review.body) + '</div>' : '') + '</div>'

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 =>
+      '<span class="review-diff-line ' + diffLineClass(String(line)) + '">' +
+      escapeHtml(String(line)) + '</span>'
+    ).join('');
+    preview = '<pre class="review-diff" id="' + panelId + '" hidden>' + lines +
+      (file.diff_truncated ? '<span class="review-diff-note">Preview truncated · open in Gitea for the full diff.</span>' : '') +
+      '</pre>';
+  } else {
+    const message = file.diff_binary ? 'Binary file · preview unavailable.' : 'Diff preview unavailable.';
+    preview = '<div class="review-diff-empty" id="' + panelId + '" hidden>' + message +
+      (file.diff_truncated ? ' The response was truncated.' : '') + '</div>';
+  }
+  return '<div class="review-file"><button class="review-file-toggle" aria-expanded="false" aria-controls="' + panelId + '">' +
+    '<strong>' + escapeHtml(file.filename || 'Unknown file') + '</strong><span class="small">' +
+    escapeHtml(file.status || 'changed') + ' · +' + Number(file.additions || 0) + ' / −' +
+    Number(file.deletions || 0) + '</span></button>' + preview + '</div>';
+}
+
+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;
 }

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)

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 <value>
++new <value>
+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 <value>",
+        "+new <value>",
+    ]
+    assert source["diff_truncated"] is True
+    assert source["diff_available"] is True
+    assert binary["diff_available"] is False
+    assert binary["diff_binary"] is True

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('&', '&amp;').replaceAll('<', '&lt;').replaceAll('>', '&gt;');
+const html = reviewSheet.renderDiffFile({{
+  filename: 'src/<api>.py', status: 'modified', additions: 1, deletions: 1,
+  diff_available: true, diff_truncated: true,
+  diff_lines: ['@@ -1 +1 @@', '-old <token>', '+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/&lt;api&gt;.py' in output["html"]
+    assert '-old &lt;token&gt;' in output["html"]
+    assert '+new &amp; 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