diff --git a/PRODUCT.md b/PRODUCT.md index de97e17..b32e152 100644 --- a/PRODUCT.md +++ b/PRODUCT.md @@ -25,9 +25,20 @@ Built as part of Nick's Omilia internship; doubles as a portfolio project. incident month) or a lean startup (1-2 minor findings, on purpose). Each run is internally coherent across its 6 months; the generators are pure and clock-free, reproducible via explicit seeds (`DEMO_SEED` in tests). -- 96 vitest tests pin the money math; CI runs type-check, lint, format, test. +- 117 vitest tests pin the money math; CI runs type-check, lint, format, test. - AGPL-3.0. +Corrected 2026-08-08: `prOpenAI` matched pricing-table keys in insertion +order, so every "-mini" variant resolved to its full-size parent — +`gpt-4o-mini` billed at GPT-4o's $2.50/$10 rather than $0.15/$0.60. The +consequence was not just mispricing: `costMiniDowngrade` returned a GPT-4o +row's own cost, so "Model Downgrade → GPT-4o-mini" computed zero savings and +was dropped by the $0.50 floor, silently disabling the engine's headline +OpenAI recommendation on the commonest OpenAI workload. Longest key now wins. +Rule 3 gained two deferrals (RAG territory, and avg input ≥ 10k tok) because +at mini's real price it outbid every other finding for the per-row savings +cap; the startup demo persona's volumes were re-sized against real pricing. + ## Where it's headed Next candidates, in rough order (none committed): diff --git a/src/__tests__/costing.test.ts b/src/__tests__/costing.test.ts index 11a75f0..8b041f1 100644 --- a/src/__tests__/costing.test.ts +++ b/src/__tests__/costing.test.ts @@ -331,7 +331,24 @@ describe("OpenAI costing functions vs rule engine", () => { }); it("costEnableCachingOpenAI matches rule 0's optimized cost", () => { - const r = oaiRow({ inp: 10_000_000, out: 2_000_000, reqs: 2000, cost: 60 }); + // Mini row on purpose. Rule 0 needs inp > 5M, and on a GPT-4o row that + // same volume always attracts a mini-downgrade finding (rule 1 when + // ao < 200, rule 3 otherwise) which takes the per-row cap's headroom + // first and clamps this one. Rules 1 and 3 both skip mini rows, so this + // is the only way to isolate rule 0. + // cost 0 so `cur` comes from the table: rule 0's saving is computed from + // table input cost, and pinning an arbitrary billed figure against cheap + // mini tokens pushed the saving under addFinding's $0.50 floor. ai is + // 7k — over rule 0's 5k signal, under rule 8's 8k gate, so prompt-bloat + // doesn't compete for the row's headroom. + const r = oaiRow({ + model: "gpt-4o-mini", + line_item: "GPT-4o mini", + inp: 60_000_000, + out: 6_000_000, + reqs: 8600, + cost: 0, + }); const f = findIssuesOpenAI([r], []).find( (x) => x.cat === OpenAICategory.PROMPT_CACHING ); @@ -340,7 +357,11 @@ describe("OpenAI costing functions vs rule engine", () => { }); it("costBatchDiscountOpenAI matches the batch rules (50% off)", () => { + // Mini row for the same reason as the caching test above: ao here is 100, + // so a GPT-4o row would pull in rule 1's mini downgrade and clamp this. const r = oaiRow({ + model: "gpt-4o-mini", + line_item: "GPT-4o mini", inp: 30_000_000, out: 3_000_000, reqs: 30_000, diff --git a/src/__tests__/openai-pricing.test.ts b/src/__tests__/openai-pricing.test.ts new file mode 100644 index 0000000..9e90150 --- /dev/null +++ b/src/__tests__/openai-pricing.test.ts @@ -0,0 +1,124 @@ +import { describe, it, expect } from "vitest"; +import { MP_OPENAI, prOpenAI, tcOpenAI } from "@/lib/openai/pricing"; +import { findIssuesOpenAI } from "@/lib/openai/analysis"; +import { OpenAICategory } from "@/types/analysis"; + +/* + * prOpenAI resolves a model id by substring, and several table keys are + * prefixes of others. Walking the table in insertion order matched the prefix + * first, so every "-mini" variant priced as its full-size parent — gpt-4o-mini + * at GPT-4o's $2.50/$10 instead of $0.15/$0.60, a 16.7x overcharge. These pin + * the resolution order and the one rule the bug silently disabled. + */ + +describe("prOpenAI resolves the most specific model", () => { + const cases: [string, string, number, number][] = [ + ["gpt-4o", "GPT-4o", 2.5, 10], + ["gpt-4o-mini", "GPT-4o Mini", 0.15, 0.6], + ["o1", "o1", 15, 60], + ["o1-mini", "o1 Mini", 3, 12], + ["o3", "o3", 2, 8], + ["o3-mini", "o3 Mini", 1.1, 4.4], + ["gpt-4", "GPT-4", 30, 60], + ["gpt-4-32k", "GPT-4 32k", 60, 120], + ["gpt-3.5-turbo", "GPT-3.5 Turbo", 0.5, 1.5], + ]; + + for (const [model, label, i, o] of cases) { + it(`${model} prices as ${label}`, () => { + const p = prOpenAI(model); + expect(p.l).toBe(label); + expect(p.i).toBe(i); + expect(p.o).toBe(o); + }); + } + + it("resolves dated and suffixed ids to the same entry", () => { + expect(prOpenAI("gpt-4o-mini-2024-07-18").l).toBe("GPT-4o Mini"); + expect(prOpenAI("gpt-4o-2024-08-06").l).toBe("GPT-4o"); + expect(prOpenAI("o3-mini-2025-01-31").l).toBe("o3 Mini"); + }); + + it("never resolves a longer key to a shorter one that it contains", () => { + // The general property behind every case above: for any two table keys + // where one contains the other, the longer must win. + const keys = Object.keys(MP_OPENAI); + for (const key of keys) { + const shadowing = keys.filter((k) => k !== key && key.includes(k)); + if (shadowing.length === 0) continue; + expect(prOpenAI(key), `${key} shadowed by ${shadowing}`).toBe( + MP_OPENAI[key] + ); + } + }); + + it("still falls back to GPT-4o for an unknown model", () => { + const p = prOpenAI("some-future-model"); + expect(p.i).toBe(2.5); + expect(p.o).toBe(10); + expect(p.g).toBe(0); + }); +}); + +describe("the mini downgrade rule the mispricing disabled", () => { + it("fires on a GPT-4o row and prices the remedy at mini rates", () => { + // Textbook candidate: high volume, short outputs. While gpt-4o-mini + // resolved to GPT-4o pricing, costMiniDowngrade returned the row's own + // cost, savings came to 0, and addFinding's `sav > 0.5` floor dropped the + // finding — so the engine's headline OpenAI advice never appeared on the + // commonest OpenAI workload there is. + const inp = 60_000_000; + const out = 3_000_000; + const row = { + model: "gpt-4o", + project_id: "proj_1", + line_item: "GPT-4o", + cost: tcOpenAI("gpt-4o", inp, out), + inp, + out, + reqs: 40_000, + activeDays: 28, + }; + + const f = findIssuesOpenAI([row], []).find( + (x) => x.cat === OpenAICategory.MODEL_DOWNGRADE_MINI + ); + + expect(f).toBeDefined(); + expect(f!.opt).toBeCloseTo(tcOpenAI("gpt-4o-mini", inp, out), 10); + // ~94% of the row's spend, not the 0 the bug produced. + expect(f!.sav / f!.cur).toBeGreaterThan(0.9); + }); + + it("defers to RAG bloat and prompt bloat instead of starving them", () => { + // Mini is ~94% cheaper, so rule 3 outbids every other finding for the + // per-row savings cap. It now stands down on rows those rules own. + const ragRow = { + model: "gpt-4o", + project_id: "proj_1", + line_item: "GPT-4o", + cost: 0, + inp: 32_000_000, // ratio 28:1, over rule 2's 12:1 and 10M gates + out: 1_150_000, + reqs: 1135, + activeDays: 26, + }; + const ragCats = findIssuesOpenAI([ragRow], []).map((f) => f.cat); + expect(ragCats).toContain(OpenAICategory.RAG_OPTIMIZATION); + expect(ragCats).not.toContain(OpenAICategory.MODEL_DOWNGRADE_MINI); + + const bloatRow = { + model: "gpt-4o", + project_id: "proj_1", + line_item: "GPT-4o", + cost: 0, + inp: 5_700_000, // avg input ~15.7k tok, over rule 3's new 10k ceiling + out: 190_000, + reqs: 362, + activeDays: 24, + }; + const bloatCats = findIssuesOpenAI([bloatRow], []).map((f) => f.cat); + expect(bloatCats).toContain(OpenAICategory.PROMPT_OPTIMIZATION); + expect(bloatCats).not.toContain(OpenAICategory.MODEL_DOWNGRADE_MINI); + }); +}); diff --git a/src/lib/demo.ts b/src/lib/demo.ts index 64c244f..965031a 100644 --- a/src/lib/demo.ts +++ b/src/lib/demo.ts @@ -656,21 +656,25 @@ const OAI_ENT_WORKLOADS: OaiCompletionsWorkload[] = [ const OAI_STARTUP_WORKLOADS: OaiCompletionsWorkload[] = [ // Product API: right-sized on mini, but repeated context isn't cached — - // the run's one deliberate (small) finding. + // the run's one deliberate (small) finding. Volumes are sized against real + // mini pricing ($0.15/$0.60): they were originally set when prOpenAI + // mis-resolved "gpt-4o-mini" to GPT-4o rates, which inflated this project's + // spend ~17x and made a sub-dollar caching saving look like a real one. { pid: "proj_app", model: "gpt-4o-mini", - inp: 250_000, - out: 40_000, - reqs: 100, + inp: 2_600_000, + out: 400_000, + reqs: 1_000, }, - // Internal tools: small and clean. + // Internal tools: small and clean. Avg input stays ~250 tok/req, well under + // the caching rule's 2k gate, so this project keeps producing no findings. { pid: "proj_tools", model: "gpt-4o-mini", - inp: 20_000, - out: 5_000, - reqs: 80, + inp: 200_000, + out: 50_000, + reqs: 800, }, ]; diff --git a/src/lib/openai/analysis.ts b/src/lib/openai/analysis.ts index 474b033..894a1b9 100644 --- a/src/lib/openai/analysis.ts +++ b/src/lib/openai/analysis.ts @@ -256,6 +256,10 @@ export function findIssuesOpenAI( r.model.toLowerCase().includes("gpt-4") && !r.model.toLowerCase().includes("gpt-4o"); + // Rule 2's gate. Rules 3 and 8 both defer to it on these rows so their + // remedies don't compete with RAG bloat for the per-row savings cap. + const ragTerritory = ratio > 12 && r.inp > 10e6; + // Track categories already added for this model to prevent duplicates const addedCategories = new Set(); @@ -458,7 +462,23 @@ export function findIssuesOpenAI( } /* ─── RULE 3: GPT-4o Overkill → GPT-4o-mini ─── */ - if (hasTokenData && isGPT4O && ao >= 200 && r.inp > 5e6) { + // Two deferrals, both forced by mini's real price. Correctly resolved, + // mini is ~94% cheaper than GPT-4o, so this rule outbids every other + // finding for the per-row savings cap and starves them. It only earns + // that on rows where mini genuinely performs comparably: + // - not RAG territory (rule 2 owns those; the fix is retrieval) + // - avg input under 10k tok (rule 8 owns the bloated ones; a request + // carrying 15k tokens of context is not a simple classification task) + // `ai < 10000` was already a confidence signal here; at real pricing it + // has to be a gate. + if ( + hasTokenData && + isGPT4O && + ao >= 200 && + r.inp > 5e6 && + !ragTerritory && + ai < 10000 + ) { const signals: FindingSignal[] = [ { weight: 0.3, met: ao < 1500, label: "avg output < 1.5k tok" }, { weight: 0.25, met: r.reqs > 100, label: "100+ requests" }, @@ -662,7 +682,6 @@ export function findIssuesOpenAI( // same input trim there, and a duplicate finding just fights it for the // row's savings headroom (the calibration sweep caught rule 8 flaking on // exactly those rows). No always-true signal — confidence must be earned. - const ragTerritory = ratio > 12 && r.inp > 10e6; if (hasTokenData && !ragTerritory && ai > 8000 && r.reqs > 100 && cur > 3) { const signals: FindingSignal[] = [ { weight: 0.35, met: ai > 12000, label: "avg input > 12k tok" }, diff --git a/src/lib/openai/pricing.ts b/src/lib/openai/pricing.ts index 3e09ed5..2f06003 100644 --- a/src/lib/openai/pricing.ts +++ b/src/lib/openai/pricing.ts @@ -60,6 +60,17 @@ export const MP_OPENAI: Record = { }, }; +/** + * Model keys longest-first. Lookup is a substring test, and several keys are + * prefixes of others ("gpt-4o" of "gpt-4o-mini", "o1" of "o1-mini", "gpt-4" of + * "gpt-4-32k"). Insertion order matched the prefix first, so every "-mini" + * variant priced as its full-size parent — gpt-4o-mini at $2.50/$10 instead of + * $0.15/$0.60. Longest match wins, so the most specific key always answers. + */ +const MP_OPENAI_KEYS = Object.keys(MP_OPENAI).sort( + (a, b) => b.length - a.length +); + /** * Get pricing information for a given OpenAI model string * @param m - Model name/identifier (e.g., "gpt-4o", "gpt-4o-mini") @@ -70,20 +81,13 @@ export function prOpenAI(m: string | undefined): PricingInfo { return { i: 2.5, o: 10, l: m || "unknown", t: OpenAIModelTier.GPT4O, g: 0 }; const k = m.toLowerCase(); - // Exact match first - for (const [key, v] of Object.entries(MP_OPENAI)) { - if (k.includes(key)) return v; + // Most specific key first + for (const key of MP_OPENAI_KEYS) { + if (k.includes(key)) return MP_OPENAI[key]; } - // Fuzzy matching - if (k.includes("gpt-4o-mini")) return MP_OPENAI["gpt-4o-mini"]; - if (k.includes("gpt-4o")) return MP_OPENAI["gpt-4o"]; - if (k.includes("o3-mini")) return MP_OPENAI["o3-mini"]; - if (k.includes("o3")) return MP_OPENAI["o3"]; - if (k.includes("o1-mini")) return MP_OPENAI["o1-mini"]; - if (k.includes("o1")) return MP_OPENAI["o1"]; - if (k.includes("gpt-4-turbo")) return MP_OPENAI["gpt-4-turbo"]; - if (k.includes("gpt-4")) return MP_OPENAI["gpt-4"]; + // "gpt-3.5" is the one alias that is not itself a key (the key is + // "gpt-3.5-turbo"), so a bare "gpt-3.5-*" id still needs catching. if (k.includes("gpt-3.5")) return MP_OPENAI["gpt-3.5-turbo"]; // Default to GPT-4o