feat(feedback): reactions poller — new singleton role + capture (#144 part 2, ADR-0035)

[stephane-segning]

1. Summary

Completes #144 / ADR-0035 (backend): capture 👍/👎 on the bot's comments as a review-quality signal. Since GitHub does not webhook reactions (verified in #161), a single-replica poller role reads them from the REST API and reconciles.

• Capture (at finalize): store the ids of the comments we create — create_issue_comment now returns the reply id; list_review_comments fetches the inline-comment ids (the create-review response omits them), correlated to findings by (file, line). Persisted in review_comments (migration 0015).
• poller role: the control-plane binary gains a poller role — run as ONE replica with the GitHub App key (keeps the credential off the multi-replica serve pods and the dispatcher, ADR-0002). Each POLLER_INTERVAL_SECS (default 300) it lists pollable comments (tasks within POLLER_WINDOW_DAYS, default 14), reads each comment's reactions (pulls/comments vs issues/comments by kind), and reconciles into review_feedback: insert new, delete vanished — the un-react case, no webhook needed. One installation token per installation per cycle.
• Read API: GET /tasks/{id}/feedback (gated review:read), joining file/line for the future dashboard.

Source of truth: #144 / ADR-0035.

---

2. Intent

A 👍/👎 on a finding is the cheapest real measure of review quality — the thing we keep saying we need. This captures it server-side and exposes it via an API; the dashboard (deferred) and the eval dataset (with #143 transcripts) build on it.

---

3. Scope

In Scope

• Comment-id capture, the poller role + reconcile loop, the review_feedback/review_comments schema, the read API.

Out of Scope

• The apps/web dashboard (feedback counts on the run page) — deferred backend-only pass.
• The ai-helm poller Deployment — see deploy follow-up below.

---

4. Verification

• Running automated tests

DATABASE_URL=postgres://lightbridge:lightbridge@localhost:5432/lightbridge cargo test -p control-plane --locked
cargo fmt --check && cargo clippy -p control-plane --all-targets

control-plane: 59 passed (feedback_reconcile: insert / un-react removal / empty-clears / pollable join)
fmt + clippy: clean

The GitHub REST calls (list_review_comments, list_comment_reactions) follow the existing hardcoded-URL client pattern (not unit-tested, like the other github.rs methods); the DB reconcile — the logic that matters — is fully tested.

---

5. Screenshots / Evidence

N/A (backend). Reconcile semantics (insert/remove/empty) covered by the DB test.

---

6. Risk Assessment

Risk level:

• Low-Medium

Potential risks:

• New role holds the GitHub App key (single replica).
• Polling adds GitHub API calls.

Mitigation: one replica only (no double-poll); the App key is isolated to the poller deployment (not spread to serve/dispatcher); polling is bounded by the time window + one token per installation per cycle; every GitHub/DB error in a cycle is logged and retried next tick (never crashes the role).

---

7. Deploy follow-up

Add a poller Deployment to ai-helm: 1 replica, args ["poller"], env DATABASE_URL + GITHUB_APP_ID/GITHUB_APP_PRIVATE_KEY (+ optional POLLER_INTERVAL_SECS/POLLER_WINDOW_DAYS). Until then the capture + API still work; only the reconcile loop is dormant.

---

8. AI Usage Declaration

AI was used for:

• Understanding existing code
• Generating code
• Generating tests

Human verification:

• I understand every meaningful change
• I checked the reconcile (insert/delete) + token-per-installation logic
• I accept responsibility for this PR

---

9. Reviewer Focus

• Security (App key isolated to the single-replica poller; trust boundary)
• Correctness (reconcile insert/delete; one token per installation/cycle)
• Architecture (new role placement)

🤖 Generated with Claude Code

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[github-actions]

✅ AI Governance check passed

This PR declares AI usage, references a source of truth, and provides verification evidence. Thank you.

[gemini-code-assist]

Code Review

This pull request implements ADR-0035 to capture 👍/👎 reactions on the bot's posted comments as a review-quality signal. It introduces a database migration for review_comments and review_feedback tables, database functions to store and reconcile feedback, a new background poller role to periodically query the GitHub API for reactions, and a GET /tasks/{id}/feedback endpoint. The reviewer feedback highlights three important improvement opportunities: implementing deterministic staggered polling in SQL to mitigate GitHub API rate-limiting risks, caching failed token minting attempts in the poller to prevent spamming the API on failures, and logging database write errors when storing reply comment IDs instead of silently ignoring them.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

Rate-Limiting Risk & Scalability Bottleneck

Polling every comment created within the last within_days (default 14) on every cycle (default 5 minutes) will scale linearly with the number of comments. For active repositories, this can easily result in thousands of sequential HTTP requests per cycle, rapidly exhausting the GitHub API rate limit (typically 5,000 requests/hour for installation tokens).

Recommendation: Deterministic Staggered Polling

Instead of adding a new database column or state, we can implement deterministic staggered/tiered polling directly in the SQL query. By using a stable hash of the comment ID and the current timestamp bucketed by the poller interval, we can poll older comments less frequently:

• Comments < 1 day old: Poll on every cycle (every 5 mins).
• Comments 1-3 days old: Poll approximately once per hour (1 in 12 cycles).
• Comments 3-14 days old: Poll approximately once every 6 hours (1 in 72 cycles).

This keeps the API rate limit usage completely flat and safe, with zero extra database writes.

pub async fn list_pollable_comments(
    pool: &PgPool,
    within_days: i32,
) -> Result<Vec<PollableComment>, sqlx::Error> {
    sqlx::query_as::<_, PollableComment>(
        "SELECT rc.task_id, rc.github_comment_id, rc.kind, r.owner, r.name, t.installation_id \
         FROM review_comments rc \
         JOIN tasks t ON t.id = rc.task_id \
         JOIN repositories r ON r.id = t.repository_id \
         WHERE t.created_at > now() - ($1 * interval '1 day') \
           AND ( \
             rc.created_at > now() - interval '1 day' \
             OR ( \
               rc.created_at BETWEEN now() - interval '3 days' AND now() - interval '1 day' \
               AND (rc.github_comment_id % 12) = (extract(epoch from now())::bigint / 300) % 12 \
             ) \
             OR ( \
               rc.created_at < now() - interval '3 days' \
               AND (rc.github_comment_id % 72) = (extract(epoch from now())::bigint / 300) % 72 \
             ) \
           ) \
         ORDER BY rc.created_at",
    )
    .bind(within_days)
    .fetch_all(pool)
    .await
}

[stephane-segning]

Great catch — real prod risk on a busy org. Adopted deterministic age-tiered polling in the SQL (0db0131): <1d every cycle, 1-3d ~1/12, 3+d ~1/72 via comment_id % N == cycle % N (cycle = wall-clock / poll interval, so it stays correct if the interval is tuned). Flat API usage, no per-comment state.

[gemini-code-assist]

Token Minting Spam on Failure

If token minting fails for an installation_id (e.g., due to a transient network issue or because the app was uninstalled), the current implementation does not cache this failure. As a result, for every subsequent comment belonging to the same failing installation in the same cycle, the poller will repeatedly attempt to mint a token, spamming the GitHub API and wasting resources.

Recommendation

Cache the failure by storing Option<String> in the tokens map. If a token fails to mint, store None and skip any subsequent comments for that installation immediately.

    let mut tokens: HashMap<i64, Option<String>> = HashMap::new();
    let mut checked = 0;
    for c in &comments {
        // Mint (and cache for this cycle) an installation token; skip the comment if it fails.
        let token = match tokens.get(&c.installation_id) {
            Some(Some(t)) => t.clone(),
            Some(None) => continue,
            None => match app.installation_token(c.installation_id).await {
                Ok(t) => {
                    tokens.insert(c.installation_id, Some(t.clone()));
                    t
                }
                Err(error) => {
                    tracing::warn!(%error, installation = c.installation_id, "mint token for poll failed");
                    tokens.insert(c.installation_id, None);
                    continue;
                }
            },
        };

References

1. When executing database operations or other fallible actions inside a loop over a batch of tasks, handle and log errors individually instead of using the ? operator to propagate the error. This prevents a single failing task from aborting the entire batch processing cycle and starving other tasks.

[stephane-segning]

Fixed (0db0131): the token cache is now HashMap<i64, Option> — a failed mint stores None and subsequent comments for that installation skip immediately.