Refactor review posting logic to consolidate inline comments

action/src/comment-handler.ts

@@ -151,10 +151,7 @@ export async function handleComment(context: Context): Promise<void> {
 
       const { findings, summary } = await runFastReview(prContext);
 
-      if (findings.length > 0) {
-        await poster.postReview(prNumber, findings);
-      }
-      await poster.postSummaryComment(prNumber, summary);
+      await poster.postReviewResults(prNumber, prContext, findings, summary);
     } catch (err) {
       const msg = err instanceof Error ? err.message : String(err);
       await poster.postAcknowledgement(prNumber, `[ERROR] **OpenReview** — Review failed: ${msg}`);

action/src/pr-handler.ts

@@ -82,15 +82,11 @@ export async function handlePullRequest(context: Context): Promise<void> {
 
     const { findings, summary } = await runFastReview(prContext);
 
-    // Post batch review with inline comments
-    if (findings.length > 0) {
-      await poster.postReview(prNumber, findings);
-    }
-
-    // Post or update summary comment
-    await poster.postSummaryComment(prNumber, summary);
+    const finalSummary = await poster.postReviewResults(prNumber, prContext, findings, summary);
 
-    core.info(`Review complete: ${summary.totalFindings} findings in ${summary.duration}`);
+    core.info(
+      `Review complete: ${finalSummary.totalFindings} open, ${finalSummary.resolvedCount ?? 0} resolved (${finalSummary.duration})`,
+    );
   } catch (err) {
     const msg = err instanceof Error ? err.message : String(err);
     core.error(`Review failed: ${msg}`);

cli/src/commands/review.ts

@@ -134,15 +134,15 @@ export function registerReviewCommand(program: Command): void {
           }
 
           const poster = new CommentPoster(client);
-
-          if (findings.length > 0) {
-            await poster.postReview(prNumber, findings);
-          }
-
-          await poster.postSummaryComment(prNumber, summary);
+          const finalSummary = await poster.postReviewResults(
+            prNumber,
+            prContext,
+            findings,
+            summary,
+          );
 
           process.stderr.write(
-            `✅ Review posted on PR #${prNumber} (${findings.length} finding${findings.length === 1 ? '' : 's'})\n`,
+            `✅ Review posted on PR #${prNumber} (${finalSummary.totalFindings} open, ${finalSummary.resolvedCount ?? 0} resolved)\n`,
           );
         }
       } catch (err) {

core/src/github/comments.ts

@@ -1,14 +1,23 @@
 import { formatInlineComment, formatSummaryComment } from '../review/formatter.js';
-import type { ReviewFinding, ReviewSummary } from '../review/types.js';
+import {
+  buildReviewState,
+  enrichReviewSummary,
+  parseReviewState,
+  type ReviewState,
+} from '../review/review-state.js';
+import type { PRContext, ReviewFinding, ReviewSummary } from '../review/types.js';
 
 import type { GitHubClient } from './client.js';
 
 // Re-export types and formatters so existing consumers don't break
 export type { ReviewFinding, ReviewSummary };
 export type { FindingCategory, FindingSeverity, FindingSource, Citation } from '../review/types.js';
 export { formatInlineComment, formatSummaryComment };
+export { parseReviewState, enrichReviewSummary, buildReviewState };
+export type { ReviewState };
 
-const SUMMARY_MARKER = '<!-- openreview-summary -->';
+export const REVIEW_SUMMARY_MARKER = '<!-- openreview-summary -->';
+export const COVERAGE_SUMMARY_MARKER = '<!-- openreview:coverage -->';
 
 /* ------------------------------------------------------------------ */
 /*  Comment Poster                                                     */
@@ -40,25 +49,57 @@ export class CommentPoster {
     );
   }
 
+  /**
+   * Incremental review post: only new inline comments, summary updated with
+   * resolved/open tracking across commits (gitar-bot style).
+   */
+  async postReviewResults(
+    prNumber: number,
+    pr: PRContext,
+    findings: ReviewFinding[],
+    baseSummary: ReviewSummary,
+  ): Promise<ReviewSummary> {
+    const previousState = await this.fetchReviewState(prNumber);
+    const summary = enrichReviewSummary(pr, findings, baseSummary, previousState);
+    const state = buildReviewState(pr, findings);
+
+    const newFindings = summary.newFindings ?? findings;
+    if (newFindings.length > 0) {
+      await this.postReview(prNumber, newFindings);
+    }
+
+    await this.postSummaryComment(prNumber, summary, state);
+    return summary;
+  }
+
   /**
    * Post or update the summary comment.
    * Uses replace-not-duplicate strategy via the HTML marker tag.
    */
-  async postSummaryComment(prNumber: number, summary: ReviewSummary): Promise<void> {
-    const body = formatSummaryComment(summary);
-    const existingId = await this.findSummaryComment(prNumber);
+  async postSummaryComment(
+    prNumber: number,
+    summary: ReviewSummary,
+    state?: ReviewState,
+  ): Promise<void> {
+    const body = formatSummaryComment(summary, state);
+    await this.postOrUpdateMarkedComment(prNumber, REVIEW_SUMMARY_MARKER, body);
+  }
 
-    if (existingId) {
-      await this.client['api'].patch(
-        `/repos/${this.client.owner}/${this.client.repo}/issues/comments/${existingId}`,
-        { body },
-      );
-    } else {
-      await this.client['api'].post(
-        `/repos/${this.client.owner}/${this.client.repo}/issues/${prNumber}/comments`,
-        { body },
-      );
-    }
+  /**
+   * Post or update the coverage summary on the original PR.
+   * Replaces the previous coverage comment instead of posting a new one each push.
+   */
+  async postCoverageComment(prNumber: number, body: string): Promise<void> {
+    const content = body.includes(COVERAGE_SUMMARY_MARKER)
+      ? body
+      : `${COVERAGE_SUMMARY_MARKER}\n${body}`;
+    await this.postOrUpdateMarkedComment(prNumber, COVERAGE_SUMMARY_MARKER, content);
+  }
+
+  /** Load persisted review state from the existing summary comment, if any. */
+  async fetchReviewState(prNumber: number): Promise<ReviewState | null> {
+    const body = await this.fetchMarkedCommentBody(prNumber, REVIEW_SUMMARY_MARKER);
+    return body ? parseReviewState(body) : null;
   }
 
   /** Reply in a PR thread by posting a new issue comment. */
@@ -79,19 +120,77 @@ export class CommentPoster {
 
   /* ---- Internal helpers ---- */
 
-  private async findSummaryComment(prNumber: number): Promise<number | null> {
+  private async postOrUpdateMarkedComment(
+    prNumber: number,
+    marker: string,
+    body: string,
+  ): Promise<void> {
+    const latestIssue = await this.fetchLatestIssueComment(prNumber);
+    const marked = await this.findMarkedCommentRecord(prNumber, marker);
+
+    // Update in place only when our summary is already the newest timeline comment.
+    // Otherwise append a fresh comment so results are visible at the bottom.
+    if (marked && latestIssue && marked.id === latestIssue.id) {
+      await this.client['api'].patch(
+        `/repos/${this.client.owner}/${this.client.repo}/issues/comments/${marked.id}`,
+        { body },
+      );
+      return;
+    }
+
+    await this.client['api'].post(
+      `/repos/${this.client.owner}/${this.client.repo}/issues/${prNumber}/comments`,
+      { body },
+    );
+  }
+
+  private async fetchLatestIssueComment(
+    prNumber: number,
+  ): Promise<{ id: number; body: string } | null> {
+    const { data } = await this.client['api'].get(
+      `/repos/${this.client.owner}/${this.client.repo}/issues/${prNumber}/comments`,
+      { params: { per_page: 1, page: 1, sort: 'created', direction: 'desc' } },
+    );
+
+    const comment = data[0];
+    if (!comment || typeof comment.body !== 'string') return null;
+    return { id: comment.id, body: comment.body };
+  }
+
+  private async fetchMarkedCommentBody(
+    prNumber: number,
+    marker: string,
+  ): Promise<string | null> {
+    const comment = await this.findMarkedCommentRecord(prNumber, marker);
+    return typeof comment?.body === 'string' ? comment.body : null;
+  }
+
+  private async findMarkedComment(prNumber: number, marker: string): Promise<number | null> {
+    const comment = await this.findMarkedCommentRecord(prNumber, marker);
+    return comment?.id ?? null;
+  }
+
+  /**
+   * Return the newest issue comment containing `marker`.
+   * Older duplicates are left untouched — we always refresh the latest one so
+   * updates appear near the bottom of busy PR threads.
+   */
+  private async findMarkedCommentRecord(
+    prNumber: number,
+    marker: string,
+  ): Promise<{ id: number; body: string } | null> {
     let page = 1;
     const perPage = 100;
 
     while (true) {
       const { data } = await this.client['api'].get(
         `/repos/${this.client.owner}/${this.client.repo}/issues/${prNumber}/comments`,
-        { params: { per_page: perPage, page } },
+        { params: { per_page: perPage, page, sort: 'created', direction: 'desc' } },
       );
 
       for (const comment of data) {
-        if (typeof comment.body === 'string' && comment.body.includes(SUMMARY_MARKER)) {
-          return comment.id;
+        if (typeof comment.body === 'string' && comment.body.includes(marker)) {
+          return { id: comment.id, body: comment.body };
         }
       }
 

core/src/index.ts

@@ -14,13 +14,23 @@ export type { ParsedDiff, Hunk, Line, MoveEvent } from './github/diff.js';
 export { CommentPoster } from './github/comments.js';
 
 // Review
-export { runFastReview } from './review/fast-review.js';
+export { runFastReview, buildFastReviewSummary } from './review/fast-review.js';
+export { fingerprintPullRequestDiff, normalizeDiffForFingerprint, extractDiffForFiles } from './review/diff-fingerprint.js';
+export { isReviewableFile, filterReviewableFiles } from './review/reviewable-files.js';
 export { runRLM } from './review/rlm-runner.js';
 export type { RLMEvent, RLMEventHandler, RLMEventType } from './review/rlm-runner.js';
 export { SnapshotBuilder } from './review/snapshot.js';
 export type { SnapshotOptions } from './review/snapshot.js';
 export { runLinters, deduplicateFindings } from './review/linters.js';
 export { formatInlineComment, formatSummaryComment } from './review/formatter.js';
+export {
+  parseReviewState,
+  enrichReviewSummary,
+  buildReviewState,
+  fingerprintFinding,
+  diffReviewFindings,
+} from './review/review-state.js';
+export type { ReviewState, StoredFinding, ReviewDiff } from './review/review-state.js';
 export { sortFindings } from './review/types.js';
 export type {
   ReviewFinding,

core/src/review/diff-fingerprint.ts

@@ -0,0 +1,44 @@
+import { createHash } from 'node:crypto';
+
+import { filterReviewableFiles } from './reviewable-files.js';
+
+/**
+ * Stable SHA-256 fingerprint for a PR's reviewable patch.
+ * Same diff on different PR numbers/commits yields the same hash.
+ */
+export function fingerprintPullRequestDiff(diff: string, files: string[]): string {
+  const reviewableFiles = filterReviewableFiles(files);
+  const filteredDiff = extractDiffForFiles(diff, reviewableFiles);
+  const normalized = normalizeDiffForFingerprint(filteredDiff);
+  const payload = `${reviewableFiles.join('\n')}\n---\n${normalized}`;
+  return createHash('sha256').update(payload, 'utf8').digest('hex');
+}
+
+export function extractDiffForFiles(rawDiff: string, files: string[]): string {
+  if (files.length === 0) return '';
+
+  const want = new Set(files);
+  const parts = rawDiff.split(/^(?=diff --git )/m);
+  const kept: string[] = [];
+
+  for (const part of parts) {
+    if (!part.trim()) continue;
+    const match = /^diff --git a\/(.+?) b\/(.+?)$/m.exec(part);
+    if (match && want.has(match[2])) {
+      kept.push(part);
+    }
+  }
+
+  return kept.join('');
+}
+
+/** Normalize diff text so equivalent patches fingerprint the same. */
+export function normalizeDiffForFingerprint(diff: string): string {
+  return diff
+    .replace(/\r\n/g, '\n')
+    .replace(/^@@ -\d+(?:,\d+)? \+\d+(?:,\d+)? @@/gm, '@@')
+    .split('\n')
+    .map((line) => line.trimEnd())
+    .join('\n')
+    .trim();
+}

core/src/review/fast-review.ts

@@ -6,6 +6,7 @@ import { config } from '../config/env.js';
 import { createMainLLM, createStructuredLLM } from '../llm/router.js';
 
 import { deduplicateFindings, runLinters } from './linters.js';
+import { isReviewableFile } from './reviewable-files.js';
 import type {
   FindingSeverity,
   PRContext,
@@ -102,37 +103,6 @@ function normalizeCategory(raw: string): 'bug' | 'flag' {
   return CATEGORY_BUG_KEYWORDS.some((kw) => s.includes(kw)) ? 'bug' : 'flag';
 }
 
-/* ------------------------------------------------------------------ */
-/*  Non-reviewable file patterns                                       */
-/* ------------------------------------------------------------------ */
-
-const SKIP_PATTERNS = [
-  /^yarn\.lock$/,
-  /^pnpm-lock\.yaml$/,
-  /^package-lock\.json$/,
-  /\.lock$/,
-  /\.min\.(js|css)$/,
-  /\.map$/,
-  /\.snap$/,
-  /\.svg$/,
-  /\.png$/,
-  /\.jpg$/,
-  /\.jpeg$/,
-  /\.gif$/,
-  /\.ico$/,
-  /\.woff2?$/,
-  /\.ttf$/,
-  /\.eot$/,
-  /\.pdf$/,
-  /dist\//,
-  /\.generated\./,
-  /vendor\//,
-];
-
-function isReviewableFile(filename: string): boolean {
-  return !SKIP_PATTERNS.some((pattern) => pattern.test(filename));
-}
-
 /* ------------------------------------------------------------------ */
 /*  Diff chunking                                                      */
 /* ------------------------------------------------------------------ */
@@ -286,7 +256,7 @@ export async function runFastReview(
     }
   }
 
-  const summary = buildSummary(sorted, pr.files.length, duration);
+  const summary = buildFastReviewSummary(sorted, pr.files.length, duration);
 
   return { findings: sorted, summary, trace };
 }
@@ -1009,7 +979,7 @@ function validateWithSnap(
 /*  Summary builder                                                    */
 /* ------------------------------------------------------------------ */
 
-function buildSummary(
+export function buildFastReviewSummary(
   findings: ReviewFinding[],
   fileCount: number,
   durationMs: number,