From 31fdff39986438e57816c81bea8651a7bc9116ef Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Sun, 27 Sep 2026 19:00:41 -0400 Subject: [PATCH 1/3] fix(executor): size each DCA buy from the rule's own amount (#840) The live executor sized every DCA buy from the config's single `dca.budget_usd` ($50) and ignored the `size_usd` the rule computed from its own `budget_usd`. With the live rules at $40, $25 and $15, that spent ~$978/month against the rules' ~$467 and rail 14's $500 cap. `_build_intent` now sizes DCA from `setup.context["size_usd"]` and falls back to `config.dca.budget_usd` only when `size_usd` is absent or not a positive, finite number (a bool is refused). The source is logged as `executor.dca_sized` (INFO for rule, WARNING for the config fallback). Paper sizes through the same `_build_intent`, so it is fixed by the same change; the account sim already preferred `size_usd`. No rail, cap, guard or veto changes: the intent's notional is computed from the new qty, so rail 14 and every guard see the real, smaller order. The "config is the operator-facing dial" rationale is recorded as reversed by the operator on 2026-09-27; `dca.budget_usd`'s docs now call it the fallback. Co-Authored-By: Claude Opus 5.5 --- config.yaml | 4 + docs/RELEASING.md | 12 +- docs/operator-runbook.md | 4 +- keel/execution/equity.py | 7 +- keel/execution/executor.py | 73 +++++-- keel/templates/config.live.yaml | 4 + keel/templates/config.yaml | 4 + packages/keel-core/keel_core/config.py | 11 ++ tests/execution/test_executor.py | 252 +++++++++++++++++++++++++ tests/test_agent.py | 38 ++++ 10 files changed, 382 insertions(+), 27 deletions(-) diff --git a/config.yaml b/config.yaml index 34c4b466..177ea526 100644 --- a/config.yaml +++ b/config.yaml @@ -69,6 +69,10 @@ money_mgmt: streak_cooloff_days: 0 dca: + # FALLBACK only (#840). Each DCA buy -- live, paper and the account sim -- spends the RULE's own + # amount (`size_usd`, computed from the rule's `budget_usd`; see `keel rules list`). This + # value sizes a DCA buy only when a setup arrives without a positive `size_usd`, and the + # executor logs `executor.dca_sized` with `source=config` when it does. budget_usd: 50 cadence_days: 7 diff --git a/docs/RELEASING.md b/docs/RELEASING.md index 2ca72430..c54f74f2 100644 --- a/docs/RELEASING.md +++ b/docs/RELEASING.md @@ -82,12 +82,14 @@ A **fresh deployment** does not. `keel init` seeds every rule at `candidate` fro **constructor defaults**, so any parameter an operator tuned by hand silently reverts — a DCA rule deliberately set to `budget_usd: 25` comes back as the built-in `50`, unpromoted, on a box that otherwise looks correctly provisioned. Nothing errors. (A worked example, not a description of -today's deployment: the live DCA rule is now `50`, deliberately matching both the constructor -default and `config.dca.budget_usd`, which is the value the live executor actually spends. That -coincidence means the value alone can no longer prove the rule wasn't reseeded — `keel init`'s -default and the operator's intended value are now the same number. `test_committed_manifest_is_valid` +today's deployment.) This matters for money, not just +bookkeeping: since #840 the live executor spends each **rule's** `budget_usd` (the `size_usd` its +setup carries), with `config.dca.budget_usd` only as the fallback — so a reseeded $15 rule that +comes back at `50` really does spend $50 a buy. A rule whose intended value happens to equal the +constructor default cannot prove by its value alone that it wasn't reseeded, since `keel init`'s +default and the operator's intended value are then the same number. `test_committed_manifest_is_valid` in `tests/test_rule_manifest.py` covers the gap by also asserting every committed rule's *status* -is `live`, since `keel init` always seeds at `candidate` regardless of what the params say.) +is `live`, since `keel init` always seeds at `candidate` regardless of what the params say. `deploy/live-rules.json` is the committed record of the live deployment's rule set, so that state is a diff in a PR rather than a fact stored on one laptop: diff --git a/docs/operator-runbook.md b/docs/operator-runbook.md index 90023584..8d9806b7 100644 --- a/docs/operator-runbook.md +++ b/docs/operator-runbook.md @@ -54,8 +54,8 @@ This is not a trading decision the system can veto; it is an account setting onl > read** is reduced by pending purification — `mark_to_market − build_report(transactions). > total_owed_usd` (`keel/execution/equity.py::sizing_equity`). Today that is the paper account's > balance-derived seed (`paper.starting_equity_usd == 0`), which then sizes every paper fill; the live -> path sizes off `caps.max_exposure_usd` and DCA off `dca.budget_usd`, both operator constants immune -> by construction. Note the boundary: the **drawdown/HWM rail-11 equity is deliberately NOT purified** — +> path sizes off `caps.max_exposure_usd` and DCA off each rule's own `budget_usd` (with +> `dca.budget_usd` as the fallback, #840), all operator constants immune by construction. Note the boundary: the **drawdown/HWM rail-11 equity is deliberately NOT purified** — > it measures what the account actually holds, and a breaker must trip on real value, not on a > post-obligation fiction. Discharging the owed amount (`keel purification` to see it) is still your > act; keep the imported transaction ledger current so the subtraction sees what actually accrued. diff --git a/keel/execution/equity.py b/keel/execution/equity.py index fb1d2cf8..6ef5e76f 100644 --- a/keel/execution/equity.py +++ b/keel/execution/equity.py @@ -121,9 +121,10 @@ def sizing_equity(mark_to_market: Decimal, pending_purification: Decimal) -> Dec inflate the equity the sizing formula reads from" -- riba compounding into position size, a correctness bug independent of the fiqh point (KB §65.9: non-compliant income is given away, never recognised as trading capital). Every path that derives sizing equity from a - LIVE balance read must go through this helper; config-constant sizing inputs - (`caps.max_exposure_usd`, `dca.budget_usd`, a funded `paper.starting_equity_usd`) are - immune by construction and pass through unchanged. + LIVE balance read must go through this helper; constant sizing inputs + (`caps.max_exposure_usd`, a DCA rule's own `budget_usd` -- or `dca.budget_usd`, its + fallback (#840) -- and a funded `paper.starting_equity_usd`) are immune by construction and + pass through unchanged. Floored at zero: pending purification can exceed the mark-to-market read (a reward-heavy ledger against a mostly-withdrawn account), and a negative equity base would size a diff --git a/keel/execution/executor.py b/keel/execution/executor.py index 05c5821b..e806de09 100644 --- a/keel/execution/executor.py +++ b/keel/execution/executor.py @@ -12,10 +12,11 @@ **Sizing.** ENTER signals size via `sizing.size` (fixed-fractional risk, off the setup's entry/stop) for risk-defined rules, or `sizing.dca_size` (budget/price, no stop) for the DCA order class (`setup.context["order_class"] == "dca"` or `["no_stop"]`, matching -`strategy/engine.py`'s own class test). `execution/guards.py` documents the same design choice -this module reuses: `config.caps.max_exposure_usd` stands in for account equity in -fixed-fractional sizing, since neither module has a separate equity oracle -- it is the funded -trading-capital ceiling (§2.8) `max_per_asset_pct` is already a fraction of. +`strategy/engine.py`'s own class test). A DCA budget is the RULE's `setup.context["size_usd"]`, +with `config.dca.budget_usd` only as the fallback (`_dca_budget`, #840). `execution/guards.py` +documents the same design choice this module reuses: `config.caps.max_exposure_usd` stands in +for account equity in fixed-fractional sizing, since neither module has a separate equity oracle +-- it is the funded trading-capital ceiling (§2.8) `max_per_asset_pct` is already a fraction of. **EXIT signals** carry no `setup` (`strategy/rules/base.Signal` docstring: `setup` is `None` for EXIT/NONE) -- the position being closed is reconstructed from the orders audit log @@ -335,6 +336,28 @@ def _is_dca_setup(context: dict[str, Any]) -> bool: return bool(context.get("no_stop")) or context.get("order_class") == "dca" +def _dca_budget(context: dict[str, Any], fallback: Decimal) -> tuple[Decimal, str]: + """`(USD to spend, where it came from)` for a DCA setup -- `"rule"` or `"config"` (#840). + + The RULE's amount wins: `context["size_usd"]`, which `Dca.detect` computes as + `budget_usd x (1 + dip bonus)`. `fallback` (the config's `dca.budget_usd`) is used only when + that key is absent or is not a positive, finite number. "Positive" alone is not enough: + `Decimal('Infinity') > 0`, and an infinite `budget_usd` becomes an infinite `size_usd` with + nothing raising (`commands/rules.py`'s non-finite-param check says why). A `bool` is an `int` + to Python and is refused too -- `True` is not an amount of dollars. + + The fallback is not checked here: a zero or negative config budget still reaches + `sizing.dca_size` exactly as it always did, and the rails see whatever notional it yields. + """ + size_usd = context.get("size_usd") + if isinstance(size_usd, bool) or not isinstance(size_usd, Decimal | int | float): + return fallback, "config" + amount = Decimal(str(size_usd)) if isinstance(size_usd, float) else Decimal(size_usd) + if not amount.is_finite() or amount <= 0: + return fallback, "config" + return amount, "rule" + + #: How long a withdrawal-capability attestation stays fresh (§65.4). Deliberately short: the #: attestation is about the account's CURRENT state, and a freeze can appear at any time, so a #: stale attestation is no better than none. 7 days. @@ -733,17 +756,31 @@ def _build_intent( is_dca = _is_dca_setup(setup.context) if is_dca: - # CAREFUL: the live path sizes DCA from the CONFIG's `dca.budget_usd`, and ignores - # the RULE's own `budget_usd` / the `size_usd` the rule computed from it (which is - # sitting right there in `setup.context`). That is deliberate -- the config is the - # operator-facing dial and a rule row is not reviewed on every deploy -- but it means - # the two can disagree silently, and a rule row saying 25 while the config says 50 - # spends 50. It surprised us once; do not assume the rule's number is what moves. - # The account simulator (`sim/portfolio_sim.py`) prefers `context["size_usd"]` and - # only falls back to this config value, so a divergence also makes the sim and the - # live path model different position sizes. Keep rule, config and - # `deploy/live-rules.json` in agreement. - qty = sizing.dca_size(config.dca.budget_usd, setup.entry) + # CAREFUL: DCA is sized from the RULE's amount -- `setup.context["size_usd"]`, which + # `Dca.detect` computes from the rule's own `budget_usd` -- and the config's + # `dca.budget_usd` is only the FALLBACK for a setup without a usable one (#840). + # This reverses the earlier design, where live always spent the config value on the + # grounds that "the config is the operator-facing dial and a rule row is not reviewed + # on every deploy". The operator reversed that on 2026-09-27: with rules at $40, $25 + # and $15 and the config at $50, every buy spent $50 -- about $978 a month against + # the rules' $467 and rail 14's $500 cap. Paper sizes through this same function, and + # the account sim (`sim/portfolio_sim.py`) already preferred `size_usd`, so all three + # now agree. Which source sized the order is logged (`executor.dca_sized`), because a + # fallback means a setup arrived without the rule's number and deserves a look. + # The rails are unchanged: `notional` below is computed from this qty, so rail 14 and + # every other guard see the smaller, real order. + budget, source = _dca_budget(setup.context, config.dca.budget_usd) + log_event( + logger, + logging.INFO if source == "rule" else logging.WARNING, + "executor.dca_sized", + product=signal.product_id, + rule=signal.rule_name, + rule_id=signal.rule_id, + source=source, + budget_usd=str(budget), + ) + qty = sizing.dca_size(budget, setup.entry) stop = None else: equity = ( @@ -1876,8 +1913,10 @@ def _order_spec(intent: OrderIntent) -> OrderSpec: finer than the product's increment, which is how the first live `turtle_breakout` entry was rejected (order 3, 2026-08-22, `quote_size: "23.00803473938010547532738517"`). - DCA never tripped this only because its `budget_usd` is a round constant: `"50.000...0"` is - 26 decimal places too, and the venue accepts it because the VALUE is exactly 50. + DCA never tripped this only because its budget was a round constant: `"50.000...0"` is + 26 decimal places too, and the venue accepts it because the VALUE is exactly 50. Since #840 + the budget is the rule's `size_usd`, which a non-zero dip bonus makes non-round; the BUY + quantization below covers it like any other entry. **SELL is quantized too (#516), but its UNKNOWN case is the opposite of BUY's, deliberately.** diff --git a/keel/templates/config.live.yaml b/keel/templates/config.live.yaml index 17ce8b33..27108026 100644 --- a/keel/templates/config.live.yaml +++ b/keel/templates/config.live.yaml @@ -80,6 +80,10 @@ money_mgmt: streak_cooloff_days: 0 dca: + # FALLBACK only (#840). Each DCA buy -- live, paper and the account sim -- spends the RULE's own + # amount (`size_usd`, computed from the rule's `budget_usd`; see `keel rules list`). This + # value sizes a DCA buy only when a setup arrives without a positive `size_usd`, and the + # executor logs `executor.dca_sized` with `source=config` when it does. budget_usd: 50 cadence_days: 7 diff --git a/keel/templates/config.yaml b/keel/templates/config.yaml index 34c4b466..177ea526 100644 --- a/keel/templates/config.yaml +++ b/keel/templates/config.yaml @@ -69,6 +69,10 @@ money_mgmt: streak_cooloff_days: 0 dca: + # FALLBACK only (#840). Each DCA buy -- live, paper and the account sim -- spends the RULE's own + # amount (`size_usd`, computed from the rule's `budget_usd`; see `keel rules list`). This + # value sizes a DCA buy only when a setup arrives without a positive `size_usd`, and the + # executor logs `executor.dca_sized` with `source=config` when it does. budget_usd: 50 cadence_days: 7 diff --git a/packages/keel-core/keel_core/config.py b/packages/keel-core/keel_core/config.py index ecc2755a..8cdfaa0c 100644 --- a/packages/keel-core/keel_core/config.py +++ b/packages/keel-core/keel_core/config.py @@ -148,6 +148,17 @@ class MoneyMgmtConfig: @dataclass(frozen=True) class DcaConfig: + """The `dca:` block. `budget_usd` is a FALLBACK, not what a DCA buy spends (#840). + + Every DCA buy -- live (`execution.executor._build_intent`), paper (the same function) and + the account sim (`sim.portfolio_sim`) -- is sized from the RULE's own amount, the + `size_usd` that `Dca.detect` puts in its setup's context (`budget_usd x (1 + dip bonus)`). + `budget_usd` here sizes a DCA buy only when a setup carries no positive, finite `size_usd`, + and the executor logs `executor.dca_sized` with `source="config"` when it does. Until + 2026-09-27 live spent this value on every buy and ignored the rule's; the operator reversed + that after the $40/$25/$15 rules were each spending the $50 configured here. + """ + budget_usd: Decimal = Decimal("0") cadence_days: int = 7 diff --git a/tests/execution/test_executor.py b/tests/execution/test_executor.py index cdc4a40f..4ae3ad4a 100644 --- a/tests/execution/test_executor.py +++ b/tests/execution/test_executor.py @@ -14,6 +14,7 @@ import json import logging +import re import sqlite3 from decimal import Decimal from typing import Any @@ -824,6 +825,257 @@ def test_dca_signal_exempt_from_averaging_into_losers_but_bound_by_allowlist(rep assert any(v.startswith("halal_allowlist") for v in result.vetoed_by) +# -- DCA sizes from the RULE's amount, not the config's (#840) --------------------------------- +# +# The live book runs DCA rules at $40, $25 and $15 while `config.dca.budget_usd` is 50. Sizing +# every buy from the config spent ~$978/month against the rules' ~$467 and rail 14's $500 cap. +# `_config()` above sets `dca.budget_usd=50`, so every amount below is deliberately NOT 50: a +# test that asserted 50 could not tell the rule's number from the config's. + + +def _dca_signal_sized(size_usd: Any, *, include: bool = True) -> Signal: + """A DCA ENTER whose setup carries `size_usd` (or no such key when `include=False`), at an + entry of 50,000 -- the price `FakeBroker`'s book is quoted around.""" + context: dict[str, Any] = {"order_class": "dca", "no_stop": True} + if include: + context["size_usd"] = size_usd + return _dca_signal( + setup=_setup(stop=Decimal("0"), target=Decimal("50000"), context=context), + ) + + +def _dca_sizing_events(caplog: pytest.LogCaptureFixture) -> list[dict[str, Any]]: + from keel_core.telemetry import _FIELDS_ATTR + + return [ + getattr(r, _FIELDS_ATTR) for r in caplog.records if r.getMessage() == "executor.dca_sized" + ] + + +def test_live_dca_sizes_from_the_rules_size_usd_not_the_config_budget(repo, caplog): + """(a) A $25 rule on a $50 config places a $25 order: qty = 25 / 50,000 = 0.0005.""" + broker = FakeBroker() + + with caplog.at_level(logging.INFO, logger="keel.execution.executor"): + result = execute( + _dca_signal_sized(Decimal("25")), + broker, + repo, + _config(), + mode="autonomous", + now_ts=NOW_TS, + ) + + assert result.placed is True, result.vetoed_by + order = repo.get_order(result.order_id) + assert order["qty"] == Decimal("0.0005") + assert order["qty"] * Decimal("50000") == Decimal("25") + # Exactly one order went to the venue, and it is the $25 one. + assert len(broker.place_calls) == 1 + assert broker.place_calls[0]["spec"].quote_size == Decimal("25") + # Which source sized it is recorded, once. + events = _dca_sizing_events(caplog) + assert len(events) == 1 + assert events[0]["source"] == "rule" + assert Decimal(events[0]["budget_usd"]) == Decimal("25") + + +@pytest.mark.parametrize( + ("size_usd", "include"), + [ + (None, False), # key absent + (None, True), # key present, value None + (Decimal("0"), True), + (Decimal("-25"), True), + (Decimal("Infinity"), True), # `> 0` passes it; see `commands/rules.py` (#840) + (Decimal("NaN"), True), + ("25", True), # not a number + (True, True), # a bool is an int in Python; it is not an amount + ], + ids=["absent", "none", "zero", "negative", "infinity", "nan", "string", "bool"], +) +def test_live_dca_falls_back_to_the_config_budget_without_a_usable_size_usd( + repo, caplog, size_usd, include +): + """(b) No positive, finite `size_usd` -> the config's `dca.budget_usd` (50) sizes the buy, + and the fallback is recorded as such.""" + broker = FakeBroker() + + with caplog.at_level(logging.INFO, logger="keel.execution.executor"): + result = execute( + _dca_signal_sized(size_usd, include=include), + broker, + repo, + _config(), + mode="autonomous", + now_ts=NOW_TS, + ) + + assert result.placed is True, result.vetoed_by + order = repo.get_order(result.order_id) + assert order["qty"] == Decimal("50") / Decimal("50000") + events = _dca_sizing_events(caplog) + assert len(events) == 1 + assert events[0]["source"] == "config" + assert Decimal(events[0]["budget_usd"]) == Decimal("50") + + +def test_a_dca_rules_detect_feeds_the_live_order_size_end_to_end(repo): + """(c) The context key is the one `Dca.detect` actually writes -- pinned by driving the real + rule, not by naming the key in a hand-built context. A $15 rule at a 30,000 close buys + 15 / 30,000 = 0.0005, not the config's 50 / 30,000.""" + from keel.strategy.rules.dca import Dca + from keel.types import Candle, Granularity + + day = NOW_TS // 86_400 + price = Decimal("30000") + candle = Candle( + ts=day * 86_400, open=price, high=price, low=price, close=price, volume=Decimal("1") + ) + rule = Dca(product_id="BTC-USD", cadence_days=1, budget_usd=Decimal("15")) + setup = rule.detect({Granularity.ONE_DAY: [candle]}) + assert setup is not None + signal = _dca_signal(setup=setup, rule_name=rule.name) + broker = FakeBroker( + preview={ + "order_total": Decimal("15.00"), + "commission_total": Decimal("0"), + "quote_size": Decimal("15"), + "base_size": Decimal("0.0005"), + "best_bid": Decimal("29999"), + "best_ask": Decimal("30000"), + } + ) + + result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) + + assert result.placed is True, result.vetoed_by + order = repo.get_order(result.order_id) + assert order["qty"] == Decimal("0.0005") + assert order["qty"] * price == Decimal("15") + + +def test_a_dip_scaled_dca_buy_spends_size_usd_not_the_rules_base_budget(repo): + """(c') `size_usd`, not the rule's `budget_usd`: they are equal when `dip_bonus_pct` is 0 + (every live rule today), so only a dip tells them apart. $15 base, 25% below the 40,000 + high, 1% extra per point -> 15 x 1.25 = $18.75 at 30,000 = 0.000625.""" + from keel.strategy.rules.dca import Dca + from keel.types import Candle, Granularity + + day = NOW_TS // 86_400 + high, close = Decimal("40000"), Decimal("30000") + candles = [ + Candle( + ts=(day - 1) * 86_400, open=high, high=high, low=high, close=high, volume=Decimal("1") + ), + Candle( + ts=day * 86_400, open=close, high=close, low=close, close=close, volume=Decimal("1") + ), + ] + rule = Dca( + product_id="BTC-USD", + cadence_days=1, + budget_usd=Decimal("15"), + dip_bonus_pct=Decimal("1"), + ) + setup = rule.detect({Granularity.ONE_DAY: candles}) + assert setup is not None + assert setup.context["budget_usd"] == Decimal("15") + assert setup.context["size_usd"] == Decimal("18.75") + broker = FakeBroker( + preview={ + "order_total": Decimal("18.75"), + "commission_total": Decimal("0"), + "quote_size": Decimal("18.75"), + "base_size": Decimal("0.000625"), + "best_bid": Decimal("29999"), + "best_ask": Decimal("30000"), + } + ) + + result = execute( + _dca_signal(setup=setup, rule_name=rule.name), + broker, + repo, + _config(), + mode="autonomous", + now_ts=NOW_TS, + ) + + assert result.placed is True, result.vetoed_by + assert repo.get_order(result.order_id)["qty"] == Decimal("0.000625") + assert broker.place_calls[0]["spec"].quote_size == Decimal("18.75") + + +def _seed_month_buy_spend(repo: Repository, usd: Decimal) -> None: + """A filled live ETH BUY this month worth `usd` -- rail 14's month-to-date figure. ETH, not + BTC, so the DCA order under test is not averaging into anything.""" + repo.insert_order( + dict( + mode="live", + product_id="ETH-USD", + side=Side.BUY.value, + order_type="market", + qty=Decimal("1"), + limit_price=usd, + status="filled", + fee=Decimal("0"), + created_at=NOW_TS, + updated_at=NOW_TS, + ) + ) + + +def _dca_intent_notional(repo: Repository, size_usd: Decimal) -> Decimal: + intent = executor._build_intent(_dca_signal_sized(size_usd), None, repo, _config(), NOW_TS) + assert intent is not None + return intent.notional + + +def test_rail_14_sees_the_rules_true_notional_and_admits_a_25_dollar_buy_that_fits(repo): + """(e) $460 spent + a $25 DCA = $485, inside a $485 cap -> placed. Under the old sizing the + guard saw the config's $50 ($510) and vetoed a buy the rule never asked for.""" + _attest(repo, free_volume_usd=Decimal("485")) + _seed_month_buy_spend(repo, Decimal("460")) + assert guards._monthly_buy_spend_usd(repo, NOW_TS) == Decimal("460") + assert _dca_intent_notional(repo, Decimal("25")) == Decimal("25") + broker = FakeBroker() + + result = execute( + _dca_signal_sized(Decimal("25")), broker, repo, _config(), "autonomous", now_ts=NOW_TS + ) + + assert result.placed is True, result.vetoed_by + assert len(broker.place_calls) == 1 + + +def test_rail_14_still_vetoes_a_25_dollar_dca_buy_that_would_cross_the_cap(repo): + """(e) The rail is not loosened: $460 + $25 = $485 > a $480 cap -> vetoed, and the veto + names the rule's $25, not the config's $50.""" + _attest(repo, free_volume_usd=Decimal("480")) + _seed_month_buy_spend(repo, Decimal("460")) + broker = FakeBroker() + + result = execute( + _dca_signal_sized(Decimal("25")), broker, repo, _config(), "autonomous", now_ts=NOW_TS + ) + + assert result.placed is False + vetoes = [v for v in result.vetoed_by if v.startswith("monthly_subscription_allowance")] + assert len(vetoes) == 1 + pattern = r"BUY spend (\S+) \+ (\S+) = (\S+) exceeds the allowance cap (\S+)" + match = re.search(pattern, vetoes[0]) + assert match is not None, vetoes[0] + spent, notional, projected, cap = (Decimal(g) for g in match.groups()) + assert (spent, notional, projected, cap) == ( + Decimal("460"), + Decimal("25"), + Decimal("485"), + Decimal("480"), + ) + assert broker.place_calls == [] + + # -- EXIT signals ------------------------------------------------------------------------------ diff --git a/tests/test_agent.py b/tests/test_agent.py index 459690c4..c8bafe7a 100644 --- a/tests/test_agent.py +++ b/tests/test_agent.py @@ -3194,6 +3194,44 @@ def test_paper_enter_sizes_off_paper_equity(repo): assert orders[0]["qty"] != Decimal("1"), "must not fill the old fixed 1-unit qty" +def test_paper_dca_fill_is_sized_from_the_rules_amount_not_the_config_budget(repo): + """(#840) Paper sizes through the same `executor._build_intent` as live, so a $15 DCA rule + on a $50 config fills $15 on paper too -- live, paper and the account sim agree. Driven by + the real `Dca.detect`, so the context key it writes is the one being read.""" + from keel.strategy.paper import PaperTrader + + trader = PaperTrader(repo) + trader.seed_cash(Decimal("30000"), now_ts=1_000) + repo.set_state("last_feed_ts", 90_000) + config = _paper_config() + assert config.dca.budget_usd == Decimal("50") + price = Decimal("30") + candle = Candle(ts=0, open=price, high=price, low=price, close=price, volume=Decimal("1")) + rule = Dca(product_id=PRODUCT, cadence_days=1, budget_usd=Decimal("15")) + setup = rule.detect({Granularity.ONE_DAY: [candle]}) + assert setup is not None + signal = Signal( + rule_name=rule.name, + product_id=PRODUCT, + action=Action.ENTER, + side=Side.BUY, + setup=setup, + cts_score=0, + entry_technique="market", + ts=setup.ts, + ) + + result = agent._paper_enter( + trader, signal, repo, config, now_ts=90_000, paper_equity=Decimal("30000") + ) + + assert result.placed, result + orders = repo.get_orders(mode="paper") + assert len(orders) == 1 + assert orders[0]["qty"] == Decimal("0.5") # 15 / 30, not 50 / 30 + assert orders[0]["qty"] * price == Decimal("15") + + def test_paper_mode_never_runs_the_entry_spread_gate(repo): """#350's max-spread gate is live-path ONLY: paper fills are synthetic and see no book, so `_paper_enter` never previews an order and the gate (fail-closed on an unreadable book for From 8205d37f4b25a83cd3ad1f23d8a619aacb078fd9 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Sun, 27 Sep 2026 19:50:01 -0400 Subject: [PATCH 2/3] fix(executor): skip a DCA buy whose rule computed an invalid size_usd (#840) Round-1 review on #843 found three issues; this addresses all three. 1. (critical, missing test at executor.py:355) Kept the prior fixer's int/float size_usd test and proved it bites: mutating `Decimal(str(size_usd))` to `Decimal(size_usd)` is killed by the float_inexact (0.1) case. 2. (defect #844, RELEASING.md:90 / test_rule_manifest.py) Fixed the dangling "the value check would catch on its own" reference in assertion (2)'s failure message -- there is no separate value check since the AGREEMENT assertion was removed; the message now says this same per-key comparison catches it. Also corrected the "$40, $25 and $15" claim in test_rule_manifest.py and scripts/rule_manifest.py: deploy/live-rules.json defines a single DCA rule, at $50, so the text no longer asserts a rule mix the repo doesn't show. 3. (suggestion, config.py:156) New orchestrator ruling, implemented with TDD (failing tests first): - `setup.context["size_usd"]` PRESENT but invalid (<=0, NaN, +-Infinity, a bool, or non-numeric) now SKIPS the DCA buy on every path -- live, paper, and the account sim -- instead of falling back to `config.dca.budget_usd`. A rule that computed an invalid amount meant to buy less; falling back would spend more than it asked for. - `config.dca.budget_usd` remains the fallback ONLY when `size_usd` is ABSENT (key missing or `None`), unchanged from #840. - New `executor.DcaSizeInvalid`, raised by `_dca_budget` and caught in `execute()` (live, logs `executor.dca_size_invalid` WARNING) and `agent._paper_enter` (paper, logs `agent.paper_dca_size_invalid`). `_build_intent`'s `None` return keeps its existing "EXIT, nothing open" meaning; the invalid-size_usd case uses a distinct exception instead. - `keel.sim.portfolio_sim._process_dca_signals` now shares `executor._dca_budget` instead of its own looser `setup.context.get("size_usd") or config.dca.budget_usd`, so all three paths agree on what "usable" means. - `Dca.__init__` now raises `ValueError` for `dip_bonus_pct < 0` (0 still allowed) -- a negative value would shrink the budget on a dip, which the executor would then treat as an invalid size_usd and skip. No existing rule, fixture, or `deploy/live-rules.json` used a negative value. - Rewrote `DcaConfig`'s docstring and the three YAML `dca:` comments (config.yaml, keel/templates/config.yaml, keel/templates/config.live.yaml) to describe this behaviour identically. `config.yaml` remains byte-identical to keel/templates/config.yaml. Mutation-proved (diffed against a saved copy before running tests, then restored): present-invalid falling back instead of skipping; None skipping instead of falling back; dropping the dip_bonus_pct check; the sim reverting to its old `or` fallback; and the float str() mutant from item 1. Each is killed by a test. Co-Authored-By: Claude Opus 5.5 --- config.yaml | 12 ++- keel/agent.py | 31 ++++++- keel/execution/executor.py | 104 ++++++++++++++++++----- keel/sim/portfolio_sim.py | 12 ++- keel/strategy/rules/dca.py | 10 +++ keel/templates/config.live.yaml | 12 ++- keel/templates/config.yaml | 12 ++- packages/keel-core/keel_core/config.py | 31 +++++-- scripts/rule_manifest.py | 9 +- tests/execution/test_executor.py | 109 ++++++++++++++++++++++--- tests/sim/test_portfolio_sim.py | 84 ++++++++++++++++++- tests/strategy/test_dca.py | 12 +++ tests/test_agent.py | 60 ++++++++++++++ tests/test_rule_manifest.py | 92 ++++++++++----------- 14 files changed, 477 insertions(+), 113 deletions(-) diff --git a/config.yaml b/config.yaml index 177ea526..47508079 100644 --- a/config.yaml +++ b/config.yaml @@ -69,10 +69,14 @@ money_mgmt: streak_cooloff_days: 0 dca: - # FALLBACK only (#840). Each DCA buy -- live, paper and the account sim -- spends the RULE's own - # amount (`size_usd`, computed from the rule's `budget_usd`; see `keel rules list`). This - # value sizes a DCA buy only when a setup arrives without a positive `size_usd`, and the - # executor logs `executor.dca_sized` with `source=config` when it does. + # FALLBACK only (#840). Each DCA buy -- live, paper and the account sim -- spends the RULE's + # own amount (`size_usd`, computed from the rule's `budget_usd`; see `keel rules list`), and + # all three paths share one predicate (`executor._dca_budget`) for when this value applies. + # It sizes a buy ONLY when a setup's `size_usd` is ABSENT (the key missing, or `None`); the + # executor logs `executor.dca_sized` with `source=config` when it falls back. A setup that + # DOES carry a `size_usd`, but an unusable one -- zero, negative, non-finite, or not a number + # -- is never sized from this value: that buy is SKIPPED instead, because a rule meant to buy + # less must never spend more. budget_usd: 50 cadence_days: 7 diff --git a/keel/agent.py b/keel/agent.py index c7be07dd..ccdb8d1a 100644 --- a/keel/agent.py +++ b/keel/agent.py @@ -789,6 +789,13 @@ def _paper_enter( max_exposure` proxy `_build_intent` falls back to absent an override -- that proxy only ever existed to gate the guard check, and sizing the fill off it would score the track record on trades no real paper balance could have produced. + + A DCA setup whose `size_usd` is PRESENT but not usable makes `_build_intent` raise + `DcaSizeInvalid` (orchestrator ruling 2026-09-27, mirroring `executor.execute`'s own catch): + caught here too, before any paper fill, and reported as a not-placed result rather than + promoted on a trade the rule never asked for. Paper scores the promotion gate on its + recorded fills, so silently resizing to `config.dca.budget_usd` here would let a broken rule + promote on evidence live would have refused to produce. """ def _result(placed, order_id=None, vetoed_by=None, reason=""): @@ -800,9 +807,27 @@ def _result(placed, order_id=None, vetoed_by=None, reason=""): reason=reason, ) - intent = executor._build_intent( - signal, None, repo, config, now_ts, equity_override=paper_equity - ) + try: + intent = executor._build_intent( + signal, None, repo, config, now_ts, equity_override=paper_equity + ) + except executor.DcaSizeInvalid as exc: + log_event( + logger, + logging.WARNING, + "agent.paper_dca_size_invalid", + product=signal.product_id, + rule=signal.rule_name, + rule_id=signal.rule_id, + size_usd=repr(exc.size_usd), + ) + return _result( + False, + reason=( + f"paper: dca: rule computed an invalid size_usd={exc.size_usd!r}; buy skipped, " + "not sized from config" + ), + ) if intent is None: return _result(False, reason="paper: nothing to size") diff --git a/keel/execution/executor.py b/keel/execution/executor.py index e806de09..62746b32 100644 --- a/keel/execution/executor.py +++ b/keel/execution/executor.py @@ -13,7 +13,10 @@ entry/stop) for risk-defined rules, or `sizing.dca_size` (budget/price, no stop) for the DCA order class (`setup.context["order_class"] == "dca"` or `["no_stop"]`, matching `strategy/engine.py`'s own class test). A DCA budget is the RULE's `setup.context["size_usd"]`, -with `config.dca.budget_usd` only as the fallback (`_dca_budget`, #840). `execution/guards.py` +with `config.dca.budget_usd` only as the fallback for a setup carrying no `size_usd` at all +(`_dca_budget`, #840). A `size_usd` that is present but not usable does not fall back to the +config -- it skips the buy instead (`DcaSizeInvalid`, orchestrator ruling 2026-09-27), since a +rule meant to buy less must never spend more. `execution/guards.py` documents the same design choice this module reuses: `config.caps.max_exposure_usd` stands in for account equity in fixed-fractional sizing, since neither module has a separate equity oracle -- it is the funded trading-capital ceiling (§2.8) `max_per_asset_pct` is already a fraction of. @@ -195,11 +198,38 @@ def execute( means the order is not placed (fails closed, never silently proceeds). `mode="autonomous"` places without a prompt but is *not* exempt from `guards.check` -- rails run before every order in every mode, un-overridable, per the main spec §14. + + A DCA setup whose `size_usd` is PRESENT but not usable raises `DcaSizeInvalid` out of + `_build_intent`/`_dca_budget`; caught here, before any broker call, and reported as a + not-placed `ExecutionResult` rather than allowed to propagate -- `None` already means "EXIT + with nothing open" for this function's return, so a distinct exception is the signal that + does not collide with that meaning (orchestrator ruling 2026-09-27). """ if now_ts is None: now_ts = int(time.time()) - intent = _build_intent(signal, broker, repo, config, now_ts) + try: + intent = _build_intent(signal, broker, repo, config, now_ts) + except DcaSizeInvalid as exc: + log_event( + logger, + logging.WARNING, + "executor.dca_size_invalid", + product=signal.product_id, + rule=signal.rule_name, + rule_id=signal.rule_id, + size_usd=repr(exc.size_usd), + ) + return ExecutionResult( + placed=False, + order_id=None, + vetoed_by=[], + preview=None, + reason=( + f"dca: rule computed an invalid size_usd={exc.size_usd!r}; buy skipped, " + "not sized from config" + ), + ) if intent is None: return ExecutionResult( placed=False, @@ -336,25 +366,50 @@ def _is_dca_setup(context: dict[str, Any]) -> bool: return bool(context.get("no_stop")) or context.get("order_class") == "dca" +class DcaSizeInvalid(Exception): + """Raised by `_dca_budget` when `setup.context["size_usd"]` is PRESENT but not a usable + amount (orchestrator ruling 2026-09-27, follow-up to #840). + + A `size_usd` that is merely ABSENT (the key missing, or explicitly `None`) means the rule + left no opinion, and `config.dca.budget_usd` fills in exactly as it always has. But a rule + that computed a size_usd of 0, negative, non-finite, or otherwise garbage HAD an opinion: it + meant to buy less, or not at all. Falling back to the config in that case would spend MORE + than the rule asked for, and a rule meant to buy less must never spend more. So this is not + a fallback case -- it is a refusal, caught by `execute()` (live) and `agent._paper_enter` + (paper), both of which skip the buy entirely rather than resize it. Carries the raw + offending value so the caller can log/report it without re-deriving it. + """ + + def __init__(self, size_usd: Any) -> None: + self.size_usd = size_usd + super().__init__(f"invalid size_usd={size_usd!r}") + + def _dca_budget(context: dict[str, Any], fallback: Decimal) -> tuple[Decimal, str]: """`(USD to spend, where it came from)` for a DCA setup -- `"rule"` or `"config"` (#840). The RULE's amount wins: `context["size_usd"]`, which `Dca.detect` computes as - `budget_usd x (1 + dip bonus)`. `fallback` (the config's `dca.budget_usd`) is used only when - that key is absent or is not a positive, finite number. "Positive" alone is not enough: - `Decimal('Infinity') > 0`, and an infinite `budget_usd` becomes an infinite `size_usd` with - nothing raising (`commands/rules.py`'s non-finite-param check says why). A `bool` is an `int` - to Python and is refused too -- `True` is not an amount of dollars. - - The fallback is not checked here: a zero or negative config budget still reaches + `budget_usd x (1 + dip bonus)`. `fallback` (the config's `dca.budget_usd`) is used ONLY when + that key is ABSENT -- missing, or explicitly `None`. A size_usd that is PRESENT but not a + positive, finite number raises `DcaSizeInvalid` instead of falling back (see that class's + docstring for why): "positive" alone is not enough, since `Decimal('Infinity') > 0`, and an + infinite `budget_usd` becomes an infinite `size_usd` with nothing raising + (`commands/rules.py`'s non-finite-param check says why). A `bool` is an `int` to Python and + is refused too -- `True` is not an amount of dollars. + + The fallback ITSELF is not checked here: a zero or negative config budget still reaches `sizing.dca_size` exactly as it always did, and the rails see whatever notional it yields. + That is unchanged from before this ruling -- only a PRESENT, invalid `size_usd` newly skips + instead of falling back. """ size_usd = context.get("size_usd") - if isinstance(size_usd, bool) or not isinstance(size_usd, Decimal | int | float): + if size_usd is None: return fallback, "config" + if isinstance(size_usd, bool) or not isinstance(size_usd, Decimal | int | float): + raise DcaSizeInvalid(size_usd) amount = Decimal(str(size_usd)) if isinstance(size_usd, float) else Decimal(size_usd) if not amount.is_finite() or amount <= 0: - return fallback, "config" + raise DcaSizeInvalid(size_usd) return amount, "rule" @@ -758,17 +813,22 @@ def _build_intent( if is_dca: # CAREFUL: DCA is sized from the RULE's amount -- `setup.context["size_usd"]`, which # `Dca.detect` computes from the rule's own `budget_usd` -- and the config's - # `dca.budget_usd` is only the FALLBACK for a setup without a usable one (#840). - # This reverses the earlier design, where live always spent the config value on the - # grounds that "the config is the operator-facing dial and a rule row is not reviewed - # on every deploy". The operator reversed that on 2026-09-27: with rules at $40, $25 - # and $15 and the config at $50, every buy spent $50 -- about $978 a month against - # the rules' $467 and rail 14's $500 cap. Paper sizes through this same function, and - # the account sim (`sim/portfolio_sim.py`) already preferred `size_usd`, so all three - # now agree. Which source sized the order is logged (`executor.dca_sized`), because a - # fallback means a setup arrived without the rule's number and deserves a look. - # The rails are unchanged: `notional` below is computed from this qty, so rail 14 and - # every other guard see the smaller, real order. + # `dca.budget_usd` is only the FALLBACK for a setup whose `size_usd` is ABSENT + # (#840). This reverses the earlier design, where live always spent the config value + # on the grounds that "the config is the operator-facing dial and a rule row is not + # reviewed on every deploy". Paper sizes through this same function, and the account + # sim (`sim/portfolio_sim.py`) shares `_dca_budget` too, so all three agree. Which + # source sized the order is logged (`executor.dca_sized`), because a fallback means a + # setup arrived without the rule's number and deserves a look. The rails are + # unchanged: `notional` below is computed from this qty, so rail 14 and every other + # guard see the smaller, real order. + # + # A `size_usd` that is PRESENT but not usable (0, negative, non-finite, a bool, or + # not a number) does NOT fall back to the config either (orchestrator ruling + # 2026-09-27): `_dca_budget` raises `DcaSizeInvalid`, which propagates out of this + # function and is caught by `execute()`, which skips the buy entirely -- no order, + # no broker call. A rule meant to buy less must never spend more, and falling back + # to a config value the rule never referenced would do exactly that. budget, source = _dca_budget(setup.context, config.dca.budget_usd) log_event( logger, diff --git a/keel/sim/portfolio_sim.py b/keel/sim/portfolio_sim.py index 81a003ce..8e2edbc9 100644 --- a/keel/sim/portfolio_sim.py +++ b/keel/sim/portfolio_sim.py @@ -85,6 +85,7 @@ from keel.analysis import regime from keel.config import Config from keel.execution import sizing +from keel.execution.executor import DcaSizeInvalid, _dca_budget from keel.execution.guards import _asset, _utc_day_bounds, _utc_month_bounds from keel.sim.account import OpenIntent, OpenPosition, SimAccount from keel.strategy import engine, indicators_cts @@ -699,9 +700,16 @@ def _process_dca_signals( decided.add(key) try: - budget = setup.context.get("size_usd") or config.dca.budget_usd + # Shares `executor._dca_budget` with the live and paper paths rather than + # re-deriving the same predicate here (orchestrator ruling 2026-09-27): `size_usd` + # ABSENT (missing or `None`) falls back to `config.dca.budget_usd`; a `size_usd` + # that is PRESENT but not usable (0, negative, non-finite, a bool, non-numeric) + # raises `DcaSizeInvalid` instead, and this cycle's decision is SKIPPED (`continue`) + # rather than sized from a config value the rule never referenced -- exactly like + # the live/paper skip, so all three paths agree on what "usable" means. + budget, _source = _dca_budget(setup.context, config.dca.budget_usd) qty = sizing.dca_size(budget, setup.entry) - except ValueError: + except DcaSizeInvalid, ValueError: continue notional = sizing.spend(qty, setup.entry) diff --git a/keel/strategy/rules/dca.py b/keel/strategy/rules/dca.py index 07f1d21d..406d765c 100644 --- a/keel/strategy/rules/dca.py +++ b/keel/strategy/rules/dca.py @@ -72,6 +72,16 @@ def __init__( raise ValueError("budget_usd must be positive") if lookback_days <= 0: raise ValueError("lookback_days must be positive") + if dip_bonus_pct < 0: + # 0 (the default) stays allowed -- it disables dip-scaling entirely, per + # `PARAM_DOCS` above. A NEGATIVE value would shrink the budget as price drops, the + # inverse of what a "dip bonus" means, and would make `Dca.detect` emit a + # `size_usd` smaller than `budget_usd` on every drawdown -- which the executor + # (`execution.executor._dca_budget`) treats as an INVALID size_usd once it goes + # non-positive, skipping the buy entirely rather than resizing it (orchestrator + # ruling 2026-09-27). Refusing it here, at construction, is cheaper than discovering + # it as a silently-skipped live buy. + raise ValueError("dip_bonus_pct must not be negative") self.name = name self.product_id = product_id diff --git a/keel/templates/config.live.yaml b/keel/templates/config.live.yaml index 27108026..31a642c4 100644 --- a/keel/templates/config.live.yaml +++ b/keel/templates/config.live.yaml @@ -80,10 +80,14 @@ money_mgmt: streak_cooloff_days: 0 dca: - # FALLBACK only (#840). Each DCA buy -- live, paper and the account sim -- spends the RULE's own - # amount (`size_usd`, computed from the rule's `budget_usd`; see `keel rules list`). This - # value sizes a DCA buy only when a setup arrives without a positive `size_usd`, and the - # executor logs `executor.dca_sized` with `source=config` when it does. + # FALLBACK only (#840). Each DCA buy -- live, paper and the account sim -- spends the RULE's + # own amount (`size_usd`, computed from the rule's `budget_usd`; see `keel rules list`), and + # all three paths share one predicate (`executor._dca_budget`) for when this value applies. + # It sizes a buy ONLY when a setup's `size_usd` is ABSENT (the key missing, or `None`); the + # executor logs `executor.dca_sized` with `source=config` when it falls back. A setup that + # DOES carry a `size_usd`, but an unusable one -- zero, negative, non-finite, or not a number + # -- is never sized from this value: that buy is SKIPPED instead, because a rule meant to buy + # less must never spend more. budget_usd: 50 cadence_days: 7 diff --git a/keel/templates/config.yaml b/keel/templates/config.yaml index 177ea526..47508079 100644 --- a/keel/templates/config.yaml +++ b/keel/templates/config.yaml @@ -69,10 +69,14 @@ money_mgmt: streak_cooloff_days: 0 dca: - # FALLBACK only (#840). Each DCA buy -- live, paper and the account sim -- spends the RULE's own - # amount (`size_usd`, computed from the rule's `budget_usd`; see `keel rules list`). This - # value sizes a DCA buy only when a setup arrives without a positive `size_usd`, and the - # executor logs `executor.dca_sized` with `source=config` when it does. + # FALLBACK only (#840). Each DCA buy -- live, paper and the account sim -- spends the RULE's + # own amount (`size_usd`, computed from the rule's `budget_usd`; see `keel rules list`), and + # all three paths share one predicate (`executor._dca_budget`) for when this value applies. + # It sizes a buy ONLY when a setup's `size_usd` is ABSENT (the key missing, or `None`); the + # executor logs `executor.dca_sized` with `source=config` when it falls back. A setup that + # DOES carry a `size_usd`, but an unusable one -- zero, negative, non-finite, or not a number + # -- is never sized from this value: that buy is SKIPPED instead, because a rule meant to buy + # less must never spend more. budget_usd: 50 cadence_days: 7 diff --git a/packages/keel-core/keel_core/config.py b/packages/keel-core/keel_core/config.py index 8cdfaa0c..d89bebc3 100644 --- a/packages/keel-core/keel_core/config.py +++ b/packages/keel-core/keel_core/config.py @@ -148,15 +148,28 @@ class MoneyMgmtConfig: @dataclass(frozen=True) class DcaConfig: - """The `dca:` block. `budget_usd` is a FALLBACK, not what a DCA buy spends (#840). - - Every DCA buy -- live (`execution.executor._build_intent`), paper (the same function) and - the account sim (`sim.portfolio_sim`) -- is sized from the RULE's own amount, the - `size_usd` that `Dca.detect` puts in its setup's context (`budget_usd x (1 + dip bonus)`). - `budget_usd` here sizes a DCA buy only when a setup carries no positive, finite `size_usd`, - and the executor logs `executor.dca_sized` with `source="config"` when it does. Until - 2026-09-27 live spent this value on every buy and ignored the rule's; the operator reversed - that after the $40/$25/$15 rules were each spending the $50 configured here. + """The `dca:` block. `budget_usd` is a FALLBACK, not what a DCA buy spends (#840), and it + fills in ONLY when a setup's `size_usd` is ABSENT -- the key missing, or explicitly `None`. + + Every DCA buy -- live (`execution.executor._build_intent`), paper (the same function, via + `agent._paper_enter`) and the account sim (`sim.portfolio_sim._process_dca_signals`) -- is + sized from the RULE's own amount, the `size_usd` that `Dca.detect` puts in its setup's + context (`budget_usd x (1 + dip bonus)`). All three paths share one predicate + (`execution.executor._dca_budget`), so "usable" means the same thing everywhere: + + - `size_usd` ABSENT (missing, or `None`) -> falls back to `budget_usd` here, logged as + `executor.dca_sized` with `source="config"`. + - `size_usd` PRESENT but not a positive, finite number (zero, negative, `NaN`/`Infinity`, a + `bool`, or anything not a number at all) -> does NOT fall back. The buy is SKIPPED instead + (a WARNING logged, no order placed, no position opened), on every path alike. + + That asymmetry is deliberate (orchestrator ruling 2026-09-27): a rule that computed an + invalid `size_usd` meant to buy LESS, or not at all, and falling back to this shared config + value in that case would spend MORE than the rule ever asked for -- a rule meant to buy + less must never spend more. Only a setup that carries no opinion at all (no `size_usd`) gets + this config's opinion instead. Until 2026-09-27 live spent this value on every buy and + ignored the rule's; the operator reversed that after finding every DCA rule was spending + this one shared value rather than its own tuned budget. """ budget_usd: Decimal = Decimal("0") diff --git a/scripts/rule_manifest.py b/scripts/rule_manifest.py index 38bf5b4d..932c2f54 100644 --- a/scripts/rule_manifest.py +++ b/scripts/rule_manifest.py @@ -7,9 +7,12 @@ replaced by the default: a DCA rule deliberately set to `budget_usd: 25` comes back as the built-in `50`, at `candidate`, on a box that otherwise looks correctly provisioned. Nothing errors. (That is a worked example rather than a description of today's deployment -- the live DCA -rule is now `50` on purpose, matching the constructor default and `config.dca.budget_usd`, which -is the value the live executor actually spends. Because it now matches the default, the VALUE -alone can no longer prove the rule wasn't reseeded; `tests/test_rule_manifest.py`'s +rule is now `50` on purpose, matching the constructor default. Since #840 the live executor spends +the RULE's own `budget_usd` (the `size_usd` its setup carries), not `config.dca.budget_usd` -- +that config value is only the FALLBACK for a setup whose `size_usd` is absent -- so the manifest +and the config are free to diverge by design and this module does not require them to agree. +Because the rule's value now matches the default, the VALUE alone can no longer prove the rule +wasn't reseeded; `tests/test_rule_manifest.py`'s `test_committed_manifest_is_valid` also asserts every committed rule's `status` is `live`, since a `keel init` reseed always lands at `candidate` no matter what the params say.) This module makes that state an artifact you can diff in a PR instead of a fact that lives only on one laptop. diff --git a/tests/execution/test_executor.py b/tests/execution/test_executor.py index 4ae3ad4a..05acae0f 100644 --- a/tests/execution/test_executor.py +++ b/tests/execution/test_executor.py @@ -880,25 +880,60 @@ def test_live_dca_sizes_from_the_rules_size_usd_not_the_config_budget(repo, capl assert Decimal(events[0]["budget_usd"]) == Decimal("25") +@pytest.mark.parametrize( + ("size_usd", "expected"), + [ + (25, Decimal("25")), # an int -- goes through `Decimal(size_usd)`, not `str()` + (25.0, Decimal("25")), # a float with an exact binary representation + # 0.1 is NOT exact in binary: `Decimal(0.1)` is + # 0.1000000000000000055511151231257827021181583404541015625. `_dca_budget` must go + # through `Decimal(str(size_usd))` for a float, or this asserts the wrong number. + (0.1, Decimal("0.1")), + ], + ids=["int", "float_exact", "float_inexact"], +) +def test_live_dca_sizes_from_an_int_or_float_size_usd(repo, caplog, size_usd, expected): + """(a') `_dca_budget` accepts an `int` or `float` `size_usd`, not just `Decimal` -- and a + float is converted via `str()` so `0.1` becomes the decimal a human meant.""" + broker = FakeBroker() + + with caplog.at_level(logging.INFO, logger="keel.execution.executor"): + result = execute( + _dca_signal_sized(size_usd), + broker, + repo, + _config(), + mode="autonomous", + now_ts=NOW_TS, + ) + + assert result.placed is True, result.vetoed_by + order = repo.get_order(result.order_id) + assert order["qty"] == expected / Decimal("50000") + assert len(broker.place_calls) == 1 + assert broker.place_calls[0]["spec"].quote_size == expected + events = _dca_sizing_events(caplog) + assert len(events) == 1 + assert events[0]["source"] == "rule" + assert Decimal(events[0]["budget_usd"]) == expected + + @pytest.mark.parametrize( ("size_usd", "include"), [ (None, False), # key absent (None, True), # key present, value None - (Decimal("0"), True), - (Decimal("-25"), True), - (Decimal("Infinity"), True), # `> 0` passes it; see `commands/rules.py` (#840) - (Decimal("NaN"), True), - ("25", True), # not a number - (True, True), # a bool is an int in Python; it is not an amount ], - ids=["absent", "none", "zero", "negative", "infinity", "nan", "string", "bool"], + ids=["absent", "none"], ) -def test_live_dca_falls_back_to_the_config_budget_without_a_usable_size_usd( +def test_live_dca_falls_back_to_the_config_budget_when_size_usd_is_absent( repo, caplog, size_usd, include ): - """(b) No positive, finite `size_usd` -> the config's `dca.budget_usd` (50) sizes the buy, - and the fallback is recorded as such.""" + """(b) `size_usd` ABSENT -- the key is missing, or present as `None` -- falls back to the + config's `dca.budget_usd` (50), and the fallback is recorded as such. A size_usd that is + PRESENT but not usable is a DIFFERENT case (orchestrator ruling 2026-09-27): see + `test_live_dca_skips_a_buy_whose_rule_computed_an_invalid_size_usd` below -- that one must + NOT fall back, or a rule meant to buy less would spend more.""" broker = FakeBroker() with caplog.at_level(logging.INFO, logger="keel.execution.executor"): @@ -920,6 +955,60 @@ def test_live_dca_falls_back_to_the_config_budget_without_a_usable_size_usd( assert Decimal(events[0]["budget_usd"]) == Decimal("50") +def _dca_size_invalid_events(caplog: pytest.LogCaptureFixture) -> list[dict[str, Any]]: + from keel_core.telemetry import _FIELDS_ATTR + + return [ + getattr(r, _FIELDS_ATTR) + for r in caplog.records + if r.getMessage() == "executor.dca_size_invalid" + ] + + +@pytest.mark.parametrize( + "size_usd", + [ + Decimal("0"), + Decimal("-25"), + Decimal("Infinity"), # `> 0` alone passes it; see `commands/rules.py` (#840) + Decimal("NaN"), + "25", # not a number + True, # a bool is an int in Python; it is not an amount + ], + ids=["zero", "negative", "infinity", "nan", "string", "bool"], +) +def test_live_dca_skips_a_buy_whose_rule_computed_an_invalid_size_usd(repo, caplog, size_usd): + """(b') `size_usd` PRESENT but not usable must NEVER fall back to the config (orchestrator + ruling 2026-09-27): a rule that computed 0, a negative amount, a non-finite value, or + garbage meant to buy LESS -- or nothing -- and spending the config's larger, unrelated + `budget_usd` would spend MORE than the rule asked for. The buy is skipped instead: no + broker call (`NoNetworkBroker` proves it), no order row, and a WARNING logged.""" + broker = NoNetworkBroker() + + with caplog.at_level(logging.WARNING, logger="keel.execution.executor"): + result = execute( + _dca_signal_sized(size_usd), + broker, + repo, + _config(), + mode="autonomous", + now_ts=NOW_TS, + ) + + assert result.placed is False + assert result.order_id is None + assert result.vetoed_by == [] + assert result.preview is None + assert result.reason == ( + f"dca: rule computed an invalid size_usd={size_usd!r}; buy skipped, not sized from config" + ) + assert repo.get_orders(mode="live") == [] + events = _dca_size_invalid_events(caplog) + assert len(events) == 1 + assert events[0]["product"] == "BTC-USD" + assert events[0]["rule"] == "dca" + + def test_a_dca_rules_detect_feeds_the_live_order_size_end_to_end(repo): """(c) The context key is the one `Dca.detect` actually writes -- pinned by driving the real rule, not by naming the key in a hand-built context. A $15 rule at a 30,000 close buys diff --git a/tests/sim/test_portfolio_sim.py b/tests/sim/test_portfolio_sim.py index 309a2b29..38d96891 100644 --- a/tests/sim/test_portfolio_sim.py +++ b/tests/sim/test_portfolio_sim.py @@ -13,6 +13,9 @@ from datetime import UTC, datetime from decimal import Decimal +from typing import Any + +import pytest from keel.config import ( Caps, @@ -425,16 +428,30 @@ def test_idle_span_recorded_on_big_move_with_no_signal(): # --------------------------------------------------------------------------- +#: Sentinel for `_OnceDcaRule.size_usd` meaning "not overridden -- use `budget_usd`, as every +#: caller before the size_usd override existed did". Distinct from `None`, which a caller can +#: pass explicitly to drive the ABSENT/`None` fallback case in `_process_dca_signals`. +_UNSET = object() + + class _OnceDcaRule(Rule): - """Fires a DCA-class setup exactly once (on the asset's first bar) -- used only to prove a - DCA lot and a RULE position can coexist for the same asset within one `run()`.""" + """Fires a DCA-class setup exactly once (on the asset's first bar) -- used to prove a DCA + lot and a RULE position can coexist for the same asset within one `run()`, and (via + `size_usd`) to drive `_process_dca_signals`'s size_usd handling with a controlled value + distinct from `budget_usd`.""" name = "dca_once" params: dict = {} - def __init__(self, product_id: str, budget_usd: Decimal = Decimal("50")) -> None: + def __init__( + self, + product_id: str, + budget_usd: Decimal = Decimal("50"), + size_usd: Any = _UNSET, + ) -> None: self.product_id = product_id self.budget_usd = budget_usd + self.size_usd = budget_usd if size_usd is _UNSET else size_usd def detect(self, candles_by_tf: dict[Granularity, list[Candle]]) -> Setup | None: hourly = candles_by_tf[Granularity.ONE_HOUR] @@ -447,7 +464,7 @@ def detect(self, candles_by_tf: dict[Granularity, list[Candle]]) -> Setup | None entry=latest.close, stop=Decimal("0"), target=latest.close, - context={"no_stop": True, "order_class": "dca", "size_usd": self.budget_usd}, + context={"no_stop": True, "order_class": "dca", "size_usd": self.size_usd}, ts=latest.ts, ) @@ -552,6 +569,65 @@ def test_dca_and_rule_positions_coexist_on_the_same_asset(): assert any(tr.asset == "BTC" for tr in result.trades) # the RULE position also opened +def test_dca_size_usd_none_falls_back_to_the_config_budget(): + """`size_usd` ABSENT -- here explicitly `None`, which `_process_dca_signals` (via the shared + `executor._dca_budget`) treats identically to a missing key -- falls back to + `config.dca.budget_usd`, not the rule's own (very different) `budget_usd`.""" + hourly = [ + _candle(0, "100", "101", "99", "100"), + _candle(_HOUR, "100", "102", "98", "101"), + _candle(2 * _HOUR, "101", "103", "100", "102"), + ] + dca_rule = _OnceDcaRule("BTC-USD", budget_usd=Decimal("999"), size_usd=None) + candles_by_asset = {"BTC": {Granularity.ONE_HOUR: hourly, Granularity.ONE_DAY: []}} + config = _config(dca=DcaConfig(budget_usd=Decimal("50"))) + + result = run( + [dca_rule], + candles_by_asset, + config, + start_ts=hourly[0].ts, + end_ts=hourly[-1].ts, + monthly_contribution=Decimal("100000"), + ) + + assert "BTC" in result.dca_positions + assert result.dca_positions["BTC"].qty * Decimal("100") == Decimal("50") # config, not 999 + + +@pytest.mark.parametrize( + "size_usd", + [Decimal("0"), Decimal("-50"), Decimal("Infinity"), Decimal("NaN"), "50", True], + ids=["zero", "negative", "infinity", "nan", "string", "bool"], +) +def test_dca_skips_a_buy_whose_size_usd_is_present_but_invalid(size_usd): + """`size_usd` PRESENT but not usable must NOT fall back to `config.dca.budget_usd` (the same + orchestrator ruling `executor._dca_budget` enforces live and on paper): a rule that computed + an invalid amount meant to buy less, and the sim spending the config's unrelated budget + instead would model a buy the rule never asked for. The cycle is skipped instead -- no DCA + lot opens at all.""" + hourly = [ + _candle(0, "100", "101", "99", "100"), + _candle(_HOUR, "100", "102", "98", "101"), + _candle(2 * _HOUR, "101", "103", "100", "102"), + ] + dca_rule = _OnceDcaRule("BTC-USD", size_usd=size_usd) + candles_by_asset = {"BTC": {Granularity.ONE_HOUR: hourly, Granularity.ONE_DAY: []}} + config = _config(dca=DcaConfig(budget_usd=Decimal("50"))) + + result = run( + [dca_rule], + candles_by_asset, + config, + start_ts=hourly[0].ts, + end_ts=hourly[-1].ts, + monthly_contribution=Decimal("100000"), + ) + + assert "BTC" not in result.dca_positions + assert result.dca_buys == [] + + # --------------------------------------------------------------------------- # Issue #86: monthly_volume_cap throttles the account to a fee-free tier's allowance # --------------------------------------------------------------------------- diff --git a/tests/strategy/test_dca.py b/tests/strategy/test_dca.py index 2f1cf49d..a1762d01 100644 --- a/tests/strategy/test_dca.py +++ b/tests/strategy/test_dca.py @@ -178,6 +178,18 @@ def test_rejects_non_positive_budget(self) -> None: with pytest.raises(ValueError): Dca(product_id="BTC-USD", budget_usd=Decimal("0")) + def test_rejects_negative_dip_bonus(self) -> None: + """A negative `dip_bonus_pct` would SHRINK the budget as price drops -- the inverse of + what "dip bonus" means, and the executor treats a shrunk `size_usd` as invalid and skips + the buy rather than resizing it (orchestrator ruling 2026-09-27), so a negative value here + would silently turn every dip into a skipped buy instead of a scaled-up one.""" + with pytest.raises(ValueError): + Dca(product_id="BTC-USD", dip_bonus_pct=Decimal("-1")) + + def test_allows_zero_dip_bonus(self) -> None: + """0 is the documented default (dip-scaling disabled) and must stay allowed.""" + Dca(product_id="BTC-USD", dip_bonus_pct=Decimal("0")) + _HOUR = 3_600 diff --git a/tests/test_agent.py b/tests/test_agent.py index c8bafe7a..aff2cdbc 100644 --- a/tests/test_agent.py +++ b/tests/test_agent.py @@ -3232,6 +3232,66 @@ def test_paper_dca_fill_is_sized_from_the_rules_amount_not_the_config_budget(rep assert orders[0]["qty"] * price == Decimal("15") +@pytest.mark.parametrize( + "size_usd", + [Decimal("0"), Decimal("-15"), Decimal("Infinity"), Decimal("NaN"), "15", True], + ids=["zero", "negative", "infinity", "nan", "string", "bool"], +) +def test_paper_dca_skips_a_buy_whose_rule_computed_an_invalid_size_usd(repo, caplog, size_usd): + """`_paper_enter` must skip a DCA buy the same way the live path does when the rule's + `size_usd` is PRESENT but not usable (orchestrator ruling 2026-09-27): falling back to + `config.dca.budget_usd` here would spend more than a rule that computed 0, a negative + amount, non-finite, or garbage ever asked for. `executor._build_intent` raises + `DcaSizeInvalid`; `_paper_enter` must catch it, place nothing, and report the skip.""" + from keel.strategy.paper import PaperTrader + + trader = PaperTrader(repo) + trader.seed_cash(Decimal("30000"), now_ts=1_000) + repo.set_state("last_feed_ts", 90_000) + config = _paper_config() + assert config.dca.budget_usd == Decimal("50") + signal = Signal( + rule_name="dca", + product_id=PRODUCT, + action=Action.ENTER, + side=Side.BUY, + setup=Setup( + product_id=PRODUCT, + direction="long", + entry=Decimal("30"), + stop=Decimal("0"), + target=Decimal("30"), + context={"order_class": "dca", "no_stop": True, "size_usd": size_usd}, + ts=90_000, + ), + cts_score=0, + entry_technique="market", + ts=90_000, + ) + + with caplog.at_level(logging.WARNING, logger="keel.agent"): + result = agent._paper_enter( + trader, signal, repo, config, now_ts=90_000, paper_equity=Decimal("30000") + ) + + assert result.placed is False + assert result.order_id is None + assert result.vetoed_by == [] + assert result.reason == ( + f"paper: dca: rule computed an invalid size_usd={size_usd!r}; buy skipped, " + "not sized from config" + ) + assert repo.get_orders(mode="paper") == [] + events = [ + getattr(r, _FIELDS_ATTR) + for r in caplog.records + if r.getMessage() == "agent.paper_dca_size_invalid" + ] + assert len(events) == 1 + assert events[0]["product"] == PRODUCT + assert events[0]["rule"] == "dca" + + def test_paper_mode_never_runs_the_entry_spread_gate(repo): """#350's max-spread gate is live-path ONLY: paper fills are synthetic and see no book, so `_paper_enter` never previews an order and the gate (fail-closed on an unreadable book for diff --git a/tests/test_rule_manifest.py b/tests/test_rule_manifest.py index be569413..6e1e2458 100644 --- a/tests/test_rule_manifest.py +++ b/tests/test_rule_manifest.py @@ -10,7 +10,6 @@ from __future__ import annotations import json -import re import sys from decimal import Decimal from pathlib import Path @@ -146,67 +145,63 @@ def test_committed_manifest_is_valid(tmp_path: Path) -> None: assert len(dca) == 1, "the live DCA rule is missing from the manifest" # This used to pin the rule's budget at "25" with the rationale "must not revert to the - # default 50". That rationale was backwards, because the number it protected was never the - # number being spent: the live executor sizes DCA from `config.dca.budget_usd` - # (`execution/executor.py::_build_intent`) and ignores the RULE's `budget_usd` entirely. So - # the rule row said 25, the config said 50, and 50 was what moved. The rule's value is read - # only by the account simulator, which means the divergence also made the sim model a - # position size the live path would never take. + # default 50", then (still before #840) switched to an AGREEMENT assertion that the + # manifest's budget must equal `config.dca.budget_usd`. Both versions were reasoning about a + # world where the live executor sized DCA from `config.dca.budget_usd` + # (`execution/executor.py::_build_intent`) and ignored the RULE's `budget_usd` entirely -- + # so the rule row could say 25, the config could say 50, and 50 is what moved live while the + # account simulator, which read the rule's value, modeled a position size live never took. + # AGREEMENT closed that gap by requiring the manifest and the config to match. # - # 50 is the intended budget, so all three now agree, and assertion (1) below checks the - # AGREEMENT rather than a literal -- a hardcoded number here would just re-create the drift - # it is supposed to catch, one deploy later. + # #840 reverses which value is real: the live executor now spends the RULE's own `budget_usd` + # (the `size_usd` its setup carries -- see `_dca_budget`), and `config.dca.budget_usd` is only + # the FALLBACK for a setup whose `size_usd` is absent. The rule row is now the number that + # actually moves, live and in the sim alike, and it is MEANT to be free to diverge from the + # config -- letting an operator tune one rule's budget away from the shared config value is + # exactly what #840 made possible. Requiring the manifest to agree with the config would break + # the moment that divergence is actually used, so this test no longer asserts that agreement. + # Right now `deploy/live-rules.json` defines a single DCA rule, at $50 -- matching this + # config's value -- but that coincidence is what assertion (2) below pins down explicitly, + # not a fact this test takes for granted or asserts on its own. # - # But agreement alone is now a WEAKER guard than the "25" literal it replaced, and that - # weakness needs to be guarded explicitly rather than left as a silent regression: 50 is - # *also* `Dca.__init__`'s constructor default (see `keel/strategy/rules/dca.py`), and - # `deploy/live-rules.json`'s DCA params are, right now, EXACTLY those constructor defaults. - # That means "the manifest's budget equals the config's budget" is satisfied both by a - # correctly-provisioned box AND by a box where `keel init` silently reseeded the rule at - # `candidate` from constructor defaults -- the exact failure mode this test used to exist to - # catch. Values can no longer tell those two states apart. Two more assertions below make up - # for that: (2) status, which is the only thing that still can, and (3) a pinned check on - # the coincidence itself, so that if the operator ever moves the budget off the default, the - # value check regains its old power and (3) is what tells them so. + # What is still true, and still worth guarding: `deploy/live-rules.json`'s DCA params are, + # right now, EXACTLY `Dca.__init__`'s constructor defaults (see `keel/strategy/rules/dca.py`). + # That means the VALUE alone cannot prove this rule wasn't reseeded by a fresh `keel init` + # rather than deliberately provisioned, since the operator's intended value and `keel init`'s + # default are, today, the same number. Two assertions below make up for that: (1) status, + # which is the only thing that still can, and (2) a pinned check on the coincidence itself, + # so that if the operator ever moves the budget off the default, the value check regains its + # own power and (2) is what tells them so. # # SCOPE: this asserts the COMMITTED FILE, not a deployment's database, so it catches a # reseeded box's state being COMMITTED -- not the reseed itself. `rule_manifest.py apply` # is what reports that drift against a live DB. - # (1) AGREEMENT -- the manifest's budget must match config.dca.budget_usd, the value the live - # executor actually spends. - live_config = REPO_ROOT / "config.live-sandbox.yaml" - configured = re.search(r"^dca:\n(?:.*\n)*? budget_usd: (\S+)$", live_config.read_text(), re.M) - assert configured is not None, "config.live-sandbox.yaml no longer declares dca.budget_usd" - assert Decimal(dca[0]["params"]["budget_usd"]) == Decimal(configured.group(1)), ( - "the live DCA rule's budget_usd must match config.dca.budget_usd, which is the value the " - "executor actually spends -- if they diverge, the simulator and the live path disagree" - ) - - # (2) NOT SEED-SHAPED -- status is the only discriminator assertion (1) leaves standing - # between "the operator's 50" and "keel init's 50". `keel init` always seeds fresh rules at - # `candidate` (`docs/RELEASING.md`), and nothing in this test path promotes them, so a - # reseeded box's manifest would show `candidate` even though its budget_usd matches the - # config byte-for-byte. A correctly-provisioned deployment has every rule at `live`. + # (1) NOT SEED-SHAPED -- status is the only discriminator standing between "the operator's + # 50" and "keel init's 50" now that the manifest's budget is not checked against anything + # else. `keel init` always seeds fresh rules at `candidate` (`docs/RELEASING.md`), and + # nothing in this test path promotes them, so a reseeded box's manifest would show + # `candidate` even though its budget_usd matches the constructor default byte-for-byte. A + # correctly-provisioned deployment has every rule at `live`. assert dca[0]["status"] == "live", ( "the live DCA rule is not status=live -- if this is a candidate, keel init likely " "reseeded it from Dca's constructor defaults rather than preserving an operator's tuned " - "value, and assertion (1) above cannot catch that on its own (see comment)" + "value, and nothing else here can catch that on its own (see comment)" ) assert all(r["status"] != "candidate" for r in rebuilt), ( "a rule in the committed live manifest is status=candidate -- that shape matches a fresh " "`keel init` reseed from constructor defaults, not a deliberately-provisioned deployment" ) - # (3) THE COINCIDENCE IS PINNED, NOT ASSUMED -- assertion (2) only carries the weight it does + # (2) THE COINCIDENCE IS PINNED, NOT ASSUMED -- assertion (1) only carries the weight it does # because the operator's intended DCA params happen, today, to equal Dca's constructor # defaults. Pin that equality explicitly instead of taking it on faith. If it ever stops # being true -- e.g. the operator deliberately moves the budget off 50 -- THIS assertion is - # what fails, and failing here is good news dressed as a test failure: it means assertion (1) - # has regained the ability to catch a `keel init` revert on its own (a reseed would then - # produce a *different* number, not a coincidentally-matching one), and the status check in - # (2) is no longer the last line of defense. Whoever hits this failure should update this - # comment, not just delete the assertion. + # what fails, and failing here is good news dressed as a test failure: it means the + # manifest's value would now visibly differ from a reseed's, so the VALUE alone regains the + # power to catch a `keel init` revert, and the status check in (1) is no longer the last + # line of defense. Whoever hits this failure should update this comment, not just delete the + # assertion. manifest_params = dca[0]["params"] default_params = Dca(product_id=manifest_params["product_id"]).describe()["params"] for key, expected in default_params.items(): @@ -214,9 +209,10 @@ def test_committed_manifest_is_valid(tmp_path: Path) -> None: continue # trivially equal -- it's the constructor arg we just passed in assert Decimal(str(manifest_params[key])) == Decimal(str(expected)), ( f"deploy/live-rules.json's DCA {key} ({manifest_params[key]!r}) no longer matches " - f"Dca's constructor default ({expected!r}). Assertions (1)/(2) above still hold, but " - "the reasoning behind assertion (2) -- that status is the ONLY thing distinguishing " - "a deliberate value from a reseeded default -- no longer applies to this parameter: " - "a reseed would now produce a visibly different value, so assertion (1) alone would " - "catch it again." + f"Dca's constructor default ({expected!r}). Assertion (1) above still holds, but the " + "reasoning behind it -- that status is the ONLY thing distinguishing a deliberate " + "value from a reseeded default -- no longer applies to this parameter: a reseed " + "would now produce a visibly different value here, and THIS comparison catches that " + "on its own, without needing assertion (1)'s status check as the last line of " + "defense." ) From 8fda72478c5ad303199032c35d9b0ce466a9417b Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Sun, 27 Sep 2026 20:05:58 -0400 Subject: [PATCH 3/3] docs(config): DcaConfig says only live and paper log the DCA skip (#840) The docstring claimed the size_usd-present-but-invalid skip logs a WARNING "on every path alike" and that the fallback is always logged as executor.dca_sized. Neither is true for the sim: _process_dca_signals calls executor._dca_budget directly and just continue's on DcaSizeInvalid/ValueError, logging nothing either way. Only live (executor.execute/_build_intent) and paper (agent._paper_enter) emit executor.dca_sized (fallback) or a WARNING (executor.dca_size_invalid / agent.paper_dca_size_invalid) for the skip. Also removed a stale "matching this config's value" in tests/test_rule_manifest.py: assertion (2) there pins the manifest value against Dca's constructor default, not against config.dca.budget_usd, which this test no longer reads. Comment/docstring-only; no behaviour change. Co-Authored-By: Claude Opus 5.5 --- packages/keel-core/keel_core/config.py | 10 +++++++--- tests/test_rule_manifest.py | 6 +++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/keel-core/keel_core/config.py b/packages/keel-core/keel_core/config.py index d89bebc3..4107a8dc 100644 --- a/packages/keel-core/keel_core/config.py +++ b/packages/keel-core/keel_core/config.py @@ -157,11 +157,15 @@ class DcaConfig: context (`budget_usd x (1 + dip bonus)`). All three paths share one predicate (`execution.executor._dca_budget`), so "usable" means the same thing everywhere: - - `size_usd` ABSENT (missing, or `None`) -> falls back to `budget_usd` here, logged as - `executor.dca_sized` with `source="config"`. + - `size_usd` ABSENT (missing, or `None`) -> falls back to `budget_usd` here. Live and paper + log this as `executor.dca_sized` with `source="config"` (emitted from `_build_intent`, + which both call); the sim's `_process_dca_signals` calls `_dca_budget` directly and does + not log this case. - `size_usd` PRESENT but not a positive, finite number (zero, negative, `NaN`/`Infinity`, a `bool`, or anything not a number at all) -> does NOT fall back. The buy is SKIPPED instead - (a WARNING logged, no order placed, no position opened), on every path alike. + (no order placed, no position opened). Live and paper log a WARNING for this + (`executor.execute`'s `executor.dca_size_invalid`, `agent._paper_enter`'s + `agent.paper_dca_size_invalid`); the sim skips silently -- a bare `continue`, no log. That asymmetry is deliberate (orchestrator ruling 2026-09-27): a rule that computed an invalid `size_usd` meant to buy LESS, or not at all, and falling back to this shared config diff --git a/tests/test_rule_manifest.py b/tests/test_rule_manifest.py index 6e1e2458..5a9468b9 100644 --- a/tests/test_rule_manifest.py +++ b/tests/test_rule_manifest.py @@ -160,9 +160,9 @@ def test_committed_manifest_is_valid(tmp_path: Path) -> None: # config -- letting an operator tune one rule's budget away from the shared config value is # exactly what #840 made possible. Requiring the manifest to agree with the config would break # the moment that divergence is actually used, so this test no longer asserts that agreement. - # Right now `deploy/live-rules.json` defines a single DCA rule, at $50 -- matching this - # config's value -- but that coincidence is what assertion (2) below pins down explicitly, - # not a fact this test takes for granted or asserts on its own. + # Right now `deploy/live-rules.json` defines a single DCA rule, at $50 -- matching `Dca`'s + # constructor default -- but that coincidence is what assertion (2) below pins down + # explicitly, not a fact this test takes for granted or asserts on its own. # # What is still true, and still worth guarding: `deploy/live-rules.json`'s DCA params are, # right now, EXACTLY `Dca.__init__`'s constructor defaults (see `keel/strategy/rules/dca.py`).