feat(review): viewport x theme completeness matrix for the screenshot-table gate#4545
Merged
Conversation
…-table gate (#4540) The deterministic screenshot-table gate only checked that a PR body contained some markdown table with any image inside it, with no concept of viewport or color-mode coverage -- even though metagraphed's own contributor skill file documents an exact 3-viewport x 2-theme x before/after = 12-image contract. metagraphed PR #4661 shipped 4/12 images and passed AI review, which had no way to know the completeness requirement existed. screenshotTableGate gains requireViewports/requireThemes (string arrays, empty by default -- opt-in per repo, byte-identical otherwise). When set, the evaluator matches each required (viewport, theme) pair against a labeled table row and requires two image-bearing cells (before + after) in that row; missing pairs are named in the rejection reason. action gains a real advisory value, distinct from the request_changes/comment values #4110 removed as dead/unwired config -- this one is actually wired: an advisory violation is computed but never reaches the close-triggering planner match. Full config-as-code wiring: DB migration + Drizzle schema + settings resolver + .gittensory.yml manifest parser (both the Worker copy and the hand-duplicated gittensory-engine package copy) + OpenAPI. Closes #4540
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
gittensory-ui | 5a34d00 | Commit Preview URL Branch Preview URL |
Jul 10 2026, 01:03 AM |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4545 +/- ##
==========================================
+ Coverage 94.07% 94.08% +0.01%
==========================================
Files 425 425
Lines 37757 37827 +70
Branches 13790 13814 +24
==========================================
+ Hits 35519 35589 +70
Misses 1583 1583
Partials 655 655
🚀 New features to boost your workflow:
|
…age (codecov/patch)
|
Important 🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪 🔍 Gittensory is reviewing…AI analysis is in progress. This comment will update when the review is complete. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed · 🟪 Reviewing |
JSONbored
added a commit
that referenced
this pull request
Jul 10, 2026
…-wide for confirmed miners (#4546) * fix(review): make submitter-reputation burst/AI-spend defense install-wide for confirmed miners getSubmitterReputation's burst/low-sample thresholds are scoped WHERE project = ? AND submitter = ? -- a single repo. A fleet identity spreading a handful of gate-passing-but-low-value PRs across dozens of repos in one self-hosted install never accumulates enough same-repo sample density to read as "burst" or "low" anywhere, so the reputation defense never fires for it and every one of its submissions burns full paid AI-review spend indefinitely. - New getSubmitterReputationAcrossInstall in submitter-reputation.ts: the identical quality-weighted signal derivation, scoped by review_targets.installation_id instead of project (that column already existed, migrations/0050; this adds the supporting index). - New getEffectiveSubmitterReputation in reputation-wire.ts: the per-repo signal, additionally widened to the install-wide view for a CONFIRMED official Gittensor miner -- but only when the per-repo signal alone doesn't already justify caution, so an ordinary contributor or an already-flagged submitter pays no extra lookup. - Extracted the miner-identity check (shared with #4512) into src/gittensor/miner-detection-cache.ts so both this module and unlinked-issue-guardrail.ts can use it without a circular import through processors.ts. - Wired into both call sites: shouldSkipAiForReputation (the main AI-spend gate) and the vision-review reputation check. Fixes #4513 * fix: renumber migration 0130 -> 0131 (0130 was claimed by #4538, already merged to main) * fix(test): account for getEffectiveSubmitterReputation's miner-identity check in visual-vision tests runVisualVisionForAdvisory now resolves reputation via getEffectiveSubmitterReputation (#4513), which checks confirmed-official-miner identity (a fetch to api.gittensor.io/miners) whenever the submitter's per-repo signal is neutral -- a real, intentional behavior change this test file's existing "no network calls at all" assertions didn't account for. Stub that one identity check to resolve cleanly and assert precisely that no OTHER (BYOK/vision-spend) network call happens, rather than asserting zero fetch calls outright. * fix(db): renumber migration 0131 -> 0133 (0131/0132 claimed by merged PRs) origin/main has since merged #4545 (0131_screenshot_table_gate_matrix.sql) and #4554 (0132_impact_map_query_cache.sql, itself a collision fix); rebase onto current main and take the next free number. * fix(db): renumber migration 0133 -> 0134 (0133 claimed by merged PR #4556) Rebase onto current main and take the next free number now that migrations/0133_screenshot_table_gate_skill_link.sql (from #4556) occupies the number this branch previously took. * fix(db): renumber migration 0134 -> 0139 (0134 has a pre-existing 3-way collision on main) main currently has three DIFFERENT already-merged files at 0134 (#4549/#4558/#4563) -- tracked separately as its own fix (PR #4577, not yet merged). Since that fix already claims through 0138, take 0139 here to avoid colliding with it once it lands; this branch's own db:migrations:check will stay red until #4577 merges (unrelated to this branch's own changes), and will need one more rebase afterward. * test(review): close 2 coverage gaps surfaced by the #4514 rebase-merge getEffectiveSubmitterReputation's getRepository().catch() needed a real read-failure test (added); its isConfirmedOfficialMiner().catch() is unreachable (that function already catches every internal failure point itself) and getSubmitterReputationAcrossInstall's results ?? [] fallback is the same "D1 always populates results" case already v8-ignored elsewhere in this codebase -- both marked accordingly. * fix(review): isolate #4507's reputation-single-read invariant from cross-test miner-cache pollution Several earlier tests in this file cache "contributor" as a confirmed official Gittensor miner (5-min TTL) in the shared per-file D1 instance. That collided with #4513's new install-wide reputation widening, which correctly detects the stale cache and adds a 4th reputation-scan D1 read for a submitter this test never intended to be a miner. Use a submitter login unique to this test instead.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.claude/skills/metagraphed/SKILL.md, Phase C2, feat(mcp): add gittensory_check_test_evidence tool (#2235) #3770) documents an exact 3-viewport x 2-theme x before/after = 12-image contract. metagraphed PR fix(github): allow self-host TRUSTED_SCANNER_BOT_LOGINS override #4661 shipped with 4/12 images and passed AI review, because the private review-context copy the AI reviewer's instructions come from was never updated to mention viewport/theme completeness.screenshotTableGateconfig gainsrequireViewports/requireThemes(string arrays, empty by default -- opt-in per repo, byte-identical for a repo that doesn't set them). WhenrequireViewportsis non-empty, the evaluator matches every required (viewport, theme) pair against a labeled table row (case-insensitive substring match on the row's first cell, e.g. "Desktop · Light") and requires at least two image-bearing cells in that row (before + after). Missing pairs are named in the rejection reason.actiongains a realadvisoryvalue, distinct from therequest_changes/commentvalues feat(review): wire bot-generated visual captures into screenshotTableGate #4110 removed as dead/unwired config -- this one is actually wired:advisorycomputes the violation but the caller (processors.ts) only ever folds a result into the close-triggering planner match whenaction === "close", soadvisoryis a real no-op on merge/close by construction..gittensory.ymlmanifest parser (both the Worker copy and the hand-duplicatedgittensory-enginepackage copy, kept in sync perengine-parity:drift-check) + OpenAPI.Closes #4540
Scope
type(scope): short summaryConventional Commit format.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run typechecknpm run db:migrations:check,npm run db:schema-drift:check,npm run cf-typegen:checknpm run manifest:drift-check,npm run engine-parity:drift-check(Worker/enginescreenshot-table-gate.ts+manifest-deps-types.tscopies confirmed in sync)npm run ui:openapi(regenerated) +npm run ui:openapi:check(no drift) +npm run ui:openapi:settings-paritynpm run test --workspace @jsonbored/gittensory-engine(325/325 passing)npm run test:coverage(full unsharded local run, 12830+ tests passing, 0 failures)npm audit --audit-level=moderate-- 0 vulnerabilitiesnpm run docs:drift-check,npm run selfhost:env-reference:check,npm run command-reference:checknpm run actionlint,npm run test:workers,npm run build:mcp,npm run test:mcp-pack,npm run build:miner,npm run test:miner-pack,npm run rees:test,npm run ui:lint,npm run ui:typecheck,npm run ui:test,npm run ui:build-- not run locally; this diff touches no UI/MCP/miner/REES/workers surface, so CI's fulltest:cigate covers these as a backstop.Safety
screenshotTableGateschema gainsrequireViewports/requireThemes/advisory).Notes
This PR builds the detection engine and config surface only. A follow-up (tracked separately, not in this PR) syncs the private per-repo review-context files on the self-host server so the AI reviewer's own instructions mention the matrix requirement too, and turns the gate on for metagraphed with
action: close.