fix(review): compare inconclusive as text so Postgres doesn't 500 (#4997)

src/db/repositories.ts

@@ -2830,7 +2830,13 @@ export async function findHottestInconclusiveReviewTargetForRepo(
         eq(aiUsageEvents.feature, "ai_review_pr"),
         gte(aiUsageEvents.createdAt, sinceIso),
         sql`json_extract(${aiUsageEvents.metadataJson}, '$.repoFullName') = ${repoFullName}`,
-        sql`json_extract(${aiUsageEvents.metadataJson}, '$.inconclusive') = 1`,
+        // #4997: `inconclusive` is stored as a JSON boolean. SQLite's json_extract surfaces a JSON boolean as the
+        // SQL integer 1/0, but the self-host Postgres translation of json_extract (pg-dialect.ts) rewrites this to
+        // `->>'inconclusive'`, which ALWAYS returns text -- comparing that text to a bare integer literal throws a
+        // Postgres type-mismatch error on every call. CAST to TEXT first so both backends compare text to text:
+        // SQLite's CAST(1 AS TEXT) = '1' (identical to the old `= 1` semantics), Postgres's `->>'inconclusive'`
+        // already yields 'true'/'false'.
+        sql`CAST(json_extract(${aiUsageEvents.metadataJson}, '$.inconclusive') AS TEXT) IN ('1', 'true')`,
       ),
     )
     .groupBy(pullNumberExpr)

test/unit/db-parsers.test.ts

@@ -8,6 +8,7 @@ import {
   countRecentAuditEventsForActorAndTarget,
   countRecentAuditEventsForActorInRepo,
   countRecentAuditEventsForActorInRepoWithTargetSuffix,
+  findHottestInconclusiveReviewTargetForRepo,
   findHottestReviewTargetForRepo,
   hasAuditEventForDelivery,
   hasAuditEventForHeadSha,
@@ -24,6 +25,7 @@ import {
   markPullRequestsRegated,
   markPullRequestsBacklogConvergenceRegated,
   markPullRequestSurfacePublished,
+  recordAiUsageEvent,
   recordAuditEvent,
   recordWebhookEvent,
   upsertOfficialMinerDetection,
@@ -748,6 +750,39 @@ describe("database row parser hardening", () => {
     });
   });
 
+  it("findHottestInconclusiveReviewTargetForRepo counts only inconclusive:true calls, scoped to ONE repo (#4997 regression: the JSON-boolean comparison must exclude inconclusive:false, not just miss on total volume)", async () => {
+    const env = createTestEnv();
+    const record = (repoFullName: string, pullNumber: number, inconclusive: boolean) =>
+      recordAiUsageEvent(env, {
+        feature: "ai_review_pr",
+        model: "self-host:claude-code",
+        status: "ok",
+        estimatedNeurons: 100,
+        metadata: { repoFullName, pullNumber, inconclusive },
+      });
+    // owner/repo#1: 3 inconclusive calls -- the hottest inconclusive target for this repo.
+    await record("owner/repo", 1, true);
+    await record("owner/repo", 1, true);
+    await record("owner/repo", 1, true);
+    // owner/repo#1 ALSO has 2 successful (non-inconclusive) calls on the SAME PR -- must not inflate the count.
+    await record("owner/repo", 1, false);
+    await record("owner/repo", 1, false);
+    // owner/repo#2: only 1 inconclusive call -- must not win over #1.
+    await record("owner/repo", 2, true);
+    // A different repo's inconclusive calls must not leak into this repo's count.
+    await record("owner/other", 1, true);
+    await record("owner/other", 1, true);
+
+    const hottest = await findHottestInconclusiveReviewTargetForRepo(env, "owner/repo", "2020-01-01T00:00:00.000Z");
+    expect(hottest).toEqual({ targetKey: "owner/repo#1", count: 3 });
+
+    // A cutoff after all the recorded events must find nothing.
+    expect(await findHottestInconclusiveReviewTargetForRepo(env, "owner/repo", "2099-01-01T00:00:00.000Z")).toBeNull();
+    // A repo with only non-inconclusive calls must find nothing.
+    await record("owner/all-ok", 1, false);
+    expect(await findHottestInconclusiveReviewTargetForRepo(env, "owner/all-ok", "2020-01-01T00:00:00.000Z")).toBeNull();
+  });
+
   it("hasAuditEventForDelivery finds a matching deliveryId inside metadata_json, scoped to actor+eventType+targetKey (#2560)", async () => {
     const env = createTestEnv();
     await recordAuditEvent(env, {

test/unit/selfhost-pg-dialect.test.ts

@@ -31,6 +31,19 @@ describe("pg-dialect (#977 SQLite → Postgres)", () => {
     expect(translateFunctions("json_extract(meta, '$.mode')")).toBe("((meta)::jsonb ->> 'mode')");
   });
 
+  it("REGRESSION (#4997): a JSON-boolean json_extract comparison survives translation as text-to-text, not text-to-integer", () => {
+    // findHottestInconclusiveReviewTargetForRepo (repositories.ts) compares a stored JSON boolean. SQLite's
+    // json_extract surfaces a JSON boolean as the SQL integer 1/0, but Postgres's `->>` ALWAYS returns text --
+    // comparing that text against a bare integer literal (the original `= 1`) throws a Postgres type-mismatch
+    // error on every call. CAST to TEXT first so the comparison is valid on both backends.
+    const translated = translateFunctions("CAST(json_extract(metadata_json, '$.inconclusive') AS TEXT) IN ('1', 'true')");
+    expect(translated).toBe("CAST(((metadata_json)::jsonb ->> 'inconclusive') AS TEXT) IN ('1', 'true')");
+    // No bare-integer comparison against a json_extract/->> expression should remain anywhere in the codebase --
+    // this is the ONE call site, and it's fixed. (Documents the invariant the fix restores; not itself testing
+    // translateFunctions with anything new.)
+    expect(translated).not.toMatch(/->>\s*'[a-z]+'\s*\)?\s*=\s*\d/);
+  });
+
   it("translates INSERT OR IGNORE / REPLACE to ON CONFLICT", () => {
     expect(translateInsertOr("INSERT OR IGNORE INTO t (a) VALUES (?)")).toBe("INSERT INTO t (a) VALUES (?) ON CONFLICT DO NOTHING");
     const replace = translateInsertOr("INSERT OR REPLACE INTO system_flags (key, value, updated_at) VALUES (?, '1', CURRENT_TIMESTAMP)");