Skip to content

fix(review): exclude maintainer PRs from the slop-discrimination alert - #5863

Merged
JSONbored merged 1 commit into
mainfrom
fix/slop-calibration-maintainer-cohort
Jul 14, 2026
Merged

fix(review): exclude maintainer PRs from the slop-discrimination alert#5863
JSONbored merged 1 commit into
mainfrom
fix/slop-calibration-maintainer-cohort

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • The ops-anomaly detector flagged "slop score NOT discriminating" on two repos, reading it as a scoring-calibration problem. It wasn't: the slop scorer's weights, band thresholds, and gate-blocker direction are all internally consistent, and the detector's own unit tests pin the intended "higher band merges less" semantics correctly.
  • The real bug is in buildSlopOutcomeCalibration's population: it pools every resolved PR regardless of author. Maintainer-authored PRs merge by human judgment regardless of severity (the same population settings-preview.ts's includeMaintainerAuthors already excludes from the public surface by default) — so a repo where the maintainer is heavily active can have its contributor-only, correctly-discriminating signal swamped by a maintainer-PR pool that merges at a high rate independent of score, inverting the blended comparison without the score itself being wrong.
  • Added an additive excludeMaintainerAuthors option (default off, every existing caller stays byte-identical) mirroring the miner/human cohorts split gate-precision.ts already applies to the sibling feat(github-app): gate false-positive telemetry #554 false-positive measurement, and opted the ops-anomaly detector's two call sites into it so the alert — and the internal ops-stats row shown alongside it — both reflect the contributor-only signal instead of the confounded one.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • 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.
  • Linked issue — N/A. Maintainer-authored fix found via a production ops-anomaly detector, not a contributor PR.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally — 854 test files passed, 0 failed (full unsharded run). New tests cover: the maintainer-PR confound reproducing the false positive (default behavior, byte-identical), excludeMaintainerAuthors:true removing the confound and revealing the true signal, a genuine contributor-only non-discriminating score still correctly flagging (not a blanket suppression), the OWNER/MEMBER/COLLABORATOR vs FIRST_TIME_CONTRIBUTOR/unset classification boundary, the option threading end-to-end through buildRepoOutcomeCalibration, and both runOpsAlerts/computeOpsStats no longer alerting on the confound while a real contributor-only inversion still does.
  • npm run test:engine-parity — clean, unaffected (this change doesn't touch packages/loopover-engine).
  • npm audit --audit-level=moderate — clean, 0 vulnerabilities.
  • npm run test:workers / npm run build:mcp / npm run test:mcp-pack / npm run ui:* — not run, see note below.

If any required check was skipped, explain why:

  • This change is confined to src/services/outcome-calibration.ts, src/review/ops-wire.ts, and their unit tests — no Workers runtime, MCP package, API route, OpenAPI schema, or UI code is touched.

Safety

  • No secrets, wallets, hotkeys, coldkeys, trust scores, or reward values touched.
  • Privacy: the fix only changes which PRs feed an aggregate rate calculation — no actor login, trust, or reward data is newly exposed anywhere (existing tests already assert the ops-stats payload never contains those terms).
  • Auth/CORS/GitHub App/Cloudflare/session, API/OpenAPI/MCP, UI, and docs/changelog: not applicable, none touched.

UI Evidence

N/A — no UI/frontend/docs change.

Notes

The ops-anomaly detector flagged "slop score NOT discriminating" on two
repos, reading it as a scoring-calibration problem. It wasn't: the slop
scorer's weights, band thresholds, and gate-blocker direction are all
internally consistent, and the detector's own unit tests pin the
intended "higher band merges less" semantics correctly.

The real bug is in buildSlopOutcomeCalibration's population: it pools
every resolved PR regardless of author. Maintainer-authored PRs merge by
human judgment regardless of severity (the same population
settings-preview.ts's includeMaintainerAuthors already excludes from the
public surface by default) -- so a repo where the maintainer is heavily
active can have its contributor-only, correctly-discriminating signal
swamped by a maintainer-PR pool that merges at a high rate independent
of score, inverting the blended comparison without the score itself
being wrong.

Add an additive excludeMaintainerAuthors option (default off, every
existing caller stays byte-identical) mirroring the miner/human cohorts
gate-precision.ts already applies to the sibling #554 false-positive
measurement, and opt the ops-anomaly detector's two call sites into it
so the alert -- and the internal ops-stats row shown alongside it --
both reflect the contributor-only signal instead of the confounded one.
@superagent-security

Copy link
Copy Markdown
Contributor

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

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

loopover-orb Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Warning

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

⏸️ LoopOver review result - manual review recommended

Review updated: 2026-07-14 20:31:49 UTC

4 files · 1 AI reviewer · 2 blockers · readiness 93/100 · CI green · clean

⏸️ Suggested Action - Manual Review

Review summary
This threads an additive `excludeMaintainerAuthors` option through `buildSlopOutcomeCalibration`/`buildRepoOutcomeCalibration` and opts both `runOpsAlerts` and `computeOpsStats` into it, so the ops-anomaly detector's slop-discrimination check now pools only contributor PRs, matching the `includeMaintainerAuthors` exclusion pattern already used in `settings-preview.ts`. The change is additive and default-off (existing callers untouched), the two call sites in `ops-wire.ts` are consistently updated together (stats row and cron alert stay in sync), and the tests exercise both the confound-suppression case and a genuine-non-discrimination case so the fix isn't a blanket suppression. The `options` object flows into `buildRecommendationOutcomeCalibration` too but that function only reads `maintainerOnly`, so recommendation behavior is unaffected as intended.

Nits — 4 non-blocking
  • src/services/outcome-calibration.ts: the JSDoc on `excludeMaintainerAuthors` is quite long (8+ lines) for a single option field — consider trimming to the essential rationale and moving the historical cross-references to the PR description.
  • test/unit/ops-wire.test.ts seedSlopMaintainerConfound: the hard-coded arithmetic in the comment (11/16=0.6875) is fragile if band-membership numbers ever change slightly; a runtime assertion of the raw (non-excluded) rate would self-document better than a comment.
  • Consider whether `excludeMaintainerAuthors` should eventually become the default for the ops-alert path specifically (not just these two call sites) via a shared options constant, to avoid future call sites forgetting to opt in.
  • The MCP tool / public API route that also call `buildRepoOutcomeCalibration` per the PR description are worth a quick follow-up check to confirm they intentionally still want the blended (non-excluded) view.

Concerns raised — review before merging

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.

2. Maintainer requires a linked issue — Link the relevant issue (for example `Closes #123`) before opening the PR.
Signal Result Evidence
Code review ❌ 2 blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
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 (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 45 registered-repo PR(s), 37 merged, 260 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 45 PR(s), 260 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Improvement ✅ Minor risk: clean · value: minor — Code changes are accompanied by test evidence. LLM value judgment: moderate — It's a narrowly-scoped, well-tested fix for a real false-positive in production ops tooling that traces the bug to its actual source (population confound) rather than patching the detector's threshold.
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: not available
  • Official Gittensor activity: 45 PR(s), 260 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
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.
[BETA] Chat with Gittensory

Ask Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @gittensory ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @gittensory chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @gittensory mention with a real question is routed to the closest matching read-only command automatically -- no exact syntax required.

Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands

🟩 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 14, 2026
@JSONbored
JSONbored merged commit 4ad4995 into main Jul 14, 2026
15 checks passed
@JSONbored
JSONbored deleted the fix/slop-calibration-maintainer-cohort branch July 14, 2026 20:41
This was referenced Jul 14, 2026
This was referenced Jul 15, 2026
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant