Add --tag and --by-tag filtering to burn summary#369
Merged
willwashburn merged 1 commit intomainfrom May 8, 2026
Merged
Conversation
`--tag k=v` filters turns by stamped enrichment values (multiple flags
AND together; rides alongside the existing `--workflow` sugar). `--by-tag
<key>` adds a new mode that groups rows by an enrichment value, with
turns missing the key bucketed as `(unset)`. JSON output emits
`byTag: { key, rows: [...] }` and the per-cell fidelity block uses
`groupBy: 'tag'`.
https://claude.ai/code/session_01TSeLhFCGujBx73mw2DpMt3
📝 WalkthroughWalkthroughThis PR adds tag-based filtering and grouping capabilities to the ChangesTag Filtering and Grouping for Burn Summary
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/cli/src/commands/summary.test.ts (1)
1416-1420: ⚡ Quick winAdd an explicit empty-key test case (
--by-tag=) to lock validation.Current missing-key test covers boolean form (
--by-tag), but not empty-string form. Adding it prevents regressions if parser shape changes.✅ Suggested test addition
it('--by-tag without a key exits non-zero', async () => { const out = await captureSummary({ 'by-tag': true }); assert.equal(out.code, 2); assert.match(out.stderr, /--by-tag requires a tag key/); }); + + it('--by-tag= (empty key) exits non-zero', async () => { + const out = await captureSummary({ 'by-tag': '' }); + assert.equal(out.code, 2); + assert.match(out.stderr, /--by-tag requires a tag key/); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/commands/summary.test.ts` around lines 1416 - 1420, Add a new test case that calls captureSummary with the explicit empty key form (e.g., {'by-tag': ''} or invoking the CLI with '--by-tag=') to ensure the parser rejects an empty tag key; it should mirror the existing test that uses the boolean form by asserting out.code is 2 and out.stderr matches /--by-tag requires a tag key/. Place the new case alongside the existing "it('--by-tag without a key exits non-zero')" test so it validates the empty-string form and prevents regressions in captureSummary and the CLI option parsing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/src/commands/summary.ts`:
- Around line 73-78: The check only rejects the boolean form of --by-tag but
allows an empty string (--by-tag=) to pass; update the handling around
byTagFlag/byTagKey so that byTagKey is set only when byTagFlag is a non-empty
string, and treat an empty string the same as the boolean true: write the same
error to stderr and return 2. Specifically, change the logic that assigns
byTagKey and the subsequent if (byTagFlag === true) branch to detect typeof
byTagFlag === 'string' && byTagFlag.length > 0 for a valid key, and otherwise
print "burn: --by-tag requires a tag key..." and return 2 when byTagFlag is true
or an empty string.
---
Nitpick comments:
In `@packages/cli/src/commands/summary.test.ts`:
- Around line 1416-1420: Add a new test case that calls captureSummary with the
explicit empty key form (e.g., {'by-tag': ''} or invoking the CLI with
'--by-tag=') to ensure the parser rejects an empty tag key; it should mirror the
existing test that uses the boolean form by asserting out.code is 2 and
out.stderr matches /--by-tag requires a tag key/. Place the new case alongside
the existing "it('--by-tag without a key exits non-zero')" test so it validates
the empty-string form and prevents regressions in captureSummary and the CLI
option parsing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 27db3f47-e126-4a25-9a32-ad4e2158f278
📒 Files selected for processing (4)
packages/cli/CHANGELOG.mdpackages/cli/src/cli.tspackages/cli/src/commands/summary.test.tspackages/cli/src/commands/summary.ts
Comment on lines
+73
to
+78
| const byTagFlag = args.flags['by-tag']; | ||
| const byTagKey = typeof byTagFlag === 'string' ? byTagFlag : undefined; | ||
| if (byTagFlag === true) { | ||
| process.stderr.write('burn: --by-tag requires a tag key (e.g. --by-tag workflow)\n'); | ||
| return 2; | ||
| } |
There was a problem hiding this comment.
Reject empty --by-tag keys, not just bare boolean form.
Line 75 only catches --by-tag (boolean). --by-tag= (empty string) can slip through and should also return code 2 per the command contract.
🔧 Suggested fix
const byTagFlag = args.flags['by-tag'];
- const byTagKey = typeof byTagFlag === 'string' ? byTagFlag : undefined;
- if (byTagFlag === true) {
+ const byTagKey = typeof byTagFlag === 'string' ? byTagFlag.trim() : undefined;
+ if (byTagFlag === true || (typeof byTagFlag === 'string' && byTagKey.length === 0)) {
process.stderr.write('burn: --by-tag requires a tag key (e.g. --by-tag workflow)\n');
return 2;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const byTagFlag = args.flags['by-tag']; | |
| const byTagKey = typeof byTagFlag === 'string' ? byTagFlag : undefined; | |
| if (byTagFlag === true) { | |
| process.stderr.write('burn: --by-tag requires a tag key (e.g. --by-tag workflow)\n'); | |
| return 2; | |
| } | |
| const byTagFlag = args.flags['by-tag']; | |
| const byTagKey = typeof byTagFlag === 'string' ? byTagFlag.trim() : undefined; | |
| if (byTagFlag === true || (typeof byTagFlag === 'string' && byTagKey.length === 0)) { | |
| process.stderr.write('burn: --by-tag requires a tag key (e.g. --by-tag workflow)\n'); | |
| return 2; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/commands/summary.ts` around lines 73 - 78, The check only
rejects the boolean form of --by-tag but allows an empty string (--by-tag=) to
pass; update the handling around byTagFlag/byTagKey so that byTagKey is set only
when byTagFlag is a non-empty string, and treat an empty string the same as the
boolean true: write the same error to stderr and return 2. Specifically, change
the logic that assigns byTagKey and the subsequent if (byTagFlag === true)
branch to detect typeof byTagFlag === 'string' && byTagFlag.length > 0 for a
valid key, and otherwise print "burn: --by-tag requires a tag key..." and return
2 when byTagFlag is true or an empty string.
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.
Summary
burn summarycouldn't slice or compare by the user-supplied tags written byburn run --tag k=v. This adds two surface changes that reuse the existing stamp/enrichment plumbing:--tag k=v(repeatable, AND-combined) narrows turns to the stamped enrichment value. Rides alongside the existing--workflowsugar; both land inq.enrichmentso the ledger walk and SQLite archive both filter at query time.--by-tag <key>is a new mode flag that groups rows by an enrichment value the same way--by-provider/--by-modelgroup today. Turns missing the key bucket as(unset)so coverage isn't silently dropped.JSON output emits
byTag: { key, rows: [{ value, turns, usage, cost }] }and the per-cell fidelity block usesgroupBy: 'tag'.--by-tagis mutually exclusive with the other--by-*modes; combining them or passing it without a key exits non-zero.Example:
Test plan
pnpm run build— cleanpnpm -w run test— 875/875 pass, including 5 new--tag/--by-tagcases (filter narrowing, group-by-value with(unset)bucket, missing-key error, mode-exclusivity error, text headertag:<key>)burn summary --by-tag workflowagainst a ledger with mixed--tag workflow=...stampsNotes
@relayburn/sdk@1.x) is intentionally untouched — it's in maintenance-only mode per the existing CHANGELOG. New programmatic surface for tags would land in@relayburn/sdk@2.x(Rust napi) once that path opens.Enrichmentis alreadyRecord<string,string>and the archive'sjson_extract(enrichment_json, ...)filter already covers arbitrary keys.https://claude.ai/code/session_01TSeLhFCGujBx73mw2DpMt3
Generated by Claude Code