Skip to content

perf(github): dedup + budget-gate the upstream ref→SHA resolve - #1998

Merged
JSONbored merged 1 commit into
mainfrom
claude/upstream-commit-dedup
Jul 1, 2026
Merged

perf(github): dedup + budget-gate the upstream ref→SHA resolve#1998
JSONbored merged 1 commit into
mainfrom
claude/upstream-commit-dedup

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

The upstream ref→SHA pin (GET /repos/{repo}/commits/{ref}) was fetched twice per hour — once in refreshScoringModelSnapshot (refresh-scoring-model) and once in the upstream-drift refresh (refresh-upstream-drift) — via two near-identical private resolvers, both firing in the same hourly cron window as the heavy maintenance fan-out.

This collapses them into one shared resolveUpstreamCommitSha and addresses both halves of the audit item (rec #7):

  • Dedup. A bare /commits/{ref} read (no /check-runs, /status, /pulls, … suffix) is now a short-TTL commit GitHub-response-cache class. Both jobs issue the identical read through timeoutFetch, so the second one within the window is served from the cache (self-host Redis) — one read per window instead of two. Only these two upstream resolves use the bare-/commits/{ref} shape, so nothing else is affected (the mutable CI subresources under /commits/{sha} stay bypassed, as before).
  • Budget-gate. The resolve yields (null → the caller falls back to the mutable ref, its existing degraded path) when the shared REST budget is at/below the low-water floor — so this best-effort audit pin never spends a scarce request during a rate-limit crunch.

TTL is configurable (GITHUB_COMMIT_CACHE_TTL_SECONDS, default 15m), documented in .env.example alongside the sibling cache TTLs.

Advances #1936 (self-host rate-limit RC; rec #7 of the #1942 audit). No Closes#1942 is already closed and there's no dedicated open issue this fully resolves.

Scope

  • Conventional Commit title.
  • Focused: one new shared module + a cache class; both call sites now delegate (removing two duplicated resolvers).
  • Behavior-preserving except the intended dedup/budget-gate; both callers already treat a null SHA as "use the mutable ref".

Validation

  • git diff --check
  • npm run typecheck
  • npm run test:coverage — new resolveUpstreamCommitSha tests (resolve / budget-gate-skips-the-request / end-to-end dedup: 2 resolves → 1 fetch / fail-open) + commit cache-class + TTL assertions; existing scoring-model + upstream-ruleset suites still green.
  • npm run test:ci
  • npm audit --audit-level=moderate

Safety

  • No secrets/wallets/etc. Public-token, read-only.
  • The commit class caches only the bare ref-resolve at a short TTL; a /commits/{sha} SHA is immutable and a branch ref is at most TTL-stale — and the caller already tolerates the fully-mutable-ref fallback.
  • No public GitHub text / auth / UI change.

@dosubot dosubot Bot added the size:M label Jul 1, 2026
@loopover-orb

loopover-orb Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-01 07:08:03 UTC

7 files · 1 AI reviewer · no blockers · readiness 93/100 · CI pending · blocked

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review

Review summary
This change correctly centralizes the upstream ref-to-SHA lookup and wires both hourly callers through the shared resolver, with cache-hit-before-budget-gate behavior covered by focused tests. The new cache class is narrowly matched to bare `/commits/{ref}` reads and keeps mutable ref results on a short TTL, while suffixed commit subresources remain uncached. I do not see a reachable correctness break in the provided diff.

Nits — 5 non-blocking
  • nit: `src/upstream/commit.ts:5` duplicates a GitHub header builder instead of reusing the existing `githubHeaders` convention from the prior callers, so future API-version/header changes can drift between helpers.
  • nit: `src/github/client.ts:108` caches any bare `/commits/{segment}` read, not only the two upstream resolver call sites described in the PR text; that is probably safe for immutable SHAs, but the comment overstates the effective scope.
  • In `src/upstream/commit.ts:5`, reuse the existing GitHub header helper or move this helper next to it so token/auth/version behavior has one source of truth.
  • In `src/github/client.ts:108`, adjust the comment to say the cache class applies to all bare commit reads and is currently used by the upstream resolver path, while suffixed commit subresources remain bypassed.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ No-issue rationale PR body explains why no issue is linked.
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 (size label size:L; no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 68 registered-repo PR(s), 58 merged, 209 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 68 PR(s), 209 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 68 PR(s), 209 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Triage stale or unlinked PRs.
  • No action.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
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.

🟩 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 added gittensor gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. labels Jul 1, 2026
@JSONbored JSONbored self-assigned this Jul 1, 2026
@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.66%. Comparing base (be006e8) to head (cd8e1b1).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1998   +/-   ##
=======================================
  Coverage   95.66%   95.66%           
=======================================
  Files         220      221    +1     
  Lines       24453    24457    +4     
  Branches     8868     8872    +4     
=======================================
+ Hits        23394    23398    +4     
  Misses        436      436           
  Partials      623      623           
Files with missing lines Coverage Δ
src/github/client.ts 100.00% <100.00%> (ø)
src/scoring/model.ts 98.00% <100.00%> (-0.12%) ⬇️
src/upstream/commit.ts 100.00% <100.00%> (ø)
src/upstream/ruleset.ts 98.49% <100.00%> (-0.03%) ⬇️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

refreshScoringModelSnapshot and the upstream-drift refresh each did their own
GET /repos/{repo}/commits/{ref} to pin the upstream ref to a commit SHA — the
same read, twice per hour, in the same cron window as the heavy fan-out.

Collapse the two near-identical resolvers into one resolveUpstreamCommitSha:
- dedup: it's a bare /commits/{ref} read, now a short-TTL 'commit' response-cache
  class, so the second hourly job hits the cache — one read per window, not two;
- budget-gate: it yields (null → caller falls back to the mutable ref) when the
  shared REST budget is depleted, so this best-effort resolve never spends a
  scarce request during a crunch.

Advances #1936.
@JSONbored
JSONbored force-pushed the claude/upstream-commit-dedup branch from 25486fb to cd8e1b1 Compare July 1, 2026 07:03
@dosubot dosubot Bot added size:L and removed size:M labels Jul 1, 2026
@JSONbored

Copy link
Copy Markdown
Owner Author

Fixed the review finding (budget-gate suppressing free cached resolves).

The gate is no longer an upfront check; it's passed as a githubSkipNetworkWhen callback that timeoutFetch consults only when it is about to make a network read. The order inside timeoutFetch is now: serve a cache hit → reuse an in-flight coalesced fetch → then (fresh network read only) consult the skip callback. So under REST pressure a cached ref→SHA is still returned for free; only a genuinely fresh read is skipped (→ synthetic 503 → the caller falls back to the mutable ref, as before). This holds on both the self-host cache path and the no-cache path.

Regression tests added: a cached resolve is served under budget pressure (two resolves, budget depleted on the second → still one fetch), and a cache-miss under pressure still skips the network (no request, null). Existing github-client + scoring + upstream suites green; full test:ci + npm audit clean.

@JSONbored
JSONbored merged commit ec8c07c into main Jul 1, 2026
12 checks passed
@JSONbored
JSONbored deleted the claude/upstream-commit-dedup branch July 1, 2026 07:16
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant