fix(compliance): review follow-ups to #270 — re-pin the admission floor, stop a NaN crash

keel/commands/admission.py

@@ -415,9 +415,13 @@ def render_propose_view(view: ProposeView) -> list[str]:
 
 @dataclass(frozen=True)
 class DiscoverReport:
+    """See `keel.compliance.screen.DiscoveryResult` for why `candidates` is a `tuple`, not a
+    `list`: on a `frozen=True` dataclass a `list` field is still mutable in place and still makes
+    the dataclass unhashable, so a tuple is what makes `frozen` mean what it claims here too."""
+
     quote: str
     venue_product_count: int
-    candidates: list[Candidate]
+    candidates: tuple[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
@@ -483,8 +487,17 @@ def render_discover_report(report: DiscoverReport) -> list[str]:
     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
+    so a truncated table can never read as the whole candidate set.
+
+    `survivor_count` is clamped to 0: it relies on the invariant `len(candidates) + excluded.total
+    == venue_product_count` (pinned directly by
+    `test_discovery_survivors_plus_excluded_always_account_for_every_product` in
+    `tests/compliance/test_screen.py`, over `discover_candidates`'s output), but `DiscoverReport`
+    is a public frozen dataclass any caller -- test or otherwise -- can construct directly with
+    fields that do not actually satisfy it. Without the clamp, an inconsistent report renders a
+    negative count (`10 venue products -> -89 candidates`), which is not a real state and must
+    never reach an operator's screen."""
+    survivor_count = max(0, report.venue_product_count - report.excluded.total)
     lines = [
         f"{report.venue_product_count} venue products -> {survivor_count} candidates "
         f"(quote={report.quote}, 24h volume >= {report.min_quote_24h_volume:,.0f}, excluding "

keel/compliance/screen.py

@@ -479,10 +479,12 @@ class DiscoveryPolicy:
     #: 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
+    #: Measured 2026-08-15: four of five wrongly-dropped assets (ATOM, AAVE, BCH, CRV) 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.
+    #: liquidity was 3.1x-6.3x the admission floor. The fifth, ALGO, was a separate low outlier at
+    #: 437,712 (430,520 an hour later) -- the lowest of the five, not part of the cluster, and the
+    #: one that most tightly constrains how low this floor may safely sit. 100,000 sits
+    #: comfortably below both, 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.
@@ -576,9 +578,15 @@ def summary_line(self) -> str:
 @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."""
+    who did not survive and why. See `DiscoveryExclusions` for the accounting half.
 
-    candidates: list[Candidate]
+    `candidates` is a `tuple`, not a `list`, so that `frozen=True` means what it claims: a `list`
+    field is mutable in place (`result.candidates.append(...)` would succeed on a "frozen"
+    dataclass) and a dataclass holding one is unhashable regardless of the decorator. A tuple
+    closes both gaps.
+    """
+
+    candidates: tuple[Candidate, ...]
     excluded: DiscoveryExclusions
 
 
@@ -656,6 +664,20 @@ def discover_candidates(
         except (TypeError, ArithmeticError, ValueError):
             unreadable_volume += 1
             continue
+        # `Decimal("NaN")`/`Decimal("sNaN")` PARSE cleanly -- the `try` above does not catch
+        # them -- and then `<` raises `decimal.InvalidOperation`, which used to crash the whole
+        # sweep on one bad venue row. Treated as unreadable, same bucket as a value that failed
+        # to parse at all: a NaN is exactly as uninformative about liquidity as `"n/a"` is, and
+        # counting it separately would only teach an operator to distrust a count that means the
+        # same thing either way.
+        #
+        # `Decimal("Infinity")` also parses cleanly and compares fine (nothing is `>= Infinity`),
+        # so left unguarded it would silently become a candidate -- but a venue reporting
+        # infinite 24h volume is not credible data, it is a malformed feed, so it is counted
+        # unreadable rather than trusted as "definitely liquid".
+        if volume.is_nan() or volume.is_infinite():
+            unreadable_volume += 1
+            continue
         if volume < policy.min_quote_24h_volume:
             below_volume_floor += 1
             continue
@@ -670,7 +692,7 @@ def discover_candidates(
         )
 
     return DiscoveryResult(
-        candidates=sorted(out, key=lambda c: c.quote_24h_volume, reverse=True),
+        candidates=tuple(sorted(out, key=lambda c: c.quote_24h_volume, reverse=True)),
         excluded=DiscoveryExclusions(
             wrong_quote_currency=wrong_quote_currency,
             not_online=not_online,

tests/commands/test_admission.py

@@ -791,6 +791,32 @@ def test_render_discover_report_includes_the_exclusion_summary(repo: Repository)
     assert "below 24h volume floor 1" in text
 
 
+def test_render_discover_report_never_renders_a_negative_survivor_count():
+    """`survivor_count = venue_product_count - excluded.total` relies on the invariant
+    `len(candidates) + excluded.total == venue_product_count`, which `build_discover_report`
+    upholds but which nothing STOPS a caller from violating: `DiscoverReport` is a public frozen
+    dataclass, constructed directly in tests (and by anything else that imports it), so a report
+    whose fields simply do not agree is one bad construction away. Confirmed: an unclamped
+    `report.venue_product_count - report.excluded.total` on the report below renders
+    `10 venue products -> -89 candidates`, which is not a real state and must never reach an
+    operator. The subtraction in `render_discover_report` is clamped to 0 for exactly this."""
+    candidate = screen_mod.Candidate(
+        product_id="SOL-USD", asset="SOL", base_name="Solana", quote_24h_volume=Decimal("9000000")
+    )
+    report = DiscoverReport(
+        quote="USD",
+        venue_product_count=10,
+        candidates=(candidate,),
+        min_quote_24h_volume=Decimal("100000"),
+        excluded=screen_mod.DiscoveryExclusions(below_volume_floor=99),
+    )
+
+    text = "\n".join(render_discover_report(report))
+
+    assert "-89" not in text
+    assert "0 candidates" 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."""
@@ -851,3 +877,23 @@ def test_the_discovery_floor_is_strictly_below_the_admission_liquidity_floor():
     from keel.compliance.screen import ScreenPolicy
 
     assert DEFAULT_MIN_QUOTE_24H_VOLUME < ScreenPolicy().min_median_daily_volume
+
+
+def test_the_admission_liquidity_floor_is_pinned_to_its_actual_value():
+    """`min_median_daily_volume` is the real admission criterion -- the number that decides
+    which assets a money-moving tool may buy -- and this is the ONLY test in the suite that pins
+    its VALUE rather than its relationship to something else.
+
+    `test_the_discovery_floor_is_strictly_below_the_admission_liquidity_floor` above only asserts
+    `DEFAULT_MIN_QUOTE_24H_VOLUME < ScreenPolicy().min_median_daily_volume`. That `<` guards the
+    relationship between discovery's pre-filter and the gate correctly, and must stay -- but a
+    `<` assertion alone permits ANY value above the discovery floor: raise or, worse, silently
+    lower `min_median_daily_volume` by 5x (verified: dropping it from 1,000,000 to 200,000 still
+    left the whole suite green, 2741 passed / 1 skipped, with the relationship test still
+    passing) and nothing else in the suite would notice. Keep BOTH assertions: the `<` one
+    guards that discovery must never be stricter than the gate, this one guards the criterion
+    itself.
+    """
+    from keel.compliance.screen import ScreenPolicy
+
+    assert ScreenPolicy().min_median_daily_volume == Decimal("1000000")

tests/commands/test_tui.py

@@ -2094,7 +2094,7 @@ def test_build_discover_overlay_with_report_is_nonempty_titled_and_headed() -> N
     report = DiscoverReport(
         quote="USD",
         venue_product_count=900,
-        candidates=[candidate],
+        candidates=(candidate,),
         min_quote_24h_volume=Decimal("5000000"),
         excluded=screen_mod.DiscoveryExclusions(below_volume_floor=899),
     )

tests/compliance/test_screen.py

@@ -549,8 +549,8 @@ def test_discovery_keeps_liquid_online_products_in_the_settlement_currency():
 def test_discovery_drops_the_wrong_quote_currency():
     from keel.compliance.screen import discover_candidates
 
-    assert discover_candidates([_product(quote="USDC")]).candidates == []
-    assert discover_candidates([_product(quote="BTC")]).candidates == []
+    assert discover_candidates([_product(quote="USDC")]).candidates == ()
+    assert discover_candidates([_product(quote="BTC")]).candidates == ()
 
 
 def test_discovery_drops_untradable_products():
@@ -562,20 +562,50 @@ def test_discovery_drops_untradable_products():
         {"is_disabled": True},
         {"view_only": True},
     ):
-        assert discover_candidates([_product(**kwargs)]).candidates == [], 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")]).candidates == []
+    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)]).candidates == []
-    assert discover_candidates([_product(volume="n/a")]).candidates == []
+    assert discover_candidates([_product(volume=None)]).candidates == ()
+    assert discover_candidates([_product(volume="n/a")]).candidates == ()
+
+
+def test_discovery_treats_nan_volume_as_unreadable_rather_than_crashing():
+    """`Decimal("NaN")` parses cleanly -- the `try/except` around the parse does not catch it --
+    and then `volume < policy.min_quote_24h_volume` raises `decimal.InvalidOperation`, which used
+    to crash the whole sweep on a single bad venue row. A NaN is exactly as uninformative about
+    liquidity as an unparseable string, so it must land in the same `unreadable_volume` bucket,
+    not blow up the command. Covers a string `"NaN"`, a `float("nan")` (the `str()` call ahead of
+    `Decimal(...)` still produces the string `"nan"`), and the signaling `"sNaN"` form."""
+    from keel.compliance.screen import discover_candidates
+
+    for nan_volume in ("NaN", "nan", float("nan"), "sNaN"):
+        result = discover_candidates([_product(volume=nan_volume)])
+        assert result.candidates == (), nan_volume
+        assert result.excluded.unreadable_volume == 1, nan_volume
+        assert result.excluded.below_volume_floor == 0, nan_volume
+
+
+def test_discovery_treats_infinite_volume_as_unreadable_not_a_credible_candidate():
+    """`Decimal("Infinity")` also parses cleanly and compares fine against the floor, so left
+    unguarded it would silently become a candidate. A venue reporting infinite 24h volume is not
+    credible data -- it is a malformed feed -- so it is counted `unreadable_volume`, the same
+    fail-closed bucket as any other value this module cannot trust, rather than treated as
+    "definitely liquid"."""
+    from keel.compliance.screen import discover_candidates
+
+    result = discover_candidates([_product(volume="Infinity")])
+    assert result.candidates == ()
+    assert result.excluded.unreadable_volume == 1
+    assert result.excluded.below_volume_floor == 0
 
 
 def test_discovery_excludes_assets_we_already_hold():
@@ -633,20 +663,27 @@ def test_a_quiet_days_snapshot_no_longer_hides_an_asset_the_gate_would_admit():
     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.
+    (3.33x) and ALGO 3,780,207 (3.78x). Four of the five -- ATOM, AAVE, BCH, CRV -- had 24h
+    snapshots clustered between 852,133 and 979,000 on that single quiet day, comfortably below
+    the old floor while their median-over-history liquidity was multiples of the admission
+    requirement. ALGO was NOT part of that cluster: it was a separate, lower outlier at
+    `437,712` (`430,520` measured again an hour later) -- the lowest 24h snapshot of the five,
+    and the one that most tightly constrains how low this floor may safely sit.
+
+    This test pins ALGO's actual measured 24h figure, `437712`, not the cluster's -- the LOWEST
+    measured value in the incident, not the highest. That is deliberate: a regression test that
+    instead pinned the top of the cluster (`852133`) would keep passing under a floor as high as
+    500,000, which would silently re-hide ALGO while looking green. `437712` is 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. (Verified separately: a floor of 500,000 fails this test once it is
+    pinned to `437712`, and would NOT have failed it pinned to `852133`.)
     """
     from keel.compliance.screen import discover_candidates
 
-    found = discover_candidates([_product(pid="ATOM-USD", volume="852133")])
+    found = discover_candidates([_product(pid="ALGO-USD", volume="437712")])
 
-    assert [c.asset for c in found.candidates] == ["ATOM"]
+    assert [c.asset for c in found.candidates] == ["ALGO"]
 
 
 def test_discovery_counts_every_exclusion_by_reason():
@@ -692,6 +729,37 @@ def test_discovery_counts_every_exclusion_by_reason():
     assert [c.asset for c in result.candidates] == ["SOL"]
 
 
+def test_discovery_survivors_plus_excluded_always_account_for_every_product():
+    """The invariant `len(candidates) + excluded.total == len(products)` that
+    `render_discover_report` (`keel/commands/admission.py`) leans on to derive its displayed
+    survivor count via subtraction (`venue_product_count - excluded.total`) instead of carrying
+    a redundant field. Nothing previously asserted this identity directly -- only that individual
+    counts landed in the right buckets -- so a future change that drops a product on the floor
+    (double-counts it, or skips it) without incrementing any `excluded` field or appending to
+    `candidates` would go unnoticed here and would render nonsense downstream.
+
+    Uses the same one-product-per-reason mix as
+    `test_discovery_counts_every_exclusion_by_reason`, plus a SECOND survivor, so the identity is
+    checked with more than one candidate on each side of the equation."""
+    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"),  # already on the allowlist
+        _product("BADVOL-USD", volume="not-a-number"),
+        _product("THIN-USD", volume="1"),
+        _product("SOL-USD"),  # survivor 1
+        _product("ETH-USD"),  # survivor 2
+    ]
+
+    result = discover_candidates(products, exclude_assets=frozenset({"BTC"}))
+
+    assert len(result.candidates) + result.excluded.total == len(products)
+
+
 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
@@ -707,7 +775,7 @@ def test_discovery_counts_both_trading_disabled_flag_variants_together():
 
     assert result.excluded.trading_disabled == 2
     assert result.excluded.total == 2
-    assert result.candidates == []
+    assert result.candidates == ()
 
 
 def test_discovery_exclusions_summary_line_names_every_reason_and_the_total():