fix(refresh): defer pr_issue_links backfill off the request path#168
Merged
Conversation
📝 WalkthroughWalkthrough
ChangesNon-blocking PR-issue links backfill
Sequence DiagramsequenceDiagram
participant Handler as RequestHandler
participant Backfill as backfillPrIssueLinksIfNeeded
participant DB as Database
participant Queue as setImmediateQueue
participant Runner as runPrIssueLinksBackfill
Handler->>Backfill: backfillPrIssueLinksIfNeeded(repoFullName)
alt Links exist in pr_issue_links
Backfill->>DB: SELECT COUNT(*) FROM pr_issue_links WHERE repo=repoFullName
DB-->>Backfill: count
Backfill-->>Handler: return count
else No links yet and not in-flight
Backfill->>Queue: schedule runPrIssueLinksBackfill(repoFullName)
Backfill-->>Handler: return 0
Queue->>Runner: invoke runPrIssueLinksBackfill(repoFullName)
Runner->>DB: read pulls, extract issues, INSERT OR IGNORE into pr_issue_links (transaction)
DB-->>Runner: inserted rows count
Runner->>DB: UPDATE repo_meta.pr_issue_links_backfilled_at
Runner-->>Runner: clear inFlightLinksBackfill in finally
else Already in-flight
Backfill-->>Handler: return 0
end
Possibly Related PRs
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/lib/db.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. src/lib/refresh.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. Comment |
Root cause of the Explorer page's "stuck loading skeleton" (Cloudflare 524 timeouts at the origin): `backfillPrIssueLinksIfNeeded` is called inline by 8 API routes (issues/route, pulls/route, issues-meta, related-issues, related-prs, gt/repos/[owner]/[name], etc.). On a cold path where `pr_issue_links` is empty for the repo (e.g. after PR #137's schema-purge migration), the function synchronously: 1. SELECTs every PR + title + body for the repo 2. Runs `extractLinkedIssues` regex over each PR's body 3. Writes resulting links in a single sqlite transaction For a repo with hundreds+ of PRs this can stall the Node event loop for tens of seconds. Cloudflare gives up at 100s and returns 524 to the client; the consumer in RepoExplorer.tsx treats !r.ok as fatal and leaves the Issues / Pull Requests tabs stuck on their skeleton. Fix: split the backfill into a request-path entry point that gates + defers, and a separate `runPrIssueLinksBackfill` that does the heavy work. The entry point now: - Returns immediately when the gate count > 0 (existing fast path, unchanged behaviour for steady state). - On cold start, schedules the backfill via setImmediate and returns 0 to the caller. A per-repo in-flight Set dedupes concurrent cold requests so we don't pile up serial writer-tx queues. The first request after a purge / cold start returns empty linked- issues / linked-PRs enrichment, but the route responds in milliseconds instead of timing out. Subsequent requests (after the deferred backfill commits) get full enrichment. Companion to df1f09c + 2fa2d13 + 99df6b1 on this branch which catch *throws* — those didn't help here because the underlying handler was *hanging*, not erroring. This is the actual root-cause patch.
MkDev11
force-pushed
the
fix/defer-pr-issue-links-backfill
branch
from
f4bfb7c to
725340e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/refresh.ts`:
- Around line 312-331: The current cold-path check only returns early when
existingCount > 0, so repos that legitimately have zero links keep being
rescanned; add a separate persistent "backfill completed" marker and check it
alongside existingCount. Modify the logic around
getReadDb()/existingCount/inFlightLinksBackfill/runPrIssueLinksBackfill so that
before scheduling a backfill you query a new table (e.g.,
pr_issue_links_backfill(repo_full_name, completed_at)) or equivalent flag to see
if backfill completed, and return early if that marker exists (even when
existingCount === 0); after runPrIssueLinksBackfill finishes successfully,
insert or upsert the completed marker for repoFullName; apply the same change to
the duplicate block referenced (lines ~338-343) and ensure markers are only
written on success and not on errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/lib/refresh.ts (1)
323-345:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftPersist “backfill completed” separately from row count.
A successful backfill can still leave
existingCount === 0(for example, no cached PRs yet or no same-repo links found), so this repo never graduates from the cold path. Becausesrc/app/api/repos/[owner]/[name]/issues/route.ts:75-85calls this on every request, those repos will keep scheduling full rescans forever once the in-flight gate clears. Please store a persistent completion marker (for example inrepo_metaor a dedicated table) and check that alongsideexistingCountbefore scheduling again.Possible shape of the fix
export function backfillPrIssueLinksIfNeeded(repoFullName: string): number { const existingCount = (getReadDb() .prepare(`SELECT COUNT(*) AS c FROM pr_issue_links WHERE repo_full_name = ?`) .get(repoFullName) as { c: number }).c; - if (existingCount > 0) return existingCount; + const backfillDone = hasPrIssueLinksBackfillCompleted(repoFullName); + if (existingCount > 0 || backfillDone) return existingCount; if (!inFlightLinksBackfill.has(repoFullName)) { inFlightLinksBackfill.add(repoFullName); setImmediate(() => { try { runPrIssueLinksBackfill(repoFullName); + markPrIssueLinksBackfillCompleted(repoFullName); } catch (err) { const msg = err instanceof Error ? err.message : String(err); console.error(`[refresh] backfillPrIssueLinks(${repoFullName}) failed:`, msg); } finally { inFlightLinksBackfill.delete(repoFullName);This needs a companion schema/migration change so the marker survives restarts and multi-instance deployments.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/refresh.ts` around lines 323 - 345, The cold-path gate in backfillPrIssueLinksIfNeeded currently only checks pr_issue_links row count so repos with zero rows (but already backfilled) will be re-scheduled repeatedly; persist a completion marker in durable storage (e.g. add a column/row in repo_meta or a dedicated backfill_marker table) when runPrIssueLinksBackfill finishes successfully, and update backfillPrIssueLinksIfNeeded to check that marker along with the existingCount before scheduling; keep using inFlightLinksBackfill to prevent concurrent in-memory runs but rely on the persistent marker to avoid re-queuing across restarts/instances and ensure the marker is cleared or versioned if you ever need to re-run the backfill.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/lib/refresh.ts`:
- Around line 323-345: The cold-path gate in backfillPrIssueLinksIfNeeded
currently only checks pr_issue_links row count so repos with zero rows (but
already backfilled) will be re-scheduled repeatedly; persist a completion marker
in durable storage (e.g. add a column/row in repo_meta or a dedicated
backfill_marker table) when runPrIssueLinksBackfill finishes successfully, and
update backfillPrIssueLinksIfNeeded to check that marker along with the
existingCount before scheduling; keep using inFlightLinksBackfill to prevent
concurrent in-memory runs but rely on the persistent marker to avoid re-queuing
across restarts/instances and ensure the marker is cleared or versioned if you
ever need to re-run the backfill.
CodeRabbit caught a real edge case in #168: my cold-path check is `if (existingCount > 0) return existingCount;`. For repos that legitimately have zero linked issues (brand-new repo, or every PR genuinely has no `Closes #N` references), `existingCount === 0` is ALWAYS true → every request to one of the 8 calling routes schedules another backfill → every request re-loads all PRs + bodies and regex-extracts them, even though the previous backfill already ran and confirmed there's nothing to write. In-flight Set only dedupes concurrent schedules, not subsequent ones. Fix: add a persistent marker column `pr_issue_links_backfilled_at` on repo_meta (idempotent migration in db.ts alongside the existing ALTER TABLE backfill columns). The cold-path check now reads the marker too — once set, the repo is considered backfilled even with zero rows. Marker is written by the deferred worker on successful completion only; failures intentionally leave it null so the next request retries. Real cost without this fix: every request for a linkless repo runs the full PR scan in the background. Defer keeps it off the request path so users don't see latency, but it's wasted CPU per-request and amplifies under traffic. Follow-up to 725340e.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/refresh.ts`:
- Around line 334-342: The early return on existingCount prevents backfill when
pr_issue_links already has opportunistic rows; change the gate so we only skip
backfill if the backfill marker pr_issue_links_backfilled_at is set. Concretely,
in the logic around existingCount and completedAt (variables repoFullName,
existingCount, completedAt and column pr_issue_links_backfilled_at) either check
completedAt first and return 0 if present, or require both completedAt and
existingCount before returning; ensure runPrIssueLinksBackfill() is invoked when
completedAt is null even if existingCount > 0 so existing opportunistic rows
don't stop the full backfill (this ties to replacePrIssueLinks() /
refreshIssueLinkedPrsIfStale()).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fc98cd54-4a24-49f0-942b-b5f3c2eceea7
📒 Files selected for processing (2)
src/lib/db.tssrc/lib/refresh.ts
Comment on lines
+334
to
+342
| const existingCount = (readDb | ||
| .prepare(`SELECT COUNT(*) AS c FROM pr_issue_links WHERE repo_full_name = ?`) | ||
| .get(repoFullName) as { c: number }).c; | ||
| if (existingCount > 0) return existingCount; | ||
| const db = getDb(); | ||
| const completedAt = (readDb | ||
| .prepare('SELECT pr_issue_links_backfilled_at FROM repo_meta WHERE full_name = ?') | ||
| .get(repoFullName) as { pr_issue_links_backfilled_at: string | null } | undefined) | ||
| ?.pr_issue_links_backfilled_at; | ||
| if (completedAt) return 0; |
There was a problem hiding this comment.
Don't use existingCount > 0 as the completion gate.
Line 337 short-circuits before the new marker is consulted, but pr_issue_links can already be non-empty from opportunistic inserts in replacePrIssueLinks() / refreshIssueLinkedPrsIfStale() without ever scanning all cached PRs. That means upgraded repos with a few existing rows will never run runPrIssueLinksBackfill() and older cached PRs stay partially linked forever.
Suggested fix
const existingCount = (readDb
.prepare(`SELECT COUNT(*) AS c FROM pr_issue_links WHERE repo_full_name = ?`)
.get(repoFullName) as { c: number }).c;
- if (existingCount > 0) return existingCount;
const completedAt = (readDb
.prepare('SELECT pr_issue_links_backfilled_at FROM repo_meta WHERE full_name = ?')
.get(repoFullName) as { pr_issue_links_backfilled_at: string | null } | undefined)
?.pr_issue_links_backfilled_at;
- if (completedAt) return 0;
+ if (completedAt) return existingCount;
// Cold path: schedule the backfill OFF the request path.
if (!inFlightLinksBackfill.has(repoFullName)) {
inFlightLinksBackfill.add(repoFullName);
setImmediate(() => {
@@
}
});
}
- return 0;
+ return existingCount;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/refresh.ts` around lines 334 - 342, The early return on existingCount
prevents backfill when pr_issue_links already has opportunistic rows; change the
gate so we only skip backfill if the backfill marker
pr_issue_links_backfilled_at is set. Concretely, in the logic around
existingCount and completedAt (variables repoFullName, existingCount,
completedAt and column pr_issue_links_backfilled_at) either check completedAt
first and return 0 if present, or require both completedAt and existingCount
before returning; ensure runPrIssueLinksBackfill() is invoked when completedAt
is null even if existingCount > 0 so existing opportunistic rows don't stop the
full backfill (this ties to replacePrIssueLinks() /
refreshIssueLinkedPrsIfStale()).
MkDev11
added a commit
that referenced
this pull request
May 27, 2026
* fix(refresh): defer pr_issue_links backfill off the request path Root cause of the Explorer page's "stuck loading skeleton" (Cloudflare 524 timeouts at the origin): `backfillPrIssueLinksIfNeeded` is called inline by 8 API routes (issues/route, pulls/route, issues-meta, related-issues, related-prs, gt/repos/[owner]/[name], etc.). On a cold path where `pr_issue_links` is empty for the repo (e.g. after PR #137's schema-purge migration), the function synchronously: 1. SELECTs every PR + title + body for the repo 2. Runs `extractLinkedIssues` regex over each PR's body 3. Writes resulting links in a single sqlite transaction For a repo with hundreds+ of PRs this can stall the Node event loop for tens of seconds. Cloudflare gives up at 100s and returns 524 to the client; the consumer in RepoExplorer.tsx treats !r.ok as fatal and leaves the Issues / Pull Requests tabs stuck on their skeleton. Fix: split the backfill into a request-path entry point that gates + defers, and a separate `runPrIssueLinksBackfill` that does the heavy work. The entry point now: - Returns immediately when the gate count > 0 (existing fast path, unchanged behaviour for steady state). - On cold start, schedules the backfill via setImmediate and returns 0 to the caller. A per-repo in-flight Set dedupes concurrent cold requests so we don't pile up serial writer-tx queues. The first request after a purge / cold start returns empty linked- issues / linked-PRs enrichment, but the route responds in milliseconds instead of timing out. Subsequent requests (after the deferred backfill commits) get full enrichment. Companion to df1f09c + 2fa2d13 + 99df6b1 on this branch which catch *throws* — those didn't help here because the underlying handler was *hanging*, not erroring. This is the actual root-cause patch. * fix(refresh): persistent backfill marker so linkless repos don't re-scan CodeRabbit caught a real edge case in #168: my cold-path check is `if (existingCount > 0) return existingCount;`. For repos that legitimately have zero linked issues (brand-new repo, or every PR genuinely has no `Closes #N` references), `existingCount === 0` is ALWAYS true → every request to one of the 8 calling routes schedules another backfill → every request re-loads all PRs + bodies and regex-extracts them, even though the previous backfill already ran and confirmed there's nothing to write. In-flight Set only dedupes concurrent schedules, not subsequent ones. Fix: add a persistent marker column `pr_issue_links_backfilled_at` on repo_meta (idempotent migration in db.ts alongside the existing ALTER TABLE backfill columns). The cold-path check now reads the marker too — once set, the repo is considered backfilled even with zero rows. Marker is written by the deferred worker on successful completion only; failures intentionally leave it null so the next request retries. Real cost without this fix: every request for a linkless repo runs the full PR scan in the background. Defer keeps it off the request path so users don't see latency, but it's wasted CPU per-request and amplifies under traffic. Follow-up to 725340e. * fix(pulls): scope cache to live gittensor repos
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
1 participant
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 by CodeRabbit