fix(cli): render Preview.synthetic at the confirm gate; an estimate must not look like a quote - #221
Merged
Merged
Conversation
…ust not look like a quote `_interactive_confirm` took a raw `dict` and knew nothing about the port's `Preview`, so it had nowhere to render `synthetic` -- the flag that says whether a human is approving numbers the venue stands behind or an estimate keel computed from a price lookup that validated nothing and reserved nothing. `Preview`'s own docstring makes surfacing that distinction a requirement of anything rendering one. Latent today: Coinbase has a native preview endpoint, so nothing synthetic reaches a human. Real for the first synthesizing venue -- and worse there, because an unpriceable synthesized preview comes back as zeroes, which on an undecorated key/value screen reads as a harmless "$0.00 order" rather than as "keel has no idea what this costs". The gate now: - accepts BOTH the port's `Preview` and the legacy Coinbase dict. Phase B has not landed -- `executor.py` still calls `broker.place_order(product_id, side, order_configuration)` and `_common.py` still builds `CoinbaseClient` -- so `dict` is what the only trading venue sends today. `_read_preview` is the seam, and its docstring says why it is transitional. - renders provenance ABOVE the numbers, not below: a footnote under a tidy key/value block is read after the decision has been made. A broker quote gets a `=` rule; a synthetic estimate gets a `!` block stating that the venue priced, validated and reserved nothing. - shows `Preview.errors` in their own alarm block instead of one quiet line among ten. The legacy dict's `errs` is promoted the same way. - calls out an unpriced preview explicitly as an order whose cost could NOT be determined, rather than letting a zero read as cheap. - upgrades the question from `[y/N]` to a typed "place anyway" when the preview is unpriced, carries errors, or cannot be parsed at all. Friction, never a refusal: an unpriced preview is exactly what a human sees when a venue's pricing endpoint is down and they are trying to close a position, and walling off the exit path is a worse money outcome than a warned approval. - never lets free-form `Preview.detail` shadow a money field -- a colliding key is namespaced. Unchanged where it matters: the live Coinbase dict still renders every field it did, in order, under the same header, and still takes an ordinary `y`. Verified end-to-end through `cb_client.preview_order` against the real `cb_preview_order.json` fixture. Still fails closed off a TTY, for every shape. `ConfirmFn` and `ExecutionResult.preview` widen to both shapes to document the contract at the call site. `_run_order`'s dict-only `fee` extraction is deliberately untouched -- that belongs to the port migration (#198), not to the gate. Closes #199. Refs #198. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…gate means The runbook printed the old undecorated preview screen and told the operator, flatly, that what they were looking at was 'the broker's own numbers, not keel's estimate'. That is now something the screen itself asserts rather than something the docs promise, and there are three screens where it is not true. Documents the '!' blocks, and the typed-phrase escalation -- including why it is passable, so nobody reads it as a wall while trying to exit a position. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review flagged the assumption: the gate read a non-positive `order_total` as unpriced, but whether Coinbase reports a SELL's `order_total` as signed proceeds is UNVERIFIED against a real sell preview. Guessing wrong in that direction would demand the typed phrase on every live sell -- which is worse than useless: it trains the operator to type it by reflex and destroys the signal on the previews that actually need it. A zero is unambiguous (no size, unknown cost); a negative is not. So the legacy dict branch keys on `== 0` and stays out of the sign question until a live probe settles it. The `Preview` branch owns its own types and stays stricter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
eaitbrahim
added a commit
that referenced
this pull request
Aug 11, 2026
…the CTS scoring fix (#241) A minor bump, not a patch, for three reasons that each require operator action or change behaviour the deployment is currently relying on. SCHEMA. `SCHEMA_VERSION` goes 9 -> 10 (#223). Both deployed databases are at 9 and must be migrated before this build can use them. BEHAVIOUR REQUIRING OPERATOR ACTION. #223 adds a second attested claim -- what CONTRACT a venue listing is, not only what the underlying asset is. It fails closed with no backfill, deliberately, so after this lands `keel assets screen` REJECTS every product with `instrument_wrapper: UNATTESTED` until `keel assets attest-instrument` is run once per product. Live trading is unaffected: rail 1 gates buys on `config.allowlist`, not on the screen. LIVE SCORING CHANGED. #227 fixed `is_round_number`, which returned True for every 2dp-quoted price and so handed BTC/ETH/PAXG a free CTS point on every bar. Scores on those three assets are genuinely lower under this build than under 0.5.7. Also ships: the Robinhood crypto adapter behind the broker port (#216/#218/#222/#229, not wired to the live path), the TUI activity feed (#235/#237), the CTS factor collinearity study (#224), `Preview.synthetic` at the confirm gate (#221), rail 9 seeing a bracket's own stop (#212), and CI gating merges on the `test` check (#234/#238). Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #199. Refs #198.
keel/cli.py:_interactive_confirmtook a rawdictand knew nothing about the port'sPreview, so it had nowhere to rendersynthetic.Preview's docstring makes that a requirement of anything rendering one:Latent today (Coinbase has a native preview endpoint, so nothing synthetic reaches a human), real for the first synthesizing venue. Worse there: an unpriceable synthesized preview comes back as zeroes, which on an undecorated key/value screen reads as a harmless "$0.00 order" rather than "keel has no idea what this costs".
What a human sees now
Native / broker-quoted — unchanged question, new provenance rule:
Synthetic, priced — same numbers, unmistakably not the same screen. Still an ordinary
y/N: the warning is the banner, not ceremony on every exit.Synthetic, unpriced, carrying errors — the case the issue is really about:
The friction, and why it is not a block
An unpriced / error-carrying / unparseable preview upgrades the question from
[y/N]to a typedplace anyway. Ayat a[y/N]prompt is muscle memory after the tenth order of the day, and the point of the banners is that this screen is not like the last ten.It is deliberately not a refusal. An unpriced preview is exactly what a human sees when a venue's pricing endpoint is down and they are trying to close a position; walling off the exit path would trap a position behind a broken preview endpoint — a worse money outcome than a warned-and-approved order. Harder to do, never impossible. Ctrl-C / EOF at the prompt declines rather than raising out of the gate.
The live Coinbase path is untouched in substance
Phase B has not landed:
executor.pystill callsbroker.place_order(product_id, side, order_configuration)and_common.pystill buildsCoinbaseClient, so adictis what the only trading venue sends today. The gate accepts both shapes;_read_preview's docstring says why that is transitional and when it can be deleted. Verified end-to-end by driving the realtests/fixtures/cb_preview_order.jsonthroughcb_client.preview_orderinto the gate — every field renders as before, in order, under the sameCoinbase order previewheader, and it still takes an ordinaryy:A dict is read as native because the dict shape is Coinbase's native-preview response — but a dict carrying a truthy
synthetickey is believed over that default, so a future dict-returning synthesizer cannot be silently dressed up as a broker quote.Also
errsis promoted out of the key/value list into the same alarm block, so a Coinbase preview that came back with errors is no longer one quiet line among ten.Preview.detailcan never shadow a money field; a colliding key renders asdetail.<key>. An adapter bug must not be able to put a wrongest_feeon a spending screen with nothing marking it as substituted.ConfirmFnandExecutionResult.previewwiden to both shapes, documenting the contract at the call site._run_order's dict-onlyfeeextraction (executor.py) is deliberately not touched — aPreviewwould needest_feeread there, but that belongs to the port migration (Prerequisites before wiring Robinhood to the live execution path #198), not to this gate. Flagging it so the migration trips over it.Second render site?
Swept for one (
grep -rn preview keel/ --include='*.py', plus everyconfirm_fn/click.confirm/click.prompt/ notification / alerting path). There is none.keel/commands/tui.pyis a real curses dashboard but is read-only overkeel statusand renders no order preview — itsconfirm_fn(toggle_autonomy) gates arming autonomy and takes no argument.keel-live-run.sh's macOS notifications carry only a status string telling the operator to go run the agent interactively. Every otherPreviewconsumer is data plumbing (adapters constructing it), structured logging, or tests.One thing to verify against the live venue
The dict path treats a zero
order_totalas unpriced, deliberately not<= 0. WhetherCoinbase reports a SELL's
order_totalas signed proceeds is unverified against a real sellpreview, and guessing wrong in the
<= 0direction would demand the typed phrase on every livesell — which trains the operator to type it by reflex and destroys the signal on the previews that
need it. Sign-agnostic until a live probe settles it; noted in the code and in a test.
Gates
Exactly the one expected skip. TDD:
tests/test_confirm_gate.pywas written first and failed 11/15 againstmain.🤖 Generated with Claude Code