Skip to content

feat(miner): throttle discovery concurrency as rate-limit budget drops - #5467

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
real-venus:feat/gittensory-miner-discovery-throttle
Jul 12, 2026
Merged

feat(miner): throttle discovery concurrency as rate-limit budget drops#5467
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
real-venus:feat/gittensory-miner-discovery-throttle

Conversation

@real-venus

Copy link
Copy Markdown
Contributor

Summary

  • The discovery fanout already records GitHub's x-ratelimit-remaining (for telemetry) but nothing slowed its own concurrent fetching in response — a discover run could sprint at full concurrency straight into a 403 (Throttle discovery concurrency when rate-limit remaining gets low #4844).
  • Adds a pure resolveThrottledConcurrency(baseConcurrency, rateLimitRemaining, lowWaterMark, highWaterMark) helper (packages/gittensory-miner/lib/discovery-throttle.js) that maps the recorded remaining budget to an allowed in-flight concurrency: full speed when the budget is unknown or at/above the high-water mark, serialized to a single request at/below the low-water mark, scaling linearly in between.
  • Converts the fanout's mapWithConcurrency pool so each worker parks against a live limit derived from summary.rateLimitRemaining as it is updated by each response — so effective concurrency tapers off as the budget approaches zero instead of continuing at full speed. The initial burst still runs at the configured concurrency (the budget is unknown until the first response), then sustained concurrency backs off.
  • Thresholds are options-configurable (rateLimitLowWaterMark / rateLimitHighWaterMark) with sensible defaults (50 / 250).
  • Strictly a rate-of-fetch change: it only decides how many requests may run concurrently. It never changes which docs are fetched, how issues are paginated, or how a policy verdict is derived.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves — Closes Throttle discovery concurrency when rate-limit remaining gets low #4844.

Validation

  • git diff --check
  • npm run typecheck (after npm --workspace @jsonbored/gittensory-engine run build, as CI does)
  • npm run test:coverage — verified the changed files at 100% line + branch coverage locally via the v8 JSON report: discovery-throttle.js is fully covered, and every changed/added line in opportunity-fanout.js (the new delay, the dynamic mapWithConcurrency, liveConcurrencyResolver, the normalizeOptions threshold wiring, and both call sites) is covered — the remaining uncovered lines in that large file are all pre-existing and outside this diff.
  • New behavior has unit tests: resolveThrottledConcurrency across every branch (unknown budget, at/below low-water, at/above high-water, linear mid-band, custom marks); mapWithConcurrency for the live cap, mid-run taper, and the real-timer park fallback; plus a fanout integration test that completes a multi-target run under a low remaining budget.

If any required check was skipped, explain why:

  • actionlint, test:workers, build:mcp / test:mcp-pack, ui:openapi:check, ui:lint / ui:typecheck / ui:build, and npm audit were not run because this change touches only packages/gittensory-miner/** and test/** — no workflow, worker, MCP, OpenAPI/API, UI, or dependency surface for those jobs to act on. The full npm run test:ci runs all of them on CI.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Public docs/changelogs are updated where needed; no changelog is edited (this is not a release-prep PR).

Auth/CORS/session, API/OpenAPI/MCP, and UI safety boxes are not applicable — this PR changes none of those surfaces. The mapWithConcurrency export is a small internal utility surfaced only so its dynamic-concurrency behavior can be unit-tested deterministically.

The fanout already recorded GitHub's x-ratelimit-remaining but never slowed
its own concurrent fetching in response, so a discover run could sprint at full
concurrency straight into a 403. Add a pure resolveThrottledConcurrency helper
that maps the recorded remaining budget to an allowed in-flight concurrency
(full speed when unknown or above the high-water mark, serialized at/below the
low-water mark, scaling linearly in between), and make mapWithConcurrency park
its pooled workers against that live limit so effective concurrency tapers off
as the budget approaches zero. Thresholds are options-configurable with
sensible defaults. Only decides how many requests run; never changes which docs
are fetched or how a policy verdict is derived.

Closes JSONbored#4844
@real-venus
real-venus requested a review from JSONbored as a code owner July 12, 2026 21:25
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@github-actions
github-actions Bot had a problem deploying to preview/pr-5467 July 12, 2026 21:26 Failure
@codecov

codecov Bot commented Jul 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.71%. Comparing base (ecb18c1) to head (6df71b5).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5467   +/-   ##
=======================================
  Coverage   94.71%   94.71%           
=======================================
  Files         555      556    +1     
  Lines       44526    44546   +20     
  Branches    14664    14664           
=======================================
+ Hits        42172    42192   +20     
  Misses       1619     1619           
  Partials      735      735           
Flag Coverage Δ
shard-1 43.88% <12.50%> (-0.15%) ⬇️
shard-2 35.32% <75.00%> (+0.09%) ⬆️
shard-3 32.01% <12.50%> (+<0.01%) ⬆️
shard-4 31.43% <100.00%> (-0.65%) ⬇️
shard-5 33.15% <87.50%> (+0.41%) ⬆️
shard-6 43.76% <79.16%> (+0.29%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ackages/gittensory-miner/lib/discovery-throttle.js 100.00% <100.00%> (ø)
...ackages/gittensory-miner/lib/opportunity-fanout.js 95.65% <100.00%> (+0.33%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 12, 2026
@loopover-orb

loopover-orb Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-12 21:33:02 UTC

5 files · 2 AI reviewers · no blockers · readiness 100/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR adds a pure `resolveThrottledConcurrency` helper and rewires `mapWithConcurrency` in `opportunity-fanout.js` so each worker parks against a live limit derived from `summary.rateLimitRemaining`, tapering concurrency as the recorded rate-limit budget drops instead of sprinting into a 403. The park/re-check-cursor logic is correctly synchronized (no interleaving await between the `active >= resolveLimit()` check and the `active`/`next` bumps), the linear scaling math is sound at both boundaries, and both new and existing call sites (`fetchCandidateIssuesWithSummary`, `searchCandidateIssuesWithSummary`) are wired through with `liveConcurrencyResolver` and `sleepFn` threaded correctly. Tests cover the pure helper's boundaries, the pool's dynamic tapering, and an end-to-end low-budget fanout run; CI is green across build/typecheck/coverage.

Nits — 5 non-blocking
  • packages/gittensory-miner/lib/opportunity-fanout.js:13 — the `throttleParkMs = 25` constant is fine as a name but is otherwise a bare magic number with no comment on why 25ms specifically (vs. e.g. 10 or 100) was chosen; a one-line rationale would help future tuning.
  • packages/gittensory-miner/lib/opportunity-fanout.js is now ~441 lines, past the repo's informal 400-line file-size guideline — consider splitting the throttle-pool logic (`mapWithConcurrency`, `delay`, `liveConcurrencyResolver`) into its own module alongside `discovery-throttle.js`.
  • The busy-park loop in `mapWithConcurrency` polls every 25ms rather than being woken on completion of an in-flight worker; for high-latency GitHub calls this adds up to 25ms of avoidable idle time per state transition — acceptable given the network-call scale involved, but worth a comment noting it's a deliberate simplicity/latency tradeoff.
  • Consider extracting the throttled worker pool (`delay`, `mapWithConcurrency`, `liveConcurrencyResolver`) into a small `lib/throttled-pool.js` module to keep `opportunity-fanout.js` under the size guideline and make the pool independently testable/reusable.
  • Document the choice of 25ms for `throttleParkMs` (e.g., 'short enough not to meaningfully delay throughput, long enough not to busy-loop') directly at the constant definition.
Signal Result Evidence
Code review ✅ No blockers 2 reviewers, synthesized
Linked issue ✅ Linked #4844
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 106 registered-repo PR(s), 56 merged, 11 issue(s).
Contributor context ✅ Confirmed Gittensor contributor real-venus; Gittensor profile; 106 PR(s), 11 issue(s).
Gate result ✅ Passing No configured blocker found.
Improvement ✅ Minor risk: clean · value: minor — Code changes are accompanied by test evidence. LLM value judgment: moderate — The diff closes a real gap (#4844) by making discovery concurrency responsive to the already-recorded rate-limit budget with a well-tested pure helper and correctly synchronized pool rewrite, though it's a narrow, single-file operational hardening change rather than a broad capability addition.
Linked issue satisfaction

Addressed
The PR adds a resolveThrottledConcurrency helper and wires it into the fanout's mapWithConcurrency so in-flight concurrency dynamically tapers based on the recorded rate-limit remaining, with configurable low/high water marks defaulting to 50/250. Included unit tests simulate a low-remaining-budget scenario (remaining=5, below the low-water mark) and verify the run is serialized (peak concurrency

Review context
  • Author: real-venus
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 106 PR(s), 11 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
[BETA] Chat with Gittensory

Ask Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @gittensory ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @gittensory chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @gittensory mention with a real question is routed to the closest matching read-only command automatically -- no exact syntax required.

Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gittensory approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 6a6426e into JSONbored:main Jul 12, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Throttle discovery concurrency when rate-limit remaining gets low

1 participant