A7: URL-path method fallback for raw fetch (closes #79) - #104
Merged
Conversation
…tching (A7 step 1)
Wire estimateLocalMonthlyCost to accept an optional URL and try lookupByUrlPath when the SDK method chain is undefined. Updates all three production callers (scan-results, scan-publishing-handler, compression) to pass the call's URL through. Adds an integration test asserting that a raw fetch to api.elevenlabs.io/v1/text-to-speech/... now resolves to a non-zero per-request cost via the fingerprint. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
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 (5)
📝 WalkthroughWalkthroughThis PR implements URL-path method fingerprint fallback for cost estimation. When provider is known but methodSignature is undefined (raw fetch), the system now matches the request URL against provider-specific URL-path keys to resolve the cost model. This enables accurate cost tracking for raw fetch calls to known API providers like ElevenLabs. ChangesURL-path cost estimation fallback
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 6
🤖 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 `@src/intelligence/compression.ts`:
- Around line 362-367: The issue: provider is chosen as context.providers[0] but
methodSig and url are always pulled from context.apiCalls[0], which can mix
providers; fix by finding the first ApiCall in context.apiCalls whose providerId
(or equivalent provider identifier) matches the selected provider (provider.id
or provider.name) and use that call to derive methodSig and url (fall back to
context.apiCalls[0] only if no matching call exists); update the same selection
logic where callsPerDay or other call-derived values are computed (e.g., the
later block around lines ~373) so all call samples are aligned with the chosen
provider variable (provider) rather than unconditionally using
context.apiCalls[0].
In `@src/scan-results.ts`:
- Line 414: monthlyCost is computed once using
estimateLocalMonthlyCost(provider, callsPerDay, call.methodSignature, call.url)
but provider, url, callsPerDay and call.methodSignature are mutated later when
merging synthetic endpoints, leaving stale costs; after the code that
merges/overwrites provider/url/callsPerDay/methodSignature (the synthetic
endpoint merge block), recompute monthlyCost by calling
estimateLocalMonthlyCost(provider, callsPerDay, call.methodSignature, call.url)
(falling back to 0) and assign that value into the result object/property that
originally used monthlyCost so merged synthetic endpoints have correct costs.
In `@src/scanner/fingerprints/registry.ts`:
- Around line 184-186: The current check in registry.ts that returns an entry
when pathAndQuery.includes(entry.urlPathKey) can misclassify when urlPathKey
appears inside other path tokens; update the matching in the function that
iterates fingerprint entries to be segment-aware: normalize the request path
(trim/normalize trailing slashes, decode, lowercase if applicable), split into
path segments and match entry.urlPathKey against full segments or match using
boundary-aware logic (e.g., ensure urlPathKey matches at segment boundaries or
as a normalized prefix) rather than raw substring includes; reference the code
that performs this check (the block using
pathAndQuery.includes(entry.urlPathKey) in the registry lookup) and replace it
with the boundary-aware matching described.
In `@src/scanner/fingerprints/types.ts`:
- Around line 12-23: The MethodFingerprint type currently allows neither pattern
nor urlPathKey to be set; change its definition in
src/scanner/fingerprints/types.ts to encode the contract as a discriminated
union so each entry must have at least one of the two fields (e.g. a union of a
shape with required pattern and optional urlPathKey and a shape with required
urlPathKey and optional pattern), preserve any other existing optional
properties on MethodFingerprint, and update any call sites or constructors that
create MethodFingerprint objects to satisfy the new union (adjust tests/fixtures
as needed).
In `@src/test/fixtures/a7/raw-elevenlabs-fetch.ts`:
- Around line 2-5: The fetch call that assigns to r posts a JSON body but omits
the Content-Type header; update the request options in the fetch(...) invocation
(the POST to "https://api.elevenlabs.io/v1/text-to-speech/voice-abc/stream") to
include headers: { "Content-Type": "application/json" } so the ElevenLabs API
correctly parses the JSON payload (ensure you add or merge into any existing
headers object in that same fetch call).
In `@src/webview/scan-publishing-handler.ts`:
- Line 476: monthlyCost is computed once using
estimateLocalMonthlyCost(provider, callsPerDay, call.methodSignature, call.url)
but later code mutates the fields (provider, callsPerDay, call.methodSignature,
call.url) during the merge/merge-update logic, leaving a stale monthlyCost;
after the merge/update block that modifies those fields (the code that updates
merged endpoints between the initial assignment and the later merging logic),
recompute monthlyCost by calling estimateLocalMonthlyCost again with the updated
provider, callsPerDay, call.methodSignature and call.url and replace the
original value so merged endpoints reflect the correct synthetic cost.
🪄 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: 0cc6c1a0-4bda-474c-b399-bb8b33493193
📒 Files selected for processing (11)
package.jsonsrc/intelligence/compression.tssrc/intelligence/cost-utils.tssrc/scan-results.tssrc/scanner/fingerprints/elevenlabs.jsonsrc/scanner/fingerprints/registry.tssrc/scanner/fingerprints/types.tssrc/test/a7-url-path-fallback.test.tssrc/test/fingerprint-registry.test.tssrc/test/fixtures/a7/raw-elevenlabs-fetch.tssrc/webview/scan-publishing-handler.ts
Comment on lines
+12
to
+23
| /** | ||
| * SDK method chain pattern, e.g. "chat.completions.create". | ||
| * Either `pattern` or `urlPathKey` must be set on every entry. | ||
| */ | ||
| pattern?: string; | ||
| /** | ||
| * URL-path substring used by `lookupByUrlPath` when an API call has a known | ||
| * provider but no SDK method chain (e.g. raw `fetch(...)`). The matcher tries | ||
| * the longest `urlPathKey` first; the special value `"_default"` is a | ||
| * provider-wide fallback (A7, issue #79). | ||
| */ | ||
| urlPathKey?: string; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Encode the pattern/urlPathKey requirement in the type, not only in comments.
MethodFingerprint currently allows entries with neither field set, despite Line 14’s contract. Enforcing this at type level prevents invalid fingerprint records from compiling.
Proposed type-safe shape
-export interface MethodFingerprint {
+type MethodFingerprintKey =
+ | { pattern: string; urlPathKey?: string }
+ | { pattern?: string; urlPathKey: string };
+
+export type MethodFingerprint = MethodFingerprintKey & {
/**
* SDK method chain pattern, e.g. "chat.completions.create".
* Either `pattern` or `urlPathKey` must be set on every entry.
*/
- pattern?: string;
+ pattern?: string;
/**
* URL-path substring used by `lookupByUrlPath` when an API call has a known
* provider but no SDK method chain (e.g. raw `fetch(...)`). The matcher tries
* the longest `urlPathKey` first; the special value `"_default"` is a
* provider-wide fallback (A7, issue `#79`).
*/
urlPathKey?: string;
/** HTTP verb: GET | POST | PUT | PATCH | DELETE | SUBSCRIBE | RPC */
httpMethod: string;
/** Full URL or URL template for the mapped endpoint */
endpoint: string;
costModel: CostModel;
...
-}
+};🤖 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/scanner/fingerprints/types.ts` around lines 12 - 23, The
MethodFingerprint type currently allows neither pattern nor urlPathKey to be
set; change its definition in src/scanner/fingerprints/types.ts to encode the
contract as a discriminated union so each entry must have at least one of the
two fields (e.g. a union of a shape with required pattern and optional
urlPathKey and a shape with required urlPathKey and optional pattern), preserve
any other existing optional properties on MethodFingerprint, and update any call
sites or constructors that create MethodFingerprint objects to satisfy the new
union (adjust tests/fixtures as needed).
Contributor
Author
There was a problem hiding this comment.
Skipping. The runtime validator in registry.ts already enforces pattern || urlPathKey at JSON load time, and MethodFingerprint instances are only constructed by JSON load (no in-code constructors). A discriminated union would force type-narrowing at every read of m.pattern (which happens many places) for a constraint already covered at the single entry point. The ergonomic cost exceeds the value of compile-time enforcement here.
There was a problem hiding this comment.
@AndresL230, understood — that's a reasonable trade-off. Centralising the invariant at the single JSON-load entry point via the runtime validator avoids scattered type-narrowing across all m.pattern read sites, and the ergonomic cost of a discriminated union would be real given the number of call sites.
✏️ Learnings added
Learnt from: AndresL230
Repo: recost-dev/extension PR: 104
File: src/scanner/fingerprints/types.ts:12-23
Timestamp: 2026-05-13T20:11:35.796Z
Learning: In `src/scanner/fingerprints/types.ts`, `MethodFingerprint` deliberately keeps `pattern` and `urlPathKey` as plain optional fields rather than a discriminated union. The `pattern || urlPathKey` invariant is enforced at the single entry point (JSON load) by the runtime validator in `src/scanner/fingerprints/registry.ts`. All `MethodFingerprint` instances are constructed only via JSON load with no in-code constructors, so compile-time union enforcement was judged to have too high an ergonomic cost (type-narrowing at every `m.pattern` read site) relative to the benefit.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
… recompute, mixed-provider sample, doc, dead fixture Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 task
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
1 participant
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.
Summary
urlPathKeyfield to fingerprint methods schema (src/scanner/fingerprints/types.ts).lookupByUrlPath(provider, url)to the fingerprint registry — matches longest URL-path key first, falls back to_default.elevenlabs.jsonwith 4 URL-path entries (text-to-speech, speech-to-text, voices, _default).cost-utils.tsestimateLocalMonthlyCostto accept optionalurland try URL-path lookup whenmethodSignatureis undefined.call.urlthrough the 3 production callers (scan-results.ts,scan-publishing-handler.ts,compression.ts).fetch("https://api.elevenlabs.io/v1/text-to-speech/...")resolves to non-stub cost.D1 measurement
No movement — the D1 corpus does not contain a raw-fetch to ElevenLabs (or any of the URL-path providers added in this PR). The in-repo integration test proves the fix works end-to-end; corpus expansion is a follow-up.
Follow-ups
_defaultperRequestCostUsdis conservative ($0.0001); revisit per provider with real pricing data.Test plan
benchmarkpasses (no metric drops > 1pp from baseline)test:scannerpasses (8 new a7 PASS lines)fetch(\"https://api.elevenlabs.io/v1/text-to-speech/...\")— endpoint resolves to elevenlabs with a non-stub costCloses #79.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests