Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions src/db/repositories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4031,6 +4031,47 @@ export async function listSignalSnapshots(env: Env, signalType: string, targetKe
return rows.map(toSignalSnapshotRecord);
}

/** Bulk variant of `listSignalSnapshots` for callers that need the LATEST snapshot per target key across many
* keys in one round trip (#3202 review finding: a per-repo loop here made the daily repo-doc refresh sweep
* scale linearly in DB round trips with the installed-repo count). Keyed by the exact `targetKey` string, same
* casing convention as `listSignalSnapshots` -- callers that key by lowercased repo name must lowercase both
* the input and the returned map's keys themselves. */
export async function listLatestSignalSnapshotsForTargets(
env: Env,
signalType: string,
targetKeys: readonly string[],
): Promise<Map<string, SignalSnapshotRecord>> {
const result = new Map<string, SignalSnapshotRecord>();
if (targetKeys.length === 0) return result;
const placeholders = targetKeys.map(() => "?").join(", ");
const { results } = await env.DB.prepare(
`
SELECT id, signal_type, target_key, repo_full_name, generated_at
FROM (
SELECT
id, signal_type, target_key, repo_full_name, generated_at,
row_number() OVER (PARTITION BY target_key ORDER BY generated_at DESC, id DESC) AS snapshot_rank
FROM signal_snapshots
WHERE signal_type = ? AND target_key IN (${placeholders})
)
WHERE snapshot_rank = 1
`,
)
.bind(signalType, ...targetKeys)
.all<{ id: string; signal_type: string; target_key: string; repo_full_name: string | null; generated_at: string }>();
for (const row of results) {
result.set(row.target_key, {
id: row.id,
signalType: row.signal_type,
targetKey: row.target_key,
repoFullName: row.repo_full_name,
payload: {},
generatedAt: row.generated_at,
});
}
return result;
}

export async function listLatestSignalSnapshotsByTarget(
env: Env,
options: { limit?: number; generatedAfter?: string; maxTargetKeyChars?: number } = {},
Expand Down
12 changes: 11 additions & 1 deletion src/github/repo-doc-refresh-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@
// this, matching #3002's own "manifest-only, no DB layer" precedent for this whole feature. The marker is
// recorded here (not in the sweep itself) so a MANUAL trigger also resets that clock, keeping the sweep from
// immediately re-checking a repo an operator just refreshed by hand.
import { getRepositorySettings, listSignalSnapshots, persistSignalSnapshot } from "../db/repositories";
import { getRepositorySettings, listLatestSignalSnapshotsForTargets, listSignalSnapshots, persistSignalSnapshot } from "../db/repositories";
import { resolveRepoActionMode } from "./client";
import { openRepoDocPullRequest, type RepoDocPullRequestResult } from "./repo-doc-pr";
import { nowIso } from "../utils/json";
import type { SignalSnapshotRecord } from "../types";

const REPO_DOC_REFRESH_ATTEMPT_SIGNAL_TYPE = "repo-doc-refresh-attempt";

Expand All @@ -23,6 +24,15 @@ export async function getLastRepoDocRefreshAttemptedAt(env: Env, repoFullName: s
return snapshots[0]?.generatedAt ?? null;
}

/** Bulk variant for the sweep's fan-out (#3202 review finding): one round trip for every candidate repo
* instead of one `getLastRepoDocRefreshAttemptedAt` call per repo. Keyed by the exact `repoFullName` string
* passed in, same casing convention as the single-repo lookup above. A repo absent from the returned map has
* never been attempted -- callers should read `.get(repoFullName)?.generatedAt ?? null`, same pattern as the
* single-repo lookup above. */
export async function getLastRepoDocRefreshAttemptedAtBulk(env: Env, repoFullNames: readonly string[]): Promise<Map<string, SignalSnapshotRecord>> {
return listLatestSignalSnapshotsForTargets(env, REPO_DOC_REFRESH_ATTEMPT_SIGNAL_TYPE, repoFullNames);
}

async function recordRepoDocRefreshAttempt(env: Env, repoFullName: string): Promise<void> {
await persistSignalSnapshot(env, {
id: crypto.randomUUID(),
Expand Down
24 changes: 16 additions & 8 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ import {
loadRepoReviewContext,
} from "../signals/focus-manifest-loader";
import { resolveRepositorySettings } from "../settings/repository-settings";
import { getLastRepoDocRefreshAttemptedAt, performRepoDocRefresh } from "../github/repo-doc-refresh-runner";
import { getLastRepoDocRefreshAttemptedAtBulk, performRepoDocRefresh } from "../github/repo-doc-refresh-runner";
import { isRepoDocRefreshDue } from "../review/repo-doc-refresh-schedule";
import type { LocalBranchAnalysisInput } from "../signals/local-branch";
import {
Expand Down Expand Up @@ -1725,14 +1725,22 @@ async function fanOutRepoDocRefreshSweepJobs(env: Env, requestedBy: "schedule" |
const now = nowIso();
const repoFullNames = (await listRepositories(env)).map((repo) => repo.fullName);
const manifests = await loadRepoFocusManifests(env, repoFullNames);
const due: string[] = [];
for (const repoFullName of repoFullNames) {
const enabledRepos = repoFullNames.flatMap((repoFullName) => {
const manifest = manifests.get(repoFullName.toLowerCase());
if (!manifest?.repoDocGeneration.enabled) continue;
const lastAttemptedAt = await getLastRepoDocRefreshAttemptedAt(env, repoFullName);
if (!isRepoDocRefreshDue(lastAttemptedAt, manifest.repoDocGeneration.refreshIntervalDays, now)) continue;
due.push(repoFullName);
}
return manifest?.repoDocGeneration.enabled ? [{ repoFullName, manifest }] : [];
});
// Bulk-loaded in ONE round trip rather than one `getLastRepoDocRefreshAttemptedAt` call per repo (#3202
// review finding) -- this sweep runs daily across every installed repo, so a per-repo query here would scale
// linearly in DB round trips with the installed-repo count.
const lastAttempts = await getLastRepoDocRefreshAttemptedAtBulk(
env,
enabledRepos.map((entry) => entry.repoFullName),
);
const due = enabledRepos
.filter((entry) =>
isRepoDocRefreshDue(lastAttempts.get(entry.repoFullName)?.generatedAt ?? null, entry.manifest.repoDocGeneration.refreshIntervalDays, now),
)
.map((entry) => entry.repoFullName);
await Promise.all(
due.map((repoFullName, index) => {
const message: JobMessage = { type: "repo-doc-refresh-sweep", requestedBy, repoFullName };
Expand Down
24 changes: 24 additions & 0 deletions test/unit/data-spine.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
listPullRequestReviews,
listRecentMergedPullRequests,
listRepoLabels,
listLatestSignalSnapshotsForTargets,
listRepoSyncStates,
listSignalSnapshots,
countOpenIssues,
Expand Down Expand Up @@ -489,3 +490,26 @@ describe("data spine repositories", () => {
await expect(updatePullRequestSlopAssessment(env, "owner/sloppr", 999, { slopRisk: 5, slopBand: "low" })).resolves.toBeUndefined();
});
});

describe("listLatestSignalSnapshotsForTargets (#3202 — bulk latest-per-target lookup)", () => {
it("returns an empty map without querying the DB for an empty target-key list", async () => {
const env = createTestEnv();
expect(await listLatestSignalSnapshotsForTargets(env, "queue-health", [])).toEqual(new Map());
});

it("resolves the LATEST snapshot per target key in one call, ignores other signal types, and omits an unmatched key", async () => {
const env = createTestEnv();
await persistSignalSnapshot(env, { id: "a-old", signalType: "repo-doc-refresh-attempt", targetKey: "owner/a", payload: {}, generatedAt: "2026-01-01T00:00:00.000Z" });
await persistSignalSnapshot(env, { id: "a-new", signalType: "repo-doc-refresh-attempt", targetKey: "owner/a", payload: {}, generatedAt: "2026-06-01T00:00:00.000Z" });
await persistSignalSnapshot(env, { id: "b-only", signalType: "repo-doc-refresh-attempt", targetKey: "owner/b", payload: {}, generatedAt: "2026-03-01T00:00:00.000Z" });
// Same target key, but a DIFFERENT signal type -- must not leak into the result.
await persistSignalSnapshot(env, { id: "a-other-signal", signalType: "queue-health", targetKey: "owner/a", payload: {}, generatedAt: "2026-12-01T00:00:00.000Z" });

const result = await listLatestSignalSnapshotsForTargets(env, "repo-doc-refresh-attempt", ["owner/a", "owner/b", "owner/c"]);

expect(result.size).toBe(2);
expect(result.get("owner/a")).toMatchObject({ id: "a-new", generatedAt: "2026-06-01T00:00:00.000Z" });
expect(result.get("owner/b")).toMatchObject({ id: "b-only", generatedAt: "2026-03-01T00:00:00.000Z" });
expect(result.has("owner/c")).toBe(false);
});
});
22 changes: 21 additions & 1 deletion test/unit/repo-doc-refresh-runner.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { generateKeyPairSync } from "node:crypto";
import { afterEach, describe, expect, it, vi } from "vitest";
import { getLastRepoDocRefreshAttemptedAt, performRepoDocRefresh } from "../../src/github/repo-doc-refresh-runner";
import { getLastRepoDocRefreshAttemptedAt, getLastRepoDocRefreshAttemptedAtBulk, performRepoDocRefresh } from "../../src/github/repo-doc-refresh-runner";
import { upsertRepositoryFromGitHub, upsertRepositorySettings } from "../../src/db/repositories";
import { upsertRepoFocusManifest } from "../../src/signals/focus-manifest-loader";
import { createTestEnv } from "../helpers/d1";
Expand Down Expand Up @@ -36,6 +36,26 @@ describe("getLastRepoDocRefreshAttemptedAt (#3003)", () => {
});
});

describe("getLastRepoDocRefreshAttemptedAtBulk (#3202 — N+1 fix)", () => {
it("returns an empty map without querying the DB for an empty repo list", async () => {
const env = createTestEnv();
expect(await getLastRepoDocRefreshAttemptedAtBulk(env, [])).toEqual(new Map());
});

it("resolves attempted repos in one call and omits a repo that was never attempted", async () => {
const env = createTestEnv();
await upsertRepositoryFromGitHub(env, { name: "attempted", full_name: "owner/attempted", private: false, owner: { login: "owner" } });
// repoDocGeneration stays disabled -- performRepoDocRefresh still records an attempt marker on decline.
const result = await performRepoDocRefresh(env, "owner/attempted");
expect(result.opened).toBe(false);
const attemptedAt = await getLastRepoDocRefreshAttemptedAt(env, "owner/attempted");

const bulk = await getLastRepoDocRefreshAttemptedAtBulk(env, ["owner/attempted", "owner/never-attempted"]);
expect(bulk.get("owner/attempted")?.generatedAt).toBe(attemptedAt);
expect(bulk.has("owner/never-attempted")).toBe(false);
});
});

describe("performRepoDocRefresh (#3003)", () => {
afterEach(() => {
vi.unstubAllGlobals();
Expand Down
Loading