Skip to content

refactor(queue): extract AI-slop-advisory gating/orchestration into its own module - #4946

Merged
JSONbored merged 1 commit into
mainfrom
refactor/4013-extract-slop-detection
Jul 11, 2026
Merged

refactor(queue): extract AI-slop-advisory gating/orchestration into its own module#4946
JSONbored merged 1 commit into
mainfrom
refactor/4013-extract-slop-detection

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • Part of refactor(queue): split processors.ts into cohesive modules #4013's module-split sequence for src/queue/processors.ts. Step 4, after transient-locks.ts (refactor(queue): extract transient-lock primitives into their own module #4157) and signal-snapshot.ts (refactor(queue): extract signal-snapshot generation into its own module #4820).
  • Moves shouldCollectSlopEvidence, shouldRunSlopAiAdvisory, runAiSlopForAdvisory into src/queue/slop-detection.ts. Pure mechanical move — no behavior change. mergeReadinessGateEnabled (a trivial one-line predicate, still used by processors.ts's own shouldCollectLinkedIssueEvidence) is inlined directly rather than imported back, for the same one-directional-dependency reason as duplicate-detection.ts's githubAdmissionKeyForToken inline.
  • buildAiReviewDiff and buildSecretScanDiff move to src/review/review-diff.ts — their natural existing home (both already wrapped buildUnifiedReviewDiff there) — rather than staying in processors.ts, which would have made the new slop-detection.ts circularly import this file just for one shared diff-builder. Both are used by 3+ other still-in-processors.ts callers, so processors.ts now imports them back from review-diff.ts alongside its pre-existing buildUnifiedReviewDiff/totalAddedLineCount import.
  • Re-export shims keep this file's own internal callers and the existing test/unit/advisory-ai-routing-call-sites.test.ts, test/unit/ai-slop.test.ts, test/unit/gate-check-policy.test.ts, test/unit/linked-issue-satisfaction-run.test.ts, test/unit/ai-review-advisory.test.ts, test/unit/patchless-secret-scan.test.ts imports from ../../src/queue/processors working unchanged.
  • One real coverage note: isolating runAiSlopForAdvisory in its own small file surfaced a genuinely untested branch — whether settings.aiReviewProvider, when explicitly set, matches or mismatches the stored BYOK key's provider (previously invisible inside processors.ts's overall coverage). Added two tests covering both outcomes (match → BYOK used; mismatch → falls back to Workers AI) rather than suppressing it.

Part of #4013 (more extraction steps remain in the sequence — not closing the tracking issue).

Test plan

  • npx tsc --noEmit -p . — zero errors
  • npx vitest run test/unit test/integration — 696/697 files passed (1 skipped, pre-existing)
  • npm run test:coverage (unsharded) — 94.43% statements / 93.42% branches / 93.6% functions / 95.03% lines, no threshold failures; slop-detection.ts is 100% line + branch + function covered; review-diff.ts's only remaining gaps are in pre-existing, untouched code (keepHighSignalHunks/buildUnifiedReviewDiff), not in the newly-added functions
  • npm run docs:drift-check, npm run manifest:drift-check, npm run engine-parity:drift-check — all ok
  • npm audit --audit-level=moderate — 0 vulnerabilities

…ts own module

Part of #4013's module-split sequence (step 4, after transient-locks.ts
and signal-snapshot.ts): shouldCollectSlopEvidence, shouldRunSlopAiAdvisory,
and runAiSlopForAdvisory move to src/queue/slop-detection.ts. Pure
mechanical move, no behavior change -- a re-export shim keeps this
file's own internal callers and existing test imports working
unchanged.

buildAiReviewDiff and buildSecretScanDiff move to src/review/review-diff.ts
(their natural existing home -- both already wrapped
buildUnifiedReviewDiff there) rather than staying in processors.ts,
which would have made the new slop-detection.ts circularly import this
file.

Closed a genuine, previously-invisible coverage gap surfaced by
isolating this code in its own small file: the BYOK
declared-provider-matches-stored-key branch had no test exercising it
either way, so two tests were added covering both the match and
mismatch outcomes.
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.24%. Comparing base (65422c8) to head (5b74d99).
⚠️ Report is 4 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4946   +/-   ##
=======================================
  Coverage   94.24%   94.24%           
=======================================
  Files         444      445    +1     
  Lines       38854    38851    -3     
  Branches    14157    14154    -3     
=======================================
- Hits        36617    36615    -2     
  Misses       1577     1577           
+ Partials      660      659    -1     
Flag Coverage Δ
shard-1 51.32% <47.50%> (-0.16%) ⬇️
shard-2 45.47% <15.00%> (+0.04%) ⬆️
shard-3 41.85% <85.00%> (-0.03%) ⬇️
shard-4 52.05% <12.50%> (+0.15%) ⬆️

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

Files with missing lines Coverage Δ
src/queue/processors.ts 95.61% <ø> (-0.03%) ⬇️
src/queue/slop-detection.ts 100.00% <100.00%> (ø)
src/review/review-diff.ts 92.85% <100.00%> (+0.92%) ⬆️
🚀 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 11, 2026
@loopover-orb

loopover-orb Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Warning

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

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-11 02:26:42 UTC

4 files · 3 blockers · readiness 93/100 · CI green · unstable

⏸️ Suggested Action - Manual Review

  • AI review already in progress for this PR head: Another Gittensory pass is already running the AI review for this exact PR head. This pass is skipping to avoid a duplicate LLM call.

Review summary
AI review is already running for this PR head in another Gittensory pass. Gittensory is holding this PR for manual review until that pass completes.

Nits — 1 non-blocking
  • AI review already in progress for this PR head — The gate is held for a human reviewer rather than passed automatically; it re-evaluates once the in-flight review completes or on the next update.

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.
  • Possible leaked secret in the diff (generic_secret_assignment) — Remove the secret from the diff, rotate the exposed credential, then re-run the gate.
Signal Result Evidence
Code review ❌ 3 blockers No AI review summary
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: 48 registered-repo PR(s), 40 merged, 312 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 48 PR(s), 312 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: Python, TypeScript, Ruby, Go, JavaScript, MDX, Shell, Solidity
  • Official Gittensor activity: 48 PR(s), 312 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.

🟩 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

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