From 68616eb7bff53788d3955dd8ae644d4776f2b51c Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Sat, 26 Sep 2026 14:49:38 -0400 Subject: [PATCH] fix(sim): DCA buys once per completed day, and the report shows its sleeve (#821) The account sim evaluated the real Dca rule on every hourly bar, and its daily window included the still-forming day, so every cadence day bought 24 times at a close that had not happened yet (9 cadence days at $50 spent ~$10,768). The sleeve was then invisible: the edge table gave DCA N 0 (it never saw daily candles) and per_asset_pnl counted closed trades only. - Dca decides on completed days only, via the forming-day guard TurtleBreakout already used, now shared as rules.base.completed_days. Live input (closed candles only) is kept unchanged. - portfolio_sim takes at most one DCA decision per (asset, rule, UTC day), logs each buy (SimResult.dca_buys) and records final prices. - edge_table skips accumulating rules (Rule.accumulates); the new accumulation_table reports buys, cost, mark-to-market value and unrealized P&L, outside __pooled__ and G2. - The account report and the HTML artifact show the DCA sleeve marked to market; per-asset P&L is labelled as realized rule P&L. Closes #821 Co-Authored-By: Claude Opus 5.5 --- keel/commands/simulate.py | 13 + keel/sim/artifact.py | 37 ++- keel/sim/portfolio_sim.py | 131 +++++++++- keel/sim/report.py | 128 +++++++++- keel/strategy/rules/base.py | 41 +++ keel/strategy/rules/dca.py | 11 +- keel/strategy/rules/turtle_breakout.py | 34 +-- tests/sim/test_artifact.py | 36 ++- tests/sim/test_dca_sleeve.py | 341 +++++++++++++++++++++++++ tests/strategy/test_dca.py | 56 ++++ 10 files changed, 784 insertions(+), 44 deletions(-) create mode 100644 tests/sim/test_dca_sleeve.py diff --git a/keel/commands/simulate.py b/keel/commands/simulate.py index 32c4077a..e227d946 100644 --- a/keel/commands/simulate.py +++ b/keel/commands/simulate.py @@ -184,7 +184,11 @@ def build_account_metrics( "sortino": metrics_mod.sortino(returns), "trade_count": len(closed_trades), "avg_hold_hours": avg_hold_hours, + # REALIZED rule P&L from closed round trips only -- the DCA sleeve is never closed. "per_asset_pnl": per_asset_pnl, + # The DCA sleeve, marked at each asset's last close (#821): in `ending_value` above, + # and until #821 nowhere else in the report. + "dca_sleeve": portfolio_sim.dca_sleeve(sim), } @@ -431,6 +435,14 @@ def run_simulation( slippage_pct=SIM_SLIPPAGE_PCT, slippage_by_product=slippage_by_product, ) + # Accumulating rules (DCA) are not round trips: their edge pass is its own row (#821). + accumulation = report_mod.accumulation_table( + rules, + candles_by_asset, + fee_pct=fee_pct, + slippage_pct=SIM_SLIPPAGE_PCT, + slippage_by_product=slippage_by_product, + ) sim = portfolio_sim.run( rules, @@ -531,6 +543,7 @@ def run_simulation( tier_results=tier_results, fee_pct=fee_pct, slippage_rows=slippage_rows, + accumulation=accumulation, ) out = out_path if out_path is not None else default_report_path(now_ts) diff --git a/keel/sim/artifact.py b/keel/sim/artifact.py index 837f8ea5..27b76575 100644 --- a/keel/sim/artifact.py +++ b/keel/sim/artifact.py @@ -393,6 +393,39 @@ def _render_gaps_table(gaps: list[GapItem]) -> str: ) +def _render_dca_sleeve(sleeve: dict) -> str: + """The never-closed DCA sleeve (#821), one row per asset, marked to market. Empty string when + there is no sleeve, so a run without DCA renders exactly what it did before.""" + if not sleeve: + return "" + headers = ["Asset", "Buys", "Qty", "Cost basis", "Last close", "Value", "Unrealized P&L"] + head = "".join(f"{_esc(h)}" for h in headers) + rows = "".join( + "" + + "".join( + f"{_esc(str(v))}" + for v in ( + asset, + row.buys, + row.qty, + row.cost_usd, + row.last_close, + row.value_usd, + row.unrealized_pnl, + ) + ) + + "" + for asset, row in sleeve.items() + ) + return ( + "

DCA sleeve (accumulation, marked to market)

\n" + "

Bought on the DCA cadence and never sold: unrealized, marked at each asset's last " + "close; cost includes entry fees. In the ending value, not in the realized P&L " + "above.

\n" + f"{head}{rows}
\n" + ) + + def render_html( sim: SimResult, benchmark: BenchmarkResult, @@ -437,9 +470,11 @@ def render_html(

Drawdown

{drawdown_svg} -

Per-asset P&L

+

Per-asset realized rule P&L

{bars_svg} +{_render_dca_sleeve(account_metrics.get("dca_sleeve") or {})} +

Knowledge & data gaps

{_render_gaps_table(gaps)} diff --git a/keel/sim/portfolio_sim.py b/keel/sim/portfolio_sim.py index a663d139..bb58ffe8 100644 --- a/keel/sim/portfolio_sim.py +++ b/keel/sim/portfolio_sim.py @@ -17,13 +17,24 @@ **DCA is a separate sleeve, not a slot-occupant (Issue #85):** a DCA-class setup (`no_stop` / `order_class == "dca"` context, `strategy/rules/dca.py`) is scheduled accumulation, not a -risk-defined trade -- it is evaluated and (re)bought on every bar regardless of whether that -asset's RULE slot is currently held, via `account.open(..., dca=True)`, which accumulates into a -separate per-asset DCA lot (`SimAccount.dca_positions`) that this simulator never closes (DCA has -no exit signal by design). Before this fix, DCA and rule trades shared the single per-asset +risk-defined trade -- it is evaluated on every bar regardless of whether that asset's RULE +slot is currently held (and bought at most once per UTC day, see below), via +`account.open(..., dca=True)`, which accumulates into a separate per-asset DCA lot +(`SimAccount.dca_positions`) that this simulator never closes (DCA has no exit signal by +design). Before this fix, DCA and rule trades shared the single per-asset `held` slot, so an asset accumulating DCA (which never exits) permanently froze that asset's rule evaluation. +**One DCA decision per UTC day, per (asset, rule) (#821):** the loop is hourly but a DCA rule +decides on DAILY candles, so its latest completed day -- and therefore its `detect` result -- is +the same on all 24 bars of a day. Without a guard every cadence day bought 24 times (a $50 budget +over 9 cadence days spent ~$10,768, not $450). `_process_dca_signals` therefore records the UTC +day of each DCA decision and takes at most one per (asset, rule) per day -- whether it filled or +was vetoed, exactly as the live agent trades once per UTC day. Each buy is logged +(`SimResult.dca_buys`), and `dca_sleeve` marks the never-closed lots at each asset's final close +(`SimResult.final_prices`) so the report can show the sleeve instead of leaving it only inside +`ending_value`. + **Interpretive notes** (the plan's Task 6 prose leaves a few specifics implicit): - **`cts_factor_populated` / `rejected_for_missing_input`** (both `dict[str, int]`, keyed by CTS @@ -93,9 +104,12 @@ "IDLE_SPAN_MIN_HOURS", "MOVE_THRESHOLD_PCT", "WINDOW_BARS", + "DcaBuy", + "DcaSleeve", "SimResult", "SimTelemetry", "SimTrade", + "dca_sleeve", "run", ] @@ -120,6 +134,7 @@ DUST_FLOOR = Decimal("1") _SECONDS_PER_HOUR = 3600 +_SECONDS_PER_DAY = 86_400 @dataclass @@ -158,6 +173,44 @@ class SimTelemetry: mfe_giveback_samples: list[Decimal] = field(default_factory=list) +@dataclass(frozen=True) +class DcaBuy: + """One DCA accumulation buy (#821). `decision_ts` is the hourly bar the decision was taken + on (its UTC day is the once-per-day key); `notional` is the budgeted spend at the decision + price (`sizing.spend(qty, setup.entry)`); `cost_usd` is the cash the fill actually took -- + the slipped next-bar open times `qty`, plus the entry fee.""" + + asset: str + rule_kind: str + decision_ts: int + fill_ts: int + qty: Decimal + notional: Decimal + cost_usd: Decimal + + +@dataclass(frozen=True) +class DcaSleeve: + """An accumulated DCA holding, marked to market (#821) -- the account sim's never-closed + sleeve for one asset, or the edge pass's accumulation row for one DCA rule. NOT a round + trip: nothing here was sold, so the P&L is unrealized. + + `cost_usd` is every buy's cash outlay, fees included, so `unrealized_pnl` is net of entry + costs; `value_usd` is `qty * last_close`.""" + + buys: int + qty: Decimal + cost_usd: Decimal + last_close: Decimal + value_usd: Decimal + unrealized_pnl: Decimal + + @classmethod + def marked(cls, buys: int, qty: Decimal, cost_usd: Decimal, last_close: Decimal) -> DcaSleeve: + value = qty * last_close + return cls(buys, qty, cost_usd, last_close, value, value - cost_usd) + + @dataclass class SimResult: trades: list[SimTrade] @@ -174,6 +227,28 @@ class SimResult: # (`SimAccount.monthly_volume`) -- feeds the Coinbase One tier/fee analysis (Issue #86, # `sim.tiers`), which needs per-month volume to compute over-cap fees correctly. monthly_volume: dict[int, Decimal] = field(default_factory=dict) + # Every DCA buy, in order (#821) -- the sleeve's buy count and fee-inclusive cost basis. + dca_buys: list[DcaBuy] = field(default_factory=list) + # Each asset's last hourly close seen by the run: what `dca_sleeve` marks the sleeve at. + final_prices: dict[str, Decimal] = field(default_factory=dict) + + +def dca_sleeve(result: SimResult) -> dict[str, DcaSleeve]: + """The run's DCA sleeve per asset, marked at that asset's final close (#821). + + `qty` is the account's own lot (`result.dca_positions`); the buy count and cost basis come + from `result.dca_buys`. An asset with a lot but no final price is marked at zero rather than + silently at cost -- a missing mark must look like one.""" + sleeve: dict[str, DcaSleeve] = {} + for asset, lot in sorted(result.dca_positions.items()): + buys = [b for b in result.dca_buys if b.asset == asset] + sleeve[asset] = DcaSleeve.marked( + buys=len(buys), + qty=lot.qty, + cost_usd=sum((b.cost_usd for b in buys), Decimal("0")), + last_close=result.final_prices.get(asset, Decimal("0")), + ) + return sleeve @dataclass @@ -299,6 +374,9 @@ def run( # -- that is pyramiding (§26.1), a separate feature with its own exposure-rail implications. held: dict[tuple[str, str], _Held] = {} latest_price: dict[str, Decimal] = {} + # (asset, rule_name, utc_day) of every DCA decision taken -- at most one per day (#821). + dca_decided: set[tuple[str, str, int]] = set() + dca_buys: list[DcaBuy] = [] idle: dict[str, _IdleAnchor] = {} last_month_start: int | None = None @@ -366,10 +444,19 @@ def run( telemetry.signals_emitted += len(signals) _record_cts_telemetry(signals, candles_by_tf, telemetry) - # DCA continuation runs every bar regardless of the RULE slot's state -- a DCA - # sleeve keeps buying on its own cadence even while a rule position is held. + # DCA runs regardless of the RULE slot's state -- a DCA sleeve keeps buying on its + # own cadence even while a rule position is held -- but decides once per UTC day. _process_dca_signals( - asset, idx, hourly, signals, account, config, t, monthly_volume_cap + asset, + idx, + hourly, + signals, + account, + config, + t, + monthly_volume_cap, + decided=dca_decided, + buys=dca_buys, ) fired = _process_rule_signals( @@ -420,6 +507,8 @@ def run( telemetry=telemetry, dca_positions=dict(account.dca_positions), monthly_volume=account.monthly_volume(), + dca_buys=dca_buys, + final_prices=dict(latest_price), ) @@ -574,6 +663,9 @@ def _process_dca_signals( config: Config, now_ts: int, monthly_volume_cap: Decimal | None = None, + *, + decided: set[tuple[str, str, int]], + buys: list[DcaBuy], ) -> None: """Buy every DCA-class signal this bar into the separate DCA sleeve (`account.dca_positions`, via `account.open(..., dca=True)`), regardless of whether the asset's RULE slot is currently @@ -583,11 +675,22 @@ def _process_dca_signals( (Issue #86) it would push this UTC month's trading volume past `monthly_volume_cap` -- DCA is SKIPPED for the cycle in that case, not partially filled, matching its fixed-budget-per-cycle semantics (unlike the RULE slot's risk-sized notional, which IS clamped, see - `_process_rule_signals`).""" + `_process_rule_signals`). + + **Once per UTC day (#821).** A DCA rule decides on daily candles, so its signal repeats on + every hourly bar of the day. `decided` holds `(asset, rule, utc_day)` for every decision + already taken; a repeat is dropped before anything else runs. The day is marked when the + decision is TAKEN, not only when it fills: a vetoed or unfillable buy is that day's + decision, as it is live, where the agent trades once per UTC day.""" + day = now_ts // _SECONDS_PER_DAY for signal in signals: setup = signal.setup if setup is None or not _is_dca_setup(setup): continue + key = (asset, signal.rule_name, day) + if key in decided: + continue + decided.add(key) try: budget = setup.context.get("size_usd") or config.dca.budget_usd @@ -621,7 +724,19 @@ def _process_dca_signals( continue # no next bar to fill at -- the signal is lost, not a rejection fill_bar = hourly[fill_idx] + cash_before = account.cash_usdc account.open(intent, fill_bar.open, fill_bar.ts, dca=True) + buys.append( + DcaBuy( + asset=asset, + rule_kind=signal.rule_name, + decision_ts=now_ts, + fill_ts=fill_bar.ts, + qty=qty, + notional=notional, + cost_usd=cash_before - account.cash_usdc, + ) + ) def _process_rule_signals( diff --git a/keel/sim/report.py b/keel/sim/report.py index acbd2da6..58a60bdd 100644 --- a/keel/sim/report.py +++ b/keel/sim/report.py @@ -8,7 +8,9 @@ **Module map:** - `edge_table` -- reruns `strategy.backtest.backtest` per rule (Task 5.1's "edge pass"), keyed by `"{rule.name}:{asset}"`, plus a pooled `"__pooled__"` entry summarizing every rule's - trades together. + trades together. Accumulating rules (`Rule.accumulates`, i.e. `Dca`) are NOT in it (#821). +- `accumulation_table` -- the edge pass for accumulating rules: buys, cost basis and + mark-to-market value from daily candles, never a round trip and never pooled. - `build_verdict` -- the three gates from spec §6.2 (G1 data sufficiency, G2 promotion floors via `strategy.promotion.check_floors`, G3 risk-adjusted edge vs a `BenchmarkResult`). - `analyze_gaps` -- the six deterministic "lacked information" detectors from spec §6.1, read @@ -60,9 +62,10 @@ from decimal import Decimal from typing import TYPE_CHECKING +from keel.execution import sizing from keel.execution.guards import _asset from keel.sim.benchmark import BenchmarkResult -from keel.sim.portfolio_sim import SimResult, SimTelemetry +from keel.sim.portfolio_sim import DcaSleeve, SimResult, SimTelemetry from keel.sim.tiers import OVER_CAP, WITHIN_CAP, TierFeeResult from keel.strategy.backtest import ( SLIPPAGE_CAP_PCT, @@ -91,6 +94,7 @@ "POOLED_KEY", "GapItem", "Verdict", + "accumulation_table", "analyze_gaps", "build_verdict", "edge_table", @@ -176,11 +180,19 @@ def edge_table( A rule whose asset has no cached candles for its timeframe gets an empty series (an all-zero `BacktestResult`, `n_trades=0`) rather than raising -- absent data is a data-coverage gap (see `analyze_gaps`), not a crash. + + An ACCUMULATING rule (`Rule.accumulates` -- `Dca`) is skipped entirely: no key, nothing + pooled (#821). It has no exit, so `backtest()` gave it N 0 (it never even saw daily + candles) or, handed them, a fee-only round trip per buy -- either way a sample G2 must not + be fed. `accumulation_table` reports it instead, and `group_trades_by_class` skips it by + the same absence. """ results: dict[str, BacktestResult] = {} pooled_trades = [] for rule in rules: + if rule.accumulates: + continue asset = _asset(rule.product_id) tf = _rule_trading_tf(rule) per_tf = candles_by_asset.get(asset, {}) @@ -201,6 +213,67 @@ def edge_table( return results +def accumulation_table( + rules: list[Rule], + candles_by_asset: dict[str, dict[Granularity, list[Candle]]], + fee_pct: Decimal, + slippage_pct: Decimal, + slippage_by_product: Callable[[str], Decimal] | None = None, +) -> dict[str, DcaSleeve]: + """The edge pass for ACCUMULATING rules (`Rule.accumulates` -- `Dca`), keyed like + `edge_table` (`"{rule.name}:{asset}"`) but never mixed into it (#821). + + Each rule is driven over its asset's `ONE_DAY` series, one completed day at a time: every + bar in that series has closed, so `Dca.detect`'s completed-day guard keeps them all, and + each day is decided once -- the same two rules `sim.portfolio_sim` now applies to the + account sleeve. A buy decided on a closed day fills at the NEXT day's open (slipped, plus + the entry fee), sized `sizing.dca_size(size_usd, entry)` exactly as the account sim sizes + it; a decision on the series' last day has no next bar and is dropped, as it is there. + + The row is accumulation, not round trips: the number of buys, the cash they cost (fees + included), and that holding marked at the last daily close, with its unrealized P&L. + Nothing here is a trade, so none of it reaches `__pooled__` or G2. Risk-defined rules are + ignored (they are `edge_table`'s); a rule whose asset has no daily candles gets a + zero-buy row rather than raising. + """ + rows: dict[str, DcaSleeve] = {} + for rule in rules: + if not rule.accumulates: + continue + asset = _asset(rule.product_id) + daily = candles_by_asset.get(asset, {}).get(Granularity.ONE_DAY, []) + slip = ( + slippage_by_product(rule.product_id) + if slippage_by_product is not None + else slippage_pct + ) + decided: set[int] = set() + buys, qty, cost = 0, Decimal("0"), Decimal("0") + for i in range(len(daily) - 1): + fill_bar = daily[i + 1] + setup = rule.detect({Granularity.ONE_DAY: daily[: i + 1]}) + if setup is None: + continue + # Once per decision day -- the day after the completed bar, as in the account + # sim. One bar per day makes this a no-op on a clean series; a duplicated daily + # bar would otherwise buy twice. + day = fill_bar.ts // 86_400 + if day in decided: + continue + decided.add(day) + budget = setup.context.get("size_usd") + if budget is None or setup.entry <= 0: + continue + buy_qty = sizing.dca_size(budget, setup.entry) + fill = fill_bar.open * (Decimal(1) + slip) + buys += 1 + qty += buy_qty + cost += fill * buy_qty * (Decimal(1) + fee_pct) + last_close = daily[-1].close if daily else Decimal("0") + rows[f"{rule.name}:{asset}"] = DcaSleeve.marked(buys, qty, cost, last_close) + return rows + + def group_trades_by_class( edge: dict[str, BacktestResult], rules: list[Rule] ) -> dict[str, BacktestResult]: @@ -724,6 +797,35 @@ def _render_edge_section( return lines +def _render_holdings_table(first_column: str, rows: dict[str, DcaSleeve]) -> list[str]: + """One Markdown row per accumulated holding (`DcaSleeve`), shared by the edge pass's + accumulation section and the account's DCA sleeve so the two read identically.""" + lines = [ + f"| {first_column} | Buys | Qty | Cost basis | Last close | Value | Unrealized P&L |", + "|---|---|---|---|---|---|---|", + ] + for key, row in rows.items(): + lines.append( + f"| {key} | {row.buys} | {row.qty} | {row.cost_usd} | {row.last_close} | " + f"{row.value_usd} | {row.unrealized_pnl} |" + ) + return lines + + +def _render_accumulation_section(accumulation: dict[str, DcaSleeve]) -> list[str]: + """Accumulating rules' edge pass (#821), kept apart from the round-trip edge table.""" + return [ + "## DCA accumulation (not round trips)", + "", + "Accumulating rules buy on a cadence and never sell, so they have no win rate, " + "expectancy or R-multiples. Each row is its buys over the daily series (decided on " + "completed days, once per day, filled at the next open), their cost including fees, and " + f"the holding marked at the last close. Not in `{POOLED_KEY}`, not in G2.", + "", + *_render_holdings_table("Rule", accumulation), + ] + + def _render_account_section(account_metrics: dict, slippage_rows=None) -> list[str]: lines = ["## Account results", "", "| Metric | Value |", "|---|---|"] for key, label in _ACCOUNT_METRIC_LABELS: @@ -734,8 +836,24 @@ def _render_account_section(account_metrics: dict, slippage_rows=None) -> list[s per_asset = account_metrics.get("per_asset_pnl") if per_asset: lines.append("") - lines.append("Per-asset P&L:") + # REALIZED rule P&L: `build_account_metrics` sums closed round trips only. The DCA + # sleeve is never closed, so it was invisible here while inside `ending_value` (#821). + lines.append("Per-asset realized rule P&L (closed round trips; the DCA sleeve is below):") lines.extend(f"- {asset}: {pnl}" for asset, pnl in sorted(per_asset.items())) + sleeve = account_metrics.get("dca_sleeve") + if sleeve: + lines.extend( + [ + "", + "### DCA sleeve (accumulation, marked to market)", + "", + "Bought on the DCA cadence and never sold: unrealized, marked at each asset's " + "last close. Cost basis includes entry fees. Included in the ending value above, " + "not in the trade count or the realized P&L.", + "", + *_render_holdings_table("Asset", sleeve), + ] + ) # #259: the edge table above priced fills PER PRODUCT; these dollar figures did not. The # note lives HERE -- where a reader compares a thin asset's edge PF against its dollar # P&L -- not only beside the edge table where it was easy to miss. @@ -935,6 +1053,7 @@ def render_markdown( pbo_gate: tuple[bool, list[str]] | None = None, fee_pct: Decimal | None = None, slippage_rows: list[SlippageAssumption] | None = None, + accumulation: dict[str, DcaSleeve] | None = None, ) -> str: """Render the full report (spec §6 structure, plus Issue #86's tier/fee matrix) as one Markdown string. @@ -949,11 +1068,14 @@ def render_markdown( the overfitting section is emitted only when a PBO run was actually performed. `slippage_rows` (#259) follows it too: the per-product assumed-slippage table is rendered beside the edge table's fee line only when the caller priced fills per product and says so. + `accumulation` (#821, `accumulation_table`'s output) renders the accumulating rules' own + section right after the edge table, only when there is at least one such rule. """ sections = [ _render_verdict_section(verdict, in_sample), _render_coverage_section(sim), _render_edge_section(edge, fee_pct, slippage_rows), + *([_render_accumulation_section(accumulation)] if accumulation else []), _render_account_section(account_metrics, slippage_rows), _render_benchmark_section(account_metrics, benchmark, slippage_rows), _render_tier_section(tier_results or []), diff --git a/keel/strategy/rules/base.py b/keel/strategy/rules/base.py index f3fdf276..961cd838 100644 --- a/keel/strategy/rules/base.py +++ b/keel/strategy/rules/base.py @@ -23,6 +23,40 @@ from keel.types import Candle, Granularity, Side +_DAY_SECONDS = 24 * 60 * 60 + + +def completed_days(candles_by_tf: dict[Granularity, list[Candle]]) -> list[Candle]: + """The `ONE_DAY` series with any still-FORMING last bar dropped. + + Shared by every rule that decides on daily candles (`TurtleBreakout`, `Dca` -- #821 moved it + here from `turtle_breakout.py`, which still imports it as `_completed_days`). + + The account sim (`sim.portfolio_sim`) slices its daily series with + `bisect_right(daily_ts, t)` against the current hourly bar `t`, so its last daily bar + always CONTAINS that hourly bar -- it is the current, still-forming day, and its DB OHLC is + the completed day (lookahead if consumed intraday). + + The live agent (`agent.run_once`) passes an `ONE_HOUR` key too, but `data.market_feed` + persists only CLOSED candles, so its last daily bar has already closed. Keying the drop on + the mere PRESENCE of `ONE_HOUR` therefore threw away a completed day there and left the + rule deciding on a bar up to 48h old -- a day of lag on every breakout. + + So the test is where the hourly series SITS, not whether it exists: the newest daily bar is + still forming unless the newest hourly bar opens at or after that day's close. In the sim + that is never true (its hourly bar is by construction inside the day), so the sim's guard is + unchanged; in the live agent it is true from the first full hour of the next UTC day. A + daily-only input (the edge backtester's native series, no `ONE_HOUR` key) is returned as + is: every bar in it has already closed. + """ + daily = candles_by_tf.get(Granularity.ONE_DAY, []) + hourly = candles_by_tf.get(Granularity.ONE_HOUR) + if not hourly or not daily: + return daily + if hourly[-1].ts < daily[-1].ts + _DAY_SECONDS: + return daily[:-1] + return daily + class Action(str, Enum): """What the evaluation engine decided to do for a rule/product on this bar.""" @@ -307,6 +341,13 @@ class Rule(ABC): #: arrives as a LIST, not a quoted string, and that function answers a narrower question #: (which values an operator may legitimately quote in `rules add --params`). tuple_params: ClassVar[tuple[str, ...]] = () + #: Whether this rule ACCUMULATES -- scheduled buys that are never sold (`Dca`) -- rather + #: than trading a risk-defined setup to an exit. Read off the class by `sim.report`, which + #: routes an accumulating rule away from the round-trip backtest into its own accumulation + #: row (#821): run through `backtest()` it has no exit, so it produced either nothing (N 0) + #: or, with a daily timeframe declared, a fee-only round trip per buy closed on its fill bar + #: by its `target=entry` sentinel -- neither of which belongs in the pooled G2 sample. + accumulates: ClassVar[bool] = False #: Why the last `detect()` call declined, or `None` if it fired (or never recorded one). #: `strategy.engine.evaluate` merges it into the `engine.no_signal` event it already emits, #: so a cycle reporting `signals=0` can say whether price was 1% or 40% off the trigger. diff --git a/keel/strategy/rules/dca.py b/keel/strategy/rules/dca.py index a9ec8511..07f1d21d 100644 --- a/keel/strategy/rules/dca.py +++ b/keel/strategy/rules/dca.py @@ -17,7 +17,7 @@ from decimal import Decimal -from keel.strategy.rules.base import Rule, Setup +from keel.strategy.rules.base import Rule, Setup, completed_days from keel.types import Candle, Granularity _SECONDS_PER_DAY = 86_400 @@ -53,6 +53,9 @@ class docstring and the help system cannot drift apart. #: `granularity_param`: DCA decides on daily candles unconditionally, so there is no #: timeframe knob to persist and nothing to convert back. decimal_params = ("budget_usd", "dip_bonus_pct") + #: Accumulation, not round trips: the edge report gives it an accumulation row (buys, cost, + #: mark-to-market) instead of a backtest, and keeps it out of the pooled G2 sample (#821). + accumulates = True def __init__( self, @@ -84,7 +87,11 @@ def detect(self, candles_by_tf: dict[Granularity, list[Candle]]) -> Setup | None """Pure. On a cadence boundary, a market-buy long `Setup` sized to `budget_usd` (scaled up on dips); off-cadence (or with no daily candles), `None`. """ - candles = candles_by_tf.get(Granularity.ONE_DAY) + # COMPLETED days only (#821): the account sim's last daily bar is the still-forming day, + # stored with the completed day's OHLC -- reading it from hour 0 bought at a close that + # hadn't happened yet and put the cadence day one day ahead of live. Live already passes + # only closed candles, which `completed_days` keeps (see its docstring). + candles = completed_days(candles_by_tf) if not candles: return None diff --git a/keel/strategy/rules/turtle_breakout.py b/keel/strategy/rules/turtle_breakout.py index bdbac604..914040dc 100644 --- a/keel/strategy/rules/turtle_breakout.py +++ b/keel/strategy/rules/turtle_breakout.py @@ -64,11 +64,14 @@ from decimal import Decimal from keel.analysis.indicators import adx, atr, donchian_high, donchian_low, macd + +# The forming-day guard lives in `rules.base` since #821 (`Dca` needs the same rule). The +# private alias keeps every `turtle_breakout._completed_days` reference -- docstrings in +# `data.freshness`/`commands.activity`, and the schedule tests -- pointing at the real thing. from keel.strategy.rules.base import ParamSpec, Rule, Setup +from keel.strategy.rules.base import completed_days as _completed_days from keel.types import Candle, Granularity -_DAY_SECONDS = 24 * 60 * 60 - def _gap_pct(close: float, entry_level: float) -> float | None: """How far below the channel top `close` sits, in percent (negative once it has cleared it). @@ -81,33 +84,6 @@ def _gap_pct(close: float, entry_level: float) -> float | None: return (entry_level - close) / close * 100 -def _completed_days(candles_by_tf: dict[Granularity, list[Candle]]) -> list[Candle]: - """The `ONE_DAY` series with any still-FORMING last bar dropped. - - The account sim (`sim.portfolio_sim`) slices its daily series with - `bisect_right(daily_ts, t)` against the current hourly bar `t`, so its last daily bar - always CONTAINS that hourly bar -- it is the current, still-forming day, and its DB OHLC is - the completed day (lookahead if consumed intraday). - - The live agent (`agent.run_once`) passes an `ONE_HOUR` key too, but `data.market_feed` - persists only CLOSED candles, so its last daily bar has already closed. Keying the drop on - the mere PRESENCE of `ONE_HOUR` therefore threw away a completed day there and left the - rule deciding on a bar up to 48h old -- a day of lag on every breakout. - - So the test is where the hourly series SITS, not whether it exists: the newest daily bar is - still forming unless the newest hourly bar opens at or after that day's close. In the sim - that is never true (its hourly bar is by construction inside the day), so the sim's guard is - unchanged; in the live agent it is true from the first full hour of the next UTC day. - """ - daily = candles_by_tf.get(Granularity.ONE_DAY, []) - hourly = candles_by_tf.get(Granularity.ONE_HOUR) - if not hourly or not daily: - return daily - if hourly[-1].ts < daily[-1].ts + _DAY_SECONDS: - return daily[:-1] - return daily - - class TurtleBreakout(Rule): """Donchian-breakout trend-follower: ADX-gated entry, asymmetric channel exit, 2xATR stop. diff --git a/tests/sim/test_artifact.py b/tests/sim/test_artifact.py index 1eb26122..340bbca5 100644 --- a/tests/sim/test_artifact.py +++ b/tests/sim/test_artifact.py @@ -14,7 +14,7 @@ from keel.sim.artifact import _svg_bars, _svg_drawdown, _svg_line, render_html from keel.sim.benchmark import BenchmarkResult -from keel.sim.portfolio_sim import SimResult, SimTelemetry +from keel.sim.portfolio_sim import DcaSleeve, SimResult, SimTelemetry from keel.sim.report import GapItem, Verdict _HOUR = 3600 @@ -223,3 +223,37 @@ def test_svg_bars_renders_one_bar_per_asset(): def test_svg_bars_empty_dict_does_not_raise(): svg = _svg_bars({}) assert "Per-asset realized rule P&L") == 1 + assert html.count("

DCA sleeve (accumulation, marked to market)

") == 1 + section = html.split("

DCA sleeve (accumulation, marked to market)

", 1)[1] + table = section.split("", 1)[0] + header = [cell.split("")[0] for cell in table.split("")[1:]] + assert header == [ + "Asset", + "Buys", + "Qty", + "Cost basis", + "Last close", + "Value", + "Unrealized P&L", + ] + body_rows = table.split("", 1)[1].split("")[1:] + cells = [[c.split("")[0] for c in row.split("")[1:]] for row in body_rows] + assert cells == [["BTC", "3", "1.5", "150", "120", "180.0", "30.0"]] + + +def test_render_html_omits_the_sleeve_table_when_there_is_no_sleeve(): + html = render_html(_sim(), _benchmark(), _verdict(), _gaps(), _account_metrics()) + assert "DCA sleeve" not in html diff --git a/tests/sim/test_dca_sleeve.py b/tests/sim/test_dca_sleeve.py new file mode 100644 index 00000000..097dd09d --- /dev/null +++ b/tests/sim/test_dca_sleeve.py @@ -0,0 +1,341 @@ +"""#821: the REAL `Dca` rule through the account sim, the edge table and the rendered report. + +`test_portfolio_sim.py`'s DCA coverage used a stub that fires once, which is why none of this was +caught: the real rule fires on every hourly bar of a cadence day (24 buys, not one), read the +still-forming day, got N 0 in the edge table (it never saw daily candles), and its sleeve was +bought but never shown in the report. + +Fixtures are synthetic and exact: a flat $100 market (so every $50 buy is exactly 0.5 units at +zero fee/slippage), with the very last close moved to $120 so a mark-to-market differs from cost. +""" + +from __future__ import annotations + +from datetime import UTC, datetime +from decimal import Decimal + +from keel.commands.simulate import build_account_metrics +from keel.config import Caps, Config, DcaConfig, MarketDataConfig, SubscriptionConfig +from keel.sim import portfolio_sim, report +from keel.sim.benchmark import BenchmarkResult +from keel.sim.portfolio_sim import SimTelemetry +from keel.strategy.rules.base import Rule, Setup +from keel.strategy.rules.dca import Dca +from keel.types import Candle, Granularity + +_HOUR = 3_600 +_DAY = 86_400 +_DAYS = 60 +_START = int(datetime(2024, 1, 1, tzinfo=UTC).timestamp()) +_START_DAY = _START // _DAY +_BUDGET = Decimal("50") +_CADENCE = 7 +_ZERO = Decimal("0") + + +def _bar(ts: int, close: str = "100") -> Candle: + c = Decimal(close) + return Candle( + ts=ts, open=Decimal("100"), high=max(c, Decimal("100")), low=Decimal("100"), close=c, + volume=Decimal("10"), + ) # fmt: skip + + +def _market() -> dict[str, dict[Granularity, list[Candle]]]: + hourly = [_bar(_START + i * _HOUR) for i in range(_DAYS * 24)] + daily = [_bar(_START + d * _DAY) for d in range(_DAYS)] + hourly[-1] = _bar(hourly[-1].ts, "120") + daily[-1] = _bar(daily[-1].ts, "120") + return {"BTC": {Granularity.ONE_HOUR: hourly, Granularity.ONE_DAY: daily}} + + +def _cadence_days() -> list[int]: + """Epoch days in the window that are cadence days AND have a following day in the window + (the buy is decided once that day has closed, i.e. on the next day, and fills there).""" + days = range(_START_DAY, _START_DAY + _DAYS - 1) + return [d for d in days if d % _CADENCE == 0] + + +def _dca() -> Dca: + return Dca("BTC-USD", cadence_days=_CADENCE, budget_usd=_BUDGET) + + +def _config() -> Config: + return Config( + allowlist=["BTC"], + target_weights={}, + risk_pct=Decimal("0.02"), + caps=Caps( + max_per_order_usd=Decimal("1000000"), + max_per_day_usd=Decimal("1000000"), + max_exposure_usd=Decimal("1000000"), + max_per_asset_pct=Decimal("1"), + ), + market_data=MarketDataConfig(granularities=[], history_days=365), + subscription=SubscriptionConfig( + assumed_free_volume_usd=Decimal("1000000"), pacing="opportunistic" + ), + dca=DcaConfig(budget_usd=_BUDGET), + ) + + +def _run() -> portfolio_sim.SimResult: + market = _market() + hourly = market["BTC"][Granularity.ONE_HOUR] + return portfolio_sim.run( + [_dca()], + market, + _config(), + start_ts=hourly[0].ts, + end_ts=hourly[-1].ts, + monthly_contribution=Decimal("100000"), + fee_pct=_ZERO, + slippage_pct=_ZERO, + ) + + +# --------------------------------------------------------------------------- +# (a) the account sim buys once per cadence day, not once per hourly bar +# --------------------------------------------------------------------------- + + +def test_the_sim_buys_the_budget_once_per_cadence_day_not_every_hour(): + cadence = _cadence_days() + assert len(cadence) == 8 # the fixture really spans several cadence days + + result = _run() + + lot = result.dca_positions["BTC"] + # Flat $100 at zero cost: each $50 buy is exactly 0.5 units. Before #821 this was ~24x. + assert lot.qty == Decimal("0.5") * len(cadence) + assert lot.qty * lot.entry_fill == _BUDGET * len(cadence) + + buys = result.dca_buys + assert [b.decision_ts // _DAY for b in buys] == [d + 1 for d in cadence] + assert [b.notional for b in buys] == [_BUDGET] * len(cadence) + assert sum((b.notional for b in buys), _ZERO) == _BUDGET * len(cadence) + assert {(b.asset, b.rule_kind) for b in buys} == {("BTC", "dca")} + + +# --------------------------------------------------------------------------- +# (d) the edge table: DCA is its own accumulation row, never a round trip +# --------------------------------------------------------------------------- + + +class _OneShot(Rule): + """A risk-defined rule that wins once -- gives `__pooled__` something real to compare.""" + + name = "one_shot" + params: dict = {} + + def __init__(self, product_id: str, trigger_ts: int) -> None: + self.product_id = product_id + self.trigger_ts = trigger_ts + + def detect(self, candles_by_tf: dict[Granularity, list[Candle]]) -> Setup | None: + latest = candles_by_tf[Granularity.ONE_HOUR][-1] + if latest.ts != self.trigger_ts: + return None + return Setup( + product_id=self.product_id, + direction="long", + entry=Decimal("100"), + stop=Decimal("90"), + target=Decimal("110"), + context={}, + ts=latest.ts, + ) + + def exit_signal(self, held: Setup, candles_by_tf: dict[Granularity, list[Candle]]) -> bool: + return False + + def describe(self) -> dict: + return {"name": self.name, "params": self.params} + + +def test_edge_table_keeps_dca_out_of_the_round_trips_and_the_pool(): + market = _market() + other = _OneShot("BTC-USD", market["BTC"][Granularity.ONE_HOUR][5].ts) + + with_dca = report.edge_table([other, _dca()], market, fee_pct=_ZERO, slippage_pct=_ZERO) + without = report.edge_table([other], market, fee_pct=_ZERO, slippage_pct=_ZERO) + + assert list(with_dca) == ["one_shot:BTC", report.POOLED_KEY] + assert repr(with_dca[report.POOLED_KEY]) == repr(without[report.POOLED_KEY]) + # G2 is neither fed fake round trips nor a zero-trade sample for the DCA rule. + assert report.group_trades_by_class(with_dca, [_dca()]) == {} + + +def test_accumulation_table_reports_buys_cost_and_mark_for_a_real_dca(): + cadence = _cadence_days() + + rows = report.accumulation_table([_dca()], _market(), fee_pct=_ZERO, slippage_pct=_ZERO) + + assert list(rows) == ["dca:BTC"] + row = rows["dca:BTC"] + n = len(cadence) + assert row.buys == n + assert row.qty == Decimal("0.5") * n + assert row.cost_usd == _BUDGET * n + assert row.last_close == Decimal("120") + assert row.value_usd == Decimal("0.5") * n * Decimal("120") + assert row.unrealized_pnl == row.value_usd - row.cost_usd + + +def test_accumulation_table_charges_fee_and_slippage_in_the_cost_basis(): + fee, slip = Decimal("0.01"), Decimal("0.002") + + row = report.accumulation_table([_dca()], _market(), fee_pct=fee, slippage_pct=slip)["dca:BTC"] + + n = len(_cadence_days()) + per_buy = Decimal("0.5") * Decimal("100") * (1 + slip) * (1 + fee) + assert row.cost_usd == per_buy * n + + +def test_accumulation_table_ignores_risk_defined_rules(): + market = _market() + other = _OneShot("BTC-USD", market["BTC"][Granularity.ONE_HOUR][5].ts) + assert report.accumulation_table([other], market, fee_pct=_ZERO, slippage_pct=_ZERO) == {} + + +# --------------------------------------------------------------------------- +# (e) the account metrics expose the DCA sleeve, marked at the last close +# --------------------------------------------------------------------------- + + +def test_account_metrics_expose_the_sleeve_marked_to_market(): + result = _run() + market = _market() + hourly = market["BTC"][Granularity.ONE_HOUR] + + metrics = build_account_metrics(result, hourly[0].ts, hourly[-1].ts) + + sleeve = metrics["dca_sleeve"] + assert list(sleeve) == ["BTC"] + row = sleeve["BTC"] + n = len(_cadence_days()) + assert row.buys == n + assert row.qty == result.dca_positions["BTC"].qty + assert row.last_close == Decimal("120") + assert row.cost_usd == _BUDGET * n + assert row.value_usd == row.qty * Decimal("120") + assert row.unrealized_pnl == row.value_usd - row.cost_usd + assert row.unrealized_pnl == Decimal("10") * n # 0.5 units x $20 up, per buy + # The sleeve is not a closed trade and never inflates the round-trip count. + assert metrics["trade_count"] == 0 + assert metrics["per_asset_pnl"] == {} + + +# --------------------------------------------------------------------------- +# (f) the rendered report has both sections, as tables +# --------------------------------------------------------------------------- + + +def _table_under(md: str, heading: str) -> list[list[str]]: + """The rows (header first, separator dropped) of the first Markdown table after `heading`.""" + lines = md.splitlines() + start = lines.index(heading) + rows: list[list[str]] = [] + for line in lines[start + 1 :]: + if line.startswith("#"): + break + if line.startswith("|"): + cells = [c.strip() for c in line.strip("|").split("|")] + if not all(set(c) <= {"-"} for c in cells): + rows.append(cells) + elif rows: + break + return rows + + +def _benchmark() -> BenchmarkResult: + return BenchmarkResult( + name="bench", + equity_curve=[], + contributions=[], + ending_value=_ZERO, + total_return_pct=_ZERO, + max_drawdown_pct=_ZERO, + sharpe=_ZERO, + sortino=_ZERO, + return_per_drawdown=_ZERO, + ) + + +def test_render_markdown_shows_the_sleeve_and_the_accumulation_row(): + result = _run() + market = _market() + hourly = market["BTC"][Granularity.ONE_HOUR] + metrics = build_account_metrics(result, hourly[0].ts, hourly[-1].ts) + edge = report.edge_table([_dca()], market, fee_pct=_ZERO, slippage_pct=_ZERO) + accumulation = report.accumulation_table([_dca()], market, fee_pct=_ZERO, slippage_pct=_ZERO) + verdict = report.Verdict("TRAIN MORE", ["x"], True, False, False) + + md = report.render_markdown( + result, + edge, + metrics, + _benchmark(), + verdict, + report.analyze_gaps(SimTelemetry(), {}, move_threshold_pct=Decimal("0.05")), + accumulation=accumulation, + ) + + n = len(_cadence_days()) + header = ["Rule", "Buys", "Qty", "Cost basis", "Last close", "Value", "Unrealized P&L"] + acc = _table_under(md, "## DCA accumulation (not round trips)") + assert acc[0] == header + a = accumulation["dca:BTC"] + assert (a.buys, a.cost_usd, a.value_usd) == (n, _BUDGET * n, Decimal("60") * n) + assert acc[1:] == [ + [ + "dca:BTC", + str(n), + str(a.qty), + str(a.cost_usd), + str(a.last_close), + str(a.value_usd), + str(a.unrealized_pnl), + ] + ] + # The DCA rule has no row in the round-trip edge table. + edge_rows = _table_under(md, "## Edge table") + assert [r[0] for r in edge_rows[1:]] == [f"**{report.POOLED_KEY}**"] + + sleeve = _table_under(md, "### DCA sleeve (accumulation, marked to market)") + assert sleeve[0] == ["Asset", *header[1:]] + row = metrics["dca_sleeve"]["BTC"] + assert sleeve[1:] == [ + [ + "BTC", + str(n), + str(row.qty), + str(row.cost_usd), + str(row.last_close), + str(row.value_usd), + str(row.unrealized_pnl), + ] + ] + assert md.index("## Account results") < md.index( + "### DCA sleeve (accumulation, marked to market)" + ) + + +def test_per_asset_pnl_is_labelled_as_realized_rule_pnl(): + metrics = {"per_asset_pnl": {"ETH": Decimal("5")}, "dca_sleeve": {}} + + lines = report._render_account_section(metrics) + + label = "Per-asset realized rule P&L (closed round trips; the DCA sleeve is below):" + assert lines[lines.index(label) + 1 :] == ["- ETH: 5"] + assert "### DCA sleeve (accumulation, marked to market)" not in lines + + +def test_backtest_still_gives_dca_no_round_trips(): + """`keel rules promote`'s docstring relies on this: a DCA rule produces no backtest trades, + so `--force` stays its only way to paper. The accumulation row is not trades and nothing on + the promotion path reads it; the round-trip backtest itself is unchanged by #821.""" + from keel.strategy.backtest import backtest + + daily = _market()["BTC"][Granularity.ONE_DAY] + assert backtest(_dca(), daily, fee_pct=_ZERO, slippage_pct=_ZERO).n_trades == 0 diff --git a/tests/strategy/test_dca.py b/tests/strategy/test_dca.py index 6f0270d4..2f1cf49d 100644 --- a/tests/strategy/test_dca.py +++ b/tests/strategy/test_dca.py @@ -177,3 +177,59 @@ def test_rejects_non_positive_cadence(self) -> None: def test_rejects_non_positive_budget(self) -> None: with pytest.raises(ValueError): Dca(product_id="BTC-USD", budget_usd=Decimal("0")) + + +_HOUR = 3_600 + + +def _hour_bar(ts: int, price: str) -> Candle: + p = Decimal(price) + return Candle(ts=ts, open=p, high=p, low=p, close=p, volume=Decimal("1")) + + +class TestDcaDecidesOnCompletedDaysOnly: + """#821: the account sim hands `detect` a daily series whose last bar is the CURRENT, + still-forming day -- stored with the completed day's OHLC. `Dca` must not read it, exactly as + `TurtleBreakout` doesn't (`rules.base.completed_days`).""" + + # Day 7 is a cadence day for cadence_days=7 (7 % 7 == 0); day 8 is not. + + def test_forming_day_close_and_high_are_ignored_at_hour_zero(self) -> None: + """Hour 0 of day 8: day 7 (cadence) is the last COMPLETED day, day 8 is forming and its + stored close/high (999) are the future. The buy must be priced off day 7's close and its + dip measured against a high that excludes day 8.""" + rule = Dca(product_id="BTC-USD", cadence_days=7, budget_usd=Decimal("50")) + daily = [_candle(day=d, price="100") for d in range(8)] + [ + _candle(day=8, price="999", high="999") + ] + hourly = [_hour_bar(8 * _DAY, "100")] # the 00:00-01:00 bar of day 8 + + setup = rule.detect({Granularity.ONE_HOUR: hourly, Granularity.ONE_DAY: daily}) + + assert setup is not None + assert setup.ts == 7 * _DAY + assert setup.entry == Decimal("100") + assert setup.context["recent_high"] == Decimal("100") + + def test_a_forming_cadence_day_does_not_fire_before_it_closes(self) -> None: + """Hour 0 of day 7 (cadence): day 7 has not closed, day 6 is off-cadence -> no buy yet.""" + rule = Dca(product_id="BTC-USD", cadence_days=7, budget_usd=Decimal("50")) + daily = [_candle(day=d, price="100") for d in range(8)] # day 7 is forming + hourly = [_hour_bar(7 * _DAY, "100")] + + assert rule.detect({Granularity.ONE_HOUR: hourly, Granularity.ONE_DAY: daily}) is None + + def test_live_shaped_input_keeps_the_closed_last_day(self) -> None: + """Live (`agent.run_once`) persists only CLOSED candles: the last daily bar is day 7, + already closed, and the hourly series is past it. That bar must NOT be dropped -- live + behaviour is unchanged by the forming-day guard.""" + rule = Dca(product_id="BTC-USD", cadence_days=7, budget_usd=Decimal("50")) + daily = [_candle(day=d, price="100") for d in range(7)] + [_candle(day=7, price="120")] + for hourly_ts in (8 * _DAY, 8 * _DAY + _HOUR): # 00:00 and 01:00 bars of day 8 + hourly = [_hour_bar(hourly_ts - _HOUR, "120"), _hour_bar(hourly_ts, "120")] + + setup = rule.detect({Granularity.ONE_HOUR: hourly, Granularity.ONE_DAY: daily}) + + assert setup is not None + assert setup.ts == 7 * _DAY + assert setup.entry == Decimal("120")