feat(media-use): derive provider cost tier from the registry - #3155
Merged
Conversation
miguel-heygen
force-pushed
the
feat/provider-tier-telemetry
branch
from
August 10, 2026 16:55
a637b96 to
f75a03a
Compare
The provider registry already declares whether a provider is local, free over the network, or paid over the network via its A/N/P constructors, but nothing downstream could read that, so anything needing to know whether resolving through a provider can spend the user's credit had to re-derive it by string-matching provider names. That gives one fact two owners and breaks quietly whenever a provider is renamed or added. Expose it as providerTierFor(name) over the same table, and carry the derived value on the resolve event alongside the provider it came from. Sparse, matching auth_method: absent when the record carries no provider or the name is not declared. A name declared under two media types must carry one tier, otherwise whether a resolve cost credit would depend on which type happened to serve it; the index throws at import instead of silently picking one.
miguel-heygen
force-pushed
the
feat/provider-tier-telemetry
branch
from
August 10, 2026 16:57
f75a03a to
c4670f1
Compare
The existing seam test only covered the negative case, an undeclared provider getting no tier, so nothing proved a declared one survives the whole path: registry lookup, result(), track(), JSON body. A unit call to providerTierFor does not cover that. Extract the interception harness and add a case per tier, each spawning the real CLI and reading the tier off the payload a local server actually receives.
The tier comes from the registry and the auth method from local credential state. Nothing stopped a future change from entangling them, and a machine with HeyGen credentials present would still have passed. Assert a non-heygen provider carries a tier and no auth method.
3 tasks
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
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.
What
A new
providerTierFor(name)export derives a provider's cost tier (local/network_free/network_paid) from the registry's existingA/N/Pdeclarations, and the resolve event carries the derived value asprovider_tieralongside the provider it came from.Why
The registry already decides whether a provider is local, free over the network, or paid over the network. That decision is what tells a caller whether resolving through a given provider can spend the user's credit. Nothing downstream could read it, so anything needing that distinction had to infer it by string-matching provider names, which gives one fact two owners and breaks quietly whenever a provider is renamed or added.
How
buildProviderTierIndex()folds the registry into a name-to-tier map and is exported so its conflict rule is testable against a fixture;providerTierFor()reads the map built from the real registry.A name declared under two media types must carry the same tier in both.
heygen.audio.soundsserves bothbgmandsfx, andheygen.asset.searchserves bothimageandicon. If a name could be free under one type and paid under another, whether a resolve cost credit would depend on which type happened to serve the request, so the index throws at import rather than silently picking one.The property is sparse, matching
auth_method: absent when the record carries no provider (cache and reuse hits) or the name is not declared. No provider, cascade, or resolve behavior changes. This is one added property on an event that already fires.Test plan
node skills/media-use/scripts/lib/registry.test.mjspasses 20/20, including five new cases: tier per declaration, cross-media-type agreement for every declared provider,undefinedfor undeclared and nullish names, a fixture with one name at two tiers throwing, and the same name repeated at one tier being accepted.node skills/media-use/scripts/resolve.test.mjsreports 45 passed / 1 failed; the same suite on an untouchedmaincheckout reports 42 passed / 1 failed, so the three added tests are the only difference and the one failure (smart grade merges measured adjust) is pre-existing and unrelated.Those three are end-to-end rather than unit: each spawns the real resolve CLI as its own process, lets
telemetry.mjsbuild the request, and reads the tier off the payload a local HTTP server actually receives, one case per tier (network_paid,network_free,local). The pre-existing seam test, which covers an undeclared provider getting no tier, now shares the same harness.Two paths a local suite cannot reach were exercised by hand against the real service:
provider_tier: network_paidandauth_method: oauthtogether, which is the combination no automated case can produce without credentials.bunx oxlintandbunx oxfmtare clean on all changed files. Repository CI reports every check as skipped for this branch, since the workflows are path-filtered and this PR touches onlyskills/**.