fix(backfill): gate the open-data crawl on its own segments, not a clock every job bumps - #10203
Merged
Merged
Conversation
…ock every job bumps 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.
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Contributor
|
Important 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏳ LoopOver is waiting…LoopOver has seen this pull request and is waiting on CI checks to finish before reviewing it. This comment will update once the review runs. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed · 🟨 Waiting |
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
24 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
recent_merged_pull_requestsstopped being written on 2026-07-09T23:26Z. The writer was never removed, flag-gated, or throwing — the entire open-data backfill fan-out (labels,open_issues,open_pull_requests,recent_merged_pull_requests) has been skipped on every 30-minute cron tick since, because #4529's freshness gate reads a clock that unrelated jobs keep rewriting.enqueueRepositoryOpenDataBackfillmeasured freshness onrepo_sync_state.lastCompletedAt.refreshRepoSyncStateFromSegmentsrewrites that repo-wide column at the tail of every segment write path, including two that dispatch no open-data work and run far more often thanFRESH_SYNC_MS: the ~2-minute re-gate sweep's ownforce: trueopen_pull_requestsrefresh (src/queue/processors.ts:1094) and thebackfill-pr-detailsfollow-on it enqueues. The clock never aged past six hours, so the gate never opened.The timing is exact:
a6fb46ab(#4529) landed 2026-07-09T23:27:09Z; the last open-data crawl completed 2026-07-09T23:26:01Z.Only
recent_merged_pull_requestsshowed the damage — it is the one open-data table with no webhook or sweep writer to mask a dead crawl. It has been serving a three-week-old window to the copycat containment engine (src/queue/copycat-detection.ts:99, wired into the live gate by #5999), the repo culture profile (worse than stale: its cache-invalidation signal iscountRecentMergedPullRequests, so a frozen count means the profile never regenerates at all), the maintainer recap, six services, seven API routes, and six MCP tools.The table is not superseded by
pull_requests: nine of twelve repos in it have zero merged rows inpull_requests, and for the three installed repos it holds ~2.5x the pre-07-09 merged history, 5,383 rows of it with the hydratedchanged_files_jsonthe copycat engine matches on. Full evidence in #10193.What changed
waiting_rate_limitminutes ago is not re-attempted. The error-backoff window still readslastCompletedAt; a repo-level error state is repo-wide by nature.warnings[]string that the cron caller (src/queue/job-dispatch.ts:202) discards, which is why three weeks of a starved crawl left no log, metric, or audit trail and was found only by noticing the table had stopped growing. It now emitsloopover_github_open_data_backfill_skipped_total(labelled by reason) and a log line carrying the measured crawl age.listContributorRecentMergedPullRequests, a reader with no production callers.Recovering the missed window — and a 30-day clock
Correcting an earlier reading of mine: the first unblocked run does not close the whole gap. A scheduled run fetches only the top
SEGMENT_PAGE_BUDGETpages of/pulls?state=closed&sort=updated&direction=desc— 2 pages (200 PRs) onlight, 10 (1000) on the 6-hourlyfullwindow — andrecent_merged_pull_requestsis deliberately excluded from theresumere-send atsrc/github/backfill.ts:625, whileprogressiveHistorymaps "complete + hasMore" tosampledrather thanrunning. So a scheduled run never paginates deeper than its page budget. The recent end of the gap self-heals immediately; the deeper 2026-07-09 → mid-July portion needs one forcedfull/resumecrawl via the admin segment endpoint.That matters more than it normally would, because the table is on a deletion clock.
src/db/retention.ts:97prunes it at 30 days onupdated_at, and nothing has refreshedupdated_atsince 2026-07-11T06:25Z — so on the current trajectory the whole table (6,927 rows) ages out by roughly 2026-08-10. The retention policy's own rationale is that these mirrors are "re-fetched from GitHub on the next sync of the PR that needs them"; that assumption is precisely what this bug broke, which turns retention from a bound on growth into an unopposed deletion path. Restoring the writer is what stops the countdown, since an upsert refreshesupdated_at.Rows for the nine uninstalled repos in the table will still age out — they are not dispatched at all — which is consistent with the table's design as a rolling mirror rather than an evidentiary trail.
Scope
wantedPathssite/,CNAME,**/lovable/**, no changelog editValidation
npm run typechecknpm run test:changed— 13,075 passed. One pre-existing failure (test/integration/api.test.ts > serves private MCP tool listing and tool calls) reproduces identically on a clean tree at this base and is unrelated to this diff.npx vitest run test/unit/backfill.test.ts test/unit/data-spine.test.ts— 166 passedselfhost-metrics,alerts-metric-name-references,docs-selfhost-troubleshooting-metric-names,env-example-metric-namesnpm run selfhost:validate-observability,npm run docs:drift-checksrc/github/backfill.ts: every changed statement and both arms of every changed branch are coveredgit diff --checkcleanTests added: a regression test reproducing the exact production shape (segments three weeks old, sync state seconds old ⇒ still syncs, and specifically re-dispatches
recent_merged_pull_requests); invariants for a missing segment row, an unparseable segment timestamp, the full status table the fresh-success window covers (success/partial/capped) plus one outside it, both error-backoff edges, and the skip's log/reason on both paths. Three existing #4497 tests now seed segment rows alongside the sync state — in production asuccessstate only exists because those rows were rolled up into it, so this makes them model reality rather than relaxing them.Safety
Closes #10193.