feat(core,cli): add session and client+session group-by - #543
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
There was a problem hiding this comment.
2 issues found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/tokscale-cli/src/tui/ui/dialog/group_by_picker.rs">
<violation number="1" location="crates/tokscale-cli/src/tui/ui/dialog/group_by_picker.rs:56">
P1: Adding Session and ClientSession group-by options causes the picker dialog to overflow: 6 options × 2 rows each = 12 list rows, but the dialog only provides ~9 lines for the list area (14 height - 2 borders - 3 fixed header/divider/hint rows). The last option and part of the second-to-last are clipped and invisible, with no scrolling logic to compensate. Increase the height cap to ~18 and/or add scroll offset logic matching source_picker.rs.</violation>
</file>
<file name="README.md">
<violation number="1" location="README.md:255">
P2: Documentation inconsistency: `workspace+model` group-by is mentioned in the TUI picker description but missing from the Group-By Strategies table.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
junhoyeo
approved these changes
May 24, 2026
junhoyeo
left a comment
Owner
There was a problem hiding this comment.
LGTM — clean additive group-by, no data leak across modes. Follow-up: normalize ms/s timestamp convention instead of magnitude heuristic.
Adds GroupBy::Session and GroupBy::ClientSession variants so users can group token usage by session_id from the CLI/JSON output. Downstream runners (e.g. kdlbs/kandev) need to attribute cost to specific agent-CLI sessions, and session_id is already extracted during parsing for dedup -- this surfaces it through aggregation + serialization.
Adding Session/ClientSession brought the picker to 6 options x 2 rows = 12 list rows, exceeding the previous height cap of 14 (only ~9 list rows after borders + header/divider/hint). Bump the cap to 18 to match source_picker so every option stays visible without scrolling, and add the existing Workspace + Model row to the README Group-By Strategies table so the keybinding description and the table agree.
junhoyeo
force-pushed
the
feat/group-by-session
branch
from
May 24, 2026 14:57
422d520 to
b8f85f5
Compare
3 tasks
Owner
|
@carlosflorencio this has been merged to v3.0.0: https://github.com/junhoyeo/tokscale/releases/tag/v3.0.0 thanks for the contribution! |
Contributor
Author
|
Thanks @junhoyeo ! |
Closed
8 tasks
This was referenced Jul 6, 2026
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
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.
Why
Tokscale already extracts
session_idduring parsing for dedup; this PR surfaces it as a group-by strategy on the CLI + JSON output so kandev can spawntokscale --json --group-by client,session,modelonce and join each row against its own session id.Downstream consumers - for example kdlbs/kandev, a multi-agent IDE runner - need to attribute cost to specific agent-CLI sessions, not just to dates or model rollups. Which this grouping we can match ACP sessions with their cost via tokscale.
What changed
GroupBy::Session(session,model) andGroupBy::ClientSession(client,session,model) tocrates/tokscale-core/src/lib.rs:100, includingFromStr/Displayand aliases (session,session-model,client,session,client-session-model).session_idfield toModelUsagepopulated only for the new variants, and a newaggregate_by_session()+SessionContributionpair incrates/tokscale-core/src/aggregator.rs(mirrors the shape ofaggregate_by_date/DailyContribution, sorted bylast_seendesc).--group-by session,modeland--group-by client,session,modelnow emit a top-levelsessionIdkey per JSON row, and the text-mode tables render Client/Session/Provider/Model/Cost columns.gpicker exposes "Session + Model" and "Client + Session + Model" options.Output sample
tokscale models --json --group-by session,model --opencodeagainst the integration-test fixture:{ "groupBy": "session,model", "entries": [ { "client": "opencode", "mergedClients": null, "sessionId": "session1", "model": "claude-sonnet-4", "provider": "anthropic", "input": 1800, "output": 800, "cacheRead": 350, "cacheWrite": 80, "reasoning": 0, "messageCount": 2, "cost": 0.08 }, { "client": "opencode", "mergedClients": null, "sessionId": "session2", "model": "gpt-4o", "provider": "openai", "input": 600, "output": 200, "cacheRead": 100, "cacheWrite": 20, "reasoning": 0, "messageCount": 1, "cost": 0.02 } ] }The
sessionIdkey is omitted entirely (viaskip_serializing_if = Option::is_none) for every non-session group-by mode, so existing consumers see no schema change.Tests
aggregate_by_session()over 10 messages / 3 sessions, top-client tiebreaker, serde round-trip forSessionContribution— incrates/tokscale-core/src/aggregator.rs.aggregate_model_usage_entriesunderGroupBy::Session(merges across clients) andGroupBy::ClientSession(keeps clients separate), plus a defensive check that no other variant populatessession_id— incrates/tokscale-core/src/lib.rs.--group-by session,modeland--group-by client,session,modelagainst the OpenCode fixture, and a negative assertion intest_models_json_with_group_by_model— incrates/tokscale-cli/tests/cli_tests.rs.cargo fmt && cargo clippy --all-targets -- -D warnings && cargo testclean (1258 passed).Per-provider session parsers, pricing, scanner, and TUI layout were left untouched.
More improvements
session_idtoday is the rollout filename stem (rollout-<ts>-<uuid>), not the canonical UUID fromsession_meta.payload.id. Worth normalizing in a follow-up so the value matches the ACP session new-response hash.inputcurrently includes cached input tokens. For session attribution it can be useful to splittotalInputvsnonCachedInput, or just document the convention in the JSON schema. Applies to every group-by mode; happy to open a separate issue.Will open follow up PRs for these if its okay.