feat: canonical numeric helpers + tier-1 (analytics) migration (#7879) (#7969)

This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-07-22 02:35:08 -03:00
committed by GitHub
parent 992fe98386
commit 5dd3c76ad7
12 changed files with 478 additions and 53 deletions

View File

@@ -0,0 +1 @@
- **feat(shared):** Add canonical numeric coercion helpers (`toNumber`, `toNumberOrNull`, `toNumberArray`) in `src/shared/utils/numeric.ts` with table-driven tests and a lint rule barring new local `toNumber` definitions, then migrate the report/analytics tier (usage analytics route, provider metrics route, usage stats, usage history helpers, call-log formatting, cost-analysis A2A skill) off their local duplicates ([#7879](https://github.com/diegosouzapw/OmniRoute/issues/7879))

View File

@@ -74,11 +74,26 @@
"count": 1
}
},
"open-sse/handlers/responseSanitizer.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"open-sse/handlers/responseTranslator.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"open-sse/handlers/search.ts": {
"@typescript-eslint/no-explicit-any": {
"count": 34
}
},
"open-sse/handlers/sseParser.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"open-sse/handlers/videoGeneration.ts": {
"@typescript-eslint/no-explicit-any": {
"count": 1
@@ -104,9 +119,22 @@
"count": 2
}
},
"open-sse/mcp-server/audit.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"open-sse/mcp-server/server.ts": {
"@typescript-eslint/no-explicit-any": {
"count": 3
},
"no-restricted-syntax": {
"count": 1
}
},
"open-sse/mcp-server/tools/advancedTools.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"open-sse/mcp-server/tools/gamificationTools.ts": {
@@ -114,14 +142,32 @@
"count": 2
}
},
"open-sse/mcp-server/tools/pickFastestModel.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"open-sse/mcp-server/tools/pluginTools.ts": {
"@typescript-eslint/no-explicit-any": {
"count": 1
}
},
"open-sse/services/agentrouterQuotaFetcher.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"open-sse/services/bailianQuotaFetcher.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"open-sse/services/batchProcessor.ts": {
"@typescript-eslint/no-explicit-any": {
"count": 17
},
"no-restricted-syntax": {
"count": 1
}
},
"open-sse/services/claudeWebAutoRefresh.ts": {
@@ -129,6 +175,16 @@
"count": 3
}
},
"open-sse/services/codexQuotaFetcher.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"open-sse/services/codexUsageQuotas.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"open-sse/services/compression/engines/headroom/gcf/decode_generic.ts": {
"@typescript-eslint/no-explicit-any": {
"count": 22
@@ -149,11 +205,41 @@
"count": 1
}
},
"open-sse/services/crofUsageFetcher.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"open-sse/services/deepseekQuotaFetcher.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"open-sse/services/genericQuotaFetcher.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"open-sse/services/inAppLoginService.ts": {
"@typescript-eslint/no-explicit-any": {
"count": 2
}
},
"open-sse/services/opencodeOllamaUsage.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"open-sse/services/opencodeQuotaFetcher.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"open-sse/services/rateLimitManager.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"open-sse/services/taskAwareRouter.ts": {
"@typescript-eslint/no-explicit-any": {
"count": 4
@@ -169,11 +255,26 @@
"count": 2
}
},
"open-sse/services/usage/scalars.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"open-sse/services/v0QuotaFetcher.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"open-sse/utils/setupPolyfill.ts": {
"@typescript-eslint/no-explicit-any": {
"count": 5
}
},
"open-sse/utils/streamPayloadCollector.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"src/app/(dashboard)/dashboard/HomePageClient.tsx": {
"react-hooks/exhaustive-deps": {
"count": 1
@@ -234,11 +335,96 @@
"count": 1
}
},
"src/domain/costRules.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"src/hooks/useLiveDashboard.ts": {
"react-hooks/exhaustive-deps": {
"count": 2
}
},
"src/lib/a2a/skills/healthReport.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"src/lib/combos/controlCenter.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"src/lib/db/comboForecast.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"src/lib/db/domainState.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"src/lib/db/prompts.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"src/lib/db/providers/lazyConnectionView.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"src/lib/db/tokenLimits.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"src/lib/monitoring/providerHealthAutopilot.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"src/lib/monitoring/providerHealthMatrix.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"src/lib/semanticCache.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"src/lib/usage/apiKeySelfService.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"src/lib/usage/apiKeyUsageLimits.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"src/lib/usage/costCalculator.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"src/lib/usage/internalUsageCommand.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"src/lib/usage/providerWindowCosts.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"src/lib/usage/routeExplain.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"src/shared/components/CursorAuthModal.tsx": {
"react-hooks/exhaustive-deps": {
"count": 1
@@ -269,6 +455,16 @@
"count": 1
}
},
"src/shared/contracts/quota.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"src/sse/services/auth.ts": {
"no-restricted-syntax": {
"count": 1
}
},
"tests/e2e/api.spec.ts": {
"@typescript-eslint/no-explicit-any": {
"count": 3

View File

@@ -1,6 +1,18 @@
import nextVitals from "eslint-config-next/core-web-vitals";
import tseslint from "typescript-eslint";
// #7879: bar NEW local `toNumber` definitions outside the canonical helper.
// Pre-existing definitions (~51 across the codebase) are frozen via
// config/quality/eslint-suppressions.json and migrated tier-by-tier; only a
// genuinely NEW `function toNumber`/`const toNumber = ...` should fail.
const TO_NUMBER_RESTRICTION = {
selector: "FunctionDeclaration[id.name='toNumber'], VariableDeclarator[id.name='toNumber']",
message:
"New local `toNumber` definitions are barred — import `toNumber` from " +
"`@/shared/utils/numeric` instead (#7879). See that module's JSDoc for the " +
"canonical coercion shape and the `toNumberOrNull`/`toNumberArray` variants.",
};
/** @type {import("eslint").Linter.Config[]} */
const eslintConfig = [
...nextVitals,
@@ -56,9 +68,28 @@ const eslintConfig = [
message:
"Türkçe-güvenli arama için matchesSearch() kullan (@/shared/utils/turkishText). Ham toLowerCase().includes() İ/ı karakterlerini bozar.",
},
TO_NUMBER_RESTRICTION,
],
},
},
// #7879: same toNumber restriction for the rest of src/ and open-sse/ — kept
// as a separate block (via `ignores`) so it does not clobber the
// app/components-scoped rule array above (flat config replaces a rule's
// options entirely per matching file, it does not merge arrays).
{
files: ["src/**/*.ts", "open-sse/**/*.ts"],
ignores: ["src/app/**", "src/components/**"],
rules: {
"no-restricted-syntax": ["error", TO_NUMBER_RESTRICTION],
},
},
// Canonical helper module itself is exempt from its own restriction.
{
files: ["src/shared/utils/numeric.ts"],
rules: {
"no-restricted-syntax": "off",
},
},
// Relaxed rules for open-sse and tests (incremental adoption)
{
files: ["open-sse/**/*.ts", "tests/**/*.mjs", "tests/**/*.ts"],

View File

@@ -4,18 +4,10 @@ import pino from "pino";
import { buildErrorBody } from "@omniroute/open-sse/utils/error.ts";
import { getProviderMetrics } from "@/lib/db/callLogStats";
import { toNumber } from "@/shared/utils/numeric";
const logger = pino({ name: "provider-metrics-api" });
function toNumber(value: unknown): number {
if (typeof value === "number" && Number.isFinite(value)) return value;
if (typeof value === "string" && value.trim().length > 0) {
const parsed = Number(value);
return Number.isFinite(parsed) ? parsed : 0;
}
return 0;
}
/**
* GET /api/provider-metrics — Aggregate per-provider stats from call_logs
* Returns aggregate metrics plus topology recency/error hints for dashboard visualization.

View File

@@ -23,6 +23,7 @@ import {
} from "@/lib/db/usageAnalytics";
import { getFallbackStats } from "@/lib/db/callLogStats";
import { buildByProviderRows } from "@/lib/usage/providerDisplayNames";
import { toNumber } from "@/shared/utils/numeric";
function getRangeStartIso(range: string): string | null {
const end = new Date();
@@ -74,15 +75,6 @@ type GetCodexFastCostMultiplier = (
serviceTier: string | null | undefined
) => number;
function toNumber(value: unknown): number {
if (typeof value === "number" && Number.isFinite(value)) return value;
if (typeof value === "string" && value.trim().length > 0) {
const parsed = Number(value);
return Number.isFinite(parsed) ? parsed : 0;
}
return 0;
}
function toStringValue(value: unknown, fallback = ""): string {
return typeof value === "string" && value.trim().length > 0 ? value : fallback;
}

View File

@@ -7,6 +7,7 @@
import type { A2ATask, TaskArtifact } from "../taskManager";
import { resolveOmniRouteBaseUrl } from "@/shared/utils/resolveOmniRouteBaseUrl";
import { formatCost } from "@/shared/utils/formatting";
import { toNumber } from "@/shared/utils/numeric";
type AnalyticsRecord = Record<string, unknown>;
@@ -45,14 +46,6 @@ async function costFetch(path: string): Promise<AnalyticsRecord> {
return response.json();
}
function toNumber(value: unknown): number {
if (typeof value === "number" && Number.isFinite(value)) return value;
if (typeof value === "string" && value.trim()) {
const parsed = Number(value);
return Number.isFinite(parsed) ? parsed : 0;
}
return 0;
}
function toCostEntries(value: unknown): CostEntry[] {
if (!value || typeof value !== "object" || Array.isArray(value)) return [];

View File

@@ -2,6 +2,9 @@ import type { RequestPipelinePayloads } from "@omniroute/open-sse/utils/requestL
import { sanitizePII } from "../../piiSanitizer";
import { protectPayloadForLog } from "../../logPayloads";
import type { CallLogDetailState } from "../callLogArtifacts";
// #7879: re-export the canonical helper so existing consumers of this module
// keep importing `toNumber` from here unchanged.
export { toNumber } from "@/shared/utils/numeric";
type JsonRecord = Record<string, unknown>;
@@ -9,15 +12,6 @@ export function asRecord(value: unknown): JsonRecord {
return value && typeof value === "object" && !Array.isArray(value) ? (value as JsonRecord) : {};
}
export function toNumber(value: unknown): number {
if (typeof value === "number" && Number.isFinite(value)) return value;
if (typeof value === "string" && value.trim().length > 0) {
const parsed = Number(value);
return Number.isFinite(parsed) ? parsed : 0;
}
return 0;
}
export function toStringOrNull(value: unknown): string | null {
return typeof value === "string" && value.trim().length > 0 ? value : null;
}

View File

@@ -3,6 +3,10 @@
* No DB access, no module-level state — safe to import anywhere.
*/
// #7879: re-export the canonical helper so existing consumers of this module
// keep importing `toNumber` from here unchanged.
export { toNumber } from "@/shared/utils/numeric";
type JsonRecord = Record<string, unknown>;
export function asRecord(value: unknown): JsonRecord {
@@ -20,15 +24,6 @@ export function normalizeServiceTier(value: unknown): string {
return "standard";
}
export function toNumber(value: unknown): number {
if (typeof value === "number" && Number.isFinite(value)) return value;
if (typeof value === "string" && value.trim().length > 0) {
const parsed = Number(value);
return Number.isFinite(parsed) ? parsed : 0;
}
return 0;
}
export function percentile(sortedValues: number[], p: number): number {
if (sortedValues.length === 0) return 0;
if (sortedValues.length === 1) return sortedValues[0];

View File

@@ -13,6 +13,7 @@ import { getPendingRequests } from "./usageHistory";
import { getAccountDisplayName } from "@/lib/display/names";
import { calculateCost } from "./costCalculator";
import { getRawDataCutoffDate, isAggregationEnabled } from "./aggregateHistory";
import { toNumber } from "@/shared/utils/numeric";
type JsonRecord = Record<string, unknown>;
type UsageBucket = {
@@ -44,15 +45,6 @@ function asRecord(value: unknown): JsonRecord {
return value && typeof value === "object" && !Array.isArray(value) ? (value as JsonRecord) : {};
}
function toNumber(value: unknown): number {
if (typeof value === "number" && Number.isFinite(value)) return value;
if (typeof value === "string" && value.trim().length > 0) {
const parsed = Number(value);
return Number.isFinite(parsed) ? parsed : 0;
}
return 0;
}
function toStringOrEmpty(value: unknown): string {
return typeof value === "string" ? value : "";
}

View File

@@ -0,0 +1,70 @@
/**
* Canonical numeric coercion helpers — DRY extraction from ~51 near-identical
* local `toNumber` definitions scattered across `src/` and `open-sse/` (#7879).
*
* All three variants share the SAME strict coercion shape as the dominant
* pre-existing pattern found across the codebase:
* - `number` inputs pass through only when `Number.isFinite`.
* - `string` inputs are `trim()`-med first; empty/whitespace-only strings
* are treated as absent. The trimmed string is coerced with `Number(...)`
* and accepted only when the result is finite (rejects `"12abc"`,
* `"Infinity"`, `"NaN"`, etc).
* - Every other type (`null`, `undefined`, `boolean`, `object`, `array`, ...)
* is treated as absent.
*
* This is intentionally the STRICT variant — it does NOT use `parseFloat`
* (which would accept `"12abc"` -> `12`). A small number of call sites in the
* codebase intentionally keep `parseFloat` (leniency is a documented,
* deliberate behavior choice there, not a bug) — see
* `open-sse/services/crofUsageFetcher.ts` for the annotated exception.
*
* Migration is happening tier-by-tier (report/analytics first, then
* quota/billing, then hot-path auth/costRules/combo) to avoid silently
* changing fallback semantics anywhere cost or quota math depends on it.
* See the issue for the full plan.
*/
/**
* Coerce an unknown value to a finite number, or return `fallback` (default
* `0`) when the value cannot be strictly coerced.
*
* @param v - the value to coerce.
* @param fallback - value returned when coercion fails (default `0`).
*/
export function toNumber(v: unknown, fallback = 0): number {
if (typeof v === "number" && Number.isFinite(v)) return v;
if (typeof v === "string" && v.trim().length > 0) {
const parsed = Number(v.trim());
return Number.isFinite(parsed) ? parsed : fallback;
}
return fallback;
}
/**
* Coerce an unknown value to a finite number, or `null` when the value
* cannot be strictly coerced. Same coercion shape as {@link toNumber}, but
* with a `null` fallback instead of `0` — useful where "absent" must stay
* distinguishable from "zero" downstream (e.g. optional metrics).
*/
export function toNumberOrNull(v: unknown): number | null {
if (typeof v === "number" && Number.isFinite(v)) return v;
if (typeof v === "string" && v.trim().length > 0) {
const parsed = Number(v.trim());
return Number.isFinite(parsed) ? parsed : null;
}
return null;
}
/**
* Coerce an unknown value to an array of finite numbers.
*
* - Non-array inputs return `fallback` (default `[]`) unchanged.
* - Each array element is coerced independently via {@link toNumber}; an
* element that fails to coerce becomes `0` (NOT the array-level
* `fallback` — the two fallbacks are intentionally independent so a
* caller can distinguish "no array at all" from "one bad element").
*/
export function toNumberArray(v: unknown, fallback: number[] = []): number[] {
if (!Array.isArray(v)) return fallback;
return v.map((item) => toNumber(item, 0));
}

View File

@@ -0,0 +1,80 @@
import assert from "node:assert/strict";
import { test } from "node:test";
import { executeCostAnalysis } from "../../src/lib/a2a/skills/costAnalysis.ts";
import type { A2ATask } from "../../src/lib/a2a/taskManager.ts";
// #7879: the cost-analysis A2A skill migrated its local `toNumber` to the
// canonical `@/shared/utils/numeric` helper. This test proves the 0-fallback
// semantics for missing/non-numeric analytics fields still hold after the
// migration (the whole point of the tier-1 move).
function buildTask(): A2ATask {
const now = new Date().toISOString();
return {
id: "test-task",
skill: "cost-analysis",
state: "working",
input: { skill: "cost-analysis", messages: [{ role: "user", content: "cost report" }] },
artifacts: [],
events: [],
metadata: {},
createdAt: now,
updatedAt: now,
expiresAt: now,
};
}
test("executeCostAnalysis: missing/non-numeric summary fields fall back to 0", async (t) => {
const originalFetch = globalThis.fetch;
t.after(() => {
globalThis.fetch = originalFetch;
});
globalThis.fetch = (async () =>
new Response(
JSON.stringify({
summary: {
totalCost: "abc", // non-numeric -> 0
// totalRequests missing entirely -> 0
fallbackRatePct: null, // -> 0
},
byProvider: {},
byModel: {},
}),
{ status: 200, headers: { "Content-Type": "application/json" } }
)) as typeof fetch;
const result = await executeCostAnalysis(buildTask());
assert.equal(result.metadata.totalCost, 0);
assert.equal(result.metadata.totalRequests, 0);
assert.equal(result.metadata.providerCosts.length, 0);
assert.equal(result.metadata.modelCosts.length, 0);
});
test("executeCostAnalysis: numeric-string summary fields coerce correctly", async (t) => {
const originalFetch = globalThis.fetch;
t.after(() => {
globalThis.fetch = originalFetch;
});
globalThis.fetch = (async () =>
new Response(
JSON.stringify({
summary: { totalCost: "12.5", totalRequests: "42", fallbackRatePct: "3.2" },
byProvider: {
openai: { cost: "1.5", requests: "3", tokens: "100" },
},
byModel: {},
}),
{ status: 200, headers: { "Content-Type": "application/json" } }
)) as typeof fetch;
const result = await executeCostAnalysis(buildTask());
assert.equal(result.metadata.totalCost, 12.5);
assert.equal(result.metadata.totalRequests, 42);
assert.equal(result.metadata.providerCosts[0]?.cost, 1.5);
assert.equal(result.metadata.providerCosts[0]?.requests, 3);
});

View File

@@ -0,0 +1,89 @@
import assert from "node:assert/strict";
import { test } from "node:test";
import {
toNumber,
toNumberOrNull,
toNumberArray,
} from "../../src/shared/utils/numeric.ts";
// Shared input matrix covering the coercion edge cases that motivated
// consolidating ~51 near-duplicate `toNumber` definitions (#7879).
const CASES: Array<{ label: string; input: unknown; finite: number | null }> = [
{ label: "null", input: null, finite: null },
{ label: "undefined", input: undefined, finite: null },
{ label: "empty string", input: "", finite: null },
{ label: "whitespace string", input: " ", finite: null },
{ label: "numeric string", input: "12", finite: 12 },
{ label: "decimal string", input: "12.5", finite: 12.5 },
{ label: "negative string", input: "-3", finite: -3 },
{ label: "zero string", input: "0", finite: 0 },
{ label: "non-numeric string", input: "abc", finite: null },
{ label: "partially-numeric string", input: "12abc", finite: null },
{ label: "NaN", input: NaN, finite: null },
{ label: "Infinity", input: Infinity, finite: null },
{ label: "-Infinity", input: -Infinity, finite: null },
{ label: "plain object", input: {}, finite: null },
{ label: "empty array", input: [], finite: null },
{ label: "exponential string", input: "1e3", finite: 1000 },
{ label: "boolean true", input: true, finite: null },
];
test("toNumber: matrix with default fallback (0)", () => {
for (const { label, input, finite } of CASES) {
const expected = finite ?? 0;
assert.equal(toNumber(input), expected, `toNumber(${label}) should be ${expected}`);
}
});
test("toNumber: matrix with custom fallback", () => {
for (const { label, input, finite } of CASES) {
const expected = finite ?? -1;
assert.equal(
toNumber(input, -1),
expected,
`toNumber(${label}, -1) should be ${expected}`
);
}
});
test("toNumber: numbers pass through untouched", () => {
assert.equal(toNumber(42), 42);
assert.equal(toNumber(-7.5), -7.5);
assert.equal(toNumber(0), 0);
});
test("toNumberOrNull: matrix returns null instead of 0 fallback", () => {
for (const { label, input, finite } of CASES) {
assert.equal(
toNumberOrNull(input),
finite,
`toNumberOrNull(${label}) should be ${finite}`
);
}
});
test("toNumberOrNull: numbers pass through untouched", () => {
assert.equal(toNumberOrNull(42), 42);
assert.equal(toNumberOrNull(0), 0);
});
test("toNumberArray: non-array input returns the fallback unchanged", () => {
assert.deepEqual(toNumberArray(null), []);
assert.deepEqual(toNumberArray(undefined), []);
assert.deepEqual(toNumberArray("not an array"), []);
assert.deepEqual(toNumberArray({}), []);
assert.deepEqual(toNumberArray(null, [1, 2]), [1, 2]);
});
test("toNumberArray: maps each element through toNumber, bad elements become 0", () => {
assert.deepEqual(toNumberArray(["12", "12.5", "abc", null, 3]), [12, 12.5, 0, 0, 3]);
assert.deepEqual(toNumberArray([]), []);
});
test("toNumberArray: element-level fallback (0) is independent of the array-level fallback", () => {
// Array itself IS present (so array-level fallback does not apply), but one
// element fails to coerce and must fall back to 0, not to the caller's
// array-level fallback value.
assert.deepEqual(toNumberArray(["abc"], [99]), [0]);
});