Add local portfolio snapshot history#27
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an optional, local-only (gitignored) portfolio snapshot history feature to Trading Lab, letting users record daily portfolio/account/order/decision context into a CSV for later analysis (plots, drift, outcomes) without broker/API access.
Changes:
- Introduces
trading_lab.portfolio.snapshotsto build/append/read/format snapshot rows indata/processed/portfolio/snapshots.csv. - Extends CLI (
tl portfolio snapshot|snapshotsandtl decide --snapshot) and GUI (new “Portfolio snapshots” card + POST/snapshot) to record/view snapshots. - Adds tests covering snapshot row generation, append/read behavior, CLI integration, and GUI integration; updates local portfolio docs with snapshot usage.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_portfolio_snapshots.py | New unit/integration coverage for snapshot row creation, appending, CLI commands, and formatting. |
| tests/test_portfolio_gui.py | Ensures GUI renders snapshot section and can record a snapshot via form action. |
| src/trading_lab/portfolio/snapshots.py | New snapshot implementation: build row from local state/reports, append to CSV, read and format recent rows. |
| src/trading_lab/portfolio/gui_render.py | Adds snapshot card and recent snapshot table to the daily tab. |
| src/trading_lab/portfolio/gui_forms.py | Adds /snapshot form action to record snapshots from GUI. |
| src/trading_lab/cli/main.py | Adds snapshot-related CLI commands and optional snapshot recording from tl decide. |
| docs/local_portfolio.md | Documents snapshot history concept and CLI usage examples. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _number(value: float | int | None) -> str: | ||
| if value is None: | ||
| return "" | ||
| return f"{float(value):.6g}" |
| "active_target_mode": summary.get("active_target_mode", ""), | ||
| "active_target_column": summary.get("active_target_column", ""), | ||
| "current_traded_price": _number( | ||
| item.latest_price if item is not None else _float_or_none(summary.get("traded_price")) |
| path.parent.mkdir(parents=True, exist_ok=True) | ||
| write_header = not path.exists() or path.stat().st_size == 0 | ||
| with path.open("a", newline="", encoding="utf-8") as handle: | ||
| writer = csv.DictWriter(handle, fieldnames=list(SNAPSHOT_COLUMNS), extrasaction="ignore") | ||
| if write_header: | ||
| writer.writeheader() | ||
| writer.writerow(row) | ||
| return SnapshotResult(path=path, row=row) |
| mode = normalize_risk_mode(risk_mode) | ||
| timestamp = (now or datetime.now()).isoformat(timespec="seconds") | ||
| config = load_trading_config() |
| "pending_sell_count": str(len(placed_sells)), | ||
| "model_probability": selected_signal.get("probability", ""), | ||
| "active_target_mode": summary.get("active_target_mode", ""), | ||
| "active_target_column": summary.get("active_target_column", ""), |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ce719df21
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| result = append_snapshot( | ||
| risk_mode=args.risk_mode, | ||
| notes=args.snapshot_notes, | ||
| account_value=args.account_value, | ||
| cash=args.cash, |
There was a problem hiding this comment.
When tl decide is run with --position ... --snapshot, the decision text is computed using the CLI position overrides, but the snapshot is appended from local CSV state only because no position overrides are forwarded. This can record a different action, traded_quantity, and exposure than the decision the user just reviewed, which makes snapshot history inaccurate for outcome analysis and what-if runs.
Useful? React with 👍 / 👎.
Adds local gitignored portfolio snapshot history for account, exposure, pending order, decision, and model state. Closes #19.