Skip to content

fix(plugin): insights shows savings with the right sign - #583

Merged
amiddavid merged 1 commit into
mainfrom
fix/insights-keepalive-sign
Oct 15, 2026
Merged

amiddavid merged 1 commit into
mainfrom
fix/insights-keepalive-sign

Conversation

@amiddavid

Copy link
Copy Markdown
Collaborator

Summary

What changed

In /api/stats, a saving is a negative delta_usd and a cost is positive (dash/overview.go, waterfall()). levers() read each key like this:

Key Sign in Go levers() before Now
compaction saving, negative negated (right) same
cachesplit_saved saving, negative as is (wrong) negated
keepalive_saved saving, negative as is (wrong) negated
keepalive_ping cost, positive abs() as is
decl_filter_saved saving, negative as is (wrong) negated
cg_llm cost, positive -abs() -value
self_removed saving, negative, outside total_saved as is (wrong) row removed
  • No blind abs(): a keep-alive that costs more than it saves still shows as a loss.
  • self_removed is the account's own doing. The waterfall keeps it out of total_saved (it is in total_reduced). With it in the rows, they could not add up to the headline. Open question: do you want it back as a separate line, labelled as not ours?
  • Same change in codex-marketplace/plugins/context-guru/scripts/cg.py. The two files stay byte-identical.
  • Test fixtures (stats_*.json, test_tv.py) had positive savings. They now use the real sign. Goldens did not change.

How it was tested

  • Two new tests in cg_test.go:
    • TestInsightsLeversAddUpToTotalSaved: every key non-zero, rows sum to total_saved.
    • TestInsightsKeepAliveLossStaysNegative: saved 10, spent 40 shows -30.
  • Mutations, each caught:
    • keepalive_saved sign reverted (the old code): 4 tests fail, both new tests among them.
    • cachesplit_saved sign reverted: the sum test fails.
    • decl_filter_saved sign reverted: the sum test and the goldens fail.
    • abs() on the keep-alive: both new tests fail.
    • self_removed row restored: the sum test fails.
  • Full go test ./context-guru-plugin passes on box 1 (318 s).
  • Read-only check against the local proxy, 7 days: rows sum to $147.15 = total_saved. Keep-alive shows +$62.34 = keepalive_net_usd.

Fixes #582

🤖 Generated with Claude Code

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) <noreply@anthropic.com>
Signed-off-by: DAVID AMID <DAVIDA@il.ibm.com>
@amiddavid

Copy link
Copy Markdown
Collaborator Author

Decision on the open question (from the repo owner): the self_removed row stays removed. What Claude Code removes by itself is not context-guru's saving, so insights does not show it.

@amiddavid amiddavid left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review, round 1

Verdict: approve.

What I checked

  • Signs. I read waterfall() in dash/overview.go. Savings are -X (compaction, cachesplit_saved, keepalive_saved, decl_filter_saved, self_removed). Costs are +X (cg_llm, keepalive_ping). The new levers() matches every key.
  • Net loss. No abs() is left. A keep-alive that costs more than it saves, or a cachesplit with a negative value, still shows as negative.
  • Fixtures. The changed fixtures now match the real shape. The live /api/stats of the local proxy (read-only) has keepalive_saved -127.69, keepalive_ping +65.46, compaction -90.03, cg_llm +6.24.
  • Sum. The live rows give 90.03 + (127.69 - 65.46) - 6.24 = 146.03. This equals total_saved.
  • Other readers. I grepped every key in the plugin and in Codex. cg.levers() is the only reader of the waterfall. tv.py:185 calls cg.levers(), so it is fixed too. statusline.py, insights.py and the skills do not read the waterfall. No other reader has the defect.
  • Copies. The two cg.py files are byte-identical.

Tests

  • go test ./context-guru-plugin -run 'Insights|Levers|CG' on box 1: pass.
  • Full go test ./context-guru-plugin on box 1: pass (313 s).
  • test_tv.py run direct (pytest is not installed here): 36 tests, OK.
  • My own mutations, one at a time, each caught:
    • cachesplit_saved sign flipped: sum test fails.
    • decl_filter_saved sign flipped: sum test and goldens fail.
    • keep-alive ping sign flipped: both new tests and goldens fail.
    • cg_llm sign flipped: sum test and goldens fail.
    • keepalive_saved sign flipped: both new tests and goldens fail.

self_removed

  • The old row showed the waterfall value as is. That value is -SelfRemovedUSD, so it showed negative. The row was wrong, and it broke the sum.
  • cg.py shows no other place for this number. total_reduced is not shown either.
  • Removing the row is correct for the "Saved" table. I suggest a follow-up, not a change here: a separate line below the table, such as "You removed this yourself, not counted above: $X". It must stay outside the sum.

Non-blocking

  • test_tv.py still has self_removed at 0, so no test checks that levers() ignores a non-zero self_removed. TestInsightsLeversAddUpToTotalSaved covers it on the Go side.

@amiddavid
amiddavid merged commit 491486b into main Oct 15, 2026
5 checks passed
@github-project-automation github-project-automation Bot moved this from New/ToDo to Done in Rossoctl Issue Prioritization Oct 15, 2026

@amiddavid amiddavid left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review, round 1 (update)

  • Withdrawn: my follow-up idea of a separate self_removed line. The row stays removed, as decided. Nothing blocks on it.
  • Checked: no other insights output adds self_removed to a context-guru saving. cg.levers() and tv.py do not read the key. In insights.py it is a separate finding ("self-removed-credit", severity ok). It is used for ranking only and is never added to a saved total.
  • Verdict: approve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(plugin): insights shows the keep-alive saving with the wrong sign

2 participants