Skip to content

fix(backfill): scheduled per-repo backfill respects freshness/error-backoff - #4529

Merged
JSONbored merged 2 commits into
mainfrom
claude/fix-scheduled-backfill-freshness-backoff
Jul 9, 2026
Merged

fix(backfill): scheduled per-repo backfill respects freshness/error-backoff#4529
JSONbored merged 2 commits into
mainfrom
claude/fix-scheduled-backfill-freshness-backoff

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Closes #4497.

Summary

  • enqueueRepositoryOpenDataBackfill is the function the real scheduled cron path routes through (processors.ts:1021-1029, when a per-repo backfill-registered-repos message has repoFullName set and requestedBy !== "test"), but it had no freshness/error-backoff check at all — every registered repo got a full 4-segment GitHub re-sync every 30 minutes forever, and a repo stuck in status: "error" got retried every 30 min instead of backing off for 1 hour.
  • The correct freshness/backoff logic already existed in backfillRegisteredRepositories, but that function is only reached in production via the synchronous /v1/internal/jobs/backfill-registered-repos/run admin endpoint or requestedBy === "test" — never the scheduled path.
  • Extracted the shared decision into syncFreshnessSkipReason (a pure helper) and wired it into both functions, checked BEFORE any GitHub/DB work in enqueueRepositoryOpenDataBackfill (moved the getRepoSyncState read earlier so a skip avoids the totals fetch too, not just the segment enqueue).
  • src/index.ts:113's comment about "backfillRegisteredRepositories's freshness window" no longer needed correcting — it's now genuinely accurate for both paths since they share the same check.

Scope

Validation

  • npm run typecheck
  • New tests added and verified to FAIL without the fix (temporarily reverted src/github/backfill.ts locally, confirmed all 3 new tests fail with the exact old behavior) and PASS with it.
  • npx vitest run test/unit/backfill.test.ts test/unit/queue.test.ts test/unit/backfill-file-hydration-scoping.test.ts (993 passed)
  • Full npm run test:coverage not re-run unsharded locally for this diff; relying on CI's full gate given the change's narrow blast radius (one shared helper + two call sites already heavily covered by the 993 tests above).

If any required check was skipped, explain why:

  • See above — targeted suite run instead of the full unsharded suite, given scope; CI runs the full gate.

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.

…ackoff

enqueueRepositoryOpenDataBackfill is the function the real scheduled cron
path routes through, but it had no freshness/error-backoff check at all --
every registered repo got a full 4-segment GitHub re-sync every 30 minutes
forever, and a repo stuck in status:error got retried every 30 min instead
of backing off for 1 hour. The correct check already existed in
backfillRegisteredRepositories but that function is only reached via a
synchronous admin endpoint or requestedBy==='test', never the scheduled
path. Extracts the freshness/backoff decision into a shared
syncFreshnessSkipReason helper both functions now call.

Closes #4497.
@superagent-security

Copy link
Copy Markdown
Contributor

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

@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.04%. Comparing base (b069dde) to head (a2d6a82).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4529   +/-   ##
=======================================
  Coverage   94.04%   94.04%           
=======================================
  Files         422      422           
  Lines       37574    37580    +6     
  Branches    13724    13727    +3     
=======================================
+ Hits        35335    35342    +7     
  Misses       1583     1583           
+ Partials      656      655    -1     
Files with missing lines Coverage Δ
src/github/backfill.ts 97.16% <100.00%> (+0.09%) ⬆️
🚀 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 9, 2026
@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 23:20:12 UTC

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

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review: This PR changes guardrail-protected path(s): src/github/backfill.ts (matched src/github/backfill.ts).

Review summary
This PR extracts the freshness/error-backoff decision from backfillRegisteredRepositories into a shared syncFreshnessSkipReason() helper and wires it into enqueueRepositoryOpenDataBackfill — the actual scheduled-cron path per processors.ts:1021-1029 — which previously had no such check at all. The refactor correctly hoists the getRepoSyncState read above the token/totals fetch so a skip avoids GitHub calls entirely, and the three new tests exercise the real scheduled call site (fresh-success skip, recent-error backoff, and a two-tick regression test simulating the incident) rather than fabricated payloads. The logic is a straight extraction with no behavioral drift in backfillRegisteredRepositories (the redundant `&& syncState` guard is harmless since skipReason can only be non-null when syncState is truthy).

Nits — 7 non-blocking
  • A repo whose syncState.status is 'running' (backfill currently in flight) falls through both freshSuccess and recentError as false, so skipReason is null and enqueueRepositoryOpenDataBackfill would re-enqueue on top of an in-progress run — this is pre-existing behavior inherited unchanged from backfillRegisteredRepositories, not introduced by this diff, but now reachable on the scheduled path too and worth a follow-up.
  • codecov/patch is at 93.33% against the repo's 99% target; given the new branch surface in syncFreshnessSkipReason (freshSuccess vs recentError vs neither) and the two call sites, worth checking which specific line/branch is uncovered before merging.
  • src/github/backfill.ts is now ~498 lines, over the repo's apparent 400-line file-size guideline — consider whether syncFreshnessSkipReason and its two call sites warrant splitting into a separate module, though this is a minor structural nit, not blocking.
  • The JSDoc comment on syncFreshnessSkipReason (backfill.ts:322-330) is fairly long for a simple pure predicate; the docstring convention elsewhere in this file tends to be terser.
  • Add a short test (or assert in an existing one) exercising the 'never_synced' / no-lastCompletedAt branch through enqueueRepositoryOpenDataBackfill specifically, to pin down the codecov/patch gap called out above.
  • 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.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #4497
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: 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 ⚠️ Not blocking Advisory; not blocking this PR.
Linked issue satisfaction

Addressed
The diff extracts a shared syncFreshnessSkipReason helper and wires it into enqueueRepositoryOpenDataBackfill before any GitHub/DB work, preserving the force override, and adds tests covering fresh-success skip, recent-error backoff, and a regression test reproducing repeated scheduled dispatch within the freshness window resolving to a single sync — directly matching the issue's core ask and most

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
Codecov flagged a partial branch: the never_synced status value (a row
exists but no sync ever completed) and the stale-syncState-proceeds path
(past both the fresh-success and error-backoff windows) were never
independently exercised. Adds both cases.
@JSONbored
JSONbored merged commit a6fb46a into main Jul 9, 2026
11 checks passed
@JSONbored
JSONbored deleted the claude/fix-scheduled-backfill-freshness-backoff branch July 9, 2026 23:27
JSONbored added a commit that referenced this pull request Jul 31, 2026
…ock every job bumps (#10203)

enqueueRepositoryOpenDataBackfill measured freshness on
repo_sync_state.lastCompletedAt, but refreshRepoSyncStateFromSegments
rewrites that repo-wide column at the tail of EVERY segment write path --
including the ~2-minute re-gate sweep's own force:true open_pull_requests
refresh and the backfill-pr-details follow-on it enqueues, neither of which
dispatches any open-data work. The clock therefore never aged past
FRESH_SYNC_MS and the gate never opened: labels, open_issues and
recent_merged_pull_requests went undispatched from the moment #4529 shipped
(2026-07-09T23:27Z, sixty-eight seconds after the last crawl completed).

Only recent_merged_pull_requests showed it. The other three open-data tables
have webhook or sweep writers that kept them current and masked the dead
crawl, so the table quietly served a three-week-old window to the copycat
containment engine, the repo culture profile (whose cache-invalidation signal
is the frozen row count, so it never regenerated at all), the maintainer
recap, six services, and the public API and MCP surfaces.

Freshness now reads the oldest completion across the four segments the
fan-out actually dispatches, treating a never-run segment as infinitely old.
That measures attempts rather than successes, so #4497's throttle is intact:
a repo that genuinely completed a crawl still backs off for six hours. The
error-backoff window keeps reading lastCompletedAt -- a repo-level error
state is repo-wide by nature.

The skip was also invisible: it was reported only as a warnings[] string that
the cron caller discards, so three weeks of a starved crawl left no log,
metric, or audit trail. It now emits a counter and a log line with the crawl
age, and listContributorRecentMergedPullRequests -- a reader with no
production callers -- is removed.

Closes #10193.
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(backfill): scheduled per-repo backfill ignores freshness/error-backoff, re-syncs every repo every 30 min forever

1 participant