fix: uncap the Overview model table, billions token rung, unpriced calls dimming (#1318) - #1443
Open
ozymandiashh wants to merge 5 commits into
Open
ozymandiashh wants to merge 5 commits into
ozymandiashh wants to merge 5 commits into
Conversation
Review follow-ups on the per-model token counts, addressed on the merged
head.
buildPeriodData walked every assistant call in every session twice: once
through sessionBillableOutputTokens for the headline and again through
sessionModelBillableOutputTokens for the per-model split. Both derive from
the same traversal and the same sawUsage fallback decision, so fold them
into one sessionBillableOutput(session) returning { total, byModel }; the
two existing exports become thin wrappers, keeping their semantics for
other callers. A call whose model resolves to no breakdown bucket still
counts toward the session total and still lands in no row, exactly as
before, so no total moves.
The macOS accessibility text grouped a signed string, counting the minus
as a leading digit and emitting it twice: a -1,234,567 count read as
"--1,234,567", a different number to a screen reader. Group the magnitude
and re-apply the sign; magnitude is unsigned, so Int.min no longer traps
on negation either. Negative counts are not expected from the CLI, but the
payload field is a plain Int and a corrupt or hand-edited snapshot carries
whatever it carries.
Two findings are disclosed rather than changed:
* buildTopModels' unknown-vs-zero guard cannot fire on the durable path.
ModelDayStats types the four counts as required numbers and daily-cache
sanitizeModels runs each through num(), so a missing value becomes a
known 0 and a carried day under-reports as exact instead of unknown.
Making it reachable means widening ModelDayStats to optional counts and
teaching every daily-cache arithmetic site (fold, subtract, reduce) plus
buildPeriodDataFromDays to propagate absence — a durable-cache contract
change, too broad to ride along here. The guard stays: PeriodData
already types these counts optional, so fresh-session and plugin-sourced
rows may legitimately omit them.
* The all-provider Overview models table now reads current.topModels and
so inherits the payload's 20-row TOP_MODELS_LIMIT, which its previous
uncapped daily-history union did not have. Kept, because that union was
itself a per-day top-five truncation whose tail rows were already
partial sums, but the PR body's "the cap is unchanged" claim was wrong
for this view and the trade-off is now recorded at the call site.
…imming unpriced calls (getagentseal#1318) The Overview model table reads current.topModels, which was capped at TOP_MODELS_LIMIT (20): on a machine with 47 models every local and free model fell out of the table. The list is now uncapped — every consumer that wants fewer rows already slices its own (desktop hero 8, MCP 5 or caller limit) and the macOS menubar reads a per-session list. compactTokenCount (menubar Models rows) gains the billions rung: period cache-read totals past a billion rendered as 12345.7M. The desktop Models tables stop dimming the calls column on unpriced rows — calls are observed usage like the token columns, not a pricing artifact; cost/saved keep their dashes and the alias affordance. Status snapshot revision 9 invalidates warm snapshots so a stale record cannot keep serving the truncated 20-row list.
Member
|
Thanks for catching the lost #1315 commits, that one is on me. This changes three screens, so per the new rule it needs before and after screenshots in the PR before review: the desktop Overview model table with more than 20 models, the desktop Models table showing calls no longer dimmed on an unpriced row, and the menubar Models row with a value in the billions rung. Dark and light for the desktop ones. Once they are up I will verify against my own data and take it from there. |
…localization catalog The per-model token line arrived with the getagentseal#1315 re-land as a raw interpolated Text(…), so the localization coverage scanner fails: a zh-Hans user got an English fragment. The key joins both tables with positional specifiers per the getagentseal#1331 rule, Chinese rebinding the word order.
Collaborator
Author
|
The first CI run's mac failure is fixed by dd0f9df: the model row's secondary token line (from the re-landed #1315) shipped as a raw interpolated |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
Fixes #1318, and re-lands the three #1315 commits it depends on.
Part 1 — re-land of #1315 (first three commits, authorship preserved)
As noted on the issue, #1315 merged into
fix/dock-pace-followupsminutes after #1314 had already merged that branch intomain, so455130f0,3d3088f0ande6cce142never reached main —git merge-base --is-ancestor ef97600d mainfails andtests/menubar-model-tokens.test.tsis not on main. #1318 is written against that code.This PR cherry-picks the three commits onto current main, preserving authors. Conflicts were small and resolved by keeping both sides:
src/menubar-json.ts— main'srawCategoryon categories + the incoming per-model token fieldsCHANGELOG.md— kept main's newer bullets plus the Show per-model cached tokens alongside cost in desktop and menubar #1265 bulletIf you would rather re-land #1315 your own way, dropping the last commit of this PR leaves you exactly with the re-land.
Part 2 — the #1318 fix (last commit)
buildTopModelsno longer slices atTOP_MODELS_LIMIT(20). On the 47-model machine from the issue, every local and free model (qwen, llama, deepseek, gemini, grok) fell out of the Overview table — exactly the rows whose $0 cost ranks them last, and exactly the rows Show per-model token counts alongside cost in desktop and menubar #1315 had just made visible in the Models tables. Every consumer wanting fewer rows already slices its own (desktop hero takes 8, MCP tables take 5 or the caller's limit); the macOS menubar reads a per-sessionmodelslist and never consumedcurrent.topModels. The Overview table already renders in a scroll container, so no UI change was needed.compactTokenCount. The menubar Models row formatter had K/M only, so a period cache-read total past a billion rendered as12345.7M; it now renders12.3B, matchingMenubarRowFormatter.compactTokensand the desktopformatCompact. Made internal so a test pins the rung.Snapshot semantics
Status snapshot revision 8 → 9: a v8 record holds only the 20 costliest rows, so without the bump a warm snapshot would keep serving the truncated list until the next recompute — the same staleness class v8 itself was bumped for.
Verification
npm test)tsc --noEmitclean, 1001 tests passedModelEntry token countssuite 9/9 (incl. the new billions-rung test); module builds viaswift testwith the CLT Swift Testing flagsmenubar-json.test.tsflipped to pin the uncapped behavior (30 models in → 30 rows out, synthetic still filtered)