Skip to content

Commit 8e06335

Browse files
committed
Replace stale question retries with current-review guidance
1 parent 81d684d commit 8e06335

3 files changed

Lines changed: 13 additions & 0 deletions

File tree

‎docs/implementation/read-only-review.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,3 +84,5 @@ Conversation width: drag the divider on its left edge to resize between 280 and
8484
Selection actions now appear in a bordered floating toolbar beside the visible selected lines, with Ask emphasized. The toolbar stays within the code viewport, leaves line numbers available for Shift-click extension, and hides when the selection scrolls out of view. Clear selection removes it. The long-diff browser regression checks proximity, viewport bounds, attachment, scrolling away/back, and clearing; existing range/highlight tests remain required.
8585

8686
PR #11 review round 1 reproduced a stale-poll race: a held question-list response hid a question submitted after the poll began. Review actions and refreshes now advance a generation counter; earlier poll successes and errors are ignored. The browser regression failed with one visible note instead of two before the fix.
87+
88+
PR #11 review round 2 confirmed the poll fix and raised an outdated-retry issue in its summary. A browser regression reproduced the retry button on an older snapshot; such unanswered questions now instruct the reviewer to ask again against current code. Completed historical answers retain their outdated label. The summary's timeout-reporting concern supplied no concrete failure; timeout and interrupted-lease regressions remain in the validation suite.

‎test/browser/review.spec.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,3 +223,13 @@ test('ignores question polls started before a newer submission',async({page})=>{
223223
await page.waitForTimeout(100);
224224
expect(await page.locator('#notes .note').count()).toBe(2);
225225
});
226+
test('outdated questions explain how to continue without offering a broken retry',async({page})=>{
227+
const service=app.service;
228+
service.act({action:'note',item:'P1',kind:'question',text:'Old question',token:service.load().token});
229+
execFileSync('git',['-c','core.hooksPath=/dev/null','commit','--allow-empty','-m','New snapshot'],{cwd:service.config.repository,stdio:'pipe'});
230+
await page.goto(app.url);
231+
await expect(page.getByText('Old question',{exact:true})).toBeVisible();
232+
await expect(page.getByRole('button',{name:'Retry answer',exact:true})).toHaveCount(0);
233+
await expect(page.getByText('This question refers to an earlier review. Ask again against the current code.',{exact:true})).toBeVisible();
234+
await expect(page.getByRole('heading',{name:'Bound exponential retries'})).toBeVisible();
235+
});

‎web/public/app.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,7 @@ function answerMarkup(note) {
558558
if(note.kind!=="question") return "";
559559
const answer=note.answer;
560560
if(answer?.status==="complete") return `<section class="agent-answer"><strong>${answer.provider === "claude" ? "Claude Code" : answer.provider === "codex" ? "Codex" : "Agent"}</strong><p>${esc(answer.text)}</p>${note.answerOutdated ? '<small>! Answer refers to an earlier review snapshot.</small>' : ""}</section>`;
561+
if(note.answerOutdated) return '<p class="warn">This question refers to an earlier review. Ask again against the current code.</p>';
561562
if(answer?.status==="pending" && answer.expiresAt>Date.now()) return '<p role="status">Agent · Answering…</p>';
562563
const error=answer?.status==="failed"?answer.error:answer?.status==="pending"?"Agent was interrupted or timed out.":"Answer not started. Choose an agent in Settings or retry when capacity is available.";
563564
return `<p class="warn">! ${esc(error)}</p><button data-retry-question="${esc(note.id)}">Retry answer</button>`;

0 commit comments

Comments
 (0)