fix(registry,aggregator): specificity-sorted priority + soft-cap enforcement + Twilio price docs (#13, #21)#38
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR implements Wave 4 of the provider registry overhaul, fixing custom rule shadowing, endpoint cardinality leaks, and async bucket cap gaps. Provider registry rules are now specificity-sorted by pathPrefix presence and length, exact host matching, and custom tie-break semantics. Unknown endpoint paths normalize to ChangesProvider Registry and Aggregator Wave 4 Overhaul
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/plans/2026-05-15-provider-registry-overhaul.md`:
- Around line 1237-1255: Several fenced code blocks in the
provider-registry-overhaul plan lack language tags, causing markdownlint MD040
and downstream parsing issues; update the code fences around the "init({
customProviders... })" example to use ```ts, change the surrounding
narrative/code blocks that show headings to ```markdown, and change the
changelog-style block to ```text (apply the same fixes to the other occurrences
noted around the Cleanup/teardown and changelog sections referenced in the
comment); ensure each opening triple-backtick has an appropriate language
identifier matching the block content.
In `@src/core/provider-registry.ts`:
- Around line 191-201: The constructor currently mixes customProviders and
BUILTIN_PROVIDERS together and sorts them jointly, changing the declared
precedence; restore "custom prepended before built-ins" by sorting
customProviders and BUILTIN_PROVIDERS separately with compareRules and then
assigning this._rules to the concatenation of the sorted custom list followed by
the sorted builtin list (use the existing compareRules, ProviderDef type,
BUILTIN_PROVIDERS, and _rules names to locate and implement this change).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 78932933-beef-4cc8-b317-559446161f39
📒 Files selected for processing (7)
README.mddocs/superpowers/plans/2026-05-15-provider-registry-overhaul.mddocs/superpowers/roadmap-2026-05-13-issue-waves.mdsrc/core/aggregator.tssrc/core/provider-registry.tstests/aggregator.test.tstests/provider-registry.test.ts
| * @param customProviders - Optional extra rules. Merged with built-ins and | ||
| * sorted by specificity (longer `pathPrefix` first, exact host before | ||
| * wildcard, custom-wins-on-tie). See the class JSDoc for the full rule. | ||
| */ | ||
| constructor(customProviders: ProviderDef[] = []) { | ||
| this._rules = [...customProviders, ...BUILTIN_PROVIDERS]; | ||
| const tagged: { rule: ProviderDef; custom: boolean }[] = [ | ||
| ...customProviders.map((rule) => ({ rule, custom: true })), | ||
| ...BUILTIN_PROVIDERS.map((rule) => ({ rule, custom: false })), | ||
| ]; | ||
| tagged.sort(compareRules); | ||
| this._rules = tagged.map((t) => t.rule); |
There was a problem hiding this comment.
Custom-provider precedence contract changed away from prepended-first behavior.
The constructor no longer prepends customProviders before built-ins; it globally sorts both lists, which changes the declared precedence model for this module. If this behavior change is intentional, the repository rule for this file should be updated in the same PR; otherwise restore prepended custom priority to match the contract.
As per coding guidelines, src/core/provider-registry.ts: “Implement Provider Registry with 34 built-in rules covering 14 providers, wildcard host matching, and custom provider priority prepended before built-ins”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/provider-registry.ts` around lines 191 - 201, The constructor
currently mixes customProviders and BUILTIN_PROVIDERS together and sorts them
jointly, changing the declared precedence; restore "custom prepended before
built-ins" by sorting customProviders and BUILTIN_PROVIDERS separately with
compareRules and then assigning this._rules to the concatenation of the sorted
custom list followed by the sorted builtin list (use the existing compareRules,
ProviderDef type, BUILTIN_PROVIDERS, and _rules names to locate and implement
this change).
- CLAUDE.md: update registry contract to reflect specificity-sorted rules with custom-wins-on-tie (the PR intentionally moved away from unconditional custom-prepended priority to stop custom catch-alls shadowing built-in path-specific rules) - plan doc: use 4-backtick outer fences around the README-replacement blocks so the nested ```ts examples parse correctly, and tag the expected-commit-list block as ```text to satisfy MD040
2 participants
Summary
Wave 4 — three correctness fixes in the provider registry / aggregator plus Twilio pricing source-of-truth comments, bundled as one PR. Also marks Wave 3 done with its merged PR link.
{ hostPattern: "api.openai.com", provider: "openai-custom" }(nopathPrefix) was prepended unconditionally and silently disabled every built-in OpenAI path-specific rule. Fix: at construction, merge custom + built-in rules and stably sort by specificity descending — rules withpathPrefixbefore those without; longerpathPrefixfirst; exact host before*.wildcard; custom-wins-on-tie. The custom catch-all now only matches paths no built-in covers.match()returned the raw pathname asendpointCategoryfor host-only catch-alls, so every Twilio account SID became a unique aggregator bucket. Fix: catch-all returns the literal"other". Twilio's refiner still produces"sms"/"voice_calls"for known paths; its fallback also changed from raw pathname to"other".init.tscallsaggregator.wouldOverflow(event)and thenflushAndSend()(async) before falling through toaggregator.ingest(event). Events ingested between the hint and the resolved flush still landed in the full map. Fix:Aggregator.ingest()now synchronously redirects over-cap new-key events into a per-provider_overflowbucket. Counts / latencies / bytes / cost are still accumulated; onlyendpointCategorycardinality is bounded. NewoverflowCountgetter reports redirects, resets on flush.wouldOverflow()keeps its early-flush hint role.reviewed 2026-05-15notes above the three Twilio cost constants (0.79¢ SMS, 1.3¢ voice, 0.5¢ default).No public API additions beyond
Aggregator.overflowCount. No changes toinit.tsor any other file.Roadmap
First commit also flips Wave 3 to
donewith merged PR link (#35) and Wave 4 toin-progresswith this plan — leftover docs maintenance bundled per the Wave 3 -> Wave 4 handoff convention.Test plan
npm run lintcleannpm run buildemits ESM + CJSnpm run test— 253/253 vitest (baseline 241 + 12 new; includes the 7 dist smoke tests;test:distre-runs those 7 separately after build)list()"other"; Twilio/Messages.json->"sms"regression guard_overflow; existing bucket at cap still ingests; multiple over-cap events accumulate in one per-provider overflow bucket;overflowCountexposure + reset"other"; 2 custom-priority tests reframed to test equal-specificity / list-ordering invariants)Plan
See
docs/superpowers/plans/2026-05-15-provider-registry-overhaul.md.Closes #13, closes #21.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation