Skip to content

feat(usage): group usage by branch and make branches excludable#979

Open
prateek wants to merge 1 commit into
kenn-io:mainfrom
prateek:split-branch-usage-grouping
Open

feat(usage): group usage by branch and make branches excludable#979
prateek wants to merge 1 commit into
kenn-io:mainfrom
prateek:split-branch-usage-grouping

Conversation

@prateek

@prateek prateek commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Adds branch grouping to the usage view, building on the branch filter foundation. Usage summaries now return range-wide branchTotals and per-day branchBreakdowns for each (project, branch) pair across SQLite, PostgreSQL, and DuckDB. The cost-over-time chart and Attribution panel get a Branch group-by option with human project/branch labels.

Usage cost chart and attribution grouped by branch (synthetic data)

Branch can now be excluded like Project, Agent, and Model. Clicking a branch tile or row hides that branch across usage panels; the toolbar shows Branch: N hidden with unhide controls, and exclude_git_branch is stored in the URL and localStorage so reloads and prior-period comparisons keep the filter.

One branch hidden, "Branch: 1 hidden" dropdown open (synthetic data)

For reviewers, the branch grouping is intentionally implemented at the usage-view layer rather than as a shared grouping primitive. The exclude filter flows through HTTP, service, and backend usage filters as a NOT-wrapped composite-pair predicate; empty or malformed token lists exclude nothing, and cursor-usage rows are bypassed when branch filters are active. The usage store fetches branch options from /branches, so the usage page does not depend on session-page state.

@roborev-ci

roborev-ci Bot commented Jul 3, 2026

Copy link
Copy Markdown

roborev: Combined Review (67d63e2)

The PR has one medium-severity issue; no critical or high findings were reported.

Medium

  • Location: internal/service/http.go:429 and internal/service/http.go:486
  • Problem: The Go HTTP backend does not forward UsageRequest.ExcludeGitBranch to /api/v1/usage/summary or /api/v1/usage/pairwise-comparison, so daemon-backed clients ignore branch exclusions while the direct backend applies them.
  • Fix: Add "exclude_git_branch": req.ExcludeGitBranch to both query-param maps and cover it in the HTTP backend roundtrip tests.

Reviewers: 2 done | Synthesis: codex, 7s | Total: 5m37s

@prateek prateek force-pushed the split-branch-usage-grouping branch from 67d63e2 to 859a841 Compare July 3, 2026 07:54
@prateek

prateek commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the automated review finding: the HTTP service backend was dropping exclude_git_branch when serializing usage requests, so daemon-mode CLI clients would silently ignore branch exclusions. Both the summary and pairwise query maps now carry it, with serialization tests for each path.

@roborev-ci

roborev-ci Bot commented Jul 3, 2026

Copy link
Copy Markdown

roborev: Combined Review (859a841)

The PR has one medium-severity issue to fix before merging.

Medium

  • frontend/src/lib/stores/usage.test.ts:140
    The mock for ../api/generated/index only exports UsageService, but usage.svelte.ts now also imports MetadataService. Vitest will fail module loading for tests that import this store with the mocked generated client.
    Fix: Add a MetadataService mock with getApiV1Branches, or use a partial mock based on importOriginal.

Reviewers: 2 done | Synthesis: codex, 6s | Total: 7m43s

@prateek prateek force-pushed the split-branch-usage-grouping branch from 859a841 to cbbabcf Compare July 3, 2026 08:22
@prateek

prateek commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the round-2 finding: the generated-client mock in usage.test.ts now includes MetadataService.getApiV1Branches (the store's new import), with a test covering loadBranches() caching. For the record, the previous state didn't fail module loading — vitest resolves a missing named export from a mocked module as undefined — but any test exercising loadBranches() would have crashed, so the mock gap was real.

@roborev-ci

roborev-ci Bot commented Jul 3, 2026

Copy link
Copy Markdown

roborev: Combined Review (cbbabcf)

Summary verdict: one medium-severity issue should be fixed before merge.

Medium

  • frontend/src/lib/components/usage/UsagePage.svelte:438
    • The branch dropdown converts the branch exclusion list from BRANCH_LIST_SEP to comma-separated text before passing it into FilterDropdown, but branch tokens can legally contain commas. FilterDropdown then splits on commas, so an excluded branch like proj\u001ffeat,one is misread as two filters and the UI shows the wrong checked/visible state.
    • Fix: Let FilterDropdown accept a separator prop, or pass excluded item names as an array/set. Use BRANCH_LIST_SEP directly for branch filters instead of adapting through commas.

Reviewers: 2 done | Synthesis: codex, 7s | Total: 7m33s

Groups usage and cost by (project, branch): range-wide branchTotals and
per-day branch breakdowns across SQLite, Postgres, and DuckDB, the
service layer, and HTTP; a Branch group-by toggle on the cost-over-time
chart and the Attribution panel with human project/branch labels; and
the per-dimension accumulate boilerplate collapsed into one generic
accumulator, matching the pattern DuckDB's usage code already used.

Branches are also excludable like every other dimension: clicking a
branch tile or row hides it from all usage panels, a "Branch: N hidden"
toolbar dropdown reviews and un-hides, exclusions persist in the URL
and localStorage, and the prior-period comparison respects them. Under
the hood, exclude_git_branch travels through HTTP, service, and all
three backends via a NOT-wrapped composite-pair predicate (empty or
malformed token lists exclude nothing) with the same cursor-usage
bypass handling as the include filter. The usage store fetches its own
branch list from the /branches endpoint, so the page works standalone.
@prateek prateek force-pushed the split-branch-usage-grouping branch from cbbabcf to 3e88a36 Compare July 3, 2026 09:10
@prateek

prateek commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the round-3 finding: FilterDropdown now takes a separator prop (default ,), and the Branch dropdown passes BRANCH_LIST_SEP directly instead of adapting the token list through commas — a comma inside a project or branch name no longer mis-splits the checked state. Covered by a component test with a comma-containing token.

@prateek prateek marked this pull request as ready for review July 3, 2026 09:12
@roborev-ci

roborev-ci Bot commented Jul 3, 2026

Copy link
Copy Markdown

roborev: Combined Review (3e88a36)

Reviewed branch usage grouping/exclusion changes: no Medium, High, or Critical findings to report.

All reported issues were Low severity and omitted per instruction.


Reviewers: 2 done | Synthesis: codex, 5s | Total: 8m58s

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant