From 9e0aaeef2876e46aae3acda5ab15f9f2758a60ba Mon Sep 17 00:00:00 2001 From: ZhenghuaBao Date: Wed, 19 Aug 2026 11:26:09 +0800 Subject: [PATCH] fix: stop --pricing from being mistaken for the usage file usage-summary.mjs picked its input with a separate scan for "the first argv entry that does not start with --". A flag's VALUE is not a positional argument, though: --pricing takes a path, and a path does not start with "--", so putting the flag first made the price list get read as the metering file. The failure was silent rather than loud. A price list is itself valid JSON, so it parsed as a single metering row with no token fields: node usage-summary.mjs --pricing pricing.json usage.jsonl --json -> calls: 1, prompt_tokens: 0 Exit 0, no warning, not even a malformed-line count -- and the real usage file never read. For a script whose entire purpose is comparing model costs, a plausible-looking $0.00 is the worst possible output. Fold the two scans into one pass that advances past a flag's value, and only consume that value when it is actually a value, so a trailing --pricing or one followed by another flag does not swallow it. Not reachable from action.yml, which calls `node "$USAGE_SUMMARY" "$USAGE_FILE"` -- file first, and no --pricing at all. This only bit manual cost-comparison runs, which is exactly when --pricing gets used. Adds 6 tests: both orders agree, the first positional wins, --json is position-independent, and the two valueless --pricing shapes are harmless. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/usage-summary.mjs | 21 +++++++++++-- scripts/usage-summary.test.mjs | 57 ++++++++++++++++++++++++++++++++-- 2 files changed, 73 insertions(+), 5 deletions(-) diff --git a/scripts/usage-summary.mjs b/scripts/usage-summary.mjs index 5a10fdb..9c5595d 100644 --- a/scripts/usage-summary.mjs +++ b/scripts/usage-summary.mjs @@ -29,13 +29,28 @@ import fs from "node:fs"; const USD_PER_1M_AT_RATIO_1 = 2.0; +// One pass, because a flag's VALUE is not a positional argument. --pricing +// takes a path, and a path does not start with "--", so scanning separately +// for "the first argv entry that isn't a flag" picks up the price list as the +// usage file whenever --pricing comes first. That failure is silent rather +// than loud: a price list is itself valid JSON, so it parses as a single +// metering row with no token fields and reports a run that cost nothing. +// Advancing the index past a flag's value keeps both argument orders working. const argv = process.argv.slice(2); -const file = argv.find((a) => !a.startsWith("--")); +let file = null; let pricingPath = null; let asJson = false; for (let i = 0; i < argv.length; i += 1) { - if (argv[i] === "--pricing") pricingPath = argv[i + 1]; - else if (argv[i] === "--json") asJson = true; + if (argv[i] === "--pricing") { + // Only consume the next entry when it is actually a value. A trailing + // --pricing, or one followed by another flag, must not swallow that flag. + const next = argv[i + 1]; + if (next !== undefined && !next.startsWith("--")) { + pricingPath = next; + i += 1; + } + } else if (argv[i] === "--json") asJson = true; + else if (!argv[i].startsWith("--") && file === null) file = argv[i]; } if (!file) { console.error("usage: node usage-summary.mjs [--pricing pricing.json] [--json]"); diff --git a/scripts/usage-summary.test.mjs b/scripts/usage-summary.test.mjs index 5ca003c..606ce97 100644 --- a/scripts/usage-summary.test.mjs +++ b/scripts/usage-summary.test.mjs @@ -59,8 +59,8 @@ function run(args) { return { status: r.status, stdout: r.stdout ?? "", stderr: r.stderr ?? "" }; } -// The flag order matters: the file is the first argv entry not starting with -// "--", so the usage file must precede --pricing. That is the documented order. +// Most cases below use the documented order (usage file first); the +// "argument order" block pins that the reverse order works too. function runJson(args) { const r = run([...args, "--json"]); assert.equal(r.status, 0, r.stderr); @@ -159,6 +159,59 @@ describe("best-effort input handling", () => { }); }); +describe("argument order", () => { + // A flag's value is not a positional argument. --pricing takes a path, and a + // path does not start with "--", so a "first non-flag argv entry" scan used + // to pick the price list as the usage file when the flag came first. It + // failed silently: a price list is valid JSON, so it parsed as one metering + // row with no token fields and reported a run that cost nothing. + test("--pricing before the usage file does not steal it", () => { + const usage = writeUsage([{ model: "m", prompt: 1_000_000 }]); + const pricing = writePricing([{ model_name: "m", model_ratio: 1 }]); + const r = run(["--pricing", pricing, usage, "--json"]); + assert.equal(r.status, 0, r.stderr); + const out = JSON.parse(r.stdout); + assert.equal(out.prompt_tokens, 1_000_000, "the usage file must be read, not the price list"); + assert.equal(out.usd, 2, "and the price list must still be applied"); + }); + + test("both orders produce the same result", () => { + const usage = writeUsage([{ model: "m", prompt: 500_000, completion: 1000 }]); + const pricing = writePricing([{ model_name: "m", model_ratio: 1 }]); + const flagFirst = JSON.parse(run(["--pricing", pricing, usage, "--json"]).stdout); + const fileFirst = JSON.parse(run([usage, "--pricing", pricing, "--json"]).stdout); + assert.deepEqual(flagFirst, fileFirst); + }); + + test("the first positional wins; a stray extra one does not override it", () => { + const usage = writeUsage([{ model: "m", prompt: 10 }]); + const other = writeUsage([{ model: "m", prompt: 999 }]); + const out = JSON.parse(run([usage, other, "--json"]).stdout); + assert.equal(out.prompt_tokens, 10); + }); + + test("--json is recognized wherever it appears", () => { + const usage = writeUsage([{ model: "m", prompt: 7 }]); + const out = JSON.parse(run(["--json", usage]).stdout); + assert.equal(out.prompt_tokens, 7); + }); + + test("a valueless --pricing does not swallow the flag after it", () => { + const usage = writeUsage([{ model: "m", prompt: 7 }]); + const r = run([usage, "--pricing", "--json"]); + assert.equal(r.status, 0, r.stderr); + const out = JSON.parse(r.stdout); + assert.equal(out.prompt_tokens, 7, "--json must still be honored"); + assert.equal(out.usd, null, "no price list was supplied"); + }); + + test("a trailing --pricing with nothing after it is harmless", () => { + const usage = writeUsage([{ model: "m", prompt: 7 }]); + const r = run([usage, "--pricing"]); + assert.equal(r.status, 0, r.stderr); + }); +}); + describe("price-list resolution", () => { const MILLION = { model: "TARGET", prompt: 1_000_000 }; // ratio 1 => $2.00/1M input, so a priced 1M-prompt run is exactly $2.0000.