Skip to content

fix(review): per-actor rate ceiling + shared budget check on unlinked-issue AI verifier - #4551

Merged
JSONbored merged 2 commits into
mainfrom
feat/unlinked-issue-verifier-rate-ceiling
Jul 10, 2026
Merged

fix(review): per-actor rate ceiling + shared budget check on unlinked-issue AI verifier#4551
JSONbored merged 2 commits into
mainfrom
feat/unlinked-issue-verifier-rate-ceiling

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

The unlinked-issue-match AI verifier (src/review/unlinked-issue-match.ts) had no cost bound of its own — every unlinked PR that produces at least one deterministic pre-filter candidate runs a real AI call, with no per-actor rate awareness and no check against the shared daily neuron budget every other free-tier AI feature (ai-review.ts, ai-slop.ts) already respects.

This adds two gates in resolveUnlinkedIssueMatchDisposition (src/review/unlinked-issue-guardrail.ts), both running before the candidate-verification loop:

  • Per-actor rate ceiling: tracks AI-verifier invocations via a new github_app.unlinked_issue_verify_attempt audit event (cross-repo, keyed on PR author login) and skips verification once an actor hits 15 attempts in an hour. An unidentifiable author (no login) can't be rate-limited individually, so only the shared budget check applies to them — consistent with how the rest of this file already treats an unknown author.
  • Shared daily neuron budget check: estimates the worst case for this PR's candidate loop (every candidate needs a fallback retry) and compares against sumAiEstimatedNeuronsSince/AI_DAILY_NEURON_BUDGET, mirroring ai-slop.ts's own pre-call budget check exactly (same default/clamp) since both draw from the same counter.

Both gates fail toward "proceed as if this layer didn't exist" on any read error — a DB hiccup must never silently disable the guardrail itself, only this additive cost control.

MAX_CANDIDATES (signals/unlinked-issue-candidates.ts) and MAX_TOKENS/DIFF_CHAR_BUDGET (review/unlinked-issue-match.ts) are now exported so the orchestrator sizes its estimate off the same numbers rather than a second, driftable copy.

Fixes #4515

Test plan

  • npx tsc --noEmit clean
  • npx vitest run test/unit/unlinked-issue-guardrail.test.ts test/unit/unlinked-issue-match.test.ts test/unit/unlinked-issue-candidates.test.ts test/unit/ai-slop.test.ts test/unit/ai-review.test.ts — 287/287 passing
  • Targeted coverage on src/review/unlinked-issue-guardrail.ts: 100% branch, 100% lines on all new code (the one function coverage gap reported is a pre-existing fix(review): velocity-aware exception in the unlinked-issue-match escalation for machine-cadence (miner) authors #4512 line untouched by this diff)
  • New tests cover: rate-ceiling-exceeded skip, rate-ceiling boundary (14 ok, 15 blocks), cross-actor isolation, attempt recording, rate-ceiling read-failure fail-safe, budget-exceeded skip, budget-unset default-to-10M, budget read-failure fail-safe

@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.08%. Comparing base (80d623f) to head (b1540df).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4551   +/-   ##
=======================================
  Coverage   94.08%   94.08%           
=======================================
  Files         427      427           
  Lines       37961    37988   +27     
  Branches    13864    13868    +4     
=======================================
+ Hits        35715    35742   +27     
  Misses       1586     1586           
  Partials      660      660           
Files with missing lines Coverage Δ
src/db/repositories.ts 96.58% <100.00%> (+<0.01%) ⬆️
src/review/unlinked-issue-guardrail.ts 100.00% <100.00%> (ø)
src/review/unlinked-issue-match.ts 100.00% <100.00%> (ø)
src/signals/unlinked-issue-candidates.ts 100.00% <100.00%> (ø)
🚀 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 added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 10, 2026
@loopover-orb

loopover-orb Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-10 02:42:59 UTC

5 files · 1 AI reviewer · no blockers · readiness 100/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
Adds two pre-loop cost-control gates (per-actor hourly rate ceiling via a new audit-event type, and a shared daily neuron budget check) to the unlinked-issue-match AI verifier, mirroring ai-slop.ts's existing budget-check pattern and exporting shared constants (MAX_CANDIDATES, MAX_TOKENS, DIFF_CHAR_BUDGET) so the estimate can't drift from the real bounds. Both gates correctly fail open on DB read errors (never disabling the guardrail on a transient hiccup), the rate-ceiling boundary (14 vs 15 attempts) and budget-exhausted/default-high paths are all explicitly tested, and no schema/migration is needed since this only adds a new audit-event `eventType` value on the existing `audit_events` table. The one open question is whether `verifyUnlinkedIssueMatch`'s actual AI calls feed back into `sumAiEstimatedNeuronsSince` via `recordAiUsageEvent` — if not, this new budget gate reads a counter that never reflects its own feature's spend, which would undercut the 'shared budget' guarantee the PR description claims.

Nits — 6 non-blocking
  • src/review/unlinked-issue-guardrail.ts: confirm verifyUnlinkedIssueMatch (or a caller) actually records its own spend via recordAiUsageEvent into the same counter sumAiEstimatedNeuronsSince reads — if it doesn't, this feature's own calls never count against the shared budget it checks, letting total daily spend creep past AI_DAILY_NEURON_BUDGET even though the gate reports 'available'.
  • src/review/unlinked-issue-guardrail.ts:57-60 (VERIFY_MAX_MODEL_ATTEMPTS_PER_CANDIDATE=2): this worst-case assumption (primary + fallback only on thrown error) isn't verifiable from the diff alone — worth a one-line comment pointer to the exact line in unlinked-issue-match.ts that caps retries at 2, so the estimate can't silently drift if that file's retry logic changes later.
  • test/unit/unlinked-issue-guardrail.test.ts: the `env.AI_DAILY_NEURON_BUDGET && Number.isFinite(rawNeuronBudget)` branch is only tested for the empty-string (falsy) arm; a non-numeric-but-truthy value (e.g. "abc") to exercise the Number.isFinite=false arm isn't covered, though this mirrors ai-slop.ts's existing untested branch too.
  • src/db/repositories.ts: countRecentAuditEventsForActor has no index-usage note — worth confirming audit_events has an index on (actor, eventType, createdAt) given this now runs on every unlinked-issue-eligible PR, not just occasionally.
  • Add a short comment or test asserting recordAiUsageEvent is called somewhere in the unlinked-issue-match verify path so the shared-budget claim in the PR description is actually enforced end-to-end, not just checked one-directionally.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #4515
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 (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 48 registered-repo PR(s), 40 merged, 347 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 48 PR(s), 347 issue(s).
Gate result ✅ Passing No configured blocker found.
Linked issue satisfaction

Addressed
The PR adds both a per-actor rate ceiling (15 attempts/hour via a new audit event) and a shared AI_DAILY_NEURON_BUDGET check, both gating the candidate loop before verifyUnlinkedIssueMatch runs, exactly as the issue's deliverables specify, and includes tests for the budget-exceeded fallback, unaffected normal-actor path, and unaffected MAX_CANDIDATES bound. The diff doesn't show a docs update noti

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: 48 PR(s), 347 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 10, 2026
JSONbored added a commit that referenced this pull request Jul 10, 2026
…hared budget counter

Review feedback on #4551: the shared-budget check added in this PR read
sumAiEstimatedNeuronsSince before running the verifier, but nothing wrote
this feature's own actual calls back into that counter -- a free rider that
respected every other feature's spend while never counting its own, so the
true aggregate could silently exceed AI_DAILY_NEURON_BUDGET by however much
this feature used. Record the same worst-case per-candidate estimate the
budget check itself uses, mirroring ai-slop.ts's pre-computed-estimate
recording convention.
@JSONbored
JSONbored force-pushed the feat/unlinked-issue-verifier-rate-ceiling branch from b1f077a to e053ed0 Compare July 10, 2026 02:07
@JSONbored

Copy link
Copy Markdown
Owner Author

Confirmed — good catch. verifyUnlinkedIssueMatch's actual calls never wrote to ai_usage_events, so the budget gate this PR added was reading a counter this feature never contributed to (it respected every other feature's spend but was a free rider on its own).

Fixed in e053ed0: records the same worst-case per-candidate estimate the budget check itself uses into the shared counter, right alongside the existing rate-ceiling attempt recording. Added tests confirming the counter increases after a verification call (including with an unknown author) and that a write failure there is swallowed.

JSONbored added 2 commits July 9, 2026 19:31
…-issue AI verifier

The unlinked-issue-match AI verifier had no cost bound of its own: an actor
opening many low-quality PRs across many repos could trigger arbitrarily many
AI calls in a short window, and the verifier's own candidate loop could run
against the shared daily neuron budget with no awareness of it. Add two gates
ahead of the verification loop, both fail-safe toward "proceed normally" on a
read error so a DB hiccup can never silently disable the guardrail: a
per-actor rate ceiling (tracked via a new audit-event type) and a check
against the same shared daily neuron budget every other free-tier AI feature
draws from.

Fixes #4515
…hared budget counter

Review feedback on #4551: the shared-budget check added in this PR read
sumAiEstimatedNeuronsSince before running the verifier, but nothing wrote
this feature's own actual calls back into that counter -- a free rider that
respected every other feature's spend while never counting its own, so the
true aggregate could silently exceed AI_DAILY_NEURON_BUDGET by however much
this feature used. Record the same worst-case per-candidate estimate the
budget check itself uses, mirroring ai-slop.ts's pre-computed-estimate
recording convention.
@JSONbored
JSONbored force-pushed the feat/unlinked-issue-verifier-rate-ceiling branch from e053ed0 to b1540df Compare July 10, 2026 02:32
@JSONbored
JSONbored merged commit aacfc3b into main Jul 10, 2026
11 checks passed
@JSONbored
JSONbored deleted the feat/unlinked-issue-verifier-rate-ceiling branch July 10, 2026 04:51
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.

fix(review): add a per-actor rate ceiling on the unlinked-issue-match AI verifier, ahead of its existing post-hoc repeat-escalation check

1 participant