diff --git a/keel/web/api.py b/keel/web/api.py index d772ef7..ea37d17 100644 --- a/keel/web/api.py +++ b/keel/web/api.py @@ -120,7 +120,20 @@ def deployment_state(cfg: ServeConfig) -> Any: def close_repo(repo: Any) -> None: - conn = getattr(repo, "conn", None) + """Close the connection `open_repo` opened. Every reader in this package runs this in a + `finally`, and until #704 none of them closed anything. + + It reached for `repo.conn`. `Repository.__init__` stores `self._conn` and exposes no `conn`, + so the `getattr` returned `None`, the guard fell through, and the function was a no-op -- + every page load left an unclosed sqlite3 connection, reclaimed only by CPython's refcounting + when the local went out of scope. Harmless in practice on CPython and wrong in the way that + matters here: the `finally` READ as the cleanup, so nothing looked missing. + + `_conn` first, `conn` second, so this keeps working against anything duck-typed as a + repository (the MCP tools hand around objects that are not `Repository`), and both are + checked rather than one being inferred from the other. + """ + conn = getattr(repo, "_conn", None) or getattr(repo, "conn", None) if conn is not None: try: conn.close() @@ -164,14 +177,20 @@ def _status_report(cfg: ServeConfig, now_ts: int) -> Any: close_repo(repo) -def read_config(cfg: ServeConfig, _query: Query, _state: Any, _now_ts: int) -> dict[str, Any]: - """The running build, and the deployment that build is serving (#597). +def read_config(cfg: ServeConfig, _query: Query, _state: Any, now_ts: int) -> dict[str, Any]: + """The running build, and the deployment that build is serving (#597, #704). + + **It answers on a machine with nothing set up, which is why its route is + `needs_database=False` -- and #704 added one OPTIONAL database read without changing that.** + The chip needs `equity_state_mode`, which lives in `agent_state`, and this is the only + endpoint every view reads. `_equity_state_mode` therefore checks the file exists before + connecting (`sqlite3.connect` CREATES what it cannot find) and degrades to unknown on any + failure, so a first run still boots the shell -- it just boots it without the equity half of + the chip, which is the honest thing for a deployment that has never run. - Still reads NO database -- it answers on a machine with nothing set up, which is why its - route is `needs_database=False`. The deployment half arrives as this process's own - arguments (`cfg.db_path`, `cfg.config_path`) plus one read of the config FILE, which opens - no database and forks no subprocess, unlike the `inspect` probe the envelope has already run - for `engine` by the time this returns. + The rest of the deployment half arrives as this process's own arguments (`cfg.db_path`, + `cfg.config_path`) plus one read of the config FILE, which forks no subprocess, unlike the + `inspect` probe the envelope has already run for `engine` by the time this returns. **`_mode` degrades rather than raising, and the whole shell depends on that.** The client boots from this one endpoint -- worker registration, docs links, the footer build line and @@ -183,11 +202,75 @@ def read_config(cfg: ServeConfig, _query: Query, _state: Any, _now_ts: int) -> d cfg.build_info, describe=cfg.build, mode=_auto_trade_mode(cfg.config_path), + profile=_profile_name(cfg.db_path), + **_session_state(cfg.db_path, now_ts), db_path=cfg.db_path, config_path=cfg.config_path, ) +def _profile_name(db_path: str) -> str: + """The deployment profile's name: the database file's stem (#704). + + ADR 0002 settles what a profile IS -- "the database is already one-per-profile", which is + also why `equity_points` has a `mode` column and no `profile` one. So there is nothing + STORED to read: the profile is the file, and this names the file. + + The stem and not a prettier word, because anything prettier would be inferred. `keel.db` and + `keel-live.db` are the operator's own names for their deployments; a mapping from those to + "paper"/"live" would be this console guessing which is which from a filename, on the one + surface built to stop paper and live being confused. The full paths stay in the mode badge's + tooltip, so the short name is checkable rather than trusted. + """ + return Path(db_path).stem if db_path else "" + + +def _session_state(db_path: str, now_ts: int) -> dict[str, Any]: + """`equity_state_mode` and `autonomous` -- the two deployment facts the chip and banner need. + + **Both from ONE connection.** They are read together because they are shown together, and two + opens on the boot path of every page would be two chances to leak and two answers that could + describe different instants. + + `autonomous` is `Profile.is_autonomous(now_ts)`, which honours the expiry the operator set -- + so a lapsed `keel autonomy on --until` stops being claimed by the banner at the moment it + stops applying, rather than at the next restart. `get_profile` FAILS CLOSED (an absent row, a + damaged database -> not autonomous), which is the direction that makes the banner's mistake, + if it makes one, the one that over-promises supervision rather than under-promising it. + + **This is the one database read on an endpoint whose route is `needs_database=False`, and the + exemption is load-bearing.** The client boots from this endpoint alone, so a database that is + missing (a first run) or unreadable must cost the chip its halves, never the page its boot. + Every failure is one answer here for the same reason `_auto_trade_mode` treats a missing file, + malformed YAML and a refused value alike: naming the narrow ones would leave this reader + deciding which failure is which, and the caller's response is the same. + + **Existence is checked before connecting, and that is not a micro-optimisation.** + `sqlite3.connect` CREATES the file it cannot find, so connecting unconditionally would have a + read-only view bring a deployment into existence merely by being polled -- and every page + would then report a healthy empty install rather than offering to set one up. + `server.ensure_schema` carries the same guard and the same reasoning, found the same way. + + No migration, like every other read in this package: a view must not take a schema write lock + on a database the agent may be mid-cycle on. + """ + unknown = {"equity_state_mode": "", "autonomous": False} + if not db_path or not Path(db_path).exists(): + return unknown + repo = None + try: + repo = open_repo(db_path) + return { + "equity_state_mode": str(repo.get_state("equity_state_mode") or ""), + "autonomous": bool(repo.get_profile().is_autonomous(now_ts)), + } + except Exception: # a database that cannot answer is one that has not answered + return unknown + finally: + if repo is not None: + close_repo(repo) + + def _auto_trade_mode(config_path: str) -> str: """The served config's own word for `auto_trade.mode`, or `""` when it cannot be read. diff --git a/keel/web/payload.py b/keel/web/payload.py index 639b788..4016018 100644 --- a/keel/web/payload.py +++ b/keel/web/payload.py @@ -2570,11 +2570,82 @@ def order_rows( # -- config (#534) ------------------------------------------------------------------------------- +#: The banner's exact words per `auto_trade.mode` (#704). +#: +#: PAPER is fixed wording, and it is the sentence that stands between an operator and mistaking a +#: simulation for their account -- so it is asserted verbatim by a test rather than left to a +#: renderer's phrasing. CONFIRM is not a warning: it restates the configuration the operator is +#: about to verify against the venue's own UI, which is why it names the equity state too. +#: +#: **Nothing here ever offers a way to go live.** Alpaca's paper banner carries an "Open Live +#: Account" button -- radical clarity used as a growth funnel wrapped around real money. Going +#: live in keel is a config edit plus a typed terminal ceremony with a runbook; this surface may +#: EXPLAIN that and must never funnel toward it. Pinned by a test that sweeps every mode and +#: equity-state pairing for the vocabulary of a funnel. +_BANNER_PAPER = "PAPER — no real money is involved" + +#: What the chip says where `equity_state_mode` has never been written -- a deployment that has +#: not yet flipped modes. NOT `paper`: that would be the console inventing the safer of the two +#: answers about which account drove the shared drawdown scalars. +_EQUITY_STATE_UNRECORDED = "not recorded" + + +def _session_banner(mode: str, equity_state_mode: str, autonomous: bool) -> str: + """The banner sentence for one session, or `""` for a config that could not be read. + + Chosen HERE and not in the renderer (Rule 2): picking between sentences on the basis of what a + deployment says is a judgement, and judgements are made in Python. The renderer places this + string and decides nothing. + + ── AUTONOMY IS THE HALF THIS BANNER SHIPPED WITHOUT, AND IT INVERTED THE CLAIM ────────────── + + The first cut said "CONFIRM — every order is previewed and waits for your approval" whenever + the config mode was `confirm`. That is FALSE on a deployment running `keel autonomy on`, which + is a supported and deliberate configuration: `agent._effective_mode` returns `"autonomous"` + when the config is `confirm` AND `Profile.is_autonomous(now)`, and an autonomous cycle places + without asking anyone. So the one persistent, full-bleed statement on every page was + promising supervision that had been switched off -- a false safety assurance about real money, + on the surface built to stop exactly that kind of confusion, and worse than the growth funnel + this issue exists to refuse. + + Autonomy and the config mode are two independent switches, and `_effective_mode`'s docstring + says why they are deliberately not one enum. This function states the same pairing for + display; `agent._effective_mode` remains the authority for execution, and + `test_the_banner_and_the_engine_agree_about_who_is_asked` sweeps both over the same inputs so + the two statements of one rule cannot drift apart. + + An unknown mode gets NO banner, the same refusal `modeBadge` already makes for the badge + itself: an absent answer is not `paper`, and a banner is a claim about whether real money is + involved with no safe default. + """ + if mode == "paper": + return _BANNER_PAPER + if not mode: + return "" + # Both halves of the pairing, always. Mode and equity state are separately settable, and a + # MISMATCH between them -- `confirm` against paper equity, or the reverse -- is precisely what + # an operator is being asked to check against the venue UI. Printing the mode alone would drop + # the half that makes the check possible. + state = equity_state_mode or _EQUITY_STATE_UNRECORDED + if autonomous and mode == "confirm": + return ( + f"{mode.upper()} · AUTONOMOUS — orders place without asking you; " + f"equity state {state}" + ) + return ( + f"{mode.upper()} — every order is previewed and waits for your approval; " + f"equity state {state}" + ) + + def config_payload( build: Any, *, describe: str = "", mode: str = "", + profile: str = "", + equity_state_mode: str = "", + autonomous: bool = False, db_path: str = "", config_path: str = "", ) -> dict[str, Any]: @@ -2621,6 +2692,25 @@ def config_payload( # paths that answer "where am I" for a process serving one --db/--config pair. See the # module note above for why these are bare strings and why mode is read-only. "mode": mode, + # The session chip (#704). `profile` is a bare string like `mode` -- it NAMES the + # database, which ADR 0002 says is the profile ("the database is already one-per-profile"), + # and there is nothing to grade about a name. + "profile": profile, + # A `label` and not bare, because unlike `mode` this one has an UNKNOWN reading that + # matters: `equity_state_mode` is written on the first mode flip, so a deployment that has + # never run has none, and "which account drove the drawdown scalars" being unanswered is a + # different fact from either answer. Rule 3 keeps that distinction here rather than letting + # a client infer it from an empty string. + # `""` and not `None` for the absent case, deliberately. `label(None)` is `absent()`, + # whose display is the em-dash -- right in a table cell, wrong here: a chip reading + # "keel · confirm · —" drops the very half the banner asks the operator to verify. The + # empty `value` still says absent to anything reading the field programmatically. + "equity_state": label( + equity_state_mode, + display=equity_state_mode or _EQUITY_STATE_UNRECORDED, + state=NEUTRAL if equity_state_mode else UNKNOWN, + ), + "banner": _session_banner(mode, equity_state_mode, autonomous), "db_path": db_path, "config_path": config_path, # keel's central honesty signal, and the one judgement this payload carries: `False` means diff --git a/keel/web/static/css/keel.css b/keel/web/static/css/keel.css index 8b36427..71efd29 100644 --- a/keel/web/static/css/keel.css +++ b/keel/web/static/css/keel.css @@ -256,6 +256,69 @@ header .mode-paper { color: var(--muted); } header .mode-confirm, header .mode-live { color: var(--accent); border-color: var(--accent); } +/* THE SESSION CHIP (#704): profile · mode · equity state, as one group. + * + * A group and not three loose items, because the three are one answer: "which deployment is this + * browser looking at, and which account drove the numbers on it". `inline-flex` so the badge in + * the middle keeps its own pill shape while the two text halves sit level with it. + * + * Both halves are muted. The mode badge carries the emphasis for all three -- it is the fact that + * decides whether orders can be placed -- and giving a profile name the same weight would make + * the header compete with itself. */ +header #session-chip { + display: inline-flex; + align-items: center; + gap: 0.4rem; + align-self: center; +} +header .sessionpart { + color: var(--muted); + font-size: 0.85em; + /* A long profile name is an operator's own filename and can be any length. Bounded here so a + * `keel-paper-hourly-experiment.db` cannot push the theme toggle off the header. */ + max-width: 12ch; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} +/* THE SEPARATORS. Generated rather than written into the markup, because each one belongs to the + * part beside it: `display: none` takes a hidden part's pseudo-element with it, so a name that + * never arrived cannot leave a lone middot behind. That is the pairing the `:empty` rule below + * exists to make -- the first cut of this file described these separators in three comments and + * shipped none, and the chip read "keel paper not recorded". + * + * `aria-hidden` is not available to a pseudo-element, so these are punctuation a screen reader + * may or may not announce -- which is why they are middots and not words: an announced "middot" + * is noise, an announced "or" would be a claim. */ +header #session-profile:not(:empty)::after { content: "\00B7"; margin-left: 0.4rem; } +header #session-equity:not(:empty)::before { content: "\00B7"; margin-right: 0.4rem; } +/* Hidden while empty, the same pairing the badge above uses and for the same reason: a lone + * separator beside a name that never arrived is a stray mark. */ +header .sessionpart:empty { display: none; } + +/* THE MODE BANNER (#704). + * + * Full-bleed and calm. It is a statement, not an alert: no `--bad`, no `--warn`, no icon, and + * nothing inside it that can be clicked. `MODE_CLASS` supplies the same emphasis the badge uses, + * so the banner and the badge cannot disagree about which mode is the quiet one -- paper reads + * muted on a muted ground, confirm carries the accent. + * + * Hidden while empty: an unreadable config costs the banner, because a banner is a claim about + * whether real money is involved and there is no safe default for that claim. */ +.modebanner { + margin: 0; + padding: 0.45rem 1rem; + border-bottom: 1px solid var(--line); + background: var(--surface); + font-size: 0.85rem; + letter-spacing: 0.01em; + text-align: center; +} +.modebanner:empty { display: none; } +.modebanner.mode-paper { color: var(--muted); } +.modebanner.mode-confirm, +.modebanner.mode-live { color: var(--accent); } + /* The toggle. It deliberately does NOT inherit the filled `button` rule: that one is for an * action that changes something on the server, and a filled teal disc beside the nav would be * the loudest thing in the header. `--control-line` for the border, not `--line`, for the diff --git a/keel/web/static/index.html b/keel/web/static/index.html index 4d0ca4c..90074c8 100644 --- a/keel/web/static/index.html +++ b/keel/web/static/index.html @@ -72,7 +72,14 @@ - + +
- + + + + + + +
+ +

+ +
+
+