From cbeca6dbfd10176f5a97290d76cadcc21effd6f7 Mon Sep 17 00:00:00 2001 From: shin-core <153108882+shin-core@users.noreply.github.com> Date: Fri, 31 Jul 2026 19:26:34 +0900 Subject: [PATCH] fix(services): batch the per-repo outcome-patterns snapshot read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit loadRepoOutcomePatternsMap fired one listSignalSnapshots query per registered repo, concurrently and unbatched, and discarded 99 of the up-to-100 full payloads each one returned to use exactly one — scaling DB round trips linearly with the installed-repo count on the contributor decision-pack build path. The bulk helper for exactly this shape already exists: listRecentSignalSnapshotsForTargets selects payload_json, batches at SIGNAL_SNAPSHOT_TARGET_KEY_SQL_BATCH keys per round trip, and takes an explicit maxPerTarget. Read every registered repo's latest snapshot in one bulk call (listRecentSignalSnapshotsForTargets(env, SIGNAL, fullNames, 1)) instead of the per-repo Promise.all loop, lowercasing the returned keys on the way out to preserve the map's existing lowercased-key contract (the helper keys by the exact targetKey string). The isRegistered filter, the single-repo loadOrComputeRepoOutcomePatternsResponse path, computeRepoOutcomePatterns, and listSignalSnapshots' signature are all unchanged; a registered repo with no snapshot is still absent from the map. Closes #10024 --- src/services/repo-outcome-patterns.ts | 22 ++++--- .../repo-outcome-patterns-service.test.ts | 61 +++++++++++++++++++ 2 files changed, 75 insertions(+), 8 deletions(-) diff --git a/src/services/repo-outcome-patterns.ts b/src/services/repo-outcome-patterns.ts index 59ef2282c7..d3e20d0bf9 100644 --- a/src/services/repo-outcome-patterns.ts +++ b/src/services/repo-outcome-patterns.ts @@ -4,6 +4,7 @@ import { listPullRequestDetailSyncStates, listPullRequests, listRecentMergedPullRequests, + listRecentSignalSnapshotsForTargets, listRepoPullRequestFiles, listRepoPullRequestReviews, listSignalSnapshots, @@ -57,14 +58,19 @@ export async function loadOrComputeRepoOutcomePatternsResponse(env: Env, fullNam export async function loadRepoOutcomePatternsMap(env: Env, repositories: Array<{ fullName: string; isRegistered: boolean }>): Promise> { const map = new Map(); - await Promise.all( - repositories - .filter((repo) => repo.isRegistered) - .map(async (repo) => { - const latest = (await listSignalSnapshots(env, REPO_OUTCOME_PATTERNS_SIGNAL, repo.fullName))[0]; - if (latest) map.set(repo.fullName.toLowerCase(), latest.payload as unknown as RepoOutcomePatterns); - }), - ); + // #10024: one BULK read (batched internally at SIGNAL_SNAPSHOT_TARGET_KEY_SQL_BATCH keys/round-trip) instead + // of one listSignalSnapshots query per registered repo, each of which pulled up to 100 full payloads to use + // exactly one. listRecentSignalSnapshotsForTargets (not the Latest variant) is the one that selects + // payload_json; maxPerTarget 1 = only the newest snapshot per repo. Mirrors repo-doc-refresh-runner's sweep. + const fullNames = repositories.filter((repo) => repo.isRegistered).map((repo) => repo.fullName); + const byTargetKey = await listRecentSignalSnapshotsForTargets(env, REPO_OUTCOME_PATTERNS_SIGNAL, fullNames, 1); + for (const repo of repositories) { + if (!repo.isRegistered) continue; + // listRecentSignalSnapshotsForTargets keys by the exact targetKey string, so read by fullName and lowercase + // on the way out to preserve the map's existing lowercased-key contract (decision-pack.ts's lookups). + const latest = byTargetKey.get(repo.fullName)?.[0]; + if (latest) map.set(repo.fullName.toLowerCase(), latest.payload as unknown as RepoOutcomePatterns); + } return map; } diff --git a/test/unit/repo-outcome-patterns-service.test.ts b/test/unit/repo-outcome-patterns-service.test.ts index cff270b76a..cb72fa0a92 100644 --- a/test/unit/repo-outcome-patterns-service.test.ts +++ b/test/unit/repo-outcome-patterns-service.test.ts @@ -188,4 +188,65 @@ describe("loadRepoOutcomePatternsMap", () => { ]); expect([...map.keys()]).toEqual(["owner/a"]); }); + + const seedSnapshot = async (env: ReturnType, fullName: string, summary: string, targetKey = fullName) => { + await persistSignalSnapshot(env, { + id: crypto.randomUUID(), + signalType: REPO_OUTCOME_PATTERNS_SIGNAL, + targetKey, + repoFullName: fullName, + payload: snapshotPayload(fullName, summary) as unknown as Record, + generatedAt: new Date().toISOString(), + }); + }; + + it("#10024: returns exactly the registered repos' lowercased keys with their payloads; unregistered is absent", async () => { + const env = createTestEnv(); + await seedSnapshot(env, "owner/one", "s1"); + await seedSnapshot(env, "owner/two", "s2"); + await seedSnapshot(env, "owner/three", "s3"); + await seedSnapshot(env, "owner/nope", "s-nope"); // unregistered + const map = await loadRepoOutcomePatternsMap(env, [ + { fullName: "owner/one", isRegistered: true }, + { fullName: "owner/two", isRegistered: true }, + { fullName: "owner/three", isRegistered: true }, + { fullName: "owner/nope", isRegistered: false }, + ]); + expect([...map.keys()].sort()).toEqual(["owner/one", "owner/three", "owner/two"]); + expect(map.get("owner/one")).toMatchObject({ summary: "s1" }); + expect(map.has("owner/nope")).toBe(false); + }); + + it("#10024: the DB round-trip count does NOT grow with the registered-repo count (one batch, 3 vs 12 repos)", async () => { + const countPrepares = async (repoCount: number): Promise => { + const env = createTestEnv(); + let prepares = 0; + const realPrepare = env.DB.prepare.bind(env.DB); + env.DB.prepare = ((sql: string) => { + prepares += 1; + return realPrepare(sql); + }) as never; + const repos = Array.from({ length: repoCount }, (_, i) => ({ fullName: `owner/repo-${i}`, isRegistered: true })); + await loadRepoOutcomePatternsMap(env, repos); + return prepares; + }; + // 3 and 12 both fit one batch (< SIGNAL_SNAPSHOT_TARGET_KEY_SQL_BATCH = 90), so the prepare count is equal. + expect(await countPrepares(3)).toBe(await countPrepares(12)); + }); + + it("#10024 REGRESSION: a stored targetKey whose casing differs from the requested fullName still resolves to a lowercased map key", async () => { + const env = createTestEnv(); + // The request uses "owner/Mixed"; listRecentSignalSnapshotsForTargets keys by the exact requested string, + // and the caller lowercases on the way out — so the map key is the lowercased form, never dropped. + await seedSnapshot(env, "owner/Mixed", "mixed", "owner/Mixed"); + const map = await loadRepoOutcomePatternsMap(env, [{ fullName: "owner/Mixed", isRegistered: true }]); + expect([...map.keys()]).toEqual(["owner/mixed"]); + expect(map.get("owner/mixed")).toMatchObject({ summary: "mixed" }); + }); + + it("#10024: no registered repos ⇒ empty map with no bulk read", async () => { + const env = createTestEnv(); + const map = await loadRepoOutcomePatternsMap(env, [{ fullName: "owner/x", isRegistered: false }]); + expect(map.size).toBe(0); + }); });