Skip to content

test(review): add a cross-order-mode invariant test for the sweep one-shot fix - #4475

Merged
JSONbored merged 1 commit into
mainfrom
claude/guardrail-review-freeze-and-invariants
Jul 9, 2026
Merged

test(review): add a cross-order-mode invariant test for the sweep one-shot fix#4475
JSONbored merged 1 commit into
mainfrom
claude/guardrail-review-freeze-and-invariants

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • Follow-up to fix(review): stop the sweep from endlessly re-evaluating already-reviewed PRs #4474 (the sweep endless-reregate fix): adds an INVARIANT test for selectRegateCandidates covering both order modes (staleness, oldest-first) across a simulated multi-sweep drain with a mixed backlog (never-regated, already-regated, repair-priority PRs), asserting an already-regated non-repair PR never resurfaces on any later sweep.
  • Test-only change; no production code touched.

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.
  • Owner PR, no linked issue required (test-hardening follow-up to an already-merged incident fix, fix(review): stop the sweep from endlessly re-evaluating already-reviewed PRs #4474).

Validation

  • npm run typecheck
  • npx vitest run test/unit/agent-sweep.test.ts test/unit/queue.test.ts (791 passed)
  • Full npm run test:coverage not re-run locally for this test-only diff; test/** is Codecov-ignored and CI runs the full gate.

If any required check was skipped, explain why:

  • This is a test-only addition (no src/** lines changed), so codecov/patch has nothing to measure; relying on CI for the full gate per house convention for test-only diffs.

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.
  • N/A — no auth/cookie/CORS/GitHub App/Cloudflare/session changes.
  • N/A — no API/OpenAPI/MCP behavior changed.
  • N/A — no UI changes.

Notes

  • Also verified live in production that the separate guardrail-hold-freeze concern raised alongside this incident is already covered by an existing mechanism (PR fix(review): stop same-head AI re-review churn and add a maintainer-gated freeze #3461, merged 2026-07-05): once a PR carries the manual-review label, subsequent contributor pushes reuse the last published AI review (github_app.ai_review_frozen_reuse) instead of spending a fresh call, until a maintainer explicitly retriggers it. No new code needed there.

…-shot fix

Adds an INVARIANT test to selectRegateCandidates covering both order modes
(staleness, oldest-first) across a multi-sweep drain with a mixed backlog of
never-regated, already-regated, and repair-priority PRs, asserting an
already-regated non-repair PR never resurfaces on any later pass. Guards
against the endless-reregate class of bug silently regressing.
@superagent-security

Copy link
Copy Markdown
Contributor

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

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

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.03%. Comparing base (8fafbdd) to head (2d508b2).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4475   +/-   ##
=======================================
  Coverage   94.03%   94.03%           
=======================================
  Files         420      420           
  Lines       37521    37521           
  Branches    13701    13701           
=======================================
  Hits        35282    35282           
  Misses       1583     1583           
  Partials      656      656           
🚀 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 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Warning

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

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-09 21:20:17 UTC

1 file · 1 AI reviewer · 1 blocker · readiness 100/100 · CI green · unstable

⏸️ Suggested Action - Manual Review

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

Review summary
This is a test-only follow-up to #4474 that adds an invariant test for selectRegateCandidates, simulating an 8-sweep drain across a 30-PR mixed backlog (never-regated, already-regated, repair-priority) under both order modes, asserting a non-repair PR that was already regated is never re-selected on a later sweep. I traced the test's setup against the actual selectRegateCandidates source: updatedAt is pinned far outside SWEEP_FRESHNESS_MS so the freshness guard never masks the assertion, the already-regated/never-regated split (number % 3, excluding the repair set) correctly seeds both the oldest-first initial-drain pool and the staleness fallback path, and the per-sweep stampedAt map correctly feeds evolving lastRegatedAt state back into the next sweep's view — this is a real invariant test exercising the real code path, not a fabricated scenario. No production code is touched, CI is fully green, and the PR is an owner follow-up to an already-merged incident fix so the issue-link requirement doesn't block it.

Nits — 4 non-blocking
  • The repair-priority carve-out in the assertion (test/unit/agent-sweep.test.ts, new block) only checks stampedAt for non-repair PRs but never separately asserts that repair-priority PRs *are* actually re-picked at least once across the 8 sweeps, so a regression that silently dropped repair bypass entirely would not be caught by this test.
  • The 54-line test body inlines a fair amount of setup (basePulls construction, per-sweep view remapping) that duplicates shape already used in the 'REGRESSION (convergence)' test above it — consider extracting a shared multi-sweep-drain helper if a third test needs the same scaffolding.
  • Consider adding one assertion that each repair-priority number in repairPullNumbers appears in `picked` at least once across the run, to make the bypass side of the invariant explicit rather than implied by absence of a failure.
  • If a shared drain-simulation helper gets extracted for a future test, put it near the existing convergence test's helpers to keep both tests visibly sharing one source of truth for the sweep simulation loop.

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.
Signal Result Evidence
Code review ❌ 1 blocker 1 reviewer
Linked issue ✅ No-issue rationale PR body explains why no issue is linked.
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: 49 registered-repo PR(s), 41 merged, 377 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 49 PR(s), 377 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
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: 49 PR(s), 377 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 9, 2026
@JSONbored
JSONbored merged commit 9bcfd5d into main Jul 9, 2026
11 checks passed
@JSONbored
JSONbored deleted the claude/guardrail-review-freeze-and-invariants branch July 9, 2026 21:29
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