feat(#788): minimize stale inline review comments on re-review

[fullsend-ai-coder]

Summary

Minimize stale inline review comments on re-review to prevent duplicate inline comments from accumulating across review runs. The sticky comment already preserves full review history, so minimizing prior inline comments loses no information.

Related Issue

Closes #788

Changes

• Add PullRequestReviewComment struct and ListPullRequestReviewComments method to the forge.Client interface
• Implement for GitHub using GET /repos/{owner}/{repo}/pulls/{number}/comments with pagination
• GitLab returns ErrNotSupported (no comment minimize feature)
• Add minimizeStaleInlineComments() in postreview.go — finds all inline comments posted by the bot user and minimizes them as OUTDATED before posting new findings
• Call the new function from submitFormalReview() after minimizing stale review bodies, following the existing pattern
• Add comprehensive tests: own-comments-only filtering, no-comments no-op, API error tolerance, ErrNotSupported silent skip, and full submitFormalReview integration

Testing

• All existing postreview tests pass (no regressions)
• 6 new tests added covering the new function and its integration
• go vet ./... passes
• Note: pre-commit could not run (sandbox network restriction); post-script handles authoritatively

---

Closes #788

Post-script verification

• Branch is not main/master (agent/788-dedup-inline-review-comments)
• Secret scan passed (gitleaks — ba77dbdbbd17e9bb18c47bf6efa023c98b697158..HEAD)
• PR body secret scan passed (gitleaks — no-git)
• Pre-commit hooks passed (authoritative run on runner)
• Tests ran inside sandbox

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:06 AM UTC · Completed 3:16 AM UTC
Commit: 6bc7f74 · View workflow run →

[fullsend-ai-review]

Review

Findings

Low

• [test-adequacy] internal/cli/postreview_test.go:1567 — TestMinimizeStaleInlineComments_MinimizeErrorIsNonFatal asserts the warning string appears in output but does not verify both comments were attempted. The FakeClient's error injection returns the same error for every MinimizeComment call without recording it, so MinimizedComments is empty. The test cannot distinguish "loop continued after first error" from "loop stopped after first error." Assert both node IDs (PRC_1 and PRC_2) appear in the warning output to confirm continuation.

• [error-handling-consistency] internal/cli/postreview.go:626 — minimizeStaleInlineComments checks forge.IsNotSupported(err) to silently skip on GitLab, but the existing ListPullRequestReviews error handler (line 316) does not distinguish error types. The IsNotSupported check is well-motivated here (GitLab returns ErrNotSupported for ListPullRequestReviewComments), but the same argument applies to ListPullRequestReviews. Consider aligning both patterns.

• [missing-doc] docs/agents/review.md — The review agent documentation does not mention inline review comment lifecycle or the new minimization behavior added by this PR. A brief note about the post-script's stale inline comment cleanup would help contributors understand the full review lifecycle.

Previous run

Looks good to me.

Clean implementation that follows existing patterns (minimizeStaleReviews, dismissStaleRequestChanges) and correctly extends the forge abstraction with all three implementations (GitHub, GitLab ErrNotSupported, fake client). Ownership filtering via c.User == user is consistent with how the codebase guards destructive forge operations. Error handling is non-fatal throughout, preventing cleanup failures from blocking review submission. Test coverage is comprehensive — 6 tests covering the happy path, no-op, list errors, minimize errors, ErrNotSupported silent skip, and full submitFormalReview integration.

Minor observations

Low

• [unnecessary-coupling] internal/cli/postreview.go:320 — minimizeStaleInlineComments is placed inside the else block gated on ListPullRequestReviews succeeding, but it does not use the reviews variable. If ListPullRequestReviews fails (transient API error, rate limit), inline comment cleanup is silently skipped even though it could proceed independently — the function only needs user and makes its own ListPullRequestReviewComments call. Consider moving it outside the reviews-dependent block in a follow-up.

• [interface-extension] internal/forge/forge.go — Adding ListPullRequestReviewComments to the forge.Client interface is routine internal interface evolution. All three in-tree implementations are correctly updated in this PR.

---

Labels: PR adds forge interface method and post-review behavior for inline comment management

[guyoron1]

/fs-fix

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 6:48 AM UTC · Completed 6:54 AM UTC
Commit: 6bc7f74 · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (human-triggered)

Addressed 1 actionable finding (unnecessary-coupling) by restructuring the if-else chain so minimizeStaleInlineComments runs independently of ListPullRequestReviews success. The second finding (interface-extension) was observational with no action required.

Fixed (1):

1. unnecessary-coupling: minimizeStaleInlineComments inside reviews-dependent else block (internal/cli/postreview.go): Moved minimizeStaleInlineComments outside the ListPullRequestReviews-dependent inner block so it runs independently when user is available. The function makes its own ListPullRequestReviewComments call and does not need the reviews variable.

Disagreed (1):

1. interface-extension: ListPullRequestReviewComments added to forge.Client: This was an observation, not an actionable finding. The reviewer noted it is routine internal interface evolution with all implementations correctly updated.

Tests: passed

Next steps:

• /fs-review — request a re-review of the changes
• /fs-fix <your instruction> — run another fix pass with specific guidance
• Push commits directly — review re-runs automatically on push
  _{Updated by fullsend fix agent}

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:59 AM UTC · Completed 7:12 AM UTC
Commit: cad4799 · View workflow run →

[fullsend-ai-review]

[low] test-adequacy

TestMinimizeStaleInlineComments_MinimizeErrorIsNonFatal asserts the warning string appears in output but does not verify both comments were attempted. The FakeClient's error injection returns the same error for every MinimizeComment call without recording it, so MinimizedComments is empty. The test cannot distinguish 'loop continued after first error' from 'loop stopped after first error.'

Suggested fix: Assert both node IDs (PRC_1 and PRC_2) appear in the warning output to confirm both minimize attempts were made.

[fullsend-ai-review]

[low] error-handling-consistency

minimizeStaleInlineComments checks forge.IsNotSupported(err) to silently skip on GitLab, but the existing ListPullRequestReviews error handler (line 316) does not distinguish error types. The IsNotSupported check is well-motivated here but the same argument applies to ListPullRequestReviews.

Suggested fix: Consider aligning both error handling patterns, either by adding IsNotSupported to ListPullRequestReviews or documenting why the difference is intentional.

[github-actions]

This pull request has been automatically marked as stale because it has not had any activity in the last month. It will be closed in 2 weeks if no further activity occurs. Remove the stale label to reset the inactivity timer.