Skip to content

services(repo-outcome-patterns): batch the per-repo snapshot read the bulk helper already exists for #10024

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

loadRepoOutcomePatternsMap fires one listSignalSnapshots query per registered repo, concurrently and
unbatched, and discards 99 of the up-to-100 rows each one returns.
src/services/repo-outcome-patterns.ts:58-70:

export async function loadRepoOutcomePatternsMap(env: Env, repositories: Array<{ fullName: string; isRegistered: boolean }>): Promise<Map<string, RepoOutcomePatterns>> {
  const map = new Map<string, RepoOutcomePatterns>();
  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);
      }),
  );
  return map;
}

listSignalSnapshots has no limit parameter and hard-caps at 100 rows including each row's payload_json
(src/db/repositories.ts:6075-6095). repo-outcome-patterns snapshots are written once per repo per
generate-signal-snapshots run (src/queue/signal-snapshot.ts:256-268) and, unlike the latest-only cache
types, are pruned to one row per key by dedupeSignalSnapshots
(src/db/retention.ts:465-475 lists repo-outcome-patterns in LATEST_ONLY_SIGNAL_SNAPSHOT_TYPES) — but
only when the retention job has run since the last write, so between prunes each query pulls up to 100 full
payloads to use exactly one.

src/db/repositories.ts:6099-6103 already documents the bulk helper written for precisely this shape:

/** Bulk variant of `listSignalSnapshots` for callers that need the LATEST snapshot per target key across many
 *  keys in bounded round trips (#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). ...

src/github/repo-doc-refresh-runner.ts:33 uses it. loadRepoOutcomePatternsMap does not, and it is on the
contributor decision-pack build path (src/services/decision-pack.ts:460-462), which runs for every login in
a batch on the scheduled build-contributor-decision-packs job.

listLatestSignalSnapshotsForTargets cannot be used directly here — its projection
(src/db/repositories.ts:6116) omits payload_json and this caller needs the payload — but
listRecentSignalSnapshotsForTargets (src/db/repositories.ts:6150-6199) selects payload_json, batches at
SIGNAL_SNAPSHOT_TARGET_KEY_SQL_BATCH = 90 keys per round trip, and takes an explicit maxPerTarget.

Requirements

  • loadRepoOutcomePatternsMap must read every registered repo's latest repo-outcome-patterns snapshot with a
    single call to listRecentSignalSnapshotsForTargets(env, REPO_OUTCOME_PATTERNS_SIGNAL, fullNames, 1),
    replacing the per-repo Promise.all loop.
  • The returned map's keys must stay lowercased repo full names, exactly as today
    (map.set(repo.fullName.toLowerCase(), ...)), because listRecentSignalSnapshotsForTargets keys by the
    exact targetKey string (src/db/repositories.ts:6149) — the caller must lowercase on the way out. A
    registered repo with no snapshot must be absent from the map, exactly as today.
  • The isRegistered filter must NOT change: only registered repos contribute target keys.
  • listSignalSnapshots' signature and its hard limit(100) must NOT change; other callers depend on it.
  • listRecentSignalSnapshotsForTargets must NOT change.
  • loadOrComputeRepoOutcomePatternsResponse (src/services/repo-outcome-patterns.ts:29-56) is a
    single-repo read and must NOT change in this PR.
  • computeRepoOutcomePatterns must NOT change.

⚠️ Required pattern: src/github/repo-doc-refresh-runner.ts:30-34 — one bulk call over the whole target
list instead of a per-target loop. What does NOT satisfy this issue: (a) adding a limit parameter to
listSignalSnapshots, which still leaves N round trips; (b) writing a third bulk helper in
src/db/repositories.ts when listRecentSignalSnapshotsForTargets already selects payload_json and
batches; (c) capping how many repos are read per call, which silently drops outcome patterns for the
remainder; (d) a test-only PR.

Deliverables

  • loadRepoOutcomePatternsMap issues one listRecentSignalSnapshotsForTargets call (batched internally)
    instead of one listSignalSnapshots call per registered repo.
  • The returned map's keys remain lowercased repo full names.
  • A test in test/unit/repo-outcome-patterns-service.test.ts seeding snapshots for three registered repos
    and one unregistered repo, asserting the returned map has exactly the three lowercased registered keys
    with their payloads, and that the unregistered repo is absent.
  • A test in test/unit/repo-outcome-patterns-service.test.ts that counts DB.prepare invocations and
    asserts the call count does NOT grow with the number of registered repos: 3 repos and 12 repos must
    produce the same number of prepared statements (one batch, since 12 < 90).
  • A regression test at test/unit/repo-outcome-patterns-service.test.ts named for this bug asserting that
    a repo whose stored targetKey casing differs from the requested fullName casing still resolves to a
    lowercased map key, so the exact-casing contract of listRecentSignalSnapshotsForTargets does not
    silently drop it.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example
switching the helper without lowercasing the returned keys, so src/services/decision-pack.ts's lookups
silently miss every repo — does not resolve this issue.

Test Coverage Requirements

This repo enforces 99%+ Codecov patch coverage, branch-counted. vitest.config.ts's coverage.include
covers src/**/*.ts, so src/services/repo-outcome-patterns.ts is measured and gated. The change introduces
one branch — the per-repo "map has an entry for this target key" check — and both arms need a test: a repo
with a snapshot and a registered repo with none. The empty-repositories early path (no registered repos, so
no bulk call at all) is a third distinct path and must be exercised.

Expected Outcome

After this ships, building the contributor decision packs reads every repo's latest outcome-patterns snapshot
in a fixed, bounded number of database round trips rather than one per registered repo, and transfers one row
per repo instead of up to a hundred — matching the batching discipline #3202 established for the sibling
sweep. The map's contents and key casing are unchanged.

Links & Resources

  • src/services/repo-outcome-patterns.ts:58-70 — the per-repo loop
  • src/db/repositories.ts:6075-6095listSignalSnapshots, hard limit(100), full payloads
  • src/db/repositories.ts:6099-6103 — the feat(selfhost): scheduled + on-demand refresh for repo-doc generation #3202 rationale for the bulk helpers
  • src/db/repositories.ts:6150-6199listRecentSignalSnapshotsForTargets (selects payload_json, batches at 90)
  • src/github/repo-doc-refresh-runner.ts:30-34 — the bulk-read precedent
  • src/services/decision-pack.ts:460-462 — the caller on the scheduled decision-pack path

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions