From 45060ca3e9a0fb84b2d2ead956ab8dead7c44b61 Mon Sep 17 00:00:00 2001 From: Andy <119136210+AndyMik90@users.noreply.github.com> Date: Thu, 15 Jan 2026 18:17:26 +0100 Subject: [PATCH] fix(pr-review): use list instead of tuple for line_range to fix SDK structured output (#1140) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(pr-review): use list instead of tuple for line_range to fix SDK structured output The FindingValidationResult.line_range field was using tuple[int, int] which generates JSON Schema with prefixItems (draft 2020-12 feature). This caused the Claude Agent SDK to silently fail to capture structured output for follow-up reviews while returning subtype=success. Root cause: - ParallelFollowupResponse schema used prefixItems: True - ParallelOrchestratorResponse schema used prefixItems: False - Orchestrator structured output worked; followup didn't Fix: - Change line_range from tuple[int, int] to list[int] with min/max length=2 - This generates minItems/maxItems instead of prefixItems - Schema is now compatible with SDK structured output handling Co-Authored-By: Claude Opus 4.5 * fix(pr-review): only show follow-up when initial review was posted to GitHub Fixed bug where "Ready for Follow-up" was shown even when the initial review was never posted to GitHub. Root cause: 1. Backend set hasCommitsAfterPosting=true when postedAt was null 2. Frontend didn't check if findings were posted before showing follow-up UI Fixes: 1. Backend (pr-handlers.ts): Return hasCommitsAfterPosting=false when postedAt is null - can't be "after posting" if nothing was posted 2. Frontend (ReviewStatusTree.tsx): Add hasPostedFindings check before showing follow-up steps (defense-in-depth) Before: User runs review → doesn't post → new commits → shows "Ready for Follow-up" After: User runs review → doesn't post → new commits → shows "Pending Post" only Co-Authored-By: Claude Opus 4.5 * fix(ui): use consistent hasPostedFindings check in follow-up logic Fixed inconsistency where Step 4 only checked postedCount > 0 while Step 3 and previous review checks used postedCount > 0 || reviewResult?.hasPostedFindings. This ensures the follow-up button correctly appears when reviewResult?.hasPostedFindings is true but postedCount is 0 (due to state sync timing issues). --------- Co-authored-by: Claude Opus 4.5 --- apps/backend/runners/github/services/pydantic_models.py | 6 ++++-- apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts | 5 +++-- .../components/github-prs/components/ReviewStatusTree.tsx | 7 ++++--- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/apps/backend/runners/github/services/pydantic_models.py b/apps/backend/runners/github/services/pydantic_models.py index 6777e976..42903dbf 100644 --- a/apps/backend/runners/github/services/pydantic_models.py +++ b/apps/backend/runners/github/services/pydantic_models.py @@ -611,8 +611,10 @@ class FindingValidationResult(BaseModel): "This is the proof that determines the validation status." ), ) - line_range: tuple[int, int] = Field( - description="Start and end line numbers of the examined code" + line_range: list[int] = Field( + min_length=2, + max_length=2, + description="Start and end line numbers of the examined code [start, end]", ) explanation: str = Field( min_length=20, diff --git a/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts b/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts index e7f67a19..6fa3c96a 100644 --- a/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts @@ -2102,8 +2102,9 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v hasCommitsAfterPosting, }); } else if (!postedAt) { - // If findings haven't been posted yet, any new commits should trigger follow-up - hasCommitsAfterPosting = true; + // If findings haven't been posted yet, we can't determine "after posting" + // Follow-up should only be available after initial review is posted to GitHub + hasCommitsAfterPosting = false; } return { diff --git a/apps/frontend/src/renderer/components/github-prs/components/ReviewStatusTree.tsx b/apps/frontend/src/renderer/components/github-prs/components/ReviewStatusTree.tsx index 7c013ee3..7c3d5562 100644 --- a/apps/frontend/src/renderer/components/github-prs/components/ReviewStatusTree.tsx +++ b/apps/frontend/src/renderer/components/github-prs/components/ReviewStatusTree.tsx @@ -178,9 +178,10 @@ export function ReviewStatusTree({ }); } - // Step 4: Follow-up (only show when not currently reviewing AND commits happened after posting) - // This prevents showing follow-up prompts for commits that were made during/before the review - if (!isReviewing && newCommitsCheck?.hasNewCommits && newCommitsCheck?.hasCommitsAfterPosting) { + // Step 4: Follow-up (only show when findings were POSTED and new commits happened after posting) + // This prevents showing follow-up prompts when initial review was never posted to GitHub + const hasPostedFindings = postedCount > 0 || reviewResult?.hasPostedFindings; + if (!isReviewing && hasPostedFindings && newCommitsCheck?.hasNewCommits && newCommitsCheck?.hasCommitsAfterPosting) { steps.push({ id: 'new_commits', label: t('prReview.newCommits', { count: newCommitsCheck.newCommitCount }),