fix(resolve-did): batch author/contributor resolution to stop 429s - #127
Merged
Conversation
Author bylines (/explore) and the activity-detail contributor list each resolved one DID per row via GET /api/resolve-did on mount. That route is capped at 60 requests/min per IP, so a page with many distinct authors plus a contributor-heavy activity blew the window and returned 429 — the "author/contributors not loaded" + "Failed to load resource: 429" report. Two amplifiers: the byline has no denormalized fallback (always resolves over the network), and the hooks deleted-but-never-recovered failed entries, so a 429'd avatar stayed broken until reload. Collapse N requests into one, mirroring how the home feed already avoids N lookups by denormalizing inline: - Extract the per-DID resolution out of the GET route into a shared resolve-core.ts (GET behaviour byte-identical; its 16 tests still pass). - Add POST /api/resolve-dids: resolves up to 50 identities (DID or handle) in one request with bounded server concurrency — one rate-limit hit per page instead of per row. - Add a DataLoader-style client coalescer (resolve-did-batch.ts): loads in a render pass batch into one POST; dedup + bounded cache; on 429 it degrades to a DID fallback and negative-caches with a TTL (no throw, no retry storm, self-heals on the next view). - Rewire useAuthorInfo / useContributorInfo / useAuthorNamesMap onto the coalescer; public APIs unchanged. A page needing K authors now issues ceil(K/50) requests, not K. Tests: coalescer (batch/dedup/429-degrade/TTL-requery/chunking) + batch route (keying/handle-resolution/cap/dedup/limiter). 533/533 pass; tsc clean; build compiles. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Round-1 implementation review (docs/resolve-did-batch/review-round-1.md): - Weight the batch route's rate limiter by identity count (cost = identities.length) against a 600-identity/min budget, instead of a flat 60-request budget that let one IP drive ~9k upstream fetches/min through the 50x fan-out. Adds a backward-compatible `cost` param to checkHttpRateLimit (incrby only when cost>1; TTL gate generalised to count===cost); every existing cost-1 caller is byte-identical. Side benefit: lifts anonymous users off the GET route's shared "anon" 60/min bucket that itself contributed to the 429s. - Coalescer: honor the 429 cooldown inside flush() (a flush armed before a concurrent chunk's 429 no longer fires early), and track negative-eviction timers in a Map so they're replaced on reschedule and cleared on reset (no timer leak / no fake-timer bleed across tests). - Tests: cooldown-defers-next-batch, mid-flush re-entrancy, incrby-weighted 429; batch mocks updated for incrby. Declined (rationale in review doc): useAuthorNamesMap self-heal (would cause a per-render setTick loop), dead error plumbing in useAuthorInfo (harmless defensive code in the public type), concurrent chunk flush (sequential is intentionally gentle on the limiter). 536/536 tests; tsc clean; build compiles. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
holkexyz
marked this pull request as ready for review
June 2, 2026 15:06
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Symptom
On
/exploreand the activity (cert) detail page, author bylines and the contributor list sometimes fail to load; the console showsFailed to load resource: the server responded with a status of 429.Root cause
/api/resolve-didis rate-limited to 60 requests/min per IP (and per session DID). The byline + contributor components each resolve one DID per row on mount, with no HTTP-level batching, so a page with many distinct authors plus a contributor-heavy activity blows the window. Two amplifiers:displayNameand only fall back to a lookup), the rawActivityRecordcarries only the author DID, so the byline always resolves over the network. The home feed already avoids this by denormalizing inline.cache.delete(did)and rethrew, so a 429'd avatar settled to a stuck skeleton and never recovered on that view."anon"bucket. Signed-out users all share one"anon"DID bucket at 60/min — a global cap that compounds the bursts.Fix
Collapse N requests into one, mirroring the home feed's inline denormalization.
POST /api/resolve-dids— resolves up to 50 identities (DID or handle) in one request with bounded server concurrency (8). The per-DID resolution was extracted verbatim intoresolve-core.tsand is shared with the GET route, so certs/bsky precedence and the indexer fast-path are unchanged (GET stays byte-for-byte; its 16 tests pass untouched).resolve-did-batch.ts) — DataLoader-style. Loads in a render pass batch into one POST; dedup + bounded cache. On a 429 it degrades to a DID fallback and negative-caches with a TTL (no throw, no retry storm, self-heals on a later view). Honors a 429 cooldown across flushes.useAuthorInfo/useContributorInfo/useAuthorNamesMaponto the coalescer. Public APIs unchanged.identities.lengthagainst a 600-identity/min budget (per IP + DID), so the budget bounds upstream fan-out rather than request count. Backward-compatiblecostparam on the shared limiter (cost-1 path unchanged for every existing route). This also gives anonymous users 10x more headroom than the GET path they replace.A page needing K authors now issues
ceil(K/50)requests, not K.Preserved
app.certified.*,org.hypercerts.*, "Certified" strings moved verbatim (net zero change, verified against the diff)./api/resolve-didbehavior, including own-DID cache-control and the edit-time cache bust.Review
Two-reviewer implementation pass; decisions logged in
docs/resolve-did-batch/review-round-1.md(accepted: weighted limit, cooldown-across-flushes, timer-leak fix; declined with rationale: names-map self-heal, dead error plumbing, concurrent chunking).Verification
tsc --noEmitcleaneslintno new errors/warnings on changed filesvitest536/536 (519 existing + 17 new); existing resolve-did + all rate-limit route suites greennext buildcompiles;/api/resolve-didsregistered/exploreand a contributor-heavy activity, confirm bylines/contributors populate without 429s, and that a forced 429 degrades to DID fallbacks rather than stuck skeletons🤖 Generated with Claude Code