From d16dabb7a7489d76091665781a43c22e97d8969d Mon Sep 17 00:00:00 2001 From: DAVID AMID Date: Thu, 15 Oct 2026 19:31:27 +0300 Subject: [PATCH] fix(plugin): insights shows savings with the right sign In the /api/stats waterfall a saving is a negative delta_usd. levers() negated compaction only. A keep-alive that saved $120 and cost $60 showed as -$180 instead of +$60, and the rows did not add up to total_saved. cachesplit_saved and decl_filter_saved had the same defect; self_removed too, but it is outside total_saved and is now left out of the rows. Same change in the Codex copy of cg.py. Test fixtures now use the real sign. New tests: the rows add up to total_saved; a keep-alive that costs more than it saves stays negative. Fixes #582 Assisted-By: Claude Opus 5.5 (1M context) Signed-off-by: DAVID AMID --- .../plugins/context-guru/scripts/cg.py | 17 +++-- context-guru-plugin/cg_test.go | 62 +++++++++++++++++++ context-guru-plugin/scripts/cg.py | 17 +++-- context-guru-plugin/scripts/test_tv.py | 4 +- .../testdata/cg/stats_today.json | 2 +- .../testdata/cg/stats_week.json | 4 +- .../testdata/cg/stats_yesterday.json | 2 +- 7 files changed, 92 insertions(+), 16 deletions(-) diff --git a/codex-marketplace/plugins/context-guru/scripts/cg.py b/codex-marketplace/plugins/context-guru/scripts/cg.py index 075c163f6..9b6ae01a9 100755 --- a/codex-marketplace/plugins/context-guru/scripts/cg.py +++ b/codex-marketplace/plugins/context-guru/scripts/cg.py @@ -149,12 +149,19 @@ def stats(api: Api, since: int, until: int | None = None, session: str | None = def levers(st: dict) -> list[tuple[str, float]]: - """Saved dollars by lever, from the stats waterfall. Own spend is subtracted, not hidden.""" + """Saved dollars by lever, from the stats waterfall. Own spend is subtracted, not hidden. + + Sign convention of `delta_usd` (dash/overview.go, waterfall()): a saving is NEGATIVE (money not + spent), a cost is POSITIVE. So the saving keys are negated here, and the cost keys are read as + they are. A key that really is a net loss then shows as negative. `self_removed` is left out on + purpose: the waterfall keeps it OUTSIDE total_saved (it is the account's own doing, not ours), + so with it the rows could not add up to the headline. + """ w = {x["key"]: x["delta_usd"] for x in st.get("waterfall", [])} - rows = [("Compaction", -w.get("compaction", 0.0)), ("Cache split", w.get("cachesplit_saved", 0.0)), - ("Keep-alive pings", w.get("keepalive_saved", 0.0) - abs(w.get("keepalive_ping", 0.0))), - ("Tool filtering", w.get("decl_filter_saved", 0.0)), ("Self-removed content", w.get("self_removed", 0.0)), - ("Context Guru's own model spend", -abs(w.get("cg_llm", 0.0)))] + rows = [("Compaction", -w.get("compaction", 0.0)), ("Cache split", -w.get("cachesplit_saved", 0.0)), + ("Keep-alive pings", -w.get("keepalive_saved", 0.0) - w.get("keepalive_ping", 0.0)), + ("Tool filtering", -w.get("decl_filter_saved", 0.0)), + ("Context Guru's own model spend", -w.get("cg_llm", 0.0))] return [(n, v) for n, v in rows if abs(v) >= 0.005] diff --git a/context-guru-plugin/cg_test.go b/context-guru-plugin/cg_test.go index 87e276068..ff17c5b22 100644 --- a/context-guru-plugin/cg_test.go +++ b/context-guru-plugin/cg_test.go @@ -473,3 +473,65 @@ func TestDocsExamplesAreTheGoldens(t *testing.T) { } } } + +// levers runs cg.levers() on a stats document and returns the rows by name. +func cgLevers(t *testing.T, st map[string]any) map[string]float64 { + t.Helper() + py := requireTool(t, "python3") + in, err := json.Marshal(st) + if err != nil { + t.Fatal(err) + } + code := "import sys, json; sys.path.insert(0, sys.argv[1]); import cg; " + + "print(json.dumps(dict(cg.levers(json.loads(sys.argv[2])))))" + out, err := exec.Command(py, "-c", code, scriptsDir(t), string(in)).CombinedOutput() + if err != nil { + t.Fatalf("cg.levers: %v\n%s", err, out) + } + rows := map[string]float64{} + if err := json.Unmarshal(out, &rows); err != nil { + t.Fatalf("cg.levers output %q: %v", out, err) + } + return rows +} + +func cgWaterfall(total float64, deltas map[string]float64) map[string]any { + var steps []map[string]any + for k, v := range deltas { + steps = append(steps, map[string]any{"key": k, "delta_usd": v}) + } + return map[string]any{"total_saved_usd": total, "waterfall": steps} +} + +// Sign convention of /api/stats (dash/overview.go, waterfall()): a saving is a NEGATIVE delta_usd, a +// cost is positive. #582: keepalive_saved was read as if positive, so a saving showed as a loss. +// The lever rows must add up to total_saved_usd, which is compaction + cache split + the keep-alive's +// net + tool filtering - own spend, and does NOT include self_removed (the account's own doing). +func TestInsightsLeversAddUpToTotalSaved(t *testing.T) { + st := cgWaterfall(80+20+(120-60)+15-6, map[string]float64{ + "baseline": 1000, "compaction": -80, "cachesplit_saved": -20, "keepalive_saved": -120, + "keepalive_ping": 60, "decl_filter_saved": -15, "self_removed": -30, "cg_llm": 6, + }) + rows := cgLevers(t, st) + var sum float64 + for _, v := range rows { + sum += v + } + if want := st["total_saved_usd"].(float64); sum < want-0.005 || sum > want+0.005 { + t.Errorf("rows add up to %.2f, total_saved is %.2f: %v", sum, want, rows) + } + for name, want := range map[string]float64{"Compaction": 80, "Cache split": 20, "Keep-alive pings": 60, + "Tool filtering": 15, "Context Guru's own model spend": -6} { + if got := rows[name]; got < want-0.005 || got > want+0.005 { + t.Errorf("%s = %.2f, want %.2f", name, got, want) + } + } +} + +// A keep-alive whose pings cost more than they saved is a real loss and must stay one. +func TestInsightsKeepAliveLossStaysNegative(t *testing.T) { + rows := cgLevers(t, cgWaterfall(0, map[string]float64{"keepalive_saved": -10, "keepalive_ping": 40})) + if got := rows["Keep-alive pings"]; got < -30.005 || got > -29.995 { + t.Errorf("Keep-alive pings = %.2f, want -30.00 (saved 10, spent 40)", got) + } +} diff --git a/context-guru-plugin/scripts/cg.py b/context-guru-plugin/scripts/cg.py index 075c163f6..9b6ae01a9 100755 --- a/context-guru-plugin/scripts/cg.py +++ b/context-guru-plugin/scripts/cg.py @@ -149,12 +149,19 @@ def stats(api: Api, since: int, until: int | None = None, session: str | None = def levers(st: dict) -> list[tuple[str, float]]: - """Saved dollars by lever, from the stats waterfall. Own spend is subtracted, not hidden.""" + """Saved dollars by lever, from the stats waterfall. Own spend is subtracted, not hidden. + + Sign convention of `delta_usd` (dash/overview.go, waterfall()): a saving is NEGATIVE (money not + spent), a cost is POSITIVE. So the saving keys are negated here, and the cost keys are read as + they are. A key that really is a net loss then shows as negative. `self_removed` is left out on + purpose: the waterfall keeps it OUTSIDE total_saved (it is the account's own doing, not ours), + so with it the rows could not add up to the headline. + """ w = {x["key"]: x["delta_usd"] for x in st.get("waterfall", [])} - rows = [("Compaction", -w.get("compaction", 0.0)), ("Cache split", w.get("cachesplit_saved", 0.0)), - ("Keep-alive pings", w.get("keepalive_saved", 0.0) - abs(w.get("keepalive_ping", 0.0))), - ("Tool filtering", w.get("decl_filter_saved", 0.0)), ("Self-removed content", w.get("self_removed", 0.0)), - ("Context Guru's own model spend", -abs(w.get("cg_llm", 0.0)))] + rows = [("Compaction", -w.get("compaction", 0.0)), ("Cache split", -w.get("cachesplit_saved", 0.0)), + ("Keep-alive pings", -w.get("keepalive_saved", 0.0) - w.get("keepalive_ping", 0.0)), + ("Tool filtering", -w.get("decl_filter_saved", 0.0)), + ("Context Guru's own model spend", -w.get("cg_llm", 0.0))] return [(n, v) for n, v in rows if abs(v) >= 0.005] diff --git a/context-guru-plugin/scripts/test_tv.py b/context-guru-plugin/scripts/test_tv.py index 2629103e5..53c366c15 100644 --- a/context-guru-plugin/scripts/test_tv.py +++ b/context-guru-plugin/scripts/test_tv.py @@ -74,8 +74,8 @@ def history(): def stats(): - wf = [("baseline", 898.05, True), ("compaction", -33.9, False), ("cachesplit_saved", 0, False), ("keepalive_saved", 9.4, False), - ("keepalive_ping", 3.1, False), ("decl_filter_saved", 7.85, False), ("self_removed", 0, False), ("cg_llm", 4.1, False)] + wf = [("baseline", 898.05, True), ("compaction", -33.9, False), ("cachesplit_saved", 0, False), ("keepalive_saved", -9.4, False), + ("keepalive_ping", 3.1, False), ("decl_filter_saved", -7.85, False), ("self_removed", 0, False), ("cg_llm", 4.1, False)] return {"total_saved_usd": 43.95, "cost_usd": 856.85, "baseline_cost_usd": 898.05, "waterfall": [{"key": k, "delta_usd": v} for k, v, _ in wf]} diff --git a/context-guru-plugin/testdata/cg/stats_today.json b/context-guru-plugin/testdata/cg/stats_today.json index b6490f2f6..fa0f17987 100644 --- a/context-guru-plugin/testdata/cg/stats_today.json +++ b/context-guru-plugin/testdata/cg/stats_today.json @@ -15,7 +15,7 @@ }, { "key": "keepalive_saved", - "delta_usd": 1.6 + "delta_usd": -1.6 }, { "key": "keepalive_ping", diff --git a/context-guru-plugin/testdata/cg/stats_week.json b/context-guru-plugin/testdata/cg/stats_week.json index a2ce0c648..da316de88 100644 --- a/context-guru-plugin/testdata/cg/stats_week.json +++ b/context-guru-plugin/testdata/cg/stats_week.json @@ -19,7 +19,7 @@ }, { "key": "keepalive_saved", - "delta_usd": 9.4 + "delta_usd": -9.4 }, { "key": "keepalive_ping", @@ -27,7 +27,7 @@ }, { "key": "decl_filter_saved", - "delta_usd": 7.85 + "delta_usd": -7.85 }, { "key": "self_removed", diff --git a/context-guru-plugin/testdata/cg/stats_yesterday.json b/context-guru-plugin/testdata/cg/stats_yesterday.json index e77fefd52..b75ce12f4 100644 --- a/context-guru-plugin/testdata/cg/stats_yesterday.json +++ b/context-guru-plugin/testdata/cg/stats_yesterday.json @@ -15,7 +15,7 @@ }, { "key": "keepalive_saved", - "delta_usd": 1.0 + "delta_usd": -1.0 }, { "key": "keepalive_ping",