fix: two silent pipeline failures — an unpaged candle request and an over-tight discovery pre-filter - #294
fix: two silent pipeline failures — an unpaged candle request and an over-tight discovery pre-filter#294eaitbrahim wants to merge 2 commits into
Conversation
`poll_once` asked for every bar between the last stored candle and now in ONE request. Coinbase rejects a span over ~350 intervals with a 400 `INVALID_ARGUMENT: "number of candles requested should be less than 350"`, and because the error propagates out of `poll_once` it does not degrade that one product -- it takes down the whole agent cycle, for every product. `history.py` has paged under this cap since it was written (`MAX_CANDLES_PER_REQUEST = 300`), which is why `keel fetch` never hit it and the agent loop did. Two modules fetch candles; only one knew about the limit. Observed in production. ZEC-USD hourly sat 570 bars stale in `keel.db`, and ZEC is on the paperforward allowlist, so paperforward died on its first poll for two consecutive days -- 10 failures on 2026-08-14, 12 on 2026-08-15. The other 18 allowlisted products were one bar behind and perfectly healthy; one stale series was enough to stop all of them. `backfill` had the same unbounded shape by a different route: `_missing_ranges` groups missing timestamps into CONTIGUOUS ranges, and a contiguous range is itself arbitrarily long. Both call sites now go through `_capped_ranges`. A span that already fits still costs exactly one request, so the ordinary one-bar-behind poll is unchanged -- pinned by `test_poll_once_still_uses_one_call_for_a_small_gap`. The existing fakes could not have caught this: they served any span asked for, so they were strictly more permissive than the venue. The new tests use a `VenueCappedFakeClient` that raises the real 400 above 350 intervals. Verified against the live venue too, replaying the actual 570-bar ZEC gap on a copy of the production DB: 569 candles written across 2 requests of 300 and 269, where `main` issues a single 570 and fails. 2727 -> 2730 tests. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e gate
The pre-filter was pinned NUMERICALLY EQUAL to
`ScreenPolicy.min_median_daily_volume`, with the stated intent that equality
kept discovery from being stricter than the criterion it screens for. The
intent is right. Equality does not achieve it.
The two are different statistics: the pre-filter reads a ONE-DAY venue snapshot
of 24h quote volume, while the gate medians `volume * close` over YEARS of
cached history. Same units, so they look comparable; wildly different
distributions, so an equal threshold is crossed constantly in both directions
by ordinary day-to-day variation. Roughly half those crossings hide an asset
the gate would have admitted, and nothing anywhere reports the exclusion.
Measured, not argued. On 2026-08-15 one quiet day put five already-attested
assets under the equal floor while their true medians ran 3.08x-6.32x OVER it.
That looked like bad luck. On 2026-08-16 a sweep run at a lowered floor
surfaced three assets that had never appeared in fifteen prior discovery runs
and are admissible on the gate's own statistic:
asset 24h snapshot vs floor real gate statistic vs floor
FIL 413,040 0.41x 3,483,442 3.48x
OP 356,063 0.36x 2,585,626 2.59x
JASMY 301,861 0.30x 4,161,283 4.16x
Not considered and rejected -- never seen. The filter that exists only to bound
the request count was silently deciding the candidate universe.
So the invariant becomes a MARGIN rather than an equality, expressed as
`DISCOVERY_FLOOR_MARGIN = 4` and derived from the admission floor instead of
restated, so the two cannot drift and the relationship is what the tests pin.
4 leaves ~3x headroom below the lowest ratio yet observed on an admissible
asset (JASMY's 0.30x). The cost is bounded: on the 2026-08-16 sweep it took the
candidate list from 35 to 82 of 920 venue products, still comfortably the
"cut ~900 down to a shortlist" job the filter is for.
`test_the_discovery_floor_matches_the_admission_liquidity_floor` asserted the
equality and so encoded the bug; it is rewritten to assert the margin, and to
pin the pre-filter below that observed 0.30x ratio.
Also fixes a pin that had already gone stale while looking authoritative:
`test_build_discover_report_applies_default_volume_floor_matching_assets_discover`
restated the floor as a literal and its docstring still claimed `5000000` long
after the default became `1000000`. It now compares against the shared constant.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Superseded — closing rather than rebasing, deliberately. I opened this against
Rebasing would mean resolving conflicts in favour of the weaker version on both counts, so there is nothing here worth carrying forward on those two fronts. One thing in this PR was not covered by #269/#271: For the record, since it may be useful context for #269: the production trigger was |
Two independent bugs found while running
/keel-asset-scouton 2026-08-16. Both were silent, both had been costing real work, and neither is in the trading core.1.
market_feednever paged under the venue's candle cappoll_oncerequested every bar between the last stored candle and now in one call. Coinbase rejects a span over ~350 intervals with a 400INVALID_ARGUMENT, and the exception propagates out ofpoll_once— so it doesn't degrade one product, it kills the whole agent cycle for every product.history.pyhas paged under this cap since it was written (MAX_CANDLES_PER_REQUEST = 300). Two modules fetch candles; only one knew about the limit — which is exactly whykeel fetchalways worked while the agent loop died.Observed in production:
ZEC-USDhourly sat 570 bars stale, and ZEC is on the paperforward allowlist. Paperforward failed on its first poll for two consecutive days — 10 failures on 2026-08-14, 12 on 2026-08-15. The other 18 allowlisted products were one bar behind and perfectly healthy. One stale series stopped all of them.backfillhad the same unbounded shape by a different route:_missing_rangesgroups missing timestamps into contiguous ranges, and a contiguous range is itself arbitrarily long.Verified against the live venue, replaying the actual 570-bar gap on a copy of the production DB: 569 candles written across 2 requests of 300 and 269, where
mainissues a single 570 and 400s.The existing fakes could not have caught this — they served any span asked for, so they were strictly more permissive than the venue. The new tests use a
VenueCappedFakeClientthat raises the real 400.2. The discovery pre-filter was pinned equal to the admission floor
--min-volume-24hdefaulted to exactlyScreenPolicy.min_median_daily_volume, with the stated intent that equality kept discovery from being stricter than the criterion it screens for. The intent is right; equality does not achieve it.The pre-filter reads a one-day venue snapshot; the gate medians
volume × closeover years. Same units, so they look comparable — wildly different distributions, so an equal threshold gets crossed constantly in both directions by ordinary variation, and about half those crossings hide an asset the gate would have admitted. Nothing reports the exclusion.On 2026-08-15 a quiet day put five already-attested assets under the floor while their true medians ran 3.08×–6.32× over it. That looked like bad luck. Then a sweep at a lowered floor surfaced three assets that had never appeared in fifteen prior discovery runs:
Not considered and rejected — never seen. A filter that exists only to bound the request count was silently deciding the candidate universe.
The invariant becomes a margin rather than an equality (
DISCOVERY_FLOOR_MARGIN = 4), derived from the admission floor rather than restated so the two cannot drift. 4 leaves ~3× headroom below the lowest ratio yet observed on an admissible asset. Cost is bounded: the sweep went from 35 to 82 candidates out of 920 products — still the "cut ~900 to a shortlist" job the filter is for.test_the_discovery_floor_matches_the_admission_liquidity_floorasserted the equality and so encoded the bug; it now asserts the margin. Also fixes a pin that had already gone stale while looking authoritative —test_build_discover_report_applies_default_volume_floor_matching_assets_discoverrestated the floor as a literal and its docstring still claimed5000000long after the default became1000000.Checks
pytest— 2730 passed, 1 skipped (2727 → 2730)ruff check— cleanmypy keel— clean, 77 source filesruff formatis not in CI; these files are no worse thanmain(one is better)Note for deployment
The running deployment uses the installed package, not this checkout, so paperforward is not fixed by this merge alone. It was unblocked immediately by closing ZEC's gap with
keel fetch --products ZEC-USD(which uses the already-correct paging) — but the next series to fall behind will reproduce it until this ships.🤖 Generated with Claude Code