From 75cd31cbfd72579bd490e16c338c61e52b8bdb43 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Thu, 13 Aug 2026 10:31:26 -0400 Subject: [PATCH] chore(release): 0.7.1, with the review follow-ups to #266 Version bump across all six distributions (`tests/test_packaging.py` fails the build if a sibling pin is left behind), plus the findings from reviewing #266 after it merged. Shipping a release with those still open would have baked a known-weak guard into a live-trading build. Follow-ups to #266: * `test_broker_strict_flags_match_mypy_strict` died with a bare `StopIteration` in exactly the case it exists to catch -- someone re-collapsing the broker block to `strict = true`. It now asserts, and the message points at the pyproject comment explaining why the block is expanded. * That test's `warn_redundant_casts` exclusion was implicit: the flag is absent from the default-vs-strict diff only because it is currently mypy's default. Were that default to flip, the test would demand the flag in a per-module section where mypy refuses to accept it -- unsatisfiable. Excluded by name. * `TradeOutcome` was spelled with the PEP 695 `type` statement. `get_type_hints` leaves such an alias as a `TypeAliasType` whose `get_origin()` is `None`, while the assignment form resolves to `Literal` -- and `commands.rules._declared_choices` validates operator-supplied `rules add --params` by testing precisely `get_origin(hint) is Literal`. No effect today (`TradeOutcome` is in no rule constructor), but it planted the spelling that silently disables that validation beside the modules whose `--params` safety depends on it. Reverted to the assignment form used by `StopMethod`/ `TargetMethod`. Three new user-visible behaviours from #266 shipped untested; all three mutations survived the whole suite. Now covered, and each verified to fail when its guard is removed: * `summarize()` rejecting a closed trade with no P&L, asserting the outcome appears in the message -- the named diagnostic is the entire point of raising. * Both `_human_dt(None)` guards in the TUI. These never raised: `time.localtime(None)` means "now", so a missing timestamp rendered the current instant as fact. Each test asserts the text says "unknown" AND does not contain the rendered current time. Not added: a test pinning `cli.avg_hold_hours`'s `exit_ts` filter. Both `SimTrade` producers set `outcome`/`exit_ts` together, so `outcome != "open"` implies `exit_ts is not None` and the filtered denominator provably cannot differ. Covering it means fabricating a state the code cannot reach, which would pin an arbitrary choice rather than a behaviour. 2723 -> 2726 tests. mypy, ruff clean. Co-Authored-By: Claude Opus 5 (1M context) --- keel/strategy/rules/base.py | 10 ++++- packages/keel-broker-api/pyproject.toml | 4 +- packages/keel-broker-coinbase/pyproject.toml | 4 +- packages/keel-broker-fake/pyproject.toml | 4 +- packages/keel-broker-robinhood/pyproject.toml | 4 +- packages/keel-core/pyproject.toml | 2 +- pyproject.toml | 8 ++-- tests/commands/test_tui.py | 38 +++++++++++++++++++ tests/strategy/test_stats.py | 16 ++++++++ tests/test_packaging.py | 22 ++++++++++- uv.lock | 12 +++--- 11 files changed, 103 insertions(+), 21 deletions(-) diff --git a/keel/strategy/rules/base.py b/keel/strategy/rules/base.py index 77f05a90..7edeb18d 100644 --- a/keel/strategy/rules/base.py +++ b/keel/strategy/rules/base.py @@ -75,7 +75,15 @@ class Signal: #: (`backtest._closed_trade`, which picks the branch) and the consumer (`Trade.outcome`) cannot #: drift: an inline `Literal[...]` repeated in both places lets one side gain a value the other #: silently rejects, and the mismatch only shows up as an `arg-type` error at the constructor. -type TradeOutcome = Literal["win", "loss", "open", "scratch"] +# +#: Spelled as a plain assignment, NOT the PEP 695 `type TradeOutcome = ...` statement, and that +#: is load-bearing rather than stylistic: `get_type_hints()` leaves a `type` alias as a +#: `TypeAliasType`, whose `get_origin()` is `None`, while the assignment form resolves through to +#: `Literal`. `commands.rules._declared_choices` validates an operator's `rules add --params` +#: by testing exactly `get_origin(hint) is Literal`, so the modern spelling would silently turn +#: that validation off for any param annotated with it. `StopMethod`/`TargetMethod` in the rule +#: modules use this same form for the same reason -- keep them consistent. +TradeOutcome = Literal["win", "loss", "open", "scratch"] @dataclass diff --git a/packages/keel-broker-api/pyproject.toml b/packages/keel-broker-api/pyproject.toml index 3458f58c..55748ef2 100644 --- a/packages/keel-broker-api/pyproject.toml +++ b/packages/keel-broker-api/pyproject.toml @@ -1,11 +1,11 @@ [project] name = "keel-broker-api" -version = "0.7.0" +version = "0.7.1" description = "Broker port, domain types, capability model, and conformance suite for keel" requires-python = ">=3.14.4" # Pinned `==`: see the note in the root `pyproject.toml`. An unpinned sibling is satisfied by # whatever is already installed, which is how a deployment ends up running mixed versions. -dependencies = ["keel-core==0.7.0"] +dependencies = ["keel-core==0.7.1"] [project.optional-dependencies] conformance = ["pytest>=9.1.1"] diff --git a/packages/keel-broker-coinbase/pyproject.toml b/packages/keel-broker-coinbase/pyproject.toml index 3905b179..60681792 100644 --- a/packages/keel-broker-coinbase/pyproject.toml +++ b/packages/keel-broker-coinbase/pyproject.toml @@ -1,11 +1,11 @@ [project] name = "keel-broker-coinbase" -version = "0.7.0" +version = "0.7.1" description = "Coinbase Advanced Trade adapter for keel" requires-python = ">=3.14.4" # Siblings pinned `==` (see the root `pyproject.toml`); the third-party SDK is not, because it # has its own release cycle and is not cut from this repo. -dependencies = ["keel-core==0.7.0", "keel-broker-api==0.7.0", "coinbase-advanced-py>=1.8.4"] +dependencies = ["keel-core==0.7.1", "keel-broker-api==0.7.1", "coinbase-advanced-py>=1.8.4"] [project.entry-points."keel.brokers"] coinbase = "keel_broker_coinbase:CoinbaseAdapter" diff --git a/packages/keel-broker-fake/pyproject.toml b/packages/keel-broker-fake/pyproject.toml index 5a8dec89..8664802d 100644 --- a/packages/keel-broker-fake/pyproject.toml +++ b/packages/keel-broker-fake/pyproject.toml @@ -1,10 +1,10 @@ [project] name = "keel-broker-fake" -version = "0.7.0" +version = "0.7.1" description = "A deliberately divergent second broker adapter, to keep the port honest" requires-python = ">=3.14.4" # Pinned `==`: see the note in the root `pyproject.toml`. -dependencies = ["keel-core==0.7.0", "keel-broker-api==0.7.0"] +dependencies = ["keel-core==0.7.1", "keel-broker-api==0.7.1"] [project.entry-points."keel.brokers"] fake = "keel_broker_fake:FakeAdapter" diff --git a/packages/keel-broker-robinhood/pyproject.toml b/packages/keel-broker-robinhood/pyproject.toml index 2ff88808..3a6d818d 100644 --- a/packages/keel-broker-robinhood/pyproject.toml +++ b/packages/keel-broker-robinhood/pyproject.toml @@ -1,13 +1,13 @@ [project] name = "keel-broker-robinhood" -version = "0.7.0" +version = "0.7.1" description = "Robinhood Crypto Trading API v2 adapter for keel" requires-python = ">=3.14.4" # `pynacl` is here and nowhere else in the workspace: Robinhood signs every request with an # Ed25519 key, which no other venue keel talks to requires. Keeping it a dependency of this # package alone means an engine that never installs this adapter never installs the crypto stack # either -- the whole point of adapters being separate distributions. -dependencies = ["keel-core==0.7.0", "keel-broker-api==0.7.0", "pynacl>=1.5.0", "requests>=2.32.0"] +dependencies = ["keel-core==0.7.1", "keel-broker-api==0.7.1", "pynacl>=1.5.0", "requests>=2.32.0"] [project.entry-points."keel.brokers"] robinhood = "keel_broker_robinhood:RobinhoodAdapter" diff --git a/packages/keel-core/pyproject.toml b/packages/keel-core/pyproject.toml index f63d337c..2ef8b942 100644 --- a/packages/keel-core/pyproject.toml +++ b/packages/keel-core/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "keel-core" -version = "0.7.0" +version = "0.7.1" description = "Shared domain types, configuration, and logging for keel" requires-python = ">=3.14.4" dependencies = ["pyyaml>=6.0.3", "python-dotenv>=1.2.2"] diff --git a/pyproject.toml b/pyproject.toml index e01f60ed..6b26ff6b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "keel-trader" -version = "0.7.0" +version = "0.7.1" description = "Offline-first, broker-agnostic, rule-based spot-trading agent (halal policy by default)" readme = "README.md" authors = [ @@ -22,11 +22,11 @@ dependencies = [ # in one build, so `==` states a fact rather than guessing at compatibility. The cost is that # a version bump must move these numbers too -- the bump already edits every package's # `version`, and `tests/test_packaging.py` fails the build if a pin is left behind. - "keel-core==0.7.0", - "keel-broker-api==0.7.0", + "keel-core==0.7.1", + "keel-broker-api==0.7.1", # keel/cli.py and keel/data/cb_client.py still import the Coinbase SDK directly; depending # on the adapter keeps it available transitively. Phase B deletes those imports and this. - "keel-broker-coinbase==0.7.0", + "keel-broker-coinbase==0.7.1", ] [project.scripts] diff --git a/tests/commands/test_tui.py b/tests/commands/test_tui.py index 36b4acfa..0f70d190 100644 --- a/tests/commands/test_tui.py +++ b/tests/commands/test_tui.py @@ -317,6 +317,23 @@ def test_available_lines_with_amount_is_ok_style_and_informative() -> None: assert "USDC" in line.text +def test_available_lines_without_updated_ts_says_unknown_rather_than_now() -> None: + """Same silent-"now" hazard as the autonomy line, on a FRESHNESS stamp. + + `updated_ts` is a separate field from `amount` and can be absent while the amount is + present. A staleness marker that quietly reports the current instant is worse than one that + admits it does not know, because an operator reads it as "just refreshed". + """ + available = AvailableBalance(Decimal("10234.5"), "USDC", None, None) + lines = _available_lines(available) + assert len(lines) == 1 + line = lines[0] + assert line.style == "ok" + assert "10,234.50" in line.text + assert "unknown" in line.text + assert _human_dt(NOW_TS) not in line.text + + def test_available_lines_with_error_is_warn_style() -> None: available = AvailableBalance(None, "USDC", NOW_TS, "no USDC balance") lines = _available_lines(available) @@ -491,6 +508,27 @@ def test_autonomy_lapsed_line_uses_human_readable_timestamp() -> None: assert _human_dt(until) in lapsed_line.text +def test_autonomy_lapsed_line_without_a_deadline_says_unknown_rather_than_now() -> None: + """A missing `autonomous_until` must not render as "lapsed this instant". + + `_human_dt` does not fail on `None` -- `time.localtime(None)` means "now" -- so without the + guard this branch printed the CURRENT time as though it were the recorded lapse. The second + assertion is the load-bearing one: absence has to read as absence, not as a fresh fact. + """ + autonomy = AutonomyStatus( + live=False, + autonomous=True, + autonomous_until=None, + updated_ts=NOW_TS, + profile_readable=True, + ) + report = _base_report(autonomy=autonomy) + lines = build_screen(report, NOW_TS) + lapsed_line = next(line for line in lines if "LAPSED" in line.text) + assert "unknown" in lapsed_line.text + assert _human_dt(NOW_TS) not in lapsed_line.text + + def test_autonomy_lapses_at_line_uses_human_readable_timestamp() -> None: until = NOW_TS + 3600 autonomy = AutonomyStatus( diff --git a/tests/strategy/test_stats.py b/tests/strategy/test_stats.py index 3cbabe7a..1e740a65 100644 --- a/tests/strategy/test_stats.py +++ b/tests/strategy/test_stats.py @@ -11,6 +11,8 @@ from decimal import Decimal +import pytest + from keel.strategy.rules.base import Trade from keel.strategy.stats import BacktestResult, summarize from keel.types import Side @@ -134,3 +136,17 @@ def test_summarize_open_trade_included_in_trades_but_excluded_from_aggregates() assert result.trades == [closed, still_open] assert result.n_trades == 1 + + +def test_summarize_rejects_a_closed_trade_carrying_no_pnl() -> None: + """A closed trade with no realised P&L is a broken input, and must SAY so. + + Only an open trade may omit `pnl`; every close path sets it. The value of raising here is + the diagnostic: unguarded, this surfaced as `TypeError: unsupported operand type(s) for +: + 'Decimal' and 'NoneType'` raised from inside a generator, with no way to tell which trade + caused it. The outcome is asserted in the message for exactly that reason. + """ + with pytest.raises(ValueError, match="pnl=None") as excinfo: + summarize([_trade("win", pnl=None)]) + + assert "outcome='win'" in str(excinfo.value) diff --git a/tests/test_packaging.py b/tests/test_packaging.py index 24c7eec4..dc16b899 100644 --- a/tests/test_packaging.py +++ b/tests/test_packaging.py @@ -178,8 +178,28 @@ def options(extra: list[str]): # `--strict` CLEARS implicit_reexport; the config states that as `no_implicit_reexport`. expected.add(name if value else f"no_{name}") + # Dropped explicitly, not by accident: `warn_redundant_casts` is part of the bundle but is + # global-only, so it lives in `[tool.mypy]`. It happens not to appear in the diff above + # because it is already mypy's default -- if that default ever flips it WOULD appear, and + # this test would then demand it in a per-module section where mypy refuses to accept it, + # leaving the config unsatisfiable. Excluding it by name keeps that impossible. + expected.discard("warn_redundant_casts") + broker_override = next( - o for o in _mypy_overrides() if o.get(_STRICT_MARKER) and "keel_broker_api.*" in o["module"] + ( + o + for o in _mypy_overrides() + if o.get(_STRICT_MARKER) and "keel_broker_api.*" in o["module"] + ), + None, + ) + # Asserted rather than left to `next()`: re-collapsing the block to `strict = true` is THE + # regression this test exists to catch, and a bare `StopIteration` from an exhausted + # generator is the least legible way pytest can report it. + assert broker_override is not None, ( + f"no broker override sets {_STRICT_MARKER!r} -- if the block was collapsed back to " + "`strict = true`, that re-enables strict mode globally for every module (see the " + "comment above the block in pyproject.toml); expand it into its flags again" ) configured = {k for k, v in broker_override.items() if k != "module" and v is True} diff --git a/uv.lock b/uv.lock index 75543790..01e45ebe 100644 --- a/uv.lock +++ b/uv.lock @@ -345,7 +345,7 @@ wheels = [ [[package]] name = "keel-broker-api" -version = "0.7.0" +version = "0.7.1" source = { editable = "packages/keel-broker-api" } dependencies = [ { name = "keel-core" }, @@ -365,7 +365,7 @@ provides-extras = ["conformance"] [[package]] name = "keel-broker-coinbase" -version = "0.7.0" +version = "0.7.1" source = { editable = "packages/keel-broker-coinbase" } dependencies = [ { name = "coinbase-advanced-py" }, @@ -382,7 +382,7 @@ requires-dist = [ [[package]] name = "keel-broker-fake" -version = "0.7.0" +version = "0.7.1" source = { editable = "packages/keel-broker-fake" } dependencies = [ { name = "keel-broker-api" }, @@ -397,7 +397,7 @@ requires-dist = [ [[package]] name = "keel-broker-robinhood" -version = "0.7.0" +version = "0.7.1" source = { editable = "packages/keel-broker-robinhood" } dependencies = [ { name = "keel-broker-api" }, @@ -416,7 +416,7 @@ requires-dist = [ [[package]] name = "keel-core" -version = "0.7.0" +version = "0.7.1" source = { editable = "packages/keel-core" } dependencies = [ { name = "python-dotenv" }, @@ -431,7 +431,7 @@ requires-dist = [ [[package]] name = "keel-trader" -version = "0.7.0" +version = "0.7.1" source = { editable = "." } dependencies = [ { name = "click" },