From d7a71708efb5b0693e03d69883630981649d1431 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Fri, 24 Jul 2026 19:10:13 -0400 Subject: [PATCH 1/2] feat(insights): add keel insights summary + journal read-only reporting Adds a strictly read-only `keel insights` command group -- `summary` (per-rule promotion-gate distance projected off `paper.track_record`/`strategy.promotion`, plus an account-level view of `gather_status`) and `journal` (a filterable, fee-honest trade journal off `trade_outcomes`, enriched with R-multiples where a matching paper trade can be found). Both support `--json` (no trailing prose) and the shared DISCLAIMER footer in human mode. Removes the unused Phase-4 `insights` stub command that this supersedes. Co-Authored-By: Claude Opus 4.8 --- keel/cli.py | 22 +- keel/commands/insights.py | 655 ++++++++++++++++++++++++++++++++ tests/commands/test_insights.py | 651 +++++++++++++++++++++++++++++++ tests/test_cli.py | 10 +- 4 files changed, 1322 insertions(+), 16 deletions(-) create mode 100644 keel/commands/insights.py create mode 100644 tests/commands/test_insights.py diff --git a/keel/cli.py b/keel/cli.py index 399c6152..9b708f5a 100644 --- a/keel/cli.py +++ b/keel/cli.py @@ -12,7 +12,8 @@ `subscription attest|set|show` (the per-venue, user-attested allowance rail 14 reads live), `status` (`commands.status`: the read-only, no-broker operator dashboard the paper-mode-fidelity spec deferred -- mode/kill-switch/autonomy/Rail 11/positions/rules/data freshness, plus `--json`), -and a Phase-4 `insights` stub. +and `insights summary|journal` (`commands.insights`: a read-only VIEW over the same substrate -- +per-rule promotion-gate distance and a filterable trade journal, also with `--json`). **Dangerous commands ask a human; nothing needs a stored secret.** The former scrypt passphrase gate is gone (see `2026-07-21-security-simplification-design.md`). Five commands re-permit trading @@ -79,6 +80,7 @@ from keel.commands._products import _default_sim_products, _history_product from keel.commands.autonomy import autonomy_group from keel.commands.db import db_group +from keel.commands.insights import insights_group from keel.commands.rules import rules_group, rules_seed from keel.commands.status import status_cmd from keel.commands.subscription import subscription_group @@ -1647,6 +1649,13 @@ def simulate( cli.add_command(tui_cmd) +# -- insights (read-only promotion-gate + journal reporting, no broker call) -------------------- + +# A pure VIEW over `gather_status`/`StatusReport`, the repository read methods, and the +# promotion/track-record machinery -- defined in `keel.commands.insights` and registered here. +cli.add_command(insights_group) + + # -- kill / resume ------------------------------------------------------------------------------ @@ -1780,16 +1789,5 @@ def reset_hwm(ctx: click.Context) -> None: click.echo("equity high-water mark reset: it will re-seed from the next cycle's equity.") -# -- insights (Phase 4 stub) --------------------------------------------------------------------- - - -@cli.command() -@click.pass_context -@with_disclaimer -def insights(ctx: click.Context) -> None: - """Portfolio insights (Phase 4 -- not yet implemented).""" - click.echo("insights: not yet implemented (see Phase 4 of the roadmap).") - - if __name__ == "__main__": # pragma: no cover cli() diff --git a/keel/commands/insights.py b/keel/commands/insights.py new file mode 100644 index 00000000..c9402d03 --- /dev/null +++ b/keel/commands/insights.py @@ -0,0 +1,655 @@ +"""`keel insights` -- a READ-ONLY reporting surface over the paper-forward's track record. + +Two subcommands: + +- `keel insights summary` -- per-rule promotion-gate distance (how close is a `paper`-status + rule to clearing its floor) plus an account-level projection of `keel status`'s own report. +- `keel insights journal` -- a chronological, filterable trade journal built off the fee-honest + `trade_outcomes` ledger, enriched with R-multiples from the paper track record where a match + can be found. + +**Strictly read-only.** This module places no orders, touches no rails/guards, and adds no new +`Repository` write method -- it is a pure VIEW over `gather_status`/`StatusReport` +(`keel.commands.status`), `Repository`'s existing read methods, `paper.track_record`, and the +promotion machinery (`keel.strategy.promotion`). It re-derives nothing rail11/drawdown/floor- +related; every such value is projected verbatim from an existing output, exactly like `keel tui` +does off `gather_status`. + +Two layers, matching `status.py`/`tui.py`: + +- Pure builders (`build_*`) are functions of `(Repository, Config, StatusReport, now_ts, ...)` -> + a frozen report dataclass. No click dependency, directly unit-testable. +- The `insights` click group opens repo/config via the standard `_open_repo`/`_load_cfg` seams + (same as every other command), calls the builders, and renders -- either `click.echo` lines + (default, ending in the shared `DISCLAIMER` footer) or `--json` (no trailing prose, so + `json.loads(output)` always succeeds). + +**NAMING CAUTION:** there is an unrelated `journal` TABLE in `keel/db.py` (a manual discipline +diary: emotion_score, rules_followed, etc.) with no accessor. `keel insights journal` never reads +or writes it -- it is built entirely off `trade_outcomes` (see `Repository.get_trade_outcomes`). + +**On `--mode`:** `paper.track_record` is inherently paper-mode/R-aware regardless of a rule's +current lifecycle `status` (it just replays `orders(mode='paper')` for a given rule name), so +`--mode` does not change *how* stats are computed -- it selects *which* rules the report scopes +to: `paper` (the default) shows rules still in the proving pipeline (`candidate`/`paper` status, +i.e. "how close to promotion"); `live` shows rules already promoted and trading (`live` status, +i.e. "how is the live edge holding up"). Every rule falls into exactly one bucket. +""" + +from __future__ import annotations + +import json +import time +from dataclasses import asdict, dataclass +from datetime import UTC, datetime +from decimal import Decimal +from typing import Any + +import click + +from keel import agent as agent_mod +from keel.commands._common import DISCLAIMER, _load_cfg, _open_repo +from keel.commands.status import StatusReport, _human_age, gather_status +from keel.commands.tui import _human_dt +from keel.config import Config +from keel.data.repository import Repository +from keel.strategy.paper import track_record +from keel.strategy.promotion import ( + PromotionConfig, + can_promote, + floor_for_class, + promotion_class_of, +) +from keel.strategy.rules.base import Rule + +# -- the pure report shapes ----------------------------------------------------------------- + + +@dataclass(frozen=True) +class JournalEntry: + closed_at: int | None + opened_at: int + rule_name: str | None + product_id: str + qty: Decimal + entry_fill: Decimal + exit_fill: Decimal | None + pnl_net: Decimal | None + fees: Decimal | None + r_multiple: Decimal | None + is_dca: bool + outcome: str + + +@dataclass(frozen=True) +class JournalReport: + now_ts: int + mode: str + entries: list[JournalEntry] + total_count: int + """The full filtered (closed) entry count BEFORE `--limit` truncates for display.""" + filters: dict[str, Any] + + +@dataclass(frozen=True) +class GateDistance: + rule_name: str + promotion_class: str + n_trades: int + min_trades: int + trades_remaining: int + win_rate: float + min_win_rate: float + realized_rr: Decimal | None + min_rr: Decimal + expectancy: Decimal + min_expectancy: Decimal + passing: bool + blocking_reasons: list[str] + + +@dataclass(frozen=True) +class RuleTrackRecord: + rule_name: str + status: str + promotion_class: str + n_trades: int + win_rate: float + avg_win: Decimal + avg_loss: Decimal + realized_rr: Decimal | None + expectancy: Decimal + profit_factor: Decimal + max_drawdown: Decimal + significant: bool + """`n_trades >= 30` -- the sample-size floor below which win-rate/expectancy are not yet + statistically distinguishable from random entry (see `strategy.promotion`'s own KB citation). + """ + gate: GateDistance | None + """`None` for any status other than `paper` -- a `candidate` hasn't backtested yet and a + `live`/`disabled` rule has already cleared (or been pulled from) the gate this measures.""" + + +@dataclass(frozen=True) +class AccountSummary: + mode: str + equity_state_mode: str | None + high_water_mark: Decimal | None + drawdown_total_pct: Decimal | None + drawdown_weekly_pct: Decimal | None + max_total_dd_pct: Decimal + max_weekly_dd_pct: Decimal + rail11_status: str + paper_cash_usdc: Decimal | None + + +@dataclass(frozen=True) +class InsightsReport: + now_ts: int + account: AccountSummary + rules: list[RuleTrackRecord] + closed_trade_count: int + + +# -- pure builders -------------------------------------------------------------------------- + + +def build_account_summary(report: StatusReport) -> AccountSummary: + """Pure projection off `StatusReport` -- never re-derives rail11/drawdown, just copies the + fields `gather_status` already computed.""" + return AccountSummary( + mode=report.mode, + equity_state_mode=report.equity_state_mode, + high_water_mark=report.high_water_mark, + drawdown_total_pct=report.drawdown_total_pct, + drawdown_weekly_pct=report.drawdown_weekly_pct, + max_total_dd_pct=report.max_total_dd_pct, + max_weekly_dd_pct=report.max_weekly_dd_pct, + rail11_status=report.rail11_status, + paper_cash_usdc=report.paper_cash_usdc, + ) + + +def _realized_rr_for_display(stats: Any) -> Decimal | None: + """`avg_win / |avg_loss|`, or `None` when there are no losing trades yet. + + Deliberately diverges from `strategy.promotion._realized_rr` (which returns `Infinity` in + that case, because a promotion-gate check needs a comparable scalar). This is a display + value, not a gate decision -- `None` ("no losses yet, nothing to measure against") is the + honest reading, not a real-looking `Infinity`. + """ + if stats.avg_loss == 0: + return None + return stats.avg_win / abs(stats.avg_loss) + + +def build_gate_distance( + rule: Rule, stats: Any, default_floor: PromotionConfig +) -> GateDistance: + """How far `stats` is from `rule`'s promotion floor (its class's floor, or `default_floor` + for classes with no code-defined override).""" + promotion_class = promotion_class_of(rule) + floor = floor_for_class(promotion_class, default=default_floor) + passing, reasons = can_promote(stats, floor) + return GateDistance( + rule_name=rule.name, + promotion_class=promotion_class, + n_trades=stats.n_trades, + min_trades=floor.min_trades, + trades_remaining=max(0, floor.min_trades - stats.n_trades), + win_rate=stats.win_rate, + min_win_rate=floor.min_win_rate, + realized_rr=_realized_rr_for_display(stats), + min_rr=floor.min_rr, + expectancy=stats.expectancy, + min_expectancy=floor.min_expectancy, + passing=passing, + blocking_reasons=reasons, + ) + + +def build_rule_track_record( + row: dict[str, Any], stats: Any, default_floor: PromotionConfig +) -> RuleTrackRecord: + """Aggregate `row` (a `repo.get_rules()` row) + its paper `stats` into a `RuleTrackRecord`. + + Degrades gracefully -- promotion_class="default", gate=None -- when `row["kind"]` is no + longer in `RULE_REGISTRY` (`agent._build_rule` raises `ValueError` for that), rather than + crashing the whole report over one stale row. + """ + try: + rule = agent_mod._build_rule(row) + promotion_class = promotion_class_of(rule) + except ValueError: + rule = None + promotion_class = "default" + + gate: GateDistance | None = None + if row["status"] == "paper" and rule is not None: + gate = build_gate_distance(rule, stats, default_floor) + + return RuleTrackRecord( + rule_name=row["kind"], + status=row["status"], + promotion_class=promotion_class, + n_trades=stats.n_trades, + win_rate=stats.win_rate, + avg_win=stats.avg_win, + avg_loss=stats.avg_loss, + realized_rr=_realized_rr_for_display(stats), + expectancy=stats.expectancy, + profit_factor=stats.profit_factor, + max_drawdown=stats.max_drawdown, + significant=stats.n_trades >= 30, + gate=gate, + ) + + +_PAPER_PIPELINE_STATUSES = {"candidate", "paper"} + + +def build_insights_report( + repo: Repository, + config: Config, + status_report: StatusReport, + now_ts: int, + *, + mode: str = "paper", + rule_filter: str | None = None, +) -> InsightsReport: + """Assemble the full `InsightsReport` -- read-only, no re-derivation of anything + `gather_status` already computed.""" + default_floor = PromotionConfig( + min_trades=config.promotion.min_trades, + min_expectancy=config.promotion.min_expectancy, + min_rr=config.promotion.min_rr, + min_win_rate=float(config.promotion.min_win_rate), + ) + + rows = repo.get_rules() + if rule_filter is not None: + rows = [r for r in rows if r["kind"] == rule_filter] + if mode == "live": + rows = [r for r in rows if r["status"] == "live"] + else: + rows = [r for r in rows if r["status"] in _PAPER_PIPELINE_STATUSES] + + rules = [ + build_rule_track_record(row, track_record(repo, row["kind"]), default_floor) + for row in rows + ] + + return InsightsReport( + now_ts=now_ts, + account=build_account_summary(status_report), + rules=rules, + closed_trade_count=len(repo.get_trade_outcomes()), + ) + + +_TS_MATCH_TOLERANCE_SEC = 1 +"""Trade-outcome timestamps and paper-trade timestamps come from the same bar clock in +practice, but are matched with a small tolerance rather than exact equality -- robust to any +off-by-a-second rounding between the two write paths without risking a false cross-rule match +(a wrong-rule collision on the exact same instant is vanishingly unlikely).""" + + +def _approx(a: int, b: int) -> bool: + return abs(a - b) <= _TS_MATCH_TOLERANCE_SEC + + +def _match_trade(entries_for_rule: list[Any], opened_at: int, closed_at: int) -> Any | None: + for trade in entries_for_rule: + if trade.exit_ts is None: + continue + if _approx(trade.entry_ts, opened_at) and _approx(trade.exit_ts, closed_at): + return trade + return None + + +def _journal_entry_from_outcome( + row: dict[str, Any], trades_by_rule: dict[str, list[Any]], repo: Repository +) -> JournalEntry: + if row["is_dca"]: + r_multiple = None + outcome = "dca" + else: + r_multiple = None + outcome = None + rule_name = row["rule_name"] + if rule_name: + if rule_name not in trades_by_rule: + trades_by_rule[rule_name] = track_record(repo, rule_name).trades + matched = _match_trade(trades_by_rule[rule_name], row["opened_at"], row["closed_at"]) + if matched is not None: + r_multiple = matched.r_multiple + outcome = matched.outcome + if outcome is None: + pnl = row["pnl_net"] + outcome = "win" if pnl > 0 else "loss" if pnl < 0 else "scratch" + + return JournalEntry( + closed_at=row["closed_at"], + opened_at=row["opened_at"], + rule_name=row["rule_name"], + product_id=row["product_id"], + qty=row["qty"], + entry_fill=row["entry_fill"], + exit_fill=row["exit_fill"], + pnl_net=row["pnl_net"], + fees=row["fees"], + r_multiple=r_multiple, + is_dca=row["is_dca"], + outcome=outcome, + ) + + +def _journal_entry_from_open_position(pos: Any) -> JournalEntry: + return JournalEntry( + closed_at=None, + opened_at=pos.opened_at, + rule_name=pos.rule_name, + product_id=pos.product_id, + qty=pos.qty, + entry_fill=pos.entry_price, + exit_fill=None, + pnl_net=None, + fees=None, + r_multiple=None, + is_dca=False, + outcome="open", + ) + + +def build_journal_report( + repo: Repository, + status_report: StatusReport, + now_ts: int, + *, + rule: str | None = None, + asset: str | None = None, + since_ts: int | None = None, + until_ts: int | None = None, + limit: int | None = None, + include_open: bool = False, +) -> JournalReport: + """Build the chronological (oldest-first, internally) trade journal. + + Sourced from `repo.get_trade_outcomes(since_ts)` (the fee-honest, mode-agnostic, DCA-aware + ledger) rather than `paper.track_record` directly -- that keeps the journal meaningful in + `live` mode too, where there is no paper track record at all. Each row is enriched with + `r_multiple`/`outcome` by matching it to a paper `Trade` for the same rule on + `(opened_at≈entry_ts, closed_at≈exit_ts)`; on no match (or for a DCA row, which has no stop + to measure R against), `r_multiple` stays `None` and `outcome` falls back to a plain + pnl-sign read (DCA rows are labelled `"dca"` regardless of pnl sign). + + `total_count` is the full filtered (closed) count BEFORE `--limit` truncates; `--limit` + keeps the MOST RECENT `limit` entries (the tail of the oldest-first list) while staying + oldest-first internally -- `render_journal` is what reverses to most-recent-first for + display. + """ + outcomes = repo.get_trade_outcomes(since_ts=since_ts) + + filtered = [] + for row in outcomes: + if until_ts is not None and row["closed_at"] > until_ts: + continue + if rule is not None and row["rule_name"] != rule: + continue + if asset is not None and row["product_id"] != asset: + continue + filtered.append(row) + + trades_by_rule: dict[str, list[Any]] = {} + entries = [_journal_entry_from_outcome(row, trades_by_rule, repo) for row in filtered] + + total_count = len(entries) + if limit is not None: + entries = entries[-limit:] + + if include_open: + for pos in status_report.open_positions: + if rule is not None and pos.rule_name != rule: + continue + if asset is not None and pos.product_id != asset: + continue + entries.append(_journal_entry_from_open_position(pos)) + + filters = { + "rule": rule, + "asset": asset, + "since_ts": since_ts, + "until_ts": until_ts, + "limit": limit, + "include_open": include_open, + } + + return JournalReport( + now_ts=now_ts, + mode=status_report.mode, + entries=entries, + total_count=total_count, + filters=filters, + ) + + +# -- render (human-readable) -------------------------------------------------------------------- + +_SMALL_SAMPLE_NOTE = ( + "n<30: not yet statistically distinguishable from random entry -- do not read this as a " + "proven edge" +) + + +def render_summary(report: InsightsReport) -> list[str]: + """The `keel insights summary` (default, non-`--json`) rendering, as a list of lines.""" + lines: list[str] = [] + a = report.account + lines.append(f"mode: {a.mode}") + lines.append(f"equity_state_mode: {a.equity_state_mode or 'unknown'}") + hwm = a.high_water_mark if a.high_water_mark is not None else "unknown" + lines.append(f"high_water_mark: {hwm}") + dd_total = a.drawdown_total_pct if a.drawdown_total_pct is not None else "unknown" + dd_weekly = a.drawdown_weekly_pct if a.drawdown_weekly_pct is not None else "unknown" + lines.append( + f"drawdown: total={dd_total} (ceiling {a.max_total_dd_pct}) " + f"weekly={dd_weekly} (ceiling {a.max_weekly_dd_pct})" + ) + lines.append(f"rail11 (drawdown breaker): {a.rail11_status}") + if a.mode == "paper": + lines.append(f"paper_cash_usdc: {a.paper_cash_usdc}") + + lines.append("") + lines.append(f"closed trades (all rules, all time): {report.closed_trade_count}") + + lines.append("") + if not report.rules: + lines.append( + "no rule track record yet -- no rules seeded, or no closed paper trades in scope." + ) + return lines + + lines.append(f"rule track record ({len(report.rules)}):") + for r in report.rules: + rr = r.realized_rr if r.realized_rr is not None else "n/a (no losses yet)" + lines.append( + f" [{r.rule_name}] status={r.status} class={r.promotion_class} n={r.n_trades} " + f"win_rate={r.win_rate:.1%} avg_win={r.avg_win} avg_loss={r.avg_loss} rr={rr} " + f"expectancy={r.expectancy} profit_factor={r.profit_factor} max_dd={r.max_drawdown}" + ) + if not r.significant: + lines.append(f" {_SMALL_SAMPLE_NOTE}") + if r.gate is not None: + verdict = "PASSING" if r.gate.passing else "blocked" + lines.append( + f" gate: {verdict} -- trades_remaining={r.gate.trades_remaining} " + f"(n>={r.gate.min_trades}, win_rate>={r.gate.min_win_rate}, " + f"rr>={r.gate.min_rr}, expectancy>{r.gate.min_expectancy})" + ) + for reason in r.gate.blocking_reasons: + lines.append(f" - {reason}") + elif r.status == "paper": + lines.append(" gate: unavailable (rule kind not recognized -- stale row?)") + elif r.status == "candidate": + lines.append(" gate: needs a backtest pass before it has a paper track record") + + return lines + + +def render_journal(report: JournalReport) -> list[str]: + """The `keel insights journal` (default, non-`--json`) rendering, most-recent-first.""" + lines: list[str] = [] + lines.append(f"mode: {report.mode}") + active_filters = {k: v for k, v in report.filters.items() if v not in (None, False)} + filters_desc = ", ".join(f"{k}={v}" for k, v in active_filters.items()) or "none" + lines.append(f"filters: {filters_desc}") + lines.append(f"showing {len(report.entries)} of {report.total_count} closed trades") + lines.append("") + + if not report.entries: + lines.append("no closed trades yet.") + return lines + + for e in reversed(report.entries): + if e.outcome == "open": + lines.append( + f" OPEN {e.product_id} qty={e.qty} entry={e.entry_fill} " + f"opened_at={_human_dt(e.opened_at)} rule={e.rule_name}" + ) + elif e.is_dca: + age = _human_age(max(report.now_ts - (e.closed_at or report.now_ts), 0)) + lines.append( + f" [{_human_dt(e.closed_at or 0)} / {age}] {e.product_id} DCA qty={e.qty} " + f"pnl_net={e.pnl_net} -- DCA: no stop, excluded from R/expectancy" + ) + else: + r_text = e.r_multiple if e.r_multiple is not None else "n/a" + age = _human_age(max(report.now_ts - (e.closed_at or report.now_ts), 0)) + lines.append( + f" [{_human_dt(e.closed_at or 0)} / {age}] {e.product_id} rule={e.rule_name} " + f"outcome={e.outcome} qty={e.qty} pnl_net={e.pnl_net} R={r_text}" + ) + + return lines + + +# -- the commands --------------------------------------------------------------------------- + + +def _parse_ts(value: str) -> int: + """Accept either a unix timestamp or a bare `YYYY-MM-DD` date (read as UTC midnight).""" + try: + return int(value) + except ValueError: + pass + try: + dt = datetime.strptime(value, "%Y-%m-%d").replace(tzinfo=UTC) + return int(dt.timestamp()) + except ValueError as exc: + raise click.BadParameter( + f"invalid timestamp {value!r} -- expected a unix integer or YYYY-MM-DD" + ) from exc + + +@click.group("insights") +def insights_group() -> None: + """Read-only reporting: promotion-gate distance and a filterable trade journal. + + Purely a VIEW over the local DB (via `gather_status`, `Repository`'s read methods, and + `paper.track_record`) -- like `keel status`/`keel tui`, this never calls the broker and + never writes anything. + """ + + +@insights_group.command("summary") +@click.option("--json", "as_json", is_flag=True, default=False, help="Emit machine-readable JSON.") +@click.option("--rule", "rule_filter", default=None, help="Restrict to one rule kind.") +@click.option( + "--mode", + "mode", + type=click.Choice(["paper", "live"]), + default="paper", + help="paper: candidate/paper-status rules (promotion pipeline). live: live-status rules.", +) +@click.pass_context +def summary_cmd(ctx: click.Context, as_json: bool, rule_filter: str | None, mode: str) -> None: + """Per-rule promotion-gate distance + an account-level snapshot. + + `--json` skips the disclaimer footer (like `keel status --json`) so it stays a clean, + scriptable payload. + """ + repo = _open_repo(ctx) + config = _load_cfg(ctx) + now_ts = int(time.time()) + status_report = gather_status(repo, config, now_ts) + report = build_insights_report( + repo, config, status_report, now_ts, mode=mode, rule_filter=rule_filter + ) + + if as_json: + click.echo(json.dumps(asdict(report), indent=2, default=str)) + return + + for line in render_summary(report): + click.echo(line) + click.echo("") + click.echo(DISCLAIMER) + + +@insights_group.command("journal") +@click.option("--json", "as_json", is_flag=True, default=False, help="Emit machine-readable JSON.") +@click.option("--rule", "rule_filter", default=None, help="Restrict to one rule kind.") +@click.option("--asset", "asset_filter", default=None, help="Restrict to one product_id.") +@click.option("--limit", "limit", type=int, default=None, help="Cap rows shown (most recent).") +@click.option("--since", "since_raw", default=None, help="Unix timestamp or YYYY-MM-DD.") +@click.option("--until", "until_raw", default=None, help="Unix timestamp or YYYY-MM-DD.") +@click.option( + "--include-open", + "include_open", + is_flag=True, + default=False, + help="Append currently-open positions as outcome=open rows.", +) +@click.pass_context +def journal_cmd( + ctx: click.Context, + as_json: bool, + rule_filter: str | None, + asset_filter: str | None, + limit: int | None, + since_raw: str | None, + until_raw: str | None, + include_open: bool, +) -> None: + """A chronological, filterable trade journal off the fee-honest `trade_outcomes` ledger. + + `--json` skips the disclaimer footer (like `keel status --json`) so it stays a clean, + scriptable payload. + """ + repo = _open_repo(ctx) + config = _load_cfg(ctx) + now_ts = int(time.time()) + status_report = gather_status(repo, config, now_ts) + since_ts = _parse_ts(since_raw) if since_raw is not None else None + until_ts = _parse_ts(until_raw) if until_raw is not None else None + + report = build_journal_report( + repo, + status_report, + now_ts, + rule=rule_filter, + asset=asset_filter, + since_ts=since_ts, + until_ts=until_ts, + limit=limit, + include_open=include_open, + ) + + if as_json: + click.echo(json.dumps(asdict(report), indent=2, default=str)) + return + + for line in render_journal(report): + click.echo(line) + click.echo("") + click.echo(DISCLAIMER) diff --git a/tests/commands/test_insights.py b/tests/commands/test_insights.py new file mode 100644 index 00000000..65afcae4 --- /dev/null +++ b/tests/commands/test_insights.py @@ -0,0 +1,651 @@ +"""Tests for `keel insights` -- the read-only summary + journal reporting surface. + +Mirrors `tests/commands/test_status.py`'s split: pure builder functions get direct unit tests +(no CliRunner needed), renderers are tested as pure `list[str]` functions, and the click +commands themselves get one thin `CliRunner` pass each (human default + `--json`). + +This module is entirely READ-ONLY over the DB -- no test here ever asserts on a write path, +because `keel/commands/insights.py` has none. +""" + +from __future__ import annotations + +import json +from decimal import Decimal +from typing import Any + +import pytest +from click.testing import CliRunner + +from keel.cli import cli +from keel.commands.insights import ( + build_account_summary, + build_gate_distance, + build_insights_report, + build_journal_report, + build_rule_track_record, + render_journal, + render_summary, +) +from keel.commands.status import StatusReport, gather_status +from keel.config import ( + AutoTradeConfig, + Caps, + Config, + DcaConfig, + MarketDataConfig, + MoneyMgmtConfig, +) +from keel.data.db import connect, migrate +from keel.data.repository import Repository +from keel.strategy.paper import track_record +from keel.strategy.promotion import PromotionConfig, floor_for_class, promotion_class_of +from keel.strategy.rules.dca import Dca +from keel.strategy.rules.turtle_breakout import TurtleBreakout +from keel.types import Granularity + +NOW_TS = 1_800_000_000 + + +@pytest.fixture +def repo() -> Repository: + conn = connect(":memory:") + migrate(conn) + r = Repository(conn) + r.set_state("kill_switch", False) + return r + + +def _config(**overrides: Any) -> Config: + base: dict[str, Any] = dict( + allowlist=["BTC", "ETH"], + target_weights={}, + risk_pct=Decimal("0.01"), + caps=Caps( + max_per_order_usd=Decimal("100000"), + max_per_day_usd=Decimal("300000"), + max_exposure_usd=Decimal("1000000"), + max_per_asset_pct=Decimal("1"), + ), + market_data=MarketDataConfig( + granularities=[Granularity.ONE_DAY, Granularity.ONE_HOUR], history_days=365 + ), + auto_trade=AutoTradeConfig(mode="paper", interval_sec=900), + money_mgmt=MoneyMgmtConfig( + max_total_dd_pct=Decimal("0.20"), max_weekly_dd_pct=Decimal("0.08") + ), + dca=DcaConfig(budget_usd=Decimal("50"), cadence_days=7), + ) + base.update(overrides) + return Config(**base) + + +def _default_floor(config: Config) -> PromotionConfig: + return PromotionConfig( + min_trades=config.promotion.min_trades, + min_expectancy=config.promotion.min_expectancy, + min_rr=config.promotion.min_rr, + min_win_rate=float(config.promotion.min_win_rate), + ) + + +def _seed_paper_trade( + repo: Repository, + rule_name: str, + *, + product_id: str = "BTC-USD", + entry: str = "100", + exit_price: str = "110", + entry_ts: int = 1_000, + exit_ts: int = 2_000, + qty: str = "1", + pnl: str = "10", + r_multiple: str | None = "1.0", + outcome: str = "win", +) -> tuple[int, int]: + """Write an entry+exit paper order pair directly (matching `paper.py`'s own payload + shape) so tests can control exact win/loss/r_multiple stats without simulating candles.""" + entry_id = repo.insert_order( + dict( + mode="paper", + product_id=product_id, + side="BUY", + order_type="market", + qty=Decimal(qty), + limit_price=Decimal(entry), + status="filled", + fee=Decimal("0"), + expected_fill=Decimal(entry), + actual_fill=Decimal(entry), + raw_response=json.dumps( + { + "role": "entry", + "rule_name": rule_name, + "entry": entry, + "stop": "90", + "target": "120", + "qty": qty, + "ts": entry_ts, + } + ), + confirmation="paper", + rule_id=None, + created_at=entry_ts, + updated_at=entry_ts, + ) + ) + exit_id = repo.insert_order( + dict( + mode="paper", + product_id=product_id, + side="SELL", + order_type="market", + qty=Decimal(qty), + limit_price=Decimal(exit_price), + status="filled", + fee=Decimal("0"), + expected_fill=Decimal(exit_price), + actual_fill=Decimal(exit_price), + raw_response=json.dumps( + { + "role": "exit", + "rule_name": rule_name, + "entry_order_id": entry_id, + "entry": entry, + "exit": exit_price, + "qty": qty, + "pnl": pnl, + "r_multiple": r_multiple, + "mfe": "0", + "mae": "0", + "outcome": outcome, + "entry_ts": entry_ts, + "exit_ts": exit_ts, + } + ), + confirmation="paper", + rule_id=None, + created_at=exit_ts, + updated_at=exit_ts, + ) + ) + return entry_id, exit_id + + +def _seed_trade_outcome( + repo: Repository, + *, + rule_name: str | None = "dca", + product_id: str = "BTC-USD", + is_dca: bool = False, + opened_at: int = 1_000, + closed_at: int = 2_000, + qty: str = "1", + entry_fill: str = "100", + exit_fill: str = "110", + fees: str = "0.5", + pnl_net: str = "9.5", +) -> int: + return repo.insert_trade_outcome( + dict( + product_id=product_id, + rule_name=rule_name, + is_dca=is_dca, + opened_at=opened_at, + closed_at=closed_at, + qty=Decimal(qty), + entry_fill=Decimal(entry_fill), + exit_fill=Decimal(exit_fill), + fees=Decimal(fees), + pnl_net=Decimal(pnl_net), + ) + ) + + +# -- build_gate_distance ----------------------------------------------------------------------- + + +def test_gate_distance_trades_remaining_is_floor_minus_n_trades(repo: Repository) -> None: + for i in range(5): + _seed_paper_trade(repo, "dca", entry_ts=1000 + i, exit_ts=1100 + i) + stats = track_record(repo, "dca") + rule = Dca(product_id="BTC-USD") + default_floor = _default_floor(_config()) + + gate = build_gate_distance(rule, stats, default_floor) + + assert gate.n_trades == 5 + assert gate.min_trades == default_floor.min_trades + assert gate.trades_remaining == default_floor.min_trades - 5 + + +def test_gate_distance_blocking_reasons_include_n_trades_at_low_n(repo: Repository) -> None: + _seed_paper_trade(repo, "dca") + stats = track_record(repo, "dca") + rule = Dca(product_id="BTC-USD") + default_floor = _default_floor(_config()) + + gate = build_gate_distance(rule, stats, default_floor) + + assert gate.passing is False + assert any("n_trades" in reason for reason in gate.blocking_reasons) + + +def test_gate_distance_trend_follow_uses_relaxed_win_floor(repo: Repository) -> None: + """`turtle_breakout` is `promotion_class = "trend_follow"`: win floor 0.30 (not the + canonical 0.55), and `min_trades` stays the canonical 100 (NOT relaxed to 30).""" + rule = TurtleBreakout(product_id="BTC-USD") + stats = track_record(repo, "turtle_breakout") # no trades seeded -- floor values only matter + default_floor = _default_floor(_config()) + + gate = build_gate_distance(rule, stats, default_floor) + + expected_floor = floor_for_class(promotion_class_of(rule), default=default_floor) + assert gate.min_win_rate == 0.30 + assert gate.min_trades == 100 + assert expected_floor.min_win_rate == 0.30 + assert expected_floor.min_trades == 100 + + +# -- build_rule_track_record ------------------------------------------------------------------- + + +def test_rule_track_record_not_significant_below_30_trades(repo: Repository) -> None: + for i in range(10): + _seed_paper_trade(repo, "dca", entry_ts=1000 + i, exit_ts=1100 + i) + repo.insert_rule("dca", {"product_id": "BTC-USD"}, status="paper") + row = next(r for r in repo.get_rules() if r["kind"] == "dca") + stats = track_record(repo, "dca") + default_floor = _default_floor(_config()) + + record = build_rule_track_record(row, stats, default_floor) + + assert record.n_trades == 10 + assert record.significant is False + + +def test_rule_track_record_gate_none_for_non_paper_status(repo: Repository) -> None: + repo.insert_rule("dca", {"product_id": "BTC-USD"}, status="live") + row = next(r for r in repo.get_rules() if r["kind"] == "dca") + stats = track_record(repo, "dca") + default_floor = _default_floor(_config()) + + record = build_rule_track_record(row, stats, default_floor) + + assert record.status == "live" + assert record.gate is None + + +def test_rule_track_record_gate_none_for_candidate_status(repo: Repository) -> None: + repo.insert_rule("dca", {"product_id": "BTC-USD"}, status="candidate") + row = next(r for r in repo.get_rules() if r["kind"] == "dca") + stats = track_record(repo, "dca") + default_floor = _default_floor(_config()) + + record = build_rule_track_record(row, stats, default_floor) + + assert record.gate is None + + +def test_rule_track_record_realized_rr_none_when_no_losses(repo: Repository) -> None: + _seed_paper_trade(repo, "dca", pnl="10", outcome="win") + repo.insert_rule("dca", {"product_id": "BTC-USD"}, status="paper") + row = next(r for r in repo.get_rules() if r["kind"] == "dca") + stats = track_record(repo, "dca") + assert stats.avg_loss == Decimal(0) + default_floor = _default_floor(_config()) + + record = build_rule_track_record(row, stats, default_floor) + + assert record.realized_rr is None + + +def test_rule_track_record_degrades_gracefully_for_unrecognized_kind(repo: Repository) -> None: + """A stale `rules` row whose `kind` is no longer in `RULE_REGISTRY` must not crash the + whole report -- `_build_rule` raises `ValueError`, which degrades to a default class and a + `None` gate rather than propagating.""" + repo.insert_rule("some_retired_rule_kind", {"product_id": "BTC-USD"}, status="paper") + row = next(r for r in repo.get_rules() if r["kind"] == "some_retired_rule_kind") + stats = track_record(repo, "some_retired_rule_kind") + default_floor = _default_floor(_config()) + + record = build_rule_track_record(row, stats, default_floor) + + assert record.promotion_class == "default" + assert record.gate is None + + +# -- build_account_summary --------------------------------------------------------------------- + + +def test_account_summary_projects_verbatim_from_status_report() -> None: + from keel.commands.status import AutonomyStatus + + status_report = StatusReport( + now_ts=NOW_TS, + mode="paper", + kill_switch_engaged=False, + autonomy=AutonomyStatus( + live=False, autonomous=False, autonomous_until=None, updated_ts=None, + profile_readable=True, + ), + equity_state_mode="paper", + high_water_mark=Decimal("1000"), + drawdown_total_pct=Decimal("0.05"), + drawdown_weekly_pct=Decimal("0.01"), + max_total_dd_pct=Decimal("0.20"), + max_weekly_dd_pct=Decimal("0.08"), + rail11_status="ok", + paper_cash_usdc=Decimal("955.25"), + open_positions=[], + rule_counts={}, + live_rules=[], + data_freshness=[], + subscriptions=[], + ) + + summary = build_account_summary(status_report) + + assert summary.mode == "paper" + assert summary.rail11_status == "ok" + assert summary.drawdown_total_pct == Decimal("0.05") + assert summary.drawdown_weekly_pct == Decimal("0.01") + assert summary.high_water_mark == Decimal("1000") + assert summary.paper_cash_usdc == Decimal("955.25") + assert summary.max_total_dd_pct == Decimal("0.20") + assert summary.max_weekly_dd_pct == Decimal("0.08") + + +# -- build_insights_report ---------------------------------------------------------------------- + + +def test_insights_report_empty_db_does_not_crash(repo: Repository) -> None: + """The real current state of a fresh DB: no crash, zero closed trades, no/zero-trade rules.""" + config = _config() + status_report = gather_status(repo, config, now_ts=NOW_TS) + + report = build_insights_report(repo, config, status_report, NOW_TS) + + assert report.closed_trade_count == 0 + assert report.rules == [] + + +def test_insights_report_rule_filter(repo: Repository) -> None: + repo.insert_rule("dca", {"product_id": "BTC-USD"}, status="paper") + repo.insert_rule("turtle_breakout", {"product_id": "ETH-USD"}, status="candidate") + config = _config() + status_report = gather_status(repo, config, now_ts=NOW_TS) + + report = build_insights_report(repo, config, status_report, NOW_TS, rule_filter="dca") + + assert len(report.rules) == 1 + assert report.rules[0].rule_name == "dca" + + +def test_insights_report_counts_closed_trades(repo: Repository) -> None: + _seed_trade_outcome(repo, closed_at=1500) + _seed_trade_outcome(repo, closed_at=1600) + config = _config() + status_report = gather_status(repo, config, now_ts=NOW_TS) + + report = build_insights_report(repo, config, status_report, NOW_TS) + + assert report.closed_trade_count == 2 + + +# -- build_journal_report ----------------------------------------------------------------------- + + +def test_journal_report_internal_order_is_oldest_first(repo: Repository) -> None: + _seed_trade_outcome(repo, closed_at=3000, opened_at=2900, rule_name="dca") + _seed_trade_outcome(repo, closed_at=1000, opened_at=900, rule_name="dca") + _seed_trade_outcome(repo, closed_at=2000, opened_at=1900, rule_name="dca") + config = _config() + status_report = gather_status(repo, config, now_ts=NOW_TS) + + report = build_journal_report(repo, status_report, NOW_TS) + + assert [e.closed_at for e in report.entries] == [1000, 2000, 3000] + + +def test_journal_report_limit_caps_and_total_count_is_pre_limit(repo: Repository) -> None: + for i in range(5): + _seed_trade_outcome(repo, closed_at=1000 + i, opened_at=900 + i) + config = _config() + status_report = gather_status(repo, config, now_ts=NOW_TS) + + report = build_journal_report(repo, status_report, NOW_TS, limit=2) + + assert len(report.entries) == 2 + assert report.total_count == 5 + + +def test_journal_report_dca_row_has_no_r_multiple(repo: Repository) -> None: + _seed_trade_outcome(repo, is_dca=True, rule_name="dca", closed_at=1500) + config = _config() + status_report = gather_status(repo, config, now_ts=NOW_TS) + + report = build_journal_report(repo, status_report, NOW_TS) + + assert len(report.entries) == 1 + entry = report.entries[0] + assert entry.is_dca is True + assert entry.r_multiple is None + + +def test_journal_report_include_open_appends_open_rows(repo: Repository) -> None: + repo.open_position( + product_id="BTC-USD", + rule_name="turtle_breakout", + opened_at=NOW_TS - 100, + qty=Decimal("0.01"), + entry_fill=Decimal("65000"), + entry_fee=Decimal("1.5"), + bracket_order_id=None, + ) + config = _config() + status_report = gather_status(repo, config, now_ts=NOW_TS) + + report = build_journal_report(repo, status_report, NOW_TS, include_open=True) + + assert any(e.outcome == "open" for e in report.entries) + open_entry = next(e for e in report.entries if e.outcome == "open") + assert open_entry.r_multiple is None + assert open_entry.product_id == "BTC-USD" + + +def test_journal_report_without_include_open_has_no_open_rows(repo: Repository) -> None: + repo.open_position( + product_id="BTC-USD", + rule_name="turtle_breakout", + opened_at=NOW_TS - 100, + qty=Decimal("0.01"), + entry_fill=Decimal("65000"), + entry_fee=Decimal("1.5"), + bracket_order_id=None, + ) + config = _config() + status_report = gather_status(repo, config, now_ts=NOW_TS) + + report = build_journal_report(repo, status_report, NOW_TS, include_open=False) + + assert report.entries == [] + + +def test_journal_report_rule_and_asset_filters(repo: Repository) -> None: + _seed_trade_outcome(repo, rule_name="dca", product_id="BTC-USD", closed_at=1000) + _seed_trade_outcome(repo, rule_name="turtle_breakout", product_id="ETH-USD", closed_at=1100) + config = _config() + status_report = gather_status(repo, config, now_ts=NOW_TS) + + report = build_journal_report(repo, status_report, NOW_TS, rule="dca") + assert len(report.entries) == 1 + assert report.entries[0].rule_name == "dca" + + report2 = build_journal_report(repo, status_report, NOW_TS, asset="ETH-USD") + assert len(report2.entries) == 1 + assert report2.entries[0].product_id == "ETH-USD" + + +def test_journal_report_since_until_window(repo: Repository) -> None: + _seed_trade_outcome(repo, closed_at=1000) + _seed_trade_outcome(repo, closed_at=2000) + _seed_trade_outcome(repo, closed_at=3000) + config = _config() + status_report = gather_status(repo, config, now_ts=NOW_TS) + + report = build_journal_report(repo, status_report, NOW_TS, since_ts=1500, until_ts=2500) + + assert [e.closed_at for e in report.entries] == [2000] + + +def test_journal_report_enriches_r_multiple_from_paper_track_record(repo: Repository) -> None: + """A `trade_outcomes` row that matches a paper trade's (entry_ts, exit_ts) gets that + trade's real `r_multiple`/`outcome` rather than a pnl-sign guess.""" + _seed_paper_trade( + repo, "dca", entry_ts=1000, exit_ts=2000, pnl="10", r_multiple="2.5", outcome="win" + ) + _seed_trade_outcome( + repo, rule_name="dca", opened_at=1000, closed_at=2000, pnl_net="9.5", is_dca=False + ) + config = _config() + status_report = gather_status(repo, config, now_ts=NOW_TS) + + report = build_journal_report(repo, status_report, NOW_TS) + + assert len(report.entries) == 1 + assert report.entries[0].r_multiple == Decimal("2.5") + assert report.entries[0].outcome == "win" + + +def test_journal_report_no_match_derives_outcome_from_pnl_sign(repo: Repository) -> None: + _seed_trade_outcome(repo, rule_name="unmatched_rule", pnl_net="-5", closed_at=1500) + config = _config() + status_report = gather_status(repo, config, now_ts=NOW_TS) + + report = build_journal_report(repo, status_report, NOW_TS) + + assert len(report.entries) == 1 + assert report.entries[0].r_multiple is None + assert report.entries[0].outcome == "loss" + + +# -- renderers ------------------------------------------------------------------------------ + + +def test_render_summary_nonempty_and_has_small_sample_note(repo: Repository) -> None: + for i in range(5): + _seed_paper_trade(repo, "dca", entry_ts=1000 + i, exit_ts=1100 + i) + repo.insert_rule("dca", {"product_id": "BTC-USD"}, status="paper") + config = _config() + status_report = gather_status(repo, config, now_ts=NOW_TS) + report = build_insights_report(repo, config, status_report, NOW_TS) + + lines = render_summary(report) + + assert len(lines) > 0 + assert any("n<30" in line for line in lines) + + +def test_render_summary_empty_db_has_friendly_line_not_blank(repo: Repository) -> None: + config = _config() + status_report = gather_status(repo, config, now_ts=NOW_TS) + report = build_insights_report(repo, config, status_report, NOW_TS) + + lines = render_summary(report) + + assert len(lines) > 0 + assert any(line.strip() for line in lines) + assert any("no" in line.lower() for line in lines) + + +def test_render_journal_empty_db_has_friendly_line(repo: Repository) -> None: + config = _config() + status_report = gather_status(repo, config, now_ts=NOW_TS) + report = build_journal_report(repo, status_report, NOW_TS) + + lines = render_journal(report) + + assert len(lines) > 0 + assert any("no" in line.lower() for line in lines) + + +def test_render_journal_dca_row_labeled_and_no_r_equals_zero(repo: Repository) -> None: + _seed_trade_outcome(repo, is_dca=True, rule_name="dca", closed_at=1500) + config = _config() + status_report = gather_status(repo, config, now_ts=NOW_TS) + report = build_journal_report(repo, status_report, NOW_TS) + + lines = render_journal(report) + + joined = "\n".join(lines) + assert "DCA" in joined + assert "R=0" not in joined + + +# -- the `keel insights` commands ---------------------------------------------------------------- + + +def _repo_at(db_path) -> Repository: + conn = connect(str(db_path)) + migrate(conn) + return Repository(conn) + + +def test_insights_summary_command_default_prints_disclaimer(tmp_path, valid_config_path) -> None: + db_path = tmp_path / "keel.db" + _repo_at(db_path).set_state("kill_switch", False) + + result = CliRunner().invoke( + cli, ["--db", str(db_path), "--config", str(valid_config_path), "insights", "summary"] + ) + + assert result.exit_code == 0, result.output + assert "not financial advice" in result.output + + +def test_insights_summary_command_json_is_valid_no_prose(tmp_path, valid_config_path) -> None: + db_path = tmp_path / "keel.db" + _repo_at(db_path).set_state("kill_switch", False) + + result = CliRunner().invoke( + cli, + ["--db", str(db_path), "--config", str(valid_config_path), "insights", "summary", "--json"], + ) + + assert result.exit_code == 0, result.output + payload = json.loads(result.output) + assert "not financial advice" not in result.output + assert "account" in payload + assert "rules" in payload + assert "closed_trade_count" in payload + assert payload["account"]["mode"] == "paper" + + +def test_insights_journal_command_default_prints_disclaimer(tmp_path, valid_config_path) -> None: + db_path = tmp_path / "keel.db" + _repo_at(db_path).set_state("kill_switch", False) + + result = CliRunner().invoke( + cli, ["--db", str(db_path), "--config", str(valid_config_path), "insights", "journal"] + ) + + assert result.exit_code == 0, result.output + assert "not financial advice" in result.output + + +def test_insights_journal_command_json_is_valid_no_prose(tmp_path, valid_config_path) -> None: + db_path = tmp_path / "keel.db" + _repo_at(db_path).set_state("kill_switch", False) + + result = CliRunner().invoke( + cli, + ["--db", str(db_path), "--config", str(valid_config_path), "insights", "journal", "--json"], + ) + + assert result.exit_code == 0, result.output + payload = json.loads(result.output) + assert "not financial advice" not in result.output + assert "entries" in payload + assert "total_count" in payload + assert "filters" in payload diff --git a/tests/test_cli.py b/tests/test_cli.py index 0b00832b..5afc9b37 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -875,16 +875,18 @@ def test_simulate_no_fetch_default_runs_full_tier_matrix(tmp_path, monkeypatch): assert "Over cap" in report_text -# -- insights (stub) ---------------------------------------------------------------------------- +# -- insights (read-only promotion-gate + journal reporting; see tests/commands/test_insights.py) +# ---------------------------------------------------------------------------------------------- -def test_insights_stub(tmp_path): +def test_insights_is_a_group_with_summary_and_journal_subcommands(tmp_path): runner = CliRunner() - result = runner.invoke(cli, ["insights"]) + result = runner.invoke(cli, ["insights", "--help"]) assert result.exit_code == 0, result.output - assert "not yet implemented" in result.output + assert "summary" in result.output + assert "journal" in result.output def test_loading_config_binds_the_venue_for_telemetry(tmp_path: Path) -> None: From 95d7d7ba98207368cfef2a1a2bb73d8d8d080735 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Fri, 24 Jul 2026 19:18:51 -0400 Subject: [PATCH 2/2] fix(insights): correct include-open count line and quantize human render Two render-only fixes from independent review, no data-layer changes: - render_journal's "showing N of total_count closed trades" line counted --include-open's appended outcome="open" rows in N, so it could read e.g. "showing 4 of 3 closed trades". N now excludes open rows and an "(+K open)" suffix is appended when any were shown. - render_summary/render_journal now quantize Decimal money/ratio fields to 2dp for human display (via a shared _quantized/_money/_ratio helper that passes non-finite Decimals and sentinel strings through unchanged). --json is untouched -- json.dumps(..., default=str) still emits full precision -- and gate.blocking_reasons stays verbatim from promotion.can_promote, as intended. Co-Authored-By: Claude Opus 4.8 --- keel/commands/insights.py | 48 +++++++++++++++++---- tests/commands/test_insights.py | 74 +++++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 7 deletions(-) diff --git a/keel/commands/insights.py b/keel/commands/insights.py index c9402d03..2e7fb913 100644 --- a/keel/commands/insights.py +++ b/keel/commands/insights.py @@ -440,6 +440,29 @@ def build_journal_report( "proven edge" ) +_TWO_DP = Decimal("0.01") + + +def _quantized(x: Any) -> Any: + """2dp-round a `Decimal` for HUMAN display only -- `--json` stays full precision + (`json.dumps(..., default=str)` never goes through this). + + Passes through anything that isn't a finite `Decimal` unchanged: a sentinel string + ("n/a (no losses yet)", "n/a", "dca"), `None`, or a non-finite `Decimal` (`Infinity`, + which `BacktestResult.profit_factor`/a zero-loss realized-rr can legitimately be, and + which `.quantize()` itself refuses to round) all render exactly as before. + """ + if isinstance(x, Decimal) and x.is_finite(): + return x.quantize(_TWO_DP) + return x + + +# One shared helper covers both money and ratio fields (both are just "round this Decimal to +# 2dp for a human, leave any sentinel alone") -- two names kept at call sites purely for the +# reader, per the field groupings above. +_money = _quantized +_ratio = _quantized + def render_summary(report: InsightsReport) -> list[str]: """The `keel insights summary` (default, non-`--json`) rendering, as a list of lines.""" @@ -474,8 +497,9 @@ def render_summary(report: InsightsReport) -> list[str]: rr = r.realized_rr if r.realized_rr is not None else "n/a (no losses yet)" lines.append( f" [{r.rule_name}] status={r.status} class={r.promotion_class} n={r.n_trades} " - f"win_rate={r.win_rate:.1%} avg_win={r.avg_win} avg_loss={r.avg_loss} rr={rr} " - f"expectancy={r.expectancy} profit_factor={r.profit_factor} max_dd={r.max_drawdown}" + f"win_rate={r.win_rate:.1%} avg_win={_money(r.avg_win)} avg_loss={_money(r.avg_loss)} " + f"rr={_ratio(rr)} expectancy={_money(r.expectancy)} " + f"profit_factor={_ratio(r.profit_factor)} max_dd={_money(r.max_drawdown)}" ) if not r.significant: lines.append(f" {_SMALL_SAMPLE_NOTE}") @@ -503,7 +527,15 @@ def render_journal(report: JournalReport) -> list[str]: active_filters = {k: v for k, v in report.filters.items() if v not in (None, False)} filters_desc = ", ".join(f"{k}={v}" for k, v in active_filters.items()) or "none" lines.append(f"filters: {filters_desc}") - lines.append(f"showing {len(report.entries)} of {report.total_count} closed trades") + # `report.total_count` is the pre-`--limit` CLOSED count; `report.entries` (once + # `--include-open` appends live positions) mixes closed rows with `outcome == "open"` ones, + # so the numerator here must exclude those or the line lies (e.g. "4 of 3 closed trades"). + shown_closed = sum(1 for e in report.entries if e.outcome != "open") + open_shown = len(report.entries) - shown_closed + count_line = f"showing {shown_closed} of {report.total_count} closed trades" + if open_shown: + count_line += f" (+{open_shown} open)" + lines.append(count_line) lines.append("") if not report.entries: @@ -513,21 +545,23 @@ def render_journal(report: JournalReport) -> list[str]: for e in reversed(report.entries): if e.outcome == "open": lines.append( - f" OPEN {e.product_id} qty={e.qty} entry={e.entry_fill} " + f" OPEN {e.product_id} qty={_money(e.qty)} entry={_money(e.entry_fill)} " f"opened_at={_human_dt(e.opened_at)} rule={e.rule_name}" ) elif e.is_dca: age = _human_age(max(report.now_ts - (e.closed_at or report.now_ts), 0)) lines.append( - f" [{_human_dt(e.closed_at or 0)} / {age}] {e.product_id} DCA qty={e.qty} " - f"pnl_net={e.pnl_net} -- DCA: no stop, excluded from R/expectancy" + f" [{_human_dt(e.closed_at or 0)} / {age}] {e.product_id} DCA " + f"qty={_money(e.qty)} pnl_net={_money(e.pnl_net)} -- " + f"DCA: no stop, excluded from R/expectancy" ) else: r_text = e.r_multiple if e.r_multiple is not None else "n/a" age = _human_age(max(report.now_ts - (e.closed_at or report.now_ts), 0)) lines.append( f" [{_human_dt(e.closed_at or 0)} / {age}] {e.product_id} rule={e.rule_name} " - f"outcome={e.outcome} qty={e.qty} pnl_net={e.pnl_net} R={r_text}" + f"outcome={e.outcome} qty={_money(e.qty)} pnl_net={_money(e.pnl_net)} " + f"R={_ratio(r_text)}" ) return lines diff --git a/tests/commands/test_insights.py b/tests/commands/test_insights.py index 65afcae4..3589bb4c 100644 --- a/tests/commands/test_insights.py +++ b/tests/commands/test_insights.py @@ -19,6 +19,9 @@ from keel.cli import cli from keel.commands.insights import ( + AccountSummary, + InsightsReport, + RuleTrackRecord, build_account_summary, build_gate_distance, build_insights_report, @@ -583,6 +586,77 @@ def test_render_journal_dca_row_labeled_and_no_r_equals_zero(repo: Repository) - assert "R=0" not in joined +def test_render_journal_include_open_count_line_excludes_open_from_closed_count( + repo: Repository, +) -> None: + """Regression for the "showing 4 of 3 closed trades" bug: `--include-open` appends + `outcome == "open"` rows onto `report.entries`, but the "showing N of total_count closed + trades" line's numerator must count only the CLOSED rows actually shown, with the open + ones called out separately.""" + _seed_trade_outcome(repo, closed_at=1000, opened_at=900, rule_name="dca") + _seed_trade_outcome(repo, closed_at=2000, opened_at=1900, rule_name="dca") + _seed_trade_outcome(repo, closed_at=3000, opened_at=2900, rule_name="dca") + repo.open_position( + product_id="BTC-USD", + rule_name="dca", + opened_at=NOW_TS - 100, + qty=Decimal("0.01"), + entry_fill=Decimal("65000"), + entry_fee=Decimal("1.5"), + bracket_order_id=None, + ) + config = _config() + status_report = gather_status(repo, config, now_ts=NOW_TS) + + report = build_journal_report(repo, status_report, NOW_TS, include_open=True) + + assert report.total_count == 3 + assert sum(1 for e in report.entries if e.outcome == "open") == 1 + + lines = render_journal(report) + joined = "\n".join(lines) + assert "showing 3 of 3 closed trades" in joined + assert "(+1 open)" in joined + + +def test_render_summary_quantizes_ratio_to_two_decimal_places() -> None: + """The human render rounds ratios (rr here) to 2dp; `--json` (untouched by this render + path) stays full precision -- this only exercises the human renderer.""" + account = AccountSummary( + mode="paper", + equity_state_mode=None, + high_water_mark=None, + drawdown_total_pct=None, + drawdown_weekly_pct=None, + max_total_dd_pct=Decimal("0.20"), + max_weekly_dd_pct=Decimal("0.08"), + rail11_status="unknown", + paper_cash_usdc=None, + ) + rule = RuleTrackRecord( + rule_name="dca", + status="live", + promotion_class="default", + n_trades=40, + win_rate=0.6, + avg_win=Decimal("7"), + avg_loss=Decimal("-3"), + realized_rr=Decimal("7") / Decimal("3"), # 2.3333333333333333333333333333... + expectancy=Decimal("2.5"), + profit_factor=Decimal("3"), + max_drawdown=Decimal("10"), + significant=True, + gate=None, + ) + report = InsightsReport(now_ts=NOW_TS, account=account, rules=[rule], closed_trade_count=40) + + lines = render_summary(report) + + joined = "\n".join(lines) + assert "rr=2.33" in joined + assert "2.3333333333333333333333333333" not in joined + + # -- the `keel insights` commands ----------------------------------------------------------------