diff --git a/keel/agent.py b/keel/agent.py index c88120dc..5250c876 100644 --- a/keel/agent.py +++ b/keel/agent.py @@ -140,7 +140,13 @@ def _build_rule(row: dict[str, Any]) -> Rule: if "signal_patterns" in kwargs: kwargs["signal_patterns"] = tuple(kwargs["signal_patterns"]) - return rule_cls(**kwargs) + rule = rule_cls(**kwargs) + # Thread the real `rules.id` onto the instance (additive `Rule.rule_id`, default `None`) so + # it can flow onto emitted `Signal`s and, from there, into `orders.rule_id`/`signals.rule_id` + # for audit -- `.get` rather than `row["id"]` so a hand-built row with no "id" (some tests) + # still builds a rule, just with `rule_id` left at its `None` default. + rule.rule_id = row.get("id") + return rule # -- freshness ----------------------------------------------------------------------------- @@ -532,6 +538,7 @@ def _handle_exits( cts_score=0, entry_technique="market", ts=now_ts, + rule_id=owning_rule.rule_id, ) result = executor.execute( exit_signal, broker, repo, config, mode, confirm_fn=confirm_fn, now_ts=now_ts diff --git a/keel/execution/executor.py b/keel/execution/executor.py index 7e8cd3ce..802c18db 100644 --- a/keel/execution/executor.py +++ b/keel/execution/executor.py @@ -346,6 +346,7 @@ def _build_intent( notional=sizing.spend(qty, setup.entry), is_dca=is_dca, rule_kind=signal.rule_name, + rule_id=signal.rule_id, available_quote=available_quote, withdrawals_enabled=withdrawals, ) @@ -365,6 +366,7 @@ def _build_intent( notional=sizing.spend(qty, entry), is_dca=False, rule_kind=signal.rule_name, + rule_id=signal.rule_id, ) @@ -590,7 +592,7 @@ def _order_row(intent: OrderIntent, mode: str, now_ts: int) -> dict[str, Any]: # called `'autonomous'`. Nothing reads this column back -- it is an audit trail only -- # so the rows are deliberately left as-written rather than rewritten by a migration. confirmation=mode, - rule_id=None, + rule_id=intent.rule_id, created_at=now_ts, updated_at=now_ts, ) diff --git a/keel/execution/guards.py b/keel/execution/guards.py index 9e9ad8a3..f1429a34 100644 --- a/keel/execution/guards.py +++ b/keel/execution/guards.py @@ -119,6 +119,12 @@ class OrderIntent: notional: Decimal is_dca: bool rule_kind: str + # The originating `rules.id` DB row (`signal.rule_id`, threaded from `agent._build_rule`), + # carried through purely so `executor._order_row` can write it to `orders.rule_id` -- no rail + # reads this, and it plays no part in any guard decision. `None` (the default) for an intent + # built from a hand-constructed `Rule`/`Signal` (most tests) or a caller that doesn't thread + # one (`place_bracket`/`scale_out`/`_roll_stop`, which only have a `rule_name` string). + rule_id: int | None = None # Rail 13 (USDC-funding): the live available quote-currency (default USDC) balance, fetched # by the caller (the executor) from the broker -- guards has no broker access of its own. # `None` means "unknown/unavailable" and fails the BUY closed, same as a missing quote diff --git a/keel/strategy/engine.py b/keel/strategy/engine.py index a734de7a..77b4f044 100644 --- a/keel/strategy/engine.py +++ b/keel/strategy/engine.py @@ -136,6 +136,7 @@ def evaluate( cts_score=cts_result.total, entry_technique=technique, ts=setup.ts, + rule_id=rule.rule_id, ) signals.append(signal) log_event( @@ -321,10 +322,11 @@ def _persist_signal(repo: Repository, signal: Signal, cts_result: indicators_cts """Write an emitted `Signal` to the `signals` table via `Repository.insert_signal` (schema: `data/db.py`; P3 Task 1 added the typed method). - `rule_id` is left `NULL` (no DB-backed `rules` row lookup is wired here -- Task 9's - promotion lifecycle owns that linkage); `indicators` carries the full CTS breakdown + - setup for audit/explainability; `fired=1` marks this as an actionable, gate-cleared - signal -- rejected candidates are never persisted at all. + `rule_id` carries `signal.rule_id` -- the originating `rules.id`, threaded from + `agent._build_rule` through the `Rule` that emitted this `Signal` (`None` for a signal from a + hand-constructed rule, e.g. most tests); `indicators` carries the full CTS breakdown + setup + for audit/explainability; `fired=1` marks this as an actionable, gate-cleared signal -- + rejected candidates are never persisted at all. """ setup = signal.setup payload = { @@ -347,7 +349,7 @@ def _persist_signal(repo: Repository, signal: Signal, cts_result: indicators_cts } repo.insert_signal( { - "rule_id": None, + "rule_id": signal.rule_id, "product_id": signal.product_id, "ts": signal.ts, "indicators": json.dumps(payload, default=str), diff --git a/keel/strategy/paper.py b/keel/strategy/paper.py index b17667c9..2b0a5262 100644 --- a/keel/strategy/paper.py +++ b/keel/strategy/paper.py @@ -9,16 +9,18 @@ `orders(mode='paper')` via `Repository`. No live order placement ever happens here (Phase 3 `execution/*` owns that). -**Schema note:** `orders.rule_id` is an `INTEGER` foreign key into the `rules` table -(populated by promotion/demotion, task 9); paper fills from a rule that hasn't been -persisted there yet would violate that FK, so paper orders leave `rule_id` NULL and -instead carry `rule_name` plus this module's own reconstruction fields (`role`, -`entry_order_id`, pnl/mfe/mae/outcome/etc.) JSON-encoded in `orders.raw_response` -- -the column live trading reserves for a broker's raw response, unused for paper -fills. `track_record()` reads that back out (via `Repository.get_orders(mode='paper')`, -P3 Task 1) to reconstruct `Trade`s and aggregates them via the shared -`strategy.stats.summarize` helper into the same `BacktestResult` shape `backtest.py` -produces, so paper and historical stats are directly comparable. +**Schema note:** `orders.rule_id` is an `INTEGER` foreign key into the `rules` table. A paper +`Signal` carries `rule_id` only when it was emitted for a rule reconstructed from a persisted +`rules` row (`agent._build_rule`) -- which is always true for a real paper cycle (paper mode +loads its rules the same way live mode does, from `repo.get_rules("paper")`), so the FK is +satisfied by construction; a hand-built `Signal` with no `rule_id` (most tests) still writes +`NULL`, exactly as before. Either way, `rule_name` plus this module's own reconstruction fields +(`role`, `entry_order_id`, pnl/mfe/mae/outcome/etc.) are still JSON-encoded in +`orders.raw_response` -- the column live trading reserves for a broker's raw response, unused for +paper fills -- since `rule_id` alone doesn't carry that bookkeeping. `track_record()` reads that +back out (via `Repository.get_orders(mode='paper')`, P3 Task 1) to reconstruct `Trade`s and +aggregates them via the shared `strategy.stats.summarize` helper into the same `BacktestResult` +shape `backtest.py` produces, so paper and historical stats are directly comparable. """ from __future__ import annotations @@ -67,6 +69,10 @@ class _OpenPaperPosition: whether a debit actually happened keeps the close-side credit and the equity mark correct-by-construction regardless of seed timing. """ + #: The entry order's `rule_id` (its DB row's real value, `None` if it had none) -- carried + #: so the paired exit order writes the SAME `rule_id`, mirroring how `rule_name` above is + #: taken from the position rather than re-read off the exit signal. + rule_id: int | None = None def _touches(candle: Candle, price: Decimal) -> bool: @@ -165,6 +171,10 @@ def _load_open_positions(self) -> None: # that id is only ever stamped by `seed_cash` -- so every rehydrated # position was opened while cash was seeded, and was costed at the time. costed=True, + # The entry order's own `rule_id` column -- a real DB value, not something the + # JSON payload needs to carry -- so a restart-rehydrated position still closes + # with the same `rule_id` a same-process exit would have written. + rule_id=order.get("rule_id"), ) def has_open_position(self, product_id: str) -> bool: @@ -319,7 +329,7 @@ def _enter(self, signal: Signal, qty: Decimal = _QTY) -> int | None: "actual_fill": entry_fill, "raw_response": json.dumps(payload), "confirmation": "paper", - "rule_id": None, + "rule_id": signal.rule_id, "created_at": signal.ts, "updated_at": signal.ts, } @@ -333,6 +343,7 @@ def _enter(self, signal: Signal, qty: Decimal = _QTY) -> int | None: entry_ts=setup.ts, qty=qty, costed=costed, + rule_id=signal.rule_id, ) if costed: self._cash -= entry_fill * qty + fee @@ -392,7 +403,7 @@ def _close(self, position: _OpenPaperPosition, exit_price: Decimal, exit_ts: int "actual_fill": exit_fill, "raw_response": json.dumps(payload), "confirmation": "paper", - "rule_id": None, + "rule_id": position.rule_id, "created_at": exit_ts, "updated_at": exit_ts, } diff --git a/keel/strategy/rules/base.py b/keel/strategy/rules/base.py index 9ff2667e..2739e850 100644 --- a/keel/strategy/rules/base.py +++ b/keel/strategy/rules/base.py @@ -65,6 +65,10 @@ class Signal: cts_score: int entry_technique: str ts: int + #: The originating `rules.id` DB row, when the emitting `Rule` was reconstructed via + #: `agent._build_rule` (which threads `Rule.rule_id` through). `None` for a hand-constructed + #: `Rule`/`Signal` (most tests) -- purely additive metadata, never read by any gate/guard. + rule_id: int | None = None @dataclass @@ -92,11 +96,18 @@ class Rule(ABC): `promotion_class` selects the rule's promotion floor (`strategy.promotion.floor_for_class`): the default `"default"` uses the canonical 100/0.55 floor; trend-followers override it to `"trend_follow"` for a low-win/high-R:R floor (KB ยง25.5). + + `rule_id` is the originating `rules.id` DB row, set by `agent._build_rule` on a rule loaded + from `repo.get_rules()`. It defaults to `None` -- same pattern as `promotion_class` -- so + every hand-constructed `Rule` in tests (or anywhere else) is unaffected; it exists purely so + the id can be threaded onto emitted `Signal`s and, from there, into `orders.rule_id` for + audit -- it plays no part in `detect`/`exit_signal`/any guard or gate. """ name: str params: dict promotion_class: str = "default" + rule_id: int | None = None @abstractmethod def detect(self, candles_by_tf: dict[Granularity, list[Candle]]) -> Setup | None: diff --git a/tests/data/test_repository.py b/tests/data/test_repository.py index d06c5016..7ab96d88 100644 --- a/tests/data/test_repository.py +++ b/tests/data/test_repository.py @@ -179,6 +179,26 @@ def test_insert_order_ids_increment(repo): assert id2 > id1 +def test_insert_order_round_trips_a_non_none_rule_id(repo): + """`orders.rule_id` is a real FK into `rules` -- once the row exists, an order naming it + must round-trip that id exactly, not just `None` (the `_order()` default).""" + rule_id = repo.insert_rule("pullback_continuation", {"lookback": 20}) + + order_id = repo.insert_order(_order(rule_id=rule_id)) + + stored = repo.get_order(order_id) + assert stored["rule_id"] == rule_id + + +def test_get_orders_also_round_trips_rule_id(repo): + rule_id = repo.insert_rule("dca", {}) + repo.insert_order(_order(rule_id=rule_id)) + repo.insert_order(_order(rule_id=None)) + + rows = repo.get_orders() + assert {r["rule_id"] for r in rows} == {rule_id, None} + + def test_update_order_updates_fields_and_round_trips_decimal(repo): order_id = repo.insert_order(_order()) @@ -348,6 +368,15 @@ def test_insert_signal_returns_id_and_round_trips(repo): assert row["fired"] == 1 +def test_insert_signal_round_trips_a_non_none_rule_id(repo): + rule_id = repo.insert_rule("rsi_meanrev", {}) + + signal_id = repo.insert_signal(_signal(rule_id=rule_id)) + + row = repo._conn.execute("SELECT * FROM signals WHERE id = ?", (signal_id,)).fetchone() + assert row["rule_id"] == rule_id + + def test_insert_signal_ids_increment(repo): id1 = repo.insert_signal(_signal()) id2 = repo.insert_signal(_signal()) diff --git a/tests/execution/test_executor.py b/tests/execution/test_executor.py index 529b7379..e131b2da 100644 --- a/tests/execution/test_executor.py +++ b/tests/execution/test_executor.py @@ -349,6 +349,98 @@ def test_confirm_mode_without_confirm_fn_defaults_to_not_placed(repo): assert len(broker.place_calls) == 0 +# -- rule_id metadata (Phase-2 debt: orders.rule_id was always written NULL) ---------------------- + + +def test_placed_order_carries_the_signals_rule_id(repo): + """The fix under test: `orders.rule_id` used to be hardcoded `None` in `_order_row` + regardless of the signal. It must now carry `signal.rule_id` end to end through + `_build_intent`'s `OrderIntent.rule_id`. + """ + rule_id = repo.insert_rule("pullback_continuation", {}, status="live") + broker = FakeBroker() + signal = _enter_signal(rule_id=rule_id) + + result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) + + assert result.placed is True + order = repo.get_order(result.order_id) + assert order["rule_id"] == rule_id + + +def test_a_signal_with_no_rule_id_still_writes_none(repo): + """Backward-compat: a signal from a hand-constructed rule (no `rule_id` supplied, the + default) still writes `NULL`, exactly as before this fix.""" + broker = FakeBroker() + signal = _enter_signal() # no rule_id override -> defaults to None + + result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) + + assert result.placed is True + order = repo.get_order(result.order_id) + assert order["rule_id"] is None + + +def test_rule_id_is_purely_additive_metadata_placement_and_guards_are_unchanged(repo): + """The metadata-only guarantee: two otherwise-identical signals, differing only in + `rule_id`, must produce byte-for-byte identical guard/placement outcomes -- same veto + decisions, same `placed`, same broker calls/order-configuration, same sized qty/notional. + The ONLY difference in the resulting order rows is the `rule_id` column. + """ + rule_id = repo.insert_rule("pullback_continuation", {}, status="live") + broker_a = FakeBroker() + broker_b = FakeBroker() + signal_no_id = _enter_signal(rule_id=None) + signal_with_id = _enter_signal(rule_id=rule_id) + + result_a = execute(signal_no_id, broker_a, repo, _config(), mode="autonomous", now_ts=NOW_TS) + result_b = execute( + signal_with_id, broker_b, repo, _config(), mode="autonomous", now_ts=NOW_TS + 1 + ) + + assert result_a.placed == result_b.placed is True + assert result_a.vetoed_by == result_b.vetoed_by == [] + assert len(broker_a.preview_calls) == len(broker_b.preview_calls) + assert len(broker_a.place_calls) == len(broker_b.place_calls) + assert ( + broker_a.place_calls[0]["order_configuration"] + == broker_b.place_calls[0]["order_configuration"] + ) + + order_a = repo.get_order(result_a.order_id) + order_b = repo.get_order(result_b.order_id) + # Every field EXCEPT rule_id/created_at/updated_at/id must match -- proving rule_id is the + # only thing that changed. + for field in ( + "mode", + "product_id", + "side", + "order_type", + "qty", + "status", + "confirmation", + ): + assert order_a[field] == order_b[field], f"{field} differs -- not metadata-only" + assert order_a["rule_id"] is None + assert order_b["rule_id"] == rule_id + + +def test_rail_violating_signal_with_a_rule_id_is_still_vetoed_the_same_way(repo): + """`rule_id` must not influence guard decisions -- a vetoed intent stays vetoed.""" + rule_id = repo.insert_rule("pullback_continuation", {}, status="live") + broker = NoNetworkBroker() + signal = _enter_signal( + product_id="DOGE-USD", setup=_setup(product_id="DOGE-USD"), rule_id=rule_id + ) + + result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) + + assert result.placed is False + assert result.order_id is None + assert any(v.startswith("halal_allowlist") for v in result.vetoed_by) + assert repo.get_orders() == [] + + # -- rail-violating signal -> vetoed, never previews/places -------------------------------------- diff --git a/tests/strategy/test_base.py b/tests/strategy/test_base.py index af670782..c03a789c 100644 --- a/tests/strategy/test_base.py +++ b/tests/strategy/test_base.py @@ -88,6 +88,36 @@ def test_constructs_with_expected_fields(self) -> None: assert signal.entry_technique == "signal_candle" assert signal.ts == 1_700_000_100 + def test_rule_id_defaults_to_none(self) -> None: + """A hand-constructed `Signal` (most tests, and any caller that predates this field) + must still build with no `rule_id` kwarg -- additive, backward-compatible default.""" + setup = _setup() + signal = Signal( + rule_name="pullback_continuation", + product_id="BTC-USD", + action=Action.ENTER, + side=Side.BUY, + setup=setup, + cts_score=7, + entry_technique="signal_candle", + ts=1_700_000_100, + ) + assert signal.rule_id is None + + def test_rule_id_round_trips_when_supplied(self) -> None: + signal = Signal( + rule_name="pullback_continuation", + product_id="BTC-USD", + action=Action.ENTER, + side=Side.BUY, + setup=None, + cts_score=7, + entry_technique="signal_candle", + ts=0, + rule_id=42, + ) + assert signal.rule_id == 42 + def test_setup_may_be_none_for_none_action(self) -> None: signal = Signal( rule_name="rsi_meanrev", @@ -187,3 +217,17 @@ def test_concrete_subclass_satisfies_the_abc(self) -> None: assert rule.describe() == {"name": "trivial", "params": {}} assert rule.name == "trivial" assert rule.params == {} + + def test_rule_id_defaults_to_none(self) -> None: + """A hand-constructed `Rule` (most tests, and every rule not loaded via + `agent._build_rule`) must still build with no `rule_id` -- additive, backward-compatible + default, same pattern as `promotion_class`.""" + rule = _TrivialRule() + assert rule.rule_id is None + + def test_rule_id_is_settable_after_construction(self) -> None: + """`agent._build_rule` sets this as a plain attribute post-construction (not a + constructor kwarg) -- `Rule` is a plain mutable object, not frozen.""" + rule = _TrivialRule() + rule.rule_id = 7 + assert rule.rule_id == 7 diff --git a/tests/strategy/test_paper.py b/tests/strategy/test_paper.py index f442918c..2d7b98a1 100644 --- a/tests/strategy/test_paper.py +++ b/tests/strategy/test_paper.py @@ -54,6 +54,7 @@ def _enter_signal( product_id: str = "BTC-USD", setup: Setup | None = None, ts: int = 1_000, + rule_id: int | None = None, ) -> Signal: return Signal( rule_name=rule_name, @@ -64,11 +65,15 @@ def _enter_signal( cts_score=7, entry_technique="signal_candle", ts=ts, + rule_id=rule_id, ) def _exit_signal( - rule_name: str = "pullback_continuation", product_id: str = "BTC-USD", ts: int = 1_000 + rule_name: str = "pullback_continuation", + product_id: str = "BTC-USD", + ts: int = 1_000, + rule_id: int | None = None, ) -> Signal: return Signal( rule_name=rule_name, @@ -79,6 +84,7 @@ def _exit_signal( cts_score=0, entry_technique="confirm_3bar", ts=ts, + rule_id=rule_id, ) @@ -100,6 +106,30 @@ def test_entry_order_is_written_and_filled(self, repo: Repository) -> None: expected_entry_fill = Decimal("100") * (Decimal(1) + SLIPPAGE_PCT) assert order["actual_fill"] == expected_entry_fill + def test_entry_order_sets_the_rule_id_column_and_keeps_the_raw_response_rule_name( + self, repo: Repository + ) -> None: + """The Phase-2 debt this branch fixes on the paper path: `orders.rule_id` used to be + hardcoded `None` for every paper fill. `raw_response.rule_name` is untouched -- other + code (`track_record`) still reads it -- this only ADDS the column.""" + rule_id = repo.insert_rule("pullback_continuation", {}, status="paper") + trader = PaperTrader(repo) + entry_order_id = trader.on_signal(_enter_signal(rule_id=rule_id)) + + order = repo.get_order(entry_order_id) + assert order["rule_id"] == rule_id + payload = json.loads(order["raw_response"]) + assert payload["rule_name"] == "pullback_continuation" + + def test_entry_order_with_no_rule_id_still_writes_none(self, repo: Repository) -> None: + """Backward-compat: a signal from a hand-constructed rule (no `rule_id`) still writes + `NULL`, exactly as before this fix.""" + trader = PaperTrader(repo) + entry_order_id = trader.on_signal(_enter_signal()) + + order = repo.get_order(entry_order_id) + assert order["rule_id"] is None + def test_target_hit_writes_exit_order_with_correct_pnl(self, repo: Repository) -> None: trader = PaperTrader(repo) trader.on_signal(_enter_signal(ts=1_000)) @@ -128,6 +158,17 @@ def test_target_hit_writes_exit_order_with_correct_pnl(self, repo: Repository) - assert payload["outcome"] == "win" assert payload["entry_order_id"] is not None + def test_exit_order_carries_the_same_rule_id_as_its_entry(self, repo: Repository) -> None: + rule_id = repo.insert_rule("pullback_continuation", {}, status="paper") + trader = PaperTrader(repo) + trader.on_signal(_enter_signal(ts=1_000, rule_id=rule_id)) + + target_candle = _candle(1_060, "115", "121", "114", "120") + exit_order_id = trader.on_candle("BTC-USD", target_candle) + + exit_order = repo.get_order(exit_order_id) + assert exit_order["rule_id"] == rule_id + def test_no_touch_leaves_position_open_and_writes_no_exit_order( self, repo: Repository ) -> None: @@ -340,6 +381,40 @@ def test_open_positions_survive_a_new_PaperTrader_over_the_same_repo(repo): assert second.has_open_position("BTC-USD") is True +def test_a_rehydrated_position_still_closes_with_its_original_rule_id(repo): + """`rule_id` isn't in the `raw_response` JSON payload (it's a real DB column) -- + `_load_open_positions` must read it off the entry order row directly, so a + restart-rehydrated position's exit still carries the same `rule_id` a same-process exit + would have written.""" + rule_id = repo.insert_rule("pullback_continuation", {}, status="paper") + first = PaperTrader(repo) + first.on_signal(_enter_signal(rule_id=rule_id)) + + resumed = PaperTrader(repo) + exit_id = resumed.on_candle("BTC-USD", _candle(2_000, "110", "125", "108", "122")) + + assert exit_id is not None + assert repo.get_order(exit_id)["rule_id"] == rule_id + + +def test_a_rehydrated_position_with_a_legacy_null_rule_id_still_closes_without_crashing(repo): + """A pre-change entry order (written before this fix, or from a signal with no `rule_id`) + has `rule_id = NULL` in the DB. `_load_open_positions` reads it via `order.get("rule_id")`, + not `order["rule_id"]`/direct attribute access on a dataclass default -- proving the legacy + NULL case rehydrates cleanly (no KeyError/crash) and the resulting exit also writes NULL, + never fabricating an id that was never there. + """ + first = PaperTrader(repo) + entry_order_id = first.on_signal(_enter_signal()) # no rule_id -> NULL in the DB + assert repo.get_order(entry_order_id)["rule_id"] is None + + resumed = PaperTrader(repo) + exit_id = resumed.on_candle("BTC-USD", _candle(2_000, "110", "125", "108", "122")) + + assert exit_id is not None + assert repo.get_order(exit_id)["rule_id"] is None + + def test_a_closed_position_does_NOT_reopen_on_rehydration(repo): first = PaperTrader(repo) first.on_signal(_enter_signal()) diff --git a/tests/test_agent.py b/tests/test_agent.py index e960d7ae..3614d76f 100644 --- a/tests/test_agent.py +++ b/tests/test_agent.py @@ -317,6 +317,35 @@ def test_run_once_polls_evaluates_and_executes_a_real_dca_rule(repo): assert repo.get_state("last_feed_ts") == 90_000 +def test_run_once_writes_the_seeded_rules_db_id_onto_the_order(repo): + """The metadata-only fix under test: a full cycle's placed order now carries the + originating rule's real `rules.id`, threaded `_build_rule` -> `Rule.rule_id` -> + `engine.evaluate`'s `Signal.rule_id` -> `executor._build_intent`'s `OrderIntent.rule_id` -> + `executor._order_row`. Everything else about the placed order matches + `test_run_once_polls_evaluates_and_executes_a_real_dca_rule` exactly -- proving this is + ADDITIVE metadata, not a change to what gets placed. + """ + rule_id = repo.insert_rule("dca", {"product_id": PRODUCT}, status="live") + broker = FakeBroker(series={(PRODUCT, Granularity.ONE_DAY): [_candle(0, "100")]}) + + result = run_once(broker, repo, _config(), now_ts=90_000) + + assert result.skipped is False + assert len(result.enter_signals) == 1 + assert result.enter_signals[0].rule_id == rule_id + assert result.enter_results[0].placed is True + + orders = repo.get_orders(mode="live", product_id=PRODUCT) + assert len(orders) == 1 + order = orders[0] + assert order["rule_id"] == rule_id + # Everything else about the order is unchanged from the pre-fix shape/values. + assert order["side"] == "BUY" + assert order["qty"] == Decimal("0.5") + assert order["status"] == "filled" + assert len(broker.place_calls) == 1 + + # -- run_once: autonomy is a live-read profile choice -------------------------------------------- @@ -408,7 +437,7 @@ def test_paper_mode_is_reported_as_paper_not_confirm(repo): def test_held_position_whose_exit_fires_gets_an_exit_order(repo): - repo.insert_rule("fake_exit", {"product_id": PRODUCT}, status="live") + rule_id = repo.insert_rule("fake_exit", {"product_id": PRODUCT}, status="live") _seed_open_position(repo, PRODUCT, Decimal("0.1"), Decimal("50000"), ts=1_000) repo.set_state(f"position_rule:{PRODUCT}", "fake_exit") broker = FakeBroker(series={(PRODUCT, Granularity.ONE_DAY): [_candle(0, "100")]}) @@ -423,6 +452,9 @@ def test_held_position_whose_exit_fires_gets_an_exit_order(repo): sell_orders = [o for o in orders if o["side"] == "SELL"] assert len(sell_orders) == 1 assert sell_orders[0]["qty"] == Decimal("0.1") + # the exit Signal built in `_handle_exits` threads the owning rule's real DB id -- + # same fix as the ENTER path, exercised here on the LIVE EXIT path. + assert sell_orders[0]["rule_id"] == rule_id # the position is no longer tracked as open once the exit is placed. assert not repo.get_state(f"position_rule:{PRODUCT}") @@ -564,6 +596,30 @@ def test_build_rule_reconstructs_a_real_pullback_continuation_rule(): assert rule.params["ema_periods"] == (8, 20, 50) +def test_build_rule_populates_rule_id_from_the_row(repo): + """The Phase-2 debt this branch fixes: `_build_rule` used to discard `row["id"]` entirely, + which is the root cause of `orders.rule_id` always being written NULL.""" + rule_id = repo.insert_rule("dca", {"product_id": PRODUCT}) + row = repo.get_rules()[0] + + rule = _build_rule(row) + + assert rule.rule_id == rule_id + + +def test_build_rule_leaves_rule_id_none_for_a_row_with_no_id(): + """A hand-built row (no "id" key -- e.g. a caller assembling params directly, not via + `repo.get_rules()`) must not raise; `rule_id` just stays at its default `None`.""" + row = { + "kind": "dca", + "params": {"product_id": PRODUCT}, + } + + rule = _build_rule(row) + + assert rule.rule_id is None + + def test_build_rule_unknown_kind_raises(): with pytest.raises(ValueError, match="dca"): _build_rule({"kind": "not_a_real_rule_dca", "params": {}}) @@ -1071,6 +1127,31 @@ def test_paper_mode_records_a_fill_and_never_places_or_reads_account_state(repo, assert track_record(repo, "fake_enter").n_trades >= 0 +def test_paper_mode_full_cycle_writes_the_seeded_rules_db_id_onto_the_order(repo, monkeypatch): + """Same fix as the live path, exercised end-to-end on the PAPER path: this test does NOT + monkeypatch `_build_rule` (unlike `_seed_rule`'s helper above), so the real rule-id threading + runs -- `repo.get_rules()` -> `_build_rule` -> `Rule.rule_id` -> `Signal.rule_id` -> + `PaperTrader._enter`'s inserted order. + """ + monkeypatch.setitem(agent.RULE_REGISTRY, "fake_enter", _AlwaysEnterRule) + rule_id = repo.insert_rule("fake_enter", {"product_id": PRODUCT}, status="paper") + + broker = _MarketDataOnlyBroker(series={(PRODUCT, Granularity.ONE_DAY): [_candle(0, "100")]}) + cfg = _paper_config(paper=PaperConfig(starting_equity_usd=Decimal("100000"))) + + result = run_once(broker, repo, cfg, now_ts=90_000) + + assert result.skipped is False + orders = repo.get_orders(mode="paper") + assert len(orders) == 1 + assert orders[0]["rule_id"] == rule_id + # the rule_name blob in raw_response is untouched by this fix -- still present. + import json + + payload = json.loads(orders[0]["raw_response"]) + assert payload["rule_name"] == "fake_enter" + + def test_paper_mode_still_enforces_the_offline_rails(repo, monkeypatch): """Paper runs the rails deliberately -- the promotion gate is scored on this record.""" # A 0.01% entry-to-stop move trips rail 7 (min-move / anti-scalping) -- an OFFLINE-computable