fix(admission): close the non-UTF-8 shortlist hole and pin the claims #176 made

[eaitbrahim]

An independent adversarial review of #176 returned SHIP WITH CHANGES: network gating and the read-only guarantees held up (34 socket-disabled checks, DB bytes identical, attest absent), with ten findings to fix. This is all ten, TDD — every test below was seen failing for the right reason before the fix, and every coverage-only test was verified to die under a deliberate mutation.

The two real bugs

F1 — build_propose_view broke its own "Never raises" contract. source.read_text() was guarded by except OSError, but UnicodeDecodeError subclasses ValueError (issubclass(UnicodeDecodeError, OSError) is False). A UTF-16LE+BOM shortlist — valid JSON, and what a scout run on Windows writes — escaped both handlers and propagated out of a function whose docstring promises "Never raises. Every failure mode is FAIL-SOFT". Observed: the propose overlay repainting propose read failed: 'utf-8' codec can't decode byte 0xff… every poll forever, naming no file and no next step, surviving only on run_live's broad except Exception; and keel assets propose --from exiting on a raw traceback on the same input. Both paths now take the existing unreadable fail-soft branch, which names the file.

The two comments this falsified — _do_propose_view's docstring and the identical inline comment in run_live's propose branch, both claiming an exception "can only come from open_state()… never from the shortlist read itself" — are corrected. Note they were not merely stale: build_propose_view screens every parsed candidate after parsing, so a locked DB surfaces there too. The handler is load-bearing for both, and now says which.

F9 — assets propose --json told a different story from every human surface. It emitted sc.result.failures raw, so at zero cached bars the payload carried history: 0 daily bars < 1460 required unflagged, while render_proposal_report, assets holdings --screen, assets screen and the TUI overlays all suppress that exact line and print the MISSING-DATA explanation. That is precisely the drift split_failures was created to end, surviving in the one surface a script reads and cannot argue with. --json now applies the same split and makes it explicit: failures (verdicts about the asset), not_assessable (suppressed, never dropped), missing_history.

F2 — the single-admission-path property, tested at the wiring point

Design constraint 4 ("every candidate routes through _screen_product, so nothing drifts onto a laxer gate") was convention-only exactly where it is wired. Replacing _screen_product with an always-ADMIT stub in _do_screen_report or _do_propose_view left all 2062 tests green — the existing overlay tests only assert a title paints and Esc closes, never a verdict.

Two run_live tests now seed an asset with ample cached history and liquidity but no attestation, so the only thing that can reject it is screen_asset failing closed on attestation=None — something only the real gate does. Both assert the overlay paints REJECT plus attestation: MISSING, and assert the absence of ✗ history/✗ liquidity so the REJECT provably comes from the shariah criterion rather than an incidental data shortfall any stub would also produce.

Both mutations verified dead. The _do_screen_report one was also run against the full suite pre-fix: it failed nothing.

Honesty fixes — comments asserting more than the code can know

• F3 split_failures claimed a shallow-but-non-empty cache means the asset "really is too young". MarketFacts carries a bar count and no first-bar timestamp, so it cannot distinguish that from keel fetch --years 2, an aborted fetch, or a venue not serving the full window (fetch prints a note about exactly this). Reworded to say plainly that the two are indistinguishable here, why the gate still fails closed on the ambiguity, and what the operator should check first (keel fetch --products <id> --years 5, then re-screen). Not re-engineered.
• F5 missing_history_lines said a zero-bar asset "is not too young, we have simply never fetched candles for it" — the same overclaim in the opposite direction, since at zero bars a three-day-old listing and a three-year-old one are literally the same input. It now refuses to rule either way.
• F10 "the SECOND deliberate network exception" appeared 3× — module docstring, help Safety notes, and the operator-facing ARMED overlay — forgetting the ~30s live-balance refresh that has been a real get_accounts call since v3. It is the THIRD of exactly three, as tui_cmd's docstring already said correctly. All three corrected and pinned by a parametrized test across all four surfaces. The help's "Live balance" section never said the refresh is a live venue call at all — the omission that made "second" read as plausible three sections later — so it now says so, and says it is a read that places no orders.
• F4 run_live said "Seven modes"; there are six.

Coverage for correct-but-untested code

• F7 config.proposals_dir resolution was never executed by the suite (every test passed directory=), so hardcoding a path left 2062 tests green — and the default is ~/keel/proposals, inside the live deployment root. Two tests with HOME redirected at tmp_path: one pins that a non-default proposals_dir is honoured, one that the default resolves to <home>/keel/proposals expanded. Both die under a hardcoded path; both die under a dropped .expanduser().
• F8 _DISCOVER_TIMEOUT_SEC had no test. Pinned per-call, not globally, so collapsing it into _BALANCE_TIMEOUT_SEC is caught too. Dies under both mutations.
• F5 (test half) the semantic sentence, plus the guarantee that these lines never restate a suppressed failure verbatim (the call-site proxy "✗ history" not in text would miss a leak, since a leak carries no ✗).

F6 — a test renamed to what it actually tests

test_run_live_discover_closing_discards_the_held_result is now test_run_live_discover_reopening_after_a_run_is_armed_not_stale.

I took the rename rather than the "pin the close branch specifically" option, and verified the reason: deleting the close-branch clear alone leaves the test green; deleting both clears kills it. Nothing observable from outside run_live can separate them, because mode only ever becomes discover via the normal-mode d branch that also clears — there is no route into the overlay that bypasses it. Keeping both clears is deliberate defence in depth, so the honest fix is the name plus a docstring that states the disjunction and names both lines.

One thing I'd flag back

F5 was written as a coverage finding, but the sentence it points at is the same overclaim F3 flags, one step further along: "it is not too young" asserts a fact about the asset at exactly the bar count where we have zero evidence about the asset. I reworded it rather than only pinning it. Called out here in case the intent was to pin the existing wording as-is.

Gates

ruff check keel tests packages clean · mypy keel packages clean (95 files) · pytest -q 2080 passed (2062 baseline + 18 new).