From 55e2e439bfca21d74c8b7b836668183eab3035e7 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Sat, 15 Aug 2026 15:50:14 -0400 Subject: [PATCH 1/2] fix(compliance): stop discovery hiding assets the gate would admit `keel assets discover` was silently dropping assets that clear the admission gate comfortably, and its own output gave an operator no way to notice. Two defects, one symptom. (a) A one-day statistic compared against a multi-year threshold. The `--min-volume-24h` default was 1,000,000, pinned EQUAL to the admission floor `ScreenPolicy.min_median_daily_volume`. The intent of that pinning (2026-08-08, superseding a 5,000,000 floor that had hidden FET at $2.94M/24h while it measured 4.8x the admission floor) was right: a pre-filter must never be stricter than the criterion it screens for. The mechanism was wrong. Discovery's number is a 24-hour venue snapshot; admission's is the median of volume x close over ALL cached history. Equal NUMBERS cannot make one non-stricter than the other when the two sides measure different statistics -- a single quiet trading day pushes the snapshot below a floor the asset's own median clears many times over. Measured 2026-08-15, five assets were silently dropped whose real gate statistic sits far ABOVE the admission floor: ATOM 3,077,474 (3.08x), AAVE 6,315,463 (6.32x), BCH 5,464,940 (5.46x), CRV 3,329,753 (3.33x), ALGO 3,780,207 (3.78x). Four of the five had 24h volumes clustered between 852,133 and 979,000 on that one quiet day. The discovery floor now sits an order of magnitude BELOW the admission floor (100,000), and the test that used to assert the two are equal now asserts discovery's is STRICTLY LESS -- preserving 2026-08-08's goal while replacing the mechanism that could not deliver it. The admission floor itself is unchanged: it is the real criterion. (b) Nothing recorded what was excluded. `discover_candidates` dropped products with a bare `continue` and returned only survivors, and the CLI printed only `N venue products -> M candidates`. A filter that can silently remove admissible assets must be auditable from its own output. It now returns a `DiscoveryResult` carrying per-reason exclusion counts (wrong quote currency, not online, trading disabled, view only, already on allowlist, unreadable 24h volume, below the 24h floor), surfaced by both `keel assets discover` and the TUI's discover overlay. The new named return type -- rather than a tuple -- is deliberate: an un-updated caller fails loudly at the attribute access instead of silently mis-indexing. Co-Authored-By: Claude Opus 5 (1M context) --- keel/cli.py | 18 ++-- keel/commands/admission.py | 50 ++++++++-- keel/compliance/screen.py | 142 +++++++++++++++++++++++++++- tests/commands/test_admission.py | 98 +++++++++++++++---- tests/commands/test_tui.py | 1 + tests/compliance/test_assets_cli.py | 29 ++++++ tests/compliance/test_screen.py | 138 ++++++++++++++++++++++++--- 7 files changed, 425 insertions(+), 51 deletions(-) diff --git a/keel/cli.py b/keel/cli.py index d1575c7d..808dd9c2 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="100000", 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, not equal to it: a 24h snapshot and the gate's all-cached-history median are " + "different statistics, so an equal number does not make discovery non-stricter than the gate " + "it feeds -- a quiet trading day can push the snapshot under a floor the asset's own median " + "clears many times over.", ) @click.option("--limit", default=25, show_default=True, help="Show at most this many candidates.") @click.option( @@ -812,15 +815,16 @@ def assets_discover( quote_currency=quote or config.quote_currency, min_quote_24h_volume=Decimal(min_volume_24h), ) - candidates = screen_mod.discover_candidates( + result = screen_mod.discover_candidates( products, policy, exclude_assets=frozenset(config.allowlist) ) click.echo( - f"{len(products)} venue products -> {len(candidates)} candidates " + f"{len(products)} venue products -> {len(result.candidates)} candidates " f"(quote={policy.quote_currency}, 24h volume >= {policy.min_quote_24h_volume:,.0f}, " - f"excluding the current allowlist)\n" + f"excluding the current allowlist)" ) + click.echo(result.excluded.summary_line() + "\n") screen_policy = screen_mod.ScreenPolicy() header = f"{'#':>3} {'product':<14} {'asset':<8} {'24h quote volume':>18}" if probe_history: @@ -832,7 +836,7 @@ def assets_discover( now_ts = int(time.time()) four_years_ago = now_ts - 4 * _DAYS_PER_YEAR * 86400 liquidity_window_start = now_ts - _LIQUIDITY_PROBE_DAYS * 86400 - for index, candidate in enumerate(candidates[:limit], start=1): + for index, candidate in enumerate(result.candidates[:limit], start=1): line = ( f"{index:>3} {candidate.product_id:<14} {candidate.asset:<8} " f"{candidate.quote_24h_volume:>18,.0f}" diff --git a/keel/commands/admission.py b/keel/commands/admission.py index 950a7b76..75136b51 100644 --- a/keel/commands/admission.py +++ b/keel/commands/admission.py @@ -35,6 +35,7 @@ from keel.commands._products import _default_sim_products from keel.compliance.screen import ( Candidate, + DiscoveryExclusions, DiscoveryPolicy, MarketFacts, ScreenResult, @@ -66,18 +67,35 @@ #: 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). +#: +#: Discovery's 24h-volume pre-filter. It bounds how many products get probed for history; 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") +#: The fix that followed pinned this EQUAL to `ScreenPolicy.min_median_daily_volume` +#: (1,000,000), reasoning that a sweep pinned to the gate's own floor could never be stricter +#: than the gate it feeds. That reasoning got the INTENT right and the MECHANISM wrong: the two +#: floors measure DIFFERENT statistics -- this one is a single 24-hour venue snapshot, the +#: admission floor is the median of volume x close over ALL cached history -- so an equal number +#: does nothing to stop a quiet trading day from pushing the snapshot below a floor the asset's +#: own median clears many times over. Measured 2026-08-15: five assets (ATOM, AAVE, BCH, CRV, +#: ALGO) were silently dropped by the equal-floor sweep despite each measuring 3.1x-6.3x the +#: admission floor on the gate's own statistic; four of the five sat in an 852,133-979,000 24h +#: snapshot cluster on that single quiet day. +#: +#: The floor is now strictly BELOW the admission floor, by an order of magnitude, so a quiet-day +#: snapshot has real room before it can hide an asset the gate would admit. See +#: `keel.compliance.screen.DiscoveryPolicy.min_quote_24h_volume`, which carries the identical +#: reasoning next to the number it actually applies. +#: +#: `tests/commands/test_admission.py` pins this to the CLI option, to `DiscoveryPolicy`'s default, +#: and to being strictly less than the admission floor; all of those move together or the suite +#: fails. +DEFAULT_MIN_QUOTE_24H_VOLUME = Decimal("100000") # -- 2a. shortlist location (offline) ------------------------------------------------------------ @@ -380,6 +398,11 @@ class DiscoverReport: venue_product_count: int candidates: list[Candidate] min_quote_24h_volume: Decimal + #: Per-reason exclusion counts over the WHOLE sweep -- every product in `venue_product_count` + #: that did not become a candidate, not just the ones cut by `limit` below. See + #: `build_discover_report`'s docstring for why that distinction matters when reading this + #: field alongside `candidates`. + excluded: DiscoveryExclusions def build_discover_report( @@ -396,7 +419,14 @@ def build_discover_report( Uses `screen.discover_candidates` with a `DiscoveryPolicy` built from `config.quote_currency` and the volume floor, excluding `config.allowlist` -- mirroring `assets_discover` in `keel/cli.py`. `min_quote_24h_volume` defaults to `DEFAULT_MIN_QUOTE_24H_VOLUME`, matching - `assets discover`'s own CLI default (`5000000`). + `assets discover`'s own CLI default (`100000`). + + `DiscoverReport.candidates` is truncated to `limit`, but `DiscoverReport.excluded` is NOT -- + it is the per-reason count over the WHOLE sweep, every product in `venue_product_count` that + did not become a candidate at all. Read the two together with that in mind: `excluded.total` + plus `len(candidates)` before truncation equals `venue_product_count`, not + `len(report.candidates)` plus `excluded.total` -- the shown rows are a further cut of the + survivors, not of the population `excluded` is counted against. """ floor = ( min_quote_24h_volume @@ -404,14 +434,15 @@ def build_discover_report( else DEFAULT_MIN_QUOTE_24H_VOLUME ) policy = DiscoveryPolicy(quote_currency=config.quote_currency, min_quote_24h_volume=floor) - candidates = discover_candidates( + result = discover_candidates( products, policy, exclude_assets=frozenset(a.upper() for a in config.allowlist) ) return DiscoverReport( quote=policy.quote_currency, venue_product_count=len(products), - candidates=candidates[:limit], + candidates=result.candidates[:limit], min_quote_24h_volume=floor, + excluded=result.excluded, ) @@ -425,6 +456,7 @@ def render_discover_report(report: DiscoverReport) -> list[str]: f"{report.venue_product_count} venue products -> {len(report.candidates)} candidates " f"(quote={report.quote}, 24h volume >= {report.min_quote_24h_volume:,.0f}, excluding " "the current allowlist)", + report.excluded.summary_line(), "", f"{'#':>3} {'product':<14} {'asset':<8} {'24h quote volume':>18} name", ] diff --git a/keel/compliance/screen.py b/keel/compliance/screen.py index b974c70f..5feaaa44 100644 --- a/keel/compliance/screen.py +++ b/keel/compliance/screen.py @@ -471,7 +471,22 @@ class DiscoveryPolicy: """ quote_currency: str = "USD" - min_quote_24h_volume: Decimal = Decimal("1000000") + #: DISCOVERY's floor, deliberately an order of magnitude BELOW the admission floor + #: `ScreenPolicy.min_median_daily_volume` (1,000,000), and deliberately NOT equal to it. The + #: two numbers measure different statistics -- this one is a single 24-hour venue snapshot, + #: while the admission floor is the median of volume x close over ALL cached history -- so + #: pinning them equal does not make the pre-filter non-stricter than the gate it feeds: a + #: quiet trading day can push the snapshot below a floor the asset's own median clears many + #: times over. + #: + #: Measured 2026-08-15: four of five wrongly-dropped assets (ATOM, BCH, CRV, ALGO) sat in an + #: 852,133-979,000 24h-snapshot cluster on a single quiet day, while their median-over-history + #: liquidity was 3.1x-6.3x the admission floor. 100,000 sits comfortably below that cluster, + #: with room to spare for a quieter day still. + #: + #: This bounds how many products get probed for history; it is not a liquidity verdict. + #: `--probe-liquidity` computes the gate's own statistic and is the honest estimator of that. + min_quote_24h_volume: Decimal = Decimal("100000") def median_daily_quote_volume(candles: Sequence[Any]) -> Decimal: @@ -496,43 +511,153 @@ def median_daily_quote_volume(candles: Sequence[Any]) -> Decimal: return volumes[len(volumes) // 2] +@dataclass(frozen=True) +class DiscoveryExclusions: + """Per-reason counts of every venue product `discover_candidates` dropped, in the SAME + declaration order the checks run in. + + `discover_candidates` used to drop an excluded product with a bare `continue`, so nothing + recorded WHY a product vanished or how many were lost to each reason -- an operator watching + `900 -> 40` had no way to tell whether the missing 860 were junk (wrong quote currency, + offline, disabled) or real candidates sitting just under the liquidity floor. This type is + the fix: every reason is a named field, counted exactly once per product (the FIRST check it + fails, never a later one it would also have failed), so the sweep's own output can explain + itself. + + Every field defaults to 0 rather than the type being optional or partial, because a reason + reading 0 is itself informative -- it tells the operator that reason was checked and simply + did not fire, which is different from a reason that was never evaluated at all. `counts()` + and `summary_line()` both list every reason unconditionally for the same cause: a summary + line whose shape changes depending on which reasons happened to be nonzero would make the + line itself something an operator has to learn to parse, rather than a fixed shape they can + scan every time. + """ + + wrong_quote_currency: int = 0 + not_online: int = 0 + trading_disabled: int = 0 + view_only: int = 0 + already_on_allowlist: int = 0 + unreadable_volume: int = 0 + below_volume_floor: int = 0 + + @property + def total(self) -> int: + return ( + self.wrong_quote_currency + + self.not_online + + self.trading_disabled + + self.view_only + + self.already_on_allowlist + + self.unreadable_volume + + self.below_volume_floor + ) + + def counts(self) -> tuple[tuple[str, int], ...]: + """`(human label, count)` pairs in declaration order. The single source of truth for the + label text, so `summary_line()` and any future renderer (the TUI overlay, say) cannot + drift into naming the same reason two different ways.""" + return ( + ("wrong quote currency", self.wrong_quote_currency), + ("not online", self.not_online), + ("trading disabled", self.trading_disabled), + ("view only", self.view_only), + ("already on allowlist", self.already_on_allowlist), + ("unreadable 24h volume", self.unreadable_volume), + ("below 24h volume floor", self.below_volume_floor), + ) + + def summary_line(self) -> str: + """`"excluded N: reason1 n1, reason2 n2, ..."` -- every reason named, including the zero + ones, for `counts()`'s reason above.""" + return f"excluded {self.total}: " + ", ".join(f"{label} {n}" for label, n in self.counts()) + + +@dataclass(frozen=True) +class DiscoveryResult: + """`discover_candidates`'s return value: the survivors, plus a full accounting of everyone + who did not survive and why. See `DiscoveryExclusions` for the accounting half.""" + + candidates: list[Candidate] + excluded: DiscoveryExclusions + + def discover_candidates( products: list[dict], policy: DiscoveryPolicy | None = None, exclude_assets: frozenset[str] | None = None, -) -> list[Candidate]: +) -> DiscoveryResult: """Propose candidates from venue metadata. **Proposes only — admits nothing.** §5's asymmetry: a proposal may come from anywhere, but activity may only INCREASE through the deterministic gate. Nothing here checks sector or backing, and nothing here may be read as approval — every survivor still has to clear `screen_asset`, which fails closed without a human attestation. + + Returns a `DiscoveryResult` rather than a bare `list[Candidate]` (its shape before this + accounting existed) precisely so that adding `excluded` could not become a silent trap for an + un-updated caller. A tuple `(candidates, excluded)` would have let an old call site that still + expected a list either mis-index (`result[0]` now the whole result, not the first candidate) + or iterate the wrong thing -- both wrong answers that run without complaint. A named + dataclass makes the same old call site fail LOUDLY at the attribute access + (`AttributeError: 'DiscoveryResult' object has no attribute 'asset'`) instead of silently + reading nonsense, which is the same fail-closed instinct the rest of this module applies to + admission itself. + + Each dropped product counts against the FIRST criterion order below that it fails, never a + later one it would also have failed -- so a product excluded for the wrong quote currency is + not ALSO counted as thin, even though it may be. `trading_disabled` and `is_disabled` are two + venue flags that mean the same thing to an operator and both land in the single + `trading_disabled` reason; a volume that fails to parse as a `Decimal` lands in + `unreadable_volume`, distinct from `below_volume_floor`, which is a volume that parsed fine + but did not clear `policy.min_quote_24h_volume`. + + Deliberately PURE and OFFLINE: no network call, no DB read or write, no logging. `products` + is the caller's already-fetched venue metadata, and this function's only job is to filter and + count it -- see the module-level split between computed facts and attested judgement for why + keeping side effects out of a function like this matters generally, and `keel/commands/ + admission.py`'s module docstring for why THIS function in particular has to be safely + callable with nothing but a fake product list in a test. """ policy = policy or DiscoveryPolicy() exclude = exclude_assets or frozenset() out: list[Candidate] = [] + wrong_quote_currency = 0 + not_online = 0 + trading_disabled = 0 + view_only = 0 + already_on_allowlist = 0 + unreadable_volume = 0 + below_volume_floor = 0 for product in products: product_id = product.get("product_id") or "" if (product.get("quote_currency_id") or "").upper() != policy.quote_currency.upper(): + wrong_quote_currency += 1 continue if product.get("status") != "online": + not_online += 1 continue if product.get("trading_disabled") or product.get("is_disabled"): + trading_disabled += 1 continue if product.get("view_only"): + view_only += 1 continue asset = product_id.split("-")[0] if asset in exclude: + already_on_allowlist += 1 continue raw_volume = product.get("quote_24h_volume") try: volume = Decimal(str(raw_volume)) except (TypeError, ArithmeticError, ValueError): + unreadable_volume += 1 continue if volume < policy.min_quote_24h_volume: + below_volume_floor += 1 continue out.append( @@ -544,4 +669,15 @@ def discover_candidates( ) ) - return sorted(out, key=lambda c: c.quote_24h_volume, reverse=True) + return DiscoveryResult( + candidates=sorted(out, key=lambda c: c.quote_24h_volume, reverse=True), + excluded=DiscoveryExclusions( + wrong_quote_currency=wrong_quote_currency, + not_online=not_online, + trading_disabled=trading_disabled, + view_only=view_only, + already_on_allowlist=already_on_allowlist, + unreadable_volume=unreadable_volume, + below_volume_floor=below_volume_floor, + ), + ) diff --git a/tests/commands/test_admission.py b/tests/commands/test_admission.py index 792add02..032c3777 100644 --- a/tests/commands/test_admission.py +++ b/tests/commands/test_admission.py @@ -646,18 +646,20 @@ 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.""" + """`keel assets discover --min-volume-24h`'s default and `build_discover_report`'s own + default must be the SAME number -- read straight from the CLI option's own default (by + name, not position, so a decorator reorder cannot silently break this pin) and compare it + against `DEFAULT_MIN_QUOTE_24H_VOLUME` rather than a hardcoded literal, so a future change + to either constant fails this test instead of silently drifting the two apart.""" 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="99999") + above = _venue_product("SHIB", volume="100001") report = build_discover_report([below, above], config) @@ -703,6 +705,42 @@ def test_render_discover_report_shows_table_and_ends_with_proposals_not_admissio assert "keel assets attest" in text +def test_build_discover_report_excluded_carries_per_reason_counts(repo: Repository): + """`DiscoverReport.excluded` is the thing that makes a `900 -> 40` sweep summary honest -- + without it, nothing distinguishes 860 products dropped for being junk (wrong quote + currency, offline, already on the allowlist) from 860 products dropped just under the + liquidity floor. One allowlisted product and one below-floor product are enough to pin that + each reason lands in its own bucket rather than a single opaque total.""" + config = _config(allowlist=["BTC"]) + below_floor = _venue_product("DOGE", volume="1") + already_allowlisted = _venue_product("BTC") + survivor = _venue_product("SOL") + + report = build_discover_report([below_floor, already_allowlisted, survivor], config) + + assert report.excluded.already_on_allowlist == 1 + assert report.excluded.below_volume_floor == 1 + assert [c.asset for c in report.candidates] == ["SOL"] + + +def test_render_discover_report_includes_the_exclusion_summary(repo: Repository): + """The rendered report must surface WHY products were dropped, not just how many candidates + survived -- otherwise an operator reading `900 -> 40` has no way to tell junk from + near-misses on the floor.""" + config = _config(allowlist=["BTC"]) + below_floor = _venue_product("DOGE", volume="1") + already_allowlisted = _venue_product("BTC") + survivor = _venue_product("SOL") + + report = build_discover_report([below_floor, already_allowlisted, survivor], config) + lines = render_discover_report(report) + text = "\n".join(lines) + + assert report.excluded.summary_line() in text + assert "already on allowlist 1" in text + assert "below 24h volume floor 1" in text + + def test_render_discover_report_never_includes_probe_history_marker(): """Out of scope by design: `--probe-history` is one extra network request per candidate, which this offline module must never make.""" @@ -717,12 +755,15 @@ def test_render_discover_report_never_includes_probe_history_marker(): def test_every_discovery_floor_default_agrees(): - """Three modules carry this default; a drift between them is silent and changes the sweep. - - `cli.assets_discover`'s option, `admission.DEFAULT_MIN_QUOTE_24H_VOLUME` and - `screen.DiscoveryPolicy` each name a floor. The first two were already pinned to each other; - `DiscoveryPolicy` was not, so a caller constructing one directly (as `cli.assets_discover` - does) could silently use a different floor from `build_discover_report`. + """Three DISCOVERY places carry this default; a drift between them is silent and changes + the sweep. `cli.assets_discover`'s `--min-volume-24h` option, `admission. + DEFAULT_MIN_QUOTE_24H_VOLUME` and `screen.DiscoveryPolicy.min_quote_24h_volume` each name + the same pre-filter floor and must move together. + + These three are deliberately NOT pinned to the admission floor + (`ScreenPolicy.min_median_daily_volume`) -- see + `test_the_discovery_floor_is_strictly_below_the_admission_liquidity_floor` for why equal + numbers there would be wrong, not merely a coincidence worth asserting. """ from keel.compliance.screen import DiscoveryPolicy @@ -732,14 +773,31 @@ 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(): - """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. +def test_the_discovery_floor_is_strictly_below_the_admission_liquidity_floor(): + """Discovery's pre-filter must not be able to hide an asset the gate would admit. + + The sweep's floor (`DEFAULT_MIN_QUOTE_24H_VOLUME`) and the gate's + (`ScreenPolicy.min_median_daily_volume`) are DIFFERENT statistics: a 24h venue snapshot + versus the median of volume x close over all cached history. Pinning them to the SAME + number, as this codebase used to, does not make the pre-filter non-stricter -- a quiet + trading day can push the snapshot below a floor the asset's own median clears many times + over. + + This was tried twice and failed twice. First at 5,000,000, retired 2026-08-08: FET sat at + $2.94M/24h and never appeared in a sweep, despite measuring 4.8x the admission floor on the + gate's own statistic. Lowering the two NUMBERS to agree (1,000,000, matching the admission + floor exactly) looked like a fix but was not one -- it only shrank the gap, because equal + numbers on different statistics still let a quiet day hide an asset. Measured again on + 2026-08-15: ATOM (3.08x the admission floor on its real median), AAVE (6.32x), BCH (5.46x), + CRV (3.33x) and ALGO (3.78x) were all silently dropped, four of them with 24h snapshots + clustered between 852,133 and 979,000 on a single quiet day -- comfortably under the + 1,000,000 floor despite being multiples above the admission requirement on the statistic + that actually gates them. + + The corrected mechanism: discovery's floor must be STRICTLY LESS than the gate's, not + merely equal to it, precisely because the two sides measure different things and only a + genuine margin protects a quiet-day snapshot from undercutting a healthy median. """ from keel.compliance.screen import ScreenPolicy - assert DEFAULT_MIN_QUOTE_24H_VOLUME == ScreenPolicy().min_median_daily_volume + assert DEFAULT_MIN_QUOTE_24H_VOLUME < ScreenPolicy().min_median_daily_volume diff --git a/tests/commands/test_tui.py b/tests/commands/test_tui.py index 0f70d190..746adc49 100644 --- a/tests/commands/test_tui.py +++ b/tests/commands/test_tui.py @@ -2096,6 +2096,7 @@ def test_build_discover_overlay_with_report_is_nonempty_titled_and_headed() -> N venue_product_count=900, candidates=[candidate], min_quote_24h_volume=Decimal("5000000"), + excluded=screen_mod.DiscoveryExclusions(below_volume_floor=899), ) lines = build_discover_overlay(report) diff --git a/tests/compliance/test_assets_cli.py b/tests/compliance/test_assets_cli.py index a3207a1e..c4960218 100644 --- a/tests/compliance/test_assets_cli.py +++ b/tests/compliance/test_assets_cli.py @@ -419,6 +419,35 @@ def test_discover_proposes_and_says_so_loudly(tmp_path, valid_config_path, monke assert "attest" in result.output +def test_discover_reports_the_exclusion_summary(tmp_path, valid_config_path, monkeypatch): + """`discover_candidates` used to drop excluded products with a bare `continue`, so `keel + assets discover`'s output never said WHY a product vanished between the venue's product + count and the candidate table -- only the bare `900 -> 40` header line. This pins that the + CLI now echoes a per-reason summary alongside that header: one product survives, one is + excluded for the wrong quote currency, and one is excluded for sitting below the 24h + volume floor.""" + db_path = tmp_path / "t.db" + _repo_at(db_path) + venue = _FakeVenue( + [ + _venue_product("SOL-USD", "50000000"), + _venue_product("EURPAIR-EUR", "50000000", quote_currency_id="EUR"), + _venue_product("THIN-USD", "1"), + ] + ) + monkeypatch.setattr(cli_module, "_build_broker", lambda config: venue) + + result = CliRunner().invoke( + cli, ["--db", str(db_path), "--config", str(valid_config_path), "assets", "discover"] + ) + + assert result.exit_code == 0, result.output + assert "SOL" in result.output + assert "wrong quote currency 1" in result.output + assert "below 24h volume floor 1" in result.output + assert "excluded 2" in result.output + + def test_discover_excludes_the_current_allowlist(tmp_path, valid_config_path, monkeypatch): db_path = tmp_path / "t.db" _repo_at(db_path) diff --git a/tests/compliance/test_screen.py b/tests/compliance/test_screen.py index 86893598..3aeda027 100644 --- a/tests/compliance/test_screen.py +++ b/tests/compliance/test_screen.py @@ -543,14 +543,14 @@ def test_discovery_keeps_liquid_online_products_in_the_settlement_currency(): from keel.compliance.screen import discover_candidates found = discover_candidates([_product()]) - assert [c.asset for c in found] == ["SOL"] + assert [c.asset for c in found.candidates] == ["SOL"] def test_discovery_drops_the_wrong_quote_currency(): from keel.compliance.screen import discover_candidates - assert discover_candidates([_product(quote="USDC")]) == [] - assert discover_candidates([_product(quote="BTC")]) == [] + assert discover_candidates([_product(quote="USDC")]).candidates == [] + assert discover_candidates([_product(quote="BTC")]).candidates == [] def test_discovery_drops_untradable_products(): @@ -562,20 +562,20 @@ def test_discovery_drops_untradable_products(): {"is_disabled": True}, {"view_only": True}, ): - assert discover_candidates([_product(**kwargs)]) == [], kwargs + assert discover_candidates([_product(**kwargs)]).candidates == [], kwargs def test_discovery_drops_thin_products(): from keel.compliance.screen import discover_candidates - assert discover_candidates([_product(volume="100")]) == [] + assert discover_candidates([_product(volume="100")]).candidates == [] def test_discovery_survives_a_malformed_volume_rather_than_crashing(): from keel.compliance.screen import discover_candidates - assert discover_candidates([_product(volume=None)]) == [] - assert discover_candidates([_product(volume="n/a")]) == [] + assert discover_candidates([_product(volume=None)]).candidates == [] + assert discover_candidates([_product(volume="n/a")]).candidates == [] def test_discovery_excludes_assets_we_already_hold(): @@ -584,7 +584,7 @@ def test_discovery_excludes_assets_we_already_hold(): found = discover_candidates( [_product("BTC-USD"), _product("SOL-USD")], exclude_assets=frozenset({"BTC"}) ) - assert [c.asset for c in found] == ["SOL"] + assert [c.asset for c in found.candidates] == ["SOL"] def test_discovery_ranks_by_liquidity(): @@ -593,14 +593,14 @@ def test_discovery_ranks_by_liquidity(): found = discover_candidates( [_product("A-USD", volume="10000000"), _product("B-USD", volume="90000000")] ) - assert [c.asset for c in found] == ["B", "A"] + assert [c.asset for c in found.candidates] == ["B", "A"] def test_discovery_proposes_but_never_admits(): """A discovered candidate is still REJECTED by the screen until a human attests it.""" from keel.compliance.screen import discover_candidates - (candidate,) = discover_candidates([_product()]) + (candidate,) = discover_candidates([_product()]).candidates result = screen_asset(_facts(asset=candidate.asset), None) assert result.admitted is False @@ -611,10 +611,124 @@ def test_discovery_matches_the_quote_currency_case_insensitively(): from keel.compliance.screen import DiscoveryPolicy, discover_candidates lowercase_venue = _product(pid="SOL-USD", quote="usd") - assert discover_candidates([lowercase_venue]), "lowercase venue quote id dropped everything" + assert discover_candidates( + [lowercase_venue] + ).candidates, "lowercase venue quote id dropped everything" assert discover_candidates( [_product(pid="SOL-USD", quote="USD")], DiscoveryPolicy(quote_currency="usd") - ), "lowercase configured quote currency dropped everything" + ).candidates, "lowercase configured quote currency dropped everything" + + +def test_a_quiet_days_snapshot_no_longer_hides_an_asset_the_gate_would_admit(): + """The 2026-08-15 regression this default change fixes. + + `--min-volume-24h` used to default to `Decimal("1000000")`, pinned EQUAL to the admission + floor `ScreenPolicy.min_median_daily_volume`. That equality does not make the pre-filter + non-stricter than the gate it feeds, because the two sides measure DIFFERENT statistics: + discovery's `quote_24h_volume` is a single 24-hour venue snapshot, while the gate's + `median_daily_volume` is the median of volume x close over ALL cached history. A quiet + trading day can push the snapshot below a floor the asset clears comfortably on a typical + day, and an equal number does nothing to prevent that. + + Measured on 2026-08-15, five assets were silently dropped by discovery at the old + 1,000,000 floor despite the gate's own statistic sitting far above the admission floor: + ATOM 3,077,474 (3.08x), AAVE 6,315,463 (6.32x), BCH 5,464,940 (5.46x), CRV 3,329,753 + (3.33x) and ALGO 3,780,207 (3.78x). Four of the five -- all but AAVE -- had 24h snapshots + clustered between 852,133 and 979,000 on that single quiet day, i.e. comfortably below the + old floor while their median-over-history liquidity was multiples of the admission + requirement. + + This test uses ATOM's actual measured 24h figure, `852133`: well under the OLD floor of + 1,000,000 but above the new default of 100,000. Under the default `DiscoveryPolicy()` it + must survive the pre-filter, so the sweep can no longer hide an asset the gate would admit. + """ + from keel.compliance.screen import discover_candidates + + found = discover_candidates([_product(pid="ATOM-USD", volume="852133")]) + + assert [c.asset for c in found.candidates] == ["ATOM"] + + +def test_discovery_counts_every_exclusion_by_reason(): + """`discover_candidates` used to drop excluded products with a bare `continue`, so nothing + recorded WHY a product was excluded or how many were. That made a quiet-day floor problem + (see `test_a_quiet_days_snapshot_no_longer_hides_an_asset_the_gate_would_admit`) invisible + in the sweep's own output -- an operator watching `900 -> 40` had no way to tell whether the + missing 860 were junk (wrong quote currency, offline, disabled) or real candidates sitting + just under the floor. + + Exactly ONE product is fed per reason, first-match-wins in the declaration order of + `DiscoveryExclusions`, plus one clean survivor -- so every count below is 1, the total is the + number of reasons, and a product excluded for one reason is pinned NOT to be double-counted + against a later check it would also fail. The `trading_disabled` reason covers two venue + flags (`trading_disabled` and `is_disabled`) and is therefore fed only its first variant + here; that the second lands in the same bucket rather than a reason of its own is pinned + separately by `test_discovery_counts_both_trading_disabled_flag_variants_together`, which + keeps this test's one-product-per-reason arithmetic honest. + """ + from keel.compliance.screen import discover_candidates + + products = [ + _product("WRONGQ-USD", quote="EUR"), + _product("OFFLINE-USD", status="offline"), + _product("HALTED-USD", trading_disabled=True), + _product("VIEWONLY-USD", view_only=True), + _product("BTC-USD"), # excluded via `exclude_assets` below: already on the allowlist + _product("BADVOL-USD", volume="not-a-number"), + _product("THIN-USD", volume="1"), + _product("SOL-USD"), # the one survivor + ] + + result = discover_candidates(products, exclude_assets=frozenset({"BTC"})) + + assert result.excluded.wrong_quote_currency == 1 + assert result.excluded.not_online == 1 + assert result.excluded.trading_disabled == 1 + assert result.excluded.view_only == 1 + assert result.excluded.already_on_allowlist == 1 + assert result.excluded.unreadable_volume == 1 + assert result.excluded.below_volume_floor == 1 + assert result.excluded.total == 7 + assert [c.asset for c in result.candidates] == ["SOL"] + + +def test_discovery_counts_both_trading_disabled_flag_variants_together(): + """`trading_disabled` covers BOTH the `trading_disabled` and `is_disabled` product flags -- + feeding one of each must total 2 under the single `trading_disabled` reason, not split + across a reason that does not exist.""" + from keel.compliance.screen import discover_candidates + + products = [ + _product("HALTED-USD", trading_disabled=True), + _product("DISABLED-USD", is_disabled=True), + ] + + result = discover_candidates(products) + + assert result.excluded.trading_disabled == 2 + assert result.excluded.total == 2 + assert result.candidates == [] + + +def test_discovery_exclusions_summary_line_names_every_reason_and_the_total(): + from keel.compliance.screen import DiscoveryExclusions + + exclusions = DiscoveryExclusions( + wrong_quote_currency=1, + not_online=1, + trading_disabled=1, + view_only=1, + already_on_allowlist=1, + unreadable_volume=1, + below_volume_floor=1, + ) + + line = exclusions.summary_line() + + assert line == ( + "excluded 7: wrong quote currency 1, not online 1, trading disabled 1, view only 1, " + "already on allowlist 1, unreadable 24h volume 1, below 24h volume floor 1" + ) # -- split_failures / missing_history_lines ------------------------------------------------- From 5cca15ae1c794e31ed61672391876d4aec965cc5 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Sat, 15 Aug 2026 16:02:34 -0400 Subject: [PATCH 2/2] fix(compliance): stop discover truncating its candidate list silently PR #270 lowered the discovery pre-filter so the five assets it exists to surface (ATOM, AAVE, BCH, CRV, ALGO) become candidates again. But `keel assets discover` sorts candidates by descending 24h volume and defaults --limit to 25, which was sized for the ~35-candidate sweeps the OLD 1,000,000 floor produced. At the new 100,000 floor a typical sweep is ~130 candidates, so the five recovered assets land at ranks 33-59 -- below the CLI's own default view, cut off with no indication anything was hidden. Same defect class the PR exists to fix, one step later in the pipeline. Two changes, both in `assets_discover` (keel/cli.py) and the offline `build_discover_report`/`render_discover_report` path it shares with the TUI's discover overlay (keel/commands/admission.py): - Never truncate silently. When more candidates survive than `--limit` shows, say so: how many exist, how many are shown, that --limit controls it. `render_discover_report` derives the pre-truncation survivor count from `venue_product_count - excluded.total` rather than adding a redundant field to `DiscoverReport`. - Raise the default --limit from 25 to 100. Verified --limit is applied before the probe loop in both paths, so this is free with neither probe flag (`assets discover` still makes exactly one venue request regardless of --limit). --probe-history/--probe-liquidity each cost one request per candidate SHOWN, so combining a probe with a large --limit multiplies the request count -- called out in --limit's own help text. Co-Authored-By: Claude Opus 5 (1M context) --- keel/cli.py | 28 +++++++++++++-- keel/commands/admission.py | 52 +++++++++++++++++++++++---- tests/commands/test_admission.py | 50 ++++++++++++++++++++++++++ tests/compliance/test_assets_cli.py | 54 +++++++++++++++++++++++++++++ 4 files changed, 175 insertions(+), 9 deletions(-) diff --git a/keel/cli.py b/keel/cli.py index 808dd9c2..32ab1029 100644 --- a/keel/cli.py +++ b/keel/cli.py @@ -772,7 +772,20 @@ def assets_holdings(ctx: click.Context, min_balance: str, run_screen: bool) -> N "it feeds -- a quiet trading day can push the snapshot under a floor the asset's own median " "clears many times over.", ) -@click.option("--limit", default=25, show_default=True, help="Show at most this many candidates.") +@click.option( + # Raised from 25 to 100 on 2026-08-15: lowering --min-volume-24h's floor (see that option's + # own help text) grew a typical sweep from ~35 candidates to ~130, sorted by descending 24h + # volume, so the assets that floor change exists to surface can land well past rank 25 and + # get cut off by this option before an operator ever sees them. 100 shows nearly all of a + # typical sweep at no extra cost: with NEITHER probe flag, `discover` makes exactly ONE venue + # request regardless of --limit -- filtering and sorting are local. --probe-history and + # --probe-liquidity are the ones with a per-row cost, called out below. + "--limit", default=100, show_default=True, + help="Show at most this many candidates. With neither --probe-history nor --probe-liquidity, " + "raising this costs nothing extra -- discovery still makes exactly one venue request. Each " + "probe flag adds one venue request PER CANDIDATE SHOWN (two requests per row if both are " + "given), so a large --limit combined with a probe flag multiplies the request count.", +) @click.option( "--probe-history", is_flag=True, @@ -824,7 +837,18 @@ def assets_discover( f"(quote={policy.quote_currency}, 24h volume >= {policy.min_quote_24h_volume:,.0f}, " f"excluding the current allowlist)" ) - click.echo(result.excluded.summary_line() + "\n") + click.echo(result.excluded.summary_line()) + # Never truncate silently: `result.candidates` above is the FULL survivor list (only the + # table loop below is cut to `limit`), so if there are more survivors than `limit` allows, + # say so explicitly -- how many exist, how many are about to be shown, and that --limit is + # the knob. A silent cap here would be exactly the defect class this command's own fix + # (the --min-volume-24h floor) exists to eliminate, just moved one step later in the pipeline. + if len(result.candidates) > limit: + click.echo( + f"showing {limit} of {len(result.candidates)} candidates -- raise --limit to see " + "the rest." + ) + click.echo("") screen_policy = screen_mod.ScreenPolicy() header = f"{'#':>3} {'product':<14} {'asset':<8} {'24h quote volume':>18}" if probe_history: diff --git a/keel/commands/admission.py b/keel/commands/admission.py index 75136b51..caa71162 100644 --- a/keel/commands/admission.py +++ b/keel/commands/admission.py @@ -97,6 +97,27 @@ #: fails. DEFAULT_MIN_QUOTE_24H_VOLUME = Decimal("100000") +#: Mirrors `keel assets discover --limit`'s own default (`keel/cli.py::assets_discover`), for the +#: same "literal here, not an import from `keel.cli`" reason `DEFAULT_MIN_QUOTE_24H_VOLUME` gives +#: above. `test_build_discover_report_applies_default_limit_matching_assets_discover` pins the two +#: together. +#: +#: Was 25, set back when `--min-volume-24h`'s floor was 1,000,000 and a sweep returned ~35 +#: candidates -- 25 showed nearly all of them. Lowering that floor to 100,000 (see +#: `DEFAULT_MIN_QUOTE_24H_VOLUME` above) grew a typical sweep to ~130 candidates, all sorted by +#: descending 24h volume, so the five assets that floor change exists to surface (ATOM, AAVE, +#: BCH, CRV, ALGO -- see that constant's docstring) landed at ranks 33-59: below the ~35 still +#: above the OLD floor, and past a limit of 25. The floor fix was real but invisible at the +#: operator's own default view. +#: +#: 100 costs nothing extra on its own: with neither `--probe-history` nor `--probe-liquidity`, +#: `assets discover` makes exactly ONE venue request (`list_products`) regardless of `--limit` -- +#: the candidate list is filtered and sorted locally. The two probe flags are the ones with a +#: per-row cost (one venue request EACH per candidate SHOWN, so two together), which is why that +#: trade-off is called out in `--limit`'s own `help=` text rather than left for an operator to +#: discover by combining the flags and watching the request count climb. +DEFAULT_DISCOVER_LIMIT = 100 + # -- 2a. shortlist location (offline) ------------------------------------------------------------ @@ -409,7 +430,7 @@ def build_discover_report( products: list[dict], config: Config, *, - limit: int = 25, + limit: int = DEFAULT_DISCOVER_LIMIT, min_quote_24h_volume: Decimal | None = None, ) -> DiscoverReport: """PURE over `products` -- the caller's already-fetched venue metadata. Builds no broker and @@ -419,14 +440,18 @@ def build_discover_report( Uses `screen.discover_candidates` with a `DiscoveryPolicy` built from `config.quote_currency` and the volume floor, excluding `config.allowlist` -- mirroring `assets_discover` in `keel/cli.py`. `min_quote_24h_volume` defaults to `DEFAULT_MIN_QUOTE_24H_VOLUME`, matching - `assets discover`'s own CLI default (`100000`). + `assets discover`'s own CLI default (`100000`); `limit` defaults to `DEFAULT_DISCOVER_LIMIT`, + matching its `--limit` default (`100`). `DiscoverReport.candidates` is truncated to `limit`, but `DiscoverReport.excluded` is NOT -- it is the per-reason count over the WHOLE sweep, every product in `venue_product_count` that did not become a candidate at all. Read the two together with that in mind: `excluded.total` plus `len(candidates)` before truncation equals `venue_product_count`, not `len(report.candidates)` plus `excluded.total` -- the shown rows are a further cut of the - survivors, not of the population `excluded` is counted against. + survivors, not of the population `excluded` is counted against. `render_discover_report` + relies on exactly that identity (`venue_product_count - excluded.total` recovers the + pre-truncation survivor count) to report when `limit` has cut the table, without this + dataclass needing a redundant field to carry the same number twice. """ floor = ( min_quote_24h_volume @@ -451,15 +476,28 @@ def render_discover_report(report: DiscoverReport) -> list[str]: with the SAME loud warning `keel assets discover` prints -- these are PROPOSALS, not admissions. Deliberately omits `--probe-history`'s per-candidate marker column: that is an extra network request per candidate, out of scope for this offline module (the caller already - made the one network call this workflow needs, to fetch `products`).""" + made the one network call this workflow needs, to fetch `products`). + + Also never truncates SILENTLY: `report.candidates` is already cut to whatever `limit` + `build_discover_report` was called with, and `venue_product_count - excluded.total` recovers + how many survivors there were before that cut (see `build_discover_report`'s docstring for + the identity this leans on). When that is more than `len(report.candidates)`, a line says so + -- how many candidates exist, how many are shown, and that `--limit`/`limit=` controls it -- + so a truncated table can never read as the whole candidate set.""" + survivor_count = report.venue_product_count - report.excluded.total lines = [ - f"{report.venue_product_count} venue products -> {len(report.candidates)} candidates " + f"{report.venue_product_count} venue products -> {survivor_count} candidates " f"(quote={report.quote}, 24h volume >= {report.min_quote_24h_volume:,.0f}, excluding " "the current allowlist)", report.excluded.summary_line(), - "", - f"{'#':>3} {'product':<14} {'asset':<8} {'24h quote volume':>18} name", ] + if survivor_count > len(report.candidates): + lines.append( + f"showing {len(report.candidates)} of {survivor_count} candidates -- raise " + "--limit to see the rest." + ) + lines.append("") + lines.append(f"{'#':>3} {'product':<14} {'asset':<8} {'24h quote volume':>18} name") for index, candidate in enumerate(report.candidates, start=1): lines.append( f"{index:>3} {candidate.product_id:<14} {candidate.asset:<8} " diff --git a/tests/commands/test_admission.py b/tests/commands/test_admission.py index 032c3777..61492f20 100644 --- a/tests/commands/test_admission.py +++ b/tests/commands/test_admission.py @@ -26,6 +26,7 @@ import keel.cli as cli_module from keel.commands.admission import ( + DEFAULT_DISCOVER_LIMIT, DEFAULT_MIN_QUOTE_24H_VOLUME, DEFAULT_PROPOSALS_DIR, DiscoverReport, @@ -692,6 +693,55 @@ def test_build_discover_report_limit_respected(repo: Repository): assert report.candidates[1].quote_24h_volume >= report.candidates[2].quote_24h_volume +def test_build_discover_report_applies_default_limit_matching_assets_discover(): + """`keel assets discover --limit`'s default and `build_discover_report`'s own default must + be the SAME number, for the identical reason + `test_build_discover_report_applies_default_volume_floor_matching_assets_discover` pins the + volume floor: read straight from the CLI option's own default, compared against + `DEFAULT_DISCOVER_LIMIT` rather than a hardcoded literal, so a future change to either one + fails this test instead of silently drifting the two apart.""" + cli_option = next(p for p in cli_module.assets_discover.params if p.name == "limit") + assert cli_option.default == DEFAULT_DISCOVER_LIMIT + + +def test_default_discover_limit_is_100(): + """Pins the new default value itself. Raised from 25 now that a lower --min-volume-24h + surfaces ~130 candidates instead of ~35 -- 25 would hide most of a typical sweep.""" + assert DEFAULT_DISCOVER_LIMIT == 100 + + +def test_render_discover_report_states_total_and_shown_when_limit_truncates(repo: Repository): + """The load-bearing case: more candidates exist than `limit` shows. The rendered output must + say the true survivor count, the shown count, and that --limit controls the cut -- never let + a truncated table read as the whole candidate set.""" + config = _config(allowlist=[]) + products = [_venue_product(f"COIN{i}", volume=str(10_000_000 + i)) for i in range(10)] + + report = build_discover_report(products, config, limit=3) + lines = render_discover_report(report) + text = "\n".join(lines) + + assert "10 venue products -> 10 candidates" in text + assert "showing 3 of 10 candidates" in text + assert "--limit" in text + assert len(report.candidates) == 3 + + +def test_render_discover_report_no_truncation_notice_when_everything_fits(repo: Repository): + """No false alarm: when every survivor fits under `limit`, nothing should claim a cut + happened.""" + config = _config(allowlist=[]) + products = [_venue_product(f"COIN{i}", volume=str(10_000_000 + i)) for i in range(3)] + + report = build_discover_report(products, config, limit=25) + lines = render_discover_report(report) + text = "\n".join(lines) + + assert "3 venue products -> 3 candidates" in text + assert "showing" not in text + assert len(report.candidates) == 3 + + def test_render_discover_report_shows_table_and_ends_with_proposals_not_admissions_warning(): config = _config(allowlist=[]) products = [_venue_product("DOGE")] diff --git a/tests/compliance/test_assets_cli.py b/tests/compliance/test_assets_cli.py index c4960218..400e5ef8 100644 --- a/tests/compliance/test_assets_cli.py +++ b/tests/compliance/test_assets_cli.py @@ -463,6 +463,60 @@ def test_discover_excludes_the_current_allowlist(tmp_path, valid_config_path, mo assert "BTC-USD" not in result.output +def test_discover_default_limit_is_100(): + """Pins the new default. Raised from 25 now that a lower --min-volume-24h surfaces ~130 + candidates instead of ~35 -- 25 would cut off most of a typical sweep before the operator + ever sees it.""" + cli_option = next(p for p in cli_module.assets_discover.params if p.name == "limit") + assert cli_option.default == 100 + + +def test_discover_states_total_and_shown_when_limit_truncates( + tmp_path, valid_config_path, monkeypatch +): + """The load-bearing case, verified at the CLI: with more candidates than --limit shows, the + output must say the true candidate count, how many are shown, and that --limit controls it -- + never let a truncated table read as the whole candidate set.""" + db_path = tmp_path / "t.db" + _repo_at(db_path) + products = [_venue_product(f"COIN{i}-USD", str(10_000_000 + i)) for i in range(5)] + venue = _FakeVenue(products) + monkeypatch.setattr(cli_module, "_build_broker", lambda config: venue) + + result = CliRunner().invoke( + cli, + ["--db", str(db_path), "--config", str(valid_config_path), + "assets", "discover", "--limit", "3"], + ) + + assert result.exit_code == 0, result.output + assert "5 venue products -> 5 candidates" in result.output + assert "showing 3 of 5 candidates" in result.output + assert "--limit" in result.output + shown_rows = [ln for ln in result.output.splitlines() if "COIN" in ln and "-USD" in ln] + assert len(shown_rows) == 3 + + +def test_discover_no_truncation_notice_when_everything_fits( + tmp_path, valid_config_path, monkeypatch +): + """No false alarm: when every candidate fits under --limit, nothing should claim a cut + happened.""" + db_path = tmp_path / "t.db" + _repo_at(db_path) + products = [_venue_product(f"COIN{i}-USD", str(10_000_000 + i)) for i in range(3)] + venue = _FakeVenue(products) + monkeypatch.setattr(cli_module, "_build_broker", lambda config: venue) + + result = CliRunner().invoke( + cli, ["--db", str(db_path), "--config", str(valid_config_path), "assets", "discover"] + ) + + assert result.exit_code == 0, result.output + assert "3 venue products -> 3 candidates" in result.output + assert "showing" not in result.output + + def test_probe_history_marks_candidates_without_a_four_year_series( tmp_path, valid_config_path, monkeypatch ):