Skip to content

fix(selfhost): bulk-load repo-doc refresh attempt markers in the sweep fan-out - #3219

Merged
loopover-orb[bot] merged 1 commit into
mainfrom
fix/repo-doc-refresh-sweep-n-plus-one
Jul 4, 2026
Merged

fix(selfhost): bulk-load repo-doc refresh attempt markers in the sweep fan-out#3219
loopover-orb[bot] merged 1 commit into
mainfrom
fix/repo-doc-refresh-sweep-n-plus-one

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

Follow-up to #3202 (merged): the Gittensory review bot flagged an N+1 database pattern in fanOutRepoDocRefreshSweepJobs (src/queue/processors.ts) -- it called getLastRepoDocRefreshAttemptedAt once per installed repo inside the fan-out loop, so the daily repo-doc refresh sweep's DB round trips scaled linearly with the installed-repo count. The finding was advisory and did not block #3202's auto-merge (CI was green and mergeable-clean), so it lands here as a targeted follow-up rather than blocking that PR further.

What changed

  • listLatestSignalSnapshotsForTargets (src/db/repositories.ts): a bulk variant of listSignalSnapshots that resolves the LATEST snapshot per target key across a list of keys in one query (window-function row_number() OVER (PARTITION BY target_key ...), same pattern already used by the existing listLatestSignalSnapshotsByTarget).
  • getLastRepoDocRefreshAttemptedAtBulk (src/github/repo-doc-refresh-runner.ts): thin wrapper that keeps the repo-doc-refresh-attempt signal-type constant encapsulated in this module, same as the existing single-repo getLastRepoDocRefreshAttemptedAt.
  • fanOutRepoDocRefreshSweepJobs now filters to enabled repos first, then does ONE bulk lookup for their attempt markers, instead of one query per repo.

Why

The sweep runs once daily across every repo with repoDocGeneration.enabled. A per-repo query here doesn't show up on a small local install, but on a large self-host fleet it turns one cron tick into N sequential DB calls.

Validation

  • npm run typecheck
  • Targeted test run: npx vitest run test/unit/data-spine.test.ts test/unit/repo-doc-refresh-runner.test.ts test/unit/repo-doc-refresh-sweep.test.ts test/unit/mcp-refresh-repo-docs.test.ts test/unit/focus-manifest.test.ts test/unit/index.test.ts -- 298/298 passing
  • npm run test:changed (diff-aware full sweep against origin/main) -- 5500/5507 passing (7 pre-existing skips, 0 failures)
  • Verified 100% branch coverage on every new/changed line via coverage/lcov.info BRDA parsing
  • No behavior change to the sweep's eligibility/due-date semantics -- confirmed via the existing repo-doc-refresh-sweep.test.ts suite passing unchanged

Scope

  • Change is narrowly scoped to this one finding
  • No changes to site/, CNAME, or **/lovable/**
  • No changelog edits
  • No secrets, wallets, hotkeys, trust scores, or reward values added anywhere

Safety

  • No secrets/wallets/hotkeys/trust-score/reward terms anywhere
  • No behavior change to which repos are refreshed or when -- purely a DB access-pattern optimization

…p fan-out

fanOutRepoDocRefreshSweepJobs called getLastRepoDocRefreshAttemptedAt
once per installed repo, an N+1 query pattern flagged in review on
PR #3202 after it merged (the finding was advisory and did not block
the owner-PR auto-merge, so it landed as follow-up work). Adds
listLatestSignalSnapshotsForTargets, a bulk latest-per-target-key
query, and a getLastRepoDocRefreshAttemptedAtBulk wrapper so the
daily sweep does one round trip across every candidate repo instead
of scaling linearly with the installed-repo count.
@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 4, 2026
@loopover-orb

loopover-orb Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Tip

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

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-04 20:40:47 UTC

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

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This change replaces the repo-doc refresh sweep's per-repo attempt-marker lookup with a single latest-per-target signal snapshot query, while preserving the existing signal-type encapsulation in the runner module. The sweep still filters by manifest-enabled repos before checking schedule due-ness, and the new repository helper follows the existing latest-snapshot window-function pattern. The visible tests exercise empty input, signal-type isolation, latest-per-target selection, and the runner wrapper, so the diff is safe to proceed from the provided context.

Nits — 4 non-blocking
  • nit: src/db/repositories.ts:listLatestSignalSnapshotsForTargets returns records with `payload: {}` even though `SignalSnapshotRecord` normally carries persisted payload; that is fine for the current caller but should be documented as metadata-only or renamed if future callers may expect payload parity with `listSignalSnapshots`.
  • nit: src/queue/processors.ts:fanOutRepoDocRefreshSweepJobs now relies on exact-case `repoFullName` keys for `lastAttempts`, so keep the comment accurate if this helper is later reused on paths that normalize repo names to lowercase.
  • src/db/repositories.ts:listLatestSignalSnapshotsForTargets could share a small row-to-metadata mapper with `toSignalSnapshotRecord` only if more metadata-only bulk callers appear; for now the local construction is acceptable.
  • test/unit/data-spine.test.ts: add a tie-break assertion with two rows sharing `generatedAt` and different ids if you want to pin the `ORDER BY generated_at DESC, id DESC` behavior explicitly.
Signal Result Evidence
Code review ✅ No 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: 60 registered-repo PR(s), 50 merged, 438 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 60 PR(s), 438 issue(s).
Gate result ✅ Passing No configured blocker found.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 60 PR(s), 438 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.
  • No action.
  • 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

@codecov

codecov Bot commented Jul 4, 2026

Copy link
Copy Markdown

Codecov Report

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

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3219   +/-   ##
=======================================
  Coverage   94.75%   94.75%           
=======================================
  Files         272      272           
  Lines       29828    29838   +10     
  Branches    10873    10874    +1     
=======================================
+ Hits        28264    28274   +10     
  Misses        918      918           
  Partials      646      646           
Files with missing lines Coverage Δ
src/db/repositories.ts 96.37% <100.00%> (+0.01%) ⬆️
src/github/repo-doc-refresh-runner.ts 100.00% <100.00%> (ø)
src/queue/processors.ts 92.93% <100.00%> (+<0.01%) ⬆️
🚀 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 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@loopover-orb
loopover-orb Bot merged commit a277680 into main Jul 4, 2026
10 checks passed
@loopover-orb
loopover-orb Bot deleted the fix/repo-doc-refresh-sweep-n-plus-one branch July 4, 2026 20:43
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant