feat(review): let a bot-captured before/after satisfy the screenshot-table gate (#4128)

evaluateScreenshotTableGate now accepts a botCaptureSatisfied input: when the
visual-capture pipeline (review.visual.enabled) already rendered a real
before/after pair for the PR's current head, the gate is satisfied without a
hand-authored body table. The capture result is persisted to a new
pull_requests.visual_capture_satisfied_sha column (keyed to head SHA, mirrors
approved_head_sha) by maybePublishPrPublicSurface and re-read by the
maintenance pass in the same webhook, so no capture is re-run and no return
value needs threading through every caller.

Also resolves the dead ScreenshotTableGateAction surface: request_changes and
comment were fully typed/validated but processors.ts only ever branched on
"close", so setting either silently did nothing. Both are removed; "close" is
now the only valid action, and a legacy config value normalizes to it with a
warning like any other invalid input.

Author: JSONbored

.gittensory.yml.example

@@ -822,13 +822,14 @@ settings:
   #   minConfidence: 0.85  # Number 0-1. Minimum AI-verifier confidence to treat a candidate as a real match.
 
   # Before/after screenshot-table gate (#2006): deterministic check that a visual/frontend PR's body has a
-  # before/after image table. Scoped to whenLabels OR whenPaths (either matches); both empty = every PR. Off
-  # by default.
+  # before/after image table -- OR (#4110) that the bot's own visual-capture pipeline (review.visual.enabled)
+  # already produced a real before/after render for this PR's head, which satisfies the gate on its own.
+  # Scoped to whenLabels OR whenPaths (either matches); both empty = every PR. Off by default.
   # screenshotTableGate:
   #   enabled: false                        # Default: false.
   #   whenLabels: [frontend, visual]         # Default: [] (no label scoping).
   #   whenPaths: ["apps/ui/**", "src/**/*.tsx"]  # Default: [] (no path scoping).
-  #   action: close                          # close | request_changes | comment. Default: close.
+  #   action: close                          # close is the only supported value. Default: close.
   #   message: "Custom close reason..."      # Default: null (built-in message).
 
 # Maintainer AI review tuning (`.gittensory.yml` top-level `review:` block). These knobs shape the advisory AI

apps/gittensory-ui/public/openapi.json

@@ -9265,9 +9265,7 @@
               "action": {
                 "type": "string",
                 "enum": [
-                  "close",
-                  "request_changes",
-                  "comment"
+                  "close"
                 ]
               },
               "message": {

config/examples/gittensory.full.yml

@@ -835,13 +835,14 @@ settings:
   #   minConfidence: 0.85  # Number 0-1. Minimum AI-verifier confidence to treat a candidate as a real match.
 
   # Before/after screenshot-table gate (#2006): deterministic check that a visual/frontend PR's body has a
-  # before/after image table. Scoped to whenLabels OR whenPaths (either matches); both empty = every PR. Off
-  # by default.
+  # before/after image table -- OR (#4110) that the bot's own visual-capture pipeline (review.visual.enabled)
+  # already produced a real before/after render for this PR's head, which satisfies the gate on its own.
+  # Scoped to whenLabels OR whenPaths (either matches); both empty = every PR. Off by default.
   # screenshotTableGate:
   #   enabled: false                        # Default: false.
   #   whenLabels: [frontend, visual]         # Default: [] (no label scoping).
   #   whenPaths: ["apps/ui/**", "src/**/*.tsx"]  # Default: [] (no path scoping).
-  #   action: close                          # close | request_changes | comment. Default: close.
+  #   action: close                          # close is the only supported value. Default: close.
   #   message: "Custom close reason..."      # Default: null (built-in message).
 
 # Maintainer AI review tuning (`.gittensory.yml` top-level `review:` block). These knobs shape the advisory AI

migrations/0125_pull_request_visual_capture_satisfied_sha.sql

@@ -0,0 +1,13 @@
+-- Visual-capture gate satisfaction (#4110, visual-capture convergence epic #3607). The bot's before/after
+-- capture pipeline (review.visual.enabled, #4093) can now satisfy the deterministic screenshotTableGate
+-- (#2006) exactly like a hand-authored before/after table -- but the capture is computed and persisted by the
+-- public-surface publish pass (maybePublishPrPublicSurface), which runs BEFORE the maintenance/gate pass
+-- (maybeRunAgentMaintenance) re-reads this same PR row. Persisting the marker lets the maintenance pass see
+-- "did the bot already prove this PR visually?" without re-running the capture or threading a new return value
+-- through every caller of either function.
+--
+-- visual_capture_satisfied_sha is the head SHA at which the capture pipeline last produced a REAL before+after
+-- render pair (not a placeholder/failed/pending shot) -- scoped to head SHA (mirrors approved_head_sha, 0053 /
+-- last_published_surface_sha, 0080: a new commit re-arms the requirement until capture succeeds again for the
+-- new head).
+ALTER TABLE pull_requests ADD COLUMN visual_capture_satisfied_sha TEXT;

packages/gittensory-engine/src/review/screenshot-table-gate.ts

@@ -28,7 +28,7 @@ export const DEFAULT_SCREENSHOT_TABLE_GATE: ScreenshotTableGateConfig = {
   action: "close",
 };
 
-const VALID_ACTIONS: readonly ScreenshotTableGateAction[] = ["close", "request_changes", "comment"];
+const VALID_ACTIONS: readonly ScreenshotTableGateAction[] = ["close"];
 
 export function isScreenshotTableGateAction(value: unknown): value is ScreenshotTableGateAction {
   return typeof value === "string" && (VALID_ACTIONS as readonly string[]).includes(value);
@@ -72,7 +72,7 @@ export function normalizeScreenshotTableGateConfig(input: unknown, warnings: str
   const action = isScreenshotTableGateAction(record.action)
     ? record.action
     : (() => {
-        if (record.action !== undefined) warnings.push(`settings.requireScreenshotTable.action must be one of close, request_changes, comment; using the default "close".`);
+        if (record.action !== undefined) warnings.push(`settings.requireScreenshotTable.action must be "close" (the only supported value; #4110 removed request_changes/comment as dead config surface); using the default "close".`);
         return DEFAULT_SCREENSHOT_TABLE_GATE.action;
       })();
   const message = typeof record.message === "string" && record.message.trim().length > 0 ? record.message.trim() : undefined;
@@ -184,16 +184,25 @@ const NO_VIOLATION: ScreenshotTableGateResult = { violated: false, reason: null
 
 /** PURE evaluator. Off (`enabled: false`) or out-of-scope (no configured label/path match) ⇒ no violation. In
  *  scope AND (no image-bearing table in the body OR an image pasted outside a table OR a committed image file
- *  under a scoped path) ⇒ violated, with the configured (or default) templated message as the reason. */
+ *  under a scoped path), UNLESS `botCaptureSatisfied` ⇒ violated, with the configured (or default) templated
+ *  message as the reason. */
 export function evaluateScreenshotTableGate(input: {
   config: ScreenshotTableGateConfig;
   prBody: string | null | undefined;
   prLabels: string[];
   changedFiles: string[];
+  /** #4110: true when the bot's own before/after capture pipeline (review.visual.enabled) already produced a
+   *  REAL before+after render pair for this PR's current head — evidence equivalent to a hand-authored table.
+   *  A successful automated capture satisfies the gate on its own, ahead of (and regardless of) the body-table
+   *  anti-gaming checks below — those exist to stop a contributor from FAKING compliance without the bot's
+   *  help, which doesn't apply once the bot has already proven the change visually. Absent/false ⇒
+   *  byte-identical to pre-#4110 behavior (body-table evidence only). */
+  botCaptureSatisfied?: boolean | undefined;
 }): ScreenshotTableGateResult {
   const { config } = input;
   if (!config.enabled) return NO_VIOLATION;
   if (!isScreenshotTableGateInScope(config, input.prLabels, input.changedFiles)) return NO_VIOLATION;
+  if (input.botCaptureSatisfied === true) return NO_VIOLATION;
   const hasTable = hasImageBearingMarkdownTable(input.prBody);
   const outsideTable = hasImageOutsideTable(input.prBody);
   const committedImage = hasCommittedImageFile(input.changedFiles, config.whenPaths);

packages/gittensory-engine/src/types/manifest-deps-types.ts

@@ -18,7 +18,9 @@ export type CombineStrategy = "single" | "consensus" | "synthesis";
 
 export type OnMerge = "either" | "both";
 
-export type ScreenshotTableGateAction = "close" | "request_changes" | "comment";
+// #4110: `request_changes`/`comment` were REMOVED (see src/types.ts's mirror of this type for why) -- `"close"`
+// is the only value this gate has ever enforced.
+export type ScreenshotTableGateAction = "close";
 
 export type ScreenshotTableGateConfig = {
   enabled: boolean;

src/db/repositories.ts

@@ -3657,6 +3657,19 @@ export async function markPullRequestSurfacePublished(env: Env, fullName: string
     .where(and(eq(pullRequests.repoFullName, fullName), eq(pullRequests.number, number), eq(pullRequests.headSha, headSha)));
 }
 
+/** Visual-capture gate satisfaction (#4110): record the head SHA at which the bot's before/after capture
+ *  pipeline just produced a REAL before+after render pair for this PR (see `hasSuccessfulBotCapture`,
+ *  `review/visual/capture.ts`). The screenshotTableGate evaluator treats `visualCaptureSatisfiedSha ===
+ *  headSha` as evidence equivalent to a hand-authored table. Scoped to headSha (mirrors markPullRequestApproved)
+ *  so a later commit re-arms the requirement until capture succeeds again for the new head. */
+export async function markPullRequestVisualCaptureSatisfied(env: Env, fullName: string, number: number, headSha: string): Promise<void> {
+  const db = getDb(env.DB);
+  await db
+    .update(pullRequests)
+    .set({ visualCaptureSatisfiedSha: headSha, updatedAt: nowIso() })
+    .where(and(eq(pullRequests.repoFullName, fullName), eq(pullRequests.number, number), eq(pullRequests.headSha, headSha)));
+}
+
 /** Sweep convergence: stamp the timestamp the scheduled re-gate sweep just recomputed this PR. A plain D1 UPDATE
  *  — NOT routed through the agent-action-executor chokepoint (#1258) — so it advances even when GitHub writes are
  *  suppressed (dry-run / paused). selectRegateCandidates orders the sweep by last_regated_at, so a just-regated PR
@@ -5799,6 +5812,7 @@ function toPullRequestRecordFromRow(row: typeof pullRequests.$inferSelect): Pull
     lastPublishedSurfaceSha: row.lastPublishedSurfaceSha,
     linkedIssueHardRuleViolatedAt: row.linkedIssueHardRuleViolatedAt,
     linkedIssueHardRuleViolationReason: row.linkedIssueHardRuleViolationReason,
+    visualCaptureSatisfiedSha: row.visualCaptureSatisfiedSha,
   };
 }
 

src/db/schema.ts

@@ -477,6 +477,14 @@ export const pullRequests = sqliteTable(
     // pairing with merge_blocked_sha) -- so a later close can still cite the concrete rule even if the live
     // re-parse can no longer reproduce it (the issue was unlinked or its state changed).
     linkedIssueHardRuleViolationReason: text("linked_issue_hard_rule_violation_reason"),
+    // Visual-capture gate satisfaction (#4110): the head SHA at which the bot's before/after capture pipeline
+    // (review.visual.enabled) last produced a REAL before+after render pair (not a placeholder/failed/pending
+    // shot) for this PR. Lets the deterministic screenshotTableGate treat a successful automated capture as
+    // equivalent evidence to a hand-authored before/after table. Keyed to head SHA (mirrors approved_head_sha /
+    // last_published_surface_sha) -- a new commit re-arms the requirement until capture succeeds again for the
+    // new head. gittensory-computed (publish-written), omitted from the GitHub-sync SET clause so a later sync
+    // cannot clobber it.
+    visualCaptureSatisfiedSha: text("visual_capture_satisfied_sha"),
     createdAt: text("created_at").notNull().$defaultFn(() => nowIso()),
     updatedAt: text("updated_at").notNull().$defaultFn(() => nowIso()),
   },

src/openapi/schemas.ts

@@ -776,7 +776,7 @@ export const RepositorySettingsSchema = z
         enabled: z.boolean(),
         whenLabels: z.array(z.string()),
         whenPaths: z.array(z.string()),
-        action: z.enum(["close", "request_changes", "comment"]),
+        action: z.enum(["close"]),
         message: z.string().optional(),
       })
       .optional(),

src/queue/processors.ts

@@ -56,6 +56,7 @@ import {
   markPullRequestsRegated,
   markPullRequestReviewsInvalidated,
   markPullRequestSurfacePublished,
+  markPullRequestVisualCaptureSatisfied,
   getLatestRegatedAt,
   claimRegateFanoutSlot,
   recordAgentCommandFeedback,
@@ -358,7 +359,7 @@ import { randomUUID } from "node:crypto";
 import { isRetryableJobError, RetryableJobError } from "./retryable";
 import { screenshotsAllowed } from "../review/visual-wire";
 import { isVisualPath } from "../review/visual/paths";
-import { buildCapture, type CaptureRoute } from "../review/visual/capture";
+import { buildCapture, hasSuccessfulBotCapture, type CaptureRoute } from "../review/visual/capture";
 import { incr } from "../selfhost/metrics";
 import {
   renderReviewingPlaceholder,
@@ -2778,17 +2779,22 @@ async function runAgentMaintenancePlanAndExecute(
   );
 
   // Screenshot-table gate (#2006): a DETERMINISTIC check (no AI) that an in-scope (label/path-matched)
-  // contributor visual/frontend PR's body contains a before/after screenshot table. Off by default
-  // (settings.screenshotTableGate.enabled === false), so the pure evaluator below is effectively free for the
-  // common case. Only "close" is wired as an enforcement action here (the other configured actions stay
-  // advisory, matching the issue's phased rollout) -- the ternary below is the ONLY place that reads `.action`.
+  // contributor visual/frontend PR's body contains a before/after screenshot table -- OR (#4110) that the
+  // bot's own visual-capture pipeline already produced a real before/after render for this exact head
+  // (markPullRequestVisualCaptureSatisfied, written earlier in this same webhook by maybePublishPrPublicSurface
+  // -- see that function's beforeAfter block -- and re-read here on `pr`, which this caller already re-fetched
+  // fresh from the DB). Off by default (settings.screenshotTableGate.enabled === false), so the pure evaluator
+  // below is effectively free for the common case. "close" is the only enforcement action this gate has (#4110
+  // removed the dead request_changes/comment surface) -- the check below is the ONLY place that reads `.action`.
   /* v8 ignore next -- defensive: resolveRepositorySettings always populates screenshotTableGate (getRepositorySettings's DB defaults), so this fallback is unreachable in practice. */
   const screenshotTableGateConfig = settings.screenshotTableGate ?? DEFAULT_SCREENSHOT_TABLE_GATE;
+  const botCaptureSatisfied = Boolean(pr.headSha) && pr.visualCaptureSatisfiedSha === pr.headSha;
   const screenshotTableGateResult = evaluateScreenshotTableGate({
     config: screenshotTableGateConfig,
     prBody: pr.body,
     prLabels: pr.labels,
     changedFiles: changedPaths,
+    botCaptureSatisfied,
   });
   const screenshotTableMatch =
     screenshotTableGateResult.violated && screenshotTableGateConfig.action === "close"
@@ -10260,6 +10266,24 @@ async function maybePublishPrPublicSurface(
               ? { routes: [], previewPending: false }
               : await buildCapture(env, token, captureTarget, visualFiles, githubRateLimitAdmissionKeyForInstallation(installationId), reviewVisualConfig);
           beforeAfter = capture.routes;
+          // Screenshot-table gate satisfaction (#4110): a successful capture (a real before+after render pair
+          // on at least one route) is evidence equivalent to a hand-authored before/after table -- persist the
+          // head SHA it was proven at so the LATER maintenance pass (runAgentMaintenancePlanAndExecute, which
+          // re-reads this PR row fresh) can see it without re-running the capture or threading a new return
+          // value through every caller of this function. Best-effort: a write failure here just means the gate
+          // falls back to requiring a body table, never blocks the rest of the review.
+          if (pr.headSha && hasSuccessfulBotCapture(beforeAfter)) {
+            await markPullRequestVisualCaptureSatisfied(env, repoFullName, pr.number, pr.headSha).catch((error) => {
+              console.log(
+                JSON.stringify({
+                  event: "visual_capture_satisfied_mark_failed",
+                  repoFullName,
+                  pull: pr.number,
+                  message: errorMessage(error).slice(0, 200),
+                }),
+              );
+            });
+          }
           // Visual self-poll: the FIRST capture returns a "loading" placeholder for the AFTER shot when the
           // preview deploy isn't live yet (capture.previewPending). Schedule a delayed re-review to re-capture
           // the now-ready shot — bounded by `attempt` so a never-resolving preview can't loop (the deployment_status