fix(maintainability): put the three paid advisories' universal spend stops behind one predicate (#9557)

[JSONbored]

Summary

Deliverable 2 of #9541: the three paid per-PR advisories each hand-wrote their own spend gate, and they drifted.

Closes #9557

The drift, and why no reviewer could have caught it

ai_slop (slop-detection.ts), ai_review (ai-review-orchestration.ts) and the linked-issue satisfaction advisory (processors.ts) each make a paid LLM call per PR head, and each grew its own guard line. #9491 found the consequence: the linked-issue advisory had no per-PR commit cap at all, so a long-lived PR kept paying for a fresh assessment on every push long after its two siblings had stopped.

That is not a mistake anyone catches by reading a function. The guards live in three files, and the incomplete one reads as complete on its own — you have to already know the other two exist to notice what is missing. advisorySpendStopReason makes it structurally impossible for a fourth advisory to be written without those stops.

What is deliberately NOT unified

!confirmedContributor appears in two of the three guards and looks shared. It isn't — ai_review deliberately reviews some unconfirmed authors via resolveAiReviewableAuthor's aiReviewAllAuthors / gate-pack policy.

Folding it in would either silently narrow ai_review's audience or silently widen the other two's. A "shared" rule that is wrong for one caller is exactly how the next #9491 gets written, so the author gate stays per-feature and the module says so — with a test asserting the shared type has not grown a confirmedContributor field.

ai_review keeps its own richer gate (resolvePublicAiReviewGateSkipReason), which already returns a named reason and is the pattern this one follows.

The caller's response to a stop also stays per-feature: ai_slop records an audit event on the cap, the linked-issue advisory returns silently. Unifying the response would have been a behaviour change dressed up as a refactor.

Behaviour-preserving, and pinned as such

The reported reason decides which audit event a PR emits, so a re-ordering would silently relabel real production history. Precedence (paused → no_head_sha → commit_threshold_reached) matches the hand-written guards exactly and is asserted explicitly rather than assumed.

A named reason rather than a boolean, mirroring resolvePublicAiReviewGateSkipReason: callers usually want to audit which stop fired, and a bare false forces every call site to re-derive it.

Validation

• npx tsc --noEmit, dead-source-files:check, git diff --check — all clean
• 244 passed across the shared gate, ai-slop, queue-4, advisory-routing call sites, and the linked-issue runner
• Patch coverage: 0 uncovered changed lines

Regression: the commit-cap stop — the one the linked-issue advisory shipped without — fires.

Invariants: each stop fires independently; null when all preconditions pass; empty-string/null/undefined head SHA all stop; precedence matches the replaced guards; the function is pure and never mutates its input.

Adoption is pinned at the source, so "a fourth advisory cannot be written without these stops" stays true rather than aspirational — the same producer-drift-guard convention db-parsers.test.ts already uses. Tests assert both adopters route through the shared gate, that neither re-implements the commit-cap stop inline, and that the shared module has not grown the author field.

One arm is annotated rather than tested: the ?? null on the audit metadata is unreachable by construction (advisorySpendStopReason checks headSha before the cap, so reaching that branch proves it is a non-empty string) and exists only because TypeScript cannot narrow through the call.

Scope

#9541's other two deliverables — the ~9 command/panel handlers behind one dispatch, and the plan-and-execute pass into its own typed-context module — are not here. They are large behaviour-preserving refactors of a 16.3k-line file, and bundling them with this would make all three unreviewable. #9541 stays open for them.

This one was taken first deliberately: it is the smallest of the three and the only one that directly closes a confirmed, already-shipped defect class rather than reducing the risk of a future one.

[loopover-orb]

Tip

✅ LoopOver review result - approve/merge recommended

_{Review updated: 2026-07-28 09:15:18 UTC}

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

✅ Suggested Action - Approve/Merge

• safe to merge

Review summary
This introduces a single pure predicate `advisorySpendStopReason` (paused → no_head_sha → commit_threshold_reached) and rewires `slop-detection.ts` and `processors.ts`'s linked-issue advisory to use it, while deliberately leaving `confirmedContributor` per-feature and untouched in `ai-review-orchestration.ts`. Tracing both call sites confirms behavior is preserved: `slop-detection.ts` still returns silently on paused/no_head_sha and records the same audit event on the commit-cap stop, and `processors.ts` still returns `null` silently for every stop reason, matching the pre-diff hand-written guards. The tests are unusually rigorous, including source-grep assertions (`readFileSync`) that both adopters actually route through the shared function and don't reintroduce the inline commit-cap check, directly enforcing the stated goal of making the drift structurally impossible.

Nits — 6 non-blocking

• The `?? null` fallback comments in slop-detection.ts (e.g. `metadata: { ... headSha: args.advisory.headSha ?? null }`) are marked unreachable/type-only in three places — consider a single shared helper or a narrower type assertion instead of repeating the same `v8 ignore` justification three times.
• The adoption tests in advisory-spend-gate.test.ts assert on raw string content of source files (`expect(source).not.toContain(...)`) rather than behavior; this is convention-consistent with `db-parsers.test.ts` per the PR description but is inherently brittle to any future refactor/rename in either adopter file.
• `AdvisorySpendPreconditions.headSha` accepts `string | null | undefined` but `commitThresholdReached: boolean` has no equivalent 'not yet computed' state — worth a one-line note on who's responsible for resolving that upstream, though not required by this diff.
• Consider exporting a small helper (e.g. `nonNullHeadSha`) to eliminate the repeated `?? null` + `v8 ignore` boilerplate in slop-detection.ts.
• If a fourth paid advisory is anticipated soon, add a one-line JSDoc pointer to `advisory-spend-gate.ts` from the module doc of `ai-review-orchestration.ts` noting it deliberately does not adopt the shared gate, mirroring the reverse note already in `advisory-spend-gate.ts`.
• 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.

Decision drivers

• ✅ Code review — No blockers (1 reviewer)
• ✅ Gate result — Passing (No configured blocker found.)

Context & advisory signals — never blocks the verdict

Signal	Result	Evidence
Linked issue	✅ Linked	#9557
Related work	✅ No active overlap found	No same-issue or scoped active PR overlap found.
Change scope	❌ 8/20	High 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: 14 registered-repo PR(s), 13 merged, 317 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	JSONbored; Gittensor profile; 14 PR(s), 317 issue(s).
Improvement	✅ Minor	risk: clean · value: minor · LLM: moderate

Linked issue satisfaction

Addressed
The PR adds advisorySpendStopReason returning the named reasons specified, adopts it in both runAiSlopForAdvisory and runLinkedIssueSatisfactionForAdvisory while preserving precedence and per-feature response handling, deliberately excludes the confirmedContributor rule with tests asserting it, and includes tests for precedence, purity, and stop firing including the previously-missing commit cap.

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: Python, TypeScript, Ruby, Go, MDX, Shell, Solidity, JavaScript
• Official Gittensor activity: 14 PR(s), 317 issue(s).
• PR-specific overlap: none found.

Contributor next steps

• Start here: Treat this as maintainer-lane context rather than normal contributor-lane activity.
• Then work through the remaining 2 steps in the Signals table above.

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.

🧪 Chat with LoopOver

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

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

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

_{🟩 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

[superagent-security]

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

[codecov]

Codecov Report

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

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9558      +/-   ##
==========================================
- Coverage   89.60%   88.71%   -0.90%     
==========================================
  Files         861      862       +1     
  Lines      110640   110646       +6     
  Branches    26341    26345       +4     
==========================================
- Hits        99144    98159     -985     
- Misses      10231    11513    +1282     
+ Partials     1265      974     -291     

Flag	Coverage Δ
backend	93.69% <100.00%> (-1.63%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/queue/advisory-spend-gate.ts	100.00% <100.00%> (ø)
src/queue/processors.ts	94.83% <100.00%> (ø)
src/queue/slop-detection.ts	100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

[loopover-orb]

LoopOver approves — the gate is satisfied and CI is green.