feat(gate): #554 false-positive telemetry for hard-blocked PRs (measurement-only) - #755
feat(gate): #554 false-positive telemetry for hard-blocked PRs (measurement-only)#755Crystora wants to merge 15 commits into
Conversation
|
Note Gittensory Gate skippedPR closed before full evaluation. No late first comment was created.
💰 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. |
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
gittensory · advisory review Reviewed 9 changed file(s) — two independent AI reviewers. Changed files (9)
Suggested action: ✅ Safe to merge — both reviewers found no blocking issues. Reviewer A · Suggestions
Worth double-checking
Reviewer B · No blocking issues spotted. |
…t/541-public-readme-badge
…toby/gittensory into feat/541-public-readme-badge
…t/541-public-readme-badge
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #755 +/- ##
==========================================
- Coverage 96.55% 96.55% -0.01%
==========================================
Files 97 98 +1
Lines 14091 14141 +50
Branches 5141 5151 +10
==========================================
+ Hits 13606 13654 +48
- Misses 105 106 +1
- Partials 380 381 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
JSONbored
left a comment
There was a problem hiding this comment.
PR #755 — feat(gate): #554 false-positive telemetry for hard-blocked PRs (measurement-only)
Verdict: REQUEST CHANGES
Resolves issue #554: FULLY
CI: Prior codecov/patch failure is RESOLVED (not present; security/workers/changes green). lint/test/ui were still in_progress at review time — no failures observed.
Highlights:
- Well-isolated design: gate_outcomes table + schema, recordGateBlockOutcome/resolveGateOutcome helpers, pure buildGateFalsePositiveReport aggregator; processor hooks are best-effort try/catch so telemetry can never disrupt gate/webhook processing (asserted via throwing-DB test).
- Correct, tested false-positive semantics: blocked-then-merged -> FP; plain close unresolved (true positive); re-block clears prior resolution. Internal-auth-only endpoint with 401 negative test; aggregate-only report (no PII/reward/trust fields).
Concerns / required changes:
- Migration number collision: adds migrations/0037_gate_outcomes.sql while PR #753 also adds migrations/0037_badge_enabled.sql (main is at 0036). Whichever merges second must renumber to 0038 — coordinate ordering.
- "overridden" resolution exists in the type/aggregator (counted as FP) but no recording path ever writes it — the #538 override correlation is effectively a stub; acceptable as forward-compat but untested end-to-end.
- failing CI (codecov) - requires additional test coverage to meet threshold
Completeness vs issue: Fully implements the AC (blocked-then-merged increments FP count; rate queryable per gate mode) with privacy-safe aggregation and meaningful tests. The 0037 collision is the one required fix.
6c3b718 to
ed0e4b4
Compare
…t/554-gate-false-positive-telemetry
|
Thanks for this, @bittoby — genuinely solid work. 🙏 Your #755 and the maintainer PR #828 independently landed on the same issue (#554), and our own We're going with #828 because it was already green and is wired into a couple of things that landed in parallel: the #538 maintainer-override path (an override becomes the strongest false-positive signal), the config-as-code surface, and it mirrors the existing #543 outcome-learning loop's ledger + service split. Your structure was very close to the same shape — the Closing this in favor of #828, but please don't let that discourage you — this was a real, well-scoped contribution and we'd love more. If you want a good next one, #540 (advisory reviewer routing via CODEOWNERS) is open and unclaimed. Thank you again. 🚀 |
|
@JSONbored Hi, why you closed my pr? I am updating pr according to your feedback currently. |
|
did you see my commits? I just committed them about 2 mins ago before you closed my pr. |
|
I suspect it was intentionally done to close my pr. |
Summary
Adds gate false-positive telemetry (#554). When the gittensory gate hard-blocks a PR, maintainers have no measurement of how often those blocks were wrong — i.e. the PR was later merged anyway. This is the evidence maintainers need before promoting any gate from advisory to hard-block.
This PR records every hard block and correlates it with the PR's eventual outcome, then exposes a per-gate-type false-positive rate. It is measurement-only: it never changes gate behavior, never blocks, and all writes are best-effort so a telemetry failure can never disrupt gate or webhook processing.
gate_outcomestable (migrations/0037_gate_outcomes.sql, schema insrc/db/schema.ts): one row per(repo_full_name, pr_number)recording the blocker codes and block time, plus a nullable resolution (merged/overridden) and resolved time.src/services/gate-telemetry.ts,src/db/repositories.ts): a hard block (conclusion: "failure") upserts an outcome; a re-block clears any prior resolution. A blocked PR that is later merged resolves the outcome tomerged— a confirmed false positive. A plain close is a true positive (the block held) and is left unresolved.src/queue/processors.ts): two straight-line, best-effort calls — one on PR close/merge, one after the gate evaluation finalizes. All branching lives in the unit-tested helpers, not the deep webhook processor.src/api/routes.ts):GET /v1/internal/repos/:owner/:repo/gate-telemetryreturns the aggregated report — overall blocked / false-positive counts and rate, plus a per-blocker-code breakdown. Internal-auth only.Closes #554.
Scope
type(scope): short summaryConventional Commit format —feat(gate): #554 false-positive telemetry for hard-blocked PRs (measurement-only).CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally; global coverage stays ≥ 97% for lines, statements, functions, and branches (Statements 98.64%, Branches 97.04%, Functions 97.03%, Lines 99.26%)npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderate(0 vulnerabilities)All required checks pass via
npm run test:ci(EXIT 0).Safety
401for unauthorized requests.UI Evidence— N/A, backend-only.UI Evidence
N/A — backend-only change (a
gate_outcomestable, recording helpers, and an internal read endpoint). No UI, frontend, docs, or extension changes.Notes
try/catchso a telemetry failure returns silently and never disrupts gate or webhook processing; a unit test asserts this via a throwing DB stub.0037, the next sequential number after main's0036.