Skip to content

miner(discover): a missing x-ratelimit-remaining header is recorded as 0 #9678

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 in packages/loopover-miner/lib/opportunity-fanout.ts records GitHub's rate-limit budget from each response:

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

Headers.get() returns null when the header is absent, and Number(null) === 0, which Number.isFinite accepts. So a forge (or reverse proxy) that does not emit x-ratelimit-remaining makes the miner record a remaining budget of 0, and the Math.min pins it at 0 for the rest of the run.

The immediately adjacent sibling in the same function guards against exactly this: if (Number.isFinite(resetSeconds) && resetSeconds > 0).

The consequence is not cosmetic. resolveThrottledConcurrency (packages/loopover-miner/lib/discovery-throttle.ts:27) reads that value and returns 1 for anything at or below DEFAULT_RATE_LIMIT_LOW_WATER_MARK (50), so the entire fan-out serializes to a single in-flight request against a budget that was never actually reported. discovery-throttle.ts's own doc says "an unknown budget (null/non-finite — nothing recorded yet) runs at full baseConcurrency" — defeated by the coercion above. discover-cli.ts:281 likewise documents that unknown "covers the no-fetch/no-header case", which is untrue for the no-header case.

This is reachable in precisely the configuration forge-config.ts exists to support (GitHub Enterprise or another GitHub-compatible forge). The package's own convention elsewhere is the null-guard: http-retry.ts's isRateLimitStatus writes remaining != null && Number(remaining) === 0.

Requirements

  • recordRateLimit must treat an absent x-ratelimit-remaining header as "nothing recorded" (leave summary.rateLimitRemaining untouched), while still recording a genuinely-present 0.
  • The check must read the raw header value first and skip when it is null (or an empty/whitespace-only string), before any Number(...) conversion — matching http-retry.ts's remaining != null guard.
  • A present-but-non-numeric header value (e.g. "abc") must also be skipped, not recorded, preserving the existing Number.isFinite behaviour.
  • The x-ratelimit-reset branch must be left unchanged.

⚠️ Required pattern: mirror isRateLimitStatus in packages/loopover-miner/lib/http-retry.ts — read the header into a local, null-check it, then convert. It does NOT satisfy this issue to change resolveThrottledConcurrency in discovery-throttle.ts to special-case 0 (that would also disable the throttle for a real exhausted budget), to change the low-water-mark constants, or to add a second "headerPresent" flag to RateLimitSummary instead of simply not recording.

Deliverables

  • recordRateLimit leaves summary.rateLimitRemaining unchanged when response.headers.get("x-ratelimit-remaining") returns null or a blank string, and still records the value when the header is present with value "0".
  • A new named regression test in test/unit/miner-opportunity-fanout.test.ts drives a fetch whose response carries no x-ratelimit-remaining header and asserts the returned summary's rateLimitRemaining is null (today it is 0).
  • A second new case asserts a response carrying x-ratelimit-remaining: 0 still records 0.
  • A third new case asserts a response carrying a non-numeric x-ratelimit-remaining records nothing.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example fixing the guard without the present-0 regression test, which is the case that proves the fix did not simply disable rate-limit recording — does not resolve this issue.

Test Coverage Requirements

packages/loopover-miner/lib/**/*.ts IS inside Codecov's coverage.include in vitest.config.ts, so the 99%+ branch-counted codecov/patch gate applies exactly as for src/**. Both arms of the new null/blank guard need a test (header absent, header present), plus the non-numeric arm. Note the existing helper jsonResponse() at test/unit/miner-opportunity-fanout.test.ts:17-26 always injects both rate-limit headers, so the new cases must construct a response without them rather than reusing that helper unchanged.

Expected Outcome

A discover run against a forge that does not emit x-ratelimit-remaining runs at the configured concurrency instead of silently serializing to one request at a time, and discover --json's rateLimitRemaining reads null (unknown) rather than a fabricated 0.

Links & Resources

packages/loopover-miner/lib/opportunity-fanout.ts:210-226, packages/loopover-miner/lib/discovery-throttle.ts:22-33, packages/loopover-miner/lib/http-retry.ts, packages/loopover-miner/lib/discover-cli.ts:281.

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