diff --git a/keel/cli.py b/keel/cli.py index d1575c7d..329dfeb5 100644 --- a/keel/cli.py +++ b/keel/cli.py @@ -763,11 +763,14 @@ def assets_holdings(ctx: click.Context, min_balance: str, run_screen: bool) -> N @assets_group.command("discover") @click.option("--quote", default=None, help="Settlement currency (default: config.quote_currency).") @click.option( - "--min-volume-24h", default="1000000", show_default=True, + "--min-volume-24h", + default=str(screen_mod.DiscoveryPolicy().min_quote_24h_volume), + show_default=True, help="Cheap pre-filter on the venue's reported 24h quote volume. Bounds the request count; it " "is NOT a liquidity criterion -- a 24h snapshot is a different statistic from the median the " - "gate applies, so use --probe-liquidity for that. Set equal to the admission floor so " - "discovery cannot be stricter than the criterion it screens for.", + "gate applies, so use --probe-liquidity for that. Deliberately set WELL BELOW the admission " + "floor (1/4 of it), not equal to it: the two measure different things, so an equal threshold " + "still hides assets the gate would admit -- on 2026-08-16 it was hiding three of them.", ) @click.option("--limit", default=25, show_default=True, help="Show at most this many candidates.") @click.option( diff --git a/keel/commands/admission.py b/keel/commands/admission.py index 950a7b76..9dbe0b26 100644 --- a/keel/commands/admission.py +++ b/keel/commands/admission.py @@ -61,23 +61,26 @@ DEFAULT_PROPOSALS_DIR = "~/keel/proposals" #: Mirrors `keel assets discover --min-volume-24h`'s own default (`keel/cli.py::assets_discover`). -#: A literal here, not an import from `keel.cli`, for the same reason `ScreenFn` is injected -#: rather than importing `_screen_product` directly: importing `keel.cli` from this module would -#: create the cycle `cli -> tui -> admission -> cli`. `test_build_discover_report_applies_ -#: default_volume_floor_matching_assets_discover` reads the CLI option's own default and asserts -#: it equals this constant, so the two cannot silently drift apart. -#: Discovery's 24h-volume pre-filter, pinned EQUAL to `ScreenPolicy.min_median_daily_volume` so a -#: sweep can never be stricter than the gate it feeds. It bounds how many products get probed; it -#: is not a liquidity verdict (that is `--probe-liquidity`, which computes the gate's own median). +#: Derived from `DiscoveryPolicy` rather than restated as a literal, so there is one definition of +#: the pre-filter floor and its relationship to the admission floor. Importing `keel.cli` here +#: would create the cycle `cli -> tui -> admission -> cli`; importing `keel.compliance.screen` +#: does not, and this module already depends on it. +#: It bounds how many products get probed; it is NOT a liquidity verdict (that is +#: `--probe-liquidity`, which computes the gate's own median). #: #: Was 5,000,000 until 2026-08-08. At that floor the sweep returned 9 candidates and exactly one #: unsettled survivor; at a lower floor, seven more cleared BOTH mechanical gates -- FET among them #: at $2.94M/24h, i.e. invisible to the sweep while measuring 4.8x the admission floor. The floor, #: not the market, was the binding constraint on the candidate pipeline. #: -#: `tests/commands/test_admission.py` pins this to the CLI option, to `DiscoveryPolicy`'s default -#: and to the admission floor; all four move together or the suite fails. -DEFAULT_MIN_QUOTE_24H_VOLUME = Decimal("1000000") +#: Was then pinned EQUAL to `ScreenPolicy.min_median_daily_volume` until 2026-08-16, on the theory +#: that equality kept the sweep from being stricter than the gate. It did not -- the two are +#: different statistics, and the equal floor was still hiding admissible assets. See +#: `DISCOVERY_FLOOR_MARGIN` for the measurements that changed it. +#: +#: `tests/commands/test_admission.py` pins this to the CLI option and to `DiscoveryPolicy`'s +#: default, and pins the MARGIN against the admission floor; all move together or the suite fails. +DEFAULT_MIN_QUOTE_24H_VOLUME = DiscoveryPolicy().min_quote_24h_volume # -- 2a. shortlist location (offline) ------------------------------------------------------------ diff --git a/keel/compliance/screen.py b/keel/compliance/screen.py index b974c70f..52bc3bec 100644 --- a/keel/compliance/screen.py +++ b/keel/compliance/screen.py @@ -462,6 +462,29 @@ class Candidate: quote_24h_volume: Decimal +#: How far BELOW the admission floor the discovery pre-filter sits. +#: +#: This is a RATIO on purpose, and it used to be 1 (the two were pinned numerically equal) with +#: the stated intent that "discovery cannot be stricter than the criterion it screens for". That +#: intent is right; equality does not achieve it. The pre-filter reads a ONE-DAY venue snapshot +#: while the gate medians `volume * close` over YEARS of history -- same units, different +#: statistics -- so an equal threshold is crossed constantly in both directions by ordinary +#: day-to-day variation, and roughly half those crossings hide an asset the gate would admit. +#: +#: Measured, not assumed. On 2026-08-15 a quiet day put five already-attested assets under the +#: equal floor while their true medians ran 3.08x-6.32x OVER it. On 2026-08-16 a sweep at a +#: lowered floor surfaced three admissible assets that had NEVER been seen in fifteen prior +#: discovery runs -- FIL, OP and JASMY, whose 24h snapshots sat at 0.41x, 0.36x and 0.30x the +#: admission floor while their gate statistics measured 3.48x, 2.59x and 4.16x OVER it. +#: +#: 4 gives ~3x headroom below the lowest ratio actually observed on an admissible asset (0.30x). +#: The cost is bounded and small: on the 2026-08-16 sweep it took the candidate list from 35 to +#: 82 out of 920 venue products, which is still the "cut ~900 to a shortlist" job this filter +#: exists to do. Raise the ratio if admissible assets are still being hidden; lower it only with +#: evidence that probe volume has become the binding constraint. +DISCOVERY_FLOOR_MARGIN = 4 + + @dataclass(frozen=True) class DiscoveryPolicy: """The cheap pre-filter, run on venue metadata BEFORE fetching any history. @@ -471,7 +494,11 @@ class DiscoveryPolicy: """ quote_currency: str = "USD" - min_quote_24h_volume: Decimal = Decimal("1000000") + #: Derived from the admission floor rather than restated, so the two cannot drift and the + #: SAFETY MARGIN between them is the invariant -- see `DISCOVERY_FLOOR_MARGIN`. + min_quote_24h_volume: Decimal = ( + ScreenPolicy().min_median_daily_volume / DISCOVERY_FLOOR_MARGIN + ) def median_daily_quote_volume(candles: Sequence[Any]) -> Decimal: diff --git a/keel/data/market_feed.py b/keel/data/market_feed.py index c6476083..eabe24bc 100644 --- a/keel/data/market_feed.py +++ b/keel/data/market_feed.py @@ -14,6 +14,7 @@ import time from typing import TYPE_CHECKING +from keel.data.history import MAX_CANDLES_PER_REQUEST from keel.types import Candle, Granularity if TYPE_CHECKING: @@ -50,6 +51,33 @@ def _align_up(ts: int, gran_sec: int) -> int: return ((ts + gran_sec - 1) // gran_sec) * gran_sec +def _capped_ranges(start: int, end: int, gran_sec: int) -> list[tuple[int, int]]: + """Split the inclusive `[start, end]` span into requests of at most the venue's cap. + + Coinbase rejects any candles request spanning more than ~350 intervals with a 400 + `INVALID_ARGUMENT`. `history.py` has always paged under that cap; this module did not, + so a single unbounded request was issued no matter how far behind a series had fallen. + That is fine until it isn't: a product only has to go `MAX_CANDLES_PER_REQUEST` bars + stale for the request to 400, and because the exception propagates out of `poll_once` + it takes the whole agent cycle down with it -- for *every* product, not just the stale + one. Observed in production 2026-08-14/15, when ZEC-USD hourly sat 570 bars stale and + stopped the paperforward agent on two consecutive days. + + Returns one `(start, end)` pair when the span already fits, so the ordinary + one-bar-behind poll still costs exactly one request. + """ + if end < start: + return [] + step = MAX_CANDLES_PER_REQUEST * gran_sec + ranges: list[tuple[int, int]] = [] + window_start = start + while window_start <= end: + window_end = min(end, window_start + step - gran_sec) + ranges.append((window_start, window_end)) + window_start = window_end + gran_sec + return ranges + + def _missing_ranges(expected: list[int], present: set[int], gran_sec: int) -> list[tuple[int, int]]: """Group the `expected` ts values not in `present` into contiguous `(start, end)` ranges.""" missing = [ts for ts in expected if ts not in present] @@ -103,15 +131,17 @@ def backfill( c.ts for c in repo.get_candles(product_id, granularity, window_start, latest_closed) } - for range_start, range_end in _missing_ranges(expected, existing, gran_sec): - fetched = client.get_candles(product_id, granularity, range_start, range_end) - gap_candles = [ - c - for c in fetched - if window_start <= c.ts <= latest_closed and c.ts not in existing - ] - if gap_candles: - total_written += repo.upsert_candles(product_id, granularity, gap_candles) + for gap_start, gap_end in _missing_ranges(expected, existing, gran_sec): + # A contiguous gap is itself unbounded, so page it under the venue's cap. + for range_start, range_end in _capped_ranges(gap_start, gap_end, gran_sec): + fetched = client.get_candles(product_id, granularity, range_start, range_end) + gap_candles = [ + c + for c in fetched + if window_start <= c.ts <= latest_closed and c.ts not in existing + ] + if gap_candles: + total_written += repo.upsert_candles(product_id, granularity, gap_candles) return total_written @@ -145,14 +175,18 @@ def poll_once( continue fetch_start = last_ts + gran_sec if last_ts is not None else latest_closed - fetched = client.get_candles(product_id, granularity, fetch_start, latest_closed) - new_candles: list[Candle] = [ - c - for c in fetched - if c.ts <= latest_closed and (last_ts is None or c.ts > last_ts) - ] - if new_candles: - total_written += repo.upsert_candles(product_id, granularity, new_candles) + # Page under the venue's per-request candle cap. A series that has fallen far + # enough behind would otherwise be one oversized request that 400s and, because + # the error propagates, kills the cycle for every other product too. + for range_start, range_end in _capped_ranges(fetch_start, latest_closed, gran_sec): + fetched = client.get_candles(product_id, granularity, range_start, range_end) + new_candles: list[Candle] = [ + c + for c in fetched + if c.ts <= latest_closed and (last_ts is None or c.ts > last_ts) + ] + if new_candles: + total_written += repo.upsert_candles(product_id, granularity, new_candles) return total_written diff --git a/tests/commands/test_admission.py b/tests/commands/test_admission.py index 792add02..c078bf33 100644 --- a/tests/commands/test_admission.py +++ b/tests/commands/test_admission.py @@ -646,18 +646,22 @@ def test_build_discover_report_excludes_allowlist_assets(repo: Repository): def test_build_discover_report_applies_default_volume_floor_matching_assets_discover(repo): - """`keel assets discover --min-volume-24h` defaults to `5000000` -- read straight from the - CLI option's own default (by name, not position, so a decorator reorder cannot silently - break this pin) so the two can never drift apart silently.""" + """`build_discover_report` and `keel assets discover --min-volume-24h` share one floor. + + The CLI option's default is read by name (not position, so a decorator reorder cannot + silently break this pin) and compared against the shared constant rather than a restated + literal -- a literal here went stale once already (this docstring claimed `5000000` long + after the default became `1000000`) and pinned nothing while looking like it did. + """ cli_option = next( p for p in cli_module.assets_discover.params if p.name == "min_volume_24h" ) default_floor = Decimal(cli_option.default) - assert default_floor == Decimal("1000000") + assert default_floor == DEFAULT_MIN_QUOTE_24H_VOLUME config = _config(allowlist=[]) - below = _venue_product("DOGE", volume="999999") - above = _venue_product("SHIB", volume="1000001") + below = _venue_product("DOGE", volume=str(default_floor - 1)) + above = _venue_product("SHIB", volume=str(default_floor + 1)) report = build_discover_report([below, above], config) @@ -732,14 +736,29 @@ def test_every_discovery_floor_default_agrees(): assert DiscoveryPolicy().min_quote_24h_volume == DEFAULT_MIN_QUOTE_24H_VOLUME -def test_the_discovery_floor_matches_the_admission_liquidity_floor(): +def test_the_discovery_floor_sits_safely_below_the_admission_liquidity_floor(): """Discovery should not hide assets the gate would admit. - The sweep's floor is a 24h snapshot and the gate's is a median over history -- different - statistics (which is why `--probe-liquidity` exists). Pinning the two NUMBERS equal keeps the - pre-filter from being stricter than the criterion it screens for: at the old 5x-higher floor, - FET sat at $2.94M/24h and never appeared, while measuring 4.8x the admission floor. + This test used to assert the two floors were EQUAL, on the reasoning that equality kept the + pre-filter from being stricter than the criterion it screens for. The goal was right and the + mechanism was wrong: the sweep's floor is a ONE-DAY snapshot and the gate's is a median over + YEARS (which is why `--probe-liquidity` exists), so equal numbers are crossed constantly by + ordinary variation -- and about half those crossings hide an admissible asset. + + It was hiding real ones. On 2026-08-16, FIL / OP / JASMY sat at 0.41x / 0.36x / 0.30x the + admission floor on 24h volume while their actual gate statistics measured 3.48x / 2.59x / + 4.16x OVER it; none had appeared in fifteen prior discovery runs. So the invariant is now a + MARGIN, not an equality. """ - from keel.compliance.screen import ScreenPolicy + from keel.compliance.screen import DISCOVERY_FLOOR_MARGIN, ScreenPolicy + + admission_floor = ScreenPolicy().min_median_daily_volume + + assert DEFAULT_MIN_QUOTE_24H_VOLUME < admission_floor, ( + "an equal-or-higher pre-filter hides assets the gate would admit" + ) + assert DEFAULT_MIN_QUOTE_24H_VOLUME == admission_floor / DISCOVERY_FLOOR_MARGIN - assert DEFAULT_MIN_QUOTE_24H_VOLUME == ScreenPolicy().min_median_daily_volume + # The lowest 24h/floor ratio observed on an asset that actually cleared the gate was JASMY's + # 0.30x. The pre-filter must stay below that, or the same class of asset is hidden again. + assert DEFAULT_MIN_QUOTE_24H_VOLUME < admission_floor * Decimal("0.30") diff --git a/tests/data/test_market_feed.py b/tests/data/test_market_feed.py index 12f69e56..2a4e9171 100644 --- a/tests/data/test_market_feed.py +++ b/tests/data/test_market_feed.py @@ -203,3 +203,91 @@ def test_is_fresh_false_when_no_candles_stored(repo): assert not is_fresh( repo, "BTC-USD", Granularity.ONE_HOUR, now_ts=NOW, max_age_sec=200 ) + + +# -- venue candle-per-request cap --------------------------------------------- +# +# Coinbase rejects any candles request spanning more than ~350 intervals with a 400 +# `INVALID_ARGUMENT: "number of candles requested should be less than 350"`. `history.py` +# has always paged under that cap; `market_feed` did not, so a product whose series had +# fallen far enough behind made every agent cycle die on the first poll. Observed in +# production 2026-08-14/15: ZEC-USD hourly sat 570 bars stale and took the paperforward +# agent down for two consecutive days. + + +class VenueCappedFakeClient(FakeClient): + """`FakeClient` that fails like the real venue when asked for too many candles. + + A fake that silently serves any span cannot catch this bug -- the old code passed every + existing test precisely because the fakes were more permissive than Coinbase. + """ + + def get_candles( + self, product_id: str, granularity: Granularity, start: int, end: int + ) -> list[Candle]: + span = (end - start) // GRAN_SEC + 1 + if span > 350: + raise RuntimeError( + "400 Client Error: Bad Request " + '{"error":"INVALID_ARGUMENT","error_details":"start and end argument is ' + 'invalid - number of candles requested should be less than 350 "}' + ) + return super().get_candles(product_id, granularity, start, end) + + +def _stale_series(hours_behind: int) -> tuple[dict, int]: + """A full hourly series ending at LATEST_CLOSED, plus the ts of the last *stored* bar.""" + oldest = LATEST_CLOSED - hours_behind * GRAN_SEC + ts_values = list(range(oldest, LATEST_CLOSED + 1, GRAN_SEC)) + series = {("ZEC-USD", Granularity.ONE_HOUR): [_candle(ts) for ts in ts_values]} + return series, oldest + + +def test_poll_once_chunks_requests_under_the_venue_candle_cap(repo): + """The production failure: 570 hourly bars behind is one 570-candle request, and 400s.""" + series, oldest = _stale_series(570) + repo.upsert_candles("ZEC-USD", Granularity.ONE_HOUR, [_candle(oldest)]) + client = VenueCappedFakeClient(series) + + written = poll_once(client, repo, ["ZEC-USD"], [Granularity.ONE_HOUR], now_ts=NOW) + + assert written == 570, "every missing bar should be fetched, across as many calls as it takes" + assert len(client.calls) > 1, "a 570-bar gap cannot be served by a single request" + for _product, _gran, start, end in client.calls: + span = (end - start) // GRAN_SEC + 1 + assert span <= 350, f"requested {span} candles in one call -- the venue rejects >350" + + +def test_poll_once_still_uses_one_call_for_a_small_gap(repo): + """Chunking must not add requests for the ordinary case -- one bar behind is one call.""" + series, oldest = _stale_series(1) + repo.upsert_candles("ZEC-USD", Granularity.ONE_HOUR, [_candle(oldest)]) + client = VenueCappedFakeClient(series) + + written = poll_once(client, repo, ["ZEC-USD"], [Granularity.ONE_HOUR], now_ts=NOW) + + assert written == 1 + assert len(client.calls) == 1 + + +def test_backfill_chunks_requests_under_the_venue_candle_cap(repo): + """`backfill` groups missing bars into contiguous ranges, which are likewise unbounded.""" + hours = 400 + oldest = LATEST_CLOSED - hours * GRAN_SEC + ts_values = list(range(oldest, LATEST_CLOSED + 1, GRAN_SEC)) + series = {("ZEC-USD", Granularity.ONE_HOUR): [_candle(ts) for ts in ts_values]} + client = VenueCappedFakeClient(series) + + backfill( + client, + repo, + ["ZEC-USD"], + [Granularity.ONE_HOUR], + history_days=hours // 24 + 1, + now_ts=NOW, + ) + + assert client.calls, "backfill should have requested something" + for _product, _gran, start, end in client.calls: + span = (end - start) // GRAN_SEC + 1 + assert span <= 350, f"requested {span} candles in one call -- the venue rejects >350"