Skip to content

feat(pnl): group by symbol or magic with uniform group/group_by output - #8

Merged
tanem merged 12 commits into
mainfrom
feat/cli-phase-3-pnl-grouping
Jun 18, 2026
Merged

feat(pnl): group by symbol or magic with uniform group/group_by output#8
tanem merged 12 commits into
mainfrom
feat/cli-phase-3-pnl-grouping

Conversation

@tanem

@tanem tanem commented Jun 17, 2026

Copy link
Copy Markdown
Owner

Summary

Phase 3 (part 1) of the read-views-and-reporting expansion (plan; spec sections 3.1/3.2 plus parts of 1.2/1.3). It generalises the pnl grouping dimension.

--by now accepts symbol and magic alongside day|week|month. Time cuts are unchanged — per-account rows plus a combined ALL row per period. symbol and magic instead aggregate across all in-scope accounts, one row per symbol or magic number, with no per-account or ALL row; the first table column becomes SYMBOL/MAGIC and the ACCOUNT column is dropped. Magic groups sort numerically.

Every cut now emits a uniform shape: JSON/CSV rows carry group (the period date, symbol, or magic) and group_by (the --by value), replacing the old period field — a deliberate pre-1.0 break. account is the login for per-account time rows and null/empty for the combined time row and for every symbol/magic row.

Because a symbol/magic cut sums across accounts, it refuses (stderr, exit 1, not silenced by --quiet) when the in-scope accounts span more than one currency — narrow --accounts to a single currency. Internally, aggregate.Row.Period is renamed to Row.Group, the summary is accumulated per-deal so totals are independent of the grouping path, and a new aggregate.AccountsInScope exposes the contributing logins for the currency guard. README and CLAUDE.md are updated.

Built with subagent-driven development: a fresh implementer per task, an independent review after each, and an opus whole-branch review at the end (verdict: ready to merge, no Critical/Important findings).

Test Plan

  • go test ./... passes
  • CI green across ubuntu/macos/windows (incl. -race) and the lint job
  • go vet and staticcheck clean locally
  • Time-cut pnl_table.golden byte-identical to main (no regression to existing output)
  • New coverage: symbol grouping, magic numeric sort, AccountsInScope range filtering, mixed-currency refusal, empty account columns, dropped ACCOUNT column, new pnl_table_symbol.golden
  • pnl --by symbol/magic --format json|csv carries group/group_by and omits per-account/ALL rows (asserted by TestPnLByMagicJSONGroupBy, TestPnLCSVSymbolEmptyAccountColumns)

🤖 Generated with Claude Code

tanem and others added 9 commits June 18, 2026 05:28
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…agic cuts

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…on cuts

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment/usage-string only; no behaviour change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@codecov

codecov Bot commented Jun 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.91837% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.39%. Comparing base (a0781b7) to head (e799b18).

Files with missing lines Patch % Lines
internal/render/render.go 90.69% 4 Missing ⚠️
internal/aggregate/aggregate.go 97.59% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main       #8      +/-   ##
==========================================
+ Coverage   89.14%   89.39%   +0.24%     
==========================================
  Files          13       13              
  Lines         783      858      +75     
==========================================
+ Hits          698      767      +69     
- Misses         71       76       +5     
- Partials       14       15       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

tanem and others added 3 commits June 18, 2026 06:30
Split the pnl Commands wall-of-text into a one-line-per-flag reference
and a separate Notes section (dates/bucketing, P&L components, max
drawdown, mixed currencies, output shape); drop the bold mini-headings.
No behaviour change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Now covered globally for all repos via the user's global writing-style
rule, so the project-level note is no longer needed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Remove the "just works" phrasing from the "One file in, answers out."
bullet; state the snapshot env var/flag and the command plainly instead.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@tanem
tanem force-pushed the feat/cli-phase-3-pnl-grouping branch from 06201ec to e799b18 Compare June 17, 2026 18:42
@tanem
tanem merged commit f1abd15 into main Jun 18, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant