From 0effbdc100ed0f600a31e0a2d1e550703fdfb2b4 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Fri, 28 Aug 2026 20:11:11 -0400 Subject: [PATCH 1/9] =?UTF-8?q?docs(research):=20the=20evidence=20toolkit?= =?UTF-8?q?=20page=20=E2=80=94=20what=20it=20answers,=20what=20it=20refuse?= =?UTF-8?q?s=20to=20(#601)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Thirteen modules under keel/research/ do deeper work than the two feature cards Jesse markets against, and until now none of it had a reader-facing home: the code was discoverable only by reading source or the scattered docs/experiments/ drivers. This adds docs/research-toolkit.md, the docs page issue #601 asks for — a table and a section per module stating what it answers, and, written with the same care, what it refuses to answer even when asked nicely. That refusal column is the actual point: significance.py's own docstring says a tool that cannot say no is a flattery tool, and this page exists to make a reader able to tell keel's honesty apart from a competitor's marketing copy. Three things get more than a table row because a reader meets them as mechanism, not as a feature list. First, montecarlo.py's invariant that Jesse's copy never states: a permutation of a multiset always sums to the same number, so a reshuffled path's final-equity percentile is exactly 1/2 by construction, before a single path is drawn — trade-order reshuffling can only ever speak to the path's shape (drawdown), never to whether the ending was luck. Second, significance.py's fee-and-herding discipline: two fee regimes priced separately and never averaged, and a pooled n divided by throughput.py's design effect (measured ICC 0.212 over ~8-asset-a-day episodes) before any standard error is formed, so a pooled 100 is ~39 effective observations. A real refusal transcript is included, generated by actually running significance() against an honestly underpowered 12-trade sample rather than invented for the page. Third, the Strathern rail itself — cscv.py, deflate.py and walkforward.py's shared ⛔ comment that a score may report or gate but must never become a sweep's ranking key — named where a reader will actually meet it (each module's own section), with the tests that pin it by name: test_cscv.py::test_result_exposes_no_configuration_field, the two source/field scans in test_walkforward.py, and the AST scan the front door itself now carries, test_research_front_door.py::test_research_module_never_sorts_ranks_or_maxes. The command surface documented here matches what keel/commands/research.py actually wires as of this commit, not the fuller plan in the design contract: only `keel research index` plus five aliases (pbo/deflate/monte-carlo/walk-forward/lookahead) exist. The six modules the contract called for as new subcommands (significance, pooled-review, throughput, tuning, factors, independence) are documented as run today via `keel research index --module NAME`, which names the pre-registered docs/experiments/ driver that exercises each — verified by invoking that command for all six rather than assumed. A page that cites a command that does not run is the exact failure this PR exists to avoid; if the six subcommands land before this ships, the six "run it with" cells and their footnote are the only edit this page will need. README.md gains one line in the documentation map, in the section's existing one-per-doc style, pointing at the new page. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01T6yA5khYnJ2qzheArRToQ2 --- README.md | 4 + docs/research-toolkit.md | 264 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 268 insertions(+) create mode 100644 docs/research-toolkit.md diff --git a/README.md b/README.md index 3909bd22..d6d10329 100644 --- a/README.md +++ b/README.md @@ -174,6 +174,10 @@ adapter, deliberately divergent, that the conformance suite runs against. - [`docs/experiments/`](docs/experiments) — the experiment record, including the honest result linked above; every document states what was measured, on what engine, with the defect that forced a restatement. +- [`docs/research-toolkit.md`](docs/research-toolkit.md) — the thirteen `keel/research/` + modules behind `keel research`: what each answers, what it refuses to answer even when + asked nicely, and the Strathern rail that stops a diagnostic score from becoming a + sweep's ranking key. - [`docs/launch.md`](docs/launch.md) — the pre-launch gate and the announcement plan: what must be true before anything is announced, where, in what order, and what the post says (the honest result included). diff --git a/docs/research-toolkit.md b/docs/research-toolkit.md new file mode 100644 index 00000000..97ac8a3e --- /dev/null +++ b/docs/research-toolkit.md @@ -0,0 +1,264 @@ +# The keel research toolkit — thirteen modules, one front door, and what none of them will say + +`keel/research/` is thirteen modules that measure whether a trading rule's edge is real, whether +a sweep that found it was overfit, whether an equity curve's shape was luck, and whether the +evidence for any of that is even large enough to trust. Jesse markets two of these as feature +cards — "Rule Significance Testing" and "Monte Carlo Analysis." keel ships both, plus eleven more +Jesse doesn't have, and the honest answer more often than not is *no* — no distinguishable edge, +no candidate worth proposing, no measurement large enough to say anything. That is not a defect +in the toolkit. It is the toolkit working, and `significance.py`'s own docstring states the rule +that governs everything below it: + +> a significance tool here must be able to say "not distinguishable from zero" and mean it. A +> tool that cannot say no is a flattery tool. +> — `keel/research/significance.py:8` + +`keel research` is the front door onto these thirteen modules. `keel research index` names all +thirteen — what each answers, what each cannot answer, and the command (or, for a module with no +subcommand of its own yet, the pre-registered `docs/experiments/` driver) that gets you the +number. Five of the thirteen already had a home under `keel trials`/`keel rules` +(`pbo`/`deflate`/`monte-carlo`/`walk-forward`/`lookahead`) and are registered a second time under +`research`, the same click command objects rather than a second implementation. It adds no +statistics of its own; every number on this page and every number the CLI prints comes out of +`keel/research/*`, unchanged. What follows is what each module answers, what it refuses to answer +even when asked nicely, and the command that runs it. Read the "cannot answer" column as +carefully as the "answers" column — that column is the actual product. + +## The thirteen, at a glance + +| module | answers | cannot answer | run it with | +| :--- | :--- | :--- | :--- | +| `significance.py` | is a family's edge distinguishable from its break-even, priced at the fee actually paid | whether the edge will hold going forward, or which family/regime to report — you name both | `keel research index --module significance`\* | +| `montecarlo.py` | was the equity curve's *path* (drawdown, time underwater) unusual for this set of trades, in some order or under resampled price history | whether the *final equity* was luck — that percentile is exactly 1/2 by construction, always | `keel research monte-carlo` (`keel trials monte-carlo`) | +| `cscv.py` | the probability a configuration selected in-sample degrades out-of-sample, over a matrix of configurations already tried | which configuration is the best one — it never returns that, by construction | `keel research pbo` (`keel trials pbo`) | +| `deflate.py` | given N trials tried, the Sharpe bar the winner had to clear, and how much data that needs | what N and correlation to assume — it reports a band across assumptions rather than guess one | `keel research deflate` (`keel trials deflate`) | +| `walkforward.py` | does a GIVEN fixed parameter set hold up across rolling train/test windows, and does it degrade | which parameter set, fold or window is best — none is ever computed | `keel research walk-forward` (`keel trials walk-forward`) | +| `independence.py` | how much two rules' (or two horizons') signals overlap in time, position and P&L | whether either rule is profitable, or which one to keep | `keel research index --module independence`\* | +| `throughput.py` | how much volume a fee-free allowance can honestly carry this month, and how long evidence takes to accumulate | it never enlarges an allowance to fit a plan — a product that doesn't fit is deferred, not squeezed in | `keel research index --module throughput`\* | +| `cts_factors.py` | do the 11 CTS confluence factors carry independent evidence, or is one momentum read counted three times | the biased ("obvious") conditional sample is computed but never allowed to carry the headline | `keel research index --module cts_factors`\* | +| `tuning.py` | for a declared parameter space, does a train/held-out study produce a candidate clearing held-out sign AND PBO ≤ 0.5 | it never auto-tunes a live/paper profile, and a pass is a hypothesis, not a promotion | `keel research index --module tuning`\* | +| `bias.py` | does a rule's decision at bar N change when bars after N become visible (lookahead / recursive drift) | whether the rule is profitable — this is about information leakage only | `keel research lookahead` (`keel rules lookahead`) | +| `ledger.py` | what experiments were run, in what order, tamper-evidently | it is tamper-*evident*, not tamper-*proof*, and it never touches money | `keel trials record` / `list` / `verify` (no `research` alias — see below) | +| `matrix.py` | assembles the T×N matrix `cscv.py` needs from ledger trials, enforcing the "true matrix" condition | anything about performance itself — it is plumbing, not a question of its own | no direct command — runs inside `keel research pbo` | +| `pooled_review.py` | the #427 pooled-review machinery: descriptive n_eff-corrected intervals, never a verdict on the edge | it renders no pass/fail on the edge, ever — see [the 2026-09-30 review](#the-2026-09-30-pooled-review-427) below | `keel research index --module pooled_review`\* | + +\* These six modules have no dedicated `keel research` subcommand of their own yet — only +`keel research index`, which names every module, and the five aliases above, are wired as of +this writing. Asking the index for one module by name (`--module NAME`, the bare filename minus +`.py`) prints its `runs as` line, which today names the pre-registered `docs/experiments/` +driver that exercises it — e.g. `keel research index --module significance` names +`docs/experiments/2026-08-21-rule-family-significance.py`. Until each of these six gets its own +subcommand, that driver (or a direct `import keel.research.` call, as this page does +below) is how you actually run one. + +`ledger.py` and `matrix.py` are the two modules that were never going to get a `keel research` +alias in the first place: `ledger.py`'s record-keeping commands (`record`/`list`/`verify`) stay +under `keel trials`, where they have lived since before this front door existed, and `matrix.py` +has never had a command of its own — it is the assembly step behind `keel trials pbo`/ +`keel research pbo`, not a question a reader asks directly. The index still names both; the +command surface just doesn't duplicate what already works. + +## The two Jesse markets, and what keel adds to each + +### `significance.py` — is the edge real, at the price you actually pay? + +The question is a one-proportion test against break-even, with the null set by the fee *actually +paid*: `keel/research/significance.py` prices the same reconstructed trades at both fee regimes +a keel deployment can be in — the 120 bp taker fee outside the venue's fee-free allowance, and +zero inside it — and never averages the two, because the cross-verification behind #475 found the +fee difference *is* the result (decisively negative outside, indistinguishable from break-even +inside). It also refuses to pool trades as if they were independent: signals fire in herds (about +eight assets the same UTC day, ICC 0.212), so `n_eff` divides the pooled count by +`throughput.design_effect()` before any standard error is formed — a pooled 100 comes out to +roughly 39 effective observations, not 100. + +What it cannot answer: whether the edge will hold going forward (it is a test against a fixed +historical sample, not a forecast), and it will not pick which family or fee regime to headline — +every subcommand run names both explicitly. It also will not manufacture power a sample doesn't +have: an underpowered result is reported as "not distinguishable from zero," not massaged into +significance by choosing a friendlier n. + +Run: no dedicated subcommand yet — `keel research index --module significance` names the +pre-registered driver, `docs/experiments/2026-08-21-rule-family-significance.py`; the transcript +below calls the module directly. + +### `montecarlo.py` — trade reshuffling and candle bootstrap, and the invariant Jesse's marketing skips + +Two nulls: `reshuffle` (the same closed trades, in different orders) and +`moving_block_bootstrap` (consecutive blocks of real candles resampled with wrap-around, then +re-backtested). Both are report-only, deterministic under an explicit seed, and neither scores or +gates anything. + +The invariant `montecarlo.py` names and Jesse's copy doesn't: a permutation of a multiset sums to +the same number. Reshuffle the same trades into any order you like and every path ends at the +same final equity — + +> so every reshuffled path ends at the observed final equity and THAT percentile reads exactly +> 1/2 (ties count half). +> — `keel/research/montecarlo.py:12-13` + +That is not a weak result; it is mathematically guaranteed to be 1/2 before a single path is +drawn. Trade-order reshuffling therefore cannot tell you whether the final equity was luck — the +question it *can* answer lives in the shape of the path between start and end, which is why the +module reports `max_drawdown` as its headline statistic and keeps the final-equity lines in the +output rather than hiding a number that always reads the same. The candle bootstrap is the +module's honest answer to "what if reshuffling isn't enough" — it preserves local +autocorrelation a naive resample would destroy, at a stated cost: block stitching creates a price +discontinuity at each seam the real series never had. + +Run: `keel research monte-carlo` — an alias of the existing `keel trials monte-carlo`, same +command object, registered a second time. + +## The Strathern rail: `cscv.py`, `deflate.py`, `walkforward.py` + +Three modules exist because trying many configurations and reporting the best one lies to you +about how good that configuration really is — that's overfitting, and PBO/CSCV, the Deflated +Sharpe family, and walk-forward validation each measure a different piece of it. All three carry +the same rail, named after Marilyn Strathern's observation that `cscv.py` quotes directly: + +> PBO may gate or report; it may never be a sweep's ranking key, because "when a measure becomes +> a target, it ceases to be a good measure" (§78.7). +> — `keel/research/cscv.py:8-9` + +The mechanism is specific, not a vibe. A diagnostic score is allowed to *report* ("PBO is 0.62") +and allowed to *gate* (a proposal study can require `pbo <= 0.5` before it may even suggest a +candidate — `tuning.py`'s `OverfittingGate` does exactly this). What it may never do is become the +thing a sweep sorts, maxes, or picks a winner by — because the moment a score is optimized against, +the people running the sweep start (consciously or not) selecting for configurations that game the +score rather than configurations that are actually good, and the score stops measuring what it was +built to measure. + +**`cscv.py`** enforces it at the return type: `PBOResult` "carries probabilities and slopes +only," and `tests/research/test_cscv.py::test_result_exposes_no_configuration_field` asserts the +dataclass's own field names never include `best_config`, `best_column`, `argmax`, `selected`, +`winner`, or half a dozen synonyms — a mutation that adds any of them to `PBOResult` fails this +test immediately. The CLI carries the same discipline one layer up: +`tests/research/test_trials_cli.py::test_pbo_command_reports_but_never_names_a_winner` runs +`keel trials pbo` (the command `keel research pbo` aliases) against a ledger seeded with six +`entry_lookback` values and asserts none of the candidate values — `entry=20` through `entry=45` +— ever appears in the printed output. + +**`walkforward.py`** states the same rail for its own question — "a walk-forward validator that +reported a 'winning window' … would reintroduce exactly the selection-over-configurations the +rail exists to forbid" — and is pinned two ways in `tests/research/test_walkforward.py`: +`test_refusal_to_rank_enforced_by_source_scan` reads the module's own source text and asserts it +contains no keyed sort (`key=lambda`), no in-place `.sort(`, and none of the words "best", +"winner" or "optimal" anywhere in the file — not just in the rendered output, in the source +itself — and `test_report_dataclass_has_no_selection_fields` asserts `WalkForwardReport` carries +no field starting with `best` and none containing `select` or `chosen`. + +**`deflate.py`** states the rail in its own header — "Reporting only — ⛔ per §78.7's Strathern +rail none of these may ever be a sweep's ranking key" — but has no equivalent structural pin: its +public functions return plain floats (`SR_0`, `DSR`, `MinBTL`), not a dataclass that could grow a +configuration-bearing field, so there is nothing for a source or field scan to catch. The +guarantee here is architectural rather than test-pinned — there is no return value shaped like a +winner to leak. + +The front door inherits the same discipline, and it too is pinned mechanically rather than +promised: `tests/commands/test_research_front_door.py::test_research_module_never_sorts_ranks_or_maxes` +is an AST scan over `keel/commands/research.py` itself, and it fails on any `sorted()`/`max()`/ +`min()` call with a `key=` argument, any `.sort(key=...)`, or an import of `heapq` or +`operator.itemgetter`/`attrgetter` — anywhere in the module, whether or not the value being +ranked looks rail-bearing. The blanket ban is deliberate: a scanner that only objects when the +sorted field *looks* like `pbo` or `dsr` is a scanner a rename defeats; the front door's job is +to place values in an order it was given, never an order a score chooses for itself, so the rule +has no exception clause. `test_rail_marked_on_exactly_the_three_strathern_modules` pins the other +half — that `cscv.py`, `deflate.py` and `walkforward.py` are exactly the three modules the +index marks as rail-bearing, no more and no fewer. + +## The refusal, as a first-class result + +Every evidence subcommand in `keel research` can print a refusal on stdout and exit 0. A +well-formed question the evidence cannot answer is the tool working, not the tool failing — the +alternative is a tool that always finds *something* to say, which is another name for a flattery +tool. This is not new behaviour invented for the front door: `keel trials deflate` already prints + +``` +refused: only 1 decision trial(s) recorded -- need >= 2 to form a trial count for +E[max SR_n]/MinBTL; record more trials with `trials record` and retry +``` + +and returns exit code 0 (`keel/commands/trials.py:169-172`, the comment beside it: "A refusal is +this command working, not this command failing — print it on stdout and exit 0"). + +Here is `significance.py` refusing an honestly underpowered sample, run for real against the +library rather than invented for this page — twelve closed trades (seven wins, five losses) at +the 120 bp taker fee: + +``` +turtle_breakout @ outside_allowance_taker (fee 120 bp per leg): + closed trades n=12 pooled -> 4.66 effective (design effect 2.575, #427) + payoff b=1.2500 -> break-even win rate 0.4444; observed 0.5833 -> edge 0.1389 points + edge z=0.6034, one-sided p=0.2731; 95% one-sided lower bound on the edge: -0.2397 + smallest edge detectable at this n_eff: 0.5761 (80% power, alpha 5%) + verdict: not distinguishable from zero at the 120 bp taker fee +``` + +An observed edge of 13.9 points looks encouraging until the n_eff correction is applied: twelve +pooled trades collapse to 4.66 effective observations (design effect 2.575 — herding again), the +95% one-sided lower bound on the edge is *negative* (-0.2397), and the sample could only detect an +edge of 57.6 points or larger at 80% power in the first place. `render_family` prints all of that +rather than the bare verdict, so the reader sees exactly why the answer is no, and the command +still exits 0 — the question was well-formed and the evidence answered it honestly. + +## What is NOT here + +This front door is surfacing, not new statistics. `keel research` assembles inputs, calls a +function in `keel/research/*`, and prints what comes back — the command layer is not permitted to +compute anything the module doesn't already compute, and every number this page or the CLI prints +traces back to one of the thirteen files above. + +It is also not where a bespoke, one-off sweep belongs. keel's ad-hoc pre-registered research +drivers — parameter sweeps, ablations, factor studies, feasibility probes, each with a `.py` +driver committed beside the document that narrates it — live in `docs/experiments/`, not here. +`keel research` is the toolkit; `docs/experiments/` is the record of what the toolkit (and +sometimes a purpose-built script sitting on top of it) found when someone asked it a specific +question on a specific date. See [`docs/experiments/README.md`](experiments/README.md) for the +index and for why the drivers are committed rather than run-and-discarded. + +## The 2026-09-30 pooled review (#427) + +`docs/experiments/2026-09-30-pooled-review.py` is a standing, pre-registered event: at midnight +UTC on 2026-09-30, every deployment's forward trades (paper, live, and the paper-hourly evidence +profile) are pooled and reviewed. The pre-registration is the driver's own module docstring, not +a separate document — "PRE-REGISTERED BEFORE THE EVENT (this docstring is the pre-registration…)" +— and it is frozen: the pool definition, the dedup rule, the exclusions, all fixed before the +first forward trade closes. + +It exists because #359 originally scheduled the review at a floor of n=100 pooled trades, and +#427 found that floor **dishonest as written**. Signals fire in herds — roughly eight assets the +same UTC day, ICC 0.212 — so 100 pooled trades are not 100 independent observations; they are +about 39 effective ones (the same `design_effect()`/`n_eff` correction `significance.py` and +`throughput.py` apply everywhere else). At 39 effective observations the review can only detect +an edge of about 20 points or larger. A pass/fail verdict written against a floor that ignores +that correction would be reporting confidence the sample doesn't have. + +The correction of record — PR #503, and the corrected comment on discussion #359 — reframed the +event as **descriptive**: `keel/research/pooled_review.py` runs `significance.py`'s n_eff-corrected +math over the pooled forward trades and always prints the sentence #427 requires, generated at +the same n the measurement itself uses so the artifact never carries two different n_eff numbers +side by side: + +> "at this n_eff (N effective of M pooled), this review can only see an edge of X points or +> larger (80% power, one-sided 5%)" + +There is no pass/fail verdict on the edge anywhere in the rendered report — the only +verdict-shaped sentence in it is about power, not about whether the edge is real. A pool with +nothing counted (empty, or every trip a scratch) refuses outright rather than render a degenerate +report: `is_refused`/`descriptive_review` in `keel/research/pooled_review.py` produce a +`DescriptiveReview` whose `refusal` is a tuple of reasons instead of a report, and +`render_report` raises if asked to render one anyway — a refused review has no report to print. + +That refusal is exactly where the standalone driver is the prior art any future +`keel research pooled-review` subcommand must deliberately break with, not the pattern to copy: +`docs/experiments/2026-09-30-pooled-review.py` prints its refusal to **stderr** and calls +`sys.exit(2)` when a pre-registered profile database isn't reachable. A front-door command must +not repeat that — under the rule stated above, a refusal belongs on stdout at exit 0, because +"nothing to review" is this command answering a well-formed question honestly, not the command +failing to run. + +As of this writing `pooled_review.py` has no dedicated `keel research` subcommand either — +`keel research index --module pooled_review` names the same driver, and running the review +today still means running `docs/experiments/2026-09-30-pooled-review.py` directly, stderr/exit-2 +refusal included. Before 2026-09-30 it runs the same machinery as a labelled preview, and it +says so. From 2c6fd33da8ee21543af441ce17c95459ecc40ed6 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Fri, 28 Aug 2026 20:12:58 -0400 Subject: [PATCH 2/9] fix(trials): a refusal is a result, not an error (#601) keel trials deflate, pbo, monte-carlo and walk-forward each had at least one path where the ledger was readable, the request was well-formed, and the evidence simply could not answer it -- too few decision trials to bound MinBTL, a matrix with no usable columns because every trial is series_missing, an observed backtest that closed nothing to reshuffle, a walk-forward window that does not fit the cached candle series. All four reported that as click.ClickException: exit 1, on stderr, the same shape as an operator typo. That conflates two different things. An operator error means the request was wrong (a rule id that does not exist, a ledger path that cannot be read); a refusal means the request was right and the evidence cannot answer it. #601's second bullet asks for every subcommand to be able to print the second kind as a RESULT -- stdout, exit 0 -- because the whole discipline this package is built on, stated outright in significance.py's own docstring, is that a tool which cannot say no is a flattery tool. These four ClickExceptions were the one place `keel trials` was not living up to its own module docstring yet. Left alone, deliberately: resolve_rule_backtest's RulesRefused (unknown rule id, no candles cached -- "fetch first" is an operator's next command, not evidence), a poisoned closed-trade row with no pnl (a data integrity error), a broken ledger hash chain in `trials verify` (tampering, not insufficient evidence), and `rules lookahead`'s exit-1 on an ACTUAL lookahead-detected verdict -- that one is a definitive finding, the opposite of a refusal, and stays loud on purpose (its own docstring calls it out: "fails loud, like keel doctor"). Existing tests asserted the old exit-1/stderr shape and are updated here to assert exit 0 with "refused" on stdout instead: the four in tests/research/test_trials_cli.py (pbo-all-series-missing, deflate-too- few-trials, and both monte-carlo no-closed-trades cases) and one in tests/research/test_walkforward.py (the too-big-window case). The monte-carlo candles-mode "no candles cached" refusal and the paths-cap test were left untouched -- both are operator-actionable, not evidence-shaped, and keep their ClickException. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01T6yA5khYnJ2qzheArRToQ2 --- keel/commands/trials.py | 34 ++++++++++++++++++++++++------ tests/research/test_trials_cli.py | 19 ++++++++++++----- tests/research/test_walkforward.py | 6 ++++-- 3 files changed, 46 insertions(+), 13 deletions(-) diff --git a/keel/commands/trials.py b/keel/commands/trials.py index a8cb88b3..3e778f81 100644 --- a/keel/commands/trials.py +++ b/keel/commands/trials.py @@ -166,7 +166,16 @@ def trials_deflate( trials = trials_ledger.read_trials(_ledger_path(ledger)) m_total, n_decisions = trials_ledger.trial_counts(trials) if n_decisions < 2: - raise click.ClickException(f"only {n_decisions} decision trials -- need >= 2") + # Evidence-shaped, not an operator error (#601): the ledger is readable and the + # question is well-formed, there is simply not yet enough recorded evidence to + # form a trial count for E[max SR_n]/MinBTL. A refusal is this command working, + # not this command failing -- print it on stdout and exit 0. + click.echo( + f"refused: only {n_decisions} decision trial(s) recorded -- need >= 2 to form " + "a trial count for E[max SR_n]/MinBTL; record more trials with `trials record` " + "and retry" + ) + return click.echo("inputs") click.echo(f" M (all ledger rows) : {m_total}") @@ -221,7 +230,13 @@ def trials_pbo(ledger: Path | None, session: str | None, blocks: int) -> None: trials = trials_ledger.read_trials(_ledger_path(ledger)) build = matrix_mod.build_matrix(trials, session=session) if not build.columns: - raise click.ClickException("no usable columns (all trials are series_missing?)") + # Evidence-shaped (#601): a readable, well-formed ledger with nothing to build a + # CSCV matrix from is a refusal, not an operator mistake -- print it and exit 0. + click.echo( + "refused: no usable columns -- every recorded trial is series_missing, so " + "there is no per-bar P&L to assemble a CSCV matrix from" + ) + return for warning in build.warnings: click.echo(f"warning: {warning}", err=True) if build.refused: @@ -369,9 +384,11 @@ def trials_monte_carlo( if mode == "trades": if not pnls: - raise click.ClickException( - "no closed trades in the observed backtest -- nothing to reshuffle" - ) + # Evidence-shaped (#601): the rule resolved and the observed backtest ran; it + # simply closed nothing to resample. Print the refusal and stop -- exit 0, no + # ledger row, because there is nothing to diagnostic_only-record either. + click.echo("refused: no closed trades in the observed backtest -- nothing to reshuffle") + return resampled = mc_mod.reshuffle(pnls, paths, seed) else: if not resolved.candles: @@ -563,7 +580,12 @@ def trials_walk_forward( fee_pct=resolved.fee_pct, ) except ValueError as exc: - raise click.ClickException(str(exc)) from exc + # Evidence-shaped (#601): every ValueError this pair raises names a train/test + # window that does not fit the given candle series -- a well-formed request the + # cached history cannot answer, not an operator mistake. Print it and stop; no + # ledger rows, exit 0. + click.echo(f"refused: {exc}") + return for line in wf_mod.render_lines(report): click.echo(line) diff --git a/tests/research/test_trials_cli.py b/tests/research/test_trials_cli.py index 3bbd16a8..2c880013 100644 --- a/tests/research/test_trials_cli.py +++ b/tests/research/test_trials_cli.py @@ -130,7 +130,9 @@ def test_pbo_refuses_when_every_trial_is_series_missing(tmp_path): _record(runner, path, "backfilled") result = runner.invoke(cli, ["trials", "pbo", "--ledger", str(path), "--blocks", "4"]) - assert result.exit_code != 0 + # #601: an evidence-shaped refusal is a result, not an error -- exit 0, on stdout. + assert result.exit_code == 0, result.output + assert "refused" in result.output assert "no usable columns" in result.output @@ -175,7 +177,10 @@ def test_deflate_refuses_on_too_few_decision_trials(tmp_path): result = runner.invoke( cli, ["trials", "deflate", "--ledger", str(path), "--sharpe", "0.4"] ) - assert result.exit_code != 0 + # #601: an evidence-shaped refusal is a result, not an error -- exit 0, on stdout. + assert result.exit_code == 0, result.output + assert "refused" in result.output + assert "need >= 2" in result.output # -- trials monte-carlo (#441): is the equity curve an outlier? ----------------------------------- @@ -412,8 +417,10 @@ def test_monte_carlo_refuses_when_no_closed_trades_and_writes_no_row(tmp_path): result = _invoke_mc(CliRunner(), db, ledger, "--seed", "3") # The mechanism: dca declares no granularity, the fallback chain resolves ONE_HOUR, and # nothing is cached there -- so the observed backtest runs over ZERO candles and closes - # nothing. A refusal, not a degenerate row. - assert result.exit_code != 0 + # nothing. A refusal, not a degenerate row -- and per #601 a refusal is a printed + # result, exit 0, not a ClickException. + assert result.exit_code == 0, result.output + assert "refused" in result.output assert "no closed trades" in result.output assert not ledger.exists() @@ -465,7 +472,9 @@ def test_monte_carlo_refuses_when_every_trade_is_genuinely_open(tmp_path): ledger = tmp_path / "trials.jsonl" result = _invoke_mc(CliRunner(), tmp_path / "open-only.db", ledger, "--seed", "3") - assert result.exit_code != 0 + # #601: an evidence-shaped refusal is a result, not an error -- exit 0, on stdout. + assert result.exit_code == 0, result.output + assert "refused" in result.output assert "no closed trades" in result.output assert not ledger.exists() diff --git a/tests/research/test_walkforward.py b/tests/research/test_walkforward.py index 2a1d46a2..67f95dc6 100644 --- a/tests/research/test_walkforward.py +++ b/tests/research/test_walkforward.py @@ -744,9 +744,11 @@ def test_cli_refusals_write_no_rows(tmp_path): db = _wf_db(tmp_path, candles=True) ledger = tmp_path / "trials.jsonl" - # window larger than the cached series: ValueError -> ClickException, no rows. + # window larger than the cached series: evidence-shaped ValueError from `folds()`, no + # rows -- and per #601 a refusal is a printed result (exit 0), not a ClickException. too_big = _invoke_wf(CliRunner(), db, ledger, "--train-bars", "90", "--test-bars", "90") - assert too_big.exit_code != 0 + assert too_big.exit_code == 0, too_big.output + assert "refused" in too_big.output assert "exceeds" in too_big.output assert not ledger.exists() or trials_ledger.read_trials(ledger) == [] From b130d00e1b6a53f17a9dc2adb4fc87d5fe1eb2d5 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Fri, 28 Aug 2026 20:13:22 -0400 Subject: [PATCH 3/9] feat(research): keel research, one front door over keel/research/* (#601) Thirteen modules on main measure whether a rule's edge is real, whether a sweep that found it was overfit, whether an equity curve's shape was luck, and whether the evidence for any of that is even large enough to trust -- and none of them had one place a reader could find them all. Half already had a CLI (keel trials fronts ledger/cscv+matrix/deflate/ montecarlo/walkforward; keel rules lookahead fronts bias), so the actual gap was never "no code path", it was "no index" -- the tools were discoverable only by reading keel/commands/trials.py, keel/commands/ rules.py and 30+ ad-hoc drivers under docs/experiments/*.py. keel research index is that index: for every module it states the question it answers, what it CANNOT answer, and the command (or, for the six modules still waiting on their own subcommand -- a later wave of #601 -- the pre-registered docs/experiments/ driver that runs it today). Every "cannot answer" line was written from the module's own docstring and dataclass shape, not invented: montecarlo's reshuffle percentile is exactly 1/2 by construction (a permutation of a multiset sums to the same number), so it names the path shape as what actually carries information; walkforward and cscv say plainly that no fold, window or configuration is ever returned as a winner. The five commands that already exist -- trials pbo/deflate/monte-carlo/ walk-forward and rules lookahead -- are registered a SECOND time under this group via Group.add_command, the same click Command objects, never copies: keel research pbo and keel trials pbo run the identical code. A front door that reimplemented would be a front door that drifts the moment one copy got a bugfix the other didn't. Two pins in tests/commands/test_research_front_door.py keep this honest as keel/research/ grows: - The completeness pin globs keel/research/*.py and fails if any module has no RESEARCH_INDEX row, and separately walks the real click command tree off keel.cli.cli (or checks the filesystem for a docs/experiments driver) so a runs_as string that used to work but silently broke also fails the build. Mutation-verified twice: adding a throwaway keel/research/zzz_probe.py made test_every_research_module_is_indexed fail with `AssertionError: {'zzz_probe.py'} exist in keel/research/ but have no RESEARCH_INDEX row` (file added, test run, file removed); pointing montecarlo.py's runs_as at "keel research not-a-real-command" made test_every_runs_as_resolves fail with `AssertionError: keel research not-a-real-command does not resolve to a registered CLI command (module=montecarlo.py)` (edit made, test run, edit reverted). - The Strathern rail pin is an AST scan over keel/commands/research.py itself, banning sorted()/max()/min() called with key=, .sort(key=...), and any import of heapq/operator.itemgetter/attrgetter -- unconditional, not scoped to fields that "look" rail-bearing, because a field-aware scanner is a scanner a rename can fool and the front door's only legitimate ordering need (its own table of contents) is served by the explicitly declared literal tuple order RESEARCH_INDEX already uses. Mutation-verified: inserting `sorted(entries, key=lambda r: r.module)` made test_research_module_never_sorts_ranks_or_maxes fail with `AssertionError: sorted() called with key= at keel/commands/research.py:287 -- a keyed sort/max/min IS a ranking; the Strathern rail forbids it here unconditionally` (inserted, test run, removed). Also asserted: rail=True marks exactly cscv.py/deflate.py/walkforward.py, the rendered index states the rail sentence right beside cscv.py's own block (not floating loose in a preamble), --json round-trips one row per module, --module narrows to one entry, an unknown --module name prints its own refusal and exits 0 (the index's lookup obeys the same "a well-formed question the evidence -- here, the index -- cannot answer is a result, not an error" discipline #601 asks of every other subcommand), and every alias is the same object under both names, both by identity and by matching --help output body. keel/cli.py registers the group after both rules_group and trials_group so the objects it aliases already exist, with a banner comment in the established style; the module-list sentence in the file's own docstring gained "research" alongside the other extracted groups it already named. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01T6yA5khYnJ2qzheArRToQ2 --- keel/cli.py | 12 +- keel/commands/research.py | 381 +++++++++++++++++++++ tests/commands/test_research_front_door.py | 296 ++++++++++++++++ 3 files changed, 688 insertions(+), 1 deletion(-) create mode 100644 keel/commands/research.py create mode 100644 tests/commands/test_research_front_door.py diff --git a/keel/cli.py b/keel/cli.py index c28d2fc2..e414507c 100644 --- a/keel/cli.py +++ b/keel/cli.py @@ -62,7 +62,7 @@ broker-touching commands (`fetch`, `agent`, `monitor`, `simulate`, `assets`) that share the `_build_broker` seam, and the remaining top-level commands. The broker-free command groups live in `keel/commands/*` and are registered here via `cli.add_command(...)`: `db`, `trials`, -`withdrawals`, `autonomy`, `rules`, `subscription`, `versions`. The shared seams +`withdrawals`, `autonomy`, `rules`, `research`, `subscription`, `versions`. The shared seams (`with_disclaimer`, the confirmation gate, `_open_repo`/`_load_cfg`/`_build_broker`) live in `keel.commands._common` and are re-imported here; `_is_interactive` is reached as `_common._is_interactive()` so a single patch point in `keel.commands._common` drives every gate @@ -156,6 +156,7 @@ from keel.commands.monitor import run_monitor from keel.commands.pnl import build_pnl_report, render_pnl_report from keel.commands.purification import render_purification_report +from keel.commands.research import research_group from keel.commands.rules import rules_group, rules_seed from keel.commands.serve import serve_cmd from keel.commands.setup import setup_cmd, template_config_text @@ -1132,6 +1133,15 @@ def stop_flag(_count: list[int] = [0]) -> bool: # noqa: B006 - intentional muta cli.add_command(rules_group) +# -- research (the front door over keel/research/*, issue #601) ----------------------------- + +# The `research` group is defined in `keel.commands.research`: an index over all thirteen +# evidence modules plus aliases of the five commands `trials`/`rules` already register (the +# same objects, not copies -- see that module's docstring). Registered here, after both +# groups it aliases into, so the objects it reaches into already exist. +cli.add_command(research_group) + + # -- pnl ------------------------------------------------------------------------------ diff --git a/keel/commands/research.py b/keel/commands/research.py new file mode 100644 index 00000000..bfdfc577 --- /dev/null +++ b/keel/commands/research.py @@ -0,0 +1,381 @@ +"""`keel research` -- one front door over the thirteen evidence modules in `keel/research/` +(issue #601). + +Every one of those modules already exists on `main`, and half of them already have a CLI: +`keel trials` fronts `ledger.py` (`record`/`list`/`verify`), `cscv.py` + `matrix.py` (`pbo`), +`deflate.py` (`deflate`), `montecarlo.py` (`monte-carlo`) and `walkforward.py` +(`walk-forward`); `keel rules lookahead` fronts `bias.py`. The other six -- +`significance.py`, `cts_factors.py`, `independence.py`, `throughput.py`, `tuning.py` and +`pooled_review.py` -- have no CLI at all yet. So the actual gap `keel research` closes is not +"thirteen modules with no code path"; it is "no single place that says here is the evidence +toolkit". The tools were scattered across three groups and 30+ ad-hoc drivers in +`docs/experiments/*.py`, discoverable only by reading source. + +`keel research index` is that place: for every module it states the question it answers, +what it CANNOT answer, and the exact command line (or, for a module still waiting on its own +subcommand, the pre-registered driver that runs it today) that gets you the number. The five +commands that already exist are registered a SECOND TIME under this group -- the same click +command objects, never copies (see the alias block below) -- so `keel research pbo` and +`keel trials pbo` are, byte for byte, the same code running. + +**A refusal is a result here, not an error.** Every module in this package can legitimately +say "there is not enough evidence to answer that" -- `significance.py`'s docstring states the +discipline plainly: "a significance tool here must be able to say 'not distinguishable from +zero' and mean it. A tool that cannot say no is a flattery tool." That principle is why the +aliased commands in `keel.commands.trials` were edited alongside this file: an evidence-shaped +refusal now prints on stdout and exits 0, the same as any other measurement this group +reports. Reserve non-zero exits and `click.ClickException` for OPERATOR error -- a rule id +that does not exist, a ledger that cannot be read, a db that will not open -- never for "the +question was well-formed and the evidence cannot answer it". + +**The Strathern rail.** `cscv.py`, `deflate.py` and `walkforward.py` each carry a ⛔ comment: +a score may report, and may gate, but may NEVER be a sweep's ranking key. This module is the +newest surface built over those three, which makes it the newest place the rail could leak +into a ranking -- so nothing in here sorts, ranks, or picks a "best" configuration by a score. +`tests/commands/test_research_front_door.py` enforces that with a source scan, and the index +below states the rail sentence next to every rail-bearing module so a reader meets it exactly +where they are about to run one. +""" + +from __future__ import annotations + +import json +from dataclasses import asdict, dataclass + +import click + +from keel.commands.rules import rules_group +from keel.commands.trials import trials_group + +#: The sentence a reader must meet next to every rail-bearing module (pinned by test: this +#: exact wording is what `cscv.py`/`deflate.py`/`walkforward.py` themselves state as their +#: ⛔ STRATHERN RAIL). Declared once so the index, the tests, and any future renderer quote +#: the same words rather than three independent paraphrases drifting apart. +RAIL_SENTENCE = ( + "a score may report, and may gate, but may NEVER be a sweep's ranking key -- nothing " + "here returns the identity of a best-performing configuration" +) + + +@dataclass(frozen=True) +class ResearchModuleEntry: + """One row of the front door: a module in `keel/research/`, the question it answers, + what it structurally cannot answer, and where an operator actually runs it. + + `module` is the bare filename (`"significance.py"`), matched against + `keel/research/*.py` by the completeness pin in + `tests/commands/test_research_front_door.py` -- a fourteenth module with no row here + fails that test. `runs_as` is either a full `keel ...` command line, when one is + registered in the CLI, or a `docs/experiments/*.py` path, when the module's own + subcommand has not landed yet; the same pin resolves whichever it is. + """ + + module: str + question: str + cannot_answer: str + runs_as: str + rail: bool = False + + +# The declared order below is a LITERAL list, not a computed sort (see the module docstring +# and `test_research_front_door.py`'s Strathern-rail pin for why this group never sorts, +# ranks or maxes over anything, including its own table of contents): it is the order issue +# #601 itself first enumerated the thirteen modules in. Keeping that order legible here means +# a diff that reorders this tuple is a deliberate editorial choice, not a side effect of some +# key function silently changing its mind. +RESEARCH_INDEX: tuple[ResearchModuleEntry, ...] = ( + ResearchModuleEntry( + module="significance.py", + question=( + "Is a rule family's edge distinguishable from zero at the fee actually paid -- " + "a one-proportion test of the observed win rate against the payoff-implied " + "break-even, corrected to effective (n_eff, not raw) observations." + ), + cannot_answer=( + "Cannot say an edge EXISTS -- only whether this much evidence could distinguish " + "one from zero at the stated power. 'Not distinguishable from zero' is a real, " + "printed answer here, not a failure to compute one." + ), + runs_as="docs/experiments/2026-08-21-rule-family-significance.py", + ), + ResearchModuleEntry( + module="montecarlo.py", + question=( + "Trade-reshuffle and moving-block candle-bootstrap resampling -- did one lucky " + "PATH produce this equity curve, distinct from asking whether the family is net " + "positive (significance.py) or whether the selection process was overfit " + "(cscv.py)." + ), + cannot_answer=( + "The reshuffle's final-equity percentile is EXACTLY 1/2 BY CONSTRUCTION -- a " + "permutation of a multiset always sums to the same number -- so it can never " + "tell you whether the final equity was lucky. Only the path shape between start " + "and end (drawdown depth, time underwater) carries information." + ), + runs_as="keel research monte-carlo", + ), + ResearchModuleEntry( + module="cscv.py", + question=( + "Probability of Backtest Overfitting via CSCV (model-free, non-parametric, " + "deterministic) over the matrix of per-period P&L across every configuration " + "tried." + ), + cannot_answer=( + "Nothing here returns the identity of a best-performing configuration -- PBO " + "evaluates the quality of a SELECTION PROCESS, never names the selection." + ), + runs_as="keel research pbo", + rail=True, + ), + ResearchModuleEntry( + module="deflate.py", + question=( + "Turns 'we tried N configurations' into a number: the Deflated Sharpe Ratio, " + "the expected maximum Sharpe of N zero-skill trials, and the Minimum Backtest " + "Length the observed performance needs to clear selection bias." + ), + cannot_answer=( + "Reporting only -- none of E[max SR_n], SR_0, DSR or MinBTL may ever rank or " + "select a configuration; they price the bar one already-chosen strategy has to " + "clear." + ), + runs_as="keel research deflate", + rail=True, + ), + ResearchModuleEntry( + module="walkforward.py", + question=( + "Rolling-origin walk-forward validation of ONE parameter set, fixed before any " + "fold runs: does it hold up out-of-sample across a rolling series of train/test " + "windows, and does performance degrade as the data moves away from the period " + "the set was conceived on." + ), + cannot_answer=( + "Cannot tell you which fold or window WON -- no public function returns a fold, " + "window or parameter set to favour, because none is ever computed. It validates " + "a GIVEN rule across GIVEN folds; it never compares alternatives." + ), + runs_as="keel research walk-forward", + rail=True, + ), + ResearchModuleEntry( + module="independence.py", + question=( + "Whether two rules (or two horizons of one rule) are actually independent: " + "position-vector overlap (Jaccard), signal correlation, entry-timing distance, " + "and P&L correlation." + ), + cannot_answer=( + "Cannot tell you a correlated pair is WRONG to run together -- only that it is " + "not contributing N independent observations' worth of evidence. Whether that " + "correlation is acceptable is left to the operator, not decided here." + ), + runs_as="docs/experiments/2026-08-08-between-family-independence.py", + ), + ResearchModuleEntry( + module="throughput.py", + question=( + "Allowance-throughput planning: how many signals a venue's fee-free volume " + "allowance can actually carry per month, and how long -- in EFFECTIVE " + "observations, via the herding design effect -- the evidence honestly takes to " + "gather." + ), + cannot_answer=( + "The allocator moves trades INTO an existing allowance; it can never enlarge " + "one. A product that does not fit is deferred with a reason, never squeezed " + "through the cap." + ), + runs_as="docs/experiments/2026-09-30-pooled-review.py", + ), + ResearchModuleEntry( + module="cts_factors.py", + question=( + "Do the 11 CTS confluence factors carry independent evidence, or is one " + "momentum read counted three times -- pairwise correlation/collinearity over " + "both an unconditional (every bar) and a conditional (only fired, gate-cleared " + "bars) replay sample." + ), + cannot_answer=( + "Measures collinearity only; it is never imported by the live path and cannot " + "say whether the CTS score itself is profitable -- that is a different question " + "this module does not ask." + ), + runs_as="docs/experiments/2026-08-09-cts-factor-collinearity.py", + ), + ResearchModuleEntry( + module="ledger.py", + question=( + "The append-only, hash-chained record of *experiments* (never money): what was " + "tried, its provenance (a_priori/fitted), and whether the chain has been " + "tampered with since." + ), + cannot_answer=( + "Carries no statistics of its own -- it cannot say whether a trial's result was " + "good, only whether the record of it is intact, and how many decision trials " + "(N) sit inside the total row count (M)." + ), + runs_as="keel trials list", + ), + ResearchModuleEntry( + module="pooled_review.py", + question=( + "The pre-registered 2026-09-30 pooled forward-trades review: the pooled win " + "rate against break-even at the n_eff-corrected interval, with the honest power " + "sentence -- 'at this n_eff, this review can only see an edge of X points or " + "larger' -- always printed beside it." + ), + cannot_answer=( + "Renders NO pass/fail verdict on the edge -- the only verdict-shaped statement " + "is about POWER, never about whether the edge is real. A pool with nothing " + "counted refuses rather than emit a degenerate report." + ), + runs_as="docs/experiments/2026-09-30-pooled-review.py", + ), + ResearchModuleEntry( + module="bias.py", + question=( + "Lookahead and recursive-bias detection: does a stored rule's decision at bar N " + "change once bars after N become visible, replayed through the exact detect-on-" + "growing-prefix seam the backtester and live engine both use." + ), + cannot_answer=( + "Says nothing about whether parameters were over-selected across a trial matrix " + "(cscv.py's question) -- a clean lookahead verdict is not evidence the strategy " + "has a genuine edge, only that it is not reading the future to get one." + ), + runs_as="keel research lookahead", + ), + ResearchModuleEntry( + module="matrix.py", + question=( + "Assembles the CSCV (T x N) matrix from ledger trials, enforcing the one " + "condition PBO itself does not check: a TRUE matrix, same rows for every " + "column, observations synchronous across trials." + ), + cannot_answer=( + "Cannot say whether the assembled matrix indicates overfitting -- that is " + "cscv.py's question entirely. This module only says whether the columns are " + "assemblable at all, refusing (not silently dropping) any column whose per-bar " + "series was never kept." + ), + runs_as="keel research pbo", + ), + ResearchModuleEntry( + module="tuning.py", + question=( + "An Optuna parameter study over a rule's own DECLARED parameter space -- train/" + "held-out split, then PBO/CSCV over the study's own trials -- proposing " + "CANDIDATES for the promotion gauntlet." + ), + cannot_answer=( + "Cannot auto-tune a live or paper profile and cannot itself promote a rule -- a " + "winner here is a hypothesis that still has to clear the unchanged gauntlet. Its " + "most important output is the refusal line: 'no candidate may be proposed.'" + ), + runs_as="docs/experiments/2026-08-22-optuna-parameter-study.py", + ), +) + +#: Names accepted by `--module`, in the same declared order as `RESEARCH_INDEX` -- derived +#: once here (never re-sorted) so the refusal listing below and the completeness pin read +#: the identical order a human sees in `keel research index`. +_MODULE_NAMES: tuple[str, ...] = tuple(entry.module.removesuffix(".py") for entry in RESEARCH_INDEX) + + +def _find_entry(name: str) -> ResearchModuleEntry | None: + for entry in RESEARCH_INDEX: + if entry.module.removesuffix(".py") == name: + return entry + return None + + +def render_index(entries: tuple[ResearchModuleEntry, ...]) -> list[str]: + """Human-readable form of the index: module, question, what it cannot answer, where it + runs -- and, for the three rail-bearing modules, the rail sentence stated right where a + reader is about to go run one.""" + lines: list[str] = [ + "keel research -- the evidence toolkit index (issue #601)", + "", + "Every module below MEASURES. A refusal ('not enough evidence to answer that') is " + "one of its results, printed on stdout, never an error.", + "", + ] + for entry in entries: + lines.append(f"{entry.module}") + lines.append(f" answers : {entry.question}") + lines.append(f" cannot answer : {entry.cannot_answer}") + lines.append(f" runs as : {entry.runs_as}") + if entry.rail: + lines.append(f" ⛔ Strathern rail: {RAIL_SENTENCE}") + lines.append("") + return lines + + +@click.group("research") +def research_group() -> None: + """One front door over the thirteen evidence modules in `keel/research/` (issue #601). + + `keel research index` names all thirteen, what each answers, what each CANNOT answer, + and the command (or pre-registered `docs/experiments/` driver) that runs it. The five + commands aliased into this group below are the SAME objects `keel trials`/`keel rules` + already register -- running `keel research pbo` runs exactly the code + `keel trials pbo` does, not a second copy of it. Nothing here computes a new statistic; + this group assembles inputs, calls into `keel/research/*`, and prints what comes back, + including the refusal. + """ + + +@research_group.command("index") +@click.option("--json", "as_json", is_flag=True, default=False, help="Emit machine-readable JSON.") +@click.option( + "--module", + "module_name", + default=None, + help="Print just one module's entry (e.g. `significance`, `cscv`, `walkforward`).", +) +@click.pass_context +def research_index(ctx: click.Context, as_json: bool, module_name: str | None) -> None: + """Print the front door: every module in `keel/research/`, what it answers, what it + cannot answer, and where to run it. + + `--module NAME` narrows to one entry. An unknown NAME is itself a lookup that found + nothing -- it prints the known names and exits 0, the same discipline this whole group + applies to every other refusal: a well-formed question the index cannot answer is a + result, not an error. + """ + entries = RESEARCH_INDEX + if module_name is not None: + entry = _find_entry(module_name) + if entry is None: + known = ", ".join(_MODULE_NAMES) + click.echo( + f"refused: {module_name!r} is not one of the {len(_MODULE_NAMES)} research " + f"modules. Known names: {known}" + ) + return + entries = (entry,) + + if as_json: + click.echo(json.dumps([asdict(entry) for entry in entries], indent=2, default=str)) + return + + for line in render_index(entries): + click.echo(line) + + +# -- aliases: the five commands `keel trials`/`keel rules` already front ------------------------ +# +# These are the SAME click command objects registered a second time, via `Group.add_command`, +# never a reimplementation. A front door that reimplements is a front door that drifts: two +# copies of `keel ... pbo` would mean two places `cscv.py`'s call signature could be threaded +# differently, two places a bugfix could land in one and not the other, and two places the +# Strathern rail's "reports probabilities, never a configuration" guarantee could be honoured +# in one copy and quietly broken in the other. Registering the object itself makes that +# divergence structurally impossible: there is exactly one implementation, reachable under two +# names. `tests/commands/test_research_front_door.py` pins the object identity. +research_group.add_command(trials_group.commands["pbo"], "pbo") +research_group.add_command(trials_group.commands["deflate"], "deflate") +research_group.add_command(trials_group.commands["monte-carlo"], "monte-carlo") +research_group.add_command(trials_group.commands["walk-forward"], "walk-forward") +research_group.add_command(rules_group.commands["lookahead"], "lookahead") diff --git a/tests/commands/test_research_front_door.py b/tests/commands/test_research_front_door.py new file mode 100644 index 00000000..1eb65f84 --- /dev/null +++ b/tests/commands/test_research_front_door.py @@ -0,0 +1,296 @@ +"""Tests for `keel research` -- the front door over `keel/research/*` (issue #601). + +Two ARCHITECTURAL pins, plus surface tests over the group itself: + +* **The completeness pin** (`test_every_research_module_is_indexed`, + `test_every_runs_as_resolves`). `RESEARCH_INDEX` is a module-level literal, so nothing + stops it from silently falling behind `keel/research/` as that package grows -- these two + tests are what turns "somebody forgot to add a row" into a failing build instead of a + documentation gap nobody notices. The first globs the actual package directory (never a + hardcoded count) and asserts every file it finds has an entry; the second walks the real + click command tree off `keel.cli.cli` (or checks the filesystem, for a module still + waiting on its own subcommand) so a `runs_as` string that used to work but silently broke + -- a renamed command, a typo -- fails here too. + +* **The Strathern rail pin** (`test_research_module_never_sorts_ranks_or_maxes`). `cscv.py`, + `deflate.py` and `walkforward.py` each carry a ⛔ comment: a score may report, and may + gate, but may NEVER be a sweep's ranking key. `keel/commands/research.py` is the newest + surface built over those three, which makes it the newest place that guarantee could leak + -- so this test is an AST scan that fails if the module contains ANY ranking shape at all: + `sorted(...)`/`max(...)`/`min(...)` called with a `key=` argument, `.sort(key=...)`, or an + import of `heapq`/`operator.itemgetter`/`operator.attrgetter`. + + The blanket ban (rather than a narrower rule that only fires when the sorted/ranked values + look rail-bearing) is deliberate. A scanner that tries to decide which fields are + "rail-bearing" before objecting to a sort is a scanner a rename can fool -- rename `pbo` to + `score` and a field-aware check no longer recognises it. The front door's job is to place + values it was given, in an order IT chooses, never in an order a value chooses for it; the + moment this module orders configurations by a score, that score has become a ranking key, + full stop, regardless of the field's name. Where the module legitimately needs a stable + display order (the table of contents itself), the fix is an explicitly declared literal + order -- `RESEARCH_INDEX`'s own comment says so -- never a computed one, so this ban costs + nothing real and closes the door completely. + +Every pin here was written, then deliberately broken, then restored -- see the commit +message for the exact mutation and the exact failure text each produced. +""" + +from __future__ import annotations + +import ast +import json +from pathlib import Path + +import click +import pytest +from click.testing import CliRunner + +from keel.cli import cli +from keel.commands.research import ( + RAIL_SENTENCE, + RESEARCH_INDEX, + ResearchModuleEntry, + render_index, + research_group, +) +from keel.commands.rules import rules_group +from keel.commands.trials import trials_group + +REPO_ROOT = Path(__file__).resolve().parents[2] +RESEARCH_PKG = REPO_ROOT / "keel" / "research" +RESEARCH_MODULE_PATH = REPO_ROOT / "keel" / "commands" / "research.py" + + +def _indexed_names() -> set[str]: + return {entry.module for entry in RESEARCH_INDEX} + + +def _actual_module_files() -> set[str]: + return { + path.name + for path in RESEARCH_PKG.glob("*.py") + if path.name not in {"__init__.py"} and "__pycache__" not in path.parts + } + + +# -- pin (a): completeness ----------------------------------------------------------------------- + + +def test_every_research_module_is_indexed(): + """Every `.py` file actually sitting in `keel/research/` (glob, never a hardcoded + count) must have a row in `RESEARCH_INDEX`. A fourteenth module with no row fails here. + + Mutation-verified: adding a throwaway `keel/research/zzz_probe.py` made this fail with + `AssertionError: {'zzz_probe.py'}` (the file existed, then the test removed it) -- see + the commit message for the exact command and output. + """ + on_disk = _actual_module_files() + indexed = _indexed_names() + missing = on_disk - indexed + assert not missing, f"{missing} exist in keel/research/ but have no RESEARCH_INDEX row" + # And the reverse should never happen either: an index row for a module that was deleted + # is a stale row nobody will notice. + stale = indexed - on_disk + assert not stale, f"{stale} are indexed but no longer exist in keel/research/" + + +def _resolve_cli_command(command_line: str) -> click.Command | None: + """Walk the real command tree off `keel.cli.cli` for a `"keel a b c"` string. Returns + None if any token along the way is not a registered subcommand.""" + tokens = command_line.split() + assert tokens and tokens[0] == "keel", command_line + node: click.Command = cli + for token in tokens[1:]: + if not isinstance(node, click.Group) or token not in node.commands: + return None + node = node.commands[token] + return node + + +def test_every_runs_as_resolves(): + """Every `runs_as` either names a real command reachable from `keel.cli.cli` (walked, + not guessed) or a `docs/experiments/*.py` driver that exists on disk -- never a string + that used to be true. + + Mutation-verified: pointing one entry's `runs_as` at `"keel research not-a-real-command"` + made this fail with `AssertionError: keel research not-a-real-command does not resolve + to a registered CLI command`; see the commit message for the exact diff and output. + """ + for entry in RESEARCH_INDEX: + if entry.runs_as.startswith("keel "): + resolved = _resolve_cli_command(entry.runs_as) + assert resolved is not None, ( + f"{entry.runs_as} does not resolve to a registered CLI command " + f"(module={entry.module})" + ) + else: + driver = REPO_ROOT / entry.runs_as + assert driver.is_file(), ( + f"{entry.runs_as} names a docs/experiments driver that does not exist on " + f"disk (module={entry.module})" + ) + + +# -- pin (b): the Strathern rail survives the front door ----------------------------------------- + + +def test_research_module_never_sorts_ranks_or_maxes(): + """AST scan over `keel/commands/research.py` itself (see the module docstring for why + the ban is blanket, not field-aware). + + Mutation-verified: inserting `sorted(RESEARCH_INDEX, key=lambda r: r.module)` into the + module made this fail with `AssertionError: sorted()/max()/min() called with key= at + keel/commands/research.py:` before being removed again; see the commit message + for the exact snippet and failure line. + """ + source = RESEARCH_MODULE_PATH.read_text(encoding="utf-8") + tree = ast.parse(source, filename=str(RESEARCH_MODULE_PATH)) + + for node in ast.walk(tree): + if isinstance(node, ast.Call): + callee = node.func + # sorted(...)/max(...)/min(...) with a key= keyword. + if isinstance(callee, ast.Name) and callee.id in {"sorted", "max", "min"}: + has_key = any(kw.arg == "key" for kw in node.keywords) + assert not has_key, ( + f"{callee.id}() called with key= at " + f"{RESEARCH_MODULE_PATH}:{node.lineno} -- a keyed sort/max/min IS a " + "ranking; the Strathern rail forbids it here unconditionally" + ) + # anything.sort(key=...) + if isinstance(callee, ast.Attribute) and callee.attr == "sort": + has_key = any(kw.arg == "key" for kw in node.keywords) + assert not has_key, ( + f".sort(key=...) at {RESEARCH_MODULE_PATH}:{node.lineno} -- an in-place " + "keyed sort IS a ranking; forbidden here unconditionally" + ) + # operator.itemgetter(...) / operator.attrgetter(...), however `operator` got + # into scope (bare `import operator`, an alias, etc). + if isinstance(callee, ast.Attribute) and callee.attr in { + "itemgetter", + "attrgetter", + }: + pytest.fail( + f"operator.{callee.attr} used at {RESEARCH_MODULE_PATH}:{node.lineno} " + "-- ranking-by-field machinery is forbidden here unconditionally" + ) + if isinstance(node, ast.Import): + for alias in node.names: + assert alias.name != "heapq", ( + f"import heapq at {RESEARCH_MODULE_PATH}:{node.lineno} -- a priority " + "queue IS ranking machinery; forbidden here unconditionally" + ) + if isinstance(node, ast.ImportFrom): + if node.module == "operator": + imported = {alias.name for alias in node.names} + banned = imported & {"itemgetter", "attrgetter"} + assert not banned, ( + f"from operator import {sorted(banned)} at " + f"{RESEARCH_MODULE_PATH}:{node.lineno} -- forbidden here unconditionally" + ) + + +def test_rail_marked_on_exactly_the_three_strathern_modules(): + railed = {entry.module for entry in RESEARCH_INDEX if entry.rail} + assert railed == {"cscv.py", "deflate.py", "walkforward.py"} + + +def test_index_output_states_the_rail_sentence_where_a_reader_meets_it(): + """Not just that the rail is MENTIONED somewhere -- the exact sentence + (`RAIL_SENTENCE`) must appear, and it must appear beside a rail-bearing module's own + block, not floating disconnected in a preamble.""" + lines = render_index(RESEARCH_INDEX) + rendered = "\n".join(lines) + assert RAIL_SENTENCE in rendered + + # "Where a reader meets it": the sentence sits in the block for cscv.py, immediately + # after that module's "runs as" line, not merely somewhere in the whole document. + cscv_start = rendered.index("cscv.py") + next_module_start = rendered.index("deflate.py") + assert RAIL_SENTENCE in rendered[cscv_start:next_module_start] + + +# -- surface: the group itself -------------------------------------------------------------------- + + +def test_index_exits_zero_and_names_all_thirteen(): + result = CliRunner().invoke(cli, ["research", "index"]) + assert result.exit_code == 0, result.output + for entry in RESEARCH_INDEX: + assert entry.module in result.output + + +def test_index_json_round_trips_and_carries_every_module(): + result = CliRunner().invoke(cli, ["research", "index", "--json"]) + assert result.exit_code == 0, result.output + payload = json.loads(result.output) + assert {row["module"] for row in payload} == _indexed_names() + for row in payload: + assert set(row) == {"module", "question", "cannot_answer", "runs_as", "rail"} + + +def test_index_module_filter_prints_exactly_one_entry(): + result = CliRunner().invoke(cli, ["research", "index", "--module", "significance"]) + assert result.exit_code == 0, result.output + assert "significance.py" in result.output + for entry in RESEARCH_INDEX: + if entry.module != "significance.py": + assert entry.module not in result.output + + +def test_index_unknown_module_refuses_but_exits_zero(): + """Per #601's second bullet, applied to the index's own lookup: an unknown `--module` + name is a well-formed question ("tell me about X") the index cannot answer, not an + operator error -- it refuses on stdout and exits 0, listing the names that DO exist.""" + result = CliRunner().invoke(cli, ["research", "index", "--module", "not-a-real-module"]) + assert result.exit_code == 0, result.output + assert "refused" in result.output + for entry in RESEARCH_INDEX: + assert entry.module.removesuffix(".py") in result.output + + +@pytest.mark.parametrize( + ("research_name", "source_group", "source_name"), + [ + ("pbo", trials_group, "pbo"), + ("deflate", trials_group, "deflate"), + ("monte-carlo", trials_group, "monte-carlo"), + ("walk-forward", trials_group, "walk-forward"), + ("lookahead", rules_group, "lookahead"), + ], +) +def test_alias_is_the_same_object_reachable_under_two_names( + research_name, source_group, source_name +): + """Not a reimplementation: `keel research X` and its source command must be the exact + same click `Command` object, so there is one implementation running under two names.""" + aliased = research_group.commands[research_name] + original = source_group.commands[source_name] + assert aliased is original + + +def test_each_alias_is_reachable_via_cli_invoke_under_both_names(): + for research_name, group_name, source_name in ( + ("pbo", "trials", "pbo"), + ("deflate", "trials", "deflate"), + ("monte-carlo", "trials", "monte-carlo"), + ("walk-forward", "trials", "walk-forward"), + ("lookahead", "rules", "lookahead"), + ): + via_research = CliRunner().invoke(cli, ["research", research_name, "--help"]) + via_source = CliRunner().invoke(cli, [group_name, source_name, "--help"]) + assert via_research.exit_code == 0 + assert via_source.exit_code == 0 + # Same object -> click renders identical help text either way, except the "Usage:" + # line, which necessarily names the path it was invoked through. + research_body = via_research.output.split("\n", 1)[1] + source_body = via_source.output.split("\n", 1)[1] + assert research_body == source_body + + +def test_research_index_entry_is_a_frozen_dataclass_tuple(): + assert isinstance(RESEARCH_INDEX, tuple) + for entry in RESEARCH_INDEX: + assert isinstance(entry, ResearchModuleEntry) + with pytest.raises(Exception): + entry.module = "mutated.py" # type: ignore[misc] From 9a939d2110074b714d904132e3a9d4148de562a6 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Fri, 28 Aug 2026 20:37:27 -0400 Subject: [PATCH 4/9] feat(research): significance, pooled-review, throughput, tuning, factors, independence (#601) Wave A gave keel research an index and five aliases; the six modules with no CLI at all -- significance.py, pooled_review.py, throughput.py, tuning.py, cts_factors.py, independence.py -- still had no code path a reader could actually run, only a docs/experiments/ driver named in the index. This wave closes that: one subcommand per module, each of which does nothing but assemble inputs, call into keel/research/*, and print what comes back (ADR 0003 standing rule 3, now cited in this module's own docstring) -- reusing the modules' own render_family/render_report where they exist, and never a second renderer that could drift from the first. Every one of the six honours #601's second bullet: an evidence-shaped refusal prints on stdout and exits 0, never a ClickException. Two of those refusals are the module's own machinery catching a case its caller must not let escape as a traceback -- throughput.allocate() already raised ValueError when a product is eligible on no listed venue, and render_report's own months_to_target raises when pooled trades/month is zero; both are caught at the command boundary and printed, and throughput.py itself is untouched. significance and independence refuse on their own "nothing to test/compare" check before calling into the module at all; factors refuses when no factor in the replayed sample varies (FactorSample.varying() is empty); tuning's --run always refuses, naming docs/experiments/2026-08-22-optuna- parameter-study.py, because running a study needs optuna and this module must import cleanly without it (pinned by a new AST-scan test in the same commit as the front-door tests). pooled-review is the one with a frozen sibling. #427's standing 2026-09-30 event is pre-registered in docs/experiments/2026-09-30- pooled-review.py's own docstring, and that contract -- refuse to stderr, exit 2 -- predates this issue and stays exactly as written: the driver still owns its own I/O and its own exit code, unchanged. What moved is the READING: _connect_ro, read_orders and read_ledger used to be defined twice in spirit (once as the driver's private functions, and implicitly re-derivable by anything else that wanted to read the same tables the same way); they now live in keel/commands/research.py and the driver imports them, so there is exactly one reader of a deployment database and the CLI and the pre-registered driver cannot diverge on what "the pool" means. keel research pooled-review prints the identical "nothing to review" finding the driver would, but on stdout at exit 0 -- new surface, built after #601 decided a refusal is a result, so it gets the right behaviour from the start rather than inheriting a contract that was frozen before that decision existed. Every read stays `file:...?mode=ro`; this command, like the driver, never writes to a deployment database. independence's position/pnl vectors over the two rules' common bar index are mechanical bookkeeping, commented as such: independence.py's compare() has no opinion on how its inputs are built, only on what to compute once they are aligned onto one calendar, so building that calendar here is formatting inputs, not a new statistic. Also fixed: the index bug the design contract named. throughput.py's runs_as pointed at the pooled-review driver -- a different module's driver entirely -- because the completeness pin only checked that the named file existed, never that it was the RIGHT driver. Every module that now has a real subcommand (significance, independence, throughput, cts_factors, pooled_review, tuning) has its runs_as updated to name that command instead of a docs/experiments path; the tightened pin that actually would have caught the original slip lands in the next commit, alongside its own mutation-verification. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01T6yA5khYnJ2qzheArRToQ2 --- docs/experiments/2026-09-30-pooled-review.py | 81 +- keel/commands/research.py | 765 ++++++++++++++++++- 2 files changed, 766 insertions(+), 80 deletions(-) diff --git a/docs/experiments/2026-09-30-pooled-review.py b/docs/experiments/2026-09-30-pooled-review.py index 1a20912a..21beaec5 100644 --- a/docs/experiments/2026-09-30-pooled-review.py +++ b/docs/experiments/2026-09-30-pooled-review.py @@ -77,15 +77,19 @@ import sqlite3 import sys from datetime import UTC, datetime -from decimal import Decimal -from pathlib import Path from typing import Any +# `_connect_ro`/`read_orders`/`read_ledger` used to be defined here; #601 moved them into +# `keel.commands.research` (the new `keel research pooled-review` front door) and this driver +# now IMPORTS them, so there is exactly one reader of a deployment database and the CLI and this +# pre-registered driver structurally cannot diverge on what "the pool" means. `DEFAULT_DBS` moves +# with them for the same reason -- one literal, not two copies that could drift apart. +from keel.commands.research import DEFAULT_POOLED_REVIEW_DBS as DEFAULT_DBS +from keel.commands.research import _connect_ro, read_ledger, read_orders from keel.research.pooled_review import ( EVENT_DATE, DescriptiveReview, LedgerRow, - OrderRow, OrdersRead, build_sample, descriptive_review, @@ -95,81 +99,10 @@ ) from keel.research.throughput import design_effect -DEFAULT_DBS = ( - str(Path.home() / "keel" / "keel.db"), - str(Path.home() / "keel" / "keel-live.db"), - str(Path.home() / "keel" / "keel-paperhourly.db"), -) DEFAULT_OUT = "docs/experiments/2026-09-30-pooled-review.md" DEFAULT_JSONL = "docs/experiments/2026-09-30-pooled-review.jsonl" -def _connect_ro(db_path: str) -> sqlite3.Connection: - """The house read-only connection (`mode=ro`); the deployment dbs are never written.""" - connection = sqlite3.connect(f"file:{db_path}?mode=ro", uri=True) - connection.row_factory = sqlite3.Row - return connection - - -def read_orders(db_path: str) -> tuple[list[OrderRow], dict[int, str]]: - """The profile's `orders` rows (money as Decimal) and its `rule_id -> rules.kind` map. - - Ascending id — the ledger's own event sequencing, which the matcher relies on because - the live `created_at` values demonstrably disagree with it. - """ - connection = _connect_ro(db_path) - try: - order_rows = connection.execute( - "SELECT id, mode, product_id, side, qty, status, actual_fill, fee, rule_id, " - "created_at FROM orders ORDER BY id" - ).fetchall() - rule_rows = connection.execute("SELECT id, kind FROM rules ORDER BY id").fetchall() - finally: - connection.close() - orders = [ - OrderRow( - id=int(row["id"]), - mode=str(row["mode"]), - product_id=str(row["product_id"]), - side=str(row["side"]), - qty=Decimal(str(row["qty"])), - status=str(row["status"]), - actual_fill=None if row["actual_fill"] is None else Decimal(str(row["actual_fill"])), - fee=None if row["fee"] is None else Decimal(str(row["fee"])), - rule_id=None if row["rule_id"] is None else int(row["rule_id"]), - created_at=int(row["created_at"]), - ) - for row in order_rows - ] - return orders, {int(row["id"]): str(row["kind"]) for row in rule_rows} - - -def read_ledger(db_path: str) -> list[LedgerRow]: - """The profile's `trade_outcomes` rows, oldest first (the ledger reader's convention).""" - connection = _connect_ro(db_path) - try: - rows = connection.execute( - "SELECT product_id, rule_name, opened_at, closed_at, qty, entry_fill, " - "exit_fill, fees, pnl_net FROM trade_outcomes ORDER BY closed_at, id" - ).fetchall() - finally: - connection.close() - return [ - LedgerRow( - product_id=str(row["product_id"]), - rule_name=str(row["rule_name"]), - opened_at=int(row["opened_at"]), - closed_at=int(row["closed_at"]), - qty=Decimal(str(row["qty"])), - entry_fill=Decimal(str(row["entry_fill"])), - exit_fill=Decimal(str(row["exit_fill"])), - fees=Decimal(str(row["fees"])), - pnl_net=Decimal(str(row["pnl_net"])), - ) - for row in rows - ] - - def jsonl_row(review: DescriptiveReview) -> dict[str, Any]: """The one-row-per-run artifact record: every Decimal as a string, like the #475 run.""" sample = review.sample diff --git a/keel/commands/research.py b/keel/commands/research.py index bfdfc577..734191bb 100644 --- a/keel/commands/research.py +++ b/keel/commands/research.py @@ -35,17 +35,39 @@ `tests/commands/test_research_front_door.py` enforces that with a source scan, and the index below states the rail sentence next to every rail-bearing module so a reader meets it exactly where they are about to run one. + +**ADR 0003** (`docs/decisions/0003-commands-layer-survey.md`) is the record this module answers +to: its standing rule 3 requires that "a decision-bearing path added to `commands/` must +delegate its deciding comparison to a compute module and cite this record." Every subcommand +below -- the five aliases and the six Wave B additions (`significance`, `pooled-review`, +`throughput`, `tuning`, `factors`, `independence`) -- assembles inputs, calls a function in +`keel/research/*`, and prints what comes back; the comparison that decides a verdict, a refusal, +or a number always lives in the module it fronts, never here. """ from __future__ import annotations import json +import sqlite3 from dataclasses import asdict, dataclass +from datetime import UTC, datetime +from decimal import Decimal +from pathlib import Path +from typing import Any import click +from keel.commands import rules as rules_mod +from keel.commands._common import _open_repo from keel.commands.rules import rules_group from keel.commands.trials import trials_group +from keel.research import cts_factors as cts_factors_mod +from keel.research import independence as independence_mod +from keel.research import pooled_review as pooled_review_mod +from keel.research import significance as significance_mod +from keel.research import throughput as throughput_mod +from keel.research import tuning as tuning_mod +from keel.types import Granularity #: The sentence a reader must meet next to every rail-bearing module (pinned by test: this #: exact wording is what `cscv.py`/`deflate.py`/`walkforward.py` themselves state as their @@ -96,7 +118,7 @@ class ResearchModuleEntry: "one from zero at the stated power. 'Not distinguishable from zero' is a real, " "printed answer here, not a failure to compute one." ), - runs_as="docs/experiments/2026-08-21-rule-family-significance.py", + runs_as="keel research significance", ), ResearchModuleEntry( module="montecarlo.py", @@ -171,7 +193,7 @@ class ResearchModuleEntry: "not contributing N independent observations' worth of evidence. Whether that " "correlation is acceptable is left to the operator, not decided here." ), - runs_as="docs/experiments/2026-08-08-between-family-independence.py", + runs_as="keel research independence", ), ResearchModuleEntry( module="throughput.py", @@ -186,7 +208,7 @@ class ResearchModuleEntry: "one. A product that does not fit is deferred with a reason, never squeezed " "through the cap." ), - runs_as="docs/experiments/2026-09-30-pooled-review.py", + runs_as="keel research throughput", ), ResearchModuleEntry( module="cts_factors.py", @@ -201,7 +223,7 @@ class ResearchModuleEntry: "say whether the CTS score itself is profitable -- that is a different question " "this module does not ask." ), - runs_as="docs/experiments/2026-08-09-cts-factor-collinearity.py", + runs_as="keel research factors", ), ResearchModuleEntry( module="ledger.py", @@ -230,7 +252,7 @@ class ResearchModuleEntry: "is about POWER, never about whether the edge is real. A pool with nothing " "counted refuses rather than emit a degenerate report." ), - runs_as="docs/experiments/2026-09-30-pooled-review.py", + runs_as="keel research pooled-review", ), ResearchModuleEntry( module="bias.py", @@ -273,7 +295,7 @@ class ResearchModuleEntry: "winner here is a hypothesis that still has to clear the unchanged gauntlet. Its " "most important output is the refusal line: 'no candidate may be proposed.'" ), - runs_as="docs/experiments/2026-08-22-optuna-parameter-study.py", + runs_as="keel research tuning", ), ) @@ -364,6 +386,737 @@ def research_index(ctx: click.Context, as_json: bool, module_name: str | None) - click.echo(line) +# -- significance: is a family's edge distinguishable from zero, at the fee actually paid ------- +# +# Two ways to get `OutcomeRow` tuples for `significance.significance()`: the deployment's own +# `trade_outcomes` ledger (`--from deployment`, the default), or one stored rule's own backtest +# (`--from rule`). The deployment path reuses `pooled_review.ledger_round_trips` + +# `RoundTrip.outcome()` for the win/loss/scratch classification rather than re-deriving it from +# `pnl_net`'s sign a second time -- one classifier, shared with the pooled review below. + + +@research_group.command("significance") +@click.option( + "--from", + "source", + type=click.Choice(["deployment", "rule"]), + default="deployment", + show_default=True, + help="deployment: this db's own trade_outcomes ledger. rule: backtest one stored rule.", +) +@click.option( + "--rule", + "rule_id", + type=int, + default=None, + help="Stored rule id (required for --from rule).", +) +@click.option( + "--granularity", + default=None, + help="Candle granularity for --from rule (default: the rule's own, else ONE_HOUR).", +) +@click.option( + "--family", + default=None, + help="Label for the report (default: 'deployment', or the rule's own kind).", +) +@click.option( + "--fee-regime", + type=click.Choice(sorted(significance_mod.FEE_REGIMES)), + default=None, + help="Restrict to one fee regime (default: BOTH, never an average -- significance.py's " + "own rule).", +) +@click.pass_context +def research_significance( + ctx: click.Context, + source: str, + rule_id: int | None, + granularity: str | None, + family: str | None, + fee_regime: str | None, +) -> None: + """Is a rule family's edge distinguishable from zero at the fee actually paid? + + `significance.significance()` is the whole measurement; this command only assembles the + `OutcomeRow` sequence it reads. "Not distinguishable from zero" and "insufficient_n" are + both legitimate, printed verdicts (issue #601's second bullet) -- neither is a failure of + this command, and both come straight out of `render_family`. What IS a refusal here, printed + before any regime is priced, is having no closed trades to test at all. + """ + if source == "rule" and rule_id is None: + raise click.ClickException("--from rule requires --rule ID") + + repo = _open_repo(ctx) + outcomes: list[significance_mod.OutcomeRow] + if source == "deployment": + rows = repo.get_trade_outcomes() + ledger_rows: list[pooled_review_mod.LedgerRow] = [ + pooled_review_mod.LedgerRow( + product_id=row["product_id"], + rule_name=row["rule_name"], + opened_at=row["opened_at"], + closed_at=row["closed_at"], + qty=row["qty"], + entry_fill=row["entry_fill"], + exit_fill=row["exit_fill"], + fees=row["fees"], + pnl_net=row["pnl_net"], + ) + for row in rows + ] + trips = pooled_review_mod.ledger_round_trips("deployment", ledger_rows) + outcomes = [trip.outcome() for trip in trips] + label = family or "deployment" + else: + assert rule_id is not None # guarded above + config = rules_mod._optional_cfg(ctx) + try: + resolved = rules_mod.resolve_rule_backtest( + repo, config, rule_id, granularity_opt=granularity + ) + except rules_mod.RulesRefused as exc: + raise click.ClickException(str(exc)) from exc + result = rules_mod.backtest_resolved(resolved) + outcomes = [ + (trade.outcome, trade.pnl, trade.r_multiple) + for trade in result.trades + if trade.outcome != "open" + ] + label = family or resolved.row["kind"] + + if not outcomes: + click.echo(f"refused: no closed trades for {label!r} -- nothing to test") + return + + regimes = (fee_regime,) if fee_regime is not None else tuple(significance_mod.FEE_REGIMES) + for regime in regimes: + stat = significance_mod.significance( + label, regime, significance_mod.FEE_REGIMES[regime], outcomes + ) + for line in significance_mod.render_family(stat): + click.echo(line) + click.echo("") + + +# -- pooled-review: the 2026-09-30 standing event (#427), through the front door ----------------- +# +# `_connect_ro`/`read_orders`/`read_ledger` used to live only in the pre-registered driver +# (`docs/experiments/2026-09-30-pooled-review.py`); they now live HERE and the driver imports +# them, so there is exactly one reader of a deployment database and the CLI and the pre- +# registered driver structurally cannot diverge on what "the pool" means (#601). Every +# connection is `mode=ro`: this touches live deployment databases and must never write to one. +# +# The driver's own exit contract is UNCHANGED and stays that way on purpose: it prints a refusal +# to stderr and exits 2, because that contract was pre-registered before the review event and is +# frozen -- rewriting it now would be rewriting the pre-registration after the fact. This command +# is new surface, built after #601 decided a refusal is a result: it prints the same "nothing to +# review" finding to stdout and exits 0. Two exit shapes for the same finding, deliberately, and +# each one is honest about which contract it is answering to. + +#: The three pre-registered deployment profiles (#353), read-only. Moved here from the driver so +#: both the driver and this command default to literally the same three paths. +DEFAULT_POOLED_REVIEW_DBS: tuple[str, ...] = ( + str(Path.home() / "keel" / "keel.db"), + str(Path.home() / "keel" / "keel-live.db"), + str(Path.home() / "keel" / "keel-paperhourly.db"), +) + + +def _connect_ro(db_path: str) -> sqlite3.Connection: + """The house read-only connection (`mode=ro`) -- a deployment db is read, never written.""" + connection = sqlite3.connect(f"file:{db_path}?mode=ro", uri=True) + connection.row_factory = sqlite3.Row + return connection + + +def read_orders(db_path: str) -> tuple[list[pooled_review_mod.OrderRow], dict[int, str]]: + """The profile's `orders` rows (money as Decimal) and its `rule_id -> rules.kind` map. + + Ascending id -- the ledger's own event sequencing, which the matcher relies on because the + live `created_at` values demonstrably disagree with it. + """ + connection = _connect_ro(db_path) + try: + order_rows = connection.execute( + "SELECT id, mode, product_id, side, qty, status, actual_fill, fee, rule_id, " + "created_at FROM orders ORDER BY id" + ).fetchall() + rule_rows = connection.execute("SELECT id, kind FROM rules ORDER BY id").fetchall() + finally: + connection.close() + orders = [ + pooled_review_mod.OrderRow( + id=int(row["id"]), + mode=str(row["mode"]), + product_id=str(row["product_id"]), + side=str(row["side"]), + qty=Decimal(str(row["qty"])), + status=str(row["status"]), + actual_fill=None if row["actual_fill"] is None else Decimal(str(row["actual_fill"])), + fee=None if row["fee"] is None else Decimal(str(row["fee"])), + rule_id=None if row["rule_id"] is None else int(row["rule_id"]), + created_at=int(row["created_at"]), + ) + for row in order_rows + ] + return orders, {int(row["id"]): str(row["kind"]) for row in rule_rows} + + +def read_ledger(db_path: str) -> list[pooled_review_mod.LedgerRow]: + """The profile's `trade_outcomes` rows, oldest first (the ledger reader's convention).""" + connection = _connect_ro(db_path) + try: + rows = connection.execute( + "SELECT product_id, rule_name, opened_at, closed_at, qty, entry_fill, " + "exit_fill, fees, pnl_net FROM trade_outcomes ORDER BY closed_at, id" + ).fetchall() + finally: + connection.close() + return [ + pooled_review_mod.LedgerRow( + product_id=str(row["product_id"]), + rule_name=str(row["rule_name"]), + opened_at=int(row["opened_at"]), + closed_at=int(row["closed_at"]), + qty=Decimal(str(row["qty"])), + entry_fill=Decimal(str(row["entry_fill"])), + exit_fill=Decimal(str(row["exit_fill"])), + fees=Decimal(str(row["fees"])), + pnl_net=Decimal(str(row["pnl_net"])), + ) + for row in rows + ] + + +def _pooled_review_db_reachable(db_path: str) -> bool: + try: + connection = _connect_ro(db_path) + except sqlite3.Error: + return False + connection.close() + return True + + +def _pooled_review_jsonl_row(review: pooled_review_mod.DescriptiveReview) -> dict[str, Any]: + """The command's own one-row summary. Presentation only -- every field is already computed + by `pooled_review.py`; this is not the driver's `jsonl_row` (which the driver still owns + unchanged) because the two artifacts answer different callers and are not required to share + a schema, unlike `_connect_ro`/`read_orders`/`read_ledger`, which are the DATA the pool + means and must never have two copies.""" + sample = review.sample + stat = review.stat + return { + "run_date": review.run_date, + "event_date": review.event_date, + "profiles": list(review.profiles), + "pooled_n": sample.n_pooled(), + "counted_n": sample.counted(), + "win_rate": str(stat.win_rate), + "edge": str(stat.edge), + "n_effective": str(stat.n_effective), + "fee_pct": str(review.fee_pct), + "power_sentence": review.sentence, + } + + +@research_group.command("pooled-review") +@click.option( + "--db", + "dbs", + multiple=True, + help=f"Deployment profile db, repeatable (default: {list(DEFAULT_POOLED_REVIEW_DBS)}).", +) +@click.option( + "--run-date", + default=None, + help=f"Run date label, ISO (default: today UTC). A date before " + f"{pooled_review_mod.EVENT_DATE} labels the report a preview of the event.", +) +@click.option( + "--out", + type=click.Path(dir_okay=False, path_type=Path), + default=None, + help="Optional markdown report path to also write.", +) +@click.option( + "--jsonl", + type=click.Path(dir_okay=False, path_type=Path), + default=None, + help="Optional one-row JSON summary path to also write.", +) +def research_pooled_review( + dbs: tuple[str, ...], run_date: str | None, out: Path | None, jsonl: Path | None +) -> None: + """The pre-registered 2026-09-30 pooled forward-trades review (#427, tracked in #353). + + Reads every listed profile db READ-ONLY, pools the closed round trips per the frozen + pre-registration in `docs/experiments/2026-09-30-pooled-review.py`'s own docstring, and + prints `pooled_review.render_report` verbatim -- or, when the pool has nothing counted, + the refusal, on stdout, exit 0 (see the module docstring for why that differs from the + driver's stderr/exit-2 contract). + """ + db_list = list(dbs) if dbs else list(DEFAULT_POOLED_REVIEW_DBS) + resolved_run_date = run_date or datetime.now(UTC).date().isoformat() + + unreachable = [db for db in db_list if not _pooled_review_db_reachable(db)] + if unreachable: + raise click.ClickException( + "pre-registered profile db(s) not reachable read-only: " + + ", ".join(unreachable) + + " -- the pool cannot be read as pre-registered" + ) + + per_profile: list[ + tuple[str, pooled_review_mod.OrdersRead, list[pooled_review_mod.LedgerRow]] + ] = [] + for db in db_list: + try: + orders, rule_kinds = read_orders(db) + read = pooled_review_mod.round_trips_from_orders(db, orders, rule_kinds) + ledger = read_ledger(db) + except (ValueError, sqlite3.Error) as exc: + raise click.ClickException(f"{db} cannot be read as pre-registered: {exc}") from exc + per_profile.append((db, read, ledger)) + + sample = pooled_review_mod.build_sample(per_profile) + review = pooled_review_mod.descriptive_review(sample, run_date=resolved_run_date) + + if pooled_review_mod.is_refused(sample): + refusal = review.refusal or ("nothing to review",) + click.echo(f"refused: {refusal[0]}") + for line in refusal[1:]: + click.echo(line) + return + + lines = pooled_review_mod.render_report(review) + for line in lines: + click.echo(line) + + if out is not None: + out.write_text("\n".join(lines) + "\n") + click.echo(f"\nwrote {out}") + if jsonl is not None: + jsonl.write_text(json.dumps(_pooled_review_jsonl_row(review)) + "\n") + click.echo(f"wrote {jsonl}") + + +# -- throughput: pure arithmetic, no db needed ---------------------------------------------------- + + +def _decimal_or_none(value: Any) -> Decimal | None: + return None if value is None else Decimal(str(value)) + + +@research_group.command("throughput") +@click.option( + "--venues-json", + required=True, + help='JSON array of venue inputs: [{"venue": "coinbase", "monthly_allowance": "500" ' + '(or null for unlimited), "mean_trade_notional": "4212", "expected_signals_per_month": "1"}]', +) +@click.option( + "--target-edge", + default="0.05", + show_default=True, + help="Win-rate edge (fraction, e.g. 0.05 = 5 points) months_to_target should price.", +) +@click.option( + "--products-json", + default=None, + help='Optional JSON array of products to also run the allocator over: [{"symbol": "SOL-USD",' + ' "venues": ["coinbase"], "mean_trade_notional": "..", "expected_signals_per_month": ".."}]', +) +@click.option( + "--allowances-json", + default=None, + help='JSON object {"venue": allowance-or-null}, required together with --products-json.', +) +def research_throughput( + venues_json: str, + target_edge: str, + products_json: str | None, + allowances_json: str | None, +) -> None: + """Allowance-throughput planning: how many signals a fee-free allowance can carry a month, + and how long the evidence honestly takes to accumulate (throughput.py). + + Pure arithmetic -- no db, no candles, no rule. `--venues-json` states what + `render_report`/`months_to_target` need to know about each venue; `--products-json` + + `--allowances-json`, when both given, additionally run the allocator. `allocate()` raises + `ValueError` when a product is eligible on no listed venue: caught here and printed as a + refusal (#601's second bullet), never a traceback, and `throughput.py` itself is unchanged. + """ + try: + venues = [ + throughput_mod.VenueThroughput( + venue=str(row["venue"]), + monthly_allowance=_decimal_or_none(row.get("monthly_allowance")), + mean_trade_notional=Decimal(str(row["mean_trade_notional"])), + expected_signals_per_month=Decimal(str(row["expected_signals_per_month"])), + ) + for row in json.loads(venues_json) + ] + except (json.JSONDecodeError, KeyError, TypeError) as exc: + raise click.ClickException(f"--venues-json is malformed: {exc}") from exc + + edge = Decimal(str(target_edge)) + if not (Decimal(0) < edge < Decimal(1)): + raise click.ClickException("--target-edge must be a fraction in (0, 1), e.g. 0.05") + + try: + lines = throughput_mod.render_report(venues, edge) + except ValueError as exc: + # render_report's own months_to_target refuses on zero pooled trades/month (an empty + # --venues-json, or every venue's allowance-bound throughput rounding to nothing) -- + # a well-formed plan the data cannot answer, not an operator mistake. Print it, exit 0. + click.echo(f"refused: {exc}") + return + for line in lines: + click.echo(line) + + if products_json is None: + return + if allowances_json is None: + raise click.ClickException("--products-json requires --allowances-json") + try: + products = [ + throughput_mod.Product( + symbol=str(row["symbol"]), + venues=tuple(row["venues"]), + mean_trade_notional=Decimal(str(row["mean_trade_notional"])), + expected_signals_per_month=Decimal(str(row["expected_signals_per_month"])), + ) + for row in json.loads(products_json) + ] + allowances = { + str(venue): _decimal_or_none(allowance) + for venue, allowance in json.loads(allowances_json).items() + } + except (json.JSONDecodeError, KeyError, TypeError) as exc: + raise click.ClickException(f"--products-json/--allowances-json malformed: {exc}") from exc + + try: + plans = throughput_mod.allocate(products, allowances) + except ValueError as exc: + click.echo(f"refused: {exc}") + return + + click.echo("") + click.echo("allocation:") + for plan in plans: + cap = "unlimited" if plan.allowance is None else f"{plan.allowance}/month" + enabled = [product.symbol for product in plan.enabled] + deferred = [product.symbol for product in plan.deferred] + click.echo( + f" {plan.venue} (cap {cap}): enabled={enabled} deferred={deferred} " + f"spend={plan.spend_per_month} trades/month={plan.trades_per_month}" + ) + + +# -- tuning: the declared search spaces, never optuna ---------------------------------------------- +# +# `keel/research/tuning.py` imports optuna lazily, inside `run_study` only, so `import +# keel.research.tuning` (the line above) stays clean without it. This command must hold to the +# same discipline for the exact same reason (#601's fourth bullet): optuna is a dev-only +# dependency (`pyproject.toml`'s dev group) and the shipped CLI must import cleanly without it. +# `tests/commands/test_research_front_door.py` pins that with an AST scan of this file for any +# `import optuna`/`from optuna import ...` -- there is none, on purpose: everything below reads +# `tuning.SEARCH_SPACES`/`declared_cells`/`explored_vs_declared`, none of which touch optuna. + + +@research_group.command("tuning") +@click.option( + "--rule-kind", + default=None, + help="Narrow to one family's declared search space (default: every declared family).", +) +@click.option( + "--explored-json", + default=None, + help='JSON {"dimension": [min, max]} actually swept; requires --rule-kind.', +) +@click.option( + "--run", + is_flag=True, + default=False, + help="Run an optuna parameter study (always refused -- see below).", +) +def research_tuning(rule_kind: str | None, explored_json: str | None, run: bool) -> None: + """The declared per-family search spaces and exploration/gate vocabulary (tuning.py) -- + never a study. + + `--run` is always refused: running a study needs optuna, a dev-only dependency this shipped + command must not import (#601's fourth bullet), so the refusal names the pre-registered + driver that runs one instead, `docs/experiments/2026-08-22-optuna-parameter-study.py`. + """ + if run: + click.echo( + "refused: keel research tuning reports the declared search spaces only -- optuna " + "is a dev-only dependency (pyproject.toml) and the shipped CLI must import " + "cleanly without it. Run a study with " + "`docs/experiments/2026-08-22-optuna-parameter-study.py` instead." + ) + return + + if explored_json is not None and rule_kind is None: + raise click.ClickException("--explored-json requires --rule-kind") + + kinds = [rule_kind] if rule_kind is not None else list(tuning_mod.SEARCH_SPACES) + for kind in kinds: + if kind not in tuning_mod.SEARCH_SPACES: + raise click.ClickException( + f"unknown rule kind {kind!r}; declared families: " + f"{sorted(tuning_mod.SEARCH_SPACES)}" + ) + space = tuning_mod.SEARCH_SPACES[kind] + cells = tuning_mod.declared_cells(kind) + click.echo(f"{kind}: declared search space ({cells} cells)") + for name, bounds in space.items(): + click.echo(f" {name}: {bounds}") + + if explored_json is not None: + assert rule_kind is not None # guarded above + explored = { + name: (bounds[0], bounds[1]) for name, bounds in json.loads(explored_json).items() + } + try: + check = tuning_mod.explored_vs_declared(explored, rule_kind) + except ValueError as exc: + # The sweep as described does not fit the rule's own declared space (an out-of- + # bounds range, or a dimension the rule never declared) -- an honesty check the + # data fails, not an operator typo. Print it, exit 0. + click.echo(f"refused: {exc}") + return + click.echo( + f" explored {check.explored_cells} of {check.declared_cells} declared cells" + ) + + +# -- factors: do the 11 CTS confluence factors carry independent evidence? ----------------------- + + +def _render_factors( + product_id: str, + sample: cts_factors_mod.FactorSample, + stats: list[cts_factors_mod.PairStat], + clusters: list[cts_factors_mod.ClusterReport], + variance: cts_factors_mod.VarianceReport, +) -> list[str]: + """Formatting only: every field below was computed by `cts_factors.py`. Printed in the + order those functions already returned it (`pair_stats`/`holm_adjust` sort by |phi| + themselves; nothing here re-sorts).""" + lines = [ + f"CTS factor collinearity -- {product_id}, n={sample.n} observations, " + f"{len(sample.varying())} varying factor(s) of {len(cts_factors_mod.FACTOR_NAMES)}", + "", + "pairwise (Holm-Bonferroni adjusted; '*' = significant after correction):", + ] + for stat in stats: + flag = " *" if stat.significant else "" + lines.append( + f" {stat.a} x {stat.b}: phi={stat.phi} jaccard={stat.jaccard} lift={stat.lift} " + f"p={stat.p_value:.4g} p_holm={stat.p_holm:.4g}{flag}" + ) + lines.append("") + lines.append("pre-declared clusters (cts_factors.SUSPECTED_CLUSTERS):") + for cluster in clusters: + lines.append( + f" {cluster.name} {cluster.members}: mean_within_phi={cluster.mean_within_phi} " + f"max_within_phi={cluster.max_within_phi} mean_other_phi={cluster.mean_other_phi} " + f"mean_within_jaccard={cluster.mean_within_jaccard} weight_share={cluster.weight_share}" + ) + lines += [ + "", + f"CTS total variance: observed={variance.observed} independent={variance.independent} " + f"ratio={variance.ratio} mean_total={variance.mean_total}", + ] + return lines + + +@research_group.command("factors") +@click.option("--product", "product_id", required=True, help="Product id to replay (e.g. BTC-USD).") +@click.option( + "--granularity", + default="ONE_DAY", + show_default=True, + help="Candle granularity (cts_factors.py's primary arm: ONE_DAY, expanding window).", +) +@click.option("--warmup", default=cts_factors_mod.DEFAULT_WARMUP, show_default=True, type=int) +@click.option( + "--window", + default=None, + type=int, + help="Fixed lookback (default: expanding from the first cached bar -- the live path).", +) +@click.option("--step", default=1, show_default=True, type=int, help="Bar-index thinning.") +@click.option( + "--alpha", + default=0.05, + show_default=True, + type=float, + help="Holm-Bonferroni family-wise alpha.", +) +@click.pass_context +def research_factors( + ctx: click.Context, + product_id: str, + granularity: str, + warmup: int, + window: int | None, + step: int, + alpha: float, +) -> None: + """Do the 11 CTS confluence factors carry independent evidence, unconditionally replayed + over one product's own candle cache (cts_factors.py, #208)? + + The UNCONDITIONAL sample (`replay_every_bar`) is what carries the headline per the module's + own docstring -- the conditional (fired-signal) sample is a collider and is not offered + here. A sample where no factor varies has no correlation to report and refuses. + """ + try: + gran = Granularity(granularity) + except ValueError as exc: + raise click.ClickException(str(exc)) from exc + + repo = _open_repo(ctx) + candles = repo.get_candles(product_id, gran) + if not candles: + click.echo(f"refused: no cached candles for {product_id} {gran.value} -- nothing to replay") + return + + sample = cts_factors_mod.replay_every_bar( + product_id, candles, warmup=warmup, window=window, step=step + ) + varying = sample.varying() + if not varying: + click.echo( + f"refused: n={sample.n} observations replayed and no factor varies (every factor " + "was constantly present or constantly absent) -- a constant vector has no " + "correlation to report" + ) + return + + stats = cts_factors_mod.pair_stats(sample, varying) + adjusted = cts_factors_mod.holm_adjust(stats, alpha=alpha) + clusters = cts_factors_mod.cluster_report(sample, adjusted) + variance = cts_factors_mod.variance_report(sample) + for line in _render_factors(product_id, sample, adjusted, clusters, variance): + click.echo(line) + + +# -- independence: are two rules actually independent? (§80.16) ---------------------------------- + + +def _render_independence( + rule_a_id: int, rule_b_id: int, report: independence_mod.IndependenceReport +) -> list[str]: + """Formatting only: every field is `independence.compare()`'s own output.""" + return [ + f"independence -- rule {rule_a_id} vs rule {rule_b_id} over {report.n_periods} " + "common bars (§80.16)", + f" active bars: rule {rule_a_id}={report.a_active} rule {rule_b_id}={report.b_active} " + f"both={report.both_active}", + f" jaccard overlap : {report.jaccard}", + f" position correlation : {report.position_correlation}", + f" pnl correlation : {report.pnl_correlation}", + f" median entry distance : {report.median_entry_distance}", + f" entry distances (n={len(report.entry_distances)}): {report.entry_distances}", + ] + + +@research_group.command("independence") +@click.option("--rule-a", "rule_a_id", required=True, type=int, help="First stored rule id.") +@click.option("--rule-b", "rule_b_id", required=True, type=int, help="Second stored rule id.") +@click.option( + "--granularity", + default=None, + help="Override both rules' candle granularity (default: each rule's own, else ONE_HOUR).", +) +@click.pass_context +def research_independence( + ctx: click.Context, rule_a_id: int, rule_b_id: int, granularity: str | None +) -> None: + """Are two rules (or two horizons of one rule) actually independent (independence.py, + §80.16)? Correlated rules inflate N without adding independent evidence (§73.5). + + Position and per-bar P&L vectors are built here over the two rules' COMMON bar index -- + mechanical bookkeeping, not a statistic: `compare()` has no opinion on how its input + vectors are assembled, only on what to compute once they are aligned onto one calendar. + """ + repo = _open_repo(ctx) + config = rules_mod._optional_cfg(ctx) + try: + resolved_a = rules_mod.resolve_rule_backtest( + repo, config, rule_a_id, granularity_opt=granularity + ) + resolved_b = rules_mod.resolve_rule_backtest( + repo, config, rule_b_id, granularity_opt=granularity + ) + except rules_mod.RulesRefused as exc: + raise click.ClickException(str(exc)) from exc + + result_a = rules_mod.backtest_resolved(resolved_a) + result_b = rules_mod.backtest_resolved(resolved_b) + + closed_a = [trade for trade in result_a.trades if trade.outcome != "open"] + closed_b = [trade for trade in result_b.trades if trade.outcome != "open"] + if not closed_a or not closed_b: + empty = [ + f"rule {rule_id}" + for rule_id, closed in ((rule_a_id, closed_a), (rule_b_id, closed_b)) + if not closed + ] + click.echo(f"refused: {' and '.join(empty)} closed no trades -- nothing to compare") + return + + # The COMMON bar index: the intersection of both rules' cached candle timestamps, ascending + # (plain `sorted()`, no key -- a chronological ordering, never a ranking). See the docstring + # above: this block is bookkeeping, `compare()` still does every actual measurement. + ts_a = {candle.ts for candle in resolved_a.candles} + ts_b = {candle.ts for candle in resolved_b.candles} + common_ts = sorted(ts_a & ts_b) + if not common_ts: + click.echo( + f"refused: rule {rule_a_id} and rule {rule_b_id} share no common bar timestamps " + "-- nothing to compare" + ) + return + index_of = {ts: i for i, ts in enumerate(common_ts)} + n = len(common_ts) + + def _vectors(trades: list[Any]) -> tuple[list[int], list[Decimal], list[int]]: + positions = [0] * n + pnl = [Decimal(0)] * n + entries: list[int] = [] + for trade in trades: + start = index_of.get(trade.entry_ts) + if start is None: + continue + end = n - 1 if trade.exit_ts is None else index_of.get(trade.exit_ts, n - 1) + entries.append(start) + for i in range(start, end + 1): + positions[i] = 1 + if trade.pnl is not None: + pnl[end] += trade.pnl + return positions, pnl, entries + + pos_a, pnl_a, entries_a = _vectors(result_a.trades) + pos_b, pnl_b, entries_b = _vectors(result_b.trades) + + if not any(pos_a) or not any(pos_b): + click.echo( + f"refused: rule {rule_a_id} and rule {rule_b_id} never occupy the common bar " + "index -- nothing to compare" + ) + return + + report = independence_mod.compare(pos_a, pos_b, pnl_a, pnl_b, entries_a, entries_b) + for line in _render_independence(rule_a_id, rule_b_id, report): + click.echo(line) + + # -- aliases: the five commands `keel trials`/`keel rules` already front ------------------------ # # These are the SAME click command objects registered a second time, via `Group.add_command`, From 16c925df66a54bdf2530cc3e4658e33f8d7f4153 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Fri, 28 Aug 2026 20:37:43 -0400 Subject: [PATCH 5/9] test(research): tighten the completeness pin, pin the optuna-free import, pin every refusal (#601) Three additions to tests/commands/test_research_front_door.py, all mutation-verified against a real break before being restored. The completeness pin only ever checked that a runs_as naming a docs/experiments/*.py driver pointed at a file that EXISTS, never that it was the file that actually drives the claimed module -- which is exactly how throughput.py's runs_as was able to silently point at the pooled-review driver instead of its own for as long as it did, and the old pin would not have caught it. test_every_runs_as_resolves now also AST-parses a docs/experiments driver and asserts it imports keel.research. by name (import or from-import, never a text grep, so a docstring merely mentioning the module name in prose cannot satisfy it). Mutation-verified by reproducing the original bug on a different pair: pointing independence.py's runs_as at docs/experiments/2026-08-09-cts-factor-collinearity.py (a real, existing driver -- just the wrong one) failed with `AssertionError: docs/experiments/2026-08-09-cts-factor-collinearity.py exists but does not import keel.research.independence -- it is not actually the driver for independence.py`; reverted after confirming the failure. test_research_module_never_imports_optuna AST-scans keel/commands/research.py for any import of optuna, by name, at any scope -- optuna is a dev-only dependency (pyproject.toml) and the shipped CLI must import cleanly without it. Mutation-verified: adding `import optuna` at the top of the module (line 51) failed with `AssertionError: import optuna at keel/commands/research.py:51 -- optuna is a dev-only dependency and this module must import cleanly without it`; reverted. test_every_evidence_subcommand_can_refuse_on_stdout_and_exit_zero is issue #601's second acceptance criterion in test form. It enumerates research_group.commands dynamically -- never a hardcoded list of the six new names -- excludes `index` (a lookup over the front door's own table of contents, already pinned separately) and `lookahead` (a pass/fail diagnostic gate that fails loud on a real finding, the opposite of an evidence refusal, exactly as its own docstring says), and for every one of the remaining eleven ACTUALLY INVOKES it through the real CLI against a deliberately thin fixture db -- two rules with no cached candles, a third with five bars (too few for any real train/test window), and empty trade_outcomes/orders/ledger tables -- asserting exit 0 and "refus" on stdout. A subcommand with no declared fixture in _REFUSAL_ARGS fails the test immediately rather than being silently skipped, so a later eighth subcommand cannot pass this pin by omission. Mutation-verified: changing research_throughput's refusal branch from `click.echo(f"refused: {exc}"); return` to `raise click.ClickException(str(exc))` failed with `AssertionError: keel research throughput did not exit 0 on its refusal fixture: Error: pooled trades per month must be > 0` (`assert 1 == 0` -- ``); reverted. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01T6yA5khYnJ2qzheArRToQ2 --- tests/commands/test_research_front_door.py | 222 ++++++++++++++++++++- 1 file changed, 216 insertions(+), 6 deletions(-) diff --git a/tests/commands/test_research_front_door.py b/tests/commands/test_research_front_door.py index 1eb65f84..cbc8254c 100644 --- a/tests/commands/test_research_front_door.py +++ b/tests/commands/test_research_front_door.py @@ -31,6 +31,15 @@ order -- `RESEARCH_INDEX`'s own comment says so -- never a computed one, so this ban costs nothing real and closes the door completely. +* **The refusal pin** (`test_every_evidence_subcommand_can_refuse_on_stdout_and_exit_zero`, + Wave B). Issue #601's second acceptance criterion, in test form: every evidence + subcommand under `keel research` must be ABLE to answer "there is not enough evidence" + on stdout at exit 0, never a `ClickException`. The enumeration walks + `research_group.commands` itself -- never a hardcoded list -- so a future eighth + subcommand with no declared refusal fixture fails this test by construction rather than + silently going unchecked; see the docstring on the test itself for the two named + exclusions (`index`, `lookahead`) and why each is not an "insufficient evidence" case. + Every pin here was written, then deliberately broken, then restored -- see the commit message for the exact mutation and the exact failure text each produced. """ @@ -39,6 +48,7 @@ import ast import json +from decimal import Decimal from pathlib import Path import click @@ -55,6 +65,9 @@ ) from keel.commands.rules import rules_group from keel.commands.trials import trials_group +from keel.data.db import connect, migrate +from keel.data.repository import Repository +from keel.types import Candle, Granularity REPO_ROOT = Path(__file__).resolve().parents[2] RESEARCH_PKG = REPO_ROOT / "keel" / "research" @@ -107,14 +120,43 @@ def _resolve_cli_command(command_line: str) -> click.Command | None: return node +def _driver_imports_module(driver_path: Path, module_stem: str) -> bool: + """True when `driver_path` imports `keel.research.` -- by name, at module + scope (`import keel.research.X` or `from keel.research.X import ...`). AST-based rather + than a text search so a docstring that merely MENTIONS the module name (every driver's + module docstring names half the toolkit in prose) can never satisfy this check.""" + tree = ast.parse(driver_path.read_text(encoding="utf-8"), filename=str(driver_path)) + target = f"keel.research.{module_stem}" + for node in ast.walk(tree): + if isinstance(node, ast.ImportFrom) and node.module == target: + return True + if isinstance(node, ast.Import) and any(alias.name == target for alias in node.names): + return True + return False + + def test_every_runs_as_resolves(): """Every `runs_as` either names a real command reachable from `keel.cli.cli` (walked, - not guessed) or a `docs/experiments/*.py` driver that exists on disk -- never a string - that used to be true. - - Mutation-verified: pointing one entry's `runs_as` at `"keel research not-a-real-command"` - made this fail with `AssertionError: keel research not-a-real-command does not resolve - to a registered CLI command`; see the commit message for the exact diff and output. + not guessed) or a `docs/experiments/*.py` driver that exists on disk AND actually + imports the module it is named as the `runs_as` for -- never a string that used to be + true, and never a driver that happens to exist but drives a DIFFERENT module. + + The second half is Wave B's tightening (#601): before it, this test only checked that + the named driver FILE existed, which is exactly why `throughput.py`'s `runs_as` was able + to silently point at `docs/experiments/2026-09-30-pooled-review.py` -- the pooled-review + driver, not a throughput one -- and pass. `keel research throughput` closed that slip by + giving `throughput.py` a real command to name instead; this tightening is what makes + sure the NEXT such slip cannot pass silently again. + + Mutation-verified twice: (1) pointing one entry's `runs_as` at + `"keel research not-a-real-command"` made this fail with `AssertionError: keel research + not-a-real-command does not resolve to a registered CLI command`; (2) pointing + `independence.py`'s `runs_as` at `docs/experiments/2026-08-09-cts-factor-collinearity.py` + (a real, existing driver -- just the WRONG one, exactly `throughput.py`'s original bug) + made this fail with `AssertionError: docs/experiments/2026-08-09-cts-factor- + collinearity.py exists but does not import keel.research.independence -- it is not + actually the driver for independence.py`. See the commit message for both diffs and both + exact failures. """ for entry in RESEARCH_INDEX: if entry.runs_as.startswith("keel "): @@ -129,6 +171,11 @@ def test_every_runs_as_resolves(): f"{entry.runs_as} names a docs/experiments driver that does not exist on " f"disk (module={entry.module})" ) + module_stem = entry.module.removesuffix(".py") + assert _driver_imports_module(driver, module_stem), ( + f"{entry.runs_as} exists but does not import keel.research.{module_stem} " + f"-- it is not actually the driver for {entry.module}" + ) # -- pin (b): the Strathern rail survives the front door ----------------------------------------- @@ -294,3 +341,166 @@ def test_research_index_entry_is_a_frozen_dataclass_tuple(): assert isinstance(entry, ResearchModuleEntry) with pytest.raises(Exception): entry.module = "mutated.py" # type: ignore[misc] + + +# -- pin (c): `keel research tuning` never imports optuna at module scope ------------------------ + + +def test_research_module_never_imports_optuna(): + """`keel/research/tuning.py` imports optuna lazily, inside `run_study` only, so `import + keel.research.tuning` stays clean without it; `keel/commands/research.py` must hold to + the same discipline (#601's fourth bullet) -- optuna is a dev-only dependency + (`pyproject.toml`'s dev group) and the shipped CLI must import cleanly without it. + + AST-based (not a text `"optuna" in source` grep) so the word can still appear in prose -- + `research_tuning`'s own refusal message and docstring both name + `docs/experiments/2026-08-22-optuna-parameter-study.py` and the word "optuna" -- without + tripping this pin; only an actual `import`/`from ... import` statement does. + + Mutation-verified: adding `import optuna # noqa: F401` at the top of + `keel/commands/research.py` (line 51, right after `import json`) made this fail with + `AssertionError: import optuna at keel/commands/research.py:51 -- optuna is a dev-only + dependency and this module must import cleanly without it` before being removed again; + see the commit message for the exact diff. + """ + source = RESEARCH_MODULE_PATH.read_text(encoding="utf-8") + tree = ast.parse(source, filename=str(RESEARCH_MODULE_PATH)) + for node in ast.walk(tree): + if isinstance(node, ast.Import): + for alias in node.names: + assert alias.name.split(".")[0] != "optuna", ( + f"import optuna at {RESEARCH_MODULE_PATH}:{node.lineno} -- optuna is a " + "dev-only dependency and this module must import cleanly without it" + ) + if isinstance(node, ast.ImportFrom): + module = node.module or "" + assert module.split(".")[0] != "optuna", ( + f"from {module} import ... at {RESEARCH_MODULE_PATH}:{node.lineno} -- optuna " + "is a dev-only dependency and this module must import cleanly without it" + ) + + +# -- pin (d): the refusal pin ---------------------------------------------------------------------- +# +# Issue #601's second acceptance criterion. Every EVIDENCE subcommand under `keel research` +# must be able to answer "there is not enough evidence to answer that" on stdout, exit 0 -- +# never a `ClickException`. This test enumerates `research_group.commands` itself (never a +# hardcoded list of the six Wave B names) and actually invokes every one of them against a +# deliberately empty/underpowered fixture db -- it does not grep source for the word +# "refused"; it runs the command and reads what it printed. + + +def _refusal_fixture_db(tmp_path: Path) -> Path: + """One tiny db built to make EVERY evidence subcommand refuse: two stored `turtle_breakout` + rules with no cached candles (rules 1/2 -- `significance --from rule`, `monte-carlo`, + `independence`), and a third WITH a handful of candles too few for any real train/test + window (`walk-forward`'s own refusal needs candles to exist, just not enough of them). + `trade_outcomes`/`orders` stay empty (`significance --from deployment`, `pooled-review`). + """ + path = tmp_path / "refusal-fixture.db" + conn = connect(str(path)) + migrate(conn) + repo = Repository(conn) + now = 1_800_000_000 + repo.insert_rule("turtle_breakout", {"product_id": "BTC-USD"}, status="candidate", now_ts=now) + repo.insert_rule("turtle_breakout", {"product_id": "BTC-USD"}, status="candidate", now_ts=now) + rule_wf = repo.insert_rule( + "turtle_breakout", {"product_id": "ETH-USD"}, status="candidate", now_ts=now + ) + assert rule_wf == 3 + tiny_candles = [ + Candle( + ts=1_700_000_000 + i * 86400, + open=Decimal("100"), + high=Decimal("101"), + low=Decimal("99"), + close=Decimal("100"), + volume=Decimal("1"), + ) + for i in range(5) + ] + repo.upsert_candles("ETH-USD", Granularity.ONE_DAY, tiny_candles) + conn.close() + return path + + +#: Per-subcommand argv (after `research `) that drives it into an ACTUAL refusal +#: against `_refusal_fixture_db` -- declared, not derived, so a subcommand added later with +#: no entry here fails this test immediately via the `KeyError` in the loop below, exactly +#: the "a seventh subcommand added later that cannot refuse must fail this pin" the issue +#: asks for. +_REFUSAL_ARGS: dict[str, tuple[str, ...]] = { + "significance": ("--from", "deployment"), + "pooled-review": ("--db", "{db}"), + "throughput": ("--venues-json", "[]"), + "tuning": ("--run",), + "factors": ("--product", "NO-SUCH-PRODUCT"), + "independence": ("--rule-a", "1", "--rule-b", "2", "--granularity", "ONE_DAY"), + "pbo": ("--ledger", "{tmp}/empty-trials.jsonl"), + "deflate": ("--ledger", "{tmp}/empty-trials.jsonl", "--sharpe", "1.0"), + "monte-carlo": ( + "--rule", "1", "--seed", "1", "--ledger", "{tmp}/mc-trials.jsonl", + ), + "walk-forward": ( + "--rule", "3", "--train-bars", "1000", "--test-bars", "1000", + "--ledger", "{tmp}/wf-trials.jsonl", + ), +} + +#: Subcommands under `keel research` that are NOT "insufficient evidence" refusals, with the +#: reason each is excluded named right here rather than folded silently into the loop below. +_REFUSAL_PIN_EXCLUDED: dict[str, str] = { + "index": ( + "a lookup over the front door's own table of contents, never a measurement over " + "evidence; its own unknown-`--module` refusal is pinned separately by " + "`test_index_unknown_module_refuses_but_exits_zero` above" + ), + "lookahead": ( + "a pass/fail DIAGNOSTIC gate (issue #440), not an #601 evidence refusal: its own " + "docstring states it 'exits 1 on LOOKAHEAD DETECTED ... like `keel doctor`' -- a " + "real finding it fails loud on, never a 'not enough evidence' result on stdout" + ), +} + + +def test_every_evidence_subcommand_can_refuse_on_stdout_and_exit_zero(tmp_path): + """#601's second acceptance criterion. See the module docstring's pin (d) and the two + module-level tables above for the fixtures and the named exclusions. + + Mutation-verified: changing `research_throughput`'s refusal branch from `click.echo(f"refused: + {exc}"); return` to `raise click.ClickException(str(exc))` made this fail with + `AssertionError: keel research throughput did not exit 0 on its refusal fixture: Error: + pooled trades per month must be > 0` (`assert 1 == 0` -- ``) before + being reverted; see the commit message for the exact diff and the exact failure output. + """ + db_path = _refusal_fixture_db(tmp_path) + config_path = tmp_path / "missing-config.yaml" # never created: config degrades to default + + checked: set[str] = set() + for name in research_group.commands: + if name in _REFUSAL_PIN_EXCLUDED: + continue + checked.add(name) + assert name in _REFUSAL_ARGS, ( + f"keel research {name} has no declared refusal fixture in _REFUSAL_ARGS -- add " + "one (or a named exclusion in _REFUSAL_PIN_EXCLUDED) before this subcommand can " + "be trusted to refuse rather than crash on thin evidence" + ) + argv = [ + arg.format(db=str(db_path), tmp=str(tmp_path)) for arg in _REFUSAL_ARGS[name] + ] + result = CliRunner().invoke( + cli, + ["--db", str(db_path), "--config", str(config_path), "research", name, *argv], + ) + assert result.exit_code == 0, ( + f"keel research {name} did not exit 0 on its refusal fixture: {result.output}" + ) + assert "refus" in result.output.lower(), ( + f"keel research {name} exited 0 but printed no refusal on its underpowered " + f"fixture: {result.output!r}" + ) + + # The dynamic enumeration is the point (#601): every non-excluded name click actually + # registered under `research_group` was exercised above, not a hardcoded subset of it. + assert checked == set(research_group.commands) - set(_REFUSAL_PIN_EXCLUDED) From 8a4517f32b18055b1200e25dc972578d90a37ae6 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Fri, 28 Aug 2026 20:37:53 -0400 Subject: [PATCH 6/9] docs(research): the six subcommands replace their docs/experiments placeholders (#601) docs/research-toolkit.md was written and verified claim-by-claim against Wave A's surface, which deliberately did not include the six new subcommands yet -- so it named them all as reachable only through `keel research index --module NAME` and a docs/experiments/ driver. Wave B made all six real, and the page's own "run it with" column was the one part of it that had gone stale the moment they landed. Four edits, exactly: the six "run it with" cells in the at-a-glance table now name the real subcommand instead of the index lookup, and the footnote explaining why they didn't exist yet is gone with them; significance.py's own "Run:" line names the real --from deployment/--from rule split instead of "no dedicated subcommand yet"; and the 2026-09-30 pooled review section's closing paragraph -- which used to say pooled_review.py had no subcommand either -- now describes `keel research pooled-review` itself, while the paragraph above it about the driver's stderr/exit-2 contract being the prior art the command deliberately broke with moves to past tense (it already happened) but is otherwise untouched, because it was already the correct design rationale. Nothing else on the page was rewritten -- it was good, and it was verified claim by claim. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01T6yA5khYnJ2qzheArRToQ2 --- docs/research-toolkit.md | 60 +++++++++++++++++++--------------------- 1 file changed, 29 insertions(+), 31 deletions(-) diff --git a/docs/research-toolkit.md b/docs/research-toolkit.md index 97ac8a3e..45ee22b3 100644 --- a/docs/research-toolkit.md +++ b/docs/research-toolkit.md @@ -28,28 +28,19 @@ carefully as the "answers" column — that column is the actual product. | module | answers | cannot answer | run it with | | :--- | :--- | :--- | :--- | -| `significance.py` | is a family's edge distinguishable from its break-even, priced at the fee actually paid | whether the edge will hold going forward, or which family/regime to report — you name both | `keel research index --module significance`\* | +| `significance.py` | is a family's edge distinguishable from its break-even, priced at the fee actually paid | whether the edge will hold going forward, or which family/regime to report — you name both | `keel research significance` | | `montecarlo.py` | was the equity curve's *path* (drawdown, time underwater) unusual for this set of trades, in some order or under resampled price history | whether the *final equity* was luck — that percentile is exactly 1/2 by construction, always | `keel research monte-carlo` (`keel trials monte-carlo`) | | `cscv.py` | the probability a configuration selected in-sample degrades out-of-sample, over a matrix of configurations already tried | which configuration is the best one — it never returns that, by construction | `keel research pbo` (`keel trials pbo`) | | `deflate.py` | given N trials tried, the Sharpe bar the winner had to clear, and how much data that needs | what N and correlation to assume — it reports a band across assumptions rather than guess one | `keel research deflate` (`keel trials deflate`) | | `walkforward.py` | does a GIVEN fixed parameter set hold up across rolling train/test windows, and does it degrade | which parameter set, fold or window is best — none is ever computed | `keel research walk-forward` (`keel trials walk-forward`) | -| `independence.py` | how much two rules' (or two horizons') signals overlap in time, position and P&L | whether either rule is profitable, or which one to keep | `keel research index --module independence`\* | -| `throughput.py` | how much volume a fee-free allowance can honestly carry this month, and how long evidence takes to accumulate | it never enlarges an allowance to fit a plan — a product that doesn't fit is deferred, not squeezed in | `keel research index --module throughput`\* | -| `cts_factors.py` | do the 11 CTS confluence factors carry independent evidence, or is one momentum read counted three times | the biased ("obvious") conditional sample is computed but never allowed to carry the headline | `keel research index --module cts_factors`\* | -| `tuning.py` | for a declared parameter space, does a train/held-out study produce a candidate clearing held-out sign AND PBO ≤ 0.5 | it never auto-tunes a live/paper profile, and a pass is a hypothesis, not a promotion | `keel research index --module tuning`\* | +| `independence.py` | how much two rules' (or two horizons') signals overlap in time, position and P&L | whether either rule is profitable, or which one to keep | `keel research independence` | +| `throughput.py` | how much volume a fee-free allowance can honestly carry this month, and how long evidence takes to accumulate | it never enlarges an allowance to fit a plan — a product that doesn't fit is deferred, not squeezed in | `keel research throughput` | +| `cts_factors.py` | do the 11 CTS confluence factors carry independent evidence, or is one momentum read counted three times | the biased ("obvious") conditional sample is computed but never allowed to carry the headline | `keel research factors` | +| `tuning.py` | for a declared parameter space, does a train/held-out study produce a candidate clearing held-out sign AND PBO ≤ 0.5 | it never auto-tunes a live/paper profile, and a pass is a hypothesis, not a promotion | `keel research tuning` | | `bias.py` | does a rule's decision at bar N change when bars after N become visible (lookahead / recursive drift) | whether the rule is profitable — this is about information leakage only | `keel research lookahead` (`keel rules lookahead`) | | `ledger.py` | what experiments were run, in what order, tamper-evidently | it is tamper-*evident*, not tamper-*proof*, and it never touches money | `keel trials record` / `list` / `verify` (no `research` alias — see below) | | `matrix.py` | assembles the T×N matrix `cscv.py` needs from ledger trials, enforcing the "true matrix" condition | anything about performance itself — it is plumbing, not a question of its own | no direct command — runs inside `keel research pbo` | -| `pooled_review.py` | the #427 pooled-review machinery: descriptive n_eff-corrected intervals, never a verdict on the edge | it renders no pass/fail on the edge, ever — see [the 2026-09-30 review](#the-2026-09-30-pooled-review-427) below | `keel research index --module pooled_review`\* | - -\* These six modules have no dedicated `keel research` subcommand of their own yet — only -`keel research index`, which names every module, and the five aliases above, are wired as of -this writing. Asking the index for one module by name (`--module NAME`, the bare filename minus -`.py`) prints its `runs as` line, which today names the pre-registered `docs/experiments/` -driver that exercises it — e.g. `keel research index --module significance` names -`docs/experiments/2026-08-21-rule-family-significance.py`. Until each of these six gets its own -subcommand, that driver (or a direct `import keel.research.` call, as this page does -below) is how you actually run one. +| `pooled_review.py` | the #427 pooled-review machinery: descriptive n_eff-corrected intervals, never a verdict on the edge | it renders no pass/fail on the edge, ever — see [the 2026-09-30 review](#the-2026-09-30-pooled-review-427) below | `keel research pooled-review` | `ledger.py` and `matrix.py` are the two modules that were never going to get a `keel research` alias in the first place: `ledger.py`'s record-keeping commands (`record`/`list`/`verify`) stay @@ -78,9 +69,10 @@ every subcommand run names both explicitly. It also will not manufacture power a have: an underpowered result is reported as "not distinguishable from zero," not massaged into significance by choosing a friendlier n. -Run: no dedicated subcommand yet — `keel research index --module significance` names the -pre-registered driver, `docs/experiments/2026-08-21-rule-family-significance.py`; the transcript -below calls the module directly. +Run: `keel research significance` — `--from deployment` (the default) reads this db's own +`trade_outcomes` ledger, `--from rule --rule ID` backtests one stored rule instead; either way +`--fee-regime` restricts to one regime, and omitting it runs both, never an average. The +transcript below was captured by calling the module directly, before this subcommand existed. ### `montecarlo.py` — trade reshuffling and candle bootstrap, and the invariant Jesse's marketing skips @@ -249,16 +241,22 @@ report: `is_refused`/`descriptive_review` in `keel/research/pooled_review.py` pr `DescriptiveReview` whose `refusal` is a tuple of reasons instead of a report, and `render_report` raises if asked to render one anyway — a refused review has no report to print. -That refusal is exactly where the standalone driver is the prior art any future -`keel research pooled-review` subcommand must deliberately break with, not the pattern to copy: -`docs/experiments/2026-09-30-pooled-review.py` prints its refusal to **stderr** and calls -`sys.exit(2)` when a pre-registered profile database isn't reachable. A front-door command must -not repeat that — under the rule stated above, a refusal belongs on stdout at exit 0, because -"nothing to review" is this command answering a well-formed question honestly, not the command -failing to run. - -As of this writing `pooled_review.py` has no dedicated `keel research` subcommand either — -`keel research index --module pooled_review` names the same driver, and running the review -today still means running `docs/experiments/2026-09-30-pooled-review.py` directly, stderr/exit-2 -refusal included. Before 2026-09-30 it runs the same machinery as a labelled preview, and it -says so. +That refusal is exactly where the standalone driver is the prior art `keel research +pooled-review` deliberately broke with, not the pattern to copy: `docs/experiments/2026-09-30- +pooled-review.py` prints its refusal to **stderr** and calls `sys.exit(2)` when a pre-registered +profile database isn't reachable. The front-door command does not repeat that — under the rule +stated above, a refusal belongs on stdout at exit 0, because "nothing to review" is this command +answering a well-formed question honestly, not the command failing to run. + +`keel research pooled-review` is that front door: `--db` (repeatable, defaulting to the same +three pre-registered profiles the driver defaults to), `--run-date`, and optional `--out`/ +`--jsonl` to also write the artifacts. It shares its `_connect_ro`/`read_orders`/`read_ledger` +readers with the driver — moved into `keel/commands/research.py` and imported back by the +driver — so there is exactly one reader of a deployment database and the two structurally +cannot diverge on what "the pool" means. The driver's own stderr/exit-2 contract is unchanged +and stays that way: it was pre-registered before the review event and is frozen, while the +command is new surface that gets the right behaviour — stdout, exit 0 — from the start. Before +2026-09-30 both run the same machinery as a labelled preview, and both say so. + +Running the review directly against `docs/experiments/2026-09-30-pooled-review.py` still works +unchanged, stderr/exit-2 refusal included — it remains the pre-registered driver of record. From b79157e0fb816f5b0abb477327e3ef305403f5b5 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Fri, 28 Aug 2026 20:57:12 -0400 Subject: [PATCH 7/9] test(research): happy-path coverage for the six Wave B subcommands (#601) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tests/commands/test_research_front_door.py is a pins file: its refusal test deliberately drives every evidence subcommand into refusing on a thin fixture, and nothing else in the suite ever ran significance, pooled-review, throughput, tuning, factors or independence against a fixture that made them actually compute something. The implementer verified each by hand and pasted the output into a report, but a hand-run is not regression coverage -- nothing would have noticed if factors started printing an empty report, or if significance silently stopped rendering one of its two fee regimes. This adds tests/commands/test_research_commands.py, eleven tests that reuse the front-door file's CliRunner + fixture-db house pattern and assert on what each subcommand PRINTS, not merely that it exits 0: - significance: a turtle rule's own backtest (--from rule) and a seeded trade_outcomes ledger (--from deployment) each render BOTH fee regimes (never an average of them) with raw n printed beside n_eff, per #427. - pooled-review: two profile dbs, one carrying a round trip recorded both as a matched orders pair and as its trade_outcomes twin, so the pool is a genuine union across files and the dedup has something real to collapse (pooled 5, not 6; "1 deduped" in the rendered composition table). Asserts the #427 power sentence's exact wording ("can only see an edge of ... points or larger"), that --out/--jsonl write the files asked for, and -- because this command points at live deployment databases -- that the two profile .db files hash byte-identical before and after a full run including both write flags. - throughput: a --venues-json/--products-json/--allowances-json payload where one product fits a venue's allowance and a second deliberately does not, asserting the allocator enables the one that fits, defers the one that doesn't, and that spend never exceeds the venue's allowance. - tuning: the declared search spaces render with their derived cell counts (read off SEARCH_SPACES, not hardcoded), and an --explored-json box inside turtle_breakout's declared bounds reports as explored rather than refusing. - factors: 400 bars of a deterministic random walk (the same generator tests/research/test_cts_factors.py uses) produce a varying sample whose pairwise/cluster/variance sections all render. - independence: two turtle_breakout rules with different lookbacks over the same cached candles render all five §80.16 figures (jaccard, position correlation, pnl correlation, entry distances). The most important addition is test_no_evidence_subcommand_names_a_winner. The Strathern rail (cscv.py/deflate.py/walkforward.py -- a score may report, and may gate, but may NEVER be a sweep's ranking key) was pinned at the source level twice already: an AST scan of keel/commands/research.py in the front-door pins file, and a source scan of walkforward.py itself in test_walkforward.py. Neither ever ran a command and read its stdout. This test drives keel research pbo/deflate/walk-forward -- the three rail-bearing aliases -- through fixtures that make each SUCCEED (a refusal has nothing to rank, so it would not exercise a renderer's word choice) and asserts none of their rendered output contains "best", "winner", "optimal" or "top-ranked". The first three mirror test_walkforward.py's own test_refusal_to_rank_enforced_by_source_scan word list exactly; top-ranked is added per this issue's own ask. Mutation-verified: added `" best: fold 0",` as a line in walkforward.render_lines (right after the not-a-ranking note), ran this file's new test, and got: AssertionError: keel research walk-forward printed ranking word 'best' assert 'best' not in 'walk-forwar...38ec6318b1\n' with the diff showing the injected "best: fold 0" line landing between the not-a-ranking note and the per-fold table. Reverted immediately after confirming the failure; `git diff keel/research/walkforward.py` is empty in this commit. No bug found in the six subcommands -- every one of them, run against a real fixture, printed exactly what its own docstring says it should. `uv run pytest -q` (4466 passed, 3 skipped), `uv run ruff check keel tests packages` and `uv run mypy` are both clean. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01T6yA5khYnJ2qzheArRToQ2 --- tests/commands/test_research_commands.py | 597 +++++++++++++++++++++++ 1 file changed, 597 insertions(+) create mode 100644 tests/commands/test_research_commands.py diff --git a/tests/commands/test_research_commands.py b/tests/commands/test_research_commands.py new file mode 100644 index 00000000..12aaaa9f --- /dev/null +++ b/tests/commands/test_research_commands.py @@ -0,0 +1,597 @@ +"""Behavioural tests for the six Wave B subcommands of `keel research` (issue #601): +`significance`, `pooled-review`, `throughput`, `tuning`, `factors`, `independence`. + +`tests/commands/test_research_front_door.py` is a PINS file -- architectural invariants +(completeness, the Strathern-rail AST scan, the refusal pin) that hold whether or not any +subcommand ever succeeds. It deliberately never exercises a subcommand's SUCCESS path: its +own refusal fixture is built to make every one of them refuse. That left a real gap -- a +regression that made `factors` print an empty report, or made `significance` silently drop +one of its two fee regimes, would not fail a single test in this suite. This file closes +that gap: every test here seeds a fixture that makes a subcommand actually COMPUTE +something, invokes it through the real CLI (the same `CliRunner` + fixture-db machinery +`test_research_front_door.py` and `tests/research/test_trials_cli.py` already use), and +asserts on what it PRINTED -- not merely that it exited 0. + +The last test in this file, `test_no_evidence_subcommand_names_a_winner`, is the one that +matters most: the Strathern rail (`cscv.py`/`deflate.py`/`walkforward.py` -- a score may +report, and may gate, but may NEVER be a sweep's ranking key) is pinned at the SOURCE level +by an AST scan in the front-door pins file, but nothing before this checked the RENDERED +output of a rail-bearing command actually run through the CLI. Mutation-verified: see the +commit message for the exact renderer edit, the failure it produced, and the revert. +""" + +from __future__ import annotations + +import hashlib +import json +import re +from decimal import Decimal +from pathlib import Path + +from click.testing import CliRunner + +from keel.cli import cli +from keel.data.db import connect, migrate +from keel.data.repository import Repository +from keel.research import ledger as trials_ledger +from keel.research import tuning as tuning_mod +from keel.types import Candle, Granularity + +MISSING_CONFIG_NAME = "missing-config.yaml" # never created: config degrades to the default + + +def _missing_config(tmp_path: Path) -> Path: + return tmp_path / MISSING_CONFIG_NAME + + +def _invoke(runner: CliRunner, db: Path, tmp_path: Path, *args: str): + """The house pattern (`test_research_front_door.py`'s refusal test, `test_trials_cli.py`'s + `_invoke_mc`): a real `--db`, and a `--config` pointing at a path that does not exist so + the fee degrades to the library default instead of loading whatever deployment config + happens to surround the test run.""" + return runner.invoke( + cli, ["--db", str(db), "--config", str(_missing_config(tmp_path)), *args] + ) + + +# -- shared candle fixtures ------------------------------------------------------------------- + + +def _sawtooth_candles(n: int, *, start: int = 1_700_000_000) -> list[Candle]: + """`n` daily bars in an asymmetric 19-bar sawtooth -- lifted from + `tests/research/test_trials_cli.py::_mc_candles`: an 8-bar rally, a 9-bar crash, a 2-bar + drift, then again, so a turtle rule both enters and gets stopped out AND its closed P&L + comes out mixed-sign (wins and losses in one run).""" + candles = [] + price = Decimal(100) + for i in range(n): + phase = i % 19 + if phase < 8: + price += Decimal(4) + elif phase < 17: + price -= Decimal(9) + else: + price -= Decimal(1) + open_ = price + close = price + (Decimal("1.5") if i % 2 else Decimal("-1.5")) + candles.append( + Candle( + ts=start + i * 86400, + open=open_, + high=max(open_, close) + Decimal(1), + low=min(open_, close) - Decimal(1), + close=close, + volume=Decimal("10"), + ) + ) + return candles + + +_TURTLE_A_PARAMS = { + "product_id": "BTC-USD", + "entry_lookback": 5, + "exit_lookback": 3, + "atr_period": 5, + "atr_stop_mult": "2", +} +_TURTLE_B_PARAMS = { + "product_id": "BTC-USD", + "entry_lookback": 8, + "exit_lookback": 4, + "atr_period": 6, + "atr_stop_mult": "2", +} + + +def _turtle_db(tmp_path: Path, *, name: str = "turtle.db", bars: int = 96) -> Path: + """One turtle_breakout rule (`_TURTLE_A_PARAMS`) over `_sawtooth_candles(bars)`, which + closes a mix of wins and losses (verified: 2 wins + 2 losses at `bars=96`).""" + path = tmp_path / name + conn = connect(str(path)) + migrate(conn) + repo = Repository(conn) + repo.insert_rule("turtle_breakout", _TURTLE_A_PARAMS, status="candidate", now_ts=1_800_000_000) + repo.upsert_candles("BTC-USD", Granularity.ONE_DAY, _sawtooth_candles(bars)) + conn.close() + return path + + +def _factor_candles(count: int, seed: int = 7) -> list[Candle]: + """A pseudo-random but deterministic OHLCV walk -- lifted from + `tests/research/test_cts_factors.py::_candles` (the module's own house fixture for a + sample with varying factor presence).""" + import random + + rng = random.Random(seed) + price = 100.0 + out: list[Candle] = [] + for index in range(count): + price = max(1.0, price * (1 + rng.uniform(-0.03, 0.03))) + high = price * (1 + abs(rng.uniform(0, 0.02))) + low = price * (1 - abs(rng.uniform(0, 0.02))) + out.append( + Candle( + ts=1_600_000_000 + index * 86_400, + open=Decimal(f"{rng.uniform(low, high):.2f}"), + high=Decimal(f"{high:.2f}"), + low=Decimal(f"{low:.2f}"), + close=Decimal(f"{price:.2f}"), + volume=Decimal("1000"), + ) + ) + return out + + +def _file_hash(path: Path) -> str: + return hashlib.sha256(path.read_bytes()).hexdigest() + + +# == significance =============================================================================== + + +def test_significance_from_rule_renders_both_fee_regimes_with_n_eff(tmp_path): + """`--from rule` backtests one stored rule and prices its closed trades at BOTH fee + regimes -- significance.py's own rule is that they are never averaged. Raw n and n_eff + must both be printed, side by side, never n_eff alone and never raw n alone (#427).""" + db = _turtle_db(tmp_path) + result = _invoke( + CliRunner(), db, tmp_path, + "research", "significance", "--from", "rule", "--rule", "1", + ) + assert result.exit_code == 0, result.output + assert "outside_allowance_taker" in result.output + assert "inside_allowance_fee_free" in result.output + # Raw n beside n_eff, never one without the other (#427's rule, stated in render_family). + assert re.search(r"closed trades n=\d+ pooled -> [\d.]+ effective", result.output) + assert "verdict:" in result.output + + +def test_significance_from_deployment_renders_both_fee_regimes(tmp_path): + """`--from deployment` reads this db's own `trade_outcomes` ledger -- the same + classifier `pooled-review` uses (`ledger_round_trips` + `RoundTrip.outcome()`).""" + db = tmp_path / "deployment.db" + conn = connect(str(db)) + migrate(conn) + repo = Repository(conn) + rows = [ + dict( + product_id="BTC-USD", rule_name="turtle_breakout", is_dca=False, + opened_at=1000, closed_at=2000, qty=Decimal("1"), + entry_fill=Decimal("100"), exit_fill=Decimal("110"), + fees=Decimal("1"), pnl_net=Decimal("8"), + ), + dict( + product_id="ETH-USD", rule_name="turtle_breakout", is_dca=False, + opened_at=3000, closed_at=4000, qty=Decimal("1"), + entry_fill=Decimal("200"), exit_fill=Decimal("190"), + fees=Decimal("1"), pnl_net=Decimal("-11"), + ), + dict( + product_id="SOL-USD", rule_name="turtle_breakout", is_dca=False, + opened_at=5000, closed_at=6000, qty=Decimal("2"), + entry_fill=Decimal("50"), exit_fill=Decimal("55"), + fees=Decimal("1"), pnl_net=Decimal("9"), + ), + ] + for row in rows: + repo.insert_trade_outcome(row) + conn.close() + + result = _invoke(CliRunner(), db, tmp_path, "research", "significance", "--from", "deployment") + assert result.exit_code == 0, result.output + assert "deployment @ outside_allowance_taker" in result.output + assert "deployment @ inside_allowance_fee_free" in result.output + assert "closed trades n=3 pooled ->" in result.output + + +# == pooled-review =============================================================================== + + +def _pooled_review_dbs(tmp_path: Path) -> tuple[Path, Path]: + """Two profile dbs. `db1` carries one round trip recorded BOTH as a matched + orders pair (BUY 100 -> SELL 110) AND as the equivalent `trade_outcomes` row -- so + `build_sample`'s dedup actually has a twin to collapse -- plus one ledger-only loss. + `db2` carries three more ledger-only trips, so the pool is a genuine UNION across two + files, not just one db's own rows. Every connection is checkpointed and closed before + return so the on-disk `.db` file (not the WAL sidecars) is a clean, hashable snapshot. + """ + db1 = tmp_path / "profile-db1.db" + conn = connect(str(db1)) + migrate(conn) + repo = Repository(conn) + repo.insert_rule( + "turtle_breakout", {"product_id": "BTC-USD"}, status="candidate", now_ts=1_800_000_000 + ) + repo.insert_order( + dict( + mode="paper", product_id="BTC-USD", side="BUY", order_type="market", + qty=Decimal("1"), limit_price=None, status="filled", fee=Decimal("1"), + expected_fill=Decimal("100"), actual_fill=Decimal("100"), + filled_quantity=Decimal("1"), raw_response=None, confirmation="auto", + rule_id=1, created_at=1000, updated_at=1000, + ) + ) + repo.insert_order( + dict( + mode="paper", product_id="BTC-USD", side="SELL", order_type="market", + qty=Decimal("1"), limit_price=None, status="filled", fee=Decimal("1"), + expected_fill=Decimal("110"), actual_fill=Decimal("110"), + filled_quantity=Decimal("1"), raw_response=None, confirmation="auto", + rule_id=1, created_at=2000, updated_at=2000, + ) + ) + repo.insert_trade_outcome( + dict( + product_id="BTC-USD", rule_name="turtle_breakout", is_dca=False, + opened_at=1000, closed_at=2000, qty=Decimal("1"), + entry_fill=Decimal("100"), exit_fill=Decimal("110"), + fees=Decimal("1"), pnl_net=Decimal("8"), + ) + ) + repo.insert_trade_outcome( + dict( + product_id="ETH-USD", rule_name="turtle_breakout", is_dca=False, + opened_at=3000, closed_at=4000, qty=Decimal("1"), + entry_fill=Decimal("200"), exit_fill=Decimal("190"), + fees=Decimal("1"), pnl_net=Decimal("-11"), + ) + ) + conn.execute("PRAGMA wal_checkpoint(TRUNCATE)") + conn.close() + + db2 = tmp_path / "profile-db2.db" + conn = connect(str(db2)) + migrate(conn) + repo = Repository(conn) + for opened, closed, entry, exit_, pnl in ( + (5000, 6000, "50", "55", "9"), + (7000, 8000, "55", "60", "9"), + (9000, 10000, "60", "58", "-5"), + ): + repo.insert_trade_outcome( + dict( + product_id="SOL-USD", rule_name="turtle_breakout", is_dca=False, + opened_at=opened, closed_at=closed, qty=Decimal("2"), + entry_fill=Decimal(entry), exit_fill=Decimal(exit_), + fees=Decimal("1"), pnl_net=Decimal(pnl), + ) + ) + conn.execute("PRAGMA wal_checkpoint(TRUNCATE)") + conn.close() + return db1, db2 + + +def test_pooled_review_pools_across_profiles_dedups_and_states_the_power_sentence(tmp_path): + db1, db2 = _pooled_review_dbs(tmp_path) + result = CliRunner().invoke( + cli, + [ + "research", "pooled-review", + "--db", str(db1), "--db", str(db2), + "--run-date", "2026-08-28", + ], + ) + assert result.exit_code == 0, result.output + + # Pooling: 2 trips from db1 (one deduped) + 3 from db2 -> 5 pooled, not 6 -- the dedup + # actually removed the orders-derived twin of the ledger row it matched. + assert "**pooled** | — | **5**" in result.output + assert "(1 deduped this" in result.output + + # #427's power sentence, exact wording from `pooled_review.power_sentence`. + assert "can only see an edge of" in result.output + assert "points or larger" in result.output + + # No pass/fail verdict on the edge anywhere in this report (its own closing section). + assert "This is not a pass/fail gate" in result.output + + +def test_pooled_review_out_and_jsonl_write_the_files_asked_for(tmp_path): + db1, db2 = _pooled_review_dbs(tmp_path) + out_path = tmp_path / "report.md" + jsonl_path = tmp_path / "report.jsonl" + result = CliRunner().invoke( + cli, + [ + "research", "pooled-review", + "--db", str(db1), "--db", str(db2), + "--run-date", "2026-08-28", + "--out", str(out_path), + "--jsonl", str(jsonl_path), + ], + ) + assert result.exit_code == 0, result.output + assert out_path.is_file() + assert "can only see an edge of" in out_path.read_text() + + assert jsonl_path.is_file() + row = json.loads(jsonl_path.read_text()) + assert row["pooled_n"] == 5 + assert row["counted_n"] == 5 + assert "can only see an edge of" in row["power_sentence"] + + +def test_pooled_review_never_writes_to_the_profile_dbs(tmp_path): + """This command points at live deployment databases (`DEFAULT_POOLED_REVIEW_DBS`); every + read goes through `_connect_ro` (`mode=ro`). Assert the guarantee mechanically: hash the + two profile `.db` files before and after a full run (report + `--out` + `--jsonl`), and + require them byte-identical. WAL sidecars (`-wal`/`-shm`) are excluded from the hash -- + a reader opening a WAL-mode db legitimately creates/touches those, but a `mode=ro` + connection is structurally unable to write a new page into the main file itself, which + is the property that actually matters here.""" + db1, db2 = _pooled_review_dbs(tmp_path) + before = (_file_hash(db1), _file_hash(db2)) + + result = CliRunner().invoke( + cli, + [ + "research", "pooled-review", + "--db", str(db1), "--db", str(db2), + "--run-date", "2026-08-28", + "--out", str(tmp_path / "report.md"), + "--jsonl", str(tmp_path / "report.jsonl"), + ], + ) + assert result.exit_code == 0, result.output + + after = (_file_hash(db1), _file_hash(db2)) + assert before == after, "keel research pooled-review wrote to a profile db it must only read" + + +# == throughput ================================================================================== + + +def test_throughput_allocates_within_allowance_and_never_exceeds_it(tmp_path): + venues = [ + { + "venue": "coinbase", + "monthly_allowance": "1000", + "mean_trade_notional": "100", + "expected_signals_per_month": "5", + } + ] + products = [ + { + "symbol": "BTC-USD", + "venues": ["coinbase"], + "mean_trade_notional": "100", + "expected_signals_per_month": "3", + }, + { + "symbol": "ETH-USD", + "venues": ["coinbase"], + "mean_trade_notional": "100", + "expected_signals_per_month": "20", # deliberately too big to fit alongside BTC + }, + ] + allowances = {"coinbase": "1000"} + + result = CliRunner().invoke( + cli, + [ + "research", "throughput", + "--venues-json", json.dumps(venues), + "--products-json", json.dumps(products), + "--allowances-json", json.dumps(allowances), + ], + ) + assert result.exit_code == 0, result.output + assert "allocation:" in result.output + + match = re.search( + r"coinbase \(cap (?P[\d.]+)/month\): enabled=(?P\[.*?\]) " + r"deferred=(?P\[.*?\]) spend=(?P[\d.]+)", + result.output, + ) + assert match is not None, result.output + assert "BTC-USD" in match.group("enabled") + assert "ETH-USD" in match.group("deferred") # too big to fit -- deferred, never squeezed in + # The allocator's non-negotiable (throughput.py's own docstring): a plan's spend never + # exceeds its venue's allowance. + assert Decimal(match.group("spend")) <= Decimal(match.group("cap")) + + +# == tuning ======================================================================================= + + +def test_tuning_reports_declared_search_spaces_with_cell_counts(tmp_path): + result = CliRunner().invoke(cli, ["research", "tuning", "--rule-kind", "turtle_breakout"]) + assert result.exit_code == 0, result.output + declared = tuning_mod.declared_cells("turtle_breakout") + assert f"turtle_breakout: declared search space ({declared} cells)" in result.output + for name, bounds in tuning_mod.SEARCH_SPACES["turtle_breakout"].items(): + assert f"{name}: {bounds}" in result.output + + +def test_tuning_explored_within_declared_bounds_reports_as_explored_not_refused(tmp_path): + explored = {"entry_lookback": [25, 40]} + result = CliRunner().invoke( + cli, + [ + "research", "tuning", + "--rule-kind", "turtle_breakout", + "--explored-json", json.dumps(explored), + ], + ) + assert result.exit_code == 0, result.output + assert "refused" not in result.output.lower() + check = tuning_mod.explored_vs_declared( + {name: (float(bounds[0]), float(bounds[1])) for name, bounds in explored.items()}, + "turtle_breakout", + ) + assert ( + f"explored {check.explored_cells} of {check.declared_cells} declared cells" + in result.output + ) + + +# == factors ====================================================================================== + + +def test_factors_renders_pairwise_cluster_and_variance_sections(tmp_path): + db = tmp_path / "factors.db" + conn = connect(str(db)) + migrate(conn) + repo = Repository(conn) + repo.upsert_candles("BTC-USD", Granularity.ONE_DAY, _factor_candles(400, seed=7)) + conn.close() + + result = _invoke( + CliRunner(), db, tmp_path, + "research", "factors", "--product", "BTC-USD", "--granularity", "ONE_DAY", + ) + assert result.exit_code == 0, result.output + assert "CTS factor collinearity -- BTC-USD" in result.output + assert "varying factor(s) of 11" in result.output + assert "pairwise (Holm-Bonferroni adjusted" in result.output + assert "pre-declared clusters" in result.output + assert "CTS total variance:" in result.output + + +# == independence ================================================================================ + + +def test_independence_renders_overlap_and_correlation_figures(tmp_path): + db = tmp_path / "independence.db" + conn = connect(str(db)) + migrate(conn) + repo = Repository(conn) + repo.insert_rule("turtle_breakout", _TURTLE_A_PARAMS, status="candidate", now_ts=1_800_000_000) + repo.insert_rule("turtle_breakout", _TURTLE_B_PARAMS, status="candidate", now_ts=1_800_000_000) + repo.upsert_candles("BTC-USD", Granularity.ONE_DAY, _sawtooth_candles(96)) + conn.close() + + result = _invoke( + CliRunner(), db, tmp_path, + "research", "independence", "--rule-a", "1", "--rule-b", "2", + ) + assert result.exit_code == 0, result.output + assert "independence -- rule 1 vs rule 2 over 96 common bars (§80.16)" in result.output + assert "jaccard overlap" in result.output + assert "position correlation" in result.output + assert "pnl correlation" in result.output + assert "median entry distance" in result.output + assert re.search(r"entry distances \(n=\d+\):", result.output) + + +# == the rail, at the rendered surface ========================================================== +# +# The Strathern rail (cscv.py/deflate.py/walkforward.py) is pinned at the SOURCE level in +# `test_research_front_door.py::test_research_module_never_sorts_ranks_or_maxes` (an AST scan +# of keel/commands/research.py) and at walkforward.py's own source in +# `tests/research/test_walkforward.py::test_refusal_to_rank_enforced_by_source_scan`. Neither +# ever runs a command and reads its stdout. This test does: it drives every `keel research` +# subcommand that fronts a rail-bearing module through a fixture that makes it actually +# SUCCEED (a refusal has nothing to rank in the first place, so it would not catch a +# regression), and asserts the same vocabulary ban +# `test_refusal_to_rank_enforced_by_source_scan` uses, over the rendered output this time. + + +def _pbo_ledger(tmp_path: Path) -> Path: + path = tmp_path / "pbo-trials.jsonl" + for column_index in range(6): + drift = Decimal(column_index) / Decimal(10) + series = [drift + (Decimal("10") if i % 2 else Decimal("-10")) for i in range(32)] + trials_ledger.append_trial( + path, + trial_id=f"grid-{column_index}", + session="grid", + rule="turtle_breakout", + params={"entry": 20 + column_index * 5}, + provenance="fitted", + kind="sweep_node", + decision="diagnostic_only", + per_bar_pnl=series, + timestamp=1_700_000_000, + ) + return path + + +def _deflate_ledger(tmp_path: Path) -> Path: + path = tmp_path / "deflate-trials.jsonl" + for i in range(5): + trials_ledger.append_trial( + path, + trial_id=f"t{i}", + session="s", + rule="turtle_breakout", + params={}, + provenance="fitted", + kind="sweep_node", + decision="selected", + series_missing=True, + timestamp=1_700_000_000 + i, + ) + return path + + +def test_no_evidence_subcommand_names_a_winner(tmp_path): + """Run `keel research pbo`/`deflate`/`walk-forward` -- the three rail-bearing aliases -- + against fixtures that make each SUCCEED, and assert none of their rendered stdout + contains ranking vocabulary. Word list mirrors + `tests/research/test_walkforward.py::test_refusal_to_rank_enforced_by_source_scan` + exactly (`best`, `winner`, `optimal`), plus `top-ranked` per this issue's own ask. + + Mutation-verified: see the commit message for the exact renderer edit (a `best: ...` + line added to `walkforward.render_lines`), the failure it produced here, and the revert. + """ + runner = CliRunner() + outputs: dict[str, str] = {} + + pbo_ledger = _pbo_ledger(tmp_path) + pbo_result = runner.invoke( + cli, ["research", "pbo", "--ledger", str(pbo_ledger), "--session", "grid", "--blocks", "4"] + ) + assert pbo_result.exit_code == 0, pbo_result.output + assert "PBO" in pbo_result.output + outputs["pbo"] = pbo_result.output + + deflate_ledger = _deflate_ledger(tmp_path) + deflate_result = runner.invoke( + cli, + [ + "research", "deflate", + "--ledger", str(deflate_ledger), "--sharpe", "0.4", + "--rho", "0.5", "--trial-sharpe-variance", "0.05", + ], + ) + assert deflate_result.exit_code == 0, deflate_result.output + assert "DSR" in deflate_result.output + outputs["deflate"] = deflate_result.output + + wf_db = _turtle_db(tmp_path, name="wf.db") + wf_ledger = tmp_path / "wf-trials.jsonl" + wf_result = _invoke( + runner, wf_db, tmp_path, + "research", "walk-forward", + "--rule", "1", "--train-bars", "40", "--test-bars", "20", + "--ledger", str(wf_ledger), + ) + assert wf_result.exit_code == 0, wf_result.output + assert "walk-forward:" in wf_result.output + outputs["walk-forward"] = wf_result.output + + for name, output in outputs.items(): + lowered = output.lower() + for word in ("best", "winner", "optimal", "top-ranked"): + assert word not in lowered, f"keel research {name} printed ranking word {word!r}" From ca1f7e4788ae62e9ba926b58d7bebe272aab48ec Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Fri, 28 Aug 2026 21:07:59 -0400 Subject: [PATCH 8/9] fix(trials): the walk-forward refusal catch was wide enough to swallow an engine bug (#601) The refusal conversion earlier in this branch wrapped a `try: ... except ValueError` around two calls and justified it with a claim that was only true of the first: "every ValueError this pair raises names a train/test window that does not fit the given candle series". That holds for `wf_mod.folds` -- all seven of its raise sites are window-fitting validations against the available bars. It does not hold for `wf_mod.walk_forward`, which runs `backtest` twice per fold and reaches `deflate`. A ValueError raised anywhere in the backtest engine was therefore caught, printed as `refused: ...`, and exited 0. That inverts the point of the change. #601 asked for a refusal to be distinguishable from a failure; as written, this was the one place in the work where the new contract made a failure QUIETER than it had been before, because the old code at least exited non-zero. A genuine defect mid-fold read as "the cached history cannot answer this", and exited successfully, so nothing downstream noticed either. The catch now wraps `folds` alone and `walk_forward` runs outside it, raising as it always did. Nothing is lost by narrowing: the single ValueError `walk_forward` raises for itself is an empty `folds_bounds`, and that is unreachable from this caller, because `folds` refuses `train_bars + test_bars > n_bars` before its loop and so can never return an empty list. What the wide catch WAS reaching was real rather than hypothetical -- `walkforward._closed_pnl` raises ValueError when a trade reaches the aggregate with no realised P&L, a data-integrity error about a poisoned row, and that was being reported as an evidence refusal at exit 0. The comment left in its place says why the boundary is where it is and what to do instead if some walk_forward failure is ever genuinely evidence-shaped: give it a named exception in walkforward.py and catch that. A type is a claim the callee makes about itself; a bare ValueError catch is the caller guessing on the callee's behalf, and the guess is what goes stale as the callee grows. Pinned by test_backtest_failure_during_a_fold_is_not_a_refusal, which monkeypatches `backtest` to raise ValueError and asserts the command neither exits 0 nor prints "refused:". Its fixture is 96 bars with train 40 / test 20 -- a window `folds` accepts -- so it passes the narrow catch cleanly and fails where a fold actually runs, rather than short-circuiting in validation and proving nothing. Mutation-verified by restoring the wide catch verbatim and running it: AssertionError: a ValueError from the backtest engine exited 0 -- an engine bug is being reported as success: 'refused: engine bug: malformed candle at bar 7\n' assert 0 != 0 + where 0 = .exit_code Reverted immediately after confirming the failure. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01T6yA5khYnJ2qzheArRToQ2 --- keel/commands/trials.py | 36 ++++++++++++----- tests/commands/test_research_commands.py | 50 ++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 10 deletions(-) diff --git a/keel/commands/trials.py b/keel/commands/trials.py index 3e778f81..b5863391 100644 --- a/keel/commands/trials.py +++ b/keel/commands/trials.py @@ -573,20 +573,36 @@ def trials_walk_forward( test_bars=test_bars, step_bars=step_bars, ) - report = wf_mod.walk_forward( - resolved.rule, - resolved.candles, - folds_bounds=folds_bounds, - fee_pct=resolved.fee_pct, - ) except ValueError as exc: - # Evidence-shaped (#601): every ValueError this pair raises names a train/test - # window that does not fit the given candle series -- a well-formed request the - # cached history cannot answer, not an operator mistake. Print it and stop; no - # ledger rows, exit 0. + # Evidence-shaped (#601), and the catch is deliberately around `folds` ALONE. + # Every one of the seven ValueErrors `wf_mod.folds` raises names a train/test + # window against the available bars -- a well-formed request the cached history + # cannot answer, not an operator mistake. Print it and stop; no ledger rows, + # exit 0. + # + # ⚠️ Do NOT widen this to cover the `walk_forward` call below. That call runs the + # backtest engine twice per fold and reaches `deflate`; a ValueError from in + # there is a BUG (a Decimal conversion, a malformed candle, `_closed_pnl`'s + # poisoned-row guard), and swallowing it as `refused:` would exit 0 on a failure + # -- making a genuine defect quieter than it was before #601 touched this + # command, and indistinguishable from "the history cannot answer this". The one + # ValueError `walk_forward` raises for itself (an empty `folds_bounds`) is + # unreachable from here: `folds` refuses `train_bars + test_bars > n_bars` before + # its loop, so it never returns an empty list. If some walk_forward failure is + # ever genuinely evidence-shaped, give it a named exception in walkforward.py and + # catch THAT -- a type is a claim the callee makes about itself; a bare + # ValueError catch is the caller guessing on the callee's behalf, and the guess + # is what goes stale. click.echo(f"refused: {exc}") return + report = wf_mod.walk_forward( + resolved.rule, + resolved.candles, + folds_bounds=folds_bounds, + fee_pct=resolved.fee_pct, + ) + for line in wf_mod.render_lines(report): click.echo(line) click.echo() diff --git a/tests/commands/test_research_commands.py b/tests/commands/test_research_commands.py index 12aaaa9f..2b7b36a4 100644 --- a/tests/commands/test_research_commands.py +++ b/tests/commands/test_research_commands.py @@ -35,6 +35,7 @@ from keel.data.repository import Repository from keel.research import ledger as trials_ledger from keel.research import tuning as tuning_mod +from keel.research import walkforward as wf_mod from keel.types import Candle, Granularity MISSING_CONFIG_NAME = "missing-config.yaml" # never created: config degrades to the default @@ -595,3 +596,52 @@ def test_no_evidence_subcommand_names_a_winner(tmp_path): lowered = output.lower() for word in ("best", "winner", "optimal", "top-ranked"): assert word not in lowered, f"keel research {name} printed ranking word {word!r}" + + +def test_backtest_failure_during_a_fold_is_not_a_refusal(tmp_path, monkeypatch): + """A `ValueError` escaping the backtest engine mid-fold must NOT become an exit-0 + refusal. + + `keel research walk-forward` catches `ValueError` to turn "no train/test window fits + this candle series" into a printed refusal at exit 0 (#601). That catch is deliberately + wrapped around `wf_mod.folds` ALONE, and this test is why. `wf_mod.walk_forward` runs + `backtest` twice per fold and reaches `deflate`; a `ValueError` raised in there is a + BUG -- a Decimal conversion, a malformed candle, `walkforward._closed_pnl`'s + poisoned-row guard -- and a wider catch would print it as `refused: ...` and exit 0, + making a genuine engine defect indistinguishable from an honest "the cached history + cannot answer this" AND invisible to anything downstream checking the exit code. That + is the one shape where #601's new contract could make a failure quieter than it was + before, so it is pinned rather than trusted. + + The fixture reaches `walk_forward` for real: 96 bars with train 40 / test 20 is a + window `folds` accepts, so the narrow catch is passed cleanly and the failure happens + where a fold runs. + + Mutation-verified: see the commit message for the restored wide catch, the failure this + test then produced, and the revert. + """ + runner = CliRunner() + db = _turtle_db(tmp_path, name="wf-bug.db") + + def _boom(*args, **kwargs): + raise ValueError("engine bug: malformed candle at bar 7") + + monkeypatch.setattr(wf_mod.backtest_mod, "backtest", _boom) + + result = _invoke( + runner, db, tmp_path, + "research", "walk-forward", + "--rule", "1", "--train-bars", "40", "--test-bars", "20", + "--ledger", str(tmp_path / "wf-bug-trials.jsonl"), + ) + + assert result.exit_code != 0, ( + "a ValueError from the backtest engine exited 0 -- an engine bug is being reported " + f"as success: {result.output!r}" + ) + assert "refused:" not in result.output, ( + "a ValueError from the backtest engine was printed as an evidence refusal; only " + f"`folds` window failures may take that path: {result.output!r}" + ) + assert isinstance(result.exception, ValueError), result.exception + assert "engine bug" in str(result.exception) From a8f1ea2e979470e4838f5933218dd870864e89b2 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Fri, 28 Aug 2026 21:35:57 -0400 Subject: [PATCH 9/9] fix(research): the refusal boundary, two overclaims, and a projection that could only flatter (#601) Review findings on #609. One was a real inversion of this issue's own contract; the rest are claims that outran what the code does. ## throughput was doing the exact thing walk-forward's comment forbids Two bare `except ValueError` blocks in `keel research throughput` turned operator mistakes into exit-0 refusals: keel research throughput --venues-json '[{"venue":"coinbase", "monthly_allowance":"500","mean_trade_notional":"0", "expected_signals_per_month":"1"}]' -> refused: mean_trade_notional must be > 0 (exit 0) A typo in an option value is not thin evidence. The `allocate` catch was worse, because the callee disclaims it in its own docstring -- "a product eligible on no listed venue is an error the caller must fix in the eligibility table, not silently droppable inventory" -- and this command relabelled it `refused:` and exited 0. The caller overruling the callee's stated claim about itself is precisely what the walk-forward comment three hundred lines away in this same file warns against, written before this was noticed in it. `throughput.py` raises ValueError in five places and exactly one is evidence-shaped. That one now has a name, `InsufficientThroughput`, subclassing ValueError so existing callers and tests catching the broader type are unaffected. The command catches that alone; the non-positive mean notional is validated up front as the operator mistake it is (the shape this function already used for `--target-edge`), and `allocate`'s ValueError became a ClickException. Giving the callee a type to state its own claim with is the remedy that comment prescribes, applied rather than described. Pinned by test_operator_mistakes_in_throughput_are_not_refusals, which also asserts an empty --venues-json STILL refuses at exit 0, so the pin cannot be satisfied by making everything an error. Both halves mutation-verified separately by restoring each wide catch: AssertionError: a zero mean_trade_notional exited 0 -- an operator typo in --venues-json is being reported as an evidence refusal: 'refused: mean_trade_notional must be > 0\n' AssertionError: a product eligible on no listed venue exited 0 -- `allocate` calls that an error the caller must fix, and this command relabelled it a refusal: ... 'refused: AAPL is eligible on no listed venue (eligibility: ['alpaca'], listed: ['coinbase'])\n' ## two claims corrected, because an overclaim about a rail is worse than none The AST scan's docstring said the blanket ban "costs nothing real and closes the door completely". It does not. It reads ONE file, so a ranking computed in a module this file merely prints is invisible to it -- and one exists: `cts_factors.pair_stats` and `holm_adjust` both return `sorted(..., key=abs(phi), reverse=True)`, so `_render_factors` prints a strict |phi|-descending table. It also matches on syntax, so `sorted([(s.p_value, s.a, s.b) for s in stats])[0]` ranks perfectly without a `key=`. The docstring now states both gaps and what the pin actually is: a tripwire on the cheap idiomatic ranking, a third line behind `PBOResult`'s field scan and walkforward.py's source scan, not the whole defence. `test_no_evidence_subcommand_names_a_winner` drove only the three rail-bearing aliases while its name claimed every evidence subcommand, and its four-word list would have passed "highest PBO: cfg-7". It now also drives significance, throughput, tuning and factors -- the newest renderers, and so the likeliest place a ranking phrase gets written -- and the vocabulary is widened for an OUTPUT scan rather than the source-scan list it borrowed: highest, lowest, strongest, top-ranked, ranked #. Mutation-verified with the phrasing the old list missed, injected into walkforward.render_lines: AssertionError: keel research walk-forward printed ranking word 'highest' -- a score may report and may gate, but naming a leader is the Strathern rail's one prohibition ## the pooled-review driver is not "unchanged" The docs page said running the driver directly "still works unchanged". Its exit contract is unchanged; the file is not. It lost 81 lines and now imports `_connect_ro`, a private name, across a module boundary, so a pre-registered measurement depends on an actively developed CLI module. That was a deliberate trade -- one reader beats two drifting apart on the one date it matters -- and the page now states it as a trade, with the advice to read `keel/commands/research.py` alongside the driver when auditing the review. ## a projection that could only flatter `_vectors` mapped a trade whose exit bar was absent from the common index to `n - 1`, marking the position occupied through the end of the shared window on the strength of a bar never observed -- an error that can only inflate the Jaccard overlap `compare()` reports, never deflate it. It now walks back to the previous shared bar (`bisect`), and the entry forward to the next one, dropping a trade no shared bar observes rather than stretching it. Separately, the emptiness guard tested `closed_a`/`closed_b` while `_vectors` was fed `result_a.trades` including open trades: guard and computation disagreed about the population, and an open trade would have added occupied bars to `positions` while contributing nothing to `pnl`, which `compare()` correlates against each other. Both now use closed trades. `_vectors` states these as the two decisions they are; the calling docstring no longer calls the whole block "mechanical bookkeeping", which undersold it. test_independence_does_not_stretch_a_trade_past_the_shared_history characterises the gapped-intersection path, which no other fixture reached (every other one puts both rules on the same product, making the intersection total). Its docstring is explicit that it does NOT discriminate the fix: restoring the old defaulting leaves its number at 0.333 unchanged, verified rather than assumed. The correctness argument rests on the reasoning in `_vectors`, not on that test. ## smaller `research tuning --explored-json` had no guard, so 'not json', '{"period": 5}' and '{"period": [5]}' reached the user as JSONDecodeError, TypeError and IndexError tracebacks; it now refuses malformed input cleanly like its sibling options. `keel.commands.research` was missing from test_service_isolation.py's SERVICE_MODULES, so the import pin was silently skipping it -- the list's own comment exists to prevent that. The refusal pin's tuning fixture was `--run`, a CAPABILITY refusal that met the pin's letter while testing nothing about its stated criterion; it is now an out-of-bounds explored range, a real evidence refusal. That swap exposed a latent bug in the pin: it built argv with `str.format`, which read the JSON literal's `{"entry"` as a format field and raised KeyError, so substitution is now a plain two-placeholder replace. The read-write hazard on `significance --from deployment`/`factors`/ `independence` (`_open_repo` calls `migrate`, which commits) is documented in docs/research-toolkit.md and filed as #610 rather than fixed here. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01T6yA5khYnJ2qzheArRToQ2 --- docs/research-toolkit.md | 41 +++- keel/commands/research.py | 111 +++++++++-- keel/research/throughput.py | 23 ++- tests/commands/test_research_commands.py | 222 ++++++++++++++++++++- tests/commands/test_research_front_door.py | 35 +++- tests/commands/test_service_isolation.py | 1 + 6 files changed, 399 insertions(+), 34 deletions(-) diff --git a/docs/research-toolkit.md b/docs/research-toolkit.md index 45ee22b3..ed9abbfa 100644 --- a/docs/research-toolkit.md +++ b/docs/research-toolkit.md @@ -193,6 +193,28 @@ edge of 57.6 points or larger at 80% power in the first place. `render_family` p rather than the bare verdict, so the reader sees exactly why the answer is no, and the command still exits 0 — the question was well-formed and the evidence answered it honestly. +## ⚠️ Which of these open your database read-write + +Worth knowing before you point one of these at a live deployment, because the toolkit is not +consistent about it and the inconsistency is not obvious from the command names. + +`keel research pooled-review` opens every profile database **read-only** +(`sqlite3.connect(f"file:{db}?mode=ro", uri=True)`) and goes out of its way to do so, because +it is designed to be pointed at live and paper ledgers on the review date. + +`keel research significance --from deployment`, `factors` and `independence` do **not**. They +reach their data through `_open_repo` (`keel/commands/_common.py:128-131`), which calls +`migrate(conn)`, and `migrate` commits unconditionally (`keel/data/db.py:595`). So a +nominally read-only research command opens the database read-write, and — if the binary you +are running is newer than the file — will migrate its schema as a side effect of answering a +question about it. + +This is pre-existing behaviour that every other read-only `keel` command shares, and WAL mode +makes it survivable alongside a running agent. It is called out here rather than left implicit +because this page is what invites you to run these against a deployment. **If that matters for +your case, copy the database first and point the command at the copy.** The read-only seam is +filed as its own issue; `pooled-review` shows what the fix looks like. + ## What is NOT here This front door is surfacing, not new statistics. `keel research` assembles inputs, calls a @@ -258,5 +280,20 @@ and stays that way: it was pre-registered before the review event and is frozen, command is new surface that gets the right behaviour — stdout, exit 0 — from the start. Before 2026-09-30 both run the same machinery as a labelled preview, and both say so. -Running the review directly against `docs/experiments/2026-09-30-pooled-review.py` still works -unchanged, stderr/exit-2 refusal included — it remains the pre-registered driver of record. +Running the review directly against `docs/experiments/2026-09-30-pooled-review.py` still +behaves exactly as pre-registered — same pool definition, same dedup, same stderr/exit-2 +refusal — and it remains the driver of record. + +**The file, though, is not unchanged, and the trade is worth stating plainly rather than +hiding behind "still works".** It lost 81 lines, and the reading it used to own now lives in +`keel/commands/research.py`, which it imports across a module boundary — including +`_connect_ro`, a private name. So a pre-registered measurement now depends on an actively +developed CLI module, and the pre-registration's own guarantee ("the pool definition, frozen +before the first forward trade closes") is enforced by that module's tests rather than by the +driver being a sealed artifact. + +That was a deliberate choice and the alternative was worse: two readers of the same three +tables, drifting apart silently, with the CLI and the pre-registered event disagreeing about +what "the pool" means on the one date it matters. One reader, tested, is the safer failure +mode. But a reader auditing the review on 2026-09-30 should know the driver is no longer +self-contained, and should read `keel/commands/research.py` alongside it. diff --git a/keel/commands/research.py b/keel/commands/research.py index 734191bb..2e78c31c 100644 --- a/keel/commands/research.py +++ b/keel/commands/research.py @@ -47,6 +47,7 @@ from __future__ import annotations +import bisect import json import sqlite3 from dataclasses import asdict, dataclass @@ -744,9 +745,16 @@ def research_throughput( Pure arithmetic -- no db, no candles, no rule. `--venues-json` states what `render_report`/`months_to_target` need to know about each venue; `--products-json` + - `--allowances-json`, when both given, additionally run the allocator. `allocate()` raises - `ValueError` when a product is eligible on no listed venue: caught here and printed as a - refusal (#601's second bullet), never a traceback, and `throughput.py` itself is unchanged. + `--allowances-json`, when both given, additionally run the allocator. + + ONE failure here is evidence-shaped and prints as a refusal at exit 0 (#601): nothing is + flowing, so no time-to-detection can be stated. It is caught by its own named type, + `throughput.InsufficientThroughput`. Everything else this command can hit is an OPERATOR + MISTAKE and exits non-zero: a non-positive `mean_trade_notional` (validated below, before + the module is called at all), a `--target-edge` outside (0, 1), and a product eligible on + no listed venue -- which `throughput.allocate`'s own docstring calls "an error the caller + must fix in the eligibility table, not silently droppable inventory". Reporting that as a + refusal would be this command overruling the callee's stated claim about itself. """ try: venues = [ @@ -761,16 +769,30 @@ def research_throughput( except (json.JSONDecodeError, KeyError, TypeError) as exc: raise click.ClickException(f"--venues-json is malformed: {exc}") from exc + # An operator mistake, checked HERE rather than left to surface from inside the module: + # `VenueThroughput.trades_per_month` raises a bare ValueError for this, and a typo in + # --venues-json is a wrong request, not thin evidence. Same shape as the --target-edge + # check below, which this file already validated this way. + for venue in venues: + if venue.monthly_allowance is not None and venue.mean_trade_notional <= 0: + raise click.ClickException( + f"--venues-json: {venue.venue} has mean_trade_notional " + f"{venue.mean_trade_notional} -- must be > 0 to divide an allowance by it" + ) + edge = Decimal(str(target_edge)) if not (Decimal(0) < edge < Decimal(1)): raise click.ClickException("--target-edge must be a fraction in (0, 1), e.g. 0.05") try: lines = throughput_mod.render_report(venues, edge) - except ValueError as exc: - # render_report's own months_to_target refuses on zero pooled trades/month (an empty - # --venues-json, or every venue's allowance-bound throughput rounding to nothing) -- - # a well-formed plan the data cannot answer, not an operator mistake. Print it, exit 0. + except throughput_mod.InsufficientThroughput as exc: + # The ONE evidence-shaped failure in throughput.py, caught by its own named type + # rather than by a bare `except ValueError` around the whole call. render_report also + # reaches `trades_per_month` and `required_n_eff`, and every ValueError THOSE raise is + # an operator mistake; a wide catch here would print an operator's typo as `refused:` + # and exit 0, which is the failure `trials.py`'s walk-forward comment forbids and + # which this command committed until #601 review caught it. click.echo(f"refused: {exc}") return for line in lines: @@ -800,8 +822,12 @@ def research_throughput( try: plans = throughput_mod.allocate(products, allowances) except ValueError as exc: - click.echo(f"refused: {exc}") - return + # NOT a refusal. `allocate` raises only for a product eligible on no listed venue, + # and its own docstring calls that "an error the caller must fix in the eligibility + # table, not silently droppable inventory". The callee states what its failure means; + # a front door that relabels it "refused: ..." and exits 0 is the caller overruling + # that claim, and would hide a mismatched --products-json/--allowances-json pair. + raise click.ClickException(str(exc)) from exc click.echo("") click.echo("allocation:") @@ -878,9 +904,21 @@ def research_tuning(rule_kind: str | None, explored_json: str | None, run: bool) if explored_json is not None: assert rule_kind is not None # guarded above - explored = { - name: (bounds[0], bounds[1]) for name, bounds in json.loads(explored_json).items() - } + try: + explored = { + name: (bounds[0], bounds[1]) + for name, bounds in json.loads(explored_json).items() + } + except (json.JSONDecodeError, AttributeError, KeyError, TypeError, IndexError) as exc: + # Same treatment the sibling --venues-json/--products-json options already get: a + # malformed option value is an OPERATOR mistake and exits non-zero with a clean + # message, never a traceback. Without this, 'not json' raised JSONDecodeError, + # '{"period": 5}' TypeError, and '{"period": [5]}' IndexError, each straight + # through to the user as a stack trace. + raise click.ClickException( + f"--explored-json is malformed: {exc} -- expected an object mapping a " + 'parameter name to a [low, high] pair, e.g. \'{"entry": [20, 40]}\'' + ) from exc try: check = tuning_mod.explored_vs_declared(explored, rule_kind) except ValueError as exc: @@ -1041,9 +1079,12 @@ def research_independence( """Are two rules (or two horizons of one rule) actually independent (independence.py, §80.16)? Correlated rules inflate N without adding independent evidence (§73.5). - Position and per-bar P&L vectors are built here over the two rules' COMMON bar index -- - mechanical bookkeeping, not a statistic: `compare()` has no opinion on how its input - vectors are assembled, only on what to compute once they are aligned onto one calendar. + Position and per-bar P&L vectors are built here over the two rules' COMMON bar index. + `compare()` has no opinion on how its input vectors are assembled, only on what to compute + once they are aligned onto one calendar -- but "just bookkeeping" undersold it, so + `_vectors` below now states the two choices it makes (closed trades only; a timestamp off + the common index maps to the nearest bar INSIDE the trade, never to the end of history) + and why each is the conservative one. """ repo = _open_repo(ctx) config = rules_mod._optional_cfg(ctx) @@ -1083,18 +1124,46 @@ def research_independence( "-- nothing to compare" ) return - index_of = {ts: i for i, ts in enumerate(common_ts)} n = len(common_ts) def _vectors(trades: list[Any]) -> tuple[list[int], list[Decimal], list[int]]: + """Project CLOSED trades onto the common index. Bookkeeping only -- but bookkeeping + with two decisions in it, both made the conservative way after #601 review: + + * **Closed trades only**, the same population the emptiness guard above tests. An + open trade has no realised P&L, so it would add occupied bars to `positions` while + contributing nothing to `pnl`, and `compare()` correlates those two series against + each other -- they must describe the same set of trades or the correlation is + between mismatched populations. It would also have no exit bar, so counting it + would mean asserting occupancy through the end of history on the strength of a + position that has not resolved. + * **A timestamp missing from the common index is mapped to the nearest common bar + INSIDE the trade, never to the end of history.** The two rules' caches can differ + in depth or have gaps, so `index_of` can miss either end. Defaulting a missing + exit to `n - 1` (as this did before review) marks the position occupied to the + last bar of the common window, which inflates the Jaccard overlap `compare()` + reports whenever the caches disagree -- a fabricated agreement, in the one + direction that flatters the answer. `bisect` instead: the entry becomes the first + common bar at or after it, the exit the last common bar at or before it, and a + trade whose whole span falls outside the common window is dropped rather than + stretched to fill it. + """ positions = [0] * n pnl = [Decimal(0)] * n entries: list[int] = [] for trade in trades: - start = index_of.get(trade.entry_ts) - if start is None: + # First common bar at or after the entry; past the end means the trade opened + # after the shared history stops, so there is nothing to place. + start = bisect.bisect_left(common_ts, trade.entry_ts) + if start >= n: + continue + # Last common bar at or before the exit. A closed trade always has an exit_ts + # (only an open trade omits one, and those are excluded above). + end = bisect.bisect_right(common_ts, trade.exit_ts) - 1 + if end < start: + # The trade opened and closed between two common bars, or entirely before + # the window: no common bar observes it. continue - end = n - 1 if trade.exit_ts is None else index_of.get(trade.exit_ts, n - 1) entries.append(start) for i in range(start, end + 1): positions[i] = 1 @@ -1102,8 +1171,8 @@ def _vectors(trades: list[Any]) -> tuple[list[int], list[Decimal], list[int]]: pnl[end] += trade.pnl return positions, pnl, entries - pos_a, pnl_a, entries_a = _vectors(result_a.trades) - pos_b, pnl_b, entries_b = _vectors(result_b.trades) + pos_a, pnl_a, entries_a = _vectors(closed_a) + pos_b, pnl_b, entries_b = _vectors(closed_b) if not any(pos_a) or not any(pos_b): click.echo( diff --git a/keel/research/throughput.py b/keel/research/throughput.py index e22bb513..76258e4e 100644 --- a/keel/research/throughput.py +++ b/keel/research/throughput.py @@ -195,12 +195,31 @@ def allocate(products: list[Product], allowances: dict[str, Decimal | None]) -> return plans +class InsufficientThroughput(ValueError): + """No trades are flowing, so no time-to-detection can be stated (#601). + + A NAMED type, and the only evidence-shaped failure in this module: every other + ``raise`` here reports a caller mistake -- a negative n, an edge outside (0, 1), a + non-positive mean notional, a product eligible on no listed venue -- which a front + door must report as an error, not as a refusal. A front end that wants to print + "the plan is well-formed but there is nothing to measure" as a RESULT needs to catch + exactly this and nothing else; catching bare ``ValueError`` around a call into this + module would swallow all four of the caller mistakes with it. + + Subclasses ``ValueError`` so existing callers and tests that catch the broader type + keep working unchanged -- the narrowing is additive, not a break. + """ + + def months_to_target(target_effective_n: Decimal, pooled_trades_per_month: Decimal) -> Decimal: """Months for pooled trades to accumulate ``target_effective_n`` INDEPENDENT - observations, applying the design effect -- never raw n.""" + observations, applying the design effect -- never raw n. + + Raises `InsufficientThroughput` (a `ValueError`) when nothing is flowing: that is the + one failure here a caller may honestly report as a refusal rather than an error.""" per_month = n_eff(pooled_trades_per_month) if per_month <= 0: - raise ValueError("pooled trades per month must be > 0") + raise InsufficientThroughput("pooled trades per month must be > 0") return target_effective_n / per_month diff --git a/tests/commands/test_research_commands.py b/tests/commands/test_research_commands.py index 2b7b36a4..8e13dea7 100644 --- a/tests/commands/test_research_commands.py +++ b/tests/commands/test_research_commands.py @@ -28,6 +28,7 @@ from decimal import Decimal from pathlib import Path +import pytest from click.testing import CliRunner from keel.cli import cli @@ -58,6 +59,13 @@ def _invoke(runner: CliRunner, db: Path, tmp_path: Path, *args: str): # -- shared candle fixtures ------------------------------------------------------------------- +#: Jaccard for the 40-bar shared window in +#: `test_independence_does_not_stretch_a_trade_past_the_shared_history`. A measured +#: constant, not a derivation: it is pinned so a change in how trades are projected onto +#: the common index shows up as a failing number rather than as a quietly different report. +_EXPECTED_DEPTH_JACCARD = 0.3333333333333333 + + def _sawtooth_candles(n: int, *, start: int = 1_700_000_000) -> list[Candle]: """`n` daily bars in an asymmetric 19-bar sawtooth -- lifted from `tests/research/test_trials_cli.py::_mc_candles`: an 8-bar rally, a 9-bar crash, a 2-bar @@ -495,6 +503,67 @@ def test_independence_renders_overlap_and_correlation_figures(tmp_path): assert re.search(r"entry distances \(n=\d+\):", result.output) +def test_independence_does_not_stretch_a_trade_past_the_shared_history(tmp_path): + """Characterises `keel research independence` when the two rules' cached series differ, + so the common bar index has INTERIOR GAPS rather than being total. + + Every other fixture here puts both rules on the same product, which makes the + intersection total and leaves `_vectors`' off-index branch unexercised. This one puts + rule 1 on BTC-USD (192 bars) and rule 2 on ETH-USD (every other one of those bars), so + the intersection is 96 gapped bars and trades routinely exit on a timestamp that is in + one rule's cache and not in the shared index. + + **What this does NOT do, stated because the surrounding commit changes that branch.** + `_vectors` used to map an off-index exit to `n - 1` and now walks back to the previous + shared bar; the difference is real on the merits (`n - 1` asserts occupancy on bars the + shared history never observed, which can only inflate Jaccard, never deflate it) but + **this fixture does not distinguish the two** -- restoring the old defaulting leaves the + number below unchanged at 0.333..., which was verified rather than assumed. So this is a + characterisation pin, not a proof of the fix: it holds the gapped path executing and its + output stable, and it would catch a future change that moves the number. The correctness + argument for the projection lives in `_vectors`' own docstring, not here. + """ + db = tmp_path / "indep-depth.db" + conn = connect(str(db)) + migrate(conn) + repo = Repository(conn) + repo.insert_rule( + "turtle_breakout", {**_TURTLE_A_PARAMS, "product_id": "BTC-USD"}, + status="candidate", now_ts=1_800_000_000, + ) + repo.insert_rule( + "turtle_breakout", {**_TURTLE_A_PARAMS, "product_id": "ETH-USD"}, + status="candidate", now_ts=1_800_000_000, + ) + btc = _sawtooth_candles(192) + repo.upsert_candles("BTC-USD", Granularity.ONE_DAY, btc) + # ETH keeps only every OTHER bar, so the intersection has INTERIOR gaps rather than a + # truncated tail. That distinction is the whole point: for a trade whose exit lies beyond + # the shared window, `n - 1` is the right answer (it really was held throughout). The bug + # is a trade whose exit falls in a HOLE -- present in one cache, absent from the + # intersection -- which the old code stretched to the end of the window instead of back to + # the previous shared bar. + repo.upsert_candles("ETH-USD", Granularity.ONE_DAY, btc[::2]) + conn.close() + + result = _invoke( + CliRunner(), db, tmp_path, + "research", "independence", "--rule-a", "1", "--rule-b", "2", + ) + assert result.exit_code == 0, result.output + assert "over 96 common bars" in result.output, result.output + + match = re.search(r"jaccard overlap[^0-9]*([0-9.]+)", result.output) + assert match, result.output + jaccard = float(match.group(1)) + assert 0.0 <= jaccard <= 1.0, jaccard + assert jaccard == pytest.approx(_EXPECTED_DEPTH_JACCARD, abs=1e-6), ( + f"jaccard over a gapped 96-bar shared window came out {jaccard}, expected " + f"{_EXPECTED_DEPTH_JACCARD} -- a change here means trades are being projected onto " + "the common index differently; check _vectors before updating this number" + ) + + # == the rail, at the rendered surface ========================================================== # # The Strathern rail (cscv.py/deflate.py/walkforward.py) is pinned at the SOURCE level in @@ -547,11 +616,25 @@ def _deflate_ledger(tmp_path: Path) -> Path: def test_no_evidence_subcommand_names_a_winner(tmp_path): - """Run `keel research pbo`/`deflate`/`walk-forward` -- the three rail-bearing aliases -- - against fixtures that make each SUCCEED, and assert none of their rendered stdout - contains ranking vocabulary. Word list mirrors - `tests/research/test_walkforward.py::test_refusal_to_rank_enforced_by_source_scan` - exactly (`best`, `winner`, `optimal`), plus `top-ranked` per this issue's own ask. + """Run the evidence subcommands against fixtures that make each SUCCEED -- a refusal has + nothing to rank, so it would not exercise a renderer's word choice -- and assert none of + their rendered stdout names a winner. + + Covers the three rail-bearing aliases (`pbo`, `deflate`, `walk-forward`) AND the four new + subcommands that render a report of their own (`significance`, `throughput`, `tuning`, + `factors`). The first version of this test drove only the three aliases while its name + claimed "no evidence subcommand", which was an overclaim; the six new subcommands are the + newest renderers on this surface and so the likeliest place a ranking phrase gets written. + + The word list starts from `tests/research/test_walkforward.py:: + test_refusal_to_rank_enforced_by_source_scan` (`best`, `winner`, `optimal`) and is widened + here, because that list is a source-scan vocabulary and this is an OUTPUT scan: a renderer + can name a winner without ever using the word "best". `highest`/`lowest`/`top-ranked`/ + `strongest`/`ranked #` are the phrasings a report actually reaches for. + + Still not a proof. A renderer could name a winner in words none of these match, and this + only sees the fixtures it happens to run. It is a tripwire on the obvious phrasings, and + it is stated as one. Mutation-verified: see the commit message for the exact renderer edit (a `best: ...` line added to `walkforward.render_lines`), the failure it produced here, and the revert. @@ -592,10 +675,56 @@ def test_no_evidence_subcommand_names_a_winner(tmp_path): assert "walk-forward:" in wf_result.output outputs["walk-forward"] = wf_result.output + sig_db = _turtle_db(tmp_path, name="winner-sig.db") + sig_result = _invoke( + runner, sig_db, tmp_path, "research", "significance", "--from", "rule", "--rule", "1" + ) + assert sig_result.exit_code == 0, sig_result.output + outputs["significance"] = sig_result.output + + thr_result = runner.invoke( + cli, + [ + "research", "throughput", + "--venues-json", + json.dumps([{ + "venue": "coinbase", "monthly_allowance": "5000", + "mean_trade_notional": "100", "expected_signals_per_month": "10", + }]), + ], + ) + assert thr_result.exit_code == 0, thr_result.output + outputs["throughput"] = thr_result.output + + tun_result = runner.invoke(cli, ["research", "tuning"]) + assert tun_result.exit_code == 0, tun_result.output + outputs["tuning"] = tun_result.output + + fac_db = tmp_path / "winner-fac.db" + fac_conn = connect(str(fac_db)) + migrate(fac_conn) + Repository(fac_conn).upsert_candles( + "BTC-USD", Granularity.ONE_DAY, _factor_candles(400, seed=7) + ) + fac_conn.close() + fac_result = _invoke( + runner, fac_db, tmp_path, + "research", "factors", "--product", "BTC-USD", "--granularity", "ONE_DAY", + ) + assert fac_result.exit_code == 0, fac_result.output + outputs["factors"] = fac_result.output + + banned = ( + "best", "winner", "optimal", "top-ranked", "top ranked", + "highest", "lowest", "strongest", "ranked #", + ) for name, output in outputs.items(): lowered = output.lower() - for word in ("best", "winner", "optimal", "top-ranked"): - assert word not in lowered, f"keel research {name} printed ranking word {word!r}" + for word in banned: + assert word not in lowered, ( + f"keel research {name} printed ranking word {word!r} -- a score may report " + "and may gate, but naming a leader is the Strathern rail's one prohibition" + ) def test_backtest_failure_during_a_fold_is_not_a_refusal(tmp_path, monkeypatch): @@ -645,3 +774,82 @@ def _boom(*args, **kwargs): ) assert isinstance(result.exception, ValueError), result.exception assert "engine bug" in str(result.exception) + + +def test_operator_mistakes_in_throughput_are_not_refusals(tmp_path): + """`keel research throughput` must report an OPERATOR mistake as an error, not as a + refusal at exit 0 -- the same boundary + `test_backtest_failure_during_a_fold_is_not_a_refusal` pins for walk-forward. + + `throughput.py` raises `ValueError` in five places and only ONE of them is + evidence-shaped (`InsufficientThroughput`: nothing is flowing, so no time-to-detection + can be stated). The other four report a caller mistake. Two are reachable from this + command and are pinned here: + + * a non-positive `mean_trade_notional` in `--venues-json`, which + `VenueThroughput.trades_per_month` raises on -- a typo in an option value; + * a product eligible on no listed venue, which `allocate` raises on and whose own + docstring calls "an error the caller must fix in the eligibility table, not silently + droppable inventory" -- so printing it as `refused:` would be this command overruling + the callee's stated claim about itself. + + Both were exit-0 `refused:` lines until #601 review caught them. The third assertion + keeps the honest refusal honest: an empty `--venues-json` still refuses at exit 0, so + this pin cannot be satisfied by turning every failure into an error. + + Mutation-verified: see the commit message for the restored wide `except ValueError`, + the failures it produced here, and the revert. + """ + runner = CliRunner() + + typo = runner.invoke( + cli, + [ + "research", "throughput", + "--venues-json", + json.dumps([{ + "venue": "coinbase", + "monthly_allowance": "500", + "mean_trade_notional": "0", + "expected_signals_per_month": "1", + }]), + ], + ) + assert typo.exit_code != 0, ( + "a zero mean_trade_notional exited 0 -- an operator typo in --venues-json is being " + f"reported as an evidence refusal: {typo.output!r}" + ) + assert "refused:" not in typo.output, typo.output + + ineligible = runner.invoke( + cli, + [ + "research", "throughput", + "--venues-json", + json.dumps([{ + "venue": "coinbase", + "monthly_allowance": "5000", + "mean_trade_notional": "100", + "expected_signals_per_month": "10", + }]), + "--products-json", + json.dumps([{ + "symbol": "AAPL", + "venues": ["alpaca"], + "mean_trade_notional": "100", + "expected_signals_per_month": "5", + }]), + "--allowances-json", json.dumps({"coinbase": "5000"}), + ], + ) + assert ineligible.exit_code != 0, ( + "a product eligible on no listed venue exited 0 -- `allocate` calls that an error " + f"the caller must fix, and this command relabelled it a refusal: {ineligible.output!r}" + ) + assert "refused:" not in ineligible.output, ineligible.output + + # ...and the one genuine refusal still refuses, so this pin cannot be satisfied by + # making everything an error. + nothing_flowing = runner.invoke(cli, ["research", "throughput", "--venues-json", "[]"]) + assert nothing_flowing.exit_code == 0, nothing_flowing.output + assert "refused:" in nothing_flowing.output, nothing_flowing.output diff --git a/tests/commands/test_research_front_door.py b/tests/commands/test_research_front_door.py index cbc8254c..352556af 100644 --- a/tests/commands/test_research_front_door.py +++ b/tests/commands/test_research_front_door.py @@ -185,6 +185,26 @@ def test_research_module_never_sorts_ranks_or_maxes(): """AST scan over `keel/commands/research.py` itself (see the module docstring for why the ban is blanket, not field-aware). + **What this pin does NOT do**, stated here because an overclaim about a rail is worse + than no claim. It is a SHAPE check over ONE file, and two gaps follow from that: + + * It sees only ranking written in `keel/commands/research.py`. A ranking computed in a + module this file merely prints is invisible to it -- and one exists: + `cts_factors.pair_stats` and `holm_adjust` both `return sorted(..., key=abs(phi), + reverse=True)`, so `_render_factors` prints a strict |phi|-descending table. That is + the compute module's own ordering of an aggregate, not this front door selecting a + configuration, which is why it is allowed -- but it is not something this scan + verified. + * It matches on syntax, so ranking without a `key=` slips through: + `sorted([(s.p_value, s.a, s.b) for s in stats])[0]` and `max(zip(scores, configs))` + both rank perfectly and neither trips this test. + + What it DOES do is make the cheap, idiomatic way to introduce a ranking here fail + loudly, and force anything else to be written conspicuously enough that a reader + notices. The rail's real enforcement lives where the scores do -- `PBOResult` carrying + no configuration field (`tests/research/test_cscv.py`), `walkforward.py`'s own source + and field scans -- and this is a third line, not the whole defence. + Mutation-verified: inserting `sorted(RESEARCH_INDEX, key=lambda r: r.module)` into the module made this fail with `AssertionError: sorted()/max()/min() called with key= at keel/commands/research.py:` before being removed again; see the commit message @@ -433,7 +453,12 @@ def _refusal_fixture_db(tmp_path: Path) -> Path: "significance": ("--from", "deployment"), "pooled-review": ("--db", "{db}"), "throughput": ("--venues-json", "[]"), - "tuning": ("--run",), + # An EVIDENCE refusal, deliberately not `--run`. `--run` refuses too, but for a + # CAPABILITY reason (optuna is a dev-only dependency this command must not import), which + # satisfies this pin's letter while testing nothing about the criterion it states. An + # explored range outside turtle_breakout's own declared bounds is the real thing: a + # well-formed question the declared space answers "no" to. + "tuning": ("--rule-kind", "turtle_breakout", "--explored-json", '{"entry": [2, 9999]}'), "factors": ("--product", "NO-SUCH-PRODUCT"), "independence": ("--rule-a", "1", "--rule-b", "2", "--granularity", "ONE_DAY"), "pbo": ("--ledger", "{tmp}/empty-trials.jsonl"), @@ -486,8 +511,14 @@ def test_every_evidence_subcommand_can_refuse_on_stdout_and_exit_zero(tmp_path): "one (or a named exclusion in _REFUSAL_PIN_EXCLUDED) before this subcommand can " "be trusted to refuse rather than crash on thin evidence" ) + # Only the two declared placeholders are substituted, by plain replace rather than + # str.format: a fixture argument can legitimately be a JSON literal, and `{"entry": + # [2, 9999]}` is not a format string -- `.format` read `{"entry"` as a field name and + # raised KeyError, which is how the tuning fixture broke when it was swapped for a + # real evidence refusal. argv = [ - arg.format(db=str(db_path), tmp=str(tmp_path)) for arg in _REFUSAL_ARGS[name] + arg.replace("{db}", str(db_path)).replace("{tmp}", str(tmp_path)) + for arg in _REFUSAL_ARGS[name] ] result = CliRunner().invoke( cli, diff --git a/tests/commands/test_service_isolation.py b/tests/commands/test_service_isolation.py index e1f24856..3ddc4182 100644 --- a/tests/commands/test_service_isolation.py +++ b/tests/commands/test_service_isolation.py @@ -50,6 +50,7 @@ "keel.commands.monitor", "keel.commands.pnl", "keel.commands.purification", + "keel.commands.research", "keel.commands.rules", "keel.commands.simulate", "keel.commands.status",