Feat/pr conversation comments by ArnabChatterjee20k · Pull Request #141 · appwrite/claudear

ArnabChatterjee20k · 2026-08-11T07:32:08Z

What does this PR do?

(Provide a description of what this PR does.)

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)

Related PRs and Issues

(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)

Have you read the Contributing Guidelines on issues?

(Write your answer here.)

greptile-apps · 2026-08-11T07:35:20Z

Greptile Summary

The PR adds GitHub PR conversation comments to the review-feedback pipeline and introduces durable, namespaced comment recovery. It also changes red-green processing to fail closed when reproduction cannot be confirmed.

Adds separate polling cursors for PR conversation comments.
Rebuilds the review-comment ledger around namespaced inline and conversation IDs.
Uses per-batch comment references for acknowledgement and retry accounting.
Aborts and cleans up red-green runs when no test tool is available or the red-phase agent fails.

Confidence Score: 4/5

The PR should not merge until shared batch failures stop exhausting retries for valid comments that have not been processed successfully.

The namespace, acknowledgement, cursor-recovery, and red-green issues are fixed, but the combined PR action still charges every comment when any feedback in the batch fails; valid comments can therefore reach the limit and be marked handled without successful processing.

Files Needing Attention: crates/claudear-engine/src/watcher.rs, crates/claudear-storage/src/sqlite.rs

Important Files Changed

Filename	Overview
crates/claudear-integrations/src/scm.rs	Adds conversation-comment polling, record-before-cursor ordering, namespaced recovery deduplication, and re-surfacing of unhandled comments.
crates/claudear-engine/src/watcher.rs	Replaces PR-wide acknowledgement with exact batch references, but shared batch failures still charge valid comments alongside failing feedback.
crates/claudear-storage/src/sqlite.rs	Implements namespaced ledger storage and targeted updates, while retaining shared retry increments that can retire valid batch siblings.
migrations/V9__pr_review_states_issue_comments.sql	Adds conversation cursors and rebuilds the comment ledger with a composite namespace-and-ID uniqueness constraint.
crates/claudear-engine/src/processing.rs	Correctly fails closed and cleans up when mandatory red-phase reproduction cannot be confirmed.
crates/claudear-integrations/src/github.rs	Adds paginated retrieval and mapping for GitHub PR conversation comments.

_{Reviews (11): Last reviewed commit: "linting" | Re-trigger Greptile}

ArnabChatterjee20k · 2026-08-11T12:28:31Z

+-- transient shadow table, recreate the real table with the namespaced schema,
+-- copy the rows back, drop the shadow. Nothing references pr_review_comments, so
+-- this is FK-safe; every pre-existing row is an inline review comment.
+CREATE TABLE pr_review_comments_shadow AS SELECT * FROM pr_review_comments;


shadow table migration

Add last_issue_comment_id/last_issue_comment_time to PrReviewState and the pr_review_states table (migration V9). PR conversation comments come from the issues/{n}/comments endpoint, a distinct GitHub comment id space from inline review comments, so they need their own polling cursor.

@claudear

Fetch a PR's issues/{n}/comments timeline so a plain "@claudear fix this" left on the PR conversation (not an inline review comment or formal review) is picked up. Trigger-matched comments emit CommentsAdded and flow through the existing feedback path, which pushes to the same PR branch. Advances a separate issue-comment cursor and forwards the new ScmProvider methods through InstrumentedScm.

Dispatch GitHub issue_comment events to a new handler that ignores non-PR issues, requires the review trigger, and re-polls via check_for_pr so PR conversation comments are addressed in real time instead of waiting for the next poll cycle.

Extend V9 with handled_at/attempts on pr_review_comments and add the tracker methods that make the ledger the authority for outstanding review feedback: get_unhandled_pr_review_comments, mark_pr_review_comments_handled, and note_pr_review_comment_failure (bumps attempts, gives up at the cap).

Re-surface any recorded-but-unhandled comments each poll so a crash or a failed fix run between detecting a comment and acting on it no longer drops it; the ledger, not the polling cursor, is the authority. The engine marks a PR's comments handled only after process_review_action succeeds, counts failures toward the give-up cap, and closes the ledger for terminal PRs.

The issue-comment cursor was saved before the comment was written to the recovery ledger, so a crash or a failed record in that window advanced the cursor past a comment the ledger never captured, permanently dropping it. Record actionable comments first and hold the cursor back when any record fails, so the comment is re-fetched next poll. Matches the inline path.

Marking every unhandled comment on a PR handled after a batch succeeded could clear a comment recorded concurrently (e.g. a webhook check_for_pr) while process_review_action was awaiting, dropping its work. Carry the batch's comment ids through group_review_feedback_by_pr and acknowledge (or count failures against) exactly those ids. PR-wide handling is kept only for terminal PRs.

require_red_green is an enforcement gate, but two paths fell through into the mutation pipeline unguarded: no test tool detected, and a red-phase agent error. Both now abort the attempt (record, mark failed, clean up the worktree, return Failed), matching the existing not-reproduced path, so a fix never proceeds without a confirmed failing reproduction.

The inline/standalone path recorded before saving its cursor, but ignored record errors and advanced the cursor anyway — a failed write left the comment neither in the ledger nor re-fetchable, dropping it. Track record success and hold the inline cursor when any write fails, so the comment is re-fetched next poll. Matches the conversation path.

…ycle P1-a: inline and conversation comments come from distinct GitHub id sequences, but pr_review_comments keyed uniqueness on scm_comment_id alone, so a colliding id let one overwrite the other (inheriting handled state). Rebuild the table in V9 (shadow-copy the rows, recreate with comment_kind and UNIQUE(comment_kind, scm_comment_id), copy back) and set comment_kind on record. A column-level UNIQUE can't be dropped in place, hence the rebuild. P1-b: a batch failure can't be blamed on one comment, so note_failure now retires only the single worst offender (most attempts, at cap) per cycle instead of the whole over-cap set. A valid comment that co-occurs with a poison one drains out on its own merits rather than being dropped alongside.

After namespacing the ledger by (comment_kind, scm_comment_id), the by-ids acknowledge/failure updates still matched on id alone, so on a collision they hit both the inline and conversation rows: processing one marked the other handled, and a failure charged both. Carry the kind alongside each id (ReviewEvent::comment_refs) and match (comment_kind, scm_comment_id) pairs in mark/note-failure so each op touches only its own namespace row.

The re-surface dedup used an id-only seen set, so an inline and a conversation comment sharing a numeric id collapsed — whichever ledger row was seen second was never re-emitted, leaving it permanently unhandled. Key the dedup on (id, comment_kind), matching the namespaced ledger.

ArnabChatterjee20k changed the base branch from main to decision-change-during-human-in-loop August 11, 2026 07:32