Test log isolation, stale references, DCA stop distance, manifest name refusal (#964 #967 #973 #974)

keel/capabilities.py

@@ -287,8 +287,9 @@ def gate_named(name: str) -> Gate:
 
 
 def render_lines() -> list[str]:
-    """The inventory as text -- ONE renderer, so the CLI and the browser view cannot drift into
-    two accounts of the same safety model."""
+    """The inventory as text, read by `keel.commands.capabilities` (`keel capabilities`). ONE
+    renderer, so a second text surface reuses it rather than writing a second account of the same
+    safety model; the MCP tool serves `CAPABILITIES` itself, as data."""
     lines: list[str] = []
     for gate in GATES:
         covered = [cap for cap in CAPABILITIES if cap.gate == gate.name]

keel/commands/_common.py

@@ -256,10 +256,11 @@ def record_cash_posture_refutation(
     and not proof of a cash posture -- promoting a record on the strength of it would manufacture
     exactly the affirmation #666 established cannot be had.
 
-    `repo=None` is the read-only call sites (`keel balances`, `keel brokers list`), which have no
-    repository in hand and will not go on to trade. The CHECK still runs and still refuses for
-    them; only the recording is skipped, so threading a repo through every inspection command to
-    record a fact none of them acts on is churn nobody needs.
+    `repo=None` is every `_build_broker` caller with no repository in hand, none of which opens a
+    position: the inspections (`keel assets holdings`, `keel assets discover`), the candle fetch
+    behind `keel fetch`, `keel simulate` and the setup steps, and `keel orders cancel`. The CHECK
+    still runs and still refuses for them; only the recording is skipped, so threading a repo
+    through every one of them to record a fact none of them acts on is churn nobody needs.
     """
     try:
         broker.verify_cash_account()

keel/commands/doctor.py

@@ -1809,8 +1809,10 @@ def position_watch_findings(
       there -- the only real way out on paper is re-promoting the rule.
     * `position.unprotected` -- an open tranche with a recorded `initial_stop > 0`, no resting
       bracket (`reconcile._has_resting_bracket`, passed in as `resting`), and no retry record.
-      The third clause makes it the complement of the reconcile sweep, not a duplicate. DCA
-      (`initial_stop` absent) is excluded for the reason the sweep skips it silently. SKIPPED
+      The third clause makes it the complement of the reconcile sweep, not a duplicate. DCA is
+      excluded for the reason the sweep skips it silently: an unattributed leg stores no
+      `initial_stop`, and an attributed one stores `Dca.detect`'s `0` sentinel (#973), which the
+      `> 0` clause reads as no stop, as `status.recorded_stop` does. SKIPPED
       ENTIRELY (reported `OK`) when `managed_status == "paper"` (#881): a paper fill never gets
       a bracket (`_paper_enter` places none), so `resting` is always False, and paper never
       writes an `unbracketed:` retry record either (`reconcile_unbracketed_positions` only runs

keel/commands/positions.py

@@ -22,9 +22,11 @@
 WHAT IS ABSENT STAYS ABSENT. A product with no cached candle has no mark, and a row with no mark
 reports `None` for every figure that depends on one -- never zero. Zero market value renders a
 held position as a total loss, which is the most alarming thing this page could say, and it
-would be saying it about missing data. `initial_stop` is `None` for a DCA leg or a pre-v12
-tranche, so its distance is `None` too: a distance measured against a substituted zero would
-read as a position comfortably clear of a stop it does not have.
+would be saying it about missing data. A tranche with no stop has no stop distance either: a
+distance measured against a substituted zero would read as a position comfortably clear of a stop
+it does not have. "No stop" is `status.recorded_stop`'s reading, not `initial_stop is None`: the
+column is NULL on a pre-v12 tranche or an unattributed DCA leg, but `agent._open_tranche` copies
+`Dca.detect`'s `stop=0` sentinel onto every attributed DCA tranche, which stores "0" (#973).
 
 NO CLOSE ACTION, EVER (#701's own refusal). This module places nothing and cancels nothing.
 Exits degrade to the typed-phrase friction path; a panic tap on a table row must never be the
@@ -41,6 +43,7 @@
 
 from keel import agent as agent_mod
 from keel.commands.doctor import ATTEST_WINDOW_APPROACHING_SEC as _DOCTOR_APPROACHING_SEC
+from keel.commands.status import recorded_stop
 from keel.config import Config
 from keel.data import freshness as freshness_mod
 from keel.data.repository import Repository
@@ -117,14 +120,18 @@ class PositionRow:
     market_value: Decimal | None
     unrealized_pnl: Decimal | None
 
-    #: The stop this tranche was SIZED against (`positions.initial_stop`). `None` means "not on
-    #: this row" -- a DCA leg, or a tranche predating v12 -- and NOT "no stop".
+    #: `positions.initial_stop` AS STORED: the stop this tranche was sized against, `None` on a
+    #: tranche predating v12 or an unattributed DCA leg, and `0` on an attributed DCA tranche (the
+    #: `Dca.detect` sentinel, which is no stop). Carried verbatim because Holdings' paper reading
+    #: names that 0 (`status.shown_stop`); anything that MEASURES against the stop reads it
+    #: through `status.recorded_stop`, as `stop_distance` does.
     initial_stop: Decimal | None
 
     #: `mark - initial_stop`, SIGNED: a tranche trading THROUGH its stop is the state an operator
     #: most needs to see, and an absolute distance would render it identically to one safely
     #: above. `stop_distance_pct` is that distance as a fraction OF THE MARK. Both `None` when
-    #: either the stop or the mark is missing.
+    #: the mark is missing or the tranche has no stop (`status.recorded_stop` is `None`: NULL,
+    #: or the zero a DCA tranche stores).
     stop_distance: Decimal | None
     stop_distance_pct: Decimal | None
 
@@ -391,15 +398,16 @@ def _row_from_dict(
     qty = raw["qty"]
     entry_fill = raw["entry_fill"]
     initial_stop = raw.get("initial_stop")
+    stop = recorded_stop(initial_stop)
 
     market_value = None if mark is None else qty * mark
     unrealized = None if mark is None else qty * (mark - entry_fill)
 
-    if mark is None or initial_stop is None:
+    if mark is None or stop is None:
         stop_distance: Decimal | None = None
         stop_distance_pct: Decimal | None = None
     else:
-        stop_distance = mark - initial_stop
+        stop_distance = mark - stop
         stop_distance_pct = _safe_ratio(stop_distance, mark)
 
     return PositionRow(

keel/commands/rules.py

@@ -2305,6 +2305,95 @@ def rules_add(ctx: click.Context, kind: str, product: str, params_json: str | No
         ctx.exit(1)
 
 
+def _require_rule_class(kind: str, echo_err: Callable[[str], None]) -> type:
+    """`kind`'s rule class, or the `unknown rule kind` refusal (echoed, then `RulesRefused`)."""
+    rule_cls = agent.RULE_REGISTRY.get(kind)
+    if rule_cls is None:
+        echo_err(f"Error: unknown rule kind {kind!r}; known kinds: {sorted(agent.RULE_REGISTRY)!r}")
+        raise RulesRefused(f"unknown rule kind {kind!r}")
+    return rule_cls
+
+
+def validated_row_params(
+    kind: str,
+    supplied: dict[str, Any],
+    product_id: str,
+    *,
+    echo_err: Callable[[str], None] = _noop,
+) -> dict[str, Any]:
+    """The params a `rules` row stores for `supplied`, or a refusal -- `add_rule_row`'s checks on
+    the params themselves, as one function so every writer of `rules` refuses the same rows.
+
+    Builds the rule, then returns its `describe()` params (JSON-plain, `product_id` re-applied):
+    exactly what `agent._build_rule` reconstructs. Refuses -- `Error: ...` lines to `echo_err`,
+    then `RulesRefused`, nothing written -- an unknown kind, a mistyped or unknown param, one the
+    rule's own validation rejects, a non-finite number, and a param the row would not persist.
+
+    That last one is the `name` refusal #961 depends on (#974): `describe()` carries no `name`
+    for any kind, so a `"name": "dca"` on a turtle is a dropped param and never reaches the row
+    whose rebuilt `rule.name` becomes `positions.rule_name`. Read by `add_rule_row` and by
+    `scripts/rule_manifest.py apply`, the two writers that take params from outside."""
+    rule_cls = _require_rule_class(kind, echo_err)
+    mismatches = _param_type_mismatches(kind, rule_cls, supplied)
+    if mismatches:
+        quotable = sorted(agent.coerced_param_keys(kind))
+        echo_err(
+            f"Error: {kind} cannot use these params:\n"
+            + "\n".join(f"       - {problem}" for problem in mismatches)
+            + f"\n       (a quoted value is right only for {quotable!r} -- keel converts "
+            f"those on the way in)"
+        )
+        raise RulesRefused(f"{kind} cannot use these params")
+
+    try:
+        rule = agent.build_rule_from_params(kind, {**supplied, "product_id": product_id})
+    except TypeError as exc:
+        # An unknown/misspelled kwarg: the constructor names it, we name the alternatives.
+        echo_err(
+            f"Error: {kind} does not accept these params: {exc}\n"
+            f"       accepted params: {_accepted_params(rule_cls)!r}"
+        )
+        raise RulesRefused(f"{kind} does not accept these params") from exc
+    except (ValueError, ArithmeticError) as exc:
+        # The rule's OWN validation (e.g. `Dca`: "cadence_days must be positive"), or a value
+        # that is not the type the coercion boundary expects (`Decimal("x")` -> InvalidOperation,
+        # an `ArithmeticError`). Either way the params cannot make a rule, so no row.
+        echo_err(f"Error: {kind} rejected these params: {exc}")
+        raise RulesRefused(f"{kind} rejected these params") from exc
+
+    nonfinite = _nonfinite_params(rule.describe()["params"])
+    if nonfinite:
+        echo_err(
+            f"Error: {kind} was given a value that is not a finite number, which no backtest "
+            f"can report on:\n" + "\n".join(f"       - {problem}" for problem in nonfinite)
+        )
+        raise RulesRefused(f"{kind} was given a non-finite value")
+
+    # `.describe()`'s params, JSON-plain -- the same thing `rules seed` stores, so the row is
+    # exactly what `agent._build_rule` reconstructs. `product_id` is re-applied because not
+    # every kind carries it in `describe()` (e.g. `TurtleBreakout` does not).
+    params: dict[str, Any] = _json_plain(rule.describe()["params"])
+    params["product_id"] = product_id
+
+    # A constructor kwarg that `describe()` does not carry would be accepted, then LOST: the row
+    # rebuilds without it and the backtest measures a different rule than the one asked for.
+    # `PullbackContinuation(granularity=ONE_DAY)` is exactly this -- it constructs, `params` has
+    # no `granularity` key, and `_build_rule` rebuilds it at the ONE_HOUR default, on a different
+    # candle series. Silently trading a rule the operator did not ask for is worse than a
+    # refusal, so this is a refusal. Derived from the rule's own `describe()`, so a kind that
+    # starts persisting a field needs no change here.
+    dropped = sorted(set(supplied) - set(params))
+    if dropped:
+        echo_err(
+            f"Error: {kind} accepts {dropped!r} but does not persist "
+            f"{'them' if len(dropped) > 1 else 'it'} in the rule row, so the value would be "
+            f"silently lost when the rule is rebuilt for a backtest or a cycle. Refusing rather "
+            f"than storing a rule that is not the one you asked for."
+        )
+        raise RulesRefused(f"{kind} does not persist {dropped!r}")
+    return params
+
+
 def add_rule_row(
     repo: Repository,
     config: Any,
@@ -2343,10 +2432,7 @@ def add_rule_row(
         )
     product_id = product_list[0]
 
-    rule_cls = agent.RULE_REGISTRY.get(kind)
-    if rule_cls is None:
-        echo_err(f"Error: unknown rule kind {kind!r}; known kinds: {sorted(agent.RULE_REGISTRY)!r}")
-        raise RulesRefused(f"unknown rule kind {kind!r}")
+    _require_rule_class(kind, echo_err)
 
     # "flag absent" and "flag given but empty" are different intentions, and only `is None`
     # tells them apart. An empty string is what a shell hands over when the proposal plumbing
@@ -2384,63 +2470,7 @@ def add_rule_row(
             param_hint="--params",
         )
 
-    mismatches = _param_type_mismatches(kind, rule_cls, supplied)
-    if mismatches:
-        quotable = sorted(agent.coerced_param_keys(kind))
-        echo_err(
-            f"Error: {kind} cannot use these params:\n"
-            + "\n".join(f"       - {problem}" for problem in mismatches)
-            + f"\n       (a quoted value is right only for {quotable!r} -- keel converts "
-            f"those on the way in)"
-        )
-        raise RulesRefused(f"{kind} cannot use these params")
-
-    try:
-        rule = agent.build_rule_from_params(kind, {**supplied, "product_id": product_id})
-    except TypeError as exc:
-        # An unknown/misspelled kwarg: the constructor names it, we name the alternatives.
-        echo_err(
-            f"Error: {kind} does not accept these params: {exc}\n"
-            f"       accepted params: {_accepted_params(rule_cls)!r}"
-        )
-        raise RulesRefused(f"{kind} does not accept these params") from exc
-    except (ValueError, ArithmeticError) as exc:
-        # The rule's OWN validation (e.g. `Dca`: "cadence_days must be positive"), or a value
-        # that is not the type the coercion boundary expects (`Decimal("x")` -> InvalidOperation,
-        # an `ArithmeticError`). Either way the params cannot make a rule, so no row.
-        echo_err(f"Error: {kind} rejected these params: {exc}")
-        raise RulesRefused(f"{kind} rejected these params") from exc
-
-    nonfinite = _nonfinite_params(rule.describe()["params"])
-    if nonfinite:
-        echo_err(
-            f"Error: {kind} was given a value that is not a finite number, which no backtest "
-            f"can report on:\n" + "\n".join(f"       - {problem}" for problem in nonfinite)
-        )
-        raise RulesRefused(f"{kind} was given a non-finite value")
-
-    # `.describe()`'s params, JSON-plain -- the same thing `rules seed` stores, so the row is
-    # exactly what `agent._build_rule` reconstructs. `product_id` is re-applied because not
-    # every kind carries it in `describe()` (e.g. `TurtleBreakout` does not).
-    params = _json_plain(rule.describe()["params"])
-    params["product_id"] = product_id
-
-    # A constructor kwarg that `describe()` does not carry would be accepted, then LOST: the row
-    # rebuilds without it and the backtest measures a different rule than the one asked for.
-    # `PullbackContinuation(granularity=ONE_DAY)` is exactly this -- it constructs, `params` has
-    # no `granularity` key, and `_build_rule` rebuilds it at the ONE_HOUR default, on a different
-    # candle series. Silently trading a rule the operator did not ask for is worse than a
-    # refusal, so this is a refusal. Derived from the rule's own `describe()`, so a kind that
-    # starts persisting a field needs no change here.
-    dropped = sorted(set(supplied) - set(params))
-    if dropped:
-        echo_err(
-            f"Error: {kind} accepts {dropped!r} but does not persist "
-            f"{'them' if len(dropped) > 1 else 'it'} in the rule row, so the value would be "
-            f"silently lost when the rule is rebuilt for a backtest or a cycle. Refusing rather "
-            f"than storing a rule that is not the one you asked for."
-        )
-        raise RulesRefused(f"{kind} does not persist {dropped!r}")
+    params = validated_row_params(kind, supplied, product_id, echo_err=echo_err)
 
     existing = [
         row

keel/commands/status.py

@@ -489,13 +489,17 @@ def _kind_never_brackets(rule_kind: str) -> bool:
     registry does not know (an empty `rule_name`, a retired kind) is NOT excused: its missing
     bracket stays a warning, the reading that cannot hide a real gap.
 
-    `rule_kind` is `positions.rule_name`, which is `rule.name`. No shipped path stores a name:
-    seed stores `describe()["params"]`, which carries no `name` for any kind, and `keel rules
-    add` refuses a `name` in `--params` (its dropped-param refusal, or `rsi_meanrev`'s
-    constructor, which takes no `name`), so a rule rebuilt from a row is named its kind -- pinned
-    by `test_every_kind_is_named_its_kind_and_stores_no_name_param` and
-    `test_add_rule_row_refuses_a_name_param_and_writes_no_row`. A hand-written row naming
-    another kind would be judged as that kind; a name the registry does not know WARNs."""
+    `rule_kind` is `positions.rule_name`, which is `rule.name`. No path that writes `rules`
+    stores a name: seed stores `describe()["params"]`, which carries no `name` for any kind, and
+    `keel rules add` and `scripts/rule_manifest.py apply` both refuse a `name` param through
+    `rules.validated_row_params` (its dropped-param refusal, or `rsi_meanrev`'s constructor,
+    which takes no `name`; #974), so a rule rebuilt from a row is named its kind -- pinned by
+    `test_every_kind_is_named_its_kind_and_stores_no_name_param`,
+    `test_add_rule_row_refuses_a_name_param_and_writes_no_row` and
+    `test_apply_refuses_a_name_param_and_writes_nothing`. Keyed off the name rather than
+    `rules.kind` through `positions.rule_id` because unattributed tranches have no `rule_id`, and
+    every caller would need a rule-id-to-kind read for the attributed rest. A hand-written SQL row
+    naming another kind would be judged as that kind; a name the registry does not know WARNs."""
     rule_cls = agent.RULE_REGISTRY.get(rule_kind)
     return rule_cls is not None and not rule_cls.places_bracket
 

keel/data/db.py

@@ -1096,7 +1096,7 @@ def _migrate_v21_positions_rule_id(conn: sqlite3.Connection) -> None:
     needed attributing: `executor.place_bracket` and the scale-out path build their
     `OrderIntent` from a POSITION rather than from a signal, so they had no `rules.id` to put on
     `orders.rule_id`. Every protective SELL was written unattributed, and
-    `payload._order_payload` rendered it as such.
+    `payload._order_row_payload` rendered it as such.
 
     That is not a cosmetic gap. Per-rule accounting reading `orders.rule_id` saw entries
     attributed and exits anonymous, which for a trend rule drops exactly the leg the outcome