From 174dac05b9a3209150eef7a19bb2fbda0ce7dd99 Mon Sep 17 00:00:00 2001
From: timmy <timmy@stackchain.ai>
Date: Fri, 7 Aug 2026 18:00:00 +0000
Subject: [PATCH] feat: make assigned pull sheets read-first (#217)

---
 frontend/index.html    | 55 ++++++++++++++++++++++++-----
 frontend/pull-sheet.js | 16 +++++++++
 src/gitea_proxy.py     | 40 +++++++++++++++-------
 src/main.py            | 31 +++++++++++++++++
 tests/test_my_work.py  | 57 ++++++++++++++++++++++++++++++
 tests/test_pull_api.py | 78 +++++++++++++++++++++++++++++++++++++++---
 6 files changed, 251 insertions(+), 26 deletions(-)

diff --git a/frontend/index.html b/frontend/index.html
index 3c156e6..1d47425 100644
--- a/frontend/index.html
+++ b/frontend/index.html
@@ -231,6 +231,10 @@ textarea { resize: vertical; min-height: 120px; }
 .pull-diff-empty { margin:8px 0; padding:10px; border:1px dashed #4e6b8a; border-radius:8px; }
 .pull-review-tools { display:flex; align-items:center; justify-content:space-between; gap:8px; margin:8px 0; }
 .pull-review-tools button { min-height:44px; }
+.pull-review { margin-top:14px; overflow:hidden; border:1px solid #2a496e; border-radius:10px; padding:0 10px 10px; }
+.pull-review summary { min-height:44px; display:flex; align-items:center; cursor:pointer; }
+.pull-review summary h2 { margin:0; font-size:16px; }
+.pull-review > #merge-pull { min-height:44px; width:100%; margin-top:10px; }
 .pull-comment-composer textarea { width:100%; min-height:110px; resize:vertical; }
 .pull-sheet-actions { position:sticky; bottom:0; display:grid; grid-template-columns:1fr 1fr; gap:8px; padding:10px 0; padding-bottom:calc(10px + env(safe-area-inset-bottom)); background:#0b1526; }
 .pull-sheet-actions a { display:grid; place-items:center; border:1px solid #60a5fa; border-radius:10px; font-weight:700; }
@@ -579,11 +583,7 @@ textarea { resize: vertical; min-height: 120px; }
     </div>
     <div id="pull-sheet-status" class="small" aria-live="polite">Choose a pull request.</div>
     <button class="pull-retry" id="retry-pull-load" type="button" hidden>Retry loading pull request</button>
-    <div class="row"><span class="pill" id="pull-ci-state">CI unknown</span><span class="pill" id="pull-merge-state">Checking merge status</span></div>
     <p class="pull-sheet-content" id="pull-sheet-body"></p>
-    <h2>Changed files</h2>
-    <div class="pull-review-tools"><span id="pull-review-progress" class="small" aria-live="polite">Review progress unavailable.</span><button id="next-unreviewed-pull-file" type="button" disabled>Next unreviewed</button></div>
-    <div id="pull-files"></div>
     <h2>Full conversation</h2><div id="pull-comments"></div>
     <button class="conversation-more" id="load-older-pull-comments" type="button" hidden>Load older messages</button>
     <div id="pull-conversation-status" class="small" aria-live="assertive"></div>
@@ -593,9 +593,17 @@ textarea { resize: vertical; min-height: 120px; }
       <button id="send-pull-comment" type="button">Post comment</button>
       <div id="pull-comment-status" class="small" aria-live="assertive"></div>
     </section>
+    <details class="pull-review" id="pull-review">
+      <summary><h2>Review &amp; merge</h2></summary>
+      <div id="pull-review-status" class="small" aria-live="polite">Expand to load changed files and merge readiness.</div>
+      <button class="pull-retry" id="pull-review-retry" type="button" hidden>Retry review data</button>
+      <div class="row"><span class="pill" id="pull-ci-state">CI unknown</span><span class="pill" id="pull-merge-state">Review data not loaded</span></div>
+      <div class="pull-review-tools"><span id="pull-review-progress" class="small" aria-live="polite">Review progress unavailable.</span><button id="next-unreviewed-pull-file" type="button" disabled>Next unreviewed</button></div>
+      <div id="pull-files"></div>
+      <button id="merge-pull" type="button" disabled>Merge</button>
+    </details>
     <div class="pull-sheet-actions">
       <button class="share-work-route" type="button">Share</button>
-      <button id="merge-pull" type="button" disabled>Merge</button>
       <a id="open-pull-gitea" href="#" target="_blank" rel="noopener noreferrer">Open in Gitea</a>
     </div>
     <nav class="work-session-nav" aria-label="Work session" hidden>
@@ -1555,6 +1563,30 @@ textarea { resize: vertical; min-height: 120px; }
     }
   }
 
+  async function loadPullReview() {
+    if (!selectedPull || !selectedPullDetail?.head_sha) return;
+    const item = selectedPull;
+    const readDetail = selectedPullDetail;
+    qs('#pull-review-status').textContent = 'Loading changed files and merge readiness…';
+    qs('#pull-review-retry').hidden = true;
+    qs('#merge-pull').disabled = true;
+    try {
+      const review = await pullController.loadReview(item, readDetail.head_sha);
+      if (selectedPull !== item) return;
+      selectedPullDetail = { ...readDetail, ...review };
+      qs('#pull-ci-state').textContent = 'CI ' + (review.ci_state || 'unknown');
+      renderPullReview(selectedPullDetail);
+      qs('#pull-review-status').textContent = review.head_sha === readDetail.head_sha ?
+        'Review data ready for the current head.' : 'New commits detected. Review progress restarted for the latest head.';
+    } catch (error) {
+      if (selectedPull !== item) return;
+      qs('#pull-review-status').textContent = error.message + ' Reading and replies remain available; retry here.';
+      qs('#pull-review-retry').hidden = false;
+      qs('#pull-merge-state').textContent = 'Review data unavailable';
+      qs('#merge-pull').disabled = true;
+    }
+  }
+
   async function openPullSheet(item, trigger) {
     if (!item) return;
     selectedPull = item;
@@ -1568,7 +1600,10 @@ textarea { resize: vertical; min-height: 120px; }
     qs('#pull-sheet-status').textContent = 'Loading pull request…';
     qs('#pull-sheet-body').textContent = '';
     qs('#pull-files').textContent = '';
-    qs('#pull-review-progress').textContent = 'Loading review progress…';
+    qs('#pull-review').open = false;
+    qs('#pull-review-status').textContent = 'Expand to load changed files and merge readiness.';
+    qs('#pull-review-retry').hidden = true;
+    qs('#pull-review-progress').textContent = 'Review progress unavailable.';
     qs('#next-unreviewed-pull-file').disabled = true;
     qs('#pull-comments').textContent = '';
     qs('#load-older-pull-comments').hidden = true;
@@ -1576,7 +1611,7 @@ textarea { resize: vertical; min-height: 120px; }
     qs('#pull-comment').value = pullController.loadDraft(item);
     qs('#pull-comment-status').textContent = '';
     qs('#pull-ci-state').textContent = 'CI unknown';
-    qs('#pull-merge-state').textContent = 'Checking merge status';
+    qs('#pull-merge-state').textContent = 'Review data not loaded';
     qs('#merge-pull').disabled = true;
     qs('#retry-pull-load').hidden = true;
     qs('#open-pull-gitea').href = item.url || '#';
@@ -1588,8 +1623,6 @@ textarea { resize: vertical; min-height: 120px; }
       pullConversation = pullController.conversation(item, detail.conversation);
       qs('#pull-sheet-title').textContent = detail.title || 'Assigned pull request';
       qs('#pull-sheet-body').textContent = detail.body || 'No description provided.';
-      qs('#pull-ci-state').textContent = 'CI ' + (detail.ci_state || 'unknown');
-      renderPullReview(detail);
       renderPullConversation(pullConversation.snapshot());
       qs('#open-pull-gitea').href = detail.url || item.url || '#';
       qs('#pull-sheet-status').textContent = 'Pull request ready · by ' + (detail.author || 'unknown author');
@@ -2609,6 +2642,10 @@ textarea { resize: vertical; min-height: 120px; }
   qs('#retry-pull-load').addEventListener('click', () => {
     if (selectedPull) openPullSheet(selectedPull, pullTrigger);
   });
+  qs('#pull-review').addEventListener('toggle', event => {
+    if (event.currentTarget.open) loadPullReview();
+  });
+  qs('#pull-review-retry').addEventListener('click', loadPullReview);
   qs('#next-unreviewed-pull-file').addEventListener('click', focusNextUnreviewedPullFile);
   qs('#load-older-pull-comments').addEventListener('click', async () => {
     if (!pullConversation) return;
diff --git a/frontend/pull-sheet.js b/frontend/pull-sheet.js
index 762dbb4..9adc047 100644
--- a/frontend/pull-sheet.js
+++ b/frontend/pull-sheet.js
@@ -40,6 +40,8 @@ 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;
+  const reviewRequests = new Map();
+  const reviewCache = new Map();
   const pathFor = item => 'api/v1/repos/' + String(item.repository || '').split('/')
     .map(encodeURIComponent).join('/') + '/pulls/' + encodeURIComponent(item.number);
   const draftKey = item => 'stackchain.pull-comment.v1:' + item.repository + '#' + item.number;
@@ -62,6 +64,20 @@ function createPullSheet({ fetchJson, storage, createConversationPager = globalT
     load(item) {
       return fetchJson(pathFor(item) + '/detail', { headers: { Accept: 'application/json' } });
     },
+    loadReview(item, headSha) {
+      const key = item.repository + '#' + item.number + ':' + headSha;
+      if (reviewCache.has(key)) return Promise.resolve(reviewCache.get(key));
+      if (reviewRequests.has(key)) return reviewRequests.get(key);
+      const request = fetchJson(pathFor(item) + '/review-data', {
+        headers: { Accept: 'application/json' },
+      }).then(detail => {
+        const confirmedKey = item.repository + '#' + item.number + ':' + detail.head_sha;
+        reviewCache.set(confirmedKey, detail);
+        return detail;
+      }).finally(() => reviewRequests.delete(key));
+      reviewRequests.set(key, request);
+      return request;
+    },
     conversation(item, initialPage) {
       const pager = createConversationPager({
         loadPage: page => fetchJson(pathFor(item) + '/comments?page=' + encodeURIComponent(page) + '&limit=20', {
diff --git a/src/gitea_proxy.py b/src/gitea_proxy.py
index e343a95..bb26042 100644
--- a/src/gitea_proxy.py
+++ b/src/gitea_proxy.py
@@ -1222,18 +1222,9 @@ async def pull_completion_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")
+    conversation = await issue_conversation_page(repository, number)
     head = pull.get("head") if isinstance(pull.get("head"), dict) else {}
     sha = head.get("sha") if isinstance(head.get("sha"), str) else ""
-    files, status, conversation, diff_result = await asyncio.gather(
-        fetch(f"{base}/files"),
-        fetch(f"repos/{repository}/commits/{sha}/status"),
-        issue_conversation_page(repository, number),
-        fetch_text(
-            f"repos/{repository}/pulls/{number}.diff", REVIEW_DIFF_MAX_BYTES
-        ),
-    )
-    diff, diff_truncated = diff_result
-    previews = _diff_previews(diff, diff_truncated)
     user = pull.get("user") if isinstance(pull.get("user"), dict) else {}
     return {
         "repository": repository,
@@ -1244,6 +1235,33 @@ async def pull_completion_detail(repository: str, number: int) -> dict:
         "author": user.get("login") if isinstance(user.get("login"), str) else "",
         "head_sha": sha,
         "state": pull.get("state") if isinstance(pull.get("state"), str) else "",
+        "conversation": conversation,
+    }
+
+
+async def pull_completion_review(repository: str, number: int) -> dict:
+    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 ""
+    files, status, diff_result = await asyncio.gather(
+        fetch(f"{base}/files"),
+        fetch(f"repos/{repository}/commits/{sha}/status"),
+        fetch_text(
+            f"repos/{repository}/pulls/{number}.diff", REVIEW_DIFF_MAX_BYTES
+        ),
+    )
+    diff, diff_truncated = diff_result
+    previews = _diff_previews(diff, diff_truncated)
+    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,
@@ -1267,8 +1285,6 @@ async def pull_completion_detail(repository: str, number: int) -> dict:
             for item in (files if isinstance(files, list) else [])[:100]
             if isinstance(item, dict) and isinstance(item.get("filename"), str)
         ],
-        "comments": conversation["comments"],
-        "conversation": conversation,
     }
 
 
diff --git a/src/main.py b/src/main.py
index 3937098..1dc1e32 100644
--- a/src/main.py
+++ b/src/main.py
@@ -1642,6 +1642,37 @@ async def assigned_pull_detail(owner: str, repo: str, number: int = PathParam(gt
         )
 
 
+@app.get("/api/v1/repos/{owner}/{repo}/pulls/{number}/review-data")
+async def assigned_pull_review_data(
+    owner: str, repo: str, number: int = PathParam(gt=0)
+):
+    repository = f"{owner}/{repo}"
+
+    async def load_assigned_pull_review():
+        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_completion_review(repository, number)
+
+    try:
+        return await asyncio.wait_for(
+            load_assigned_pull_review(), timeout=REVIEW_DETAIL_TIMEOUT_SECONDS
+        )
+    except HTTPException:
+        raise
+    except TimeoutError:
+        return JSONResponse(
+            {"error": "Loading review and merge data timed out. Please retry."},
+            status_code=503,
+            headers={"Retry-After": "1"},
+        )
+    except Exception:
+        return JSONResponse(
+            {"error": "Review and merge data is 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,
diff --git a/tests/test_my_work.py b/tests/test_my_work.py
index 3239b7b..2786b57 100644
--- a/tests/test_my_work.py
+++ b/tests/test_my_work.py
@@ -2023,6 +2023,63 @@ process.stdout.write(JSON.stringify({{
     assert output["next"] == "frontend/app.js"
 
 
+def test_pull_sheet_lazy_review_is_single_flight_cached_by_head_and_retryable():
+    script = f"""
+const createPullSheet = require({json.dumps(str(PULL_SHEET))});
+const calls = [];
+let rejectFirst;
+const fetchJson = url => {{
+  calls.push(url);
+  if (calls.length === 1) return new Promise((_resolve, reject) => {{ rejectFirst = reject; }});
+  return Promise.resolve({{head_sha:'abc123', files:[]}});
+}};
+const sheet = createPullSheet({{fetchJson, storage:null}});
+const item = {{repository:'stackchain/api', number:7}};
+const first = sheet.loadReview(item, 'abc123');
+const concurrent = sheet.loadReview(item, 'abc123');
+rejectFirst(new Error('diff unavailable'));
+Promise.allSettled([first, concurrent]).then(async failed => {{
+  const retried = await sheet.loadReview(item, 'abc123');
+  const cached = await sheet.loadReview(item, 'abc123');
+  process.stdout.write(JSON.stringify({{
+    same:first === concurrent,
+    failed:failed.map(result => result.status),
+    retried,
+    cached,
+    calls,
+  }}));
+}});
+"""
+    result = subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True)
+    output = json.loads(result.stdout)
+
+    assert output["same"] is True
+    assert output["failed"] == ["rejected", "rejected"]
+    assert output["retried"]["head_sha"] == "abc123"
+    assert output["cached"] == output["retried"]
+    assert output["calls"] == [
+        "api/v1/repos/stackchain/api/pulls/7/review-data",
+        "api/v1/repos/stackchain/api/pulls/7/review-data",
+    ]
+
+
+@pytest.mark.anyio
+async def test_mobile_pull_sheet_puts_reading_before_collapsed_review_controls():
+    html = await dashboard()
+
+    body = html.index('id="pull-sheet-body"')
+    conversation = html.index('<h2>Full conversation</h2>', body)
+    composer = html.index('id="pull-comment-title"', conversation)
+    review = html.index('id="pull-review"', composer)
+    files = html.index('id="pull-files"', review)
+
+    assert '<summary><h2>Review &amp; merge</h2></summary>' in html
+    assert '<details class="pull-review" id="pull-review">' in html
+    assert 'id="pull-review-retry"' in html
+    assert 'id="pull-review-status"' in html
+    assert body < conversation < composer < review < files
+
+
 def test_pull_sheet_renders_mobile_diff_fallbacks_and_review_controls():
     script = f"""
 const createPullSheet = require({json.dumps(str(PULL_SHEET))});
diff --git a/tests/test_pull_api.py b/tests/test_pull_api.py
index b05789a..95cdb37 100644
--- a/tests/test_pull_api.py
+++ b/tests/test_pull_api.py
@@ -54,7 +54,77 @@ async def test_assigned_pull_detail_rejects_unassigned_pull(monkeypatch):
 
 
 @pytest.mark.anyio
-async def test_gitea_assigned_pull_detail_includes_bounded_diff_previews():
+async def test_gitea_assigned_pull_detail_loads_reading_without_review_resources():
+    requests = []
+
+    async def handler(request):
+        requests.append((request.method, request.url.path))
+        if request.url.path.endswith("/pulls/7"):
+            return httpx.Response(200, json={
+                "number": 7,
+                "title": "Read this first",
+                "body": "The discussion should not wait for the diff.",
+                "state": "open",
+                "mergeable": True,
+                "head": {"sha": "abc123"},
+                "user": {"login": "alex"},
+            })
+        if request.url.path.endswith("/issues/7/comments"):
+            return httpx.Response(200, json=[{
+                "id": 9, "body": "Question", "user": {"login": "sam"},
+                "created_at": "2026-08-07T18:00:00Z",
+            }], headers={"X-Total-Count": "1"})
+        raise AssertionError(f"review resource requested during read load: {request.url.path}")
+
+    gitea_proxy.start_client(transport=httpx.MockTransport(handler))
+    try:
+        detail = await gitea_proxy.pull_completion_detail("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/issues/7/comments"),
+    ]
+    assert detail["head_sha"] == "abc123"
+    assert detail["conversation"]["comments"][0]["body"] == "Question"
+    assert "files" not in detail
+    assert "ci_state" not in detail
+
+
+@pytest.mark.anyio
+async def test_assigned_pull_review_endpoint_loads_review_payload_on_demand(monkeypatch):
+    calls = []
+
+    async def assigned(repository, number):
+        calls.append(("assigned", repository, number))
+        return True
+
+    async def review(repository, number):
+        calls.append(("review", repository, number))
+        return {
+            "repository": repository, "number": number, "head_sha": "abc123",
+            "state": "open", "draft": False, "mergeable": True, "merged": False,
+            "ci_state": "success", "files": [{"filename": "src/api.py"}],
+        }
+
+    monkeypatch.setattr(main.gitea_proxy, "is_assigned_pull", assigned)
+    monkeypatch.setattr(main.gitea_proxy, "pull_completion_review", review, raising=False)
+    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-data")
+
+    assert response.status_code == 200
+    assert response.headers["cache-control"] == "no-store"
+    assert response.json()["files"] == [{"filename": "src/api.py"}]
+    assert calls == [
+        ("assigned", "stackchain/api", 7),
+        ("review", "stackchain/api", 7),
+    ]
+
+
+@pytest.mark.anyio
+async def test_gitea_assigned_pull_review_includes_bounded_diff_previews():
     async def handler(request):
         path = request.url.path
         if path.endswith("/pulls/7"):
@@ -86,7 +156,7 @@ async def test_gitea_assigned_pull_detail_includes_bounded_diff_previews():
 
     gitea_proxy.start_client(transport=httpx.MockTransport(handler))
     try:
-        detail = await gitea_proxy.pull_completion_detail("stackchain/api", 7)
+        detail = await gitea_proxy.pull_completion_review("stackchain/api", 7)
     finally:
         await gitea_proxy.stop_client()
 
@@ -94,9 +164,7 @@ async def test_gitea_assigned_pull_detail_includes_bounded_diff_previews():
     assert "+new" in detail["files"][0]["diff_lines"]
     assert detail["files"][1]["diff_binary"] is True
     assert detail["files"][1]["diff_available"] is False
-    assert detail["conversation"] == {
-        "comments": [], "page": 1, "older_page": None, "total": 0
-    }
+    assert "conversation" not in detail
 
 
 @pytest.mark.anyio