Skip to content

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

Open
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/836-minimize-stale-inline-comments
Open

feat(#836): minimize stale inline review comments on re-review#868
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/836-minimize-stale-inline-comments

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown

Summary

Minimize stale inline review comments on re-review to prevent comment accumulation. When the review agent re-reviews a PR, all prior bot-authored inline comments are now minimized (marked as OUTDATED) before posting new findings. The sticky comment already preserves full review history, so no information is lost.

Related Issue

Fixes #836

Changes

  • Added PullRequestReviewComment struct to forge.Client interface for reading existing inline comments
  • Added ListPullRequestReviewComments method to the GitHub forge client (paginated REST API)
  • Added GitLab stub returning ErrNotSupported (GitLab notes lack minimize/hide)
  • Added minimizeStaleInlineComments() to the post-review flow, called before posting new findings
  • Restructured submitFormalReview to separate review-listing from inline-comment cleanup, so inline comment minimization runs even if review listing fails
  • Added 6 tests: own-comment filtering, no-op, list error tolerance, minimize error tolerance, GitLab ErrNotSupported silent skip, and full integration through submitFormalReview

Testing

  • All 6 new tests pass
  • All 55 existing postreview tests pass (no regressions)
  • All forge package tests pass (github, gitlab, fake)
  • go vet clean
  • go build clean
  • Secret scan clean

Checklist

  • PR title follows Conventional Commits (correct type, ! for breaking changes)
  • Commits are signed off (DCO) — human and human-directed agent sessions only
  • I wrote this contribution myself and can explain all changes in it

Closes #836

Post-script verification

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

Before this change, inline review comments accumulated across
re-review runs — each run posted new inline comments without
cleaning up prior ones. On PRs receiving multiple reviews, this
created excessive noise (e.g. 54 comments on 78 lines).

Add minimizeStaleInlineComments() to the post-review flow. Before
posting new inline comments, it lists all existing inline review
comments on the PR, filters for those authored by the bot user,
and minimizes them as OUTDATED via the GraphQL API. The sticky
comment already preserves full review history, so minimizing
prior inline comments loses no information.

Changes:
- internal/forge/forge.go: add PullRequestReviewComment struct
  and ListPullRequestReviewComments to the Client interface
- internal/forge/github/github.go: implement the method using
  the GitHub "List review comments" REST API with pagination
- internal/forge/gitlab/mr.go: return ErrNotSupported (GitLab
  notes lack a minimize/hide feature)
- internal/forge/fake.go: add PRReviewComments field and fake
  implementation for testing
- internal/cli/postreview.go: add minimizeStaleInlineComments()
  and call it in submitFormalReview before posting new findings
- internal/cli/postreview_test.go: add tests covering own-comment
  filtering, no-op, error tolerance, GitLab ErrNotSupported skip,
  and full integration through submitFormalReview

Note: pre-commit could not run in sandbox (network restrictions).
The post-script runs pre-commit authoritatively on the runner.

Closes #836
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 2, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:45 AM UTC · Completed 9:56 AM UTC
Commit: 4c50530 · View workflow run →

@fullsend-ai-review

Copy link
Copy Markdown

Looks good to me

@fullsend-ai-review fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label Aug 2, 2026
@guyoron1

guyoron1 commented Aug 2, 2026

Copy link
Copy Markdown
Owner

/fs-fix

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 2, 2026

Copy link
Copy Markdown
Author

🤖 Finished Fix · ✅ Success · Started 10:32 AM UTC · Completed 10:43 AM UTC
Commit: 4c50530 · View workflow run →

@fullsend-ai-coder

Copy link
Copy Markdown
Author

🔧 Fix agent — iteration 1 (human-triggered)

No changes made. The review bot approved the PR with no findings, and the human /fs-fix command did not include specific instructions. All PR-specific tests pass, go vet is clean, and secret scan passed. Two pre-existing test failures in run_test.go (TestPostScriptEnv_NoSchemaAppendedWhenEmpty, TestPostScriptEnv_NoSchemaAppendedWhenNoValidationLoop) are caused by the sandbox environment having FULLSEND_OUTPUT_SCHEMA set and are unrelated to this PR.

Disagreed (1):

  1. no review findings to address: The review bot approved the PR with 'Looks good to me' and the human /fs-fix command did not include a specific instruction. All PR-specific tests pass (6 new tests for inline comment minimization, all 55 existing postreview tests, all forge package tests). go vet and go build are clean. Secret scan passed. No code changes are warranted.

Tests: passed

Decision points
  • Whether to make any changes given no review findings and no human instruction (alternatives: Make no changes, Attempt to find and fix issues proactively; rationale: The review bot approved the PR. The human /fs-fix command had no instruction text. The fix agent's scope is strictly limited to addressing review feedback, and no feedback was provided. Making no changes is the correct conservative interpretation.)

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

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

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.

@github-actions github-actions Bot added the stale label Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge All reviewers approved — ready to merge stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Review agent posts excessive inline comments on re-review — 54 comments on a 78-line file across 6 runs

1 participant