Refresh pull request checks without reloading review data #522

Merged
timmy merged 1 commits from timmy/521-status-only-check-refresh into main 2026-08-10 23:28:27 +00:00
8 changed files with 255 additions and 10 deletions

View File

@ -2333,6 +2333,22 @@
}
}
function renderPullCheckStatus(status) {
qs('#pull-ci-state').textContent = 'CI ' + (status.ci_state || 'unknown');
renderCheckSection('pull', status);
if (status.head_sha !== selectedPullDetail?.head_sha) {
qs('#pull-review-status').textContent = 'New commits detected. Reload review data before merging.';
qs('#pull-merge-state').textContent = 'Review data is stale';
qs('#merge-pull').disabled = true;
return;
}
selectedPullDetail = { ...selectedPullDetail, ...status };
const eligibility = createPullSheet.mergeEligibility(selectedPullDetail, pullReviewState);
qs('#pull-merge-state').textContent = eligibility.reason;
qs('#merge-pull').disabled = !eligibility.allowed;
qs('#pull-review-status').textContent = 'Checks refreshed for the current head.';
}
function focusNextUnreviewedPullFile() {
if (!selectedPullDetail || !pullReviewState) return;
const filename = pullController.nextUnreviewed(selectedPullDetail, pullReviewState);
@ -4280,11 +4296,18 @@
});
qs('#pull-review-retry').addEventListener('click', loadPullReview);
qs('#refresh-pull-checks').addEventListener('click', async () => {
if (!selectedPull || !selectedPullDetail?.head_sha) return;
const item = selectedPull;
const button = qs('#refresh-pull-checks');
button.disabled = true;
qs('#pull-checks-summary').textContent = 'Refreshing…';
try { await loadPullReview({ refresh: true }); }
finally { button.disabled = false; }
try {
const status = await pullController.loadChecks(item);
if (selectedPull !== item) return;
renderPullCheckStatus(status);
} catch (_error) {
if (selectedPull === item) qs('#pull-checks-summary').textContent = 'Refresh failed · retry';
} finally { button.disabled = false; }
});
qs('#next-unreviewed-pull-file').addEventListener('click', focusNextUnreviewedPullFile);
qs('#load-older-pull-comments').addEventListener('click', async () => {
@ -4490,7 +4513,7 @@
button.disabled = true;
qs('#review-checks-summary').textContent = 'Refreshing…';
try {
const detail = await reviewController.load(item);
const detail = await reviewController.loadChecks(item);
if (selectedReview !== item) return;
if (selectedReviewHead && detail.head_sha !== selectedReviewHead) {
qs('#review-sheet-status').textContent = 'New commits detected. Reload the review before submitting feedback.';

View File

@ -44,6 +44,7 @@ function renderFile(file, index, reviewed, escapeHtml) {
function createPullSheet({ fetchJson, storage, createConversationPager = globalThis.createConversationPager, createOperationId = () => globalThis.crypto?.randomUUID?.() || String(Date.now()) + '-' + Math.random() }) {
let commentRequest = null;
let mergeRequest = null;
let checkRequest = null;
const reviewRequests = new Map();
const reviewCache = new Map();
const pathFor = item => 'api/v1/repos/' + String(item.repository || '').split('/')
@ -82,6 +83,13 @@ function createPullSheet({ fetchJson, storage, createConversationPager = globalT
reviewRequests.set(key, request);
return request;
},
loadChecks(item) {
if (checkRequest) return checkRequest;
checkRequest = fetchJson(pathFor(item) + '/checks', {
headers: { Accept: 'application/json' },
}).finally(() => { checkRequest = null; });
return checkRequest;
},
conversation(item, initialPage) {
const pager = createConversationPager({
loadPage: page => fetchJson(pathFor(item) + '/comments?page=' + encodeURIComponent(page) + '&limit=20', {

View File

@ -4,6 +4,7 @@ function createReviewController({
createOperationId = () => globalThis.crypto?.randomUUID?.() || String(Date.now()) + '-' + Math.random(),
}) {
let pendingSubmission = null;
let pendingChecks = null;
function endpoint(item) {
const [owner, repo] = String(item.repository || '').split('/');
@ -18,6 +19,14 @@ function createReviewController({
return fetchJson(endpoint(item), { headers: { Accept: 'application/json' } });
}
function loadChecks(item) {
if (pendingChecks) return pendingChecks;
pendingChecks = fetchJson(endpoint(item) + '/checks', {
headers: { Accept: 'application/json' },
}).finally(() => { pendingChecks = null; });
return pendingChecks;
}
function submit(item, payload) {
if (pendingSubmission) return pendingSubmission;
const operationKey = 'stackchain.review-submit.v1:' + item.repository + '#' + item.number;
@ -44,7 +53,7 @@ function createReviewController({
return pendingSubmission;
}
return { load, submit };
return { load, loadChecks, submit };
}
function createWrapPreference({ storage, mobile }) {

View File

@ -1809,6 +1809,30 @@ async def pull_completion_review(repository: str, number: int) -> dict:
}
async def pull_check_status(repository: str, number: int) -> dict:
"""Load only mutable pull and CI state, without immutable review data."""
base = f"repos/{repository}/pulls/{number}"
pull = await fetch(base)
if not isinstance(pull, dict):
raise ValueError("Gitea pull request response was not an object")
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")
return {
"repository": repository,
"number": number,
"head_sha": sha,
"state": pull.get("state") if isinstance(pull.get("state"), str) else "",
"draft": pull.get("draft") is True,
"mergeable": pull.get("mergeable") is True,
"merged": pull.get("merged") is True,
"ci_state": status.get("state", "unknown") if isinstance(status, dict) else "unknown",
"checks": _normalize_commit_checks(status),
}
async def is_pull_merged_at_head(
repository: str, number: int, expected_head_sha: str
) -> bool:

View File

@ -3718,6 +3718,37 @@ async def review_detail(owner: str, repo: str, number: int):
)
@app.get("/api/v1/repos/{owner}/{repo}/pulls/{number}/review/checks")
async def requested_review_checks(
owner: str, repo: str, number: int = PathParam(gt=0)
):
repository = f"{owner}/{repo}"
async def load_checks():
if not await is_requested_review(repository, number):
raise HTTPException(status_code=404, detail="Review request not found")
return await gitea_proxy.pull_check_status(repository, number)
try:
return await asyncio.wait_for(
load_checks(), timeout=REVIEW_DETAIL_TIMEOUT_SECONDS
)
except HTTPException:
raise
except TimeoutError:
return JSONResponse(
{"error": "Refreshing review checks timed out. Please retry."},
status_code=503,
headers={"Retry-After": "1"},
)
except Exception:
return JSONResponse(
{"error": "Review checks are temporarily unavailable. Please retry."},
status_code=503,
headers={"Retry-After": "1"},
)
@app.get("/api/v1/repos/{owner}/{repo}/pulls/{number}/detail")
async def assigned_pull_detail(owner: str, repo: str, number: int = PathParam(gt=0)):
repository = f"{owner}/{repo}"
@ -3778,6 +3809,37 @@ async def assigned_pull_review_data(
)
@app.get("/api/v1/repos/{owner}/{repo}/pulls/{number}/checks")
async def assigned_pull_checks(
owner: str, repo: str, number: int = PathParam(gt=0)
):
repository = f"{owner}/{repo}"
async def load_checks():
if not await gitea_proxy.is_assigned_pull(repository, number):
raise HTTPException(status_code=404, detail="Assigned pull request not found")
return await gitea_proxy.pull_check_status(repository, number)
try:
return await asyncio.wait_for(
load_checks(), timeout=REVIEW_DETAIL_TIMEOUT_SECONDS
)
except HTTPException:
raise
except TimeoutError:
return JSONResponse(
{"error": "Refreshing pull request checks timed out. Please retry."},
status_code=503,
headers={"Retry-After": "1"},
)
except Exception:
return JSONResponse(
{"error": "Pull request checks are temporarily unavailable. Please retry."},
status_code=503,
headers={"Retry-After": "1"},
)
@app.get("/api/v1/repos/{owner}/{repo}/pulls/{number}/comments")
async def assigned_pull_conversation(
owner: str,

View File

@ -4055,7 +4055,7 @@ Promise.allSettled([first, concurrent]).then(async failed => {{
]
def test_pull_sheet_refreshes_checks_without_clearing_head_scoped_progress():
def test_pull_sheet_refreshes_status_only_without_clearing_review_progress():
script = f"""
const createPullSheet = require({json.dumps(str(PULL_SHEET))});
const values = new Map();
@ -4070,15 +4070,19 @@ const sheet = createPullSheet({{storage, fetchJson: url => {{
const item = {{repository:'stackchain/api', number:7}};
sheet.loadReview(item, 'abc123').then(first => {{
sheet.toggleReviewed(item, first, 'src/api.py');
return sheet.loadReview(item, 'abc123', {{refresh:true}}).then(refreshed => {{
process.stdout.write(JSON.stringify({{calls, refreshed, progress:sheet.reviewState(item, refreshed)}}));
return sheet.loadChecks(item).then(refreshed => {{
const combined = {{...first, ...refreshed}};
process.stdout.write(JSON.stringify({{calls, refreshed, progress:sheet.reviewState(item, combined)}}));
}});
}});
"""
result = subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True)
output = json.loads(result.stdout)
assert len(output["calls"]) == 2
assert output["calls"] == [
"api/v1/repos/stackchain/api/pulls/7/review-data",
"api/v1/repos/stackchain/api/pulls/7/checks",
]
assert output["refreshed"]["ci_state"] == "success"
assert output["progress"] == {"reviewed": ["src/api.py"], "total": 1, "complete": True}
@ -4483,6 +4487,28 @@ controller.load({{ repository: 'stackchain/api', number: 7 }}).then(detail =>
}
def test_review_controller_refreshes_checks_through_status_only_path():
script = f"""
const createReviewController = require({json.dumps(str(REVIEW_SHEET))});
const calls = [];
const controller = createReviewController({{ fetchJson: async (url, options) => {{
calls.push({{url, accept:options.headers.Accept}});
return {{head_sha:'abc123', ci_state:'success', checks:[]}};
}} }});
controller.loadChecks({{repository:'stackchain/api', number:7}}).then(status =>
process.stdout.write(JSON.stringify({{calls, status}}))
);
"""
result = subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True)
output = json.loads(result.stdout)
assert output["calls"] == [{
"url": "api/v1/repos/stackchain/api/pulls/7/review/checks",
"accept": "application/json",
}]
assert output["status"]["ci_state"] == "success"
def test_review_diff_rows_escape_content_and_toggle_accessibly():
script = f"""
const reviewSheet = require({json.dumps(str(REVIEW_SHEET))});
@ -4553,7 +4579,7 @@ process.stdout.write(JSON.stringify({{result, offline}}));
@pytest.mark.anyio
async def test_mobile_pull_review_sheets_expose_actionable_refreshable_checks():
async def test_mobile_pull_review_sheets_refresh_checks_without_reloading_diffs():
html = await dashboard()
assert html.index('id="pull-checks"') < html.index('id="pull-files"')
@ -4561,7 +4587,10 @@ async def test_mobile_pull_review_sheets_expose_actionable_refreshable_checks():
assert 'id="refresh-pull-checks"' in html
assert 'id="refresh-review-checks"' in html
assert "createReviewController.renderChecks" in html
assert "{ refresh: true }" in html
assert "pullController.loadChecks(item)" in html
assert "reviewController.loadChecks(item)" in html
assert "New commits detected. Reload review data before merging." in html
assert "New commits detected. Reload the review before submitting feedback." in html
assert "qs('#refresh-review-checks').disabled = offlineReview" in html
assert ".ci-check-link" in html and "min-height:44px" in html
assert ".ci-check-copy" in html and "overflow-wrap:anywhere" in html

View File

@ -125,6 +125,69 @@ async def test_assigned_pull_review_endpoint_loads_review_payload_on_demand(monk
]
@pytest.mark.anyio
async def test_gitea_pull_check_status_skips_files_reviews_and_diff():
requests = []
async def handler(request):
requests.append((request.method, request.url.path))
if request.url.path.endswith("/pulls/7"):
return httpx.Response(200, json={
"state": "open", "draft": False, "mergeable": True, "merged": False,
"head": {"sha": "abc123"},
})
if request.url.path.endswith("/commits/abc123/status"):
return httpx.Response(200, json={
"state": "success",
"statuses": [{"context": "tests", "status": "success", "description": "Passed"}],
})
raise AssertionError(f"unexpected request: {request.method} {request.url.path}")
gitea_proxy.start_client(transport=httpx.MockTransport(handler))
try:
status = await gitea_proxy.pull_check_status("stackchain/api", 7)
finally:
await gitea_proxy.stop_client()
assert requests == [
("GET", "/api/v1/repos/stackchain/api/pulls/7"),
("GET", "/api/v1/repos/stackchain/api/commits/abc123/status"),
]
assert status == {
"repository": "stackchain/api", "number": 7, "head_sha": "abc123",
"state": "open", "draft": False, "mergeable": True, "merged": False,
"ci_state": "success",
"checks": [{"name": "tests", "state": "success", "description": "Passed", "url": ""}],
}
@pytest.mark.anyio
async def test_assigned_pull_checks_endpoint_authorizes_and_returns_status_only(monkeypatch):
calls = []
async def assigned(repository, number):
calls.append(("assigned", repository, number))
return True
async def checks(repository, number):
calls.append(("checks", repository, number))
return {"head_sha": "abc123", "ci_state": "pending", "checks": []}
monkeypatch.setattr(main.gitea_proxy, "is_assigned_pull", assigned)
monkeypatch.setattr(main.gitea_proxy, "pull_check_status", checks)
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/checks")
assert response.status_code == 200
assert response.headers["cache-control"] == "no-store"
assert response.json() == {"head_sha": "abc123", "ci_state": "pending", "checks": []}
assert calls == [
("assigned", "stackchain/api", 7),
("checks", "stackchain/api", 7),
]
@pytest.mark.anyio
async def test_gitea_assigned_pull_review_includes_bounded_diff_previews():
async def handler(request):

View File

@ -36,6 +36,33 @@ async def test_review_detail_endpoint_returns_normalized_no_store_payload(monkey
assert response.json()["files"][0]["filename"] == "src/api.py"
@pytest.mark.anyio
async def test_requested_review_checks_endpoint_authorizes_and_returns_status_only(monkeypatch):
calls = []
async def requested(repository, number):
calls.append(("requested", repository, number))
return True
async def checks(repository, number):
calls.append(("checks", repository, number))
return {"head_sha": "def456", "ci_state": "success", "checks": []}
monkeypatch.setattr(main, "is_requested_review", requested)
monkeypatch.setattr(main.gitea_proxy, "pull_check_status", checks)
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/checks")
assert response.status_code == 200
assert response.headers["cache-control"] == "no-store"
assert response.json() == {"head_sha": "def456", "ci_state": "success", "checks": []}
assert calls == [
("requested", "stackchain/api", 7),
("checks", "stackchain/api", 7),
]
@pytest.mark.anyio
async def test_gitea_review_detail_returns_bounded_actionable_checks(monkeypatch):
monkeypatch.setattr(gitea_proxy, "GITEA_URL", "https://forge.example/git")