From 106530c06300e52c21be524c21d4c27a8a13be75 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Sat, 25 Jul 2026 00:04:54 -0400 Subject: [PATCH 1/5] feat(proposer): proposal schema parsing with per-candidate validation Co-Authored-By: Claude Opus 4.8 --- keel/proposer.py | 102 +++++++++++++++++++++++++++++++++++++++++ tests/test_proposer.py | 64 ++++++++++++++++++++++++++ 2 files changed, 166 insertions(+) create mode 100644 keel/proposer.py create mode 100644 tests/test_proposer.py diff --git a/keel/proposer.py b/keel/proposer.py new file mode 100644 index 00000000..06002b56 --- /dev/null +++ b/keel/proposer.py @@ -0,0 +1,102 @@ +"""LLM asset proposer -- ingest an externally-produced shortlist and route each candidate +through the EXISTING admission gate. Admits nothing. See +docs/superpowers/specs/2026-07-24-llm-asset-proposer-design.md. + +Pure and dependency-free: no LLM, no network, no DB writes. The gate is injected as `screen_fn` +so this module never imports `keel.cli` (which would cycle) and stays unit-testable. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import Any +from urllib.parse import urlparse + + +class ProposalError(ValueError): + """Malformed proposal at the top level (not a per-candidate issue).""" + + +@dataclass(frozen=True) +class Candidate: + asset: str + rationale: str + sources: list[str] + shariah_hypothesis: str | None = None + + +@dataclass(frozen=True) +class InvalidEntry: + raw: dict[str, Any] + reason: str + + +@dataclass(frozen=True) +class ParsedProposal: + candidates: list[Candidate] + invalid: list[InvalidEntry] + + +def _is_http_url(value: Any) -> bool: + if not isinstance(value, str) or not value.strip(): + return False + try: + parsed = urlparse(value) + except (ValueError, TypeError): + return False + return parsed.scheme in ("http", "https") and bool(parsed.netloc) + + +def _candidate_error(entry: Any) -> str | None: + """Return None if the entry is a valid candidate, else a human reason string.""" + if not isinstance(entry, dict): + return "entry is not an object" + asset = entry.get("asset") + if not isinstance(asset, str) or not asset.strip(): + return "missing or empty 'asset'" + rationale = entry.get("rationale") + if not isinstance(rationale, str) or not rationale.strip(): + return "missing or empty 'rationale'" + sources = entry.get("sources") + if not isinstance(sources, list) or not sources: + return "missing or empty 'sources' (>= 1 citation required)" + if not all(_is_http_url(s) for s in sources): + return "every source must be a non-empty http(s) URL" + hypothesis = entry.get("shariah_hypothesis") + if hypothesis is not None and not isinstance(hypothesis, str): + return "'shariah_hypothesis' must be a string when present" + return None + + +def parse_proposal(raw: Any) -> ParsedProposal: + """Validate a decoded JSON proposal into valid Candidates + InvalidEntries. + + Raises ProposalError for a malformed top-level structure. A per-candidate problem (missing + citation, bad URL, empty field) does NOT raise -- the entry is collected into `invalid` and + excluded from screening, never silently dropped. + """ + if not isinstance(raw, dict) or not isinstance(raw.get("candidates"), list): + raise ProposalError("proposal must be an object with a 'candidates' list") + + candidates: list[Candidate] = [] + invalid: list[InvalidEntry] = [] + for entry in raw["candidates"]: + reason = _candidate_error(entry) + if reason is not None: + raw_entry = entry if isinstance(entry, dict) else {"value": entry} + invalid.append(InvalidEntry(raw=raw_entry, reason=reason)) + continue + hypothesis = entry.get("shariah_hypothesis") + candidates.append( + Candidate( + asset=entry["asset"].strip().upper(), + rationale=entry["rationale"].strip(), + sources=[s.strip() for s in entry["sources"]], + shariah_hypothesis=( + hypothesis.strip() + if isinstance(hypothesis, str) and hypothesis.strip() + else None + ), + ) + ) + return ParsedProposal(candidates=candidates, invalid=invalid) diff --git a/tests/test_proposer.py b/tests/test_proposer.py new file mode 100644 index 00000000..786d2917 --- /dev/null +++ b/tests/test_proposer.py @@ -0,0 +1,64 @@ +import pytest +from keel.proposer import ParsedProposal, ProposalError, parse_proposal + + +def _entry(**over): + e = { + "asset": "sol", + "rationale": "high liquidity and developer activity", + "sources": ["https://coinmarketcap.com/currencies/solana/"], + } + e.update(over) + return e + + +def test_valid_proposal_parses_and_normalizes_asset(): + parsed = parse_proposal({"candidates": [_entry()]}) + assert isinstance(parsed, ParsedProposal) + assert len(parsed.candidates) == 1 + c = parsed.candidates[0] + assert c.asset == "SOL" # upper-cased + assert c.sources == ["https://coinmarketcap.com/currencies/solana/"] + assert c.shariah_hypothesis is None + assert parsed.invalid == [] + + +def test_optional_shariah_hypothesis_is_captured(): + parsed = parse_proposal({"candidates": [_entry(shariah_hypothesis="utility L1")]}) + assert parsed.candidates[0].shariah_hypothesis == "utility L1" + + +def test_missing_sources_makes_entry_invalid_not_screened(): + parsed = parse_proposal({"candidates": [_entry(sources=[])]}) + assert parsed.candidates == [] + assert len(parsed.invalid) == 1 + assert "sources" in parsed.invalid[0].reason + + +def test_non_url_source_is_invalid(): + parsed = parse_proposal({"candidates": [_entry(sources=["not-a-url"])]}) + assert parsed.candidates == [] + assert "URL" in parsed.invalid[0].reason + + +def test_empty_rationale_is_invalid(): + parsed = parse_proposal({"candidates": [_entry(rationale=" ")]}) + assert "rationale" in parsed.invalid[0].reason + + +def test_missing_asset_is_invalid(): + parsed = parse_proposal({"candidates": [_entry(asset="")]}) + assert "asset" in parsed.invalid[0].reason + + +def test_malformed_top_level_raises(): + with pytest.raises(ProposalError): + parse_proposal({"not_candidates": []}) + with pytest.raises(ProposalError): + parse_proposal([]) # not a dict + + +def test_mixed_valid_and_invalid_are_partitioned(): + parsed = parse_proposal({"candidates": [_entry(asset="BTC"), _entry(sources=[])]}) + assert [c.asset for c in parsed.candidates] == ["BTC"] + assert len(parsed.invalid) == 1 From 31bdb0d7c8a72580d5de31457c5274ccf1e87bf7 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Sat, 25 Jul 2026 00:06:15 -0400 Subject: [PATCH 2/5] feat(proposer): screened-report builder over an injected gate Co-Authored-By: Claude Opus 4.8 --- keel/proposer.py | 59 +++++++++++++++++++++++++++ tests/test_proposer.py | 90 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 148 insertions(+), 1 deletion(-) diff --git a/keel/proposer.py b/keel/proposer.py index 06002b56..eb4b92b0 100644 --- a/keel/proposer.py +++ b/keel/proposer.py @@ -8,10 +8,19 @@ from __future__ import annotations +from collections.abc import Callable from dataclasses import dataclass from typing import Any from urllib.parse import urlparse +from keel.commands._products import _history_product +from keel.compliance import screen as screen_mod +from keel.data.repository import Repository + +ScreenFn = Callable[ + [Repository, str, str], tuple[screen_mod.MarketFacts, screen_mod.ScreenResult] +] + class ProposalError(ValueError): """Malformed proposal at the top level (not a per-candidate issue).""" @@ -100,3 +109,53 @@ def parse_proposal(raw: Any) -> ParsedProposal: ) ) return ParsedProposal(candidates=candidates, invalid=invalid) + + +@dataclass(frozen=True) +class ScreenedCandidate: + candidate: Candidate + product: str + on_allowlist: bool + attested: bool + facts: screen_mod.MarketFacts + result: screen_mod.ScreenResult + + +@dataclass(frozen=True) +class ProposalReport: + screened: list[ScreenedCandidate] + invalid: list[InvalidEntry] + + @property + def admitted_count(self) -> int: + return sum(1 for s in self.screened if s.result.admitted) + + +def build_proposal_report( + parsed: ParsedProposal, + repo: Repository, + quote: str, + allowlist: list[str], + screen_fn: ScreenFn, +) -> ProposalReport: + """Route each valid candidate through the injected admission gate. Writes nothing. + + `screen_fn` receives only (repo, product, quote) -- the LLM's rationale and shariah_hypothesis + are never passed to the gate, so they cannot influence admission (asymmetry, by construction). + """ + allow = {a.upper() for a in allowlist} + screened: list[ScreenedCandidate] = [] + for cand in parsed.candidates: + product = _history_product(cand.asset, quote) + facts, result = screen_fn(repo, product, quote) + screened.append( + ScreenedCandidate( + candidate=cand, + product=product, + on_allowlist=cand.asset in allow, + attested=repo.get_asset_attestation(cand.asset) is not None, + facts=facts, + result=result, + ) + ) + return ProposalReport(screened=screened, invalid=parsed.invalid) diff --git a/tests/test_proposer.py b/tests/test_proposer.py index 786d2917..a18eb2a4 100644 --- a/tests/test_proposer.py +++ b/tests/test_proposer.py @@ -1,5 +1,17 @@ +from decimal import Decimal + import pytest -from keel.proposer import ParsedProposal, ProposalError, parse_proposal + +from keel.compliance import screen as screen_mod +from keel.data.db import connect, migrate +from keel.data.repository import Repository +from keel.proposer import ( + ParsedProposal, + ProposalError, + ProposalReport, + build_proposal_report, + parse_proposal, +) def _entry(**over): @@ -62,3 +74,79 @@ def test_mixed_valid_and_invalid_are_partitioned(): parsed = parse_proposal({"candidates": [_entry(asset="BTC"), _entry(sources=[])]}) assert [c.asset for c in parsed.candidates] == ["BTC"] assert len(parsed.invalid) == 1 + + +def _repo(): + conn = connect(":memory:") + migrate(conn) + return Repository(conn) + + +def _fake_screen(admitted, bars=2000): + calls = [] + + def screen_fn(repo, product, quote): + calls.append((product, quote)) + facts = screen_mod.MarketFacts( + asset=product.split("-")[0], + daily_bars=bars, + median_daily_volume=Decimal("2000000"), + quotable_in_settlement_currency=True, + ) + result = screen_mod.ScreenResult( + asset=product.split("-")[0], + admitted=admitted, + failures=[] if admitted else ["attestation: MISSING."], + ) + return facts, result + + return screen_fn, calls + + +def test_build_routes_each_candidate_through_screen_fn(): + parsed = parse_proposal({"candidates": [_entry(asset="BTC")]}) + screen_fn, calls = _fake_screen(admitted=True) + report = build_proposal_report(parsed, _repo(), "USD", ["BTC"], screen_fn) + assert isinstance(report, ProposalReport) + assert calls == [("BTC-USD", "USD")] + sc = report.screened[0] + assert sc.product == "BTC-USD" + assert sc.on_allowlist is True + assert sc.attested is False + assert sc.result.admitted is True + assert report.admitted_count == 1 + + +def test_build_marks_off_allowlist(): + parsed = parse_proposal({"candidates": [_entry(asset="SOL")]}) + screen_fn, _ = _fake_screen(admitted=False) + report = build_proposal_report(parsed, _repo(), "USD", ["BTC"], screen_fn) + assert report.screened[0].on_allowlist is False + + +def test_shariah_hypothesis_is_never_passed_to_the_gate(): + # screen_fn only ever receives (repo, product, quote) -- the hypothesis cannot leak in. + parsed = parse_proposal( + {"candidates": [_entry(asset="SOL", shariah_hypothesis="totally halal, trust me")]} + ) + captured = [] + + def screen_fn(repo, product, quote): + captured.append((repo, product, quote)) + return ( + screen_mod.MarketFacts("SOL", 0, Decimal(0), True), + screen_mod.ScreenResult("SOL", admitted=False, failures=["attestation: MISSING."]), + ) + + report = build_proposal_report(parsed, _repo(), "USD", [], screen_fn) + assert all(len(args) == 3 for args in captured) # no 4th "hypothesis" arg exists + assert report.screened[0].result.admitted is False # hypothesis did not admit it + + +def test_invalid_entries_pass_through_to_report(): + parsed = parse_proposal({"candidates": [_entry(sources=[])]}) + screen_fn, calls = _fake_screen(admitted=True) + report = build_proposal_report(parsed, _repo(), "USD", [], screen_fn) + assert report.screened == [] + assert calls == [] # invalid entries are never screened + assert len(report.invalid) == 1 From 72e0bb5ebbccb75a29b96f9a4a0dc881610770b5 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Sat, 25 Jul 2026 00:07:08 -0400 Subject: [PATCH 3/5] feat(proposer): human + JSON renderers with no-history + attest next-steps Co-Authored-By: Claude Opus 4.8 --- keel/proposer.py | 84 ++++++++++++++++++++++++++++++++++++++ tests/test_proposer.py | 93 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 177 insertions(+) diff --git a/keel/proposer.py b/keel/proposer.py index eb4b92b0..3b7b0ea0 100644 --- a/keel/proposer.py +++ b/keel/proposer.py @@ -159,3 +159,87 @@ def build_proposal_report( ) ) return ProposalReport(screened=screened, invalid=parsed.invalid) + + +_DATA_DERIVED_FAILURES = frozenset({"liquidity"}) # keep in sync with cli.py `assets holdings` + + +def render_proposal_report(report: ProposalReport) -> list[str]: + """Human-readable lines. Admits nothing -- this only reports gate verdicts + next steps.""" + lines: list[str] = [] + if not report.screened and not report.invalid: + return ["no candidates in proposal."] + + for sc in report.screened: + cand = sc.candidate + allow = "on-allowlist" if sc.on_allowlist else "not-on-allowlist" + attested = "attested" if sc.attested else "UNATTESTED" + lines.append("") + lines.append( + f"{sc.result.summary:<7} {cand.asset:<8} bars={sc.facts.daily_bars} " + f"{allow} {attested}" + ) + lines.append(f" rationale: {cand.rationale}") + for src in cand.sources: + lines.append(f" source: {src}") + if cand.shariah_hypothesis: + lines.append( + f" UNVERIFIED hypothesis (never used for admission): {cand.shariah_hypothesis}" + ) + failures = list(sc.result.failures) + 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)") + for failure in failures: + lines.append(f" ✗ {failure}") + for warning in sc.result.warnings: + lines.append(f" ! {warning}") + if not sc.result.admitted and not sc.attested: + lines.append( + f" next: human-classify with `keel assets attest {cand.asset} " + "--sector --backing --source `, then fetch data " + "and backtest." + ) + + for entry in report.invalid: + lines.append("") + lines.append(f"INVALID {entry.reason}: {entry.raw}") + + invalid_word = "entry" if len(report.invalid) == 1 else "entries" + lines.append("") + lines.append( + f"{report.admitted_count}/{len(report.screened)} admitted " + f"({len(report.invalid)} invalid {invalid_word})" + ) + return lines + + +def report_to_jsonable(report: ProposalReport) -> dict[str, Any]: + return { + "screened": [ + { + "asset": sc.candidate.asset, + "product": sc.product, + "rationale": sc.candidate.rationale, + "sources": sc.candidate.sources, + "shariah_hypothesis": sc.candidate.shariah_hypothesis, + "on_allowlist": sc.on_allowlist, + "attested": sc.attested, + "admitted": sc.result.admitted, + "summary": sc.result.summary, + "daily_bars": sc.facts.daily_bars, + "failures": sc.result.failures, + "warnings": sc.result.warnings, + } + for sc in report.screened + ], + "invalid": [{"reason": e.reason, "raw": e.raw} for e in report.invalid], + "admitted_count": report.admitted_count, + } diff --git a/tests/test_proposer.py b/tests/test_proposer.py index a18eb2a4..01da201f 100644 --- a/tests/test_proposer.py +++ b/tests/test_proposer.py @@ -1,3 +1,4 @@ +import json from decimal import Decimal import pytest @@ -11,6 +12,8 @@ ProposalReport, build_proposal_report, parse_proposal, + render_proposal_report, + report_to_jsonable, ) @@ -150,3 +153,93 @@ def test_invalid_entries_pass_through_to_report(): assert report.screened == [] assert calls == [] # invalid entries are never screened assert len(report.invalid) == 1 + + +def _report(admitted, bars, attested=False, hypothesis=None): + parsed = parse_proposal( + {"candidates": [_entry(asset="SOL", shariah_hypothesis=hypothesis)]} + ) + + def screen_fn(repo, product, quote): + facts = screen_mod.MarketFacts("SOL", bars, Decimal("0"), True) + failures = ( + [] + if admitted + else ( + ["history: too few bars"] + if bars + else ["liquidity: 0", "attestation: MISSING."] + ) + ) + return facts, screen_mod.ScreenResult("SOL", admitted=admitted, failures=failures) + + repo = _repo() + if attested: + repo.upsert_asset_attestation( + asset="SOL", + sector="payments", + backing="native", + pays_yield=False, + source="https://x.invalid", + attested_by="t", + attested_at=0, + ) + return build_proposal_report(parsed, repo, "USD", [], screen_fn) + + +def test_render_admit_shows_summary_and_sources(): + lines = render_proposal_report(_report(admitted=True, bars=2000)) + text = "\n".join(lines) + assert "ADMIT" in text + assert "SOL" in text + assert "source: https://coinmarketcap.com/currencies/solana/" in text + assert "1/1 admitted" in text + + +def test_render_unverified_hypothesis_is_labeled(): + lines = render_proposal_report(_report(admitted=False, bars=2000, hypothesis="halal L1")) + text = "\n".join(lines) + assert "UNVERIFIED" in text + assert "halal L1" in text + + +def test_render_no_history_shows_missing_data_next_step(): + lines = render_proposal_report(_report(admitted=False, bars=0)) + text = "\n".join(lines) + 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 + + +def test_render_unattested_reject_shows_attest_next_step(): + lines = render_proposal_report(_report(admitted=False, bars=2000, attested=False)) + assert any("keel assets attest SOL" in line for line in lines) + + +def test_render_empty_report_is_friendly_not_blank(): + parsed = parse_proposal({"candidates": []}) + report = build_proposal_report(parsed, _repo(), "USD", [], lambda *a: None) + lines = render_proposal_report(report) + assert lines and "no candidates" in "\n".join(lines).lower() + + +def test_render_invalid_entries_are_listed(): + parsed = parse_proposal({"candidates": [_entry(sources=[])]}) + report = build_proposal_report(parsed, _repo(), "USD", [], lambda *a: None) + text = "\n".join(render_proposal_report(report)) + assert "INVALID" in text + assert "1 invalid" in text + + +def test_jsonable_is_json_serializable_and_has_keys(): + payload = report_to_jsonable(_report(admitted=True, bars=2000)) + dumped = json.dumps(payload, indent=2, default=str) # must not raise + back = json.loads(dumped) + assert back["admitted_count"] == 1 + row = back["screened"][0] + assert row["asset"] == "SOL" + assert row["admitted"] is True + assert row["sources"] == ["https://coinmarketcap.com/currencies/solana/"] + assert "shariah_hypothesis" in row From ba77a22403c9f33b60be242d3721f1730af43a72 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Sat, 25 Jul 2026 00:09:22 -0400 Subject: [PATCH 4/5] feat(proposer): keel assets propose -- screen an LLM shortlist, admit nothing Co-Authored-By: Claude Opus 4.8 --- keel/cli.py | 49 +++++++++++++ tests/compliance/test_assets_cli.py | 103 ++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+) diff --git a/keel/cli.py b/keel/cli.py index 9b708f5a..0d13b6e4 100644 --- a/keel/cli.py +++ b/keel/cli.py @@ -56,6 +56,7 @@ from __future__ import annotations +import json import time from datetime import UTC, datetime from decimal import Decimal, InvalidOperation @@ -71,6 +72,7 @@ from keel.commands._common import ( DEFAULT_CONFIG_PATH, DEFAULT_DB_PATH, + DISCLAIMER, _build_broker, _load_cfg, _open_repo, @@ -786,6 +788,53 @@ def assets_screen(ctx: click.Context, products: str | None) -> None: click.echo(f"\n{admitted}/{len(product_list)} admitted") +@assets_group.command("propose") +@click.option( + "--from", "from_file", required=True, + type=click.Path(exists=True, dir_okay=False), + help="JSON shortlist file produced OUTSIDE keel (an LLM + web-search scout).", +) +@click.option("--json", "as_json", is_flag=True, default=False, help="Emit machine-readable JSON.") +@click.pass_context +def assets_propose(ctx: click.Context, from_file: str, as_json: bool) -> None: + """Screen an externally-produced LLM asset shortlist. ADMITS NOTHING. + + The shortlist is produced outside keel (you, or your Claude + the firecrawl skills). Each + candidate is routed through the SAME admission gate as `assets screen`; unattested or + history-less candidates fail closed. This command never attests, never edits the allowlist, + never writes to the DB -- it only reports verdicts and next steps. + """ + from keel.proposer import ( + ProposalError, + build_proposal_report, + parse_proposal, + render_proposal_report, + report_to_jsonable, + ) + + config = _load_cfg(ctx) + repo = _open_repo(ctx) + try: + raw = json.loads(Path(from_file).read_text()) + except (OSError, json.JSONDecodeError) as exc: + raise click.ClickException(f"could not read/parse {from_file}: {exc}") from exc + try: + parsed = parse_proposal(raw) + except ProposalError as exc: + raise click.ClickException(str(exc)) from exc + + report = build_proposal_report( + parsed, repo, config.quote_currency, config.allowlist, _screen_product + ) + if as_json: + click.echo(json.dumps(report_to_jsonable(report), indent=2, default=str)) + return + for line in render_proposal_report(report): + click.echo(line) + click.echo("") + click.echo(DISCLAIMER) + + @assets_group.command("attest") @click.option("--asset", required=True, help="Asset code, e.g. BTC.") @click.option("--sector", required=True, help="Core business line / purpose of the token.") diff --git a/tests/compliance/test_assets_cli.py b/tests/compliance/test_assets_cli.py index b9e8083d..c40cc022 100644 --- a/tests/compliance/test_assets_cli.py +++ b/tests/compliance/test_assets_cli.py @@ -2,6 +2,7 @@ from __future__ import annotations +import json from decimal import Decimal from pathlib import Path @@ -809,3 +810,105 @@ def test_the_settlement_criterion_still_catches_an_EXTERNALLY_supplied_product( assert "settlement" in result.output, "a cross-settled product must fail the settlement check" assert "REJECT" in result.output + + +# -- assets propose ----------------------------------------------------------------------------- + + +def _write_shortlist(tmp_path, candidates): + path = tmp_path / "shortlist.json" + path.write_text(json.dumps({"candidates": candidates})) + return path + + +_SOL = { + "asset": "SOL", + "rationale": "high liquidity", + "sources": ["https://coinmarketcap.com/currencies/solana/"], +} + + +def test_propose_rejects_an_unattested_candidate(tmp_path, valid_config_path): + db_path = tmp_path / "t.db" + _repo_at(db_path) + 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 "REJECT" in result.output + assert "0/1 admitted" 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" + repo = _repo_at(db_path) + _seed_history(repo, "BTC-USD") + runner = CliRunner() + assert _attest(runner, db_path, valid_config_path, "BTC").exit_code == 0 + shortlist = _write_shortlist( + tmp_path, [{"asset": "BTC", "rationale": "reserve asset", "sources": ["https://bitcoin.org"]}] + ) + proposed = runner.invoke( + cli, ["--db", str(db_path), "--config", str(valid_config_path), + "assets", "propose", "--from", str(shortlist)], + ) + screened = runner.invoke( + cli, ["--db", str(db_path), "--config", str(valid_config_path), + "assets", "screen", "--products", "BTC-USD"], + ) + assert "ADMIT" in proposed.output + assert "ADMIT" in screened.output + + +def test_propose_writes_nothing(tmp_path, valid_config_path): + db_path = tmp_path / "t.db" + repo = _repo_at(db_path) + shortlist = _write_shortlist(tmp_path, [_SOL]) + CliRunner().invoke( + cli, ["--db", str(db_path), "--config", str(valid_config_path), + "assets", "propose", "--from", str(shortlist)], + ) + assert repo.get_asset_attestation("SOL") is None # nothing attested/admitted + + +def test_propose_json_is_valid_and_has_no_trailing_prose(tmp_path, valid_config_path): + db_path = tmp_path / "t.db" + _repo_at(db_path) + shortlist = _write_shortlist(tmp_path, [_SOL]) + result = CliRunner().invoke( + cli, ["--db", str(db_path), "--config", str(valid_config_path), + "assets", "propose", "--from", str(shortlist), "--json"], + ) + payload = json.loads(result.output) # must parse cleanly + assert payload["admitted_count"] == 0 + assert payload["screened"][0]["asset"] == "SOL" + + +def test_propose_missing_file_is_a_clean_error(tmp_path, valid_config_path): + db_path = tmp_path / "t.db" + _repo_at(db_path) + result = CliRunner().invoke( + cli, ["--db", str(db_path), "--config", str(valid_config_path), + "assets", "propose", "--from", str(tmp_path / "nope.json")], + ) + assert result.exit_code != 0 + + +def test_propose_hypothesis_never_admits(tmp_path, valid_config_path): + db_path = tmp_path / "t.db" + _repo_at(db_path) + shortlist = _write_shortlist( + tmp_path, + [{"asset": "SOL", "rationale": "x", "sources": ["https://x.invalid"], + "shariah_hypothesis": "definitely halal"}], + ) + result = CliRunner().invoke( + cli, ["--db", str(db_path), "--config", str(valid_config_path), + "assets", "propose", "--from", str(shortlist)], + ) + assert "REJECT" in result.output # unattested + no history => rejected despite the hypothesis + assert "UNVERIFIED" in result.output From e2b3f95a6c6f0067144dd3a11009b5f875a1d87d Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Sat, 25 Jul 2026 00:18:43 -0400 Subject: [PATCH 5/5] fix(proposer): hardening from review -- shared failure-tag constant, hyphenated-asset rejection, stronger writes-nothing test - Single source of truth for the zero-bar failure-tag suppression: add DATA_DERIVED_FAILURES to keel/compliance/screen.py (it owns the tags) and have both keel/cli.py's `_DATA_DERIVED_FAILURES` and keel/proposer.py import it instead of each defining its own copy, plus a pin test asserting the set matches screen_asset's real output. - Reject an asset code containing non-alphanumeric characters (e.g. "sol-usd") at schema validation: `_history_product` and the gate key off product.split("-")[0], but the report's on_allowlist/attested labels keyed off the full string would silently disagree with what was actually screened. - Strengthen test_propose_writes_nothing to assert the full repo.get_asset_attestations() == [] on a freshly reopened DB handle, so a stray write to any asset/table would be caught, not just the one candidate proposed. - Add coverage: human output includes the DISCLAIMER footer; a non-dict candidate entry lands in `invalid` with a clear reason instead of crashing. Co-Authored-By: Claude Opus 4.8 --- keel/cli.py | 6 +++-- keel/compliance/screen.py | 10 ++++++++ keel/proposer.py | 11 ++++++--- tests/compliance/test_assets_cli.py | 19 +++++++++++++-- tests/test_proposer.py | 38 +++++++++++++++++++++++++++++ 5 files changed, 76 insertions(+), 8 deletions(-) diff --git a/keel/cli.py b/keel/cli.py index 0d13b6e4..df498d8f 100644 --- a/keel/cli.py +++ b/keel/cli.py @@ -91,6 +91,7 @@ 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 @@ -546,8 +547,9 @@ def _screen_product( # 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. -_DATA_DERIVED_FAILURES = frozenset({"liquidity"}) +# 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 diff --git a/keel/compliance/screen.py b/keel/compliance/screen.py index 6346d04c..eb8762ea 100644 --- a/keel/compliance/screen.py +++ b/keel/compliance/screen.py @@ -56,6 +56,16 @@ #: decision, not a default -- do not add to it to make a test pass. 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 +#: elsewhere. +DATA_DERIVED_FAILURES = frozenset({"liquidity"}) + @dataclass(frozen=True) class AssetAttestation: diff --git a/keel/proposer.py b/keel/proposer.py index 3b7b0ea0..a2cb85fe 100644 --- a/keel/proposer.py +++ b/keel/proposer.py @@ -15,6 +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.data.repository import Repository ScreenFn = Callable[ @@ -63,6 +64,11 @@ def _candidate_error(entry: Any) -> str | None: asset = entry.get("asset") if not isinstance(asset, str) or not asset.strip(): return "missing or empty 'asset'" + if not asset.strip().isalnum(): + # `_history_product`/the gate key off `product.split("-")[0]`; an asset containing "-" + # (or any other punctuation) would build a malformed product id and, worse, make the + # on_allowlist/attested labels in the report disagree with what was actually screened. + return "'asset' must be an alphanumeric symbol" rationale = entry.get("rationale") if not isinstance(rationale, str) or not rationale.strip(): return "missing or empty 'rationale'" @@ -161,9 +167,6 @@ def build_proposal_report( return ProposalReport(screened=screened, invalid=parsed.invalid) -_DATA_DERIVED_FAILURES = frozenset({"liquidity"}) # keep in sync with cli.py `assets holdings` - - def render_proposal_report(report: ProposalReport) -> list[str]: """Human-readable lines. Admits nothing -- this only reports gate verdicts + next steps.""" lines: list[str] = [] @@ -188,7 +191,7 @@ def render_proposal_report(report: ProposalReport) -> list[str]: ) failures = list(sc.result.failures) if sc.facts.daily_bars == 0: - derived = [f for f in failures if f.split(":")[0] in _DATA_DERIVED_FAILURES] + 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, " diff --git a/tests/compliance/test_assets_cli.py b/tests/compliance/test_assets_cli.py index c40cc022..33cd8016 100644 --- a/tests/compliance/test_assets_cli.py +++ b/tests/compliance/test_assets_cli.py @@ -11,6 +11,7 @@ import keel.cli as cli_module from keel.cli import cli +from keel.commands._common import DISCLAIMER from keel.data.db import connect, migrate from keel.data.repository import Repository from keel.types import Candle, Granularity @@ -865,14 +866,17 @@ def test_propose_and_screen_agree_for_the_same_asset(tmp_path, valid_config_path def test_propose_writes_nothing(tmp_path, valid_config_path): + """A read-only report: no attestation, no allowlist change, no DB mutation.""" db_path = tmp_path / "t.db" - repo = _repo_at(db_path) + _repo_at(db_path) shortlist = _write_shortlist(tmp_path, [_SOL]) CliRunner().invoke( cli, ["--db", str(db_path), "--config", str(valid_config_path), "assets", "propose", "--from", str(shortlist)], ) - assert repo.get_asset_attestation("SOL") is None # nothing attested/admitted + # Reopen from the path (not the handle held from before the run) so a stray write to ANY + # asset/table would actually be caught, not just the one candidate we happened to propose. + assert _repo_at(db_path).get_asset_attestations() == [] def test_propose_json_is_valid_and_has_no_trailing_prose(tmp_path, valid_config_path): @@ -912,3 +916,14 @@ def test_propose_hypothesis_never_admits(tmp_path, valid_config_path): ) assert "REJECT" in result.output # unattested + no history => rejected despite the hypothesis assert "UNVERIFIED" in result.output + + +def test_propose_human_output_ends_with_the_disclaimer(tmp_path, valid_config_path): + db_path = tmp_path / "t.db" + _repo_at(db_path) + 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 DISCLAIMER in result.output diff --git a/tests/test_proposer.py b/tests/test_proposer.py index 01da201f..2c912c5d 100644 --- a/tests/test_proposer.py +++ b/tests/test_proposer.py @@ -4,6 +4,7 @@ import pytest from keel.compliance import screen as screen_mod +from keel.compliance.screen import DATA_DERIVED_FAILURES from keel.data.db import connect, migrate from keel.data.repository import Repository from keel.proposer import ( @@ -66,6 +67,15 @@ def test_missing_asset_is_invalid(): assert "asset" in parsed.invalid[0].reason +def test_asset_with_a_hyphen_is_invalid(): + """`sol-usd` would make `_history_product` build `SOL-USD-USD` and would make the + on_allowlist/attested labels (keyed on the full string) disagree with what the gate actually + screens (keyed on `product.split("-")[0]`) -- reject it at schema validation instead.""" + parsed = parse_proposal({"candidates": [_entry(asset="sol-usd")]}) + assert parsed.candidates == [] + assert "alphanumeric" in parsed.invalid[0].reason + + def test_malformed_top_level_raises(): with pytest.raises(ProposalError): parse_proposal({"not_candidates": []}) @@ -79,6 +89,13 @@ def test_mixed_valid_and_invalid_are_partitioned(): assert len(parsed.invalid) == 1 +def test_non_dict_candidate_entry_is_invalid_not_a_crash(): + parsed = parse_proposal({"candidates": ["just a string"]}) + assert parsed.candidates == [] + assert len(parsed.invalid) == 1 + assert parsed.invalid[0].reason == "entry is not an object" + + def _repo(): conn = connect(":memory:") migrate(conn) @@ -243,3 +260,24 @@ def test_jsonable_is_json_serializable_and_has_keys(): assert row["admitted"] is True assert row["sources"] == ["https://coinmarketcap.com/currencies/solana/"] assert "shariah_hypothesis" in row + + +def test_data_derived_failures_tags_actually_match_screen_asset_output(): + """Pins the shared `DATA_DERIVED_FAILURES` constant to what `screen_asset` actually emits. + + `keel/proposer.py` and `keel/cli.py` both import this single constant (rather than each + defining their own copy) specifically so a failure-tag rename in `screen_asset` breaks THIS + test instead of silently disabling the zero-bar suppression in both callers. + """ + facts = screen_mod.MarketFacts( + asset="SOL", + daily_bars=0, + median_daily_volume=Decimal(0), + quotable_in_settlement_currency=False, + ) + tags = {f.split(":")[0] for f in screen_mod.screen_asset(facts, None).failures} + missing = DATA_DERIVED_FAILURES - tags + assert not missing, ( + f"{missing} no longer appear as failure tags in screen_asset -- the zero-bar " + "suppression is now silently inert; update DATA_DERIVED_FAILURES in screen.py" + )