Skip to content

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

Merged
JSONbored merged 1 commit into
mainfrom
feat/gate-wiring-screenshot-4110-v2
Jul 8, 2026
Merged

feat(review): let a bot-captured before/after satisfy the screenshot-table gate#4128
JSONbored merged 1 commit into
mainfrom
feat/gate-wiring-screenshot-4110-v2

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • evaluateScreenshotTableGate (both src/review/screenshot-table-gate.ts and its packages/gittensory-engine mirror) now accepts an optional botCaptureSatisfied input. When true it short-circuits to "no violation" ahead of the body-table / outside-table / committed-image anti-gaming checks, so a successful automated before/after capture is treated as evidence equivalent to a hand-authored table.
  • src/queue/processors.ts's maybePublishPrPublicSurface computes hasSuccessfulBotCapture (new pure helper in src/review/visual/capture.ts) over the capture pipeline's routes and, on success, persists the head SHA to a new pull_requests.visual_capture_satisfied_sha column via markPullRequestVisualCaptureSatisfied (mirrors approved_head_sha/last_published_surface_sha). runAgentMaintenancePlanAndExecute re-reads that same PR row (already re-fetched fresh in the same webhook, after maybePublishPrPublicSurface runs) and threads botCaptureSatisfied = visualCaptureSatisfiedSha === headSha into the evaluator. No return-value threading through the publish/maintenance call chain was needed.
  • Resolved the dead request_changes/comment config surface named in the issue: ScreenshotTableGateAction is narrowed to "close" only (the sole value processors.ts ever branched on). A legacy .gittensory.yml/DB value of either removed action normalizes to the default "close" with a warning, exactly like any other invalid value — chosen over wiring them to a real effect (which would require a first-ever use of the unused actionClass: "review" planner surface) to keep this PR narrow and scoped to the gate-wiring issue.
  • Updated the screenshotTableGate doc blocks in .gittensory.yml.example and config/examples/gittensory.full.yml, and regenerated apps/gittensory-ui/public/openapi.json for the narrowed action enum.

Verified against the current code before implementing (per the epic's due-diligence note): evaluateScreenshotTableGate truly had no bot-capture input, and processors.ts's screenshot-table close path (~line 2787) only ever read .action === "close", so this is a real capability addition, not a no-op.

Closes #4110

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves (e.g. Closes #123) — a linked open issue is required for every contributor PR.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally; codecov/patch requires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

Ran the full local gate via npm run test:ci (aggregates actionlint, db:migrations:check, db:schema-drift:check, selfhost:env-reference:check, selfhost:validate-observability, cf-typegen:check, typecheck, test:coverage, test:engine-parity, test:workers, build:mcp, test:mcp-pack, build:miner, test:miner-pack, rees:test, ui:openapi:check, ui:openapi:settings-parity, ui:version-audit, docs:drift-check, manifest:drift-check, command-reference:check, ui:lint, ui:typecheck, ui:test, ui:build) — all green, plus npm audit --audit-level=moderate reporting 0 vulnerabilities. Also ran npx vitest run test/unit/worker-entry-boundary.test.ts since this touches src/review/**/src/queue/** (green — no forbidden pixelmatch/pngjs/visual-diff/gifenc reference introduced).

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests.
  • API/OpenAPI/MCP behavior is updated and tested where needed.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks.
  • Visible UI changes include a UI Evidence section below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

No visible UI change — this PR is backend-only (queue processor, DB schema/repositories, review engine, config docs); the only apps/gittensory-ui/** diff is the regenerated public/openapi.json.

UI Evidence

Not applicable — no visible UI change.

Notes

  • This touches src/queue/** and src/db/** (guarded/crucial paths per .gittensory.yml's hard-guardrail globs), so I expect this PR to be held for manual owner review even with a fully green gate — that is the documented, correct behavior for this path, not a failure signal.

…table gate

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.
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui 8988f58 Commit Preview URL

Branch Preview URL
Jul 08 2026, 04:53 AM

@codecov

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.66%. Comparing base (189f824) to head (8988f58).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4128   +/-   ##
=======================================
  Coverage   93.66%   93.66%           
=======================================
  Files         384      384           
  Lines       35936    35949   +13     
  Branches    13188    13196    +8     
=======================================
+ Hits        33660    33673   +13     
  Misses       1618     1618           
  Partials      658      658           
Files with missing lines Coverage Δ
...tensory-engine/src/review/screenshot-table-gate.ts 100.00% <100.00%> (ø)
src/db/repositories.ts 96.71% <100.00%> (+<0.01%) ⬆️
src/db/schema.ts 72.13% <ø> (ø)
src/openapi/schemas.ts 100.00% <ø> (ø)
src/queue/processors.ts 95.16% <100.00%> (+<0.01%) ⬆️
src/review/screenshot-table-gate.ts 100.00% <100.00%> (ø)
src/review/visual/capture.ts 89.76% <100.00%> (+0.41%) ⬆️
src/settings/agent-actions.ts 97.72% <ø> (ø)
src/types.ts 100.00% <ø> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 8, 2026
@loopover-orb

loopover-orb Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-08 05:20:48 UTC

18 files · 1 AI reviewer · no blockers · readiness 100/100 · CI green · clean

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review: This PR changes guardrail-protected path(s): src/queue/processors.ts (matched src/queue/**), src/settings/agent-actions.ts (matched src/settings/agent-actions.ts).

Review summary
This PR wires a successful bot before/after visual capture into evaluateScreenshotTableGate as evidence equivalent to a hand-authored table, short-circuiting the anti-gaming body checks only after scope (label/path) is already confirmed, and persists the satisfying head SHA via a new nullable pull_requests.visual_capture_satisfied_sha column (migration 0125) that re-arms per commit, mirroring the existing approved_head_sha/last_published_surface_sha pattern. It also removes the dead request_changes/comment config values, narrowing ScreenshotTableGateAction to "close" with matching schema/openapi regeneration. The critical same-webhook read-after-write ordering claim (maybePublishPrPublicSurface writes, runAgentMaintenancePlanAndExecute re-reads) isn't independently visible in the truncated diff, but it is exercised end-to-end by a new queue.test.ts test that drives a full processJob call through both functions in one webhook and asserts the persisted SHA plus the resulting non-close, with a companion test pinning the write-failure fail-safe path (falls back to requiring a body table). CI is green on this exact commit, including these tests, so the wiring is verified via real integration coverage rather than a fabricated fixture.

Nits — 6 non-blocking
  • src/settings/agent-actions.ts:356,674 retain pre-existing 'blacklist' terminology in comments you're already touching in this diff; consider 'denylist' since you're editing these lines anyway.
  • isRealShotUrl (src/review/visual/capture.ts:61) uses a bare .includes("placeholder=") substring match — fine today since capturePage fully controls the URL shape, but worth a comment noting the contract so a future capture-URL change doesn't silently break the gate.
  • test/unit/screenshot-table-gate.test.ts and test/unit/screenshot-table-gate-engine.test.ts duplicate the entire new botCaptureSatisfied describe block verbatim across the src/engine mirror — expected given the existing mirror pattern, just flagging for awareness.
  • Consider a short comment on isRealShotUrl documenting the placeholder-substring contract so a future capturePage change doesn't silently break the gate.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #4110
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 51 registered-repo PR(s), 43 merged, 505 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 51 PR(s), 505 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Linked issue satisfaction

Addressed
The PR adds a botCaptureSatisfied input to evaluateScreenshotTableGate that short-circuits the gate to no-violation, wires it end-to-end from a new hasSuccessfulBotCapture helper through processors.ts persistence (visual_capture_satisfied_sha) into agent-actions.ts's screenshotTableMatch, and resolves the dead action config by narrowing ScreenshotTableGateAction to "close" only with a documented r

Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 51 PR(s), 505 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 8, 2026
@JSONbored
JSONbored merged commit c2c6eb3 into main Jul 8, 2026
13 checks passed
@JSONbored
JSONbored deleted the feat/gate-wiring-screenshot-4110-v2 branch July 8, 2026 05:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Development

Successfully merging this pull request may close these issues.

feat(review): wire bot-generated visual captures into screenshotTableGate

1 participant