Skip to content

miner(discover): the search endpoint's rate-limit budget is folded into the core budget, pinning --search fan-out to serial concurrency #10005

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

recordRateLimit folds every response's x-ratelimit-remaining into one number with Math.min
packages/loopover-miner/lib/opportunity-fanout.ts:210-225:

  const rawRemaining = response.headers.get("x-ratelimit-remaining");
  if (rawRemaining !== null && rawRemaining.trim() !== "") {
    const remaining = Number(rawRemaining);
    if (Number.isFinite(remaining)) {
      summary.rateLimitRemaining =
        summary.rateLimitRemaining === null
          ? remaining
          : Math.min(summary.rateLimitRemaining, remaining);
    }
  }

It is called from githubGetJson (packages/loopover-miner/lib/opportunity-fanout.ts:257), which serves both
endpoint families:

  • the search endpoint, forge.searchEndpoint = /search/issues
    (packages/loopover-miner/lib/forge-config.ts:29), used by fetchSearchIssues
    (packages/loopover-miner/lib/opportunity-fanout.ts:572-581);
  • the per-repo contents endpoint, `${repoPathPrefix}/${owner}/${repo}/contents/AI-USAGE.md` used by
    fetchRepoDoc (packages/loopover-miner/lib/opportunity-fanout.ts:351-358), and the issues list.

GitHub bills those against two independent primary rate-limit resources — search and core — and reports each
response's own resource in x-ratelimit-resource. The search budget is a small per-minute allowance; core is a
large per-hour allowance. Folding them with Math.min therefore records the search bucket's tiny remaining count
as if it were the whole run's budget.

That number is not just telemetry — it drives the concurrency throttle.
searchCandidateIssuesWithSummary runs the search first (packages/loopover-miner/lib/opportunity-fanout.ts:759)
and only then fans out policy resolution over every discovered repo
(packages/loopover-miner/lib/opportunity-fanout.ts:767-776), gated by:

function liveConcurrencyResolver(normalizedOptions: NormalizedOptions, summary: RateLimitSummary): () => number {
  return () =>
    resolveThrottledConcurrency(
      normalizedOptions.concurrency,
      summary.rateLimitRemaining,
      normalizedOptions.rateLimitLowWaterMark,
      normalizedOptions.rateLimitHighWaterMark,
    );
}

resolveThrottledConcurrency (packages/loopover-miner/lib/discovery-throttle.ts:20-33) returns 1 for any
remaining budget at or below lowWaterMark, whose default is 50
(packages/loopover-miner/lib/discovery-throttle.ts:8). A single /search/issues request leaves a search-bucket
remaining well under 50, so summary.rateLimitRemaining is pinned there and every subsequent policy-doc fetch
in the run is serialized to one in-flight request — even though the core budget those requests actually spend has
thousands left.

The same wrong number is printed to the operator. renderRateLimitLine
(packages/loopover-miner/lib/discover-cli.ts:293-297) is documented at :290-292 as showing "how close a
discover run is to being throttled"
, and reports the search bucket's count as the run's remaining budget.

Note that recordRateLimit's own header (packages/loopover-miner/lib/opportunity-fanout.ts:211-215) shows the
intent is a faithful budget reading — #9678 fixed an absent header being recorded as 0 for exactly this
reason. Folding a second, unrelated bucket into the same field is the same class of wrong reading.

packages/loopover-miner/lib/discovery-throttle.ts:1-5 states the helper's contract plainly: "the fanout already
records GitHub's x-ratelimit-remaining, but nothing slowed its own concurrent fetching in response ... This pure
helper maps the recorded remaining budget to an allowed in-flight concurrency so the fanout tapers off as the
budget approaches zero."
The budget it is meant to taper against is the one the fanned-out requests spend.

Requirements

  • recordRateLimit must only fold a response's x-ratelimit-remaining into
    summary.rateLimitRemaining when that response was billed against the same resource the fanned-out per-repo
    requests spend. Use the response's x-ratelimit-resource header: a response reporting a resource other than
    core must be skipped.
  • A response with no x-ratelimit-resource header must be treated exactly as today (recorded), so a forge or
    proxy that omits it, and every existing test fixture, behaves identically — this must not become a new way to
    silently record nothing.
  • The existing #9678 guard must be preserved verbatim: an absent or blank x-ratelimit-remaining is skipped, a
    present "0" is recorded, a present-but-non-numeric value is skipped.
  • x-ratelimit-reset recording (packages/loopover-miner/lib/opportunity-fanout.ts:226-233) must be scoped the
    same way, so rateLimitResetAt cannot describe a bucket rateLimitRemaining no longer refers to.
  • The public CandidateIssueSummary shape (packages/loopover-miner/lib/opportunity-fanout.ts:62-67) must not
    change — no new fields, no renames. Its rateLimitRemaining simply becomes the core budget.
  • Non-goal, stated explicitly so it is not implemented: search paging is bounded by maxPages
    (packages/loopover-miner/lib/opportunity-fanout.ts:101, default 10) and needs no throttle of its own. Do NOT
    add a second throttle, a second summary field, or a per-resource concurrency resolver.
  • resolveThrottledConcurrency and discovery-throttle.ts must not change.

⚠️ Required pattern: keep the change inside recordRateLimit
(packages/loopover-miner/lib/opportunity-fanout.ts:210-234), mirroring the defensive header-read style already
there (response.headers.get(...), null/blank check, then convert). What does NOT satisfy this issue:
(a) skipping recordRateLimit for responses from forge.searchEndpoint by string-matching the request URL,
which breaks for a custom-forge searchEndpoint and for any future endpoint on a non-core bucket; (b) adding a
separate searchRateLimitRemaining field to CandidateIssueSummary and leaving rateLimitRemaining conflated;
(c) raising DEFAULT_RATE_LIMIT_LOW_WATER_MARK so the search bucket stops tripping the throttle, which just
hides the wrong reading and weakens the core-budget taper.

Deliverables

  • recordRateLimit in packages/loopover-miner/lib/opportunity-fanout.ts skips a response whose
    x-ratelimit-resource is present and not core, and records one whose header is absent or core.
  • searchCandidateIssuesWithSummary with a stubbed fetch where the /search/issues response carries
    x-ratelimit-remaining: 29, x-ratelimit-resource: search and every /contents/ response carries
    x-ratelimit-remaining: 4990, x-ratelimit-resource: core returns
    rateLimitRemaining === 4990 — asserted in test/unit/miner-opportunity-fanout.test.ts (or the existing
    test/unit/miner-opportunity-fanout-forge.test.ts, whichever already holds the rate-limit fixtures).
  • In that same scenario the policy fan-out is NOT serialized: with concurrency: 5 and several distinct
    repos in the search results, the observed maximum in-flight /contents/ requests is greater than 1 —
    asserted with an instrumented fetch stub in the same test file.
  • A response with x-ratelimit-remaining: 12 and no x-ratelimit-resource header is still recorded
    (rateLimitRemaining === 12) — asserted in the same test file.
  • rateLimitResetAt is likewise taken only from core-billed (or resource-header-less) responses — asserted
    with a search response carrying a far-future x-ratelimit-reset that must NOT appear in the summary.
  • A regression test named for this bug (e.g. REGRESSION: the search bucket's remaining budget does not pin the core fan-out to serial concurrency) that fails against the current code.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example one that
scopes rateLimitRemaining but leaves rateLimitResetAt reporting the search bucket's reset time — does not
resolve this issue.

Test Coverage Requirements

This repo enforces 99%+ Codecov patch coverage, branch-counted. vitest.config.ts's coverage.include lists
packages/loopover-miner/lib/**/*.ts, so opportunity-fanout.ts is measured and gated. Every branch the change
introduces needs both arms tested: x-ratelimit-resource absent vs present; present-and-core vs
present-and-not-core; and both must be combined with the existing rawRemaining !== null && trim() !== "" and
Number.isFinite(remaining) arms, plus the summary.rateLimitRemaining === null first-write vs Math.min
subsequent-write arms, and the same matrix for the resetSeconds > 0 guard.

Expected Outcome

A loopover-miner discover --search … run reports the core rate-limit budget its per-repo requests actually spend,
and its policy-doc fan-out runs at the configured concurrency instead of being pinned to one in-flight request by
the unrelated, much smaller search-endpoint budget.

Links & Resources

  • packages/loopover-miner/lib/opportunity-fanout.ts:210-234recordRateLimit
  • packages/loopover-miner/lib/opportunity-fanout.ts:236-260githubGetJson, the single call site for both
    endpoint families
  • packages/loopover-miner/lib/opportunity-fanout.ts:748-794searchCandidateIssuesWithSummary: search first,
    then the throttled policy fan-out
  • packages/loopover-miner/lib/discovery-throttle.ts:8, :20-33 — the low-water mark and the taper
  • packages/loopover-miner/lib/discover-cli.ts:290-297 — the operator-facing rate-limit line
  • packages/loopover-miner/lib/forge-config.ts:29searchEndpoint

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