feat(promotion): pool min_trades across same-parameter rules, with a diversity floor (#338) - #343
Merged
Merged
Conversation
…diversity floor (#338)
…, scratch caveat stated, pooled-pass named in words, str-vs-int mismatch case
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
The promotion gate judged
min_trades = 100per rule per product. At the measureddaily rates (1.19–3.20 trades/asset-year, the go-live runbook's own table) a single
product needs 31–84 years to reach 100 trades — the floor was unreachable in a human
timespan, which is why the sandbox's five rules were live-seeded and why #337 adds an
hourly paper profile that makes samples collectable at all. This PR makes the gate able
to COUNT that evidence.
The change is to the gate's unit of evaluation, not its floors — 100 stays 100. This
is the agreement CONTRIBUTING requires for a gate change, recorded in the issue: the
operator approved it on 2026-08-17. The recorded discipline in
promotion.py(thewin-rate axis was relaxed ALONE; axes move only with their own justification) is
respected:
PromotionConfigis untouched.Concretely, when promoting, sibling evidence is gathered — same
kind, sameparams(exact match on the stored JSON-plain form, minus
product_id), differentproduct_id,status
paper— plus the candidate itself, once. The sample-size axis passes if EITHER:min_tradesexactly as today (a rule with nosiblings is judged byte-for-byte as before, and a rule whose own sample is full is
still judged on its own stats — pooling is not a quality rescue for the sample-rich),
OR
MIN_POOLED_PRODUCTS(5) distinctproducts each contributing ≥
MIN_TRADES_PER_PRODUCT_POOLED(10) trades.The diversity floor is the honest discount on pooling: crypto assets correlate, so a
pool of correlated samples carries less information than its trade count claims —
pooled-but-correlated evidence overstates its statistical power. Requiring breadth
(products each with an independently meaningful sample) is how the pooled path pays for
the larger n instead of just collecting it.
Quality floors (expectancy / rr / win rate) are judged on the POOLED aggregates when
path (b) carries the decision; on the rule's own stats otherwise.
The PBO/overfitting gate (G4) stays per-rule, deliberately. It consumes the CSCV
result for the trial matrix the rule's parameters were selected from (
--pbo-session) —evidence about the parameter selection, which is per-parameter-set already — and this
PR does not change its scope. A pooled promotion without a run overfitting check still
refuses, exactly as before.
keel rules promoteprints BOTH readings whenever a pool exists — per-rule n, pooledn, and the per-product census — and failure reasons name their path:
Pooling arithmetic (stated in
pool_stats' docstring)Pooled from the per-result aggregates
BacktestResultactually carries: pooled n = Σn;per-result wins recovered as
round(n × win_rate)(the float round-trip error ofwins/nis ~1e-16, far below 0.5); pooled win rate = Σwins/Σn; pooled avg_win/avg_loss= win-/loss-weighted means; pooled expectancy/profit_factor/avg_mfe/avg_mae = trade-
weighted means / gross sums with
summarize's conventions.max_drawdownandmax_losing_streakare NOT pooled — they are path-dependent and unrecoverable fromaggregates — and are set to 0; the gate reads neither.
One adjacent fix
transition()'s kind-level row lookup ("newest row of this kind") predates multi-rowkinds; with pools made of same-kind sibling rows it would have advanced a sibling the
operator never typed.
keel rules promotenow pins the target row (rule_id=); thekind-level lookup remains for library callers that don't name a row (pinned by test).
Closes #338
Tests-first evidence
Red run (tests written before any implementation):
New coverage: single-product pass/fail unchanged (including "no pooled lines at all");
pooled pass (8×16=128, candidate's own quality fails, pooled quality carries it); pooled
fail on n (8×10=80, reason names path+census); pooled fail on diversity (4×30=120 but
only 4 products); pooled quality judged on pooled aggregates; a full own-sample rule not
rescued by the pool; pooled reading reported even when per-rule passes; params-mismatch
and non-paper rows are not siblings; candidate's own product never double-counted;
duplicate sibling rows pool once;
transitionpromotes via the pooled path and stillrefuses without a run overfitting check; CLI prints both readings + census.
Gates (all must pass)
uv run ruff checkcleanuv run mypycleanuv run pytest -qgreenScope check
leave checked only if true, and if so: cite the source and open the discussion
BEFORE review (CONTRIBUTING.md, "Governance: rulings vs. machinery").