mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-14 02:42:24 +03:00
fix(usage): declare the Adobe Firefly usage fetcher the dispatcher already calls (#12321)
usage/fetcherProviders.ts exists, in its own words, "so the registration list can't drift from the dispatcher's switch statement". It drifted: #8006 added adobe-firefly and firefly to the dispatcher and to USAGE_SUPPORTED_PROVIDERS but not to this list, so the connection UI advertised usage support while the provider-plugin manifest, genericQuotaFetcher and the free-access quota cache all reported no fetcher — for two ids getUsageForProvider would happily serve. Declaring them is what makes the balance actually get fetched (registerGenericQuotaFetchers wires a generic fetcher per declared id, and resolveFreeAccessState stops returning early), which the PR states plainly rather than burying as a side effect. The test turns the docstring's prose invariant into enforcement: it reads the dispatcher's cases from source and compares both directions, and records each accepted difference against USAGE_SUPPORTED_PROVIDERS with a reason plus a staleness check, so the next drift can't hide among them. xiaomi-mimo-token-plan is left flagged as a real gap rather than widening the PR. Verified in a combined batch worktree: 174/174 focused tests across all 11 PRs of this batch, typecheck:core clean, complexity 2706/3218, cognitive-complexity 1221/1437, check:cycles and check:docs-counts green. Thanks @maxmad64bis.
This commit is contained in:
1
changelog.d/fixes/12321-usage-fetcher-registration.md
Normal file
1
changelog.d/fixes/12321-usage-fetcher-registration.md
Normal file
@@ -0,0 +1 @@
|
||||
- **fix(usage):** `adobe-firefly` and `firefly` have had a working usage fetcher since Adobe Firefly landed, but neither was ever added to the registration list, so the provider-plugin manifest, `genericQuotaFetcher` and the free-access quota cache all reported them as having no usage support — while `USAGE_SUPPORTED_PROVIDERS` said the opposite. Both are now declared, which also means their credit balance is fetched like any other declared provider's: `registerGenericQuotaFetchers` now registers a generic quota fetcher for them, and `resolveFreeAccessState` no longer returns early. A test holds the registration list to the dispatcher's switch in both directions, which is what the module's own docstring already asked for in prose ([#12321](https://github.com/diegosouzapw/OmniRoute/pull/12321))
|
||||
@@ -59,6 +59,14 @@ export const USAGE_FETCHER_PROVIDERS = [
|
||||
// PromptQL playground credits (data.pro.ql.app getCreditSummary)
|
||||
"promptql",
|
||||
"pql",
|
||||
// Adobe Firefly credit balance (GET firefly.adobe.io/v1/credits/balance).
|
||||
// Dispatched since #8006; the registration list was missed at the time, so the
|
||||
// manifest and the quota fetchers reported no usage support for either id.
|
||||
// Declaring them here is what makes `registerGenericQuotaFetchers` wire a
|
||||
// generic fetcher for them and `resolveFreeAccessState` stop returning early —
|
||||
// i.e. the balance is now actually fetched, which is the point.
|
||||
"adobe-firefly",
|
||||
"firefly",
|
||||
// HyperAgent billing usage (creditBlocks USD)
|
||||
"hyperagent",
|
||||
"ha",
|
||||
|
||||
134
tests/unit/usage-fetcher-registration-coverage.test.ts
Normal file
134
tests/unit/usage-fetcher-registration-coverage.test.ts
Normal file
@@ -0,0 +1,134 @@
|
||||
/**
|
||||
* `usage/fetcherProviders.ts` says of itself that it exists "so the registration
|
||||
* list can't drift from the dispatcher's switch statement", and asks whoever adds
|
||||
* a case to remember to add it here too. Nothing enforced that, and it drifted:
|
||||
* #8006 added `adobe-firefly`/`firefly` to the switch and to
|
||||
* `USAGE_SUPPORTED_PROVIDERS` but not to this list, so three consumers
|
||||
* (`genericQuotaFetcher.ts`, `freeAccessQuota.ts`, the provider-plugin manifest)
|
||||
* were told those providers had no usage fetcher when in fact they do.
|
||||
*
|
||||
* This test is the enforcement the comment asked for.
|
||||
*/
|
||||
import assert from "node:assert/strict";
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
import test from "node:test";
|
||||
import { fileURLToPath } from "node:url";
|
||||
|
||||
import { USAGE_FETCHER_PROVIDERS } from "../../open-sse/services/usage/fetcherProviders.ts";
|
||||
import { USAGE_SUPPORTED_PROVIDERS } from "../../src/shared/constants/providers.ts";
|
||||
|
||||
const here = path.dirname(fileURLToPath(import.meta.url));
|
||||
|
||||
/**
|
||||
* The provider ids `getUsageForProvider` actually dispatches on. Read from the
|
||||
* source rather than by calling the function: importing the dispatcher pulls in
|
||||
* its whole fetcher graph (DB, sockets, child_process), which is precisely the
|
||||
* weight `fetcherProviders.ts` was extracted to avoid.
|
||||
*/
|
||||
function dispatchedProviderIds(): string[] {
|
||||
const source = fs.readFileSync(path.resolve(here, "../../open-sse/services/usage.ts"), "utf8");
|
||||
|
||||
// Anchor on the dispatcher itself, not on the first occurrence of the word
|
||||
// "switch" -- that one is in a doc comment twenty lines above it. Then stop at
|
||||
// the closing brace of that switch, so a second switch added later in the file
|
||||
// cannot contribute cases to an invariant that is only about this one.
|
||||
const start = source.indexOf("switch (provider) {");
|
||||
assert.notEqual(start, -1, "could not find `switch (provider) {` in services/usage.ts");
|
||||
assert.equal(
|
||||
source.indexOf("switch (provider) {", start + 1),
|
||||
-1,
|
||||
"more than one `switch (provider)` — this reader would merge them"
|
||||
);
|
||||
|
||||
let depth = 0;
|
||||
let end = start;
|
||||
for (let i = source.indexOf("{", start); i < source.length; i += 1) {
|
||||
if (source[i] === "{") depth += 1;
|
||||
else if (source[i] === "}") {
|
||||
depth -= 1;
|
||||
if (depth === 0) {
|
||||
end = i;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
assert.ok(end > start, "unbalanced braces in the dispatcher switch");
|
||||
|
||||
const body = source.slice(start, end);
|
||||
const ids = [...body.matchAll(/^\s*case ["']([^"']+)["']:/gm)].map((match) => match[1]);
|
||||
assert.ok(ids.length > 0, "parsed no cases out of the dispatcher — the reader is broken");
|
||||
return [...new Set(ids)];
|
||||
}
|
||||
|
||||
test("every provider the dispatcher handles is declared as having a fetcher", () => {
|
||||
const undeclared = dispatchedProviderIds().filter(
|
||||
(id) => !(USAGE_FETCHER_PROVIDERS as readonly string[]).includes(id)
|
||||
);
|
||||
assert.deepEqual(
|
||||
undeclared,
|
||||
[],
|
||||
"these providers have a usage fetcher the dispatcher will happily call, but nothing " +
|
||||
"declares it — so genericQuotaFetcher, freeAccessQuota and the plugin manifest all " +
|
||||
"report them as having no usage support: " +
|
||||
undeclared.join(", ")
|
||||
);
|
||||
});
|
||||
|
||||
test("nothing is declared that the dispatcher would not handle", () => {
|
||||
const dispatched = dispatchedProviderIds();
|
||||
const phantom = (USAGE_FETCHER_PROVIDERS as readonly string[]).filter(
|
||||
(id) => !dispatched.includes(id)
|
||||
);
|
||||
assert.deepEqual(
|
||||
phantom,
|
||||
[],
|
||||
`declared but unreachable, so a quota fetch would silently return nothing: ${phantom.join(", ")}`
|
||||
);
|
||||
});
|
||||
|
||||
/**
|
||||
* The fetcher list and the "usage supported" connection list are not the same
|
||||
* set, and should not be forced to converge — but every difference needs a
|
||||
* reason on record, or the next drift hides among the ones we accepted.
|
||||
*/
|
||||
const ACCEPTED_DIVERGENCE: Record<string, string> = {
|
||||
// Aggregators: a usage fetcher exists, but a connection to them is not itself
|
||||
// presented as a usage-reporting account in the UI.
|
||||
opencode: "aggregator — fetcher exists, not surfaced as a usage-reporting connection",
|
||||
"opencode-zen": "aggregator — same as opencode",
|
||||
xai: "reached through xai-oauth for connection purposes",
|
||||
openrouter: "aggregator — fetcher exists, not surfaced as a usage-reporting connection",
|
||||
// Declared supported, no fetcher: a real gap, left alone here on purpose so
|
||||
// this PR stays about the two providers whose fetcher already exists.
|
||||
"xiaomi-mimo-token-plan": "declared supported with no fetcher — open question, not fixed here",
|
||||
};
|
||||
|
||||
test("every difference between the two lists is one we have written down", () => {
|
||||
const fetcher = new Set<string>(USAGE_FETCHER_PROVIDERS as readonly string[]);
|
||||
const supported = new Set<string>(USAGE_SUPPORTED_PROVIDERS as readonly string[]);
|
||||
const unexplained = [
|
||||
...[...fetcher].filter((id) => !supported.has(id)),
|
||||
...[...supported].filter((id) => !fetcher.has(id)),
|
||||
].filter((id) => !(id in ACCEPTED_DIVERGENCE));
|
||||
|
||||
assert.deepEqual(
|
||||
unexplained,
|
||||
[],
|
||||
"the two lists differ here for no recorded reason — either wire it up, or add it to " +
|
||||
`ACCEPTED_DIVERGENCE with why: ${unexplained.join(", ")}`
|
||||
);
|
||||
});
|
||||
|
||||
test("the recorded divergences are still real, so the list does not rot", () => {
|
||||
const fetcher = new Set<string>(USAGE_FETCHER_PROVIDERS as readonly string[]);
|
||||
const supported = new Set<string>(USAGE_SUPPORTED_PROVIDERS as readonly string[]);
|
||||
const stale = Object.keys(ACCEPTED_DIVERGENCE).filter(
|
||||
(id) => fetcher.has(id) === supported.has(id)
|
||||
);
|
||||
assert.deepEqual(
|
||||
stale,
|
||||
[],
|
||||
`these no longer differ; drop them from ACCEPTED_DIVERGENCE: ${stale.join(", ")}`
|
||||
);
|
||||
});
|
||||
Reference in New Issue
Block a user