Repository navigation
feat: cross-run finding dedup + updatable summary on re-run - #8
Merged
Merged
Conversation
Each push to a PR no longer duplicates review findings. Inline comment bodies now carry a hidden anchor (file:category:title-hash) for cross-run dedup. On re-runs, existing anchors are fetched and matched — only genuinely new findings get posted. An updatable issue comment summarizes re-run results (PATCHed in place on subsequent runs). Three-branch posting logic in orchestrate.ts: - First run: unchanged behavior (body + inline comments) - Re-run with new findings: inline-only review + upsert summary - Re-run with zero new: upsert summary only, no review posted GithubClient gains fetchReviewComments (paginated, 10-page cap) and upsertSummaryComment (search-by-anchor, create-or-update). The anchor fetch is wrapped in try/catch — degrades to first-run on failure. 235 tests (24 new), 0 lint warnings, clean build + Docker. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- extractAnchorKeys currently only captures the first matching anchor per comment body; if you expect multiple findings per comment, consider using a global regex or iterative matching so all keys are collected.
- In upsertSummaryComment, the
totalFetchedvalue logged when the page cap is reached is computed aspage * PER_PAGE, which can over-count if the last page is not full; using the actual accumulatedparsed.data.lengthor a running total would make this diagnostic more accurate.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- extractAnchorKeys currently only captures the first matching anchor per comment body; if you expect multiple findings per comment, consider using a global regex or iterative matching so all keys are collected.
- In upsertSummaryComment, the `totalFetched` value logged when the page cap is reached is computed as `page * PER_PAGE`, which can over-count if the last page is not full; using the actual accumulated `parsed.data.length` or a running total would make this diagnostic more accurate.
## Individual Comments
### Comment 1
<location path="src/github/client.ts" line_range="110" />
<code_context>
+
+const issueCommentListSchema = z.array(
+ z.object({
+ id: z.int().positive(),
+ body: z.string(),
+ html_url: z.string(),
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `z.int()` will fail at runtime because Zod does not expose an `int()` primitive.
This will throw at module load time because Zod only supports `z.number().int()`, not `z.int()`. Please update this (and other uses of `z.int()`) to `z.number().int().positive()` to prevent a runtime crash on import.
</issue_to_address>
### Comment 2
<location path="src/github/__tests__/client.test.ts" line_range="435-444" />
<code_context>
+describe("fetchReviewComments", () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests for malformed issue comment responses in upsertSummaryComment
You already test malformed response shapes for `fetchReviewComments` (`"unexpected review comments response shape"`). For consistency with the implementation, please add analogous tests for `upsertSummaryComment`:
- If `issues.listComments` returns a non-array or otherwise invalid shape, assert it throws `"unexpected issue comments response shape"`.
- If `issues.updateComment` or `issues.createComment` return a payload that fails `issueCommentResponseSchema`, assert the corresponding `"unexpected issue comment response shape"` error is thrown.
This will exercise the validation branches for Octokit/stub shape changes and ensure errors are thrown rather than failing silently.
Suggested implementation:
```typescript
describe("fetchReviewComments", () => {
it("returns mapped path and body from review comments", async () => {
const stub = makeOctokitStub({
listReviewCommentsResponses: [
{
data: [
{ path: "src/a.ts", body: "comment 1" },
{ path: "src/b.ts", body: "comment 2" },
],
},
],
})
const client = makeClient({ octokit: stub.octokit })
const result = await client.fetchReviewComments({ /* existing args */ })
expect(result).toEqual([
{ path: "src/a.ts", body: "comment 1" },
{ path: "src/b.ts", body: "comment 2" },
])
})
it("throws on unexpected review comments response shape", async () => {
const stub = makeOctokitStub({
listReviewCommentsResponses: [
{
data: { not: "an array" },
},
],
})
const client = makeClient({ octokit: stub.octokit })
await expect(
client.fetchReviewComments({ /* existing args */ }),
).rejects.toThrow("unexpected review comments response shape")
})
})
describe("upsertSummaryComment", () => {
it("throws on unexpected issue comments response shape from listComments", async () => {
const stub = makeOctokitStub({
// issues.listComments returns a non-array / invalid shape
listIssueCommentsResponses: [
{
data: { not: "an array" },
},
],
})
const client = makeClient({ octokit: stub.octokit })
await expect(
client.upsertSummaryComment({
/* use the same arg shape as other upsertSummaryComment tests */
owner: "acme",
repo: "demo",
pullNumber: 42,
body: "summary body",
}),
).rejects.toThrow("unexpected issue comments response shape")
})
it("throws on unexpected issue comment response shape from updateComment", async () => {
const stub = makeOctokitStub({
// First call finds an existing summary comment so we go down the update path
listIssueCommentsResponses: [
{
data: [
{
id: 123,
body: "existing summary",
},
],
},
],
// issues.updateComment returns payload that fails issueCommentResponseSchema
updateIssueCommentResponses: [
{
data: { invalid: "shape" },
},
],
})
const client = makeClient({ octokit: stub.octokit })
await expect(
client.upsertSummaryComment({
owner: "acme",
repo: "demo",
pullNumber: 42,
body: "updated summary body",
}),
).rejects.toThrow("unexpected issue comment response shape")
})
it("throws on unexpected issue comment response shape from createComment", async () => {
const stub = makeOctokitStub({
// No existing summary comment so we go down the create path
listIssueCommentsResponses: [
{
data: [],
},
],
// issues.createComment returns payload that fails issueCommentResponseSchema
createIssueCommentResponses: [
{
data: { invalid: "shape" },
},
],
})
const client = makeClient({ octokit: stub.octokit })
await expect(
client.upsertSummaryComment({
owner: "acme",
repo: "demo",
pullNumber: 42,
body: "new summary body",
}),
).rejects.toThrow("unexpected issue comment response shape")
})
```
1. Align the `fetchReviewComments` calls and expectations in the modified block with the existing tests (replace the `/* existing args */` placeholder with the actual argument object used elsewhere in the file).
2. Ensure the `upsertSummaryComment` argument object (`owner`, `repo`, `pullNumber`, `body`) matches the real function signature and any required fields already used in other tests (e.g. `summaryHeader`, `commitSha`, or similar, if present).
3. Verify the stub keys (`listIssueCommentsResponses`, `updateIssueCommentResponses`, `createIssueCommentResponses`) correspond to the actual options supported by `makeOctokitStub`. If they differ (e.g. `issuesListCommentsResponses`, `issuesUpdateCommentResponses`), rename them accordingly.
4. Confirm the error message strings thrown by `upsertSummaryComment` in the implementation are exactly `"unexpected issue comments response shape"` for the list path and `"unexpected issue comment response shape"` for the create/update paths; adjust the `toThrow` strings if the implementation uses slightly different wording.
</issue_to_address>
### Comment 3
<location path="src/github/__tests__/client.test.ts" line_range="466-475" />
<code_context>
+ ])
+ })
+
+ it("paginates when a page is full", async () => {
+ const fullPage = Array.from({ length: 100 }, (_, index) => ({
+ path: `src/file-${index}.ts`,
+ body: `body-${index}`,
+ }))
+ const lastPage = [{ path: "src/last.ts", body: "last" }]
+ const stub = makeOctokitStub({
+ listReviewCommentsResponses: [{ data: fullPage }, { data: lastPage }],
+ })
+ const { client } = makeClient(stub)
+
+ const comments = await client.fetchReviewComments({ prNumber: 7 })
+
+ expect(comments).toHaveLength(101)
+ expect(stub.listReviewCommentsCalls).toHaveLength(2)
+ expect(stub.listReviewCommentsCalls[1]).toMatchObject({ page: 2 })
+ })
+
</code_context>
<issue_to_address>
**suggestion (testing):** Consider a test that hits the MAX_PAGES cap and warns
The current pagination test only verifies fetching a second page, but doesn’t cover the `MAX_PAGES` cap or the `logger.warn("review comments page cap reached", ...)` path in `fetchReviewComments`. Please add a test that stubs `listReviewCommentsResponses` with `MAX_PAGES` full pages and asserts:
- `listReviewComments` is called exactly `MAX_PAGES` times
- The last call uses `page: MAX_PAGES`
- All collected comments are still returned
If possible, also assert that the warning is logged to guard against regressions in the cap behavior.
Suggested implementation:
```typescript
it("paginates when a page is full", async () => {
const fullPage = Array.from({ length: 100 }, (_, index) => ({
path: `src/file-${index}.ts`,
body: `body-${index}`,
}))
const lastPage = [{ path: "src/last.ts", body: "last" }]
const stub = makeOctokitStub({
listReviewCommentsResponses: [{ data: fullPage }, { data: lastPage }],
})
const { client } = makeClient(stub)
const comments = await client.fetchReviewComments({ prNumber: 7 })
expect(comments).toHaveLength(101)
expect(stub.listReviewCommentsCalls).toHaveLength(2)
expect(stub.listReviewCommentsCalls[1]).toMatchObject({ page: 2 })
})
it("hits the review comments MAX_PAGES cap and warns", async () => {
const fullPage = Array.from({ length: 100 }, (_, index) => ({
path: `src/file-${index}.ts`,
body: `body-${index}`,
}))
// We construct MAX_PAGES full pages plus one extra page that should not be fetched
const responses = Array.from({ length: MAX_PAGES + 1 }, (_, pageIndex) => {
const isExtraPage = pageIndex === MAX_PAGES
return {
data: isExtraPage
? [{ path: `src/extra-${pageIndex}.ts`, body: `extra-${pageIndex}` }]
: fullPage,
}
})
const stub = makeOctokitStub({
listReviewCommentsResponses: responses,
})
const { client, logger } = makeClient(stub)
const comments = await client.fetchReviewComments({ prNumber: 7 })
// We should only call listReviewComments up to the MAX_PAGES cap
expect(stub.listReviewCommentsCalls).toHaveLength(MAX_PAGES)
expect(stub.listReviewCommentsCalls[MAX_PAGES - 1]).toMatchObject({
page: MAX_PAGES,
})
// All comments up to the cap should be returned
expect(comments).toHaveLength(MAX_PAGES * fullPage.length)
// The cap behaviour should emit a warning
expect(logger.warn).toHaveBeenCalledWith(
"review comments page cap reached",
expect.objectContaining({
prNumber: 7,
maxPages: MAX_PAGES,
fetchedPages: MAX_PAGES,
}),
)
})
```
To make this test compile and behave as intended, you may need to:
1. Export `MAX_PAGES` from the `src/github/client.ts` module (or wherever `fetchReviewComments` is defined) and import it at the top of `src/github/__tests__/client.test.ts`, e.g.:
- `import { MAX_PAGES } from "../client"` (adjust the path as needed).
2. Ensure `makeClient` returns a `logger` mock with a `warn` method:
- If it currently only returns `{ client }`, update `makeClient` to accept a logger (defaulting to a Jest mock) and return it alongside `client`, so that `const { client, logger } = makeClient(stub)` works and `logger.warn` is a Jest mock that can be asserted against.
3. If your logger's `warn` signature differs from the one used in `fetchReviewComments`, align the expectation in the test (the `expect(logger.warn).toHaveBeenCalledWith(...)` line) with the actual arguments passed in your implementation.
</issue_to_address>
### Comment 4
<location path="src/__tests__/orchestrate.test.ts" line_range="683-684" />
<code_context>
})
})
+
+ describe("cross-run dedup", () => {
+ it("first run — posts normal review, no summary comment", async () => {
+ const stubs = makeOrchestrateDeps()
+ const logger = createTestLogger()
</code_context>
<issue_to_address>
**suggestion (testing):** Assert findingsCount for first-run orchestration to guard the new return value semantics
With `orchestrate` now returning `findingsCount: newFindings.length`, `findingsCount` represents "posted new findings" on re-runs rather than "selected findings." The first-run test currently only checks call patterns, not this return value.
To make the new semantics explicit and protect against regressions, please add an assertion in `"first run — posts normal review, no summary comment"` on the returned `findingsCount`, e.g.:
```ts
const result = await orchestrate(stubs.deps, logger)
expect(result.findingsCount).toBe(expectedSelection.selected.length)
```
(or whatever the correct selected count is for the fixture).
Suggested implementation:
```typescript
it("first run — posts normal review, no summary comment", async () => {
const stubs = makeOrchestrateDeps()
const logger = createTestLogger()
const result = await orchestrate(stubs.deps, logger)
expect(stubs.fetchReviewCommentsCalls).toHaveLength(1)
expect(stubs.submitReviewCalls).toHaveLength(1)
expect(stubs.upsertSummaryCommentCalls).toHaveLength(0)
expect(result.findingsCount).toBe(expectedSelection.selected.length)
})
```
1. Ensure `expectedSelection` is available in this test file and correctly represents the selection used during the first-run orchestration. If it is not yet defined/imported:
- Either import it from the appropriate fixture module, or
- Derive it from existing fixtures already used in this test suite (e.g., whatever is driving `fixtureReviewResponse` for the first run).
2. If the correct expected count is represented by a different symbol than `expectedSelection.selected.length`, update the assertion accordingly, e.g. `expect(result.findingsCount).toBe(<correctExpectedCount>)`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds deterministic anchors to review comments, GitHub APIs for retrieving review comments and upserting rerun summaries, and orchestration logic that deduplicates findings across runs while covering first-run, rerun, skip, empty, and failure scenarios. ChangesCross-run review deduplication
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/review/comment-mapping.ts (1)
28-28: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a documentation comment for
ANCHOR_PATTERN.The regex constant carries the cross-run dedup contract (the capture group is the anchor key), but has no explanatory comment. As per coding guidelines, "regex constants require documentation comments."
📝 Proposed doc comment
+/** Matches the hidden anchor embedded in inline comment bodies; capture group 1 + * is the dedup key produced by `computeAnchorKey`. */ const ANCHOR_PATTERN = /<!-- umm-actually:(.+?) -->/🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/review/comment-mapping.ts` at line 28, Add a documentation comment directly above ANCHOR_PATTERN explaining that it identifies umm-actually anchors for cross-run deduplication and that its capture group contains the anchor key.Source: Coding guidelines
src/orchestrate.ts (1)
337-368: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
upsertSummaryCommentfailures are unhandled, unlikefetchReviewComments.The dedup fetch degrades gracefully on failure (Step 12.5 try/catch), but the two
upsertSummaryCommentawaits are unguarded. IflistComments/create/update fails aftersubmitReviewhas already posted the review, the whole run rejects even though the review succeeded. Consider wrapping the summary upsert in a warn-and-continue guard for a consistent resilience posture.Please confirm whether a higher-level handler already tolerates a post-review upsert failure (e.g., in the action entrypoint) so a failed summary doesn't mark an otherwise-successful review run as failed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/orchestrate.ts` around lines 337 - 368, Guard both upsertSummaryComment calls in the rerun summary branches with the same warn-and-continue handling used by fetchReviewComments, so failures after submitReview do not reject the successful review run. Include the caught error and relevant context in the warning, and verify the action entrypoint does not convert this post-review summary failure into an overall run failure.src/__tests__/orchestrate.test.ts (1)
754-757: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer an exact body assertion here.
The rerun-filter test (Line 724) already asserts the full body via
buildRerunSummary; this "all duplicates" case only checks a substring. Asserting the whole deterministic body would catch regressions in the zero-new-findings summary. As per coding guidelines, "Prefer exact assertions and assert whole deterministic values rather than substrings."💚 Proposed exact assertion
- const summaryCall = first(stubs.upsertSummaryCommentCalls) - expect(summaryCall.body).toContain("No new findings") + const summaryCall = first(stubs.upsertSummaryCommentCalls) + expect(summaryCall.body).toBe( + buildRerunSummary({ + sha: fixturePrContext.headSha, + newCount: 0, + totalCount: findings.length, + model: "test/model", + }), + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/orchestrate.test.ts` around lines 754 - 757, Update the all-duplicates test assertion for summaryCall.body to compare the complete deterministic summary string, using the existing summary-building helper or expected value rather than toContain. Preserve the test’s verification that the zero-new-findings summary is emitted.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/__tests__/orchestrate.test.ts`:
- Around line 754-757: Update the all-duplicates test assertion for
summaryCall.body to compare the complete deterministic summary string, using the
existing summary-building helper or expected value rather than toContain.
Preserve the test’s verification that the zero-new-findings summary is emitted.
In `@src/orchestrate.ts`:
- Around line 337-368: Guard both upsertSummaryComment calls in the rerun
summary branches with the same warn-and-continue handling used by
fetchReviewComments, so failures after submitReview do not reject the successful
review run. Include the caught error and relevant context in the warning, and
verify the action entrypoint does not convert this post-review summary failure
into an overall run failure.
In `@src/review/comment-mapping.ts`:
- Line 28: Add a documentation comment directly above ANCHOR_PATTERN explaining
that it identifies umm-actually anchors for cross-run deduplication and that its
capture group contains the anchor key.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f4cd41a1-1f57-4cd2-ac86-e95fa80b9596
📒 Files selected for processing (6)
src/__tests__/orchestrate.test.tssrc/github/__tests__/client.test.tssrc/github/client.tssrc/orchestrate.tssrc/review/__tests__/comment-mapping.test.tssrc/review/comment-mapping.ts
Wrap upsertSummaryComment in try/catch so a failed summary comment does not crash the action after the review is already posted. Update README and action.yml to reflect the shipped dedup feature: move from "In progress" to "Shipped", add dedup step to "How it works", and clarify findings_count output includes cross-run dedup. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![umm-actually[bot]](https://avatars.githubusercontent.com/in/4233716?s=60&v=4){kind=small}
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
umm-actually re-reviewed at No new findings (26 finding(s) from prior reviews). umm-actually · deepseek/deepseek-v4-pro-20260423 |
Replaced substring assertions (toContain, toMatchObject) with exact full-value matches on deterministic outputs. Added missing error-path tests for upsertSummaryComment (malformed list, create, update responses) and a resilience test proving orchestrate continues when upsert fails. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Addresses Sourcery review findings: add a test that exercises the 10-page pagination cap with warning log, and assert findingsCount on the first-run dedup test to guard the new return value semantics. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Owner
Author
Bot review body findings — addressedSourcery "Overall Comments" (review body)
CodeRabbit nitpicks (review body)
🔍 ship-check · pr-monitor · claude-opus-4-6[1m] |
5 tasks done
`if (existingComment)` over `if (existingComment !== undefined)` — the type is `T | undefined`, so the only falsy case is undefined. Explicit undefined checks are only warranted when the value could be another falsy type (null, 0, empty string, false). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- extractAnchorKeys: `match?.[1]` over `match?.[1] !== undefined` - orchestrate: extract `totalCount` const to avoid duplicate computation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Prefer `if (value)` over `if (value !== undefined)` unless distinguishing undefined from other falsy values matters. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Extract error formatting into a named const and include error.name for immediate context in log output. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Return early from the !isRerun branch so re-run logic flows linearly without nesting. Eliminates the 3-branch if/else if/else and the separate if (isRerun) guard around summary upsert. Also strengthens the early-return convention in AGENTS.md. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add anchor assertion to the snapped-finding test so the dedup anchor is verified on comments that include a snap note - Track actual totalFetched count in upsertSummaryComment instead of page * PER_PAGE, matching fetchReviewComments' pattern Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
These constants are only used by fetchReviewComments and upsertSummaryComment — both inside the factory closure. Module-level overstates their visibility. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Return early from inside the pagination loop when the anchor comment
is found, eliminating the let + break + post-loop if pattern
- Consolidate reviewUrlSchema and issueCommentResponseSchema into one
urlResponseSchema — both validate the same { html_url } shape
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2 of 3 tasks
10 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
<!-- umm-actually:file:category:titleHash -->), and re-runs fetch existing comments to skip already-posted findings<!-- umm-actually-rerun -->anchor, PATCHed on subsequent runs)Changes by file
src/review/comment-mapping.ts—computeAnchorKey,extractAnchorKeys,RERUN_ANCHOR,buildRerunSummary, anchor rendering inrenderCommentBodysrc/github/client.ts—OctokitLikeexpanded withlistReviewComments+ 3 issues methods;GithubClientgainsfetchReviewComments(paginated, 10-page cap) andupsertSummaryComment(search-by-anchor create-or-update)src/orchestrate.ts— step 12.5 cross-run dedup (try/catch resilient), 3-branch step 13,findingsCountreturns new findings countDesign decisions
file:category:titleHash(NOT line-based — lines shift between pushes)umm-actually:prefix prevents collisions)fetchReviewCommentswrapped in try/catch — degrades to first-run on permission/network errorsTest plan
npm testfetchReviewCommentsfetchReviewCommentsthrowsOctokitLiketype compatibility test passes (real Octokit still assignable)🤖 Generated with Claude Code
Summary by CodeRabbit