Compare commits

...
Author SHA1 Message Date
timmy bd0ae619db Merge pull request 'Make mobile pull review loading bounded, concurrent, and retryable' (#128) from timmy/127-review-load-retry into main
CI / lint (push) Successful in 10s
Release / release-candidate (push) Successful in 4s
CI / build-frontend (push) Successful in 5s
2026-08-06 18:25:53 +00:00
timmy 173af9f698 feat: make mobile review loading retryable (#127)
CI / lint (pull_request) Successful in 9s
CI / build-frontend (pull_request) Successful in 5s
2026-08-06 18:25:10 +00:00
rockachopa 3bf635fb89 Merge pull request 'Resume mobile pull review progress across interruptions' (#126)
CI / lint (push) Successful in 10s
Release / release-candidate (push) Successful in 4s
CI / build-frontend (push) Successful in 5s
Closes #125
2026-08-06 17:55:21 +00:00
timmy d8937f0096 feat: resume mobile review progress (#125)
CI / lint (pull_request) Successful in 10s
CI / build-frontend (pull_request) Successful in 5s
2026-08-06 17:54:35 +00:00
timmy c4a9c76781 Merge pull request 'Preview requested pull request diffs on mobile' (#124)
CI / lint (push) Successful in 10s
Release / release-candidate (push) Successful in 4s
CI / build-frontend (push) Successful in 5s
Closes #123
2026-08-06 17:32:26 +00:00
7 changed files with 313 additions and 26 deletions

No files matched your search

+77 -1
View File
@@ -73,6 +73,12 @@ textarea { resize: vertical; min-height: 120px; }
.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-file.reviewed { opacity:.72; }
.review-file.reviewed .review-file-toggle { border-color:#22c55e; }
.review-mark { min-height:44px; width:100%; margin-top:8px; }
.review-mark[aria-pressed="true"] { border-color:#22c55e; background:#123c2a; }
.review-progress-actions { position:sticky; bottom:0; z-index:2; display:flex; align-items:center; justify-content:space-between; gap:10px; margin:8px -4px 0; padding:10px 4px; background:rgba(11,21,38,.96); border-top:1px solid #2a496e; }
.review-progress-actions button { min-height:44px; max-width:100%; }
.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; }
@@ -80,6 +86,7 @@ textarea { resize: vertical; min-height: 120px; }
.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; }
.review-retry { min-height:44px; margin-top:10px; }
@media (max-width: 600px) {
header { align-items:flex-start; }
.my-work { margin:0; }
@@ -216,10 +223,15 @@ textarea { resize: vertical; min-height: 120px; }
<button class="review-action" id="close-review-sheet">Close</button>
</div>
<div id="review-sheet-status" class="small" aria-live="polite">Choose a review request.</div>
<button class="review-retry" id="retry-review-load" hidden>Retry loading review</button>
<p class="review-sheet-body" id="review-sheet-body"></p>
<div class="row"><span class="pill" id="review-ci-state">CI unknown</span><a id="open-review-gitea" href="#" target="_blank" rel="noopener noreferrer">Open in Gitea</a></div>
<h2>Changed files</h2>
<div id="review-files" class="muted"></div>
<div class="review-progress-actions">
<span class="small" id="review-progress" aria-live="polite">0 of 0 files reviewed</span>
<button id="next-unreviewed-review" disabled>Next unreviewed</button>
</div>
<h2>Review history</h2>
<div id="review-history" class="muted"></div>
</section>
@@ -271,6 +283,7 @@ textarea { resize: vertical; min-height: 120px; }
let hasContextSnapshot = false;
let selectedReview = null;
let reviewTrigger = null;
let progress = null;
async function fetchReviewJson(url, options) {
const response = await fetch(url, options);
@@ -364,6 +377,39 @@ textarea { resize: vertical; min-height: 120px; }
});
}
function reviewFileElement(filename) {
return Array.from(document.querySelectorAll('.review-file')).find(
element => element.dataset.reviewFilename === filename
);
}
function showReviewProgress(snapshot) {
const complete = snapshot.total > 0 && snapshot.reviewedCount === snapshot.total;
qs('#review-progress').textContent = complete ?
'All files reviewed — finish in Gitea' :
(snapshot.newHead ? 'New commits detected · ' : '') + snapshot.reviewedCount + ' of ' + snapshot.total + ' files reviewed';
qs('#next-unreviewed-review').disabled = !snapshot.nextFilename;
document.querySelectorAll('.review-mark').forEach(button => {
const reviewed = snapshot.reviewed.includes(button.dataset.reviewFilename);
button.setAttribute('aria-pressed', String(reviewed));
button.textContent = reviewed ? 'Reviewed' : 'Mark reviewed';
button.closest('.review-file')?.classList.toggle('reviewed', reviewed);
});
}
function openNextUnreviewed(snapshot) {
if (!snapshot?.nextFilename) return;
const file = reviewFileElement(snapshot.nextFilename);
if (!file) return;
const toggle = file.querySelector('.review-file-toggle');
const panel = toggle && document.getElementById(toggle.getAttribute('aria-controls'));
if (toggle && panel && toggle.getAttribute('aria-expanded') !== 'true') {
createReviewController.toggleDiff(toggle, panel);
}
file.scrollIntoView({ behavior: 'smooth', block: 'center' });
toggle?.focus();
}
async function openReviewSheet(item, trigger) {
selectedReview = item;
reviewTrigger = trigger;
@@ -371,8 +417,11 @@ textarea { resize: vertical; min-height: 120px; }
qs('#review-sheet-key').textContent = item.key;
qs('#review-sheet-title').textContent = item.title;
qs('#review-sheet-status').textContent = 'Loading review details…';
qs('#retry-review-load').hidden = true;
qs('#review-sheet-body').textContent = '';
qs('#review-files').textContent = '';
qs('#review-progress').textContent = '0 of 0 files reviewed';
qs('#next-unreviewed-review').disabled = true;
qs('#review-history').textContent = '';
qs('#open-review-gitea').href = item.url;
qs('#close-review-sheet').focus();
@@ -390,19 +439,40 @@ textarea { resize: vertical; min-height: 120px; }
if (panel) createReviewController.toggleDiff(button, panel);
});
});
progress = createReviewController.createProgress({
storage: localStorage,
repository: item.repository,
number: item.number,
headSha: detail.head_sha || 'unknown',
files: detail.files || [],
});
document.querySelectorAll('.review-mark').forEach(button => {
button.addEventListener('click', () => {
const snapshot = progress.markReviewed(button.dataset.reviewFilename);
showReviewProgress(snapshot);
openNextUnreviewed(snapshot);
});
});
const progressSnapshot = progress.snapshot();
showReviewProgress(progressSnapshot);
openNextUnreviewed(progressSnapshot);
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>'
).join('') : '<div>No prior reviews.</div>';
qs('#review-sheet-status').textContent = 'Ready to review · by ' + (detail.author || 'unknown author');
} catch (error) {
qs('#review-sheet-status').textContent = error.message + ' Use Open in Gitea or close and retry.';
if (selectedReview !== item) return;
qs('#review-sheet-status').textContent = error.message + ' Retry here or use Open in Gitea.';
qs('#retry-review-load').hidden = false;
qs('#retry-review-load').focus();
}
}
function closeReviewSheet() {
qs('#review-sheet').classList.remove('open');
selectedReview = null;
progress = null;
if (reviewTrigger?.isConnected) reviewTrigger.focus();
}
@@ -513,6 +583,12 @@ textarea { resize: vertical; min-height: 120px; }
document.addEventListener('keydown', (e) => { if ((e.metaKey||e.ctrlKey) && e.key==='k') { e.preventDefault(); qs('#cmd-palette').classList.toggle('open'); if(qs('#cmd-palette').classList.contains('open')){ qs('#cmd-input').focus(); renderCommands(''); } } });
qs('#close-whiteboard').addEventListener('click', () => closeModal('whiteboard-modal'));
qs('#close-review-sheet').addEventListener('click', closeReviewSheet);
qs('#retry-review-load').addEventListener('click', () => {
if (selectedReview) openReviewSheet(selectedReview, reviewTrigger);
});
qs('#next-unreviewed-review').addEventListener('click', () => {
if (progress) openNextUnreviewed(progress.snapshot());
});
const contextPoller = createContextPoller({
fetchContext: fetchContextSnapshot,
+45 -3
View File
@@ -38,10 +38,12 @@ function renderDiffFile(file, index, escapeHtml) {
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">' +
const filename = escapeHtml(file.filename || 'Unknown file');
return '<div class="review-file" data-review-filename="' + filename + '"><button class="review-file-toggle" aria-expanded="false" aria-controls="' + panelId + '">' +
'<strong>' + filename + '</strong><span class="small">' +
escapeHtml(file.status || 'changed') + ' · +' + Number(file.additions || 0) + ' / −' +
Number(file.deletions || 0) + '</span></button>' + preview + '</div>';
Number(file.deletions || 0) + '</span></button>' + preview +
'<button class="review-mark" data-review-filename="' + filename + '" aria-pressed="false">Mark reviewed</button></div>';
}
function toggleDiff(button, panel) {
@@ -50,8 +52,48 @@ function toggleDiff(button, panel) {
panel.hidden = expanded;
}
function createProgress({ storage, repository, number, headSha, files }) {
const filenames = (files || []).map(file => file && file.filename).filter(Boolean);
const key = 'stackchain.review-progress.v1:' + repository + '#' + number + '@' + headSha;
const headKey = 'stackchain.review-progress.v1:' + repository + '#' + number + ':head';
let reviewed = [];
let newHead = false;
try {
const previousHead = storage.getItem(headKey);
newHead = Boolean(previousHead && previousHead !== headSha);
storage.setItem(headKey, headSha);
const saved = JSON.parse(storage.getItem(key) || '[]');
if (Array.isArray(saved)) reviewed = filenames.filter(filename => saved.includes(filename));
} catch (error) {
reviewed = [];
}
function snapshot() {
const pending = filenames.find(filename => !reviewed.includes(filename)) || null;
return {
reviewed: [...reviewed],
reviewedCount: reviewed.length,
total: filenames.length,
nextFilename: pending,
...(newHead ? { newHead: true } : {}),
};
}
function markReviewed(filename) {
if (filenames.includes(filename) && !reviewed.includes(filename)) {
reviewed.push(filename);
reviewed = filenames.filter(item => reviewed.includes(item));
try { storage.setItem(key, JSON.stringify(reviewed)); } catch (error) { /* local progress remains usable */ }
}
return snapshot();
}
return { snapshot, markReviewed };
}
createReviewController.renderDiffFile = renderDiffFile;
createReviewController.toggleDiff = toggleDiff;
createReviewController.createProgress = createProgress;
if (typeof module !== 'undefined' && module.exports) {
module.exports = createReviewController;
+14 -8
View File
@@ -1,3 +1,4 @@
import asyncio
import os
import shlex
from typing import Any
@@ -119,14 +120,15 @@ async def pull_requests() -> list[dict]:
async def is_requested_review(repository: str, number: int) -> bool:
pulls = await pull_requests()
pulls = await fetch(
"repos/issues/search?state=open&review_requested=true&type=pulls&limit=50"
)
return any(
isinstance(pull, dict)
and pull.get("number") == number
and "review_requested" in (pull.get("work_reasons") or [])
and isinstance(pull.get("repository"), dict)
and pull["repository"].get("full_name") == repository
for pull in pulls
for pull in (pulls or [])
)
@@ -135,16 +137,19 @@ async def pull_review_detail(repository: str, number: int) -> dict:
pull = await fetch(base)
if not isinstance(pull, dict):
raise ValueError("Gitea pull request response was not an object")
files = await fetch(f"{base}/files")
head_value = pull.get("head")
head: dict = head_value if isinstance(head_value, dict) else {}
sha_value = head.get("sha")
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
files, status, reviews, diff_result = await asyncio.gather(
fetch(f"{base}/files"),
fetch(f"repos/{repository}/commits/{sha}/status"),
fetch(f"{base}/reviews"),
fetch_text(
f"repos/{repository}/pulls/{number}.diff", REVIEW_DIFF_MAX_BYTES
),
)
diff, diff_truncated = diff_result
previews = _diff_previews(diff, diff_truncated)
user_value = pull.get("user")
user: dict = user_value if isinstance(user_value, dict) else {}
@@ -189,6 +194,7 @@ async def pull_review_detail(repository: str, number: int) -> dict:
"body": pull.get("body") or "",
"url": pull.get("html_url", ""),
"author": user.get("login", ""),
"head_sha": sha,
"ci_state": status.get("state", "unknown") if isinstance(status, dict) else "unknown",
"files": normalized_files,
"reviews": normalized_reviews,
+17 -7
View File
@@ -24,6 +24,7 @@ app = FastAPI(title="Stackchain Dashboard")
CONTEXT_TIMEOUT_SECONDS = 5.0
EVENT_STREAM_TIMEOUT_SECONDS = 5.0
READINESS_TIMEOUT_SECONDS = 5.0
REVIEW_DETAIL_TIMEOUT_SECONDS = 5.0
FRONTEND_DIR = Path(__file__).resolve().parent.parent / "frontend"
@@ -262,19 +263,28 @@ async def event_stream():
@app.get("/api/v1/repos/{owner}/{repo}/pulls/{number}/review")
async def review_detail(owner: str, repo: str, number: int):
try:
async def load_requested_review():
repository = f"{owner}/{repo}"
if not await asyncio.wait_for(
is_requested_review(repository, number),
timeout=CONTEXT_TIMEOUT_SECONDS,
):
if not await is_requested_review(repository, number):
raise HTTPException(status_code=404, detail="Review request not found")
return await pull_review_detail(repository, number)
try:
return await asyncio.wait_for(
pull_review_detail(repository, number),
timeout=CONTEXT_TIMEOUT_SECONDS,
load_requested_review(), timeout=REVIEW_DETAIL_TIMEOUT_SECONDS
)
except HTTPException:
raise
except TimeoutError:
return JSONResponse(
{"error": "Pull request review details timed out. Please retry."},
status_code=503,
headers={
"Retry-After": str(
max(1, math.ceil(REVIEW_DETAIL_TIMEOUT_SECONDS))
)
},
)
except Exception:
return JSONResponse(
{"error": "Pull request review details are temporarily unavailable"},
+55 -7
View File
@@ -1,6 +1,8 @@
import pytest
import asyncio
import json
import pytest
from src import gitea_proxy
from src import main
@@ -54,26 +56,32 @@ async def test_pull_requests_merge_assignment_and_review_responsibilities(monkey
@pytest.mark.anyio
async def test_requested_review_guard_matches_repository_number_and_reason(monkeypatch):
async def pulls():
async def test_requested_review_guard_uses_only_dedicated_review_search(monkeypatch):
requested_paths = []
async def fake_fetch(path):
requested_paths.append(path)
return [
{
"number": 7,
"repository": {"full_name": "stackchain/api"},
"work_reasons": ["review_requested"],
},
{
"number": 8,
"repository": {"full_name": "stackchain/api"},
"work_reasons": ["assigned_to_me"],
},
]
monkeypatch.setattr(gitea_proxy, "pull_requests", pulls)
monkeypatch.setattr(gitea_proxy, "fetch", fake_fetch)
assert await gitea_proxy.is_requested_review("stackchain/api", 7) is True
assert await gitea_proxy.is_requested_review("stackchain/api", 8) is False
assert await gitea_proxy.is_requested_review("stackchain/api", 9) is False
assert await gitea_proxy.is_requested_review("stackchain/private", 7) is False
assert requested_paths == [
"repos/issues/search?state=open&review_requested=true&type=pulls&limit=50",
"repos/issues/search?state=open&review_requested=true&type=pulls&limit=50",
"repos/issues/search?state=open&review_requested=true&type=pulls&limit=50",
]
@pytest.mark.anyio
@@ -175,6 +183,7 @@ async def test_pull_review_detail_combines_pr_files_status_and_reviews(monkeypat
"repos/stackchain/api/pulls/7/reviews",
]
assert detail["author"] == "alex"
assert detail["head_sha"] == "abc123"
assert detail["ci_state"] == "success"
assert detail["files"][0]["filename"] == "src/api.py"
assert detail["reviews"][0]["state"] == "APPROVED"
@@ -182,6 +191,45 @@ async def test_pull_review_detail_combines_pr_files_status_and_reviews(monkeypat
assert len(detail["reviews"]) == 1
@pytest.mark.anyio
async def test_pull_review_detail_fetches_independent_head_resources_concurrently(monkeypatch):
started = {name: asyncio.Event() for name in ("files", "status", "reviews", "diff")}
release = asyncio.Event()
async def wait_for_release(name, result):
started[name].set()
await release.wait()
return result
async def fake_fetch(path):
if path.endswith("/pulls/7"):
return {"head": {"sha": "abc123"}, "user": {"login": "alex"}}
if path.endswith("/files"):
return await wait_for_release("files", [])
if path.endswith("/reviews"):
return await wait_for_release("reviews", [])
return await wait_for_release("status", {"state": "success"})
async def fake_fetch_text(path, max_bytes):
return await wait_for_release("diff", ("", False))
monkeypatch.setattr(gitea_proxy, "fetch", fake_fetch)
monkeypatch.setattr(gitea_proxy, "fetch_text", fake_fetch_text)
detail_task = asyncio.create_task(
gitea_proxy.pull_review_detail("stackchain/api", 7)
)
try:
await asyncio.wait_for(
asyncio.gather(*(event.wait() for event in started.values())), timeout=1
)
finally:
release.set()
detail = await detail_task
assert detail["head_sha"] == "abc123"
@pytest.mark.anyio
async def test_pull_review_detail_attaches_bounded_per_file_diff_previews(monkeypatch):
async def fake_fetch(path):
+78
View File
@@ -197,10 +197,69 @@ process.stdout.write(JSON.stringify({{ html, expanded: button.attrs['aria-expand
assert '-old &lt;token&gt;' in output["html"]
assert '+new &amp; safe' in output["html"]
assert 'Preview truncated' in output["html"]
assert 'class="review-mark"' in output["html"]
assert 'data-review-filename="src/&lt;api&gt;.py"' in output["html"]
assert 'Mark reviewed' in output["html"]
assert output["expanded"] == "true"
assert output["hidden"] is False
def test_review_progress_is_explicit_and_restores_for_the_same_head_sha():
script = f"""
const reviewSheet = require({json.dumps(str(REVIEW_SHEET))});
const values = new Map();
const storage = {{
getItem(key) {{ return values.has(key) ? values.get(key) : null; }},
setItem(key, value) {{ values.set(key, value); }}
}};
const options = {{
storage, repository: 'stackchain/api', number: 7, headSha: 'abc123',
files: [{{filename:'src/a.py'}}, {{filename:'src/b.py'}}, {{filename:'README.md'}}]
}};
const first = reviewSheet.createProgress(options);
const before = first.snapshot();
const marked = first.markReviewed('src/a.py');
const restored = reviewSheet.createProgress(options).snapshot();
process.stdout.write(JSON.stringify({{ before, marked, restored }}));
"""
result = subprocess.run(
["node", "-e", script], check=True, capture_output=True, text=True
)
output = json.loads(result.stdout)
assert output["before"] == {
"reviewed": [], "reviewedCount": 0, "total": 3, "nextFilename": "src/a.py"
}
assert output["marked"] == {
"reviewed": ["src/a.py"], "reviewedCount": 1, "total": 3,
"nextFilename": "src/b.py",
}
assert output["restored"] == output["marked"]
def test_review_progress_resets_when_new_commits_change_the_head_sha():
script = f"""
const reviewSheet = require({json.dumps(str(REVIEW_SHEET))});
const values = new Map();
const storage = {{
getItem(key) {{ return values.has(key) ? values.get(key) : null; }},
setItem(key, value) {{ values.set(key, value); }}
}};
const base = {{ storage, repository: 'stackchain/api', number: 7, files: [{{filename:'src/a.py'}}] }};
reviewSheet.createProgress({{...base, headSha:'abc123'}}).markReviewed('src/a.py');
const changed = reviewSheet.createProgress({{...base, headSha:'def456'}}).snapshot();
process.stdout.write(JSON.stringify(changed));
"""
result = subprocess.run(
["node", "-e", script], check=True, capture_output=True, text=True
)
assert json.loads(result.stdout) == {
"reviewed": [], "reviewedCount": 0, "total": 1,
"nextFilename": "src/a.py", "newHead": True,
}
@pytest.mark.anyio
async def test_review_requests_open_an_accessible_mobile_detail_sheet():
html = await dashboard()
@@ -214,6 +273,13 @@ async def test_review_requests_open_an_accessible_mobile_detail_sheet():
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-mark' in html and 'min-height:44px' in html
assert 'id="review-progress"' in html and 'aria-live="polite"' in html
assert 'id="next-unreviewed-review"' in html
assert '.review-progress-actions' in html and 'position:sticky' in html
assert "createReviewController.createProgress" in html
assert "progress.markReviewed" in html
assert "scrollIntoView" in html
assert '.review-diff' in html and 'overflow-x:auto' in html
@@ -227,3 +293,15 @@ async def test_review_sheet_loads_details_and_preserves_safe_gitea_handoff():
assert "review-history" in html
assert "open-review-gitea" in html
assert "reviewController.submit" not in html
@pytest.mark.anyio
async def test_mobile_review_failure_offers_an_in_place_retry_for_the_same_item():
html = await dashboard()
assert 'id="retry-review-load"' in html
assert '.review-retry' in html and 'min-height:44px' in html
assert "qs('#retry-review-load').hidden = false" in html
assert "qs('#retry-review-load').hidden = true" in html
assert "openReviewSheet(selectedReview, reviewTrigger)" in html
assert "qs('#retry-review-load').focus()" in html
+27
View File
@@ -1,3 +1,5 @@
import asyncio
import httpx
import pytest
@@ -48,6 +50,31 @@ async def test_review_detail_rejects_pulls_not_requested_from_service_user(monke
assert response.headers["cache-control"] == "no-store"
@pytest.mark.anyio
async def test_review_detail_has_one_retryable_deadline_and_cancels_pending_work(monkeypatch):
cancelled = asyncio.Event()
async def requested(repository, number):
try:
await asyncio.Event().wait()
finally:
cancelled.set()
monkeypatch.setattr(main, "is_requested_review", requested)
monkeypatch.setattr(main, "REVIEW_DETAIL_TIMEOUT_SECONDS", 0.01)
transport = httpx.ASGITransport(app=main.app)
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
response = await client.get("/api/v1/repos/stackchain/api/pulls/7/review")
assert response.status_code == 503
assert response.headers["cache-control"] == "no-store"
assert response.headers["retry-after"] == "1"
assert response.json() == {
"error": "Pull request review details timed out. Please retry."
}
assert cancelled.is_set()
@pytest.mark.anyio
async def test_public_review_endpoint_does_not_expose_service_token_mutations():
transport = httpx.ASGITransport(app=main.app)