Skip to content

feat(research): keel research — one front door over the thirteen evidence modules (#601) - #609

Merged
eaitbrahim merged 9 commits into
mainfrom
feat-601-research-front-door
Aug 29, 2026
Merged

feat(research): keel research — one front door over the thirteen evidence modules (#601)#609
eaitbrahim merged 9 commits into
mainfrom
feat-601-research-front-door

Conversation

@eaitbrahim

Copy link
Copy Markdown
Contributor

Closes #601.

The issue's premise was half wrong, and the correction is the design

#601 says "thirteen modules with no front door." Six of them already had one. keel trials
fronts ledger.py (record/list/verify), cscv.py+matrix.py (pbo), deflate.py,
montecarlo.py and walkforward.py; keel rules lookahead fronts bias.py. Only six were
genuinely unreachable: significance.py, cts_factors.py, independence.py,
throughput.py, tuning.py, pooled_review.py.

So the real gap was never "no code path." It was no single place that says here is the
evidence toolkit
— the tools were scattered across two command groups and 30+ ad-hoc
drivers under docs/experiments/, discoverable only by reading source. keel research is
that place, and it is three things:

  1. keel research index — every module, the question it answers, what it cannot
    answer
    , and the command that runs it. --json, and --module NAME for one entry.
  2. Six new subcommands for the six modules that had none.
  3. Five aliasespbo, deflate, monte-carlo, walk-forward, lookahead are the
    same click command objects registered a second time, never copies. keel research pbo
    and keel trials pbo are byte-for-byte the same code. A front door that reimplements is
    a front door that drifts the moment one copy gets a bugfix the other doesn't.

No new statistics anywhere. Every number traces to a function already in keel/research/;
the command layer assembles inputs, calls in, and prints what comes back, reusing each
module's own render_family/render_report rather than a second renderer that could drift.
keel/commands/research.py's docstring cites ADR 0003 per its standing rule 3.

A refusal is a result, not an error

#601's second bullet, applied everywhere — including to code that predates it.
keel trials deflate, pbo, monte-carlo and walk-forward each had a path where the
ledger was readable, the request well-formed, and the evidence simply could not answer it —
and all four reported that as click.ClickException: exit 1, on stderr, indistinguishable
from an operator typo. Those are now printed on stdout at exit 0.

The line drawn, and held: an operator error means the request was wrong (a rule id that
doesn't exist, a db that won't open, a broken ledger hash chain — that last is tampering, not
insufficient evidence); a refusal means the request was right and the evidence cannot
answer it.
Only the second became a result. rules lookahead's exit-1 on an actual
lookahead finding stays loud on purpose — a definitive finding is the opposite of a refusal.

Observable today:

$ keel research pooled-review --db /nonexistent.db          # operator error
Error: pre-registered profile db(s) not reachable read-only   → exit 1, stderr

$ keel research pooled-review --db <empty-but-real.db>       # evidence refusal
refused: nothing to review: 0 counted win/loss trades of 0 pooled …  → exit 0, stdout

The Strathern rail

cscv.py, deflate.py and walkforward.py each carry it: a score may report, and may gate,
but may never be a sweep's ranking key. The front door is the newest surface over those
three, so it is the newest place a ranking could leak. Four pins, all mutation-verified:

  • An AST scan over keel/commands/research.py banning sorted/max/min with key=,
    .sort(key=…), and heapq/itemgetter/attrgetter — unconditionally, not scoped to
    fields that look rail-bearing, because a field-aware scanner is one a rename defeats.
    The front door's only legitimate ordering need is its own table of contents, served by a
    declared literal tuple.
  • A rendered-output pin — the source scans never ran a command and read its stdout.
    test_no_evidence_subcommand_names_a_winner drives pbo/deflate/walk-forward to
    success (a refusal has nothing to rank) and asserts no "best", "winner", "optimal" or
    "top-ranked". Mutation: injecting " best: fold 0" into walkforward.render_lines failed
    it with AssertionError: keel research walk-forward printed ranking word 'best'.
  • A completeness pin — every module in keel/research/ must have an index row, and every
    command the index names must resolve in the real CLI. It also caught its own weakness: it
    checked a named docs/experiments/ driver existed, never that it was the right one, which
    is how throughput.py's entry silently pointed at the pooled-review driver. It now
    AST-parses the driver and asserts it imports the module it claims to drive.
  • A refusal pin — enumerates research_group.commands dynamically, invokes each against
    a deliberately thin fixture, and asserts exit 0 with a refusal on stdout. A subcommand with
    no declared fixture fails rather than being silently skipped, so a seventh added later
    cannot pass by omission.

Every pin was written, then deliberately broken, then restored; each commit records the exact
mutation and its exact failure text.

The 2026-09-30 pooled review (#427)

Runnable through the front door: keel research pooled-review. _connect_ro, read_orders
and read_ledger moved out of docs/experiments/2026-09-30-pooled-review.py into the command
layer, and the driver imports them back — one reader of a deployment database, so the CLI
and the pre-registered driver 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 event and is frozen. The command is 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 frozen before that decision existed. Both read file:…?mode=ro and
neither ever writes to a deployment database — pinned by a test that hashes the fixture .db
files before and after a full run including both write flags.

Documentation

docs/research-toolkit.md — all thirteen with an "answers / cannot answer / run it with"
table, then prose where a reader meets a mechanism rather than a feature list: the invariant
Jesse's marketing skips (a permutation of a multiset sums to the same number, so a reshuffle's
final-equity percentile is exactly 1/2 by construction — trade-order reshuffling can only
speak to the path's shape, never to whether the ending was luck), significance.py's two fee
regimes never averaged and its design-effect correction (a pooled 100 is ~39 effective
observations), the rail explained in each of the three modules' own sections, and a real
refusal transcript generated by running the code rather than written from memory.

Verification

  • uv run pytest -q4466 passed, 3 skipped in 140.83s
  • uv run ruff check keel tests packages — All checks passed!
  • uv run mypy — Success: no issues found in 353 source files

Subcommands shipped: index, significance, pooled-review, throughput, tuning,
factors, independence, plus aliases pbo, deflate, monte-carlo, walk-forward,
lookahead.

🤖 Generated with Claude Code

https://claude.ai/code/session_01T6yA5khYnJ2qzheArRToQ2

eaitbrahim and others added 8 commits August 28, 2026 20:11
…refuses to (#601)

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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T6yA5khYnJ2qzheArRToQ2
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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T6yA5khYnJ2qzheArRToQ2
)

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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T6yA5khYnJ2qzheArRToQ2
…ors, 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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T6yA5khYnJ2qzheArRToQ2
…ort, 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.<module> 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` --
`<Result SystemExit(1)>`); reverted.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T6yA5khYnJ2qzheArRToQ2
…aceholders (#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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T6yA5khYnJ2qzheArRToQ2
)

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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T6yA5khYnJ2qzheArRToQ2
…w 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 = <Result okay>.exit_code

Reverted immediately after confirming the failure.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T6yA5khYnJ2qzheArRToQ2
… 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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T6yA5khYnJ2qzheArRToQ2
@eaitbrahim
eaitbrahim merged commit 7dd56db into main Aug 29, 2026
4 checks passed
@eaitbrahim
eaitbrahim deleted the feat-601-research-front-door branch August 29, 2026 05:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

keel research: thirteen evidence modules with no front door

1 participant