fix(pr-review): use list instead of tuple for line_range to fix SDK structured output (#1140)
* 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com>
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 }),
|
||||
|
||||
Reference in New Issue
Block a user