fix(review): append the contributor skill-file link without losing the specific rejection reason#4556
Merged
Merged
Conversation
…e specific rejection reason screenshotTableGate's message field was an all-or-nothing override: setting it to include a skill-file link meant losing the auto-generated matrix message's specific "still missing: X, Y, Z" list, since a maintainer had no way to have both. Adds skillFileUrl as its own field, appended to whichever auto-generated message is already being shown (message, when set, still wins outright and skillFileUrl is ignored -- a maintainer who wants total control over the wording keeps it). Full config-as-code wiring: migration + schema + resolver + manifest parser (Worker + engine package copies) + OpenAPI, matching the same pattern as every other field on this config.
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 | b2f3b14 | Commit Preview URL Branch Preview URL |
Jul 10 2026, 01:57 AM |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4556 +/- ##
=======================================
Coverage 94.07% 94.08%
=======================================
Files 427 427
Lines 37938 37961 +23
Branches 13851 13864 +13
=======================================
+ Hits 35692 35715 +23
Misses 1586 1586
Partials 660 660
🚀 New features to boost your workflow:
|
JSONbored
added a commit
that referenced
this pull request
Jul 10, 2026
…lag machine-paced submitters (#4549) * feat(review): add a submission-cadence/inter-arrival-time signal to flag machine-paced submitters Every existing anti-abuse signal (reputation burst-floor, slop score) counts volume or ratios within a time window; none has an inter-arrival-time or rate term. A fast, well-formed, strategically low-value submitter is, by construction, invisible to the one dimension (superhuman pace) that would otherwise be a strong tell -- a submitter can clear every quality bar (good outcomes, real descriptions) while operating at a cadence no human plausibly sustains, and today that cadence carries zero weight anywhere. - computeSubmissionCadence / isMachinePacedCadence (pure): the median gap between a submitter's recent review_targets rows, flagged only once both a real sample size (5+) AND a sub-10-minute median gap are present -- a lone fast submission is not a pattern. - getSubmitterCadence: queries created_at across ALL review_targets rows (not just terminal ones -- a fresh burst of still-open submissions is exactly what this needs to catch, since the AI review already ran on each one by the time it becomes terminal). - Wired into shouldSkipAiForReputation as an independent, additional check alongside the existing quality/burst signal -- a submitter whose individual PRs all look fine can still be caught on cadence alone, and the (extra) cadence read is skipped once the quality/burst check alone already justifies downgrading. Scoped to per-repo cadence for now, matching this PR's own scope; an install-wide variant (mirroring #4513's confirmed-miner-aware cross-repo pattern) is a natural fast-follow once #4513 merges. Fixes #4514 * fix(db): renumber migration 0132 -> 0133 (0132 claimed by merged PR #4554) Rebase onto current main and take the next free number now that migrations/0132_impact_map_query_cache.sql (from #4554, itself a collision fix) occupies the number this branch originally guessed. * 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.
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 was referenced Jul 10, 2026
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
1 participant
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
screenshotTableGate.messagewas an all-or-nothing override: setting it to include a link to the contributor skill file meant giving up the auto-generated matrix message's specific "still missing: Desktop · Light, Tablet · Dark, ..." list, since a maintainer had no way to get both from one field. Discovered by deploying the feat(review): viewport x theme completeness matrix for the screenshot-table gate #4540 feature to metagraphed and verifying it end-to-end -- the live config usedmessageto add the skill-file link and, as a result, every rejection showed the exact same generic text regardless of which viewport/theme pairs were actually missing.skillFileUrlas its own field, appended to whichever message is already being shown (auto-generated matrix message, or the plain presence-mode default).message, when set, still wins outright andskillFileUrlis ignored in that case -- a maintainer who wants total control over the wording keeps it exactly as before..gittensory.ymlmanifest parser (both the Worker andgittensory-enginepackage copies, engine-parity confirmed) + OpenAPI.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 audit --audit-level=moderate-- 0 vulnerabilitiesskillFileUrl, appended in both matrix and presence mode, ignored whenmessageis set (both modes), sparse-override parsing (present/omitted), resolver merge (override present/DB fallback), full DB round-tripnpm 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, fullnpm run test:coverage-- not run locally this pass; this diff is scoped to the same files as feat(review): viewport x theme completeness matrix for the screenshot-table gate #4545 (no new surface), which already validated clean on the full suite; CI'stest:cigate covers these as a backstop.Safety
screenshotTableGateschema gainsskillFileUrl).