diff --git a/keel/cli.py b/keel/cli.py index 003d93e2..83f8dc08 100644 --- a/keel/cli.py +++ b/keel/cli.py @@ -95,7 +95,6 @@ from keel.commands.withdrawals import withdrawals_group from keel.compliance import purification as purification_mod from keel.compliance import screen as screen_mod -from keel.compliance.screen import DATA_DERIVED_FAILURES as _DATA_DERIVED_FAILURES from keel.config import Config from keel.data import freshness as freshness_mod from keel.data import history as history_mod @@ -550,14 +549,6 @@ def _screen_product( return facts, screen_mod.screen_asset(facts, attestation, waived=waived) -# Failure classes that are DOWNSTREAM of having no cached history: with zero bars `liquidity` -# reports on our data (median volume is 0 *because* there are no bars), not on the asset, so -# `assets holdings` must not print it as a verdict. `settlement` is deliberately NOT here -- it -# compares the product's quote leg to the settlement currency and never touches candles, so it -# stays a real, assessable verdict even with zero bars. Single source of truth lives in -# `screen.py` (it owns the failure tags); `keel/proposer.py` imports the same constant so the two -# callers cannot silently drift apart. - # Never candidates: you cannot trade the currency you settle in, and fiat is funding rather than # a position. Coinbase quotes many fiats, so the list is deliberately broad -- a missing one is # only cosmetic (an extra row), never an admission. @@ -656,26 +647,30 @@ def assets_holdings(ctx: click.Context, min_balance: str, run_screen: bool) -> N facts, result = _screen_product(repo, product, quote) click.echo(f" {result.summary} ({facts.daily_bars} daily bars cached)") - failures = result.failures + # The likeliest misreading of this whole feature. With no cached bars, `history` and + # `liquidity` cannot say anything about the ASSET -- `0 daily bars, need 1460` measures + # the depth of OUR CACHE, and median volume is 0 *because* there are no bars -- so + # printing either as a finding would assert exactly what this message exists to deny: a + # candidate never fetched would read as indistinguishable from one genuinely too young. + # They are shown as derived-from-an-empty-cache, not as verdicts. + # + # The split and the explanation both live in `keel.compliance.screen` (`split_failures` / + # `missing_history_lines`), not here -- it owns `DATA_DERIVED_FAILURES`, the tag set that + # decides the split, so the decision and the tags cannot silently drift apart the way two + # independent per-caller copies could (and had, before `keel/proposer.py` and this + # function were unified onto the same two functions). + # + # NOTE: `settlement` is deliberately NOT suppressed. It compares the product's quote leg + # to the settlement currency and never reads candles, so it stays assessable at zero + # bars. Do not add it to `DATA_DERIVED_FAILURES` -- no test would catch that here (a + # derived product can never fail settlement), and it would hide a real verdict on any + # externally supplied product. + failures, not_assessable = screen_mod.split_failures(facts, result) if facts.daily_bars == 0: - # The likeliest misreading of this whole feature. With no cached bars `liquidity` - # cannot say anything about the asset -- median volume is 0 *because* there are no - # bars -- so printing it as a finding would assert about the asset exactly what this - # message exists to deny. It is shown as derived, not as a verdict. - # NOTE: `settlement` is deliberately NOT suppressed. It compares the product's quote - # leg to the settlement currency and never reads candles, so it stays assessable at - # zero bars. Do not add it to `_DATA_DERIVED_FAILURES` -- no test would catch that - # here (a derived product can never fail settlement), and it would hide a real - # verdict on any externally supplied product. - derived = [f for f in failures if f.split(":")[0] in _DATA_DERIVED_FAILURES] - failures = [f for f in failures if f not in derived] - click.echo( - f" ! no local history -- run `keel fetch --products {product}` first, then " - "re-screen.\n" - " This is a MISSING-DATA verdict, not a verdict about the asset." - ) - for failure in derived: - click.echo(f" · ({failure.split(':')[0]}: not assessable without history)") + explanation = screen_mod.missing_history_lines(product, not_assessable) + click.echo(f" ! {explanation[0]}") + for extra_line in explanation[1:]: + click.echo(f" {extra_line}") for failure in failures: click.echo(f" ✗ {failure}") # Warnings carry compliance constraints that apply even to an ADMITted asset (§65.5's @@ -800,7 +795,24 @@ def assets_screen(ctx: click.Context, products: str | None) -> None: f"\n{result.summary:<7} {asset:<8} bars={facts.daily_bars} " f"median_daily_volume={facts.median_daily_volume:.0f}" ) - for failure in result.failures: + # Same zero-bars split `assets holdings --screen` and `assets propose` use, via the same + # two helpers in `screen.py`. This command is the SIBLING of the TUI's `s` screen overlay + # -- both screen `_default_sim_products(config)` through `_screen_product` -- so leaving + # only one of them able to explain an empty cache would hand an operator two different + # stories about the same allowlist depending on which surface they looked at. That is the + # drift `_screen_product` exists to prevent, applied to the REPORTING rather than to the + # verdict. At zero bars `history`/`liquidity` measure OUR CACHE, not the asset, so + # printing them as verdicts would say "too young" about something we simply never + # fetched. `settlement`/`spot_instrument` read the product id alone and never touch + # candles, so they stay real verdicts here -- which is exactly why this command may still + # be asked about an unvalidated `--products` id (see the note above). + failures, not_assessable = screen_mod.split_failures(facts, result) + if facts.daily_bars == 0: + explanation = screen_mod.missing_history_lines(product, not_assessable) + click.echo(f" ! {explanation[0]}") + for extra_line in explanation[1:]: + click.echo(f" {extra_line}") + for failure in failures: click.echo(f" ✗ {failure}") for warning in result.warnings: click.echo(f" ! {warning}") diff --git a/keel/commands/admission.py b/keel/commands/admission.py new file mode 100644 index 00000000..bbc734ee --- /dev/null +++ b/keel/commands/admission.py @@ -0,0 +1,403 @@ +"""The OFFLINE report layer for the allowlist-admission workflow -- the substrate the TUI's three +new overlays (**screen**, **propose**, **discover**) render. See +`docs/superpowers/specs/2026-07-24-llm-asset-proposer-design.md` and `keel/proposer.py` for the +proposal-report path this module reuses rather than duplicates. + +**Every verdict comes from the injected `screen_fn`** (`keel.cli._screen_product` in production), +never from a second, laxer path -- exactly the discipline `keel/proposer.py` already keeps, for +the same reason: `_screen_product`'s own docstring is explicit that every candidate source must +route through it, "so none of them can drift onto a laxer path". This module never imports +`keel.cli` (which would cycle back through here once `tui.py` wires these overlays in) and stays +importable, and unit-testable, with nothing but a fake `screen_fn`. + +This module **admits nothing, attests nothing, writes nothing**. `build_screen_report` and +`build_propose_view` only ever read the DB (`Repository.get_candles`/`get_asset_attestation`/ +`get_screen_exceptions`, all reached through `screen_fn`) -- see +`tests/commands/test_admission.py::test_build_screen_report_and_propose_view_write_nothing`. + +**`discover` is the one part of this workflow that needs the network**, and it is handled +differently on purpose: `build_discover_report` takes already-fetched venue product dicts as a +plain argument instead of building a broker itself. That is what keeps THIS module fully +unit-testable (no fake broker, no monkeypatched `CoinbaseClient`) and, more importantly, is what +lets the TUI worker gate the one network-touching action in this whole workflow behind an +explicit keypress: the overlay can call `_build_broker(config).list_products()` itself, in its own +try/except, only when the operator asks for it, and hand the plain list here. +""" + +from __future__ import annotations + +import json +from collections.abc import Callable +from dataclasses import dataclass +from decimal import Decimal +from pathlib import Path + +from keel.commands._products import _default_sim_products +from keel.compliance.screen import ( + Candidate, + DiscoveryPolicy, + MarketFacts, + ScreenResult, + discover_candidates, + missing_history_lines, + split_failures, +) +from keel.config import Config +from keel.data.repository import Repository +from keel.proposer import ( + ProposalError, + ProposalReport, + build_proposal_report, + parse_proposal, + render_proposal_report, +) + +ScreenFn = Callable[[Repository, str, str], tuple[MarketFacts, ScreenResult]] + +#: Kept in sync with `Config.proposals_dir`'s own default BY HAND -- there is no single source of +#: truth to import from without creating a `keel.commands.admission` <-> `keel_core.config` +#: dependency neither module needs otherwise. `test_default_proposals_dir_matches_configs_default` +#: pins the two together so a drift breaks a test instead of silently disagreeing. +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. +DEFAULT_MIN_QUOTE_24H_VOLUME = Decimal("5000000") + + +# -- 2a. shortlist location (offline) ------------------------------------------------------------ + + +def latest_shortlist(directory: Path) -> Path | None: + """The newest `*.json` file under `directory` by mtime, or `None`. + + `None` covers three cases the dashboard must never be errored out by: `directory` does not + exist, `directory` exists but is not a directory (a stray file at that path), or it exists and + is empty of `*.json` files. **Never creates `directory`** -- a fresh install legitimately has + no proposals directory yet, and a read-only report screen must not have the side effect of + creating one. Any `OSError` encountered while probing the filesystem (a permissions problem, a + race where the directory is removed mid-call) is treated the same way, for the same reason. + + Ties in mtime are broken by filename (the alphabetically-last name wins), so the result is + deterministic across repeated calls on the same directory contents rather than depending on + filesystem iteration order. + """ + try: + if not directory.is_dir(): + return None + candidates = list(directory.glob("*.json")) + if not candidates: + return None + return max(candidates, key=lambda p: (p.stat().st_mtime, p.name)) + except OSError: + return None + + +# -- 2b. screen report (offline, DB reads only) -------------------------------------------------- + + +@dataclass(frozen=True) +class ScreenedProduct: + product: str + asset: str + facts: MarketFacts + result: ScreenResult + on_allowlist: bool + attested: bool + + +@dataclass(frozen=True) +class ScreenReport: + quote: str + screened: list[ScreenedProduct] + + @property + def admitted_count(self) -> int: + return sum(1 for s in self.screened if s.result.admitted) + + +def build_screen_report(repo: Repository, config: Config, screen_fn: ScreenFn) -> ScreenReport: + """Screen `_default_sim_products(config)` -- the configured allowlist, in the deployment's + settlement currency, exactly the set `keel assets screen` screens by default -- through the + injected `screen_fn`. + + Mirrors `keel/proposer.py::build_proposal_report`'s shape closely: `screen_fn` is injected for + the identical reason (no `keel.cli` import, so no import cycle once `tui.py` wires this + module in, and fully unit-testable with a fake). Read-only: this never calls + `repo.upsert_asset_attestation` or any other write method. + """ + quote = config.quote_currency + allow = {asset.upper() for asset in config.allowlist} + screened: list[ScreenedProduct] = [] + for product in _default_sim_products(config): + asset = product.split("-")[0] + facts, result = screen_fn(repo, product, quote) + screened.append( + ScreenedProduct( + product=product, + asset=asset, + facts=facts, + result=result, + on_allowlist=asset in allow, + attested=repo.get_asset_attestation(asset) is not None, + ) + ) + return ScreenReport(quote=quote, screened=screened) + + +def render_screen_report(report: ScreenReport) -> list[str]: + """Human-readable lines, one product at a time, shaped to read the same way an operator + already reads `keel/proposer.py::render_proposal_report`'s output. + + Uses `screen.split_failures`/`screen.missing_history_lines` -- NOT a reimplementation of that + split -- so the zero-cached-bars case says "no local history, run keel fetch" and never prints + a `✗ history:` depth failure for a candidate this deployment has simply never fetched. See + `tests/commands/test_admission.py`'s two headline tests, which pin exactly this distinction. + """ + if not report.screened: + return ["allowlist is empty -- nothing to screen."] + + lines: list[str] = [] + for sp in report.screened: + allow = "on-allowlist" if sp.on_allowlist else "not-on-allowlist" + attested = "attested" if sp.attested else "UNATTESTED" + lines.append("") + lines.append( + f"{sp.result.summary:<7} {sp.asset:<8} bars={sp.facts.daily_bars} " + f"median_daily_volume={sp.facts.median_daily_volume:.0f} {allow} {attested}" + ) + failures, not_assessable = split_failures(sp.facts, sp.result) + if sp.facts.daily_bars == 0: + explanation = missing_history_lines(sp.product, not_assessable) + lines.append(f" ! {explanation[0]}") + for extra_line in explanation[1:]: + lines.append(f" {extra_line}") + for failure in failures: + lines.append(f" ✗ {failure}") + for warning in sp.result.warnings: + lines.append(f" ! {warning}") + + lines.append("") + lines.append(f"{report.admitted_count}/{len(report.screened)} admitted") + return lines + + +# -- 2c. propose view (offline) ------------------------------------------------------------------- + +_PROPOSAL_SCHEMA_SUMMARY = ( + 'expected schema: {"candidates": [{"asset": "", "rationale": "", ' + '"sources": ["", ...], "shariah_hypothesis": ""}]}' +) + + +@dataclass(frozen=True) +class ProposeView: + source: Path | None + status: str # "ok" | "no-directory" | "no-shortlist" | "unreadable" | "malformed" + detail: str | None # the human reason when status != "ok" + report: ProposalReport | None + + +def build_propose_view( + repo: Repository, + config: Config, + screen_fn: ScreenFn, + *, + directory: Path | None = None, + path: Path | None = None, +) -> ProposeView: + """Locate and screen the shortlist the `propose` overlay should show. **Never raises.** + + Resolution order: an explicit `path` (names an exact shortlist file, skipping the + newest-file search) beats `latest_shortlist(directory)`. `directory` defaults to + `Path(config.proposals_dir).expanduser()`. + + Every failure mode is FAIL-SOFT: a missing directory, an empty one, an unreadable file + (`OSError` -- e.g. a permissions problem, or the file vanishing mid-read), invalid JSON + (`json.JSONDecodeError`), or a malformed top-level shortlist (`ProposalError` from + `parse_proposal`, e.g. `{"candidates": "nope"}`) each produce a `ProposeView` carrying the + matching `status` and a plain-English `detail` -- never an exception. This function backs a + dashboard overlay: a stray file on disk, or a directory that has not been created yet on a + fresh install, must never be able to kill the process reading it. A per-CANDIDATE problem + (missing citation, bad URL) is a different, much softer case -- it does NOT reach this + function's fail-soft branches at all, because `parse_proposal` already handles it by + collecting the entry into `ParsedProposal.invalid` rather than raising; see + `render_propose_view`, which renders those as `INVALID` lines from the underlying + `ProposalReport`, same as `keel assets propose` does. + + On `"ok"`, the report is built with `parse_proposal` + `build_proposal_report` -- reused + verbatim from `keel/proposer.py`, never reimplemented here. + """ + if directory is None: + directory = Path(config.proposals_dir).expanduser() + + source = path + if source is None: + try: + directory_exists = directory.is_dir() + except OSError: + directory_exists = False + if not directory_exists: + return ProposeView( + source=None, + status="no-directory", + detail=( + f"no proposals directory at {directory} -- drop a shortlist JSON there " + "(produced externally, e.g. an LLM + web-search scout, or via the discover " + f"overlay) and reopen this screen. {_PROPOSAL_SCHEMA_SUMMARY}" + ), + report=None, + ) + source = latest_shortlist(directory) + if source is None: + return ProposeView( + source=None, + status="no-shortlist", + detail=( + f"{directory} has no *.json shortlist yet -- drop one there and reopen this " + f"screen. {_PROPOSAL_SCHEMA_SUMMARY}" + ), + report=None, + ) + + try: + raw_text = source.read_text() + except OSError as exc: + return ProposeView( + source=source, + status="unreadable", + detail=f"could not read {source}: {exc}", + report=None, + ) + + try: + raw = json.loads(raw_text) + except json.JSONDecodeError as exc: + return ProposeView( + source=source, + status="malformed", + detail=f"{source} is not valid JSON: {exc}. {_PROPOSAL_SCHEMA_SUMMARY}", + report=None, + ) + + try: + parsed = parse_proposal(raw) + except ProposalError as exc: + return ProposeView( + source=source, + status="malformed", + detail=f"{source}: {exc}. {_PROPOSAL_SCHEMA_SUMMARY}", + report=None, + ) + + report = build_proposal_report( + parsed, repo, config.quote_currency, config.allowlist, screen_fn + ) + return ProposeView(source=source, status="ok", detail=None, report=report) + + +_PROPOSE_STATUS_HEADERS = { + "no-directory": "no proposals directory found", + "no-shortlist": "no shortlist file found", + "unreadable": "could not read the shortlist file", + "malformed": "the shortlist file is malformed", +} + + +def render_propose_view(view: ProposeView) -> list[str]: + """Human-readable lines. On `"ok"` this reuses `render_proposal_report(view.report)` + VERBATIM -- a hard requirement, not a convenience: this module must never grow a second + rendering path for the same report shape (see `keel/proposer.py`'s own docstring on why a + duplicated split/render invites drift). It is prefixed with one line naming the shortlist file + that was read, since the overlay otherwise never says which file produced what is on screen. + + For every non-`"ok"` status this renders a calm, actionable explanation -- naming the + directory it looked in and what the operator should do about it, plus the expected schema -- + and never anything that reads like a crash or a traceback. + """ + if view.status == "ok": + assert view.report is not None # invariant of "ok": build_propose_view guarantees this + lines = [f"shortlist: {view.source}"] + lines.extend(render_proposal_report(view.report)) + return lines + + header = _PROPOSE_STATUS_HEADERS.get(view.status, "propose view unavailable") + lines = [header] + if view.detail: + lines.append(f" {view.detail}") + return lines + + +# -- 2d. discover view ----------------------------------------------------------------------- + + +@dataclass(frozen=True) +class DiscoverReport: + quote: str + venue_product_count: int + candidates: list[Candidate] + min_quote_24h_volume: Decimal + + +def build_discover_report( + products: list[dict], + config: Config, + *, + limit: int = 25, + min_quote_24h_volume: Decimal | None = None, +) -> DiscoverReport: + """PURE over `products` -- the caller's already-fetched venue metadata. Builds no broker and + makes no network call; see the module docstring for why that split is what lets the TUI gate + the one network-touching action in this whole workflow behind an explicit keypress. + + 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`). + """ + floor = ( + min_quote_24h_volume + if min_quote_24h_volume is not None + else DEFAULT_MIN_QUOTE_24H_VOLUME + ) + policy = DiscoveryPolicy(quote_currency=config.quote_currency, min_quote_24h_volume=floor) + candidates = 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], + min_quote_24h_volume=floor, + ) + + +def render_discover_report(report: DiscoverReport) -> list[str]: + """Mirrors `assets_discover`'s table (rank, product, asset, 24h quote volume, name) and ends + 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`).""" + lines = [ + 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)", + "", + 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} " + f"{candidate.quote_24h_volume:>18,.0f} {candidate.base_name}" + ) + lines.append("") + lines.append( + "⚠️ These are PROPOSALS, not admissions. Nothing above has been screened for sector " + "or backing -- those cannot be derived from market data. Each one needs " + "`keel assets attest` with a source before `keel assets screen` can admit it." + ) + return lines diff --git a/keel/commands/tui.py b/keel/commands/tui.py index d711101e..62c712c5 100644 --- a/keel/commands/tui.py +++ b/keel/commands/tui.py @@ -16,15 +16,39 @@ Two layers, mirroring `status.py`'s split: -- `build_screen`, `build_help_screen`, `_visible_slice`, `_footer_lines`, `_freshness_style`, - `toggle_autonomy` and `_guarded` are all PURE (or take only injected fakes), directly - unit-testable without curses, a CliRunner, or the network. `render_plain` is the same, dropping - styles. +- `build_screen`, `build_help_screen`, `_visible_slice`, `_scroll_offset`, `_footer_lines`, + `_freshness_style`, `toggle_autonomy` and `_guarded` are all PURE (or take only injected + fakes), directly unit-testable without curses, a CliRunner, or the network. `render_plain` is + the same, dropping styles. - `_paint` (curses rendering), `run_once` (single-frame, `--once`/pipes/CI), `run_live` (the auto-refreshing `curses.wrapper` loop), and `_confirm_arm_autonomy` (the cooked-mode typed-`yes` prompt) are the thin I/O layer. `curses` is imported lazily inside the functions that need it, so this module stays importable -- and the pure-function tests stay portable -- even where a real terminal is absent. + +v3 (this revision) wires the allowlist-admission workflow (`keel/commands/admission.py`, already +fully built and covered by its own tests) into three more overlays, reusing that module's report +builders/renderers VERBATIM rather than reimplementing any of it -- exactly the same discipline +`i` insights already keeps toward `keel/commands/insights.py`: + +- `s` **screen** -- `build_admission_screen_overlay` over `build_screen_report`: the current + allowlist's admission verdicts. OFFLINE, DB reads only. +- `p` **propose** -- `build_propose_overlay` over `build_propose_view`: screens the newest + shortlist file in `config.proposals_dir` (or names why there is none). OFFLINE, DB + local + filesystem reads only. +- `d` **discover** -- `build_discover_overlay` over `build_discover_report`: proposes NEW + candidates from the venue's own product list. This is the one of the three that needs the + network, and it is the SECOND deliberate network exception in this dashboard (the first is `f` + fetch): opening the overlay makes no call at all (it renders an ARMED, not-yet-run + explanation), and only an explicit Enter keypress *inside* the overlay triggers + `_do_discover_report`'s one `_build_broker(config).list_products()` call. The result is then + HELD -- every following poll while the overlay stays open repaints the same cached result (or + error) rather than re-fetching, and closing the overlay discards it, so reopening is armed but + not yet run again. + +None of the three attests, admits, or trades -- `attest` (the human judgment the whole gate rests +on) stays deliberately CLI-only, `keel assets attest`. `screen`/`propose`/`discover` only ever +PROPOSE or REPORT; they cannot themselves put an asset on `allowlist` in `config.yaml`. """ from __future__ import annotations @@ -39,6 +63,17 @@ import click from keel.commands._common import DISCLAIMER, _load_cfg, _open_repo +from keel.commands.admission import ( + DiscoverReport, + ProposeView, + ScreenReport, + build_discover_report, + build_propose_view, + build_screen_report, + render_discover_report, + render_propose_view, + render_screen_report, +) from keel.commands.status import StatusReport, _human_age, gather_status from keel.config import Config from keel.data.repository import Repository @@ -264,12 +299,23 @@ def _subscription_lines(report: StatusReport) -> list[ScreenLine]: def _footer_lines() -> list[ScreenLine]: """The keybinding hint bar shown at the bottom of the normal-mode dashboard. Deliberately - interval-independent (see `build_screen`'s note) and pure, so it's directly testable.""" + interval-independent (see `build_screen`'s note) and pure, so it's directly testable. + + Two lines, not one: the first (kept byte-for-byte as it was before the admission overlays + existed, so nothing that already reads it needs to change) is already close to 80 columns, + and cramming `[s] screen [p] propose [d] discover` onto the end of it would either wrap on a + normal terminal or silently truncate (`_paint` clips every line to the window width). A + second line costs one more row of screen -- cheap, next to a footer line an operator can no + longer read.""" return [ ScreenLine( "keys: [q] quit [h] help [i] insights [r] refresh [a] autonomy [f] fetch", "muted", - ) + ), + ScreenLine( + "admission: [s] screen [p] propose [d] discover (network)", + "muted", + ), ] @@ -336,6 +382,10 @@ def _note(text: str) -> None: _row(" f fetch all data (pull candles for every configured product)") _note(" can pull up to 5y of candles; the dashboard freezes until it finishes") _note(" (Ctrl-C aborts the whole TUI, not just the fetch)") + _row(" s open the screen overlay (allowlist admission verdicts, read-only)") + _row(" p open the propose overlay (screens the newest shortlist file, read-only)") + _row(" d open the discover overlay (propose NEW candidates from the venue)") + _note(" armed, not run, on open -- see 'Discover overlay' below") lines.append(_blank()) _row("Live balance") _note(" 'live account' shows the REAL account's spendable quote balance (e.g. USDC),") @@ -357,6 +407,30 @@ def _note(text: str) -> None: _note(" scrolling keys as help mode (up/k, down/j, PgUp/PgDn, Home/End).") _row(" q / Esc / i close insights, back to the dashboard") lines.append(_blank()) + _row("Screen overlay (s)") + _note(" OFFLINE, read-only: runs the current allowlist through the SAME admission gate") + _note(" `keel assets screen` uses (`_screen_product`) -- ADMIT/REJECT per product, plus WHY.") + _note(" DB reads only; never builds a broker, never touches the network.") + _row(" q / Esc / s close screen, back to the dashboard") + lines.append(_blank()) + _row("Propose overlay (p)") + _note(" OFFLINE, read-only: screens the newest *.json shortlist file in config.proposals_dir") + _note(" (produced externally -- an LLM + web-search scout, or the discover overlay's output") + _note(" saved to disk) through the same admission gate. No shortlist yet is reported plainly,") + _note(" not as an error. DB + local filesystem reads only; never touches the network.") + _row(" q / Esc / p close propose, back to the dashboard") + lines.append(_blank()) + _row("Discover overlay (d)") + _note(" Opens ARMED, NOT yet run -- pressing 'd' makes NO network call. It explains what") + _note(" running it will do and that it is a LIVE call to the venue. Only Enter, pressed") + _note(" INSIDE this overlay, actually contacts the venue (`list_products`) and proposes") + _note(" candidates from the result -- the same cheap pre-filter `keel assets discover` runs.") + _note(" The result is then HELD: every poll while the overlay stays open repaints the same") + _note(" cached result, with NO further network calls, until Enter is pressed again. Closing") + _note(" the overlay discards the held result, so reopening it is armed-but-not-run again.") + _row(" Enter run discover now (the ONE network call this overlay ever makes)") + _row(" q / Esc / d close discover, back to the dashboard (discards the held result)") + lines.append(_blank()) _row("Safety notes") _note( " autonomy OFF is immediate and ungated -- de-risking must never be obstructed, so" @@ -379,6 +453,27 @@ def _note(text: str) -> None: ) _note(" endpoints. It places no orders and touches no rails.") lines.append(_blank()) + _note(" screen and propose are fully OFFLINE: DB (and, for propose, local filesystem)") + _note(" reads only. Neither ever constructs a broker or touches the network.") + lines.append(_blank()) + _note( + " discover is the SECOND deliberate network exception in this dashboard (after fetch):" + ) + _note(" it never fires on opening the overlay and never fires again on its own while the") + _note(" overlay stays open -- only an explicit Enter, pressed inside it, runs the one live") + _note(" venue call it ever makes.") + lines.append(_blank()) + _note( + " NONE of screen, propose or discover attests, admits, or trades. They can only PROPOSE" + ) + _note( + " or REPORT -- putting an asset on `allowlist` in config.yaml still needs a human to run" + ) + _note( + " `keel assets attest` with a source. attest is deliberately CLI-only, never a keypress" + ) + _note(" here: it is the one step in this whole gate that rests on human judgment, not code.") + lines.append(_blank()) _row("Every action shows a one-line result at the bottom of the dashboard until the next") _row("action replaces it.") lines.append(_blank()) @@ -448,6 +543,152 @@ def build_insights_screen( return lines +def _admission_line_style(text: str) -> str: + """Style a single rendered line from `keel.commands.admission`'s renderers + (`render_screen_report`/`render_propose_view`/`render_discover_report` -- plain, unstyled + `str`s, exactly the shape `_insights_line_style` already keys off) by the textual conventions + those renderers already share with `keel assets screen`/`propose`/`discover`'s own CLI output. + + `ADMIT`/`REJECT` are the screen's actual verdict, so they carry the strongest legible + contrast: reassuring green for an admit, a warning (never `"alert"` -- a reject is the system + working as intended, not an emergency) for a reject. A `✗ ` line is a real, FAILED admission + criterion -- `"warn"`. An `INVALID` line (`render_propose_view`'s malformed-shortlist-entry + report) is a data problem in a file on disk, not a live threat -- also `"warn"`, not `"alert"`. + + The `! no local history` line -- and its MISSING-DATA continuation line from + `missing_history_lines` -- are deliberately `"muted"`, NOT `"warn"`/`"alert"`, even though the + first starts with the same `!` marker every other warning does. `keel.compliance.screen. + split_failures`'s entire reason for existing is that "never fetched" is not a verdict about + the asset (see `render_screen_report`'s own docstring: a candidate this deployment has simply + never fetched candles for must not read as indistinguishable from one genuinely too young) -- + painting it in the same colour as a real rejection reason would visually assert the opposite + of what the text says. Every OTHER `! ` line is a genuine warning (`ScreenResult.warnings`, + e.g. a §65.5 bay' al-sarf note that applies even to an ADMITted asset) and stays `"warn"`. + + `render_discover_report`'s closing `⚠️ These are PROPOSALS, not admissions` line is the one + line in this whole workflow that must never be missed -- discover is the network-touching + overlay, and every candidate it lists is unvetted -- so it is `"alert"`, the same weight + `_message_style` gives to arming autonomy ON.""" + stripped = text.strip() + if stripped.startswith("ADMIT"): + return "ok" + if stripped.startswith("REJECT"): + return "warn" + if stripped.startswith("✗"): + return "warn" + if stripped.startswith("!"): + if "no local history" in stripped.lower(): + return "muted" + return "warn" + if "missing-data" in stripped.lower(): + return "muted" + if stripped.startswith("INVALID"): + return "warn" + if stripped.startswith("⚠"): + return "alert" + return "normal" + + +def build_admission_screen_overlay(report: ScreenReport) -> list[ScreenLine]: + """A titled, scrollable, READ-ONLY overlay over an already-built `ScreenReport` -- PURE, + mirroring `build_insights_screen` exactly: the caller (`run_live`'s `screen` branch, via + `_do_screen_report`) does the OFFLINE work of building the report fresh each poll; this + function only styles the lines `render_screen_report` already rendered. Never touches the + repo, network, or broker itself, and never admits, attests, or trades -- see the module + docstring.""" + lines: list[ScreenLine] = [ScreenLine("keel tui -- screen", "heading"), _blank()] + for text in render_screen_report(report): + lines.append(ScreenLine(text, _admission_line_style(text)) if text else _blank()) + lines.append(_blank()) + lines.append(ScreenLine("Press s or Esc to return to the dashboard.", "muted")) + return lines + + +def build_propose_overlay(view: ProposeView) -> list[ScreenLine]: + """Same shape as `build_admission_screen_overlay`, over an already-built `ProposeView` + (which itself NEVER raises -- every failure mode, a missing directory through a malformed + shortlist file, is already a calm `status`/`detail` pair; see its own docstring). + PURE -- only styles what `render_propose_view` already rendered.""" + lines: list[ScreenLine] = [ScreenLine("keel tui -- propose", "heading"), _blank()] + for text in render_propose_view(view): + lines.append(ScreenLine(text, _admission_line_style(text)) if text else _blank()) + lines.append(_blank()) + lines.append(ScreenLine("Press p or Esc to return to the dashboard.", "muted")) + return lines + + +#: Named once so the ARMED explanation's own text and the actual keypress `run_live`'s discover +#: branch checks for can't silently drift apart -- a mismatch here (the overlay says one key, +#: the loop listens for another) would be worse than almost anywhere else in this module, since +#: the whole point of the ARMED state is that the operator can trust what it says before it ever +#: touches the network. +_DISCOVER_RUN_KEY_HINT = "Enter" + + +def build_discover_overlay( + report: DiscoverReport | None, error: str | None = None +) -> list[ScreenLine]: + """A titled, scrollable overlay over `keel.commands.admission.build_discover_report` -- PURE, + but unlike `build_admission_screen_overlay`/`build_propose_overlay` it renders THREE distinct + states, not one, because `discover` is the one overlay of this trio that needs the network + (see the module docstring and `run_live`'s discover branch for the full gating story): + + - `report is None and error is None`: **ARMED, not yet run.** This is the state the overlay + opens into on `d` -- no network call has happened yet, and this rendering is the proof of + that: it names what pressing `_DISCOVER_RUN_KEY_HINT` will do (fetch the venue's product + list and propose candidates from it, the same cheap pre-filter `keel assets discover` + runs), that it is a LIVE call to the venue, and which key runs it. A test asserting this + state renders (rather than, say, a blank or "loading" screen) is the test that proves + opening the overlay alone never touches the network. + - `error is not None`: the last Enter's fetch failed (broker construction, auth, a network + error) -- rendered as one readable line, never a raw traceback, with the same key hint so + the operator knows how to retry. + - `report is not None`: the HELD result of the last successful Enter, rendered via + `render_discover_report` exactly like the other two overlays reuse their own renderer. + + Whichever state, `report`/`error` are furnished by the caller -- this function itself never + fetches, never re-fetches, and never decides staleness; it only styles whatever it is handed. + """ + lines: list[ScreenLine] = [ScreenLine("keel tui -- discover", "heading"), _blank()] + if error is not None: + lines.append(ScreenLine(f"discover failed: {error}", "alert")) + lines.append(_blank()) + lines.append( + ScreenLine(f"Press {_DISCOVER_RUN_KEY_HINT} to contact the venue again.", "normal") + ) + elif report is None: + lines.append(ScreenLine("ARMED -- no network call has been made yet.", "normal")) + lines.append(_blank()) + lines.append( + ScreenLine( + f"Pressing {_DISCOVER_RUN_KEY_HINT} makes ONE live call to the venue " + "(list_products) and proposes candidates from it -- the same cheap pre-filter " + "`keel assets discover` runs, on the exact same data.", + "normal", + ) + ) + lines.append( + ScreenLine( + "This is the second deliberate network exception in this dashboard (the first " + "is [f] fetch): it never fires just from opening this overlay, and it never " + "fires again on its own while this overlay stays open.", + "normal", + ) + ) + lines.append(_blank()) + lines.append(ScreenLine("Nothing here is admitted -- discover only proposes.", "muted")) + lines.append(_blank()) + lines.append( + ScreenLine(f"Press {_DISCOVER_RUN_KEY_HINT} to contact the venue now.", "normal") + ) + else: + for text in render_discover_report(report): + lines.append(ScreenLine(text, _admission_line_style(text)) if text else _blank()) + lines.append(_blank()) + lines.append(ScreenLine("Press d or Esc to return to the dashboard.", "muted")) + return lines + + def _visible_slice(lines: list[ScreenLine], offset: int, height: int) -> list[ScreenLine]: """The `height`-line window of `lines` starting at `offset`, clamped so `offset` never runs past what would leave a partial screen at the end (or before the start). PURE -- never raises @@ -459,6 +700,39 @@ def _visible_slice(lines: list[ScreenLine], offset: int, height: int) -> list[Sc return lines[offset : offset + height] +def _scroll_offset(ch: int, offset: int, height: int, total: int, curses_mod: Any) -> int: + """The new, clamped scroll offset for a keypress inside any of the five scrollable overlays + (help, insights, screen, propose, discover). Factored out of `run_live` because its help and + insights branches used to each hand-roll an identical ~8-line up/down/PgUp/PgDn/Home/End + chain -- copy-pasting that a further three times for the new overlays, onto a function that + was already long, would have made it worse rather than better. + + PURE: takes `curses_mod` as a parameter rather than importing `curses` itself, for two + reasons that both matter here -- `curses` is imported lazily inside `run_live` (this module + must stay importable with no real terminal present, and the pure-function tests must stay + portable), and passing it in is what lets this function be unit-tested against the SAME fake + `curses` module the rest of the `run_live` test suite already builds, with no real terminal + or `curses.wrapper` involved. + + `total` is the number of lines in the overlay being scrolled (`len(lines)`), used exactly the + way `help_offset`/`insights_offset` always were: `End` jumps toward the bottom (clamped, like + every other result, to the last full page) and every key's result is clamped to `[0, max(0, + total - height)]` so the view can never scroll past either end.""" + if ch in (curses_mod.KEY_UP, ord("k")): + offset -= 1 + elif ch in (curses_mod.KEY_DOWN, ord("j")): + offset += 1 + elif ch == curses_mod.KEY_PPAGE: + offset -= max(height - 1, 1) + elif ch == curses_mod.KEY_NPAGE: + offset += max(height - 1, 1) + elif ch == curses_mod.KEY_HOME: + offset = 0 + elif ch == curses_mod.KEY_END: + offset = total + return max(0, min(offset, max(0, total - height))) + + # -- actions (injectable, unit-testable without curses/network) ---------------------------------- @@ -577,6 +851,14 @@ def _paint(stdscr: Any, lines: list[ScreenLine]) -> None: #: background refresh, not something the operator is blocked waiting on) but finite. _BALANCE_TIMEOUT_SEC = 10 +#: Network timeout (seconds) bounding the discover overlay's one `list_products` call. TIGHTER +#: than `_BALANCE_TIMEOUT_SEC`'s rationale would suggest is needed, because the operator is +#: BLOCKED on this one: they pressed Enter, the screen is frozen behind a "contacting venue" +#: frame, and there is no other feedback until the call returns. A hung venue must become a +#: readable `discover failed:` line they can retry, never an indefinitely frozen dashboard whose +#: only exit is Ctrl-C (which kills the whole TUI, not just the request). +_DISCOVER_TIMEOUT_SEC = 20 + def _refresh_balance( open_state: OpenState, now_fn: NowFn, balance_fn: Callable[[Config], Decimal | None] @@ -687,28 +969,96 @@ def _do_fetch(open_state: OpenState, now_fn: NowFn) -> str: return f"fetch complete ({len(products)} products, {years}y history)" +def _do_screen_report(open_state: OpenState) -> ScreenReport: + """Build a fresh `ScreenReport` for the current allowlist -- OFFLINE, DB reads only, rebuilt + every poll while the screen overlay is open, exactly like insights' per-poll rebuild. + `_screen_product` (THE single admission gate every candidate source must route through) lives + in `keel.cli`, which itself imports THIS module to wire up `tui_cmd`/`run_live` -- importing + it at module load time here would cycle, so it is lazy-imported inside this function, exactly + like `_do_fetch` lazy-imports `_SIM_GRANULARITIES` from the same module for the same reason. + Thin I/O -- not unit-tested directly, only smoke-tested via `run_live`.""" + from keel.cli import _screen_product + + repo, config = open_state() + return build_screen_report(repo, config, _screen_product) + + +def _do_propose_view(open_state: OpenState) -> ProposeView: + """Build a fresh `ProposeView` over the newest shortlist in `config.proposals_dir` -- OFFLINE + (DB + local filesystem reads only), rebuilt every poll while the propose overlay is open. + `_screen_product` is lazy-imported for the identical reason `_do_screen_report` lazy-imports + it. `build_propose_view` itself never raises (see its docstring) -- any exception reaching + this function's caller can only come from `open_state()` (e.g. a locked DB), never from the + shortlist read itself.""" + from keel.cli import _screen_product + + repo, config = open_state() + return build_propose_view(repo, config, _screen_product) + + +def _do_discover_report(open_state: OpenState) -> DiscoverReport: + """THE one network call anywhere in the discover overlay -- `_build_broker(config). + list_products()` -- followed by the PURE `build_discover_report`, which turns the venue's raw + product list into candidates. Called ONLY from `run_live`'s discover branch, and only from + its Enter-key handler: never on opening the overlay, never on an ordinary poll while it stays + open. `_build_broker` is lazy-imported from `keel.commands._common`, mirroring `_do_fetch`'s + own lazy broker import, so a test can monkeypatch `keel.commands._common._build_broker` (to + record calls, or to raise if called at all) and prove this function -- and therefore this + whole overlay -- was, or crucially was NOT, ever invoked. Thin I/O -- not unit-tested + directly, only smoke-tested via `run_live`. + + BOUNDED by `_DISCOVER_TIMEOUT_SEC`, for the reason `_refresh_balance` is bounded and `_do_fetch` + deliberately is not: this is a single, small metadata request the operator is actively waiting + on with the screen frozen behind a "contacting venue" frame, so a hung connection must fail and + say so rather than freeze the dashboard until Ctrl-C. (`f` fetch legitimately runs for minutes + pulling 5y of candles, which is why it is documented as freezing the dashboard instead of being + given a timeout that would abort honest work.)""" + from keel.commands._common import _build_broker + + _repo, config = open_state() + products = _build_broker(config, timeout=_DISCOVER_TIMEOUT_SEC).list_products() + return build_discover_report(products, config) + + def run_live(open_state: OpenState, now_fn: NowFn, interval: float) -> None: """The auto-refreshing, interactive dashboard: `curses.wrapper` a loop that re-opens the repo (via `open_state`) every poll -- so it reflects writes committed by a separate `keel agent` process -- gathers a fresh report, paints it, then waits up to `interval` seconds for a keypress. - Three modes: `normal` (the dashboard, plus a transient one-line `message` toast from the last - action), `help` (a scrolled window of `build_help_screen()`), and `insights` (a scrolled - window of `build_insights_screen()` -- a READ-ONLY overlay over `build_insights_report`/ + Seven modes: `normal` (the dashboard, plus a transient one-line `message` toast from the last + action), `help` (a scrolled window of `build_help_screen()`), `insights` (a scrolled window of + `build_insights_screen()` -- a READ-ONLY overlay over `build_insights_report`/ `build_journal_report`, rebuilt fresh each poll while open, fail-soft exactly like the - normal-mode status read below). All scrolling goes through `_visible_slice`. - Normal-mode keys: `q`/`Q` quit; `h`/`?` open help; `i` open insights; `r` refresh now; `a` - toggle autonomy (`toggle_autonomy`, gated by `_confirm_arm_autonomy` on the OFF->ON direction - only); `f` fetch all data (`_do_fetch`, money-safe). `a` and `f` are both wrapped in - `_guarded` so a failure becomes a toast, never a crash. Help-mode and insights-mode keys both - scroll (`up`/`k`, `down`/`j`, `PgUp`/`PgDn`, `Home`/`End`) and close back to normal on - `q`/`Esc`/`h`/`?` (help) or `q`/`Esc`/`i` (insights). + normal-mode status read below), `screen` and `propose` (the OFFLINE admission-workflow + overlays, `build_admission_screen_overlay`/`build_propose_overlay` over `_do_screen_report`/ + `_do_propose_view`, rebuilt fresh each poll while open, fail-soft the same way insights is), + and `discover` (the one overlay that touches the network -- see below). All five scrollable + overlays share one scrolling helper, `_scroll_offset`. + + Normal-mode keys: `q`/`Q` quit; `h`/`?` open help; `i` open insights; `s` open screen; `p` + open propose; `d` open discover; `r` refresh now; `a` toggle autonomy (`toggle_autonomy`, + gated by `_confirm_arm_autonomy` on the OFF->ON direction only); `f` fetch all data + (`_do_fetch`, money-safe). `a` and `f` are both wrapped in `_guarded` so a failure becomes a + toast, never a crash. help/insights/screen/propose all scroll (`up`/`k`, `down`/`j`, + `PgUp`/`PgDn`, `Home`/`End`) and close back to normal on `q`/`Esc`/. + + `discover` is different on purpose, and is the whole reason this docstring calls out FIVE + overlays rather than treating them identically: it needs the network, and that network call + must never fire just from pressing `d`. Opening it renders an ARMED, not-yet-run explanation + (`build_discover_overlay(None)`) with NO call made. Only Enter (`10`/`13`/`curses.KEY_ENTER`), + pressed INSIDE the overlay, runs `_do_discover_report` -- the one + `_build_broker(config).list_products()` call in this entire workflow. The result (or error) + is then HELD in `discover_result`/`discover_error`: every later poll while the overlay stays + open just repaints what is held, with NO further network calls, until Enter is pressed again. + Closing the overlay (`q`/`Esc`/`d`) discards the held result, so reopening it is armed but not + yet run again. `discover` still scrolls with the same keys as the other four. Also refreshes the live "available to buy" balance (`_refresh_balance`) on its own slow cadence (`_BALANCE_REFRESH_SEC`, not every repaint -- it's a real broker call), and - immediately on `r`. This is the one other live-network read besides `f` fetch, and it is - likewise money-safe: `get_accounts` only, the exact same read rail 13 funds a buy against. + immediately on `r`. Between that automatic slow-cadence read, `f` fetch, and now `d`+Enter, + those are the only three places this whole dashboard ever touches the network -- everything + else, including all of `screen`/`propose`/`discover`'s own reads, is DB/filesystem-only. Quits on `q`/`Q` in normal mode; a `KeyboardInterrupt` (Ctrl-C) exits gracefully rather than dumping a traceback onto a terminal `curses.wrapper` may not have fully restored.""" @@ -737,6 +1087,11 @@ def _loop(stdscr: Any) -> None: mode = "normal" help_offset = 0 insights_offset = 0 + screen_offset = 0 + propose_offset = 0 + discover_offset = 0 + discover_result: DiscoverReport | None = None + discover_error: str | None = None message: str | None = None message_ts = 0 available: AvailableBalance | None = None @@ -749,22 +1104,11 @@ def _loop(stdscr: Any) -> None: _paint(stdscr, _visible_slice(help_lines, help_offset, height)) stdscr.timeout(int(interval * 1000)) ch = stdscr.getch() - if ch in (curses.KEY_UP, ord("k")): - help_offset -= 1 - elif ch in (curses.KEY_DOWN, ord("j")): - help_offset += 1 - elif ch == curses.KEY_PPAGE: - help_offset -= max(height - 1, 1) - elif ch == curses.KEY_NPAGE: - help_offset += max(height - 1, 1) - elif ch == curses.KEY_HOME: - help_offset = 0 - elif ch == curses.KEY_END: - help_offset = len(help_lines) - elif ch in (ord("q"), 27, ord("h"), ord("?")): + if ch in (ord("q"), 27, ord("h"), ord("?")): mode = "normal" help_offset = 0 - help_offset = max(0, min(help_offset, max(0, len(help_lines) - height))) + else: + help_offset = _scroll_offset(ch, help_offset, height, len(help_lines), curses) continue if mode == "insights": @@ -799,22 +1143,100 @@ def _loop(stdscr: Any) -> None: _paint(stdscr, _visible_slice(insights_lines, insights_offset, height)) stdscr.timeout(int(interval * 1000)) ch = stdscr.getch() - if ch in (curses.KEY_UP, ord("k")): - insights_offset -= 1 - elif ch in (curses.KEY_DOWN, ord("j")): - insights_offset += 1 - elif ch == curses.KEY_PPAGE: - insights_offset -= max(height - 1, 1) - elif ch == curses.KEY_NPAGE: - insights_offset += max(height - 1, 1) - elif ch == curses.KEY_HOME: - insights_offset = 0 - elif ch == curses.KEY_END: - insights_offset = len(insights_lines) - elif ch in (ord("q"), 27, ord("i")): + if ch in (ord("q"), 27, ord("i")): mode = "normal" insights_offset = 0 - insights_offset = max(0, min(insights_offset, max(0, len(insights_lines) - height))) + else: + insights_offset = _scroll_offset( + ch, insights_offset, height, len(insights_lines), curses + ) + continue + + if mode == "screen": + # OFFLINE + fail-soft, mirroring the insights branch above: `_do_screen_report` + # only reads the DB (never a broker, never the network -- see its own docstring), + # and a transient read error paints an alert line and keeps polling instead of + # crashing the loop. + try: + screen_report = _do_screen_report(open_state) + screen_lines = build_admission_screen_overlay(screen_report) + except Exception as exc: + screen_lines = [ + ScreenLine(f"screen read failed: {exc} -- retrying...", "alert") + ] + + height, _width = stdscr.getmaxyx() + _paint(stdscr, _visible_slice(screen_lines, screen_offset, height)) + stdscr.timeout(int(interval * 1000)) + ch = stdscr.getch() + if ch in (ord("q"), 27, ord("s")): + mode = "normal" + screen_offset = 0 + else: + screen_offset = _scroll_offset( + ch, screen_offset, height, len(screen_lines), curses + ) + continue + + if mode == "propose": + # OFFLINE + fail-soft, same shape as screen above. `_do_propose_view` itself + # never raises (`build_propose_view`'s own docstring) -- an exception here can + # only come from `open_state()` (e.g. a locked DB), never from the shortlist read. + try: + propose_lines = build_propose_overlay(_do_propose_view(open_state)) + except Exception as exc: + propose_lines = [ + ScreenLine(f"propose read failed: {exc} -- retrying...", "alert") + ] + + height, _width = stdscr.getmaxyx() + _paint(stdscr, _visible_slice(propose_lines, propose_offset, height)) + stdscr.timeout(int(interval * 1000)) + ch = stdscr.getch() + if ch in (ord("q"), 27, ord("p")): + mode = "normal" + propose_offset = 0 + else: + propose_offset = _scroll_offset( + ch, propose_offset, height, len(propose_lines), curses + ) + continue + + if mode == "discover": + # NETWORK-GATED, on purpose -- see the module docstring and `build_discover_ + # overlay`'s. Unlike every branch above, this one does NOT rebuild anything on an + # ordinary poll: `discover_result`/`discover_error` are HELD from the last Enter + # (both `None` if Enter has never been pressed since the overlay opened), and + # every poll just repaints whatever is currently held. The Enter-key check below + # is the ONLY place in this whole branch that calls `_do_discover_report`. + discover_lines = build_discover_overlay(discover_result, error=discover_error) + + height, _width = stdscr.getmaxyx() + _paint(stdscr, _visible_slice(discover_lines, discover_offset, height)) + stdscr.timeout(int(interval * 1000)) + ch = stdscr.getch() + if ch in (ord("q"), 27, ord("d")): + mode = "normal" + discover_offset = 0 + # Discard the held result -- reopening the overlay is armed-but-not-run again. + discover_result = None + discover_error = None + elif ch in (10, 13, curses.KEY_ENTER): + _paint(stdscr, [ScreenLine("contacting venue... please wait", "normal")]) + try: + discover_result = _do_discover_report(open_state) + discover_error = None + except Exception as exc: + discover_result = None + # Truncated for the same reason `_refresh_balance`'s error is: a stray + # huge or sensitive blob (an HTTP error body, say) must never be painted + # full-screen verbatim. + discover_error = str(exc)[:200] + discover_offset = 0 + else: + discover_offset = _scroll_offset( + ch, discover_offset, height, len(discover_lines), curses + ) continue # mode == "normal" @@ -862,6 +1284,23 @@ def _loop(stdscr: Any) -> None: mode = "insights" insights_offset = 0 continue + if ch == ord("s"): + mode = "screen" + screen_offset = 0 + continue + if ch == ord("p"): + mode = "propose" + propose_offset = 0 + continue + if ch == ord("d"): + mode = "discover" + discover_offset = 0 + # Always opens ARMED, not yet run -- even if a previous visit left a held result, + # a fresh 'd' press starts over rather than silently showing stale data. (Closing + # the overlay already clears these too; this is belt-and-braces.) + discover_result = None + discover_error = None + continue if ch == ord("r"): last_balance_ts = 0 # force the balance to re-fetch on the next iteration too # Toast it. `r` always succeeds and usually changes nothing on screen (the DB @@ -936,20 +1375,35 @@ def tui_cmd(ctx: click.Context, interval: float, once: bool) -> None: A view over the same `gather_status` report `keel status` prints once: mode, kill-switch, autonomy, Rail 11 drawdown/equity state, open positions, rule counts, per-product data - freshness, and subscriptions, auto-refreshing on an interval. Never places an order and never - touches the network except when explicitly asked to (`f`). Press `h`/`?` for the in-app help - (every keybinding and the safety notes); `i` opens a browsable, READ-ONLY insights overlay - (per-rule track record, promotion-gate distance, account summary, and a recent-trades tail -- - reusing `keel insights`' own pure builders/renderers verbatim); `a` toggles autonomy (turning - it OFF is instant, turning it ON needs a typed "yes" at the terminal, exactly like `keel - autonomy on`); `f` fetches fresh candle history for every configured product (money-safe: no - orders); `r` refreshes immediately. Quit with `q`. + freshness, and subscriptions, auto-refreshing on an interval. Never places an order. + + Network touches are the exception, not the rule, and there are exactly three: an automatic + read of the real account's spendable balance every ~30s (`get_accounts`, the same read rail + 13 funds a buy against -- see `run_live`'s docstring), `f` fetch (pulls candle history, no + orders), and `d`+Enter inside the discover overlay (pulls the venue's product list, no + orders). Everything else -- including the whole `s` screen / `p` propose / `d` discover + admission workflow up until that one Enter keypress -- is DB/filesystem reads only. + + Press `h`/`?` for the in-app help (every keybinding and the safety notes); `i` opens a + browsable, READ-ONLY insights overlay (per-rule track record, promotion-gate distance, + account summary, and a recent-trades tail -- reusing `keel insights`' own pure + builders/renderers verbatim); `s` opens a READ-ONLY screen overlay (the allowlist's current + admission verdicts, reusing `keel assets screen`'s own gate); `p` opens a READ-ONLY propose + overlay (screens the newest shortlist file in `config.proposals_dir`); `d` opens the discover + overlay ARMED but not yet run -- it explains itself and waits for Enter before making its one + live venue call, then holds that result until Enter is pressed again or the overlay closes. + None of `screen`/`propose`/`discover` attests, admits, or trades -- `attest` (the human + judgment this whole gate rests on) stays deliberately CLI-only, `keel assets attest`. `a` + toggles autonomy (turning it OFF is instant, turning it ON needs a typed "yes" at the + terminal, exactly like `keel autonomy on`); `f` fetches fresh candle history for every + configured product (money-safe: no orders); `r` refreshes immediately. Quit with `q`. `--once` renders a single, static frame to stdout and exits without touching curses, for pipes/CI, matching `status`'s scripting-friendliness (and prints the disclaimer footer after - the frame) -- none of the interactive actions are available there. The default, interactive - path owns the whole screen via `curses.wrapper` and re-opens the repo every poll so it - reflects writes committed by a separate `keel agent` process. + the frame) -- none of the interactive actions (including `screen`/`propose`/`discover`) are + available there. The default, interactive path owns the whole screen via `curses.wrapper` and + re-opens the repo every poll so it reflects writes committed by a separate `keel agent` + process. """ if interval <= 0: raise click.ClickException("--interval must be > 0") diff --git a/keel/compliance/screen.py b/keel/compliance/screen.py index ec4d5388..c35d3d00 100644 --- a/keel/compliance/screen.py +++ b/keel/compliance/screen.py @@ -22,7 +22,7 @@ from __future__ import annotations -from collections.abc import Mapping +from collections.abc import Mapping, Sequence from dataclasses import dataclass, field from decimal import Decimal @@ -59,14 +59,17 @@ WAIVABLE_CRITERIA = frozenset({"history"}) #: Failure classes that are DOWNSTREAM of having no cached history: with zero bars `liquidity` -#: reports on our data (median volume is 0 *because* there are no bars), not on the asset, so -#: callers presenting a zero-bar candidate (`assets holdings`, `assets propose`) must not print it -#: as a verdict. `settlement` is deliberately NOT here -- it compares the product's quote leg to -#: the settlement currency and never touches candles, so it stays a real, assessable verdict even -#: with zero bars. This is the single source of truth for that tag set; callers import it rather -#: than duplicating the string, so a tag rename here cannot silently disable the suppression +#: reports on our data (median volume is 0 *because* there are no bars), not on the asset. `history` +#: belongs here for the identical reason and was the bug this set used to miss: at zero bars, +#: `history: 0 daily bars, need 1460` measures the depth of OUR CACHE, not the age of the asset -- +#: a candidate never fetched is indistinguishable from one genuinely too young unless this tag is +#: suppressed exactly like `liquidity` is. `settlement` is deliberately NOT here -- it compares the +#: product's quote leg to the settlement currency and never touches candles, so it stays a real, +#: assessable verdict even with zero bars. This is the single source of truth for that tag set; +#: callers import it (or, better, call `split_failures`/`missing_history_lines` below rather than +#: reimplementing the split) so a tag rename here cannot silently disable the suppression #: elsewhere. -DATA_DERIVED_FAILURES = frozenset({"liquidity"}) +DATA_DERIVED_FAILURES = frozenset({"history", "liquidity"}) @dataclass(frozen=True) @@ -254,6 +257,65 @@ def screen_asset( ) +def split_failures(facts: MarketFacts, result: ScreenResult) -> tuple[list[str], list[str]]: + """Partition `result.failures` into `(about_the_asset, about_our_cache)`. + + This is the DECISION half of the zero-bar-history fix, kept in `screen.py` rather than in + each caller, because this module already owns `DATA_DERIVED_FAILURES` -- the tag set that + decides which failures are "downstream of an empty cache" in the first place. Duplicating the + split next to each caller (as `assets holdings` and `assets propose` used to, independently) + let the two copies drift: a tag rename here would silently stop suppressing one of them and + keep suppressing the other. Putting the split next to the tag set makes that impossible -- + there is exactly one place either can change. + + With `facts.daily_bars > 0` every failure is returned as `about_the_asset` and + `about_our_cache` is empty, UNSPLIT -- including a genuine `history` shortfall, because an + asset with some cached history that still falls short of the floor really is too young, and + that verdict must not be silenced. The split only ever happens at EXACTLY zero bars, which is + the one condition where `history` (and `liquidity`) measure our cache instead of the asset. + Original ordering is preserved within each returned list, so a caller that renders them in + order does not see failures reshuffled relative to how `screen_asset` produced them. + """ + if facts.daily_bars > 0: + return list(result.failures), [] + about_the_asset: list[str] = [] + about_our_cache: list[str] = [] + for failure in result.failures: + if failure.split(":")[0] in DATA_DERIVED_FAILURES: + about_our_cache.append(failure) + else: + about_the_asset.append(failure) + return about_the_asset, about_our_cache + + +def missing_history_lines(product_id: str, not_assessable: Sequence[str]) -> list[str]: + """The MISSING-DATA explanation shown INSTEAD OF `not_assessable`'s raw failures. + + Lives here, beside `split_failures`, for the same reason: the wording references the exact + tag set this module owns, so the explanation and the tags it explains cannot drift apart. + Formatting (indentation, bullets, `!`/`·` markers) is deliberately left to the caller instead + of baked in here -- `keel/proposer.py` and `keel/cli.py`'s `assets holdings` indent by + different amounts, and a third caller (the TUI) will have its own widget-native layout again. + This function returns plain, UNINDENTED lines; callers prepend whatever presentation they need. + + The third line -- naming what is still unassessable -- is included only when `not_assessable` + is non-empty, so a candidate that fails purely on shape/settlement/attestation (nothing + downstream of the cache) does not get a trailing "not assessable until then:" with nothing + after the colon. Tags are deduplicated and sorted so two failures sharing a tag collapse to + one mention and the order is stable regardless of how `screen_asset` happened to emit them. + """ + lines = [ + f"no local history for {product_id} -- run `keel fetch --products {product_id}` first, " + "then re-screen.", + "This is a MISSING-DATA verdict, not a verdict about the asset: it is not too young, " + "we have simply never fetched candles for it.", + ] + if not_assessable: + tags = sorted({f.split(":")[0] for f in not_assessable}) + lines.append(f"not assessable until then: {', '.join(tags)}") + return lines + + # -- discovery (candidate PROPOSAL, not admission) ----------------------------- diff --git a/keel/proposer.py b/keel/proposer.py index a2cb85fe..0838fd33 100644 --- a/keel/proposer.py +++ b/keel/proposer.py @@ -15,7 +15,7 @@ from keel.commands._products import _history_product from keel.compliance import screen as screen_mod -from keel.compliance.screen import DATA_DERIVED_FAILURES +from keel.compliance.screen import missing_history_lines, split_failures from keel.data.repository import Repository ScreenFn = Callable[ @@ -189,17 +189,17 @@ def render_proposal_report(report: ProposalReport) -> list[str]: lines.append( f" UNVERIFIED hypothesis (never used for admission): {cand.shariah_hypothesis}" ) - failures = list(sc.result.failures) + # The zero-bars split/explanation is decided ONCE, in `keel.compliance.screen` -- see + # `split_failures`'s docstring for why the decision cannot safely live here: this module + # and `keel/cli.py`'s `assets holdings` used to each keep their own copy of the same + # split, which could (and had) drift apart. `keel fetch --products {sc.product}` matches + # `test_render_no_history_shows_missing_data_next_step`'s exact-substring pin. + failures, not_assessable = split_failures(sc.facts, sc.result) if sc.facts.daily_bars == 0: - derived = [f for f in failures if f.split(":")[0] in DATA_DERIVED_FAILURES] - failures = [f for f in failures if f not in derived] - lines.append( - f" ! no local history -- run `keel fetch --products {sc.product}` first, " - "then re-screen." - ) - lines.append(" This is a MISSING-DATA verdict, not a verdict about the asset.") - for failure in derived: - lines.append(f" · ({failure.split(':')[0]}: not assessable without history)") + explanation = missing_history_lines(sc.product, not_assessable) + lines.append(f" ! {explanation[0]}") + for extra_line in explanation[1:]: + lines.append(f" {extra_line}") for failure in failures: lines.append(f" ✗ {failure}") for warning in sc.result.warnings: diff --git a/packages/keel-core/keel_core/config.py b/packages/keel-core/keel_core/config.py index 85aae07b..3c18cfa3 100644 --- a/packages/keel-core/keel_core/config.py +++ b/packages/keel-core/keel_core/config.py @@ -297,6 +297,19 @@ class Config: tiers: tuple[TierConfig, ...] = field(default_factory=_default_tiers) fees: FeesConfig = field(default_factory=FeesConfig) quote_currency: str = "USD" + # Where `keel assets propose`/the TUI's propose overlay look for an externally-produced + # shortlist JSON (see `keel/commands/admission.py::latest_shortlist`). This is a FIELD, not a + # constant baked into that module, for the same reason `settlement_currencies` is a field + # rather than a `guards.py` literal: a user-specific absolute path hardcoded into library code + # is untestable (every test would either write to it or monkeypatch it) and wrong for anyone + # else's machine. `~` is expanded by the READER (`Path(config.proposals_dir).expanduser()`) + # at USE time, never here at parse time -- expanding at parse would bake the parsing + # machine's home directory into a `Config` that a test fixture or a different operator's + # checkout might reasonably load, which is exactly the kind of environment-dependent value + # `load_config` is supposed to keep out of a `Config` object. Keeping it as the literal string + # `"~/keel/proposals"` is what makes the default portable across machines and safe to assert + # on directly in a test (see `test_load_config_proposals_dir_defaults_to_keel_proposals`). + proposals_dir: str = "~/keel/proposals" # Rail 18's allowed settlement legs -- the currencies an order may settle in, matched against # `quote_currency_of(product_id)`. NOT the same field as `quote_currency` above, which names # the ONE currency this deployment trades in (it screens candidates and excludes the @@ -655,6 +668,18 @@ def load_config(path: str | Path) -> Config: if not isinstance(quote_currency, str) or not quote_currency: raise ConfigError(f"quote_currency: must be a non-empty string, got {quote_currency!r}") + # Mirrors `quote_currency` immediately above: optional, but if present must be a non-empty + # string. No shape/existence check beyond that -- unlike `allowlist`/`settlement_currencies`, + # this is a filesystem path, not a venue identifier the rails compare against, so there is no + # "would silently veto every order" failure mode to catch here. A missing directory is handled + # by the reader (`latest_shortlist` returns `None` rather than raising) precisely because the + # directory legitimately does not exist yet on a fresh install. + proposals_dir = raw.get("proposals_dir", "~/keel/proposals") + if not isinstance(proposals_dir, str) or not proposals_dir: + raise ConfigError( + f"proposals_dir: must be a non-empty string, got {proposals_dir!r}" + ) + settlement_currencies = _parse_settlement_currencies(raw) # The two settings are independent knobs that describe the SAME leg, and a config that moves @@ -758,6 +783,7 @@ def load_config(path: str | Path) -> Config: tiers=_parse_tiers(raw), fees=_parse_fees(raw), quote_currency=quote_currency, + proposals_dir=proposals_dir, settlement_currencies=settlement_currencies, logging=_parse_logging(raw), research=_parse_research(raw), diff --git a/tests/commands/test_admission.py b/tests/commands/test_admission.py new file mode 100644 index 00000000..74d70382 --- /dev/null +++ b/tests/commands/test_admission.py @@ -0,0 +1,619 @@ +"""Tests for `keel.commands.admission` -- the OFFLINE report layer behind the TUI's `screen`, +`propose` and `discover` overlays. + +Mirrors `tests/commands/test_insights.py`/`tests/commands/test_tui.py`'s fixture style: an +in-memory `Repository` via `connect(":memory:")` + `migrate`, a `_config(**overrides)` helper, a +`NOW_TS` constant. `cli_module._screen_product` (THE admission gate) is used as the injected +`screen_fn` throughout, exactly as the module docstring says every verdict must come from it -- +using anything else here would test a screen this module does not actually ship with. + +Two tests are the correctness headline of this file and are docstringed as such where they live: +`test_render_screen_report_zero_bars_reads_as_missing_data_not_an_asset_verdict` and +`test_render_screen_report_insufficient_bars_reads_as_a_real_history_failure`. Together they pin +that "never fetched" and "genuinely too new" stay distinguishable in the rendered report, which is +the exact bug `keel.compliance.screen.split_failures`/`missing_history_lines` exist to prevent. +""" + +from __future__ import annotations + +import json +import os +from decimal import Decimal +from pathlib import Path +from typing import Any + +import pytest + +import keel.cli as cli_module +from keel.commands.admission import ( + DEFAULT_PROPOSALS_DIR, + DiscoverReport, + ProposeView, + ScreenReport, + build_discover_report, + build_propose_view, + build_screen_report, + latest_shortlist, + render_discover_report, + render_propose_view, + render_screen_report, +) +from keel.compliance import screen as screen_mod +from keel.config import ( + AutoTradeConfig, + Caps, + Config, + DcaConfig, + MarketDataConfig, + MoneyMgmtConfig, +) +from keel.data.db import connect, migrate +from keel.data.repository import Repository +from keel.types import Candle, Granularity + +NOW_TS = 1_800_000_000 +_DAY = 86400 + + +@pytest.fixture +def repo() -> Repository: + conn = connect(":memory:") + migrate(conn) + r = Repository(conn) + r.set_state("kill_switch", False) + return r + + +def _repo_at(db_path: Path) -> Repository: + conn = connect(str(db_path)) + migrate(conn) + return Repository(conn) + + +def _config(**overrides: Any) -> Config: + base: dict[str, Any] = dict( + allowlist=["BTC", "SOL"], + target_weights={}, + risk_pct=Decimal("0.01"), + caps=Caps( + max_per_order_usd=Decimal("100000"), + max_per_day_usd=Decimal("300000"), + max_exposure_usd=Decimal("1000000"), + max_per_asset_pct=Decimal("1"), + ), + market_data=MarketDataConfig( + granularities=[Granularity.ONE_DAY, Granularity.ONE_HOUR], history_days=365 + ), + auto_trade=AutoTradeConfig(mode="paper", interval_sec=900), + money_mgmt=MoneyMgmtConfig( + max_total_dd_pct=Decimal("0.20"), max_weekly_dd_pct=Decimal("0.08") + ), + dca=DcaConfig(budget_usd=Decimal("50"), cadence_days=7), + ) + base.update(overrides) + return Config(**base) + + +def _seed_history(repo: Repository, product: str, bars: int) -> None: + repo.upsert_candles( + product, + Granularity.ONE_DAY, + [ + Candle( + ts=i * _DAY, + open=Decimal("100"), + high=Decimal("101"), + low=Decimal("99"), + close=Decimal("100"), + volume=Decimal("100000"), + ) + for i in range(bars) + ], + ) + + +def _write_shortlist(directory: Path, candidates: list[dict], name: str = "shortlist.json") -> Path: + directory.mkdir(parents=True, exist_ok=True) + path = directory / name + path.write_text(json.dumps({"candidates": candidates})) + return path + + +_SOL_CANDIDATE = { + "asset": "sol", + "rationale": "high liquidity and developer activity", + "sources": ["https://coinmarketcap.com/currencies/solana/"], +} + + +# -- latest_shortlist (offline, filesystem only) ------------------------------------------------ + + +def test_latest_shortlist_picks_newest_by_mtime(tmp_path): + directory = tmp_path / "proposals" + directory.mkdir() + older = directory / "a.json" + newer = directory / "b.json" + older.write_text("{}") + newer.write_text("{}") + now = 1_800_000_000 + os.utime(older, (now, now)) + os.utime(newer, (now + 100, now + 100)) + + assert latest_shortlist(directory) == newer + + +def test_latest_shortlist_ties_broken_by_name(tmp_path): + directory = tmp_path / "proposals" + directory.mkdir() + a = directory / "a.json" + z = directory / "z.json" + a.write_text("{}") + z.write_text("{}") + same_ts = 1_800_000_000 + os.utime(a, (same_ts, same_ts)) + os.utime(z, (same_ts, same_ts)) + + assert latest_shortlist(directory) == z + + +def test_latest_shortlist_missing_directory_returns_none_and_creates_nothing(tmp_path): + directory = tmp_path / "does-not-exist" + + assert latest_shortlist(directory) is None + assert not directory.exists() # never creates the directory it was asked to read + + +def test_latest_shortlist_non_directory_returns_none(tmp_path): + not_a_dir = tmp_path / "a_file" + not_a_dir.write_text("not a directory") + + assert latest_shortlist(not_a_dir) is None + + +def test_latest_shortlist_empty_directory_returns_none(tmp_path): + directory = tmp_path / "proposals" + directory.mkdir() + + assert latest_shortlist(directory) is None + + +def test_latest_shortlist_ignores_non_json_files(tmp_path): + directory = tmp_path / "proposals" + directory.mkdir() + (directory / "readme.txt").write_text("not json") + + assert latest_shortlist(directory) is None + + +def test_default_proposals_dir_matches_configs_default(): + """Kept in sync manually (per the module's own comment) -- pinned here so a drift breaks a + test instead of silently disagreeing with `Config.proposals_dir`'s default.""" + assert DEFAULT_PROPOSALS_DIR == "~/keel/proposals" + assert Config.__dataclass_fields__["proposals_dir"].default == DEFAULT_PROPOSALS_DIR + + +# -- build_screen_report / render_screen_report (offline, DB reads only) ------------------------ + + +def test_build_screen_report_routes_every_allowlist_product_through_injected_screen_fn( + repo: Repository, +): + """No candidate can reach a laxer path -- every product the report screens must come from + the SAME injected gate, called once per allowlist product with the right product id.""" + config = _config(allowlist=["BTC", "SOL"]) + calls: list[tuple[str, str]] = [] + + def fake_screen(repo: Repository, product: str, quote: str): + calls.append((product, quote)) + facts = screen_mod.MarketFacts( + asset=product.split("-")[0], + daily_bars=2000, + median_daily_volume=Decimal("2000000"), + quotable_in_settlement_currency=True, + product_id=product, + ) + result = screen_mod.ScreenResult(asset=facts.asset, admitted=True) + return facts, result + + report = build_screen_report(repo, config, fake_screen) + + assert calls == [("BTC-USD", "USD"), ("SOL-USD", "USD")] + assert isinstance(report, ScreenReport) + assert [s.product for s in report.screened] == ["BTC-USD", "SOL-USD"] + assert report.admitted_count == 2 + + +def test_build_screen_report_empty_allowlist_produces_no_screened_products(repo: Repository): + config = _config(allowlist=[]) + + report = build_screen_report(repo, config, cli_module._screen_product) + + assert report.screened == [] + assert report.admitted_count == 0 + + +def test_render_screen_report_empty_allowlist_says_so_plainly(repo: Repository): + config = _config(allowlist=[]) + report = build_screen_report(repo, config, cli_module._screen_product) + + lines = render_screen_report(report) + + assert lines # never renders nothing + text = "\n".join(lines).lower() + assert "empty" in text + + +def test_render_screen_report_zero_bars_reads_as_missing_data_not_an_asset_verdict( + repo: Repository, +): + """THE correctness headline (1/2). With zero cached candles, `history`/`liquidity` measure + OUR CACHE, not the asset -- see `screen.split_failures`'s docstring. The rendered report must + say "no local history, run keel fetch" and must NOT print a `✗ history:` line, or a candidate + never fetched would read as indistinguishable from one genuinely too young. + """ + config = _config(allowlist=["SOL"]) # no candles seeded for SOL-USD at all + + report = build_screen_report(repo, config, cli_module._screen_product) + lines = render_screen_report(report) + text = "\n".join(lines) + + assert "no local history" in text + assert "keel fetch --products SOL-USD" in text + assert "✗ history" not in text + + +def test_render_screen_report_insufficient_bars_reads_as_a_real_history_failure(repo: Repository): + """THE correctness headline (2/2), the counterpart to the zero-bars test above. An asset with + SOME cached history that still falls short of the floor really IS too young, and that verdict + must not be silenced -- `✗ history:` must appear, proving "never fetched" and "genuinely too + new" are distinguishable in the report.""" + config = _config(allowlist=["SOL"]) + _seed_history(repo, "SOL-USD", bars=500) # < 4*365 required, but > 0 + + report = build_screen_report(repo, config, cli_module._screen_product) + lines = render_screen_report(report) + text = "\n".join(lines) + + assert "✗ history" in text + assert "no local history" not in text + + +def test_render_screen_report_ends_with_admitted_count(repo: Repository): + config = _config(allowlist=["SOL"]) + _seed_history(repo, "SOL-USD", bars=500) + + report = build_screen_report(repo, config, cli_module._screen_product) + lines = render_screen_report(report) + + assert lines[-1] == f"{report.admitted_count}/{len(report.screened)} admitted" + + +# -- build_propose_view / render_propose_view (offline, fail-soft) ------------------------------ + + +def test_build_propose_view_missing_directory_is_fail_soft(repo: Repository, tmp_path): + config = _config() + missing = tmp_path / "no-such-dir" + + view = build_propose_view(repo, config, cli_module._screen_product, directory=missing) + + assert view.status == "no-directory" + assert view.detail is not None + assert view.report is None + assert not missing.exists() # never creates the directory it looked in + + +def test_build_propose_view_empty_directory_is_fail_soft(repo: Repository, tmp_path): + config = _config() + directory = tmp_path / "proposals" + directory.mkdir() + + view = build_propose_view(repo, config, cli_module._screen_product, directory=directory) + + assert view.status == "no-shortlist" + assert view.detail is not None + assert view.report is None + + +def test_build_propose_view_unreadable_file_is_fail_soft(repo: Repository, tmp_path): + """A directory named `*.json` triggers `IsADirectoryError` (an `OSError` subclass) on + `.read_text()` -- a portable way to force an unreadable-file path without relying on chmod + semantics that differ across platforms/CI users (e.g. running as root).""" + config = _config() + directory = tmp_path / "proposals" + directory.mkdir() + (directory / "shortlist.json").mkdir() + + view = build_propose_view(repo, config, cli_module._screen_product, directory=directory) + + assert view.status == "unreadable" + assert view.detail is not None + assert view.report is None + + +def test_build_propose_view_invalid_json_is_fail_soft(repo: Repository, tmp_path): + config = _config() + directory = tmp_path / "proposals" + directory.mkdir() + (directory / "shortlist.json").write_text("{not json") + + view = build_propose_view(repo, config, cli_module._screen_product, directory=directory) + + assert view.status == "malformed" + assert view.detail is not None + assert view.report is None + + +def test_build_propose_view_malformed_top_level_proposal_is_fail_soft(repo: Repository, tmp_path): + config = _config() + directory = tmp_path / "proposals" + directory.mkdir() + (directory / "shortlist.json").write_text(json.dumps({"candidates": "nope"})) + + view = build_propose_view(repo, config, cli_module._screen_product, directory=directory) + + assert view.status == "malformed" + assert view.detail is not None + assert view.report is None + + +def test_build_propose_view_happy_path(repo: Repository, tmp_path): + config = _config() + directory = tmp_path / "proposals" + shortlist = _write_shortlist(directory, [_SOL_CANDIDATE]) + + view = build_propose_view(repo, config, cli_module._screen_product, directory=directory) + + assert view.status == "ok" + assert view.source == shortlist + assert view.detail is None + assert view.report is not None + assert view.report.screened[0].candidate.asset == "SOL" + + +def test_build_propose_view_explicit_path_skips_the_newest_file_search(repo: Repository, tmp_path): + config = _config() + directory = tmp_path / "proposals" + directory.mkdir() + older = _write_shortlist(directory, [_SOL_CANDIDATE], name="older.json") + os.utime(older, (NOW_TS, NOW_TS)) + newer = _write_shortlist( + directory, [{"asset": "btc", "rationale": "r", "sources": ["https://x.invalid"]}], + name="newer.json", + ) + os.utime(newer, (NOW_TS + 100, NOW_TS + 100)) + + view = build_propose_view( + repo, config, cli_module._screen_product, directory=directory, path=older + ) + + assert view.source == older + assert view.report.screened[0].candidate.asset == "SOL" + + +@pytest.mark.parametrize( + "bad_candidate", + [ + {"rationale": "r", "sources": ["https://x.invalid"]}, # missing asset + {"asset": "SOL", "rationale": "r"}, # missing sources + {"asset": "SOL", "rationale": "r", "sources": ["not-a-url"]}, # non-http source + {"asset": "sol-usd", "rationale": "r", "sources": ["https://x.invalid"]}, # non-alnum + ], +) +def test_build_propose_view_never_raises_and_always_returns_a_status( + repo: Repository, tmp_path, bad_candidate: dict +): + """The fail-soft contract, exercised across the whole matrix in one parametrized sweep -- + this function must never raise regardless of what is on disk.""" + config = _config() + directory = tmp_path / "proposals" + _write_shortlist(directory, [bad_candidate]) + + view = build_propose_view(repo, config, cli_module._screen_product, directory=directory) + + assert isinstance(view, ProposeView) + assert view.status == "ok" # the top-level structure is still valid; the ENTRY is invalid + assert len(view.report.invalid) == 1 + assert view.report.screened == [] + + +def test_invalid_shortlist_entries_are_reported_not_dropped(repo: Repository, tmp_path): + config = _config() + directory = tmp_path / "proposals" + _write_shortlist(directory, [{"asset": "SOL", "rationale": "r", "sources": []}]) + + view = build_propose_view(repo, config, cli_module._screen_product, directory=directory) + lines = render_propose_view(view) + text = "\n".join(lines) + + assert "INVALID" in text + + +def test_render_propose_view_never_crashes_on_a_missing_directory(repo: Repository, tmp_path): + config = _config() + view = build_propose_view( + repo, config, cli_module._screen_product, directory=tmp_path / "missing" + ) + + lines = render_propose_view(view) + + assert lines + assert all(isinstance(line, str) for line in lines) + joined = "\n".join(lines).lower() + assert "traceback" not in joined + assert "exception" not in joined + + +def test_render_propose_view_never_crashes_on_an_empty_directory(repo: Repository, tmp_path): + config = _config() + directory = tmp_path / "empty" + directory.mkdir() + view = build_propose_view(repo, config, cli_module._screen_product, directory=directory) + + lines = render_propose_view(view) + + assert lines + assert all(isinstance(line, str) for line in lines) + joined = "\n".join(lines).lower() + assert "traceback" not in joined + assert "exception" not in joined + + +def test_render_propose_view_happy_path_reuses_render_proposal_report_verbatim( + repo: Repository, tmp_path +): + """Pins the reuse: `render_propose_view` must not grow a parallel rendering path. `"source: + "` is a distinctive substring ONLY `render_proposal_report` produces (see + `keel/proposer.py`), so its presence here proves the real renderer ran.""" + config = _config() + directory = tmp_path / "proposals" + shortlist = _write_shortlist(directory, [_SOL_CANDIDATE]) + + view = build_propose_view(repo, config, cli_module._screen_product, directory=directory) + lines = render_propose_view(view) + text = "\n".join(lines) + + assert str(shortlist) in lines[0] + assert "source: https://coinmarketcap.com/currencies/solana/" in text + assert "admitted" in text # the trailing "N/M admitted" summary line + + +# -- writes nothing (screen + propose are both purely read-only) -------------------------------- + + +def test_build_screen_report_and_propose_view_write_nothing(tmp_path): + """Mirrors `test_propose_writes_nothing` in `tests/compliance/test_assets_cli.py`: reopen the + DB from its path (not the handle held from before the calls) so a stray write to ANY + asset/table would actually be caught.""" + db_path = tmp_path / "t.db" + repo = _repo_at(db_path) + config = _config(allowlist=["SOL"]) + directory = tmp_path / "proposals" + _write_shortlist(directory, [_SOL_CANDIDATE]) + + build_screen_report(repo, config, cli_module._screen_product) + build_propose_view(repo, config, cli_module._screen_product, directory=directory) + + assert _repo_at(db_path).get_asset_attestations() == [] + + +# -- build_discover_report / render_discover_report (pure over already-fetched products) -------- + + +def _venue_product( + asset: str, volume: str = "10000000", quote: str = "USD", **overrides: Any +) -> dict: + base = { + "product_id": f"{asset}-{quote}", + "quote_currency_id": quote, + "status": "online", + "trading_disabled": False, + "is_disabled": False, + "view_only": False, + "base_name": asset.title(), + "quote_24h_volume": volume, + } + base.update(overrides) + return base + + +def test_build_discover_report_is_offline_no_broker_constructed( + repo: Repository, monkeypatch: pytest.MonkeyPatch +): + """Pure over `products` -- the caller fetches; this module never touches the network. Patch + `_build_broker` to blow up if anything here ever reaches for it.""" + + def _must_not_build_broker(config: Config) -> Any: + raise AssertionError("build_discover_report must never construct a broker") + + monkeypatch.setattr(cli_module, "_build_broker", _must_not_build_broker) + config = _config() + products = [_venue_product("DOGE")] + + report = build_discover_report(products, config) + + assert isinstance(report, DiscoverReport) + assert report.venue_product_count == 1 + + +def test_build_discover_report_excludes_allowlist_assets(repo: Repository): + config = _config(allowlist=["BTC"]) + products = [_venue_product("BTC"), _venue_product("DOGE")] + + report = build_discover_report(products, config) + + assets = [c.asset for c in report.candidates] + assert "BTC" not in assets + assert "DOGE" in assets + + +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.""" + 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("5000000") + + config = _config(allowlist=[]) + below = _venue_product("DOGE", volume="4999999") + above = _venue_product("SHIB", volume="5000001") + + report = build_discover_report([below, above], config) + + assert report.min_quote_24h_volume == default_floor + assets = [c.asset for c in report.candidates] + assert "DOGE" not in assets + assert "SHIB" in assets + + +def test_build_discover_report_custom_volume_floor(repo: Repository): + config = _config(allowlist=[]) + products = [_venue_product("DOGE", volume="1000")] + + report = build_discover_report(products, config, min_quote_24h_volume=Decimal("500")) + + assert [c.asset for c in report.candidates] == ["DOGE"] + + +def test_build_discover_report_limit_respected(repo: Repository): + 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) + + assert len(report.candidates) == 3 + # sorted descending by volume, so the top 3 by volume survive the limit + assert report.candidates[0].quote_24h_volume >= report.candidates[1].quote_24h_volume + assert report.candidates[1].quote_24h_volume >= report.candidates[2].quote_24h_volume + + +def test_render_discover_report_shows_table_and_ends_with_proposals_not_admissions_warning(): + config = _config(allowlist=[]) + products = [_venue_product("DOGE")] + report = build_discover_report(products, config) + + lines = render_discover_report(report) + text = "\n".join(lines) + + assert "DOGE-USD" in text + assert "PROPOSALS, not admissions" in lines[-1] + assert "keel assets attest" 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.""" + config = _config(allowlist=[]) + products = [_venue_product("DOGE")] + report = build_discover_report(products, config) + + lines = render_discover_report(report) + text = "\n".join(lines).lower() + + assert "4yr?" not in text diff --git a/tests/commands/test_tui.py b/tests/commands/test_tui.py index 3d872e12..eaabf43a 100644 --- a/tests/commands/test_tui.py +++ b/tests/commands/test_tui.py @@ -20,6 +20,7 @@ from click.testing import CliRunner from keel.cli import cli +from keel.commands.admission import DiscoverReport from keel.commands.insights import ( AccountSummary as InsightsAccountSummary, ) @@ -43,6 +44,7 @@ _SHORT_VERSION, AvailableBalance, ScreenLine, + _admission_line_style, _available_lines, _confirm_arm_autonomy, _footer_lines, @@ -52,18 +54,23 @@ _message_style, _paint, _refresh_balance, + _scroll_offset, _short_version, _stdio_is_interactive, _style_attrs, _visible_slice, + build_admission_screen_overlay, + build_discover_overlay, build_help_screen, build_insights_screen, + build_propose_overlay, build_screen, render_plain, run_live, run_once, toggle_autonomy, ) +from keel.compliance import screen as screen_mod from keel.config import ( AutoTradeConfig, Caps, @@ -241,24 +248,40 @@ def test_build_screen_includes_subscriptions() -> None: def test_build_screen_footer_is_present_and_interval_independent() -> None: + """The footer is now TWO lines (`_footer_lines`); `build_screen` appends both as its last two + rows, and both must carry the original keybinding hints -- the second line does not replace + the first, it adds the admission keys the first line had no room for.""" report = _base_report() lines = build_screen(report, NOW_TS) - footer = lines[-1] - assert footer.style == "muted" - assert "quit" in footer.text.lower() - assert "help" in footer.text.lower() + footer = lines[-2:] + assert all(line.style == "muted" for line in footer) + joined = " ".join(line.text.lower() for line in footer) + assert "quit" in joined + assert "help" in joined def test_footer_lines_contains_keybinding_hints() -> None: + """The FIRST footer line is kept byte-for-byte as it was before the admission overlays + existed (see `_footer_lines`'s docstring) -- every hint that used to live in the single line + must still be found there, not merely somewhere across the two lines.""" lines = _footer_lines() - assert len(lines) == 1 - footer = lines[0] - assert footer.style == "muted" - text = footer.text.lower() + assert len(lines) == 2 + first = lines[0] + assert first.style == "muted" + text = first.text.lower() for hint in ("quit", "help", "refresh", "autonomy", "fetch", "insights"): assert hint in text +def test_footer_lines_second_line_documents_admission_keys() -> None: + lines = _footer_lines() + second = lines[1] + assert second.style == "muted" + text = second.text.lower() + for hint in ("screen", "propose", "discover"): + assert hint in text + + # -- available-to-buy balance (v3) --------------------------------------------------------------- @@ -584,6 +607,7 @@ def _fake_curses(*, has_colors: bool = True) -> SimpleNamespace: KEY_NPAGE=1004, KEY_HOME=1005, KEY_END=1006, + KEY_ENTER=1007, error=_FakeCursesError, has_colors=lambda: has_colors, start_color=lambda: calls.append("start_color"), @@ -880,6 +904,361 @@ def open_state() -> tuple[Repository, Any]: ) +# -- run_live: screen / propose overlays (offline, DB-only) --------------------------------------- + + +def test_run_live_s_opens_screen_overlay_and_esc_closes_it( + repo: Repository, monkeypatch: pytest.MonkeyPatch +) -> None: + """Mirrors `test_run_live_i_opens_insights_overlay_and_esc_closes_it`: 's' opens the screen + overlay, Esc closes it back to the dashboard.""" + config = _config() + keys = [ord("s"), -1, 27] + stdscr = _KeySequenceStdscr(height=24, width=80, keys=keys) + + fake_curses = _fake_curses() + fake_curses.wrapper = lambda fn: fn(stdscr) + monkeypatch.setitem(sys.modules, "curses", fake_curses) + + def open_state() -> tuple[Repository, Any]: + return repo, config + + run_live(open_state, lambda: NOW_TS, interval=0.01) + + painted_texts = [call[2] for call in stdscr.calls] + screen_idx = next(i for i, t in enumerate(painted_texts) if "keel tui -- screen" in t) + dashboard_after_idx = next( + i for i, t in enumerate(painted_texts) if i > screen_idx and "paper mode" in t + ) + assert dashboard_after_idx > screen_idx + + +def test_run_live_p_opens_propose_overlay_and_esc_closes_it( + repo: Repository, monkeypatch: pytest.MonkeyPatch, tmp_path +) -> None: + """Mirrors the 's' test above, for 'p'. `proposals_dir` points at a tmp_path subdirectory + (rather than the config default, `~/keel/proposals`) so this test never reads a real + deployment's proposals directory.""" + config = _config(proposals_dir=str(tmp_path / "proposals")) + keys = [ord("p"), -1, 27] + stdscr = _KeySequenceStdscr(height=24, width=80, keys=keys) + + fake_curses = _fake_curses() + fake_curses.wrapper = lambda fn: fn(stdscr) + monkeypatch.setitem(sys.modules, "curses", fake_curses) + + def open_state() -> tuple[Repository, Any]: + return repo, config + + run_live(open_state, lambda: NOW_TS, interval=0.01) + + painted_texts = [call[2] for call in stdscr.calls] + propose_idx = next(i for i, t in enumerate(painted_texts) if "keel tui -- propose" in t) + dashboard_after_idx = next( + i for i, t in enumerate(painted_texts) if i > propose_idx and "paper mode" in t + ) + assert dashboard_after_idx > propose_idx + + +def test_run_live_screen_survives_transient_read_error_and_keeps_polling( + repo: Repository, monkeypatch: pytest.MonkeyPatch +) -> None: + """The screen branch has its OWN try/except, mirroring insights' -- a transient failure (e.g. + `database is locked`) must paint a `screen read failed` alert line, not crash or hang, and the + loop must still be able to close the overlay and keep running afterwards.""" + config = _config() + # poll1: normal -> open_state call #1 (status) + #2 (balance refresh) both succeed; 's' opens + # screen. poll2: screen -> open_state call #3 (inside `_do_screen_report`) raises; Esc closes + # back to normal. poll3: normal -> open_state call #4 succeeds; 'q' quits (default). + keys = [ord("s"), 27] + stdscr = _KeySequenceStdscr(height=24, width=80, keys=keys) + + fake_curses = _fake_curses() + fake_curses.wrapper = lambda fn: fn(stdscr) + monkeypatch.setitem(sys.modules, "curses", fake_curses) + + opens: list[int] = [] + + def open_state() -> tuple[Repository, Any]: + opens.append(1) + if len(opens) == 3: + raise RuntimeError("database is locked") + return repo, config + + run_live(open_state, lambda: NOW_TS, interval=0.01) + + assert len(opens) >= 4 + painted_texts = [call[2] for call in stdscr.calls] + failed_idx = next(i for i, t in enumerate(painted_texts) if "screen read failed" in t) + assert "database is locked" in painted_texts[failed_idx] + assert any(i > failed_idx and "paper mode" in t for i, t in enumerate(painted_texts)) + + +def test_run_live_propose_survives_transient_read_error_and_keeps_polling( + repo: Repository, monkeypatch: pytest.MonkeyPatch, tmp_path +) -> None: + """Same shape as the screen version above, for 'p'.""" + config = _config(proposals_dir=str(tmp_path / "proposals")) + keys = [ord("p"), 27] + stdscr = _KeySequenceStdscr(height=24, width=80, keys=keys) + + fake_curses = _fake_curses() + fake_curses.wrapper = lambda fn: fn(stdscr) + monkeypatch.setitem(sys.modules, "curses", fake_curses) + + opens: list[int] = [] + + def open_state() -> tuple[Repository, Any]: + opens.append(1) + if len(opens) == 3: + raise RuntimeError("database is locked") + return repo, config + + run_live(open_state, lambda: NOW_TS, interval=0.01) + + assert len(opens) >= 4 + painted_texts = [call[2] for call in stdscr.calls] + failed_idx = next(i for i, t in enumerate(painted_texts) if "propose read failed" in t) + assert "database is locked" in painted_texts[failed_idx] + assert any(i > failed_idx and "paper mode" in t for i, t in enumerate(painted_texts)) + + +def test_run_live_screen_and_propose_never_construct_a_broker( + repo: Repository, monkeypatch: pytest.MonkeyPatch, tmp_path +) -> None: + """`screen`/`propose` are fully OFFLINE -- opening either, polling, and closing must never + reach `_build_broker` of their own accord. `_build_broker` IS still called once during this + run -- by `run_live`'s pre-existing, unrelated automatic "available to buy" balance refresh, + which (with a constant `now_fn`) fires exactly once, on the very first poll, and never again. + `len(calls) == 1` here is exactly that one call, proving screen/propose contributed zero + calls of their own -- the DIRECT proof (screen/propose never import `_build_broker` at all) + lives in `_do_screen_report`'s/`_do_propose_view`'s own source; this is the behavioural + cross-check.""" + config = _config(proposals_dir=str(tmp_path / "proposals")) + calls: list[Any] = [] + + class _FakeBroker: + def get_accounts(self) -> list[Any]: + return [] + + def _fake_build_broker(cfg: Any, timeout: int | None = None) -> _FakeBroker: + calls.append(cfg) + return _FakeBroker() + + monkeypatch.setattr("keel.commands._common._build_broker", _fake_build_broker) + + # poll1: normal -> 's'. poll2: screen, no key. poll3: Esc closes. poll4: normal -> 'p'. + # poll5: propose, no key. poll6: Esc closes. poll7: normal -> 'q' (post-exhaustion default). + keys = [ord("s"), -1, 27, ord("p"), -1, 27] + stdscr = _KeySequenceStdscr(height=24, width=80, keys=keys) + + fake_curses = _fake_curses() + fake_curses.wrapper = lambda fn: fn(stdscr) + monkeypatch.setitem(sys.modules, "curses", fake_curses) + + def open_state() -> tuple[Repository, Any]: + return repo, config + + run_live(open_state, lambda: NOW_TS, interval=0.01) + + assert len(calls) == 1 + + +# -- run_live: discover overlay (the network-gated one) -------------------------------------------- + + +def test_run_live_discover_opens_armed_and_never_touches_the_network_until_enter( + repo: Repository, monkeypatch: pytest.MonkeyPatch +) -> None: + """THE most important test in this batch. Pressing 'd', polling several times, then closing + must never call `list_products` -- the ONE network call this whole overlay can ever make is + gated behind an explicit Enter keypress, not behind opening the overlay or an ordinary poll. + (`_build_broker` itself is still called once by the pre-existing automatic balance refresh, + unrelated to discover -- see `test_run_live_screen_and_propose_never_construct_a_broker`'s + docstring for why that call doesn't confuse this assertion; `list_products` is the call that + is unique to, and gated by, discover, and it is the one this test pins to zero.)""" + config = _config() + list_products_calls: list[int] = [] + + class _FakeBroker: + def get_accounts(self) -> list[Any]: + return [] + + def list_products(self) -> list[dict]: + list_products_calls.append(1) + return [] + + def _fake_build_broker(cfg: Any, timeout: int | None = None) -> _FakeBroker: + return _FakeBroker() + + monkeypatch.setattr("keel.commands._common._build_broker", _fake_build_broker) + + # poll1: normal -> 'd' opens discover, ARMED. poll2, poll3: no key -- repaint the armed state, + # no fetch. poll4: Esc closes. poll5: normal -> 'q' quits (post-exhaustion default). + keys = [ord("d"), -1, -1, 27] + stdscr = _KeySequenceStdscr(height=24, width=80, keys=keys) + + fake_curses = _fake_curses() + fake_curses.wrapper = lambda fn: fn(stdscr) + monkeypatch.setitem(sys.modules, "curses", fake_curses) + + def open_state() -> tuple[Repository, Any]: + return repo, config + + run_live(open_state, lambda: NOW_TS, interval=0.01) + + assert list_products_calls == [] + painted_texts = [call[2] for call in stdscr.calls] + assert any("ARMED" in t for t in painted_texts) + assert any("paper mode" in t for t in painted_texts) # closed back to the dashboard + + +def test_run_live_discover_enter_calls_list_products_once_then_holds_the_result( + repo: Repository, monkeypatch: pytest.MonkeyPatch +) -> None: + """The counterpart to the gating test above: Enter DOES run the one network call, exactly + once -- and further polls while the overlay stays open repaint the HELD result rather than + re-fetching (no further `list_products` calls without another Enter).""" + config = _config() + list_products_calls: list[int] = [] + + class _FakeBroker: + def get_accounts(self) -> list[Any]: + return [] + + def list_products(self) -> list[dict]: + list_products_calls.append(1) + return [ + { + "product_id": "SOL-USD", + "quote_currency_id": "USD", + "status": "online", + "trading_disabled": False, + "is_disabled": False, + "view_only": False, + "quote_24h_volume": "9000000", + "base_name": "Solana", + } + ] + + def _fake_build_broker(cfg: Any, timeout: int | None = None) -> _FakeBroker: + return _FakeBroker() + + monkeypatch.setattr("keel.commands._common._build_broker", _fake_build_broker) + + # poll1: normal -> 'd'. poll2: discover ARMED -> Enter runs the one fetch. poll3, poll4: no + # key -- repaint the held result, no further call. poll5: Esc closes. + keys = [ord("d"), 10, -1, -1, 27] + stdscr = _KeySequenceStdscr(height=24, width=80, keys=keys) + + fake_curses = _fake_curses() + fake_curses.wrapper = lambda fn: fn(stdscr) + monkeypatch.setitem(sys.modules, "curses", fake_curses) + + def open_state() -> tuple[Repository, Any]: + return repo, config + + run_live(open_state, lambda: NOW_TS, interval=0.01) + + assert len(list_products_calls) == 1 + painted_texts = [call[2] for call in stdscr.calls] + assert any("SOL-USD" in t for t in painted_texts) + + +def test_run_live_discover_enter_raising_paints_readable_failure_and_keeps_polling( + repo: Repository, monkeypatch: pytest.MonkeyPatch +) -> None: + """A broker/network/auth failure on Enter must paint a readable `discover failed` line, not + crash the loop -- and Esc still closes the overlay afterwards, repainting the dashboard.""" + config = _config() + + class _FakeBroker: + def get_accounts(self) -> list[Any]: + return [] + + def list_products(self) -> list[dict]: + raise RuntimeError("venue unreachable") + + def _fake_build_broker(cfg: Any, timeout: int | None = None) -> _FakeBroker: + return _FakeBroker() + + monkeypatch.setattr("keel.commands._common._build_broker", _fake_build_broker) + + keys = [ord("d"), 10, 27] + stdscr = _KeySequenceStdscr(height=24, width=80, keys=keys) + + fake_curses = _fake_curses() + fake_curses.wrapper = lambda fn: fn(stdscr) + monkeypatch.setitem(sys.modules, "curses", fake_curses) + + def open_state() -> tuple[Repository, Any]: + return repo, config + + run_live(open_state, lambda: NOW_TS, interval=0.01) + + painted_texts = [call[2] for call in stdscr.calls] + failed_idx = next(i for i, t in enumerate(painted_texts) if "discover failed" in t) + assert "venue unreachable" in painted_texts[failed_idx] + assert any(i > failed_idx and "paper mode" in t for i, t in enumerate(painted_texts)) + + +def test_run_live_discover_closing_discards_the_held_result( + repo: Repository, monkeypatch: pytest.MonkeyPatch +) -> None: + """Closing the overlay (Esc) must discard the held result -- reopening it must be armed but + not yet run again, not silently show the previous run's stale candidates.""" + config = _config() + + class _FakeBroker: + def get_accounts(self) -> list[Any]: + return [] + + def list_products(self) -> list[dict]: + return [ + { + "product_id": "SOL-USD", + "quote_currency_id": "USD", + "status": "online", + "trading_disabled": False, + "is_disabled": False, + "view_only": False, + "quote_24h_volume": "9000000", + "base_name": "Solana", + } + ] + + def _fake_build_broker(cfg: Any, timeout: int | None = None) -> _FakeBroker: + return _FakeBroker() + + monkeypatch.setattr("keel.commands._common._build_broker", _fake_build_broker) + + # poll1: normal -> 'd'. poll2: discover ARMED -> Enter fetches SOL-USD. poll3: Esc closes + # (discards). poll4: normal -> 'd' reopens. poll5: discover -- must be ARMED again, no + # candidates carried over. poll6: Esc closes. + keys = [ord("d"), 10, 27, ord("d"), -1, 27] + stdscr = _KeySequenceStdscr(height=24, width=80, keys=keys) + + fake_curses = _fake_curses() + fake_curses.wrapper = lambda fn: fn(stdscr) + monkeypatch.setitem(sys.modules, "curses", fake_curses) + + def open_state() -> tuple[Repository, Any]: + return repo, config + + run_live(open_state, lambda: NOW_TS, interval=0.01) + + painted_texts = [call[2] for call in stdscr.calls] + sol_idx = next(i for i, t in enumerate(painted_texts) if "SOL-USD" in t) + # Every frame painted AFTER the SOL-USD result must be the armed re-explanation, not a + # repaint of the stale candidate list. + reopened_armed_idx = next( + i for i, t in enumerate(painted_texts) if i > sol_idx and "ARMED" in t + ) + assert not any( + "SOL-USD" in t for t in painted_texts[reopened_armed_idx:] + ) + + def test_run_live_read_error_does_not_swallow_keyboard_interrupt( repo: Repository, monkeypatch: pytest.MonkeyPatch ) -> None: @@ -905,9 +1284,23 @@ def test_build_help_screen_documents_every_key_and_safety_notes() -> None: text = " ".join(line.text.lower() for line in lines) for word in ("autonomy", "fetch", "quit", "scroll", "refresh", "help"): assert word in text + # v3: the admission workflow's three overlays and the CLI-only attest step. + for word in ("screen", "propose", "discover", "attest"): + assert word in text assert lines[0].style == "heading" +def test_build_help_screen_documents_discover_network_gating_and_attest_is_cli_only() -> None: + """The safety notes must be explicit about the two things that make `discover` different + from `screen`/`propose`, and that `attest` -- the one step that actually changes the + allowlist -- is never reachable from here.""" + lines = build_help_screen() + text = " ".join(line.text.lower() for line in lines) + assert "second deliberate network exception" in text + assert "cli-only" in text + assert "keel assets attest" in text + + def test_build_help_screen_is_longer_than_a_small_terminal() -> None: lines = build_help_screen() assert len(lines) > 24 @@ -1126,6 +1519,220 @@ def test_build_insights_screen_is_read_only_pure() -> None: assert [line.text for line in first] == [line.text for line in second] +# -- _scroll_offset (pure, shared by help/insights/screen/propose/discover) ---------------------- + + +def test_scroll_offset_up_and_down_move_by_one() -> None: + fake_curses = _fake_curses() + assert _scroll_offset(fake_curses.KEY_UP, 5, height=10, total=50, curses_mod=fake_curses) == 4 + assert _scroll_offset(ord("k"), 5, height=10, total=50, curses_mod=fake_curses) == 4 + assert _scroll_offset(fake_curses.KEY_DOWN, 5, height=10, total=50, curses_mod=fake_curses) == 6 + assert _scroll_offset(ord("j"), 5, height=10, total=50, curses_mod=fake_curses) == 6 + + +def test_scroll_offset_page_up_and_down_move_by_almost_a_screen() -> None: + fake_curses = _fake_curses() + result = _scroll_offset(fake_curses.KEY_PPAGE, 20, height=10, total=50, curses_mod=fake_curses) + assert result == 11 + result = _scroll_offset(fake_curses.KEY_NPAGE, 20, height=10, total=50, curses_mod=fake_curses) + assert result == 29 + + +def test_scroll_offset_home_jumps_to_top() -> None: + fake_curses = _fake_curses() + result = _scroll_offset(fake_curses.KEY_HOME, 20, height=10, total=50, curses_mod=fake_curses) + assert result == 0 + + +@pytest.mark.parametrize( + ("height", "total", "expected"), + [ + (10, 50, 40), # the ordinary case: last full page + (10, 51, 41), # +1 line of content moves the floor by exactly 1 (catches an off-by-one) + (10, 10, 0), # content exactly fills the window -- nowhere to scroll + (10, 3, 0), # content SHORTER than the window -- End must not scroll past the top + (1, 50, 49), # a one-row terminal still lands on the true last line + ], +) +def test_scroll_offset_end_jumps_to_the_last_full_page( + height: int, total: int, expected: int +) -> None: + """`End` sets the offset to `total` and lets the shared clamp bring it back to the last full + page. + + Parametrized rather than asserted at a single point because one point does not pin the + RELATIONSHIP between window and content: the interesting cases are the boundaries, where + content exactly fills the window, is shorter than it (End must be a no-op, not a scroll into + blank space), or is one line longer than a page (the floor must move by exactly one). + + Worth knowing before "tightening" this: `offset = total` is not the only correct + implementation. The trailing clamp is `min(offset, max(0, total - height))`, so ANY value at + or above `total - height` is indistinguishable from any other -- `total - 1` included. That + is not an off-by-one waiting to be caught, it is the same function; a test asserting `total` + specifically would be pinning an implementation detail rather than the behaviour. What these + cases do catch is an End that lands BELOW the floor (e.g. `total // 2`, or a forgotten clamp + letting it run past the end).""" + fake_curses = _fake_curses() + result = _scroll_offset( + fake_curses.KEY_END, 0, height=height, total=total, curses_mod=fake_curses + ) + assert result == expected + + +def test_scroll_offset_clamps_negative_to_zero() -> None: + fake_curses = _fake_curses() + assert _scroll_offset(fake_curses.KEY_UP, 0, height=10, total=50, curses_mod=fake_curses) == 0 + + +def test_scroll_offset_clamps_past_the_last_full_page() -> None: + fake_curses = _fake_curses() + result = _scroll_offset(fake_curses.KEY_DOWN, 40, height=10, total=50, curses_mod=fake_curses) + assert result == 40 + + +def test_scroll_offset_unrecognized_key_is_a_noop() -> None: + """A no-key poll (`getch()` returns `-1` on timeout) or any other unmapped keycode must leave + the offset exactly where it was (still clamped) -- this is what makes it safe to route EVERY + keypress in a scrollable overlay through this function, not just the six scroll keys.""" + fake_curses = _fake_curses() + assert _scroll_offset(-1, 5, height=10, total=50, curses_mod=fake_curses) == 5 + + +# -- _admission_line_style (pure) ----------------------------------------------------------------- + + +@pytest.mark.parametrize( + "text,expected", + [ + ("ADMIT BTC bars=2000 median_daily_volume=2000000 on-allowlist attested", "ok"), + ("REJECT SOL bars=0 median_daily_volume=0 not-on-allowlist UNATTESTED", "warn"), + (" ✗ history: only 500 bars, need 1460", "warn"), + (" ! sector unknown -- treated as non-yielding until attested", "warn"), + ("INVALID missing asset: {'rationale': 'r'}", "warn"), + ("⚠️ These are PROPOSALS, not admissions. Nothing above has been screened.", "alert"), + ("some other plain line", "normal"), + ("shortlist: /home/user/keel/proposals/2026-08-01.json", "normal"), + ], +) +def test_admission_line_style_conventions(text: str, expected: str) -> None: + assert _admission_line_style(text) == expected + + +def test_admission_line_style_missing_history_line_is_muted_not_alert_or_warn() -> None: + """`! no local history` -- and its MISSING-DATA continuation line from `missing_history_ + lines` -- must NOT read as an alarm: `keel.compliance.screen.split_failures`'s whole reason + for existing is that "never fetched" is not a verdict about the asset (see + `render_screen_report`'s own docstring), so painting it `"warn"`/`"alert"`, the colours a + REAL rejection reason gets, would visually assert the opposite of what the text says.""" + missing = ( + " ! no local history for SOL-USD -- run `keel fetch --products SOL-USD` first, " + "then re-screen." + ) + style = _admission_line_style(missing) + assert style == "muted" + assert style not in ("alert", "warn") + + continuation = ( + " This is a MISSING-DATA verdict, not a verdict about the asset: it is not too " + "young, we have simply never fetched candles for it." + ) + style = _admission_line_style(continuation) + assert style == "muted" + assert style not in ("alert", "warn") + + +# -- build_admission_screen_overlay / build_propose_overlay / build_discover_overlay (pure) ------ + + +def _fake_screen_fn(*, admitted: bool = True): + """A minimal `ScreenFn` stub -- deliberately NOT `_screen_product` itself, since these tests + exercise `build_admission_screen_overlay`/`build_propose_overlay` (pure styling over an + already-built report), not the admission gate's own logic (covered by + `tests/commands/test_admission.py`).""" + + def _screen(repo: Repository, product: str, quote: str): + facts = screen_mod.MarketFacts( + asset=product.split("-")[0], + daily_bars=2000, + median_daily_volume=Decimal("2000000"), + quotable_in_settlement_currency=True, + product_id=product, + ) + result = screen_mod.ScreenResult(asset=facts.asset, admitted=admitted) + return facts, result + + return _screen + + +def test_build_admission_screen_overlay_is_nonempty_titled_and_headed(repo: Repository) -> None: + from keel.commands.admission import build_screen_report + + config = _config(allowlist=["BTC"]) + report = build_screen_report(repo, config, _fake_screen_fn()) + + lines = build_admission_screen_overlay(report) + + assert lines + assert lines[0].style == "heading" + assert lines[0].text == "keel tui -- screen" + + +def test_build_propose_overlay_is_nonempty_titled_and_headed(repo: Repository, tmp_path) -> None: + from keel.commands.admission import build_propose_view + + config = _config(proposals_dir=str(tmp_path / "proposals")) + view = build_propose_view(repo, config, _fake_screen_fn(), directory=tmp_path / "proposals") + + lines = build_propose_overlay(view) + + assert lines + assert lines[0].style == "heading" + assert lines[0].text == "keel tui -- propose" + + +def test_build_discover_overlay_with_report_is_nonempty_titled_and_headed() -> None: + 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=900, + candidates=[candidate], + min_quote_24h_volume=Decimal("5000000"), + ) + + lines = build_discover_overlay(report) + + assert lines + assert lines[0].style == "heading" + assert lines[0].text == "keel tui -- discover" + assert any("SOL-USD" in line.text for line in lines) + + +def test_build_discover_overlay_none_renders_armed_explanation_and_the_run_key() -> None: + """The state `build_discover_overlay(None)` renders is the proof that opening the discover + overlay makes NO network call -- it must say so plainly, name what Enter will do, and name + Enter itself, not just render a blank or "loading" screen.""" + lines = build_discover_overlay(None) + + assert lines[0].style == "heading" + assert lines[0].text == "keel tui -- discover" + text = " ".join(line.text for line in lines) + assert "ARMED" in text + assert "no network call" in text.lower() + assert "Enter" in text + + +def test_build_discover_overlay_with_error_renders_readable_failure_not_a_traceback() -> None: + lines = build_discover_overlay(None, error="could not reach coinbase.com") + + text = " ".join(line.text for line in lines) + assert "discover failed" in text.lower() + assert "could not reach coinbase.com" in text + failure_line = next(line for line in lines if "discover failed" in line.text.lower()) + assert failure_line.style == "alert" + + # -- toggle_autonomy / _guarded (injectable actions, no curses/network) --------------------------- diff --git a/tests/compliance/test_assets_cli.py b/tests/compliance/test_assets_cli.py index 2a8fbe44..99da78e1 100644 --- a/tests/compliance/test_assets_cli.py +++ b/tests/compliance/test_assets_cli.py @@ -666,7 +666,13 @@ def test_derivative_failures_are_not_asserted_as_verdicts_without_history( ): """With zero cached bars, liquidity and settlement report on our DATA, not the asset: median volume is 0 because there are no bars. Printing them as findings would assert - exactly what the missing-data message exists to deny.""" + exactly what the missing-data message exists to deny. + + `history` used to be exempted from this and printed as `0 daily bars, need 1460` -- + itself the same lie in different clothes: zero bars means we never fetched the asset, not + that it is too young. See `test_zero_cached_bars_never_prints_a_history_depth_failure` + below for that invariant asserted head-on; this test was updated (not just extended) to + stop asserting the old, wrong behaviour.""" db_path = tmp_path / "t.db" _repo_at(db_path) _with_broker(monkeypatch, _FakeBroker([_account("SOL", "12")])) @@ -676,8 +682,99 @@ def test_derivative_failures_are_not_asserted_as_verdicts_without_history( assert "no local history" in result.output assert "✗ settlement" not in result.output, "settlement is a naming artifact here" assert "✗ liquidity" not in result.output, "median volume is 0 only because bars are 0" - assert "not assessable without history" in result.output - assert "✗ history" in result.output # the REAL, primary finding stays + assert "✗ history" not in result.output, "zero bars is a cache gap, not a history verdict" + assert "not assessable until then" in result.output + + +def test_zero_cached_bars_never_prints_a_history_depth_failure( + tmp_path, valid_config_path, monkeypatch +): + """The headline invariant for `keel assets holdings --screen`: at ZERO cached bars, the + output must never contain a `✗ history: 0 daily bars, need 1460` line. That line reads as + "this asset is too young" when the truth is "we have never fetched it" -- a candidate that + was never fetched must be indistinguishable, in the failure list, from one this deployment + has simply not pulled data for yet, and distinguishable from one that is genuinely too + young (see the 400-bar counterpart test below).""" + db_path = tmp_path / "t.db" + _repo_at(db_path) # no candles seeded at all + _with_broker(monkeypatch, _FakeBroker([_account("SOL", "12")])) + + result = _holdings(db_path, valid_config_path, "--screen") + + assert "✗ history" not in result.output + assert "no local history" in result.output + assert "keel fetch" in result.output + assert "not assessable until then" in result.output + + +def test_a_genuinely_young_asset_still_reports_history_as_a_real_verdict_via_holdings( + tmp_path, valid_config_path, monkeypatch +): + """The counterpart to the zero-bars invariant above: the suppression must be scoped to + EXACTLY zero bars. An asset with SOME cached history that is still short of the 1460-bar + floor is genuinely too young, and `holdings --screen` must keep saying so -- proving the fix + closes one specific lie (zero bars misread as "too young") rather than becoming a blanket + silencer for every `history` verdict.""" + db_path = tmp_path / "t.db" + repo = _repo_at(db_path) + _seed_history(repo, "PAXG-USD", bars=400) # real bars, genuinely short of the floor + runner = CliRunner() + assert _attest( + runner, db_path, valid_config_path, "PAXG", **{"--backing": "ayn"} + ).exit_code == 0 + _with_broker(monkeypatch, _FakeBroker([_account("PAXG", "3")])) + + result = _holdings(db_path, valid_config_path, "--screen") + + assert "✗ history" in result.output + assert "no local history" not in result.output + + +def test_zero_cached_bars_never_prints_a_history_depth_failure_via_assets_screen( + tmp_path, valid_config_path +): + """The same invariant as the `holdings --screen` and `propose` versions, for `assets screen`. + + This command is the SIBLING of the TUI's `s` screen overlay -- both screen the same + `_default_sim_products(config)` set through the same `_screen_product` gate -- so if only one + of them explains a zero-bar cache, an operator gets two different stories about the same + allowlist depending on which surface they happened to look at. That is precisely the drift + `_screen_product`'s docstring exists to prevent, applied to the REPORTING of a verdict rather + than to the verdict itself. + """ + db_path = tmp_path / "t.db" + _repo_at(db_path) # no candles seeded at all + + result = CliRunner().invoke( + cli, + ["--db", str(db_path), "--config", str(valid_config_path), + "assets", "screen", "--products", "SOL-USD"], + ) + + assert result.exit_code == 0, result.output + assert "✗ history" not in result.output + assert "no local history" in result.output + assert "keel fetch" in result.output + assert "not assessable until then" in result.output + + +def test_assets_screen_still_reports_a_genuinely_short_history_as_a_real_verdict( + tmp_path, valid_config_path +): + """The counterpart: the suppression is scoped to EXACTLY zero bars here too, so an asset that + really is too young still gets told so by `assets screen`.""" + db_path = tmp_path / "t.db" + repo = _repo_at(db_path) + _seed_history(repo, "PAXG-USD", bars=400) + + result = CliRunner().invoke( + cli, + ["--db", str(db_path), "--config", str(valid_config_path), + "assets", "screen", "--products", "PAXG-USD"], + ) + + assert "✗ history" in result.output + assert "no local history" not in result.output def test_a_lowercase_settlement_currency_is_still_excluded( @@ -711,10 +808,14 @@ def test_min_balance_rejects_garbage_and_non_finite_values( def test_the_derived_failure_tags_actually_match_screen_asset_output(): """Pins the string coupling that the zero-bars suppression depends on. - `assets holdings` suppresses derivative failures by matching `failure.split(":")[0]` against - `_DATA_DERIVED_FAILURES`. Renaming a tag in `screen_asset` would silently stop the + `screen.split_failures` suppresses cache-derived failures by matching `failure.split(":")[0]` + against `DATA_DERIVED_FAILURES`. Renaming a tag in `screen_asset` would silently stop the suppression -- reintroducing 'data artifacts printed as verdicts about the asset' with a fully green suite. This asserts the tags are real. + + Reads the constant from `screen.py` rather than through `keel.cli`: the split moved into + `screen.py` beside the tag set, so `cli.py` no longer names the constant at all, and an + alias kept alive purely to be asserted on here would prove nothing about live code. """ from keel.compliance import screen as screen_mod @@ -727,10 +828,10 @@ def test_the_derived_failure_tags_actually_match_screen_asset_output(): ) tags = {f.split(":")[0] for f in screen_mod.screen_asset(facts, None).failures} - missing = cli_module._DATA_DERIVED_FAILURES - tags + missing = screen_mod.DATA_DERIVED_FAILURES - tags assert not missing, ( - f"{missing} no longer appear as failure tags in screen_asset -- the holdings " - "suppression is now silently inert; update _DATA_DERIVED_FAILURES" + f"{missing} no longer appear as failure tags in screen_asset -- the zero-bars " + "suppression is now silently inert; update DATA_DERIVED_FAILURES" ) @@ -926,6 +1027,30 @@ def test_propose_rejects_an_unattested_candidate(tmp_path, valid_config_path): assert "0/1 admitted" in result.output +def test_zero_cached_bars_never_prints_a_history_depth_failure_via_propose( + tmp_path, valid_config_path +): + """Same invariant as the `holdings --screen` version above, for `keel assets propose`: a + candidate with ZERO cached bars must not print a `✗ history: 0 daily bars, need 1460` line. + That line is exactly the lie the MISSING-DATA explanation two lines above it exists to + deny -- "too young" and "never fetched" must not be indistinguishable in the failure list.""" + db_path = tmp_path / "t.db" + _repo_at(db_path) # no candles seeded -- SOL has zero cached bars + shortlist = _write_shortlist(tmp_path, [_SOL]) + + result = CliRunner().invoke( + cli, + ["--db", str(db_path), "--config", str(valid_config_path), + "assets", "propose", "--from", str(shortlist)], + ) + + assert result.exit_code == 0 + assert "✗ history" not in result.output + assert "no local history" in result.output + assert "keel fetch" in result.output + assert "not assessable until then" in result.output + + def test_propose_and_screen_agree_for_the_same_asset(tmp_path, valid_config_path): """One gate, shared by construction -- the proposer must not get a laxer path.""" db_path = tmp_path / "t.db" diff --git a/tests/compliance/test_screen.py b/tests/compliance/test_screen.py index f4acfa62..fe65d127 100644 --- a/tests/compliance/test_screen.py +++ b/tests/compliance/test_screen.py @@ -10,7 +10,9 @@ AssetAttestation, MarketFacts, ScreenPolicy, + missing_history_lines, screen_asset, + split_failures, ) @@ -410,3 +412,88 @@ def test_discovery_matches_the_quote_currency_case_insensitively(): assert discover_candidates( [_product(pid="SOL-USD", quote="USD")], DiscoveryPolicy(quote_currency="usd") ), "lowercase configured quote currency dropped everything" + + +# -- split_failures / missing_history_lines ------------------------------------------------- +# +# The single source of truth for "is a zero-bar `history` failure a lie about the asset, or a +# fact about our cache" now lives here, not duplicated per-caller (`assets holdings`, `assets +# propose`, and the TUI to come). These tests pin the split and the wording directly, so a +# regression shows up here rather than as a re-appeared `✗ history: 0 daily bars` line three +# call sites away. + + +def test_split_failures_leaves_a_nonzero_history_asset_entirely_unsplit(): + """With ANY cached bars, every failure -- including a genuine `history` shortfall for a + young asset -- is a real verdict about the asset. Nothing is downstream of the cache.""" + facts = _facts(bars=400) + result = screen_asset(facts, _attestation()) + about_asset, about_cache = split_failures(facts, result) + assert about_asset == result.failures + assert about_cache == [] + assert any(f.startswith("history") for f in about_asset) + + +def test_split_failures_splits_only_at_exactly_zero_bars(): + """At zero bars, `history` and `liquidity` are downstream of having no cache -- they measure + OUR data, not the asset -- so both move to `about_cache`. `settlement` and `attestation` + keep being real verdicts about the asset regardless of bar count.""" + facts = _facts(bars=0, volume="0", quotable=False) + result = screen_asset(facts, None) # unattested too, so `attestation` also fails + about_asset, about_cache = split_failures(facts, result) + + cache_tags = {f.split(":")[0] for f in about_cache} + asset_tags = {f.split(":")[0] for f in about_asset} + assert cache_tags == {"history", "liquidity"} + assert "settlement" in asset_tags + assert "attestation" in asset_tags + + +def test_split_failures_preserves_original_ordering_in_both_lists(): + """Callers render these lists in order; a silent reorder would scramble the report even + though the same failures are all still present somewhere in it.""" + facts = _facts(bars=0, volume="0", quotable=False) + result = screen_asset(facts, None) + about_asset, about_cache = split_failures(facts, result) + + def _positions(subset: list[str]) -> list[int]: + return [result.failures.index(f) for f in subset] + + assert _positions(about_asset) == sorted(_positions(about_asset)) + assert _positions(about_cache) == sorted(_positions(about_cache)) + + +def test_split_failures_keeps_settlement_assessable_at_zero_bars(): + """`settlement` reads the product id, never a candle, so it must stay a real verdict even + when there is no cached history at all -- the one criterion this split must NOT catch.""" + facts = _facts(bars=0, volume="0", quotable=False) + result = screen_asset(facts, _attestation()) + about_asset, about_cache = split_failures(facts, result) + assert any(f.startswith("settlement") for f in about_asset) + assert not any(f.startswith("settlement") for f in about_cache) + + +def test_missing_history_lines_omits_the_third_line_when_nothing_is_suppressed(): + """A candidate that fails on shape/settlement/attestation alone (no derived failures at all) + gets the two-line MISSING-DATA explanation and nothing more -- a trailing "not assessable + until then:" with nothing after the colon would be worse than no line at all.""" + lines = missing_history_lines("SOL-USD", []) + assert len(lines) == 2 + assert "no local history" in lines[0] + assert "keel fetch --products SOL-USD" in lines[0] + assert "MISSING-DATA verdict" in lines[1] + assert not any("not assessable" in line for line in lines) + + +def test_missing_history_lines_dedupes_and_sorts_tags(): + """Two failures sharing a tag collapse to one mention, and the tags print in a stable, + predictable order rather than whatever order `screen_asset` happened to emit them in.""" + lines = missing_history_lines( + "SOL-USD", + [ + "liquidity: median daily volume 0 < 1000000 required", + "history: 0 daily bars < 1460 required", + "liquidity: a second liquidity-tagged failure, hypothetically", + ], + ) + assert lines[2] == "not assessable until then: history, liquidity" diff --git a/tests/fixtures/config_golden_defaults.json b/tests/fixtures/config_golden_defaults.json index 41c3c722..bb77964b 100644 --- a/tests/fixtures/config_golden_defaults.json +++ b/tests/fixtures/config_golden_defaults.json @@ -49,6 +49,7 @@ "min_trades": 100, "min_win_rate": "0.55" }, + "proposals_dir": "~/keel/proposals", "quote_currency": "USD", "research": { "pbo_max": "0.05", diff --git a/tests/fixtures/config_golden_full.json b/tests/fixtures/config_golden_full.json index 1211700f..63f5d89e 100644 --- a/tests/fixtures/config_golden_full.json +++ b/tests/fixtures/config_golden_full.json @@ -54,6 +54,7 @@ "min_trades": 42, "min_win_rate": "0.45" }, + "proposals_dir": "/var/keel/proposals-full-fixture", "quote_currency": "USDC", "research": { "pbo_max": "0.1", diff --git a/tests/fixtures/config_golden_full.yaml b/tests/fixtures/config_golden_full.yaml index cbeccedb..1530064e 100644 --- a/tests/fixtures/config_golden_full.yaml +++ b/tests/fixtures/config_golden_full.yaml @@ -83,6 +83,9 @@ fees: # Non-default on purpose (the default is USD) -- see the `tiers` note above. quote_currency: USDC +# Non-default on purpose (the default is ~/keel/proposals) -- see the `tiers` note above. +proposals_dir: /var/keel/proposals-full-fixture + # Rail 18's allowed settlement legs. Non-default on purpose (the default is [USD, USDC]) and # lowercase on purpose, so the golden pins the case-folding too. settlement_currencies: diff --git a/tests/test_config.py b/tests/test_config.py index f612c1c0..c4af7c95 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -240,6 +240,35 @@ def test_load_config_quote_currency_empty_raises_configerror(write_config): load_config(path) +# -- proposals_dir (where `assets propose`/the TUI's propose overlay look for a shortlist) ----- + + +def test_load_config_proposals_dir_defaults_to_keel_proposals(valid_config_path): + """`VALID_CONFIG_YAML` has no `proposals_dir:` -- absent, it falls back to `~/keel/proposals`, + unexpanded (expansion is the READER's job, at use, not the parser's -- see the field's own + comment in `keel_core/config.py`).""" + config = load_config(valid_config_path) + + assert config.proposals_dir == "~/keel/proposals" + + +def test_load_config_proposals_dir_overridable(write_config): + text = VALID_CONFIG_YAML + "\nproposals_dir: /var/keel/shortlists\n" + path = write_config(text) + + config = load_config(path) + + assert config.proposals_dir == "/var/keel/shortlists" + + +def test_load_config_proposals_dir_empty_raises_configerror(write_config): + text = VALID_CONFIG_YAML + "\nproposals_dir: ''\n" + path = write_config(text) + + with pytest.raises(ConfigError, match="proposals_dir"): + load_config(path) + + # -- settlement_currencies (rail 18's allowed set) ------------------------------------------ diff --git a/tests/test_proposer.py b/tests/test_proposer.py index 73d87ee4..f4b89c9c 100644 --- a/tests/test_proposer.py +++ b/tests/test_proposer.py @@ -227,8 +227,11 @@ def test_render_no_history_shows_missing_data_next_step(): assert "no local history" in text assert "keel fetch --products SOL-USD" in text assert "MISSING-DATA verdict" in text - # the liquidity failure is suppressed as not-assessable-without-history - assert "not assessable without history" in text + # the liquidity failure is suppressed as not-assessable-until-then, and no longer printed + # per-tag as `· (liquidity: not assessable without history)` -- it is now one summary line, + # shared by construction with `keel/cli.py` via `screen.missing_history_lines`. + assert "not assessable until then: liquidity" in text + assert "✗ liquidity" not in text def test_render_unattested_reject_shows_attest_next_step():