diff --git a/README.md b/README.md index 45b43ec..cf28c4e 100644 --- a/README.md +++ b/README.md @@ -58,9 +58,9 @@ search result. under the project's ceiling, and no stale uncollected run. These are programs that refuse, not sentences in a prompt. - **Compute where you have it.** The same verbs — `submit`, `status`, `collect` - — against Hugging Face Jobs, any SSH host you can reach, and Kaggle's free - GPU/TPU, where an hours-based quota is enforced because a dollar ceiling - measures nothing. + — against Modal (T4 through B200, billed by the second), Hugging Face Jobs, any + SSH host you can reach, and Kaggle's free GPU/TPU, where an hours-based quota + is enforced because a dollar ceiling measures nothing. - **Evidence that outlives the session.** An append-only ledger of expectations and results. Replicated runs are compared interval against interval, so "matches the prediction" is a statement with a spread behind it. @@ -135,6 +135,7 @@ have no second rail. |---|---| | `hf_token` | Hugging Face Jobs | | `kaggle_key` + account | free GPU/TPU, rationed by the hour | +| `modal_token_id` + `modal_token_secret` | Modal sandboxes: T4 through B200, by the second | | SSH host or key | your own box | | `openrouter_key` | a second rail for reranking | | `context7_key`, `asta_api_key` | higher rate limits on docs and discovery | @@ -257,7 +258,7 @@ Each is a CLI with `--json` on every subcommand. | `paper_ingest` | arXiv LaTeX → section-aware chunks → the local index | | `nb` | persistent Jupyter kernel: `exec`, `verify`, `restart` | | `preflight` | the QA gate, and the record the submitters read | -| `jobs` / `gpu` / `kaggle` | the same verbs against three backends | +| `jobs` / `gpu` / `kaggle` / `modal` | the same verbs against four backends | | `ledger` | `expect`, `query`, `verdict`, `falsify`, `abandon`, `verify` | | `quota` / `budget` | what was spent, and what may still be | | `evolve` | evolutionary search as a budgeted campaign | diff --git a/agent.py b/agent.py index 2b42d59..3af3be3 100644 --- a/agent.py +++ b/agent.py @@ -179,6 +179,10 @@ def build_options( "disallowed_tools": DENIED_TOOLS, "permission_mode": mode, "cwd": str(paths.root()), + # The other half of `cwd`. Without it the agent's shell resolves `python` + # through the launcher's ambient PATH, which on this platform is not the + # environment Grad is running in -- see `interpreter_env`. + "env": interpreter_env(), "hooks": hook_matchers, # Off by default in the SDK, and the default is why an answer used to # arrive in one lump: without it `receive_response` yields nothing until @@ -188,6 +192,7 @@ def build_options( "include_partial_messages": True, } options.update(rewind_option(sdk, resume_at, drops_turn)) + options.update(checkpointing_option(sdk)) options.update(thinking_option(cfg, sdk)) # How hard it thinks, from `core/effort.py`. Applied here rather than # anywhere later because there is nowhere later: the SDK exposes no control @@ -198,6 +203,101 @@ def build_options( return sdk.ClaudeAgentOptions(**options) +def interpreter_env() -> dict[str, str]: + """The environment the agent's shell runs in, so that `python` means this one. + + `cwd` was the only thing this session told its shell about the world, and + `PATH` is the other half of that sentence. A `Bash` call inherits the + launcher's ambient environment, and `core/spawn.py:console_script` already + documents what that means on the platform this ships to: Grad is started + from a shortcut pointing at `.venv\\Scripts\\pythonw.exe`, Explorer hands it + the machine's environment, and so the interpreter is the venv's while `PATH` + is not. That reasoning was applied to `shutil.which` there and never to the + shell the model types into. + + **What it costs is worse than a missing package.** Every tool in the system + prompt is spelled `python -m tools.`, so the resolution of the bare + word `python` decides *which Grad* the agent's own instrument panel is. On a + machine carrying a second one on `PATH` -- a stale global install, an + editable checkout -- the answer is not this one, and the app then reads the + workspace through one installation while the agent writes it through + another. Nothing in either would ever name that disagreement. + + So the interpreter running this process wins, and it wins by being *first* + rather than by being alone: `PATH` is prepended to, never replaced, because + the agent legitimately needs `git`, `docker` and `latexmk` and this is not + the place to decide what a research machine has on it. + + Three details are load-bearing. + + `sysconfig` is asked for the scripts directory rather than taking + `sys.executable`'s parent, because the two differ on a non-venv install: + `python.exe` sits in `C:\\Python314` and `pip.exe` in `C:\\Python314 + \\Scripts`, and a fix that does not cover `pip` does not cover the command + people actually get wrong. + + Nothing here is venv-specific, deliberately. The invariant is "the agent's + `python` is the `python` running Grad", which is the right one under a venv, + a conda environment or a bare system install -- and a check for a venv would + turn the third case into a silent no-op. + + `PYTHONPATH` is set only when Grad is not an installed distribution, which + is the checkout someone runs with `python agent.py` and never pip-installed. + Unconditionally exporting the install directory would put `config`, `data`, + `notes` and `figures` on the import path as namespace packages, and `import + data` is a thing research code genuinely does. + + `PYTHONUTF8` comes from `core/spawn.py:utf8_env`, which explains at length + why reading arXiv LaTeX on a Windows machine crashes twice over and why + remembering `encoding="utf-8"` is not enough to stop it. It is here rather + than in the prompt for the reason the gates are programs rather than + sentences: the model does get this right most of the time, and most of the + time is not a property. + """ + import sysconfig # noqa: PLC0415 - only this function needs it + + from core import spawn # noqa: PLC0415 + + scripts = sysconfig.get_path("scripts") or str(Path(sys.executable).parent) + ambient = os.environ.get("PATH", "") + # Prepend rather than append, and skip the work when it is already in front: + # a session rebuilt by a compaction runs this again, and PATH should not grow + # a copy of the same directory once per compaction. + parts = ambient.split(os.pathsep) if ambient else [] + if not parts or Path(parts[0] or ".") != Path(scripts): + ambient = os.pathsep.join([scripts, *parts]) if parts else scripts + env = {"PATH": ambient, **spawn.utf8_env()} + + # `pip` and `uv` both read this, and an ambient one pointing at a *different* + # environment is the exact confusion this function exists to end -- so it is + # set to what is true here rather than left to whatever Explorer passed in. + if sys.prefix != sys.base_prefix: + env["VIRTUAL_ENV"] = sys.prefix + + if not _installed_as_distribution(): + install = str(paths.install_dir()) + existing = os.environ.get("PYTHONPATH", "") + env["PYTHONPATH"] = os.pathsep.join([install, existing]) if existing else install + return env + + +def _installed_as_distribution() -> bool: + """Is this Grad on the interpreter's path by installation rather than by cwd? + + False is the safe answer on any failure: it adds `PYTHONPATH`, and a + redundant entry costs nothing next to an agent whose every tool call raises + `ModuleNotFoundError` because its cwd is the workspace and the workspace has + no `tools/` in it. + """ + try: + import importlib.metadata as md # noqa: PLC0415 + + md.distribution("grad") + return True + except Exception: # noqa: BLE001 - see the docstring + return False + + def rewind_option(sdk: Any, resume_at: str | None, drops_turn: str | None) -> dict[str, Any]: """Where a resumed conversation should stop loading, when a rewind says so. @@ -224,6 +324,55 @@ def rewind_option(sdk: Any, resume_at: str | None, drops_turn: str | None) -> di return options +def checkpointing_option(sdk: Any) -> dict[str, Any]: + """Ask the CLI to keep a copy of a file before the agent changes it. + + This is the third half of a rewind. `core/rewind.py` moves the transcript, + `resume_session_at` moves the model's memory, and until this flag neither of + them moved the *work*: a turn that rewrote a training script and was then + rewound left the rewritten script on disk, with a conversation that no longer + contained the instruction that produced it. The two surfaces the user can see + agreed with each other and disagreed with the filesystem, which is the worst + of the three arrangements. + + Feature-detected, like `thinking_option` and `rewind_option` above, and for + the same reason: this option is newer than most of what this file passes, and + an SDK without it must give a session that cannot restore files rather than + one that cannot be built. + + **What it does not cover.** The backups are taken by the CLI around its own + file-editing tools, so `Write` and `Edit` are the case it is for. Most of + what this agent does is a `Bash` command -- and a file a *command* wrote is + outside that, as is anything on the other side of a submitter. That is not a + gap worth closing here: the ledger is append-only precisely so the record of + a run cannot be rewound, and a rewind that reached into `ledger/runs.jsonl` + would be undoing evidence rather than work. `ui/app.py:rewind_to` words its + result on what actually moved rather than on what was asked for. + + Deliberately not combined with `session_store`, which the SDK rejects + outright (`_internal/session_store_validation.py`). Nothing here sets one. + """ + if "enable_file_checkpointing" not in _option_fields(sdk): + return {} + return {"enable_file_checkpointing": True} + + +def checkpointing_supported(sdk: Any = None) -> bool: + """Can this SDK put files back? Asked before anything promises it. + + Separate from the option for the reason `rewind_supported` is separate from + `rewind_option`: the answer decides what the rewind's result *says*, and a + message claiming the work went back when it did not is the failure + `core/rewind.py` is written to make impossible. + """ + if sdk is None: + try: + sdk = _sdk() + except BaseException: # noqa: BLE001 - `_sdk` exits rather than raising ImportError + return False + return "enable_file_checkpointing" in _option_fields(sdk) + + def _option_fields(sdk: Any) -> set[str]: """The option names this SDK accepts, or nothing if it cannot be asked.""" try: @@ -319,8 +468,19 @@ def preflight_environment() -> dict[str, Any]: hydrated = credentials.hydrate_environment() cfg = config_mod.load() project_id = budget.current_project() + shell = interpreter_env() return { "removed_env": removed, + # What the agent's own `python` resolves to, which is the diagnostic that + # would have found the defect this reports on: `prompts/system.md` spells + # every tool `python -m tools.`, so a second Grad earlier on PATH + # means the app and the agent are reading the same workspace through + # different installations. Names and booleans only -- this output goes + # into bug reports. + "interpreter": sys.executable, + "shell_path_head": shell["PATH"].split(os.pathsep)[0], + "venv": sys.prefix if sys.prefix != sys.base_prefix else None, + "installed_as_distribution": _installed_as_distribution(), "oauth_token_present": bool(os.environ.get("CLAUDE_CODE_OAUTH_TOKEN")), "oauth_token_source": ( "environment" if ambient else ("credential store" if hydrated else "absent") diff --git a/config/grad.toml b/config/grad.toml index 448b23c..6317e9f 100644 --- a/config/grad.toml +++ b/config/grad.toml @@ -229,6 +229,68 @@ Tpu1VmV38 = "tpu" TpuV5E8 = "tpu" TpuV6E8 = "tpu" +[modal] +# Modal runs the pipeline in a Sandbox: a container started from a registry +# image with the code copied in, an entrypoint, and a GPU. It is the one backend +# here that bills per *second* against a published rate table, which is why it +# needs none of Kaggle's hours machinery -- the [spend] ceilings above are +# already the right instrument, and `collect` prices the real elapsed seconds. +# +# Authentication is a token pair, both halves secret, both in the OS credential +# store: +# python -m tools.jobs credential set modal_token_id +# python -m tools.jobs credential set modal_token_secret +# python -m tools.modal account --check --json +# +# Unlike every other backend here, the secret never enters a child process: +# `modal.Client.from_credentials` takes it as an argument and sends it as a gRPC +# header. There is nothing to scrub because nothing is exported. +default_gpu = "H100" +# The Modal App every sandbox is created under. One name rather than one per +# run, so the Modal dashboard groups a project's work the way the ledger does. +app_name = "grad" +# Where the sandbox writes what `collect` reads back. A Modal Volume, because a +# sandbox's filesystem is gone once it exits and `metrics.json` has to outlive +# it -- see `tools/modal.py`. Created on first use. +volume_name = "grad-runs" +mount_path = "/grad/out" +# Where the pipeline is copied to inside the image, and the working directory +# the entrypoint runs in. +workdir = "/grad/pipeline" +poll_interval_s = 20 +# Modal's own ceiling on a Sandbox's lifetime is 24 hours and it is hard: the +# container is killed there whatever the job was doing. This is the local bound, +# taken from the spec's `[estimate] hours` plus a margin, and it refuses a spec +# asking for more rather than starting a run that cannot finish. +max_hours = 24.0 +timeout_margin = 1.25 + +[modal.gpu_rates] +# Dollars per *hour*, converted from Modal's published per-second prices (H100 is +# $0.001097/s = $3.9492/h). Per hour rather than per second because that is the +# unit the rest of this file and every estimate in a spec already use. +# +# `collect` prices the sandbox's real start and end against this table, so a rate +# that is stale in the optimistic direction makes the ceiling decoration. Modal +# publishes these at modal.com/pricing; check them when a new card appears. +"T4" = 0.5904 +"L4" = 0.7992 +"A10G" = 1.1016 +"L40S" = 1.9512 +"A100-40GB" = 2.0988 +"A100-80GB" = 2.4984 +"H100" = 3.9492 +"H200" = 4.5396 +"B200" = 6.2496 +# +# GPU time only. Modal also bills for the CPU and memory a sandbox holds +# ($0.0000131/core/s and $0.00000222/GiB/s), and those are deliberately not in +# this table: `tools/modal.py` passes no `cpu=` or `memory=`, so the allocation +# is Modal's default and this side does not know what it is. A rate that cannot +# be multiplied by a quantity is not a price, and two entries sitting here +# unused would read as if they were being charged. They are small next to an +# H100 and they are not zero, so the run record says the cost excludes them. + # SSH hosts are a fixed inventory; an unknown name is a configuration error, # never an ad-hoc connection. Uncomment and fill in for a real host. # diff --git a/config/jupyter/custom/custom.css b/config/jupyter/custom/custom.css index 22464f4..c4a3cd1 100644 --- a/config/jupyter/custom/custom.css +++ b/config/jupyter/custom/custom.css @@ -201,4 +201,5 @@ body, .jp-Notebook, .jp-NotebookPanel { background: #F7F3E8; } .cm-operator, .cm-punctuation { color: #8A8272; } .cm-editor .cm-ruler { border-right: 1px dashed rgba(20,16,12,0.3); } .cm-cursor { border-left: 2px solid #14100C; } -.cm-editor .cm-selectionBackground { background: #FFD400 !important; } +.cm-editor .cm-selectionBackground { background: #FFD400 !important; + color: #14100C !important; } diff --git a/core/config.py b/core/config.py index 7161f1c..d937578 100644 --- a/core/config.py +++ b/core/config.py @@ -225,6 +225,40 @@ "a100-large": 4.13, }, }, + # Modal Sandboxes: the fourth submitter, and the first whose billing model + # the §6 dollar ceilings were already the right instrument for. Modal charges + # per second against a published table, so unlike Kaggle there is nothing + # here to ration in another unit -- `[spend]` is the gate, and `collect` + # prices the elapsed time against `gpu_rates` below. + "modal": { + "default_gpu": "H100", + "app_name": "grad", + # A Volume, because a Sandbox's filesystem does not outlive it and + # `collect` runs after it has exited. See `tools/modal.py`. + "volume_name": "grad-runs", + "mount_path": "/grad/out", + "workdir": "/grad/pipeline", + "poll_interval_s": 20, + # Modal kills a Sandbox at 24 hours whatever it was doing, so a spec + # asking for more is refused rather than started. + "max_hours": 24.0, + "timeout_margin": 1.25, + # Dollars per hour, from Modal's published per-second prices. An + # accelerator absent from this table is refused at submit rather than + # booked at zero: `[spend]` is this backend's only gate, and a ceiling + # that cannot price a run is not bounding it. + "gpu_rates": { + "T4": 0.5904, + "L4": 0.7992, + "A10G": 1.1016, + "L40S": 1.9512, + "A100-40GB": 2.0988, + "A100-80GB": 2.4984, + "H100": 3.9492, + "H200": 4.5396, + "B200": 6.2496, + }, + }, # Kaggle kernels: a third submitter, and the first whose scarce resource is # not money. Every run costs $0.00, so the §6 dollar ceilings can never # refuse one -- which would make them decoration on this backend rather than @@ -802,6 +836,28 @@ def _validate(cfg: Config, path: Path) -> None: "[hf.flavor_rates] must be a table of flavor -> dollars per hour", fix=f"fix the [hf.flavor_rates] section in {path}", ) + modal_rates = cfg.get("modal", "gpu_rates", {}) + if not isinstance(modal_rates, dict): + raise ConfigError( + "[modal.gpu_rates] must be a table of GPU name -> dollars per hour", + fix=f"fix the [modal.gpu_rates] section in {path}", + ) + for name, value in modal_rates.items(): + # Checked here rather than at submit, where the failure would be a run + # that got as far as the ceiling before anything noticed the ceiling + # could not be computed. + # + # `_check_number` rather than a bespoke test, for the reason its own + # docstring gives about `[kaggle.quota]`: a second copy is a second + # place to drift, and this one had already drifted -- it caught bools + # and non-numbers and let `nan` through, which TOML has a literal for + # and which makes every comparison against a ceiling false. + _check_number(value, f"modal.gpu_rates.{name}", path) + if value < 0: + raise ConfigError( + f"[modal.gpu_rates] {name} must not be negative", + fix=f'write it as "{name}" = 3.9492 in {path}', + ) cfg.hosts # noqa: B018 - raises ConfigError on a malformed inventory diff --git a/core/credentials.py b/core/credentials.py index 5382605..9ec246e 100644 --- a/core/credentials.py +++ b/core/credentials.py @@ -59,6 +59,20 @@ # belongs in a file you can read; the thing that authorises it does not. KAGGLE_KEY = "kaggle_key" +# The ninth and tenth, and the fourth path that can reach a machine. Modal +# authenticates with a token *pair* and both halves are secret -- unlike Kaggle, +# where the username is a name and only the key authorises, and unlike HF, where +# the namespace is config. `ak-...` identifies the token rather than the account, +# so there is nothing here worth putting in a file you can read. +# +# Both are stored rather than exported. `modal.Client.from_credentials` takes +# them as arguments and sends them as gRPC headers, so unlike every other +# backend in this project the secret never has to enter a child's environment at +# all -- which is the strongest form of what §9 asks for, and the reason +# `tools/modal.py` never sets MODAL_TOKEN_ID. +MODAL_TOKEN_ID = "modal_token_id" +MODAL_TOKEN_SECRET = "modal_token_secret" + #: Every credential this project knows, in one tuple so nothing derived from it #: can be added to and then forgotten. `status()` reports these, #: `tools/jobs.py` accepts these, and `scrub_environment` removes the `GRAD_*` @@ -74,6 +88,8 @@ CLAUDE_TOKEN, ASTA_KEY, KAGGLE_KEY, + MODAL_TOKEN_ID, + MODAL_TOKEN_SECRET, ) diff --git a/core/projects.py b/core/projects.py index 3b50619..a8af741 100644 --- a/core/projects.py +++ b/core/projects.py @@ -36,6 +36,7 @@ from __future__ import annotations import hashlib +import logging import re from pathlib import Path from typing import Any @@ -379,6 +380,16 @@ def sync(project_id: str, *, force: bool = False) -> dict[str, Any]: for name, body in bodies.items(): _write(directory / name, _with_marker(GENERATED[name], body)) written.append(name) + # The third workspace checkpoint. `sync` is run after a collect or a verdict + # and rewrites the three files a human actually reads, so this is the commit + # whose diff shows what changed about the *project* rather than about the + # ledger. Never raises -- see `core/vcs.py`. + try: + from core import vcs # noqa: PLC0415 + + vcs.checkpoint(f"sync {project_id}") + except Exception: # noqa: BLE001 - a history is never worth a failed command + logging.getLogger("grad.vcs").debug("checkpoint skipped", exc_info=True) return { "project": project_id, "dir": str(directory), diff --git a/core/rewind.py b/core/rewind.py index 81b5d77..f8dca03 100644 --- a/core/rewind.py +++ b/core/rewind.py @@ -7,13 +7,28 @@ every turn for the rest of the session. Nothing was wrong with the question; the only way to ask it cleanly was to start a new session and lose the thread. -**A rewind is two operations and they fail independently.** Dropping records +**A rewind is three operations and they fail independently.** Dropping records from the transcript is ours and always works. Putting the *model's* memory back is the SDK's: it needs an anchor -- the uuid of the last transcript entry of the last turn being kept -- handed to `resume_session_at` when the client is rebuilt. When there is no usable anchor the transcript rewinds anyway and the agent goes on remembering the dropped turns. +Putting the *work* back is the SDK's too, by a different route: with +`enable_file_checkpointing` on, `client.rewind_files(uuid)` restores files to +their state at a given user message. It is a control request rather than an +option, so it is the one half that has to happen while the client is still +alive -- `ui/app.py:rewind_to` does it before `close()`, and doing it after +would have made it the half that silently never ran. + +That third one is narrower than the other two and the wording everywhere is +careful about it. The CLI checkpoints around its own editing tools, so a rewind +returns what `Write` and `Edit` changed; a file a `Bash` command wrote stays +written. For this agent that is most of them, and it is the right boundary +rather than a shortfall: the ledger is append-only precisely so a run cannot be +un-recorded, and an undo that reached into `ledger/runs.jsonl` would be erasing +evidence rather than work. + That degradation is deliberate, and it is reported rather than hidden, because it is the same split `ui/sessions.py` draws between resuming a conversation and redisplaying a transcript: two promises, and the one nobody can see is the one @@ -126,15 +141,27 @@ def plan( def record( - *, dropped: list[dict[str, Any]], resumed: bool, anchor: str | None = None + *, + dropped: list[dict[str, Any]], + resumed: bool, + anchor: str | None = None, + files: bool = False, ) -> dict[str, Any]: """The transcript entry a rewind leaves in place of what it dropped. A record rather than a silent truncation, for the reason a compaction leaves one: a transcript that quietly loses turns is indistinguishable from one that never had them, and the next confusing answer has nothing to point at. This - also makes a rewind honest about the half of itself that can fail -- the - marker says whether the agent's memory came back with the screen. + also makes a rewind honest about the halves of itself that can fail -- the + marker says whether the agent's memory came back with the screen, and whether + the work did. + + `files` is stated only when it is true. The common rewind restores none -- + most turns edit nothing -- and a marker reading "no files were restored" + describes a failure of something that was never attempted. What it claims is + also deliberately narrow: the CLI checkpoints around its own editing tools, + so this is `Write` and `Edit`, not what a `Bash` command wrote and not + anything a submitter did on a backend. See `agent.checkpointing_option`. `dropped` is carried whole, blocks and tool calls included, so the file keeps everything the conversation no longer does. @@ -152,6 +179,11 @@ def record( "back, so the agent still remembers them and the next turn still pays " "for them." ) + if files: + tail += ( + " Files the agent edited were restored to their state before the first " + "dropped prompt; anything a command wrote was not." + ) return { "role": "system", "kind": MARK_KIND, @@ -162,6 +194,9 @@ def record( # back. It is what says *which* conversation the rewind was against when # a transcript has been through several. "anchor": anchor, + #: Whether the work moved with the conversation. Third of the three + #: claims a rewind makes, and the newest. + "files": bool(files), } diff --git a/core/settings.py b/core/settings.py index 0e92279..2842e65 100644 --- a/core/settings.py +++ b/core/settings.py @@ -52,7 +52,7 @@ #: #: In `core/` rather than in the tool, because a *setting* naming a backend is #: read by the config layer, and `core` importing `tools` is backwards. -BACKENDS: tuple[str, ...] = ("ssh", "hf_jobs", "kaggle") +BACKENDS: tuple[str, ...] = ("ssh", "hf_jobs", "kaggle", "modal") #: The models the setup window offers as buttons. **Not a restriction.** #: `set_models` takes any non-empty string, because a hardcoded list of model ids @@ -211,6 +211,44 @@ def set_backend(name: str, root: Path | None = None) -> dict[str, Any]: return _write(document, root) +# --------------------------------------------------------------------------- +# theme +# --------------------------------------------------------------------------- +#: The palettes `ui/tokens.py` ships. Spelled out here rather than imported, +#: because `core` importing `ui` is backwards -- the same reason `BACKENDS` is a +#: tuple here rather than a reference to `tools/evolve.py`. `tests/ +#: test_settings.py` asserts the two lists still agree. +THEMES: tuple[str, ...] = ("light", "dark") + + +def theme(root: Path | None = None) -> str: + """Which palette the workspace draws in. `light` unless it was changed. + + Per workspace, like everything else in this overlay, and that is the useful + scope rather than an accident of where it landed: the theme is read by the + splash process before any window exists, and a splash that flashed cream + before a dark workspace painted would be the one frame the whole setting is + judged on. + + Never raises and never returns something `ui/tokens.py` cannot resolve: a + value written by a newer version falls back to the default there too. + """ + chosen = str(load(root).get("theme") or "").strip().lower() + return chosen if chosen in THEMES else "light" + + +def set_theme(name: str, root: Path | None = None) -> dict[str, Any]: + chosen = str(name or "").strip().lower() + if chosen not in THEMES: + raise UsageError( + f"unknown theme {name!r}", + fix=f"themes are: {', '.join(THEMES)}", + ) + document = load(root) + document["theme"] = chosen + return _write(document, root) + + # --------------------------------------------------------------------------- # the agent's own knobs # --------------------------------------------------------------------------- diff --git a/core/spawn.py b/core/spawn.py index 00ae2ca..2447c6f 100644 --- a/core/spawn.py +++ b/core/spawn.py @@ -80,6 +80,52 @@ def detached() -> dict[str, Any]: return {"start_new_session": True} +def utf8_env() -> dict[str, str]: + """Environment for a Python child that will meet text from the internet. + + Python's `open()` defaults to `locale.getpreferredencoding()`, which on + Windows is the ANSI code page rather than UTF-8. On the machine this was + found on that is **cp1251** -- a Cyrillic code page, on a machine doing + English-language ML research -- because the ANSI code page follows the + system locale and has nothing to do with what the files contain. + + What the files contain is arXiv LaTeX, and there are two failures, one of + which survives doing the obvious thing right: + + open(tex).read() + UnicodeDecodeError: 'charmap' codec can't decode byte 0x98 + + print(open(tex, encoding='utf-8').read()) + UnicodeEncodeError: 'charmap' codec can't encode character '\\xf6' + + The first is a curly quote -- `\\u2018` is `e2 80 98` in UTF-8, and cp1251 + has no character at 0x98. The second is the one worth the paragraph: the + read was *correct*, and the crash moved to `print`, because the standard + streams take their encoding from the same code page. Remembering + `encoding="utf-8"` on every open is not sufficient and never was. + + `PYTHONUTF8=1` is PEP 540's UTF-8 Mode and answers both: it makes + `getpreferredencoding` return utf-8, so `open()` defaults to it, and it + reconfigures stdin/stdout/stderr to utf-8 with `surrogateescape`. + + **`PYTHONIOENCODING` is set too, and the error handler is why.** UTF-8 Mode + alone looked sufficient and is not: `PYTHONIOENCODING` takes precedence over + it for the standard streams, so an ambient one -- a stray export, a shell + profile, a CI image -- silently defeats half the fix and leaves the `print` + failure above exactly as it was. Setting it here overrides that. + + It is spelled `utf-8:surrogateescape` rather than `utf-8` for the reason + that made leaving it out tempting: the bare form defaults the handler to + `strict`, which turns a byte that survived a lossy read into a crash on the + way out. Naming the handler keeps UTF-8 Mode's behaviour instead of + replacing it with a stricter one. + + Nothing here is Windows-only. A Linux box with `LC_ALL=C` has the same + problem in ASCII, and the fix is the same two variables. + """ + return {"PYTHONUTF8": "1", "PYTHONIOENCODING": "utf-8:surrogateescape"} + + def run(argv: list[str], **kwargs: Any) -> subprocess.CompletedProcess: """`subprocess.run`, without a window. Callers pass everything else.""" return subprocess.run(argv, **{**quiet(), **kwargs}) diff --git a/core/submit.py b/core/submit.py index 4869614..f4842eb 100644 --- a/core/submit.py +++ b/core/submit.py @@ -239,6 +239,7 @@ def attach_handle(run_id: str, handle: dict[str, Any]) -> None: "hf_jobs": "python -m tools.jobs collect", "kaggle": "python -m tools.kaggle collect", "ssh": "python -m tools.gpu collect", + "modal": "python -m tools.modal collect", } @@ -535,9 +536,39 @@ def finish( record["samples"] = replicated ls.append_run_event(record) archive_quietly(run_id) + # A collected run is the first of the three moments the workspace history + # marks. After the ledger write, never before: a checkpoint that ran first + # would commit a state the ledger does not yet describe, and the ordering + # matters more than it looks because this is the record the commit exists to + # preserve. Failure here is swallowed by `checkpoint` itself -- see + # `core/vcs.py` for why a wedged git must not fail a collect. + checkpoint_workspace(f"collected {run_id} ({status})") return record +def checkpoint_workspace(reason: str) -> None: + """Commit the workspace, if it is versioned at all. Never raises. + + A thin wrapper rather than a direct call so the import stays off the hot + path of every module that already imports this one, and so the three call + sites read as one decision rather than three. + """ + log = logging.getLogger("grad.vcs") + try: + from core import vcs # noqa: PLC0415 + + result = vcs.checkpoint(reason) + except Exception: # noqa: BLE001 - a history is never worth a failed command + log.debug("checkpoint skipped", exc_info=True) + return + # `checkpoint` reports rather than raises, so its `error` reaches nothing + # unless it is read here. Logged without `exc_info`, because there is no + # exception -- git ran and said no, and a traceback would describe a stack + # that has nothing to do with why. + if result.get("error"): + log.debug("workspace checkpoint failed: %s", result["error"]) + + def archive_quietly(run_id: str) -> dict[str, Any] | None: """Snapshot a terminal run into the cross-workspace archive. diff --git a/core/vcs.py b/core/vcs.py new file mode 100644 index 0000000..cad09b9 --- /dev/null +++ b/core/vcs.py @@ -0,0 +1,402 @@ +"""Version control for the *workspace*: what it tracks, and when it commits. + +The ledger is append-only, so a collected run cannot be un-recorded by anything +in this system working as designed. What is genuinely at risk is everything +around it -- `notes/`, a project's `MEMORY.md` and `PLAN.md`, the pipeline code +the agent is editing, a report's `.tex` -- all of which are ordinary files that +an ordinary mistake can truncate or delete. `agent.checkpointing_option` covers +the span of one session; this is what covers the span of a project. + +Four decisions worth knowing before changing anything here. + +**It is initialised deliberately and never automatically.** Creating a git +repository inside somebody's folder is a side effect they did not ask for, and a +research workspace can already be inside one -- a shared drive, a dotfiles +repository, somebody's own versioning. `initialise` is a command and a setup +step; everything after it is automatic. + +**It refuses to run when the workspace is the installation.** The default +workspace *is* the checkout, and Grad's own source is already versioned there: +auto-committing would put a user's notebooks on the same branch as upstream's +releases and turn every `grad update` into a merge, which is the exact failure +`tools/workspace.py:move` exists to prevent. The refusal names `move`. + +**It commits at events, not at writes.** A commit per file write would produce +hundreds a session and a history nobody can read. The boundaries are the ones +the system already treats as meaningful -- a run collected, a verdict recorded, +a project's documents regenerated -- so the log reads as a record of the +research rather than of the editor. + +**Nothing here may fail a caller.** `checkpoint` is called just after the ledger +has been written, and a git that is missing, wedged, or mid-rebase must not turn +a successful collect into a failed command. Every entry point returns a result +document with an `error` field instead of raising. + +There is no remote, and adding one is not a small change: pushing a research +workspace publishes the pipeline, the data pointers and any credential that +found its way into a notebook. The ignore list below is written to make that +survivable if it is ever added, but the argument in `config/grad.toml` for +`is_private = true` applies with more force here. +""" + +from __future__ import annotations + +import logging +from pathlib import Path +from typing import Any + +from core import paths, version + +log = logging.getLogger("grad.vcs") + +#: Set on the repository at `initialise`, and the thing `enabled()` tests. +#: +#: A marker in the repository's own config rather than a file in the workspace: +#: it travels with the repository, it cannot be mistaken for research, and it +#: answers the question that actually matters -- "is this a repository Grad made +#: and may therefore commit to" -- rather than "is there a `.git` here", which +#: is true of a workspace somebody keeps inside their own dotfiles. +MARKER = "grad.workspace" + +#: The commit identity used when the machine has none configured. +#: +#: Only when there is none: a user with a global `user.email` gets their own, +#: because these are commits in their research folder and attributing them to a +#: tool would be wrong. Git refuses to commit at all without one, and "your +#: research is not being checkpointed because git has never been configured on +#: this machine" is a bad way to find out. +FALLBACK_NAME = "Grad" +FALLBACK_EMAIL = "grad@localhost" + +#: What a workspace repository tracks, and what it must not. +#: +#: Lifted from the repository's own `.gitignore`, which had already worked this +#: out: the JSONL ledgers are tracked *because* they are the source of truth and +#: are meant to be human-diffable, and the SQLite indexes beside them are not, +#: because they are derived from the JSONL and rebuildable at any time. The two +#: rules that matter most are the last two -- a corpus of downloaded papers is +#: large and re-fetchable, and a credential is never a thing to commit. +IGNORE = """\ +# Written by `grad-workspace vcs init`. Safe to edit: nothing regenerates it. +# +# The JSONL ledgers ARE tracked, deliberately. They are the source of truth for +# every run, expectation and verdict, they are append-only, and they diff +# usefully line by line -- which is the whole reason this repository is worth +# having. The SQLite files beside them are derived from those ledgers and are +# rebuilt on demand, so they would only ever be merge conflicts. +ledger/ledger.sqlite +ledger/ledger.sqlite-* +data/corpus.sqlite +data/corpus.sqlite-* + +# Downloaded papers: large, and re-fetchable from the arXiv ids in the ledger. +data/papers/ + +# Collected run artifacts. The run *record* is in ledger/runs.jsonl and is +# tracked; the checkpoints and logs it points at can be hundreds of megabytes. +ledger/runs/ + +# Figures are outputs. They are regenerated by re-running the notebook that drew +# them, and a binary that changes on every run is the worst thing to put in a +# diff. A figure that made it into a paper is referenced from reports/ instead. +figures/ + +# Live machine state, not research: a Lab server's port, pid and token; the +# selected project, which is one machine's choice rather than a fact about the +# work; JupyterLab's own generated runtime files. +data/lab/ +data/wiki/ +ledger/.current_project +config/jupyter/lab/ +config/jupyter/migrated + +# LaTeX build byproducts. The .tex, claims.json and references.bib ARE tracked: +# they are the checkable artifacts. +reports/**/*.pdf +reports/**/*.aux +reports/**/*.bbl +reports/**/*.blg +reports/**/*.fls +reports/**/*.log +reports/**/*.out +reports/**/*.fdb_latexmk +reports/**/claims.tex + +# Never, under any circumstances. Credentials live in the OS credential store; +# this is belt and braces for the one that gets written to a file by a library +# that did not ask. +.env +*.pem +credentials.json +kaggle.json +.grad-workspace.json + +# Python and editor litter. +__pycache__/ +*.py[cod] +.ipynb_checkpoints/ +.venv/ +venv/ +""" + + +def root() -> Path: + return paths.root() + + +def _git(*args: str, timeout: float = 15.0) -> str | None: + return version.git(*args, cwd=root(), timeout=timeout) + + +def _result(*args: str, timeout: float = 15.0) -> Any: + return version.git_result(*args, cwd=root(), timeout=timeout) + + +def is_repository() -> bool: + """Is the workspace the top of a git repository? + + `--show-toplevel` rather than `--git-dir`, and the comparison is the point: + a workspace *inside* somebody's repository answers yes to "are you in a + repository" and must still answer no here. Committing there would sweep up + whatever else that repository holds. + """ + top = _git("rev-parse", "--show-toplevel") + if not top: + return False + try: + return Path(top.strip()).resolve() == root().resolve() + except OSError: + return False + + +def enabled() -> bool: + """May `checkpoint` commit here? Never raises.""" + try: + if root().resolve() == paths.install_dir().resolve(): + return False + except OSError: + return False + if not is_repository(): + return False + return (_git("config", "--local", "--get", MARKER) or "").strip() == "true" + + +def _refuse_if_installation() -> str | None: + """The one refusal that is about the *layout* rather than about git.""" + try: + same = root().resolve() == paths.install_dir().resolve() + except OSError: + return None + if not same: + return None + return ( + "the workspace is the installation folder, and versioning it would put your " + "research on the same branch as Grad's own source -- which is what makes an " + "update a merge. Move the research out first." + ) + + +def initialise(*, identity: bool = True) -> dict[str, Any]: + """Create the repository, write the ignore list, make the first commit. + + Idempotent in the way that matters: a workspace that is already a marked + repository is reported as such and nothing is rewritten. A workspace that is + a repository somebody *else* made is refused rather than adopted -- the + marker is what separates the two, and quietly taking over a repository is + not a thing to do to somebody's folder. + """ + out: dict[str, Any] = { + "root": str(root()), + "created": False, + "already": False, + "error": None, + "fix": None, + } + refusal = _refuse_if_installation() + if refusal: + out["error"] = refusal + out["fix"] = "python -m tools.workspace move --to " + return out + + if version.git("--version", cwd=root()) is None: + out["error"] = "git is not available on this machine" + out["fix"] = "install git, or leave the workspace unversioned" + return out + + if is_repository(): + if enabled(): + out["already"] = True + return out + out["error"] = ( + "the workspace is already a git repository that Grad did not create. " + "Adopting it would mean committing to a history that is not ours." + ) + out["fix"] = f"git -C {root()} config --local {MARKER} true # to opt in deliberately" + return out + + if not _ok(_result("init")): + out["error"] = "git init failed" + return out + _git("config", "--local", MARKER, "true") + if identity: + _ensure_identity() + + _write_ignore(root() / ".gitignore") + + out["created"] = True + first = checkpoint("workspace initialised", force=True) + out["commit"] = first.get("commit") + out["error"] = first.get("error") + return out + + +#: Fences the block this module manages, so it can be added to a file somebody +#: else wrote without being written twice on the next `init`. +IGNORE_BEGIN = "# --- grad: managed, do not edit between these markers ---" +IGNORE_END = "# --- end grad ---" + + +def _ok(result: Any) -> bool: + """Did a git command actually succeed? + + `None` means it never ran; a non-zero return code means it ran and refused, + and the two were being conflated. `if _result("init") is None` treated a + *failed* `git init` as a success and went on to configure and commit into a + directory that is not a repository -- which fails again, later, somewhere + less obvious. + """ + return result is not None and getattr(result, "returncode", 1) == 0 + + +def _write_ignore(path: Path) -> None: + """Put the managed rules in, without touching anybody else's. + + A workspace can already have a `.gitignore` -- somebody's own, or one left + by a checkout it used to be. The first version skipped the file entirely in + that case, which meant `.env`, `kaggle.json` and `data/papers/` were not + excluded and the very first checkpoint committed them. A credential in a + commit survives the file being deleted, so this is the one rule in here + worth being careful about. + + Appended between markers rather than merged line by line, so a second `init` + replaces the block instead of duplicating it, and so anything the user wrote + is visibly not ours. + """ + block = f"{IGNORE_BEGIN}\n{IGNORE}{IGNORE_END}\n" + try: + existing = path.read_text(encoding="utf-8") if path.exists() else "" + except OSError: + existing = "" + if IGNORE_BEGIN in existing: + head, _, rest = existing.partition(IGNORE_BEGIN) + _, _, tail = rest.partition(IGNORE_END) + existing = head + tail.lstrip("\n") + prefix = existing.rstrip("\n") + "\n\n" if existing.strip() else "" + path.write_text(prefix + block, encoding="utf-8") + + +def _ensure_identity() -> None: + """Give git an identity only if the machine has not got one.""" + if (_git("config", "--get", "user.email") or "").strip(): + return + _git("config", "--local", "user.name", FALLBACK_NAME) + _git("config", "--local", "user.email", FALLBACK_EMAIL) + + +def checkpoint(reason: str, *, force: bool = False) -> dict[str, Any]: + """Commit whatever has changed, if anything has. Never raises. + + Called from the paths that have just recorded something durable, so the two + properties it needs are that it is cheap when there is nothing to do and + that it cannot fail its caller. A `collect` that fetched results, wrote the + run record and then reported failure because git was mid-rebase would be a + strictly worse outcome than an uncommitted workspace. + + `force` is for the first commit, where "nothing has changed" is true of the + index and false of the world. + """ + out: dict[str, Any] = {"committed": False, "commit": None, "error": None, "reason": reason} + try: + if not force and not enabled(): + return out + staged_ok = _result("add", "-A") + if not _ok(staged_ok): + # Stop here rather than committing what happened to be staged + # already: an `add` that refused (a lock file, a permission, an + # index mid-rebase) means the commit below would record a state + # nobody chose. + out["error"] = (getattr(staged_ok, "stderr", "") or "").strip() or "git add failed" + return out + # `--porcelain` on the *index*: `diff --cached --quiet` exits 1 when + # there is something staged, which is the cheap way to ask "is this + # commit going to be empty" without parsing anything. + staged = _result("diff", "--cached", "--quiet") + if staged is not None and staged.returncode == 0 and not force: + return out + message = _message(reason) + commit = _result("commit", "-m", message, "--no-verify") + if commit is None or commit.returncode != 0: + detail = (getattr(commit, "stderr", "") or "").strip() + # An empty commit is not an error -- it is the race where something + # else committed between the check above and here. + if "nothing to commit" in detail or "nothing added" in detail: + return out + out["error"] = detail or "git commit failed" + return out + out["committed"] = True + out["commit"] = (_git("rev-parse", "--short", "HEAD") or "").strip() or None + except Exception as exc: # noqa: BLE001 - see the docstring + log.debug("workspace checkpoint failed", exc_info=True) + out["error"] = f"{type(exc).__name__}: {exc}" + return out + + +def _message(reason: str) -> str: + """One line, and the project it belongs to when there is one. + + Deliberately plain. The log is read to answer "what did the afternoon of the + fourteenth actually establish", and a subject line naming the event and the + project answers it without opening the diff. + """ + text = " ".join(str(reason or "checkpoint").split())[:120] + try: + from core import budget # noqa: PLC0415 + + project = budget.current_project() + except Exception: # noqa: BLE001 - a message is not worth failing over + project = None + return f"{project}: {text}" if project else text + + +def status() -> dict[str, Any]: + """What the workspace repository is, and what is uncommitted in it.""" + out: dict[str, Any] = { + "root": str(root()), + "repository": False, + "enabled": False, + "commits": 0, + "dirty": [], + "head": None, + "error": _refuse_if_installation(), + } + if not is_repository(): + return out + out["repository"] = True + out["enabled"] = enabled() + out["head"] = (_git("rev-parse", "--short", "HEAD") or "").strip() or None + count = _git("rev-list", "--count", "HEAD") + out["commits"] = int(count.strip()) if count and count.strip().isdigit() else 0 + dirty = _git("status", "--porcelain") + out["dirty"] = [line for line in (dirty or "").splitlines() if line.strip()] + return out + + +def history(limit: int = 20) -> list[dict[str, str]]: + """The last few checkpoints, newest first. Empty on any failure.""" + raw = _git("log", f"-{max(1, int(limit))}", "--format=%h%x1f%aI%x1f%s") + entries = [] + for line in (raw or "").splitlines(): + parts = line.split("\x1f") + if len(parts) == 3: + entries.append({"commit": parts[0], "at": parts[1], "subject": parts[2]}) + return entries diff --git a/hooks.py b/hooks.py index 5e1f689..325a0e1 100644 --- a/hooks.py +++ b/hooks.py @@ -63,6 +63,34 @@ def message(self) -> str: "gates in §6 and the weekly accelerator allowance the dollar ceilings cannot see", "python -m tools.kaggle submit --spec --expect --json", ), + # The environment rail, and the only one here that is about *this* machine + # rather than a remote. `agent.interpreter_env` puts Grad's own scripts + # directory first on PATH, so bare `pip` now resolves correctly -- but it + # resolves correctly by *ordering*, and ordering is a property a `cd`, a + # `PATH=...` prefix or a wrapper script can quietly change. `python -m pip` + # cannot: it installs into the interpreter that runs it, which is the same + # interpreter the kernel (`tools/nb.py`) and the dry run + # (`tools/preflight.py`) use, so a package the agent installs is a package + # the notebook can import. + # + # This machine is the argument: `pip` had three entries on PATH ahead of the + # venv's, and `python` was a global install carrying a second, editable Grad. + "pip": Denial( + "bare pip is denied: it installs into whichever environment PATH happens to name, " + "which is not necessarily the interpreter running Grad, the Jupyter kernel and the " + "preflight dry run", + "python -m pip install ", + ), + "pip3": Denial( + "bare pip3 is denied for the same reason as pip: the environment it installs into is " + "decided by PATH rather than by the interpreter", + "python -m pip install ", + ), + "conda": Denial( + "conda is denied: it manages a separate environment from the one Grad, the kernel and " + "the preflight dry run all share, so a package installed here is not importable there", + "python -m pip install ", + ), } # Cost-bearing commands, denied while the current project is over budget @@ -347,6 +375,11 @@ def probe(commands: list[str] | None = None) -> list[dict[str, Any]]: # speed bump is still a speed bump. "true\nssh gpu-box nvidia-smi", "rm -r -f ledger/", + # The environment rail. `python -m pip` is in the list precisely because + # it must come back *not* denied: a probe that only showed the refusals + # would not say whether the route out of them still works. + "pip install torch", + "python -m pip install torch", "python -m tools.gpu submit --spec pipeline/spec.toml --expect exp-1 --json", # Denied only while the current project is over budget, so its verdict # here depends on ledger state -- which is the point: the probe reports diff --git a/prompts/system.md b/prompts/system.md index ca577b7..bdd6ef9 100644 --- a/prompts/system.md +++ b/prompts/system.md @@ -21,6 +21,11 @@ submitter refuses, it is telling you something real, and the fix is in the error the prediction. A single seed is a legitimate result for some questions and a guess for most; `report check` will say which of your numbers rest on one. - Write what you learn to `notes/` as you go, and cite paths and paper ids. +- The workspace may be under local version control — `python -m tools.workspace + vcs status --json` says whether it is. When it is, a collect, a verdict and a + `project sync` each leave a commit, so a file you overwrite by accident is + recoverable with ordinary git. Don't commit by hand as a matter of course, and + never rewrite that history: it is a record of what the research did. - Keep the project's `MEMORY.md` current. It is the only thing you carry between sessions: a convention you settled, an approach you abandoned and why, a fact about the data or the hardware that cost you an hour. Write it down when you @@ -36,6 +41,11 @@ submitter refuses, it is telling you something real, and the fix is in the error learn nothing. - Check a library call against the installed signature before trusting it, and against `docs.py` before assuming it is current. +- Your interpreter runs in UTF-8 mode, so reading and printing paper source + works without saying so. What that cannot fix is a file that genuinely is not + UTF-8 — some arXiv LaTeX is latin-1 — so when you read paper source yourself, + pass `errors="replace"` and carry on rather than letting one accented name + stop the ingest. That is what `paper_ingest` does. ## Tools @@ -74,6 +84,18 @@ carries a `fix` field that is usually the literal next command. to be submitted here. `forget --reason "..."` writes off a run whose kernel was deleted in the Kaggle UI — it asks Kaggle first and refuses unless the answer is a 404, so it is not a way out of a job that still exists. +- `python -m tools.modal submit --spec --expect --json` — the same + verbs on Modal, which rents GPUs from a T4 to a B200 by the second. The `[spend]` ceilings + are the only gate here, so exit 13 never comes from this backend — but every + hour is real money, unlike Kaggle. `gpus` shows what is priced and refuses + anything that is not; `ceilings` shows the headroom; `account --check` says + whether the stored token pair actually authenticates. A spec needs `[target] + platform = "modal"`, a digest-pinned `image`, and `[estimate] hours`, which + sets the sandbox timeout. **Modal kills a sandbox at 24 hours**, so a longer + run has to checkpoint and resume across submissions; `submit` refuses rather + than starting one that cannot finish. Results come back through a Volume — + write to `$GRAD_METRICS_FILE` and put anything else worth keeping in + `$GRAD_OUT_DIR`, because the container's own disk does not outlive it. - `python -m tools.quota summary --json` — where the tokens and credits went; `--by-role` answers what each model cost. - `python -m tools.budget status --json` — the current project's remaining GPU @@ -88,7 +110,7 @@ carries a `fix` field that is usually the literal next command. `--pressure` tune the search; `--jobs` proposes several at once. `TASK.md` in the task dir is put in front of the operator every time — write it. `promote` turns a winner into an ordinary run, which still needs its own preflight and - prediction. `--remote {ssh|hf_jobs|kaggle} --remote-spec ` evaluates + prediction. `--remote {ssh|hf_jobs|kaggle|modal} --remote-spec ` evaluates every candidate on real hardware instead of on this machine — the loop stays here, the training goes there. It refuses unless that spec's preflight is complete and passing including the smoke run, so run the preflight first. A @@ -136,6 +158,14 @@ denied directly — use `gpu.py`, `jobs.py`, and `kaggle.py`, which hold the credentials. These are not obstacles to route around; they are the parts of the system that survive a deadline. +`pip`, `pip3` and `conda` are denied too, and for a different reason: they +install into whichever environment `PATH` names, which is not necessarily the +one you are running in. Use `python -m pip install `, which installs +into the interpreter that runs it — the same interpreter as the Jupyter kernel +and the preflight dry run, so a package you install is a package the notebook +can import. You do not need a venv and should not make one; `python` here is +already Grad's own environment. + A project that is out of allocation refuses cost-bearing commands with exit 12 — distinct from 6, which is the machine running out of money. Raising a ceiling is deliberate and logged: `python -m tools.budget raise`. Don't route around it; diff --git a/pyproject.toml b/pyproject.toml index 18d7dc5..0fbe387 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -39,6 +39,22 @@ remote = ["keyring>=25.0", "huggingface-hub>=0.24"] # unrecognised and every kernel runs on CPU, which is a job that burns no quota # and produces nothing. kaggle = ["keyring>=25.0", "kaggle>=1.6.14"] +# `tools/modal.py` imports the SDK rather than shelling out to the CLI, which is +# the opposite of every other backend here and is the *stronger* position on §9: +# `modal.Client.from_credentials` takes the token pair as arguments and sends +# them as gRPC headers, so the secret never enters any process's environment -- +# not this one's and not a child's. Shelling out would have required writing +# `.modal.toml` or exporting MODAL_TOKEN_ID, both of which are worse. +# +# The floor is a hint to the resolver and deliberately not the gate. What this +# module actually needs is `Client.from_credentials`, `Sandbox.create`, +# `Sandbox.from_id`, `Image.from_registry`, `Volume.from_name` and `App.lookup`, +# and `tools/modal.py:_modal()` checks for exactly those at the moment of use -- +# the same shape `tools/jobs.py:_hub` uses for the Jobs API. A pinned minimum +# that nobody verified is worse than a check that cannot be wrong: it looks +# authoritative and fails as an AttributeError several frames into a submit that +# has already written a ledger record. +modal = ["keyring>=25.0", "modal>=0.64"] # NiceGUI 3 is a floor, not a preference: the window system needs `Element.move` # to reparent a window between panes without rebuilding it, and `shared=True` on # `add_head_html`/`add_body_html`, which 3.0 requires when global-scope markup diff --git a/skills/modal/SKILL.md b/skills/modal/SKILL.md new file mode 100644 index 0000000..57b0114 --- /dev/null +++ b/skills/modal/SKILL.md @@ -0,0 +1,130 @@ +--- +name: modal +description: Modal's per-second GPU backend — H100s and up, how a spec becomes a Sandbox, why results come back through a Volume, and the 24-hour ceiling that cannot be raised. Load before the first Modal run. +--- + +# Modal sandboxes + +`modal.py` is the fourth submitter and the first whose billing model the §6 +dollar ceilings were already the right instrument for. Modal charges **per +second** against a published rate table, so `[spend]` is the gate here and there +is no second allowance to exhaust: **exit 13 never comes from this backend**. + +That makes it the opposite of Kaggle in the way that matters for planning. On +Kaggle a run is free and the hours are scarce; here the hours are yours and the +money is the constraint. `python -m tools.modal ceilings --json` is the question +to ask before sizing a run. + +## The hardware + +```bash +python -m tools.modal gpus --json # what is priced, and at what +``` + +| GPU | $/hour | note | +|---|---|---| +| T4 | 0.59 | | +| L4 | 0.80 | | +| A10G | 1.10 | | +| L40S | 1.95 | | +| A100-40GB | 2.10 | | +| A100-80GB | 2.50 | | +| H100 | 3.95 | the default | +| H200 | 4.54 | | +| B200 | 6.25 | | + +A count suffix multiplies the rate: `H100:8` is eight cards at eight times the +price. **An accelerator absent from `[modal.gpu_rates]` is refused at submit**, +not booked at zero — the spend ceiling is the only gate here, and a ceiling that +cannot price a run is not bounding it. + +## The 24-hour ceiling + +Modal kills a Sandbox at 24 hours, whatever it was doing. `submit` refuses a +spec whose `[estimate] hours` (times `timeout_margin`, default 1.25) would need +longer, rather than starting a run that cannot finish. This is not a local +policy that can be raised: the container is stopped either way, and a higher +local number only changes when you find out. + +A run that genuinely needs longer wants checkpointing and resuming across +several submissions — the same answer as a Kaggle session cap. + +## What a spec must declare + +```toml +entrypoint = "train.py" +image = "nvcr.io/nvidia/pytorch@sha256:..." # digest, not a tag +argv = ["--config", "config.toml"] +metrics_file = "metrics.json" + +[target] +platform = "modal" +gpu = "H100" # optional; [modal] default_gpu otherwise + +[estimate] +hours = 3.0 # required: it sets the sandbox timeout +cost_usd = 12.0 +``` + +The image must be **digest-pinned**, exactly as on HF Jobs, and for the same +reason: `core/submission.py` hashes the digest, and the preflight record is keyed +by that hash. Modal's own image DSL (`pip_install`, `run_commands`) is +deliberately not reachable from a spec — an image assembled from a Python +expression has no digest until it is built, and a preflight keyed by a hash that +does not cover the environment certifies nothing. + +## How your code gets there, and how results come back + +The spec's directory is copied into the image at `/grad/pipeline` and the +entrypoint runs there. Nothing is uploaded separately and there is no notebook +to pack. + +**Results come back through a Modal Volume, not the container filesystem.** A +sandbox's disk is gone the moment it exits, and `collect` runs afterwards by +construction — so a metrics file written beside the entrypoint would be +unreadable by the time anyone looked. Write to `$GRAD_METRICS_FILE`, which +points into the mounted Volume at `/grad/out//`. `$GRAD_OUT_DIR` is the +same directory, for checkpoints and figures worth keeping. + +A pipeline that ignores `$GRAD_METRICS_FILE` and writes `metrics.json` in its +working directory is still collected — the wrapper copies it into the Volume +afterwards — but anything *else* it wrote is lost. Put it in `$GRAD_OUT_DIR`. + +## Credentials + +A token **pair**, both halves secret, both in the OS credential store: + +```bash +python -m tools.jobs credential set modal_token_id +python -m tools.jobs credential set modal_token_secret +python -m tools.modal account --check --json # does the pair authenticate +``` + +`--check` is the useful command. `credential status` says a secret is *stored*, +which is a different claim from a secret that *works*, and the gap between them +is otherwise discovered at the worst moment. + +## The loop + +```bash +python -m tools.preflight run --spec pipeline/spec.toml --json +python -m tools.ledger expect --task speedrun --quantity val_loss@1e9_tokens \ + --low 3.1 --high 3.4 --basis 'modded-nanogpt|README|3.28|8xH100' --json +python -m tools.modal submit --spec pipeline/spec.toml --expect exp-... --json +python -m tools.wakeup arm --run run-... --timeout 14400 --note 'nanogpt speedrun' --json +# end the turn; you will be woken +python -m tools.modal collect run-... --json +``` + +`collect` is non-blocking by default and exits 10 while the sandbox is still +running. Do not poll it in a loop — arm a wakeup and end the turn. + +## What the cost number means + +`collect` prices **wall clock from submission** against the rate table, bounded +by the sandbox's own timeout. That is an upper bound, not Modal's billing: it +includes the image pull and any delay between the run finishing and being +collected. Every Modal run carries a `cost_warning` saying so, and `cost_basis` +on the record is `wall_clock` rather than `measured`. + +Collect promptly if the number matters. A wakeup is what makes that automatic. diff --git a/tests/test_agent_options.py b/tests/test_agent_options.py new file mode 100644 index 0000000..cea384a --- /dev/null +++ b/tests/test_agent_options.py @@ -0,0 +1,280 @@ +"""The options a session runs under, for the two that describe its *world*. + +`cwd` told the agent's shell where it was and nothing told it what `python` +meant, so the bare word resolved through whatever `PATH` the launcher happened +to carry. On the machine this was found on that was a global interpreter holding +a second, editable Grad -- so the app read one workspace through one +installation while the agent wrote it through another, and every tool call in +`prompts/system.md` is spelled `python -m tools.`. + +The file-checkpointing half is the same kind of claim: `core/rewind.py` puts the +conversation back and has never been able to put the *files* back, and the +option that changes that is one flag the SDK defaults to off. +""" + +from __future__ import annotations + +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +import agent + + +# --------------------------------------------------------------------------- +# PATH +# --------------------------------------------------------------------------- +def elsewhere(*names: str) -> list[str]: + """Fake PATH entries valid on the platform running the test. + + `C:\\Windows` looked like harmless local colour and is not: entries here are + joined and split on `os.pathsep`, which is `;` on Windows and `:` on POSIX, + so a Windows-shaped literal splits down its *drive letter* on Linux and + `["C:\\Windows"]` comes back as `["C", "\\Windows"]`. Two tests below passed + on the machine they were written on and failed in CI, which is the whole + reason CI runs both. + """ + root = "C:\\" if os.name == "nt" else "/" + return [root + name for name in names] + + +def test_this_interpreters_scripts_directory_is_first(monkeypatch): + """Not merely present -- first. `pip` had three entries ahead of the venv's + on the machine this was written on, so anything but the front is a coin + toss.""" + monkeypatch.setenv("PATH", os.pathsep.join(elsewhere("python314", "windows"))) + import sysconfig + + first = agent.interpreter_env()["PATH"].split(os.pathsep)[0] + assert Path(first) == Path(sysconfig.get_path("scripts")) + + +def test_the_machines_path_is_kept_behind_it(monkeypatch): + """Prepended, never replaced: the agent legitimately needs `git`, `docker` + and `latexmk`, and this is not the place to decide what a research machine + has installed.""" + ambient = elsewhere("windows", "tools") + monkeypatch.setenv("PATH", os.pathsep.join(ambient)) + parts = agent.interpreter_env()["PATH"].split(os.pathsep) + assert parts[1:] == ambient + + +def test_it_does_not_grow_a_copy_per_compaction(monkeypatch): + """A compaction builds a fresh client through `build_options`, so this runs + again in a process whose PATH it has already fixed. Applying it twice has to + be the same as applying it once.""" + monkeypatch.setenv("PATH", elsewhere("windows")[0]) + once = agent.interpreter_env()["PATH"] + monkeypatch.setenv("PATH", once) + assert agent.interpreter_env()["PATH"] == once + + +def test_an_empty_ambient_path_is_not_a_leading_separator(monkeypatch): + """`"".split(os.pathsep)` is `[""]`, and joining that produces a PATH whose + first entry is the empty string -- which POSIX reads as the current + directory.""" + monkeypatch.setenv("PATH", "") + value = agent.interpreter_env()["PATH"] + assert value + assert not value.endswith(os.pathsep) + assert "" not in value.split(os.pathsep) + + +# --------------------------------------------------------------------------- +# VIRTUAL_ENV and PYTHONPATH +# --------------------------------------------------------------------------- +@pytest.mark.skipif(sys.prefix == sys.base_prefix, reason="not running in a venv") +def test_virtual_env_names_the_environment_actually_in_use(monkeypatch): + """`pip` and `uv` both read it, and an ambient one pointing somewhere else is + the exact confusion this function exists to end.""" + monkeypatch.setenv("VIRTUAL_ENV", elsewhere("somewhere-else")[0]) + assert agent.interpreter_env()["VIRTUAL_ENV"] == sys.prefix + + +def test_an_installed_grad_gets_no_pythonpath(monkeypatch): + """Exporting the install directory unconditionally would put `config`, + `data`, `notes` and `figures` on the import path as namespace packages, and + `import data` is a thing research code genuinely does.""" + monkeypatch.setattr(agent, "_installed_as_distribution", lambda: True) + assert "PYTHONPATH" not in agent.interpreter_env() + + +def test_a_bare_checkout_gets_one(monkeypatch): + """`python agent.py` from a checkout nobody pip-installed: the agent's cwd is + the workspace, and the workspace has no `tools/` in it.""" + monkeypatch.setattr(agent, "_installed_as_distribution", lambda: False) + monkeypatch.delenv("PYTHONPATH", raising=False) + from core import paths + + assert agent.interpreter_env()["PYTHONPATH"] == str(paths.install_dir()) + + +def test_an_existing_pythonpath_is_kept_behind_ours(monkeypatch): + monkeypatch.setattr(agent, "_installed_as_distribution", lambda: False) + theirs = elsewhere("theirs")[0] + monkeypatch.setenv("PYTHONPATH", theirs) + assert agent.interpreter_env()["PYTHONPATH"].split(os.pathsep)[-1] == theirs + + +def test_the_distribution_check_never_raises(monkeypatch): + """False is the safe answer: it adds a redundant path entry, where True on a + machine that cannot answer is every tool call raising ModuleNotFoundError.""" + import importlib.metadata as md + + def boom(_name): + raise RuntimeError("no metadata here") + + monkeypatch.setattr(md, "distribution", boom) + assert agent._installed_as_distribution() is False + + +# --------------------------------------------------------------------------- +# text from the internet +# --------------------------------------------------------------------------- +#: A line of the sort every arXiv paper has in it: curly quotes and an umlaut. +#: Written as bytes so the test does not depend on this file's own encoding. +TEX_SAMPLE = b"the \xe2\x80\x98scaling law\xe2\x80\x99 of Sch\xc3\xb6lkopf\n" + + +def test_the_shell_asks_for_utf8_mode(): + from core import spawn + + assert agent.interpreter_env()["PYTHONUTF8"] == "1" + assert spawn.utf8_env()["PYTHONUTF8"] == "1" + + +def test_the_stream_encoding_names_its_error_handler(): + """Two ways to get this wrong, and the test exists because the first draft + got the first one wrong. + + Omitting `PYTHONIOENCODING` leaves an *ambient* one in force -- it takes + precedence over UTF-8 Mode for the standard streams, so a stray export + silently restores the `print` crash. Setting it to a bare `utf-8` defaults + the handler to `strict`, which is a different regression: a byte that + survived a lossy read then crashes on the way out. + """ + assert agent.interpreter_env()["PYTHONIOENCODING"] == "utf-8:surrogateescape" + + +@pytest.mark.parametrize( + "snippet", + [ + # Reading it: the curly quote is `e2 80 98`, and a Windows ANSI code page + # has no character at 0x98. + "print(open(PATH).read())", + # Reading it *correctly* and printing it, which is the failure that + # survives remembering `encoding=`: the standard streams take their + # encoding from the same code page. + "print(open(PATH, encoding='utf-8').read())", + ], +) +def test_reading_arxiv_latex_works_in_the_environment_the_agent_gets(tmp_path, snippet): + """Run for real, in a child, because this bug lives entirely in the defaults + of a fresh interpreter -- there is nothing to assert about it in-process, + where the encoding was decided before the test started.""" + sample = tmp_path / "paper.tex" + sample.write_bytes(TEX_SAMPLE) + code = f"PATH = r'{sample}'\n{snippet}" + + hostile = {**os.environ, "PYTHONUTF8": "0", "PYTHONIOENCODING": "cp1251"} + broken = subprocess.run( + [sys.executable, "-c", code], capture_output=True, text=True, env=hostile + ) + if broken.returncode == 0: + pytest.skip("this machine's default encoding decodes the sample anyway") + assert "Unicode" in broken.stderr + + fixed = subprocess.run( + [sys.executable, "-c", code], + capture_output=True, + text=True, + encoding="utf-8", + env={**hostile, **agent.interpreter_env()}, + ) + assert fixed.returncode == 0, fixed.stderr + assert "Schölkopf" in fixed.stdout + + +def test_the_kernel_is_given_the_same_environment_as_the_shell(monkeypatch): + """The kernel has two parents: the agent's Bash, which carries UTF-8 Mode, + and the desktop app, which does not. "Reading a paper crashes in the + notebook but not in the shell" is not a difference anyone should meet.""" + import tools.nb as nb + + captured = {} + + class _Proc: + pid = 4321 + + def fake_popen(_argv, **kwargs): + captured.update(kwargs) + return _Proc() + + monkeypatch.setattr(nb.subprocess, "Popen", fake_popen) + monkeypatch.setattr(nb, "_jupyter", lambda: _FakeJupyterClient()) + + nb._start_kernel("default", "python3") + assert captured["env"]["PYTHONUTF8"] == "1" + # Merged over the ambient environment, never replacing it: a kernel without + # SYSTEMROOT or TEMP does not start. + assert "PATH" in captured["env"] + + +class _FakeJupyterClient: + @staticmethod + def write_connection_file(fname: str, kernel_name: str) -> None: # noqa: ARG004 + from pathlib import Path as _P + + _P(fname).write_text("{}", encoding="utf-8") + + +# --------------------------------------------------------------------------- +# what actually reaches the SDK +# --------------------------------------------------------------------------- +def test_the_options_carry_the_environment(monkeypatch): + """The function above can be perfect and change nothing if the dict never + reaches `ClaudeAgentOptions` -- which is the state this file was written in. + """ + sdk = pytest.importorskip("claude_agent_sdk", reason="the SDK is not installed") + from core import config as config_mod + + monkeypatch.setattr(agent, "system_prompt", lambda: "prompt") + options = agent.build_options(config_mod.load()) + assert isinstance(options, sdk.ClaudeAgentOptions) + assert "PATH" in options.env + import sysconfig + + first = options.env["PATH"].split(os.pathsep)[0] + assert Path(first) == Path(sysconfig.get_path("scripts")) + + +def test_the_options_ask_the_cli_to_checkpoint_files(monkeypatch): + """Defaults to off in the SDK, and every test of the *rewind* half passes + with it dropped -- they stub the client. This is the one that notices.""" + sdk = pytest.importorskip("claude_agent_sdk", reason="the SDK is not installed") + from core import config as config_mod + + monkeypatch.setattr(agent, "system_prompt", lambda: "prompt") + options = agent.build_options(config_mod.load()) + assert options.enable_file_checkpointing is True + # The SDK refuses the combination outright, so this is not a style note. + assert options.session_store is None + + +def test_the_sdk_merges_our_environment_over_the_ambient_one(): + """The whole fix rests on `options.env` being a *patch* rather than a + replacement -- if the SDK swapped the environment wholesale, the child would + lose SYSTEMROOT, TEMP and the credential the session authenticates with. + + Pinned against the installed SDK's source rather than assumed, because this + is the kind of thing a release changes quietly. + """ + pytest.importorskip("claude_agent_sdk", reason="the SDK is not installed") + from claude_agent_sdk._internal.transport import subprocess_cli + + source = Path(subprocess_cli.__file__).read_text(encoding="utf-8") + assert "**inherited_env," in source + assert "**self._options.env," in source diff --git a/tests/test_desktop_app.py b/tests/test_desktop_app.py index 4e27f12..4372e9a 100644 --- a/tests/test_desktop_app.py +++ b/tests/test_desktop_app.py @@ -926,6 +926,66 @@ def test_the_geometry_is_actually_handed_to_the_window(workspace, fresh_geometry nicegui_app.native.window_args.clear() +def test_text_in_the_desktop_app_can_be_selected(workspace, fresh_geometry): + """pywebview defaults `text_select` to False, and that default is not a + preference -- it injects `body { user-select: none }` into the page. + + Everything in this workspace is text somebody needs to copy: a run id to + paste into a command, a traceback to search for, a number out of the ledger. + None of it could be selected in the desktop app and all of it could in the + browser, which is exactly why it survived -- the rule is injected at runtime + and is in no stylesheet to grep. + """ + assert desktop.window_args()["text_select"] is True + + +def test_the_stylesheet_only_suppresses_selection_where_a_drag_needs_it(workspace): + """The other half of the same claim, and the evidence the design always + assumed selection was on: `user-select: none` appears on the title bar, the + split handle and the in-flight drag, and nowhere else. A rule that turned it + off for a *pane* or the app root would put the setting above back.""" + import re + + from ui import tokens + + suppressed = [ + match.group(1).strip() + for match in re.finditer(r"([^{}]+)\{[^}]*user-select:\s*none", tokens.stylesheet()) + ] + for selector in suppressed: + assert any( + token in selector + for token in (".grad-handle", ".grad-titlebar", "body.grad-dragging") + ), f"{selector} makes text unselectable" + + +def test_it_reaches_the_window_and_not_the_browser(workspace, fresh_geometry, monkeypatch): + """`window_args` only applies in native mode, which is the mode with the + problem: in a browser the text was always selectable.""" + from nicegui import app as nicegui_app + + from ui import app as grad_app + + _screens(monkeypatch, (0, 0, 2560, 1440)) + nicegui_app.native.window_args.clear() + grad_app._install_desktop(True) + assert nicegui_app.native.window_args.get("text_select") is True + nicegui_app.native.window_args.clear() + + grad_app._install_desktop(False) + assert nicegui_app.native.window_args == {} + + +def test_pywebview_still_takes_the_argument(workspace): + """Pinned against the installed pywebview rather than assumed. A release that + renamed or dropped this would leave `create_window` raising a TypeError at + launch -- or worse, silently ignoring it and putting the rule back.""" + import inspect + + webview = pytest.importorskip("webview", reason="pywebview is not installed") + assert "text_select" in inspect.signature(webview.create_window).parameters + + def test_browser_mode_is_handed_no_window_at_all(workspace, fresh_geometry): """`native=False` is the documented fallback and there is no window to place. Setting these there would turn native mode back on -- the same trap the diff --git a/tests/test_first_run.py b/tests/test_first_run.py new file mode 100644 index 0000000..a27a165 --- /dev/null +++ b/tests/test_first_run.py @@ -0,0 +1,235 @@ +"""The first run: what a fresh machine is told it still needs. + +The thing under test is mostly a *decision*, not a widget -- whether this +workspace is mid-setup, which is the question that decides what opens on a +machine with no saved layout. It is derived from three conditions rather than +stored as a flag, and the tests below are largely about that choice: a stored +flag is wrong in both directions, and both are asserted here. +""" + +from __future__ import annotations + +import pytest + +from core import budget, credentials +from ui import models, state as state_mod + + +@pytest.fixture(autouse=True) +def bare_machine(monkeypatch): + """A machine with nothing in its credential store. + + `conftest.py` isolates the workspace (`GRAD_ROOT`) and the app directory + (`GRAD_APP_DIR`) and stops there -- the OS keyring is neither, so without + this every assertion here would be about the *developer's* stored + credentials. It showed up as "a fresh workspace already has a backend + configured", which is true of this machine and of no fresh install. + """ + stored: dict[str, bool] = {} + monkeypatch.setattr(credentials, "status", lambda: dict(stored)) + monkeypatch.setattr(credentials, "present", lambda name: bool(stored.get(name))) + monkeypatch.delenv("CLAUDE_CODE_OAUTH_TOKEN", raising=False) + return stored + + +def _no_token(monkeypatch): + monkeypatch.setattr(models, "setup_needed", lambda: True) + + +def _token(monkeypatch): + monkeypatch.setattr(models, "setup_needed", lambda: False) + + +def _project(name: str = "proj-1") -> None: + budget.create(name, title="a project", budget={"gpu_usd": 10.0}) + budget.set_current(name) + + +# --------------------------------------------------------------------------- +# when it is active +# --------------------------------------------------------------------------- +def test_a_fresh_workspace_is_mid_setup(workspace, monkeypatch): + _no_token(monkeypatch) + run = models.first_run() + + assert run["active"] is True + assert run["next"]["id"] == "token" + assert run["done"] == 0 + + +def test_a_token_alone_is_not_a_configured_workspace(workspace, monkeypatch): + """The condition this widened. A token and no project opens four windows, + three of which are empty because there is nothing to file against -- and + nothing on screen used to say that was the reason.""" + _token(monkeypatch) + run = models.first_run() + + assert run["active"] is True + assert run["next"]["id"] == "project" + + +def test_a_token_and_a_project_are_enough(workspace, monkeypatch): + _token(monkeypatch) + _project() + run = models.first_run() + + assert run["active"] is False + # The backend step is still *undone*, and that is not the same as unfinished + # setup -- it is reported, and it does not hold the panel open. + assert run["next"]["id"] == "backend" + assert [s["done"] for s in run["steps"]] == [True, True, False] + + +def test_an_unconfigured_backend_never_holds_the_panel_open(workspace, monkeypatch): + """Remote training is a real limitation and not a reason to put a wizard in + front of someone who opened the app to read a ledger.""" + _token(monkeypatch) + _project() + backend = next(s for s in models.first_run()["steps"] if s["id"] == "backend") + assert backend["blocking"] is False + + +def test_a_selected_project_that_no_longer_exists_does_not_count(workspace, monkeypatch): + """The selection file is machine-local state and outlives the project it + names. A dangling pointer is not something for a run to be charged to.""" + _token(monkeypatch) + budget.set_current("proj-that-was-deleted") + + assert models.first_run()["next"]["id"] == "project" + + +# --------------------------------------------------------------------------- +# why it is derived and not stored +# --------------------------------------------------------------------------- +def test_it_goes_away_by_being_satisfied_rather_than_dismissed(workspace, monkeypatch): + """A `first_run_done` flag is wrong in both directions. This is the first: + dismissing it on a machine that still has no project would hide the panel + that says so, permanently, with nothing to bring it back.""" + _token(monkeypatch) + assert models.first_run()["active"] is True + + _project() + assert models.first_run()["active"] is False + + +def test_a_second_workspace_gets_its_own_answer(workspace, monkeypatch, tmp_path): + """And this is the other direction: a flag stored per machine would leave a + brand-new workspace with no panel because a *different* one once set it.""" + _token(monkeypatch) + _project() + assert models.first_run()["active"] is False + + other = tmp_path.parent / f"{tmp_path.name}-second" + other.mkdir(exist_ok=True) + monkeypatch.setenv("GRAD_ROOT", str(other)) + from core import config + + config._cache.clear() + assert models.first_run()["active"] is True, "a fresh workspace is fresh" + + +def test_nothing_here_is_persisted(workspace, monkeypatch): + """Stated as an assertion because the tempting fix for every bug above is to + add a flag, and the flag is the bug.""" + from core import settings + + _token(monkeypatch) + models.first_run() + assert "first_run" not in settings.load() + + +# --------------------------------------------------------------------------- +# what it changes on screen +# --------------------------------------------------------------------------- +def test_a_mid_setup_workspace_opens_the_window_that_fixes_it(workspace, monkeypatch): + monkeypatch.setattr(models, "first_run_needed", lambda: True) + assert state_mod.opening_windows()[0] == "setup" + + +def test_a_configured_workspace_opens_the_ordinary_four(workspace, monkeypatch): + """`first_run_needed` and not `first_run`: the arrangement asks the cheap + question, because the full model loads a Config and this path must not -- + see the docstring on `first_run_needed`.""" + from ui import registry + + monkeypatch.setattr(models, "first_run_needed", lambda: False) + assert state_mod.opening_windows() == registry.defaults() + assert "setup" not in state_mod.opening_windows() + + +def test_an_unreadable_machine_still_opens_a_workspace(workspace, monkeypatch): + """This runs on the path that decides what is on screen at all. A credential + store that cannot be reached is not a reason to fail to open.""" + def boom(): + raise OSError("no keyring here") + + monkeypatch.setattr(models, "first_run_needed", boom) + from ui import registry + + assert state_mod.opening_windows() == registry.defaults() + + +def test_the_model_never_raises_on_a_broken_machine(workspace, monkeypatch): + """The panel is drawn by the one window whose job is to be usable when + nothing else is.""" + def boom(*_a, **_k): + raise RuntimeError("the ledger is on fire") + + monkeypatch.setattr(budget, "current_project", boom) + _no_token(monkeypatch) + + run = models.first_run() + assert run["active"] is True + assert run["steps"][1]["done"] is False + + +def test_the_setup_model_carries_the_panel(workspace, monkeypatch): + """One snapshot, not two reads: `authenticate` ticking green above a token + step that still says missing is the disagreement this avoids.""" + _no_token(monkeypatch) + model = models.setup_model() + + assert model["first_run"]["active"] is True + token_step = next(s for s in model["steps"] if s["id"] == "token") + assert token_step["ready"] is False + + +# --------------------------------------------------------------------------- +# the backend that was added last +# --------------------------------------------------------------------------- +def test_modal_counts_as_a_backend_once_both_halves_are_stored(workspace, bare_machine): + """A fifth list of backends, and the one this nearly drifted out of: + `tools/setup.py:REQUIREMENTS` decides what "a backend is configured" means, + and a backend missing from it can never satisfy the step.""" + from core import config as config_mod + from tools import setup as setup_tool + + assert "modal" in setup_tool.REQUIREMENTS + + stored = bare_machine + ready = {b["backend"]: b for b in setup_tool.readiness(config_mod.load())} + assert ready["modal"]["ready"] is False + # Both halves named, so nobody goes round the loop twice. + assert set(ready["modal"]["missing"]) == { + credentials.MODAL_TOKEN_ID, + credentials.MODAL_TOKEN_SECRET, + } + + stored[credentials.MODAL_TOKEN_ID] = True + assert setup_tool.readiness(config_mod.load()) + ready = {b["backend"]: b for b in setup_tool.readiness(config_mod.load())} + assert ready["modal"]["ready"] is False, "half a token pair authenticates nothing" + + stored[credentials.MODAL_TOKEN_SECRET] = True + ready = {b["backend"]: b for b in setup_tool.readiness(config_mod.load())} + assert ready["modal"]["ready"] is True + + +def test_every_backend_that_can_be_chosen_can_be_reported_on(workspace): + """`settings.BACKENDS` is what the setup window offers and `REQUIREMENTS` is + what says whether one is ready. A backend in the first and not the second is + one the window offers and can never mark configured.""" + from core import settings + from tools import setup as setup_tool + + assert set(settings.BACKENDS) == set(setup_tool.REQUIREMENTS) diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 42b987e..1a0e30f 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -49,6 +49,27 @@ def test_direct_credential_reads_are_denied(): assert evaluate_bash("keyring get grad hf_token") is not None +@pytest.mark.parametrize( + "command", + [ + "pip install torch", + "pip3 install -r requirements.txt", + "pip.exe install numpy", + "/c/Python314/Scripts/pip install torch", + "conda install pytorch", + "cd pipeline && pip install -e .", + ], +) +def test_installing_through_path_rather_than_the_interpreter_is_denied(command): + """`agent.interpreter_env` puts the right scripts directory first on PATH, + but it does so by *ordering*, and a wrapper or a `PATH=` prefix can change + an ordering. `python -m pip` installs into the interpreter that runs it, + which is the one the kernel and the preflight dry run also use.""" + denial = evaluate_bash(command) + assert denial is not None + assert denial.suggestion == "python -m pip install " + + @pytest.mark.parametrize( "command", [ @@ -58,6 +79,9 @@ def test_direct_credential_reads_are_denied(): "git status", "rm figures/001.png", "ls -la", + # The route out of the pip denial has to stay open, or the rail is a wall. + "python -m pip install torch", + "python -m pip install -r pipeline/requirements.txt", ], ) def test_the_intended_path_is_allowed(command): diff --git a/tests/test_modal.py b/tests/test_modal.py new file mode 100644 index 0000000..29c23d2 --- /dev/null +++ b/tests/test_modal.py @@ -0,0 +1,325 @@ +"""The Modal backend: the parts that have rules, without a live sandbox. + +What is under test is everything that decides *before* the SDK is reached -- +which GPU, at what rate, for how long, and what the gates say -- plus the two +refusals that only exist on this backend: an accelerator with no price, and a +run that asks for longer than Modal will let a Sandbox live. + +The SDK is not installed in this suite and is not stubbed at the module level +either. `_modal()` raising a ConfigError with a `pip install` in it *is* the +behaviour for a machine without the extra, and it is asserted rather than +mocked away. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from core import config as config_mod, credentials, settings, submit as submit_lib +from core.errors import ConfigError, GradError +from core.submission import Submission +from tools import modal as modal_tool + + +def spec(tmp_path: Path, **overrides) -> Path: + """A minimal submittable spec, digest-pinned so nothing reaches a registry.""" + (tmp_path / "train.py").write_text("print('hi')\n", encoding="utf-8") + document = { + "entrypoint": "train.py", + "image": "nvcr.io/nvidia/pytorch@sha256:" + "a" * 64, + "metrics_file": "metrics.json", + "target": {"platform": "modal"}, + "estimate": {"hours": 2.0, "cost_usd": 8.0}, + } + # Replaced, not merged. Merging looks helpful and made + # `estimate={"cost_usd": 1.0}` keep the default `hours`, so the test for "a + # spec with no estimate" was asserting against a spec that had one. + document.update(overrides) + path = tmp_path / "spec.json" + path.write_text(json.dumps(document), encoding="utf-8") + return path + + +def submission(tmp_path: Path, **overrides) -> Submission: + return Submission.load(spec(tmp_path, **overrides), resolve_digest=False) + + +# --------------------------------------------------------------------------- +# registration +# --------------------------------------------------------------------------- +def test_the_backend_is_registered_everywhere_a_backend_is_listed(workspace): + """Four lists name the backends and they drift independently. The one that + actually bit: `modal` in `REMOTE_BACKENDS` without a matching branch in the + candidate dispatcher ran the whole campaign on Hugging Face Jobs.""" + from tools import evolve as evolve_tool + + assert "modal" in settings.BACKENDS + assert "modal" in evolve_tool.REMOTE_BACKENDS + assert submit_lib.COLLECTORS["modal"] == "python -m tools.modal collect" + + +def test_the_collect_command_a_refusal_names_is_this_one(workspace): + """`submit_lib.collect_command` is what a stale-run refusal prints, and an + unknown platform degrades to the ledger rather than to a wrong instruction.""" + from core import ledger_store as ls + + run = ls.Run("run-1", {"id": "run-1", "platform": "modal"}) + assert submit_lib.collect_command(run) == "python -m tools.modal collect run-1 --json" + + +def test_both_halves_of_the_credential_are_registered(workspace): + """Both are secret, unlike Kaggle where the username is a name. A credential + missing from `ALL` is one `scrub_environment` does not remove.""" + assert credentials.MODAL_TOKEN_ID in credentials.ALL + assert credentials.MODAL_TOKEN_SECRET in credentials.ALL + + +# --------------------------------------------------------------------------- +# hardware and money +# --------------------------------------------------------------------------- +def test_the_gpu_resolves_flag_then_spec_then_config(workspace, tmp_path): + cfg = config_mod.load() + sub = submission(tmp_path, target={"gpu": "A100-80GB"}) + + assert modal_tool.resolve_gpu("H200", sub, cfg) == "H200" + assert modal_tool.resolve_gpu(None, sub, cfg) == "A100-80GB" + assert modal_tool.resolve_gpu(None, submission(tmp_path), cfg) == "H100" + + +def test_a_count_suffix_multiplies_the_rate(workspace): + """Modal spells eight H100s `H100:8`, and eight cards cost eight times as + much. A table with a row per count would go stale one row at a time.""" + cfg = config_mod.load() + single = modal_tool.gpu_rate("H100", cfg) + assert single == pytest.approx(3.9492) + assert modal_tool.gpu_rate("H100:8", cfg) == pytest.approx(single * 8) + + +def test_an_unpriced_gpu_is_refused_rather_than_booked_at_zero(workspace, tmp_path): + """`[spend]` is this backend's only gate. A run it cannot price is a run it + is not bounding, and $0 would make the ceiling decoration.""" + cfg = config_mod.load() + assert modal_tool.gpu_rate("GB200", cfg) is None + + with pytest.raises(ConfigError) as caught: + modal_tool._rate_or_refuse("GB200", cfg) + assert "modal.gpu_rates" in caught.value.fix + # The fix names what *is* priced, so the next command is obvious. + assert "H100" in caught.value.fix + + +# --------------------------------------------------------------------------- +# the 24-hour ceiling +# --------------------------------------------------------------------------- +def test_the_timeout_comes_from_the_estimate_with_a_margin(workspace, tmp_path): + """An estimate that was exactly right is the one case a job would be killed + for being on time.""" + cfg = config_mod.load() + seconds = modal_tool._timeout_seconds(submission(tmp_path, estimate={"hours": 2.0}), cfg) + assert seconds == int(2.0 * 1.25 * 3600) + + +def test_a_spec_with_no_estimate_cannot_set_a_timeout(workspace, tmp_path): + cfg = config_mod.load() + with pytest.raises(ConfigError) as caught: + modal_tool._timeout_seconds(submission(tmp_path, estimate={"cost_usd": 1.0}), cfg) + assert "estimate" in str(caught.value).lower() + + +def test_a_run_longer_than_modal_allows_is_refused_before_it_starts(workspace, tmp_path): + """Modal kills a Sandbox at 24 hours whatever it was doing. Starting a + 20-hour run at a 1.25 margin means 25 hours of sandbox, and the failure would + arrive a day later with nothing collected.""" + cfg = config_mod.load() + with pytest.raises(ConfigError) as caught: + modal_tool._timeout_seconds(submission(tmp_path, estimate={"hours": 20.0}), cfg) + assert "24" in str(caught.value) + assert "checkpoint" in caught.value.fix + + +def test_the_local_ceiling_can_lower_modals_but_never_raise_it(workspace, tmp_path, monkeypatch): + """A config asking for 48 hours does not get 48 hours -- the container is + stopped at 24 either way, and the only thing a higher local number changes + is when you find out.""" + cfg = config_mod.load() + monkeypatch.setitem(cfg.raw.setdefault("modal", {}), "max_hours", 48.0) + with pytest.raises(ConfigError): + modal_tool._timeout_seconds(submission(tmp_path, estimate={"hours": 30.0}), cfg) + + +# --------------------------------------------------------------------------- +# what the sandbox is told to do +# --------------------------------------------------------------------------- +def test_the_metrics_file_points_into_the_volume(workspace, tmp_path): + """A sandbox's disk is gone when it exits and `collect` runs afterwards by + construction, so a metrics file written beside the entrypoint is unreadable + by the time anyone looks.""" + env = modal_tool._job_env(submission(tmp_path), "/grad/out/run-1") + assert env["GRAD_METRICS_FILE"] == "/grad/out/run-1/metrics.json" + assert env[modal_tool.OUT_ENV] == "/grad/out/run-1" + + +def test_the_wrapper_copies_a_stray_metrics_file_into_the_volume(workspace, tmp_path): + """A pipeline written for another harness writes `metrics.json` beside + itself. Losing it means the money was spent and the result is gone.""" + sub = submission(tmp_path) + command = modal_tool._wrapped_command(["python", "train.py"], sub, "/grad/out/run-1") + assert command[:2] == ["sh", "-c"] + script = command[2] + assert "python train.py" in script + assert "cp -f metrics.json" in script + + +def test_the_wrapper_preserves_the_exit_code_across_the_copy(workspace, tmp_path): + """`cp` failing must not turn a failed run into a successful one, or the + reverse. This is the whole reason the wrapper is not a one-liner.""" + script = modal_tool._wrapped_command(["python", "train.py"], submission(tmp_path), "/out")[2] + assert "rc=$?" in script + assert script.rstrip().endswith("exit $rc") + + +def test_a_spec_command_overrides_the_entrypoint(workspace, tmp_path): + sub = submission(tmp_path, target={"command": ["torchrun", "--nproc", "8", "train.py"]}) + assert modal_tool._command_for(sub) == ["torchrun", "--nproc", "8", "train.py"] + + +def test_the_run_directory_is_named_for_the_run(workspace): + """One Volume, many runs. A shared output directory would let a later run + overwrite the metrics of an earlier one that had not been collected yet.""" + cfg = config_mod.load() + assert modal_tool._run_dir(cfg, "run-abc") == "/grad/out/run-abc" + + +# --------------------------------------------------------------------------- +# cost +# --------------------------------------------------------------------------- +def test_the_cost_is_wall_clock_and_says_so(workspace): + """Modal bills per second from container start; what is measurable here is + the ledger's own interval, which includes the image pull and any delay + before collection. It is an upper bound and the record must not imply it is + a measurement.""" + from core import ledger_store as ls + + cfg = config_mod.load() + run = ls.Run("run-1", { + "id": "run-1", + "platform": "modal", + "submitted_at": ls.now_iso(), + "target": {"gpu": "H100"}, + }) + cost, warning = modal_tool._actual_cost(run, {"gpu": "H100"}, cfg) + assert cost >= 0.0 + assert warning and "wall clock" in warning + + +def test_the_cost_cannot_exceed_the_sandbox_timeout(workspace): + """Collecting a week later must not book a week of H100 time: the container + cannot have run longer than Modal would let it.""" + from core import ledger_store as ls + + cfg = config_mod.load() + run = ls.Run("run-1", { + "id": "run-1", + "platform": "modal", + "submitted_at": "2020-01-01T00:00:00+00:00", + "target": {"gpu": "H100"}, + }) + cost, _ = modal_tool._actual_cost(run, {"gpu": "H100", "timeout_s": 3600}, cfg) + assert cost == pytest.approx(3.9492, rel=1e-3) + + +def test_an_unpriced_gpu_at_collect_books_zero_and_admits_it(workspace): + """Different from the submit-time refusal: by now the money is spent, so the + only useful thing is to say the number is not one.""" + from core import ledger_store as ls + + run = ls.Run( + "run-1", {"id": "run-1", "submitted_at": ls.now_iso(), "target": {"gpu": "GB200"}} + ) + cost, warning = modal_tool._actual_cost(run, {"gpu": "GB200"}, config_mod.load()) + assert cost == 0.0 + assert "no rate configured" in warning + + +# --------------------------------------------------------------------------- +# the SDK's absence +# --------------------------------------------------------------------------- +def test_a_machine_without_the_extra_is_told_which_extra(workspace): + """Not mocked away: this is the behaviour on a machine that has not + installed it, which is every machine until someone does.""" + try: + import modal # noqa: F401, PLC0415 + except ImportError: + with pytest.raises(ConfigError) as caught: + modal_tool._modal() + assert "modal" in caught.value.fix + else: + pytest.skip("the modal SDK is installed here") + + +def test_a_missing_credential_names_both_halves(workspace, monkeypatch): + """Both are secret and either can be absent. Naming only the first would + send someone round the loop twice.""" + monkeypatch.setattr(modal_tool, "_modal", lambda: object()) + monkeypatch.setattr(credentials, "get", lambda *_a, **_k: None) + + with pytest.raises(ConfigError) as caught: + modal_tool._client() + assert credentials.MODAL_TOKEN_ID in str(caught.value) + assert credentials.MODAL_TOKEN_SECRET in str(caught.value) + + +def test_the_token_is_never_put_in_the_environment(workspace): + """The strongest form of §9 available in this project: `from_credentials` + sends the pair as gRPC headers, so unlike every other backend there is + nothing to scrub because nothing is exported.""" + import ast + + source = Path(modal_tool.__file__).read_text(encoding="utf-8") + tree = ast.parse(source) + + # Parsed rather than grepped. Two earlier versions of this test matched the + # module's own *explanation* of why the environment is untouched -- first in + # the module docstring, then in a function docstring that line-based + # stripping could not see. The AST cannot be fooled by prose about the code. + imported = { + alias.name.split(".")[0] + for node in ast.walk(tree) + if isinstance(node, ast.Import) + for alias in node.names + } | { + node.module.split(".")[0] + for node in ast.walk(tree) + if isinstance(node, ast.ImportFrom) and node.module + } + assert "os" not in imported, "this module has no business reading the environment" + + calls = { + node.func.attr + for node in ast.walk(tree) + if isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute) + } + assert "from_credentials" in calls + + +# --------------------------------------------------------------------------- +# the gates still apply +# --------------------------------------------------------------------------- +def test_submitting_without_an_expectation_is_refused(workspace, tmp_path): + """The §6 gates are `core/submit.py`'s and this backend does not get its own + version of them -- but a backend that forgot to *call* them would look + exactly like one that had, until the first unpredicted run.""" + import argparse + + args = argparse.Namespace( + spec=str(spec(tmp_path)), expect=None, overrides=[], gpu=None, + task=None, project=None, smoke=False, no_digest=True, + ) + with pytest.raises(GradError) as caught: + modal_tool.cmd_submit(args) + # Exit 4 (no preflight) or 5 (no expectation) -- both are gate refusals and + # both must arrive before anything reaches Modal. + assert caught.value.exit_code in (4, 5) diff --git a/tests/test_ui_registry.py b/tests/test_ui_registry.py index c8fed6e..228d834 100644 --- a/tests/test_ui_registry.py +++ b/tests/test_ui_registry.py @@ -82,13 +82,15 @@ def test_the_defaults_reproduce_the_mocks_opening_arrangement(): def test_an_unauthenticated_machine_opens_on_setup(monkeypatch): """Those four windows are four windows that cannot do anything without a token, so the first thing on screen should be the one that fixes it.""" - monkeypatch.setattr(state_mod.models, "setup_needed", lambda: True) + monkeypatch.setattr(state_mod.models, "first_run_needed", lambda: True) assert state_mod.opening_windows()[0] == "setup" assert set(registry.defaults()) <= set(state_mod.opening_windows()) def test_a_configured_machine_opens_on_the_mocks_four(monkeypatch): - monkeypatch.setattr(state_mod.models, "setup_needed", lambda: False) + """Configured now means a token *and* a project: the arrangement asks + `first_run_needed`, which is the cheap half of `models.first_run`.""" + monkeypatch.setattr(state_mod.models, "first_run_needed", lambda: False) assert state_mod.opening_windows() == registry.defaults() @@ -96,7 +98,7 @@ def test_a_credential_store_that_raises_does_not_stop_a_workspace_opening(monkey def boom(): raise RuntimeError("no keyring, no registry, nothing") - monkeypatch.setattr(state_mod.models, "setup_needed", boom) + monkeypatch.setattr(state_mod.models, "first_run_needed", boom) assert state_mod.opening_windows() == registry.defaults() diff --git a/tests/test_ui_rewind.py b/tests/test_ui_rewind.py index 544bf51..9de82b8 100644 --- a/tests/test_ui_rewind.py +++ b/tests/test_ui_rewind.py @@ -385,6 +385,166 @@ def test_an_sdk_that_cannot_resume_is_not_told_the_memory_went_back(workspace, m assert session.settled[-1].get("anchor") is None +# --------------------------------------------------------------------------- +# the third half: the files +# --------------------------------------------------------------------------- +class FakeCheckpointClient: + """A client that records the control requests it was sent, and in what order. + + `closed` is what makes the ordering assertion possible: `rewind_files` is a + control request and `close` tears down the subprocess that answers it, so + the only bug worth testing for here is the two in the wrong order. + """ + + def __init__(self, *, fails: bool = False) -> None: + self.rewound_to: str | None = None + self.closed = False + self.rewound_after_close = False + self._fails = fails + + async def rewind_files(self, user_message_id: str) -> None: + if self.closed: + self.rewound_after_close = True + if self._fails: + raise RuntimeError("no checkpoint for that message") + self.rewound_to = user_message_id + + async def __aexit__(self, *_exc) -> None: + self.closed = True + + +def checkpointing_session(monkeypatch, *, fails: bool = False, supported: bool = True): + """A session whose client can checkpoint and whose dropped prompt is known. + + `_drops_turn` reads the SDK's own transcript off disk, which no test has -- + so it is stubbed here. What is under test is what the rewind *does* with the + uuid, not the parse that finds it, which `_drops_turn` owns. + """ + import agent + + monkeypatch.setattr(agent, "checkpointing_supported", lambda *_: supported) + session = session_with(conversation()) + # Mapped rather than constant. A stub that answers "prompt-3" whatever it is + # asked cannot tell "the earliest dropped prompt" from "some prompt", which + # is the claim the multi-turn test below is making. + prompts = {None: "prompt-1", "entry-1": "prompt-2", "entry-2": "prompt-3"} + monkeypatch.setattr(session, "_drops_turn", lambda anchor: prompts.get(anchor)) + client = FakeCheckpointClient(fails=fails) + session.client = client + return session, client + + +def test_the_files_go_back_to_the_prompt_the_rewind_drops(workspace, monkeypatch): + """Not to the anchor. The anchor is the *end of the last kept turn* and the + files should be as they were before the first dropped prompt ran, which is + the next user message after it.""" + session, client = checkpointing_session(monkeypatch) + outcome = asyncio.run(session.rewind_to(4)) + + assert outcome["files"] is True + assert client.rewound_to == "prompt-3" + + +def test_the_files_are_rewound_before_the_client_is_closed(workspace, monkeypatch): + """`rewind_files` is a control request and `rewind_to` closes the client. + Ordered the other way round this is the one half of a rewind that would + silently never run -- and it would fail into the same `except` as an SDK + that cannot checkpoint at all, so nothing would say so.""" + session, client = checkpointing_session(monkeypatch) + asyncio.run(session.rewind_to(4)) + + assert client.closed is True, "the rewind still has to drop the client" + assert client.rewound_after_close is False + + +def test_a_multi_turn_rewind_still_restores_the_files(workspace, monkeypatch): + """`resume_drops_turn` is only sent for a single-turn rewind because the SDK + validates it. That restriction is about the *conversation*; restoring files + to the earliest dropped prompt is right for any number of turns.""" + session, client = checkpointing_session(monkeypatch) + outcome = asyncio.run(session.rewind_to(2)) + + assert outcome["files"] is True + # The prompt at index 2, not the one at index 4: two exchanges go, and the + # files belong at the point before the *first* of them ran. + assert client.rewound_to == "prompt-2" + assert session._rewind_drops is None, "two turns go, so the SDK is not told one does" + + +def test_rewinding_to_the_very_first_prompt_still_restores_the_files(workspace, monkeypatch): + """The "start over" rewind, and the one where the work matters most. + + Rewinding to index 0 keeps nothing, so there is no last-entry-of-the-last- + kept-turn to anchor on -- and `dropped_prompt` was computed only `if anchor`, + so this case moved the transcript and left every file the session had + written. A missing anchor means "the first prompt in the conversation", not + "no prompt at all". + """ + session, client = checkpointing_session(monkeypatch) + outcome = session and asyncio.run(session.rewind_to(0)) + + assert outcome["ok"] is True + assert outcome["files"] is True + assert client.rewound_to == "prompt-1" + + +def test_an_sdk_that_cannot_checkpoint_rewinds_everything_else(workspace, monkeypatch): + session, client = checkpointing_session(monkeypatch, supported=False) + outcome = asyncio.run(session.rewind_to(4)) + + assert outcome["ok"] is True + assert outcome["files"] is False + assert client.rewound_to is None + assert len(session.settled) == 5, "the transcript still moved" + + +def test_a_failed_file_rewind_does_not_take_the_rewind_down(workspace, monkeypatch): + """Every reason this can fail is ordinary -- a prompt with no checkpoint, a + session already gone -- and none of them is a reason to refuse to rewind the + transcript.""" + session, client = checkpointing_session(monkeypatch, fails=True) + outcome = asyncio.run(session.rewind_to(4)) + + assert outcome["ok"] is True + assert outcome["files"] is False + assert session.settled[-1]["kind"] == rewind.MARK_KIND + + +def test_the_restore_is_only_claimed_when_it_happened(workspace, monkeypatch): + """Most conversations edit no files, so an absence is the common case and + reporting it reads as a failure of something never attempted.""" + session, _ = checkpointing_session(monkeypatch, supported=False) + outcome = asyncio.run(session.rewind_to(4)) + assert "files" not in outcome["message"] + + session, _ = checkpointing_session(monkeypatch) + outcome = asyncio.run(session.rewind_to(4)) + assert "files it edited are back" in outcome["message"] + + +def test_the_marker_says_what_the_restore_did_and_did_not_cover(workspace): + """The agent works mostly through Bash, so "files were restored" on its own + would be read as a promise the checkpointing does not make.""" + marker = rewind.record(dropped=[user("x")], resumed=True, files=True) + assert marker["files"] is True + assert "anything a command wrote was not" in marker["text"] + + quiet = rewind.record(dropped=[user("x")], resumed=True) + assert quiet["files"] is False + assert "restored" not in quiet["text"] + + +def test_checkpointing_support_is_detected_rather_than_assumed(workspace): + import agent + + assert agent.checkpointing_supported(_sdk_stub(fields=("enable_file_checkpointing",))) is True + assert agent.checkpointing_supported(_sdk_stub(fields=("resume",))) is False + assert agent.checkpointing_option(_sdk_stub(fields=("enable_file_checkpointing",))) == { + "enable_file_checkpointing": True + } + assert agent.checkpointing_option(_sdk_stub(fields=("resume",))) == {} + + def test_a_turn_that_starts_while_the_client_is_closing_aborts_the_rewind(workspace): """`close` tears down a CLI subprocess and the composer is live throughout. A prompt sent in that window has already appended to `settled` and started a diff --git a/tests/test_ui_shell.py b/tests/test_ui_shell.py index a5b023f..e767535 100644 --- a/tests/test_ui_shell.py +++ b/tests/test_ui_shell.py @@ -166,12 +166,111 @@ def test_the_window_menu_marks_what_is_open(rendered): _open_window_menu(client, space) rows = _menu_rows(client) - assert len(rows) == len(registry.ids()) + len(shell.PRESET_ROWS) + assert len(rows) == len(registry.ids()) + len(shell.PRESET_ROWS) + len(shell.THEME_ROWS) assert [r.props.get("title") for r in rows].count(None) == 0 - assert len([r for r in rows if "open" in r.classes]) == 1 + # Two marked rows, and they mean different things: one window is open, and + # one theme is in effect. The arrangement presets are actions and carry no + # state, which is why they are not in this count. + assert len([r for r in rows if "open" in r.classes]) == 2 assert "open" in _menu_row(client, "chat").classes +class RecordingClient: + """A client that records what was sent to the browser. + + Faked rather than driven for real because the assertion is about *reaching* + a client at all. The bug this guards was that the repaint went through + `ui.run_javascript`, which resolves `context.client` from the slot stack -- + empty in the spawned task the switch used to run in -- so nothing was ever + sent. A fake still catches that: code that does not use the held client + never touches this object. + """ + + def __init__(self) -> None: + self.sent: list[str] = [] + + def run_javascript(self, code: str, **_kwargs) -> None: + self.sent.append(code) + + +def _theme_row(client: Client, theme: str): + from ui import kit + + wanted = kit.attr(dict((t[0], t[2]) for t in shell.THEME_ROWS)[theme]) + for element in _menu_rows(client): + if element.props.get("title") == wanted: + return element + raise AssertionError(f"no menu row for the {theme} theme") + + +def test_choosing_a_theme_repaints_the_page_and_not_only_the_settings_file(rendered): + """The regression this file exists to hold. + + Every part of the theme worked except the one that mattered: `set_theme` + wrote the file, `say` put "theme: dark" in the status bar, and the repaint + raised `The current slot cannot be determined` into a debug log. The setting + changed, the notice appeared, and the workspace stayed cream -- so an + assertion on the *file* passed while the feature did nothing. + """ + from core import settings + + client, space = rendered(["chat"]) + space.client = RecordingClient() + _open_window_menu(client, space) + + click(_theme_row(client, "dark")) + + assert settings.theme() == "dark", "the choice is recorded" + assert space.painted_theme == "dark", "and the live page was told about it" + assert space.client.sent, "something has to reach the browser" + assert 'setAttribute("data-grad-theme", "dark")' in space.client.sent[-1] + + +def test_the_repaint_survives_the_menu_redrawing_underneath_it(rendered): + """The click handler redraws the menu, which deletes the row that was + clicked. That is what emptied the slot the old version resolved its client + through -- so the redraw is part of the test, not incidental to it.""" + client, space = rendered(["chat"]) + space.client = RecordingClient() + _open_window_menu(client, space) + + row = _theme_row(client, "dark") + click(row) + # The row really is gone: the assertion above would be vacuous otherwise. + assert row.id not in client.elements + assert space.painted_theme == "dark" + + +def test_switching_back_repaints_again(rendered): + """Two switches, because a mechanism that fires once and then holds a stale + reference would pass the test above.""" + client, space = rendered(["chat"]) + space.client = RecordingClient() + _open_window_menu(client, space) + + click(_theme_row(client, "dark")) + click(_theme_row(client, "light")) + + assert space.painted_theme == "light" + assert len(space.client.sent) == 2 + + +def test_a_saved_theme_is_on_the_page_before_it_paints(rendered, monkeypatch): + """The other half, and it needs a different mechanism from the switch. + + At build time there is no socket, so a `run_javascript` would be sent to + nobody -- and even a deferred one would let the page paint cream first and + flip. The attribute goes into the document as inline markup instead, so it + is set before the first frame. + """ + from core import settings + + settings.set_theme("dark") + client, _ = rendered(["chat"]) + + assert 'setAttribute("data-grad-theme", "dark")' in client.body_html + + def test_the_window_menu_toggles_in_place_rather_than_closing(rendered): """Opening three windows is three clicks. A menu that dismissed itself after each one would be three trips back to the same button.""" @@ -922,7 +1021,18 @@ async def fake_run_tool(*argv, timeout=120.0, stdin=None): monkeypatch.setattr(tasks_mod, "run_tool", fake_run_tool) monkeypatch.setattr("ui.state.run_tool", fake_run_tool) + # Both, because two different questions are asked on this path and stubbing + # either alone leaves the other reading the real machine. + # + # `create_project` consults `setup_needed` -- narrowly and correctly: after + # creating a project, the project half of `first_run_needed` is satisfied by + # definition, so only the token is left to ask about. But it also calls + # `reload()`, which re-derives the layout through `opening_windows` and + # therefore `first_run_needed`. `run_tool` is faked here so no project + # actually appears, and the setup window arrived from the *layout* rather + # than from the branch this test is named after. monkeypatch.setattr(models_mod, "setup_needed", lambda: False) + monkeypatch.setattr(models_mod, "first_run_needed", lambda: False) _, space = rendered(["projects"]) asyncio.run(space.create_project("proj-a", "A")) diff --git a/tests/test_ui_state.py b/tests/test_ui_state.py index 9a24c28..cb62505 100644 --- a/tests/test_ui_state.py +++ b/tests/test_ui_state.py @@ -34,11 +34,29 @@ def workspace_for(project: str | None = "proj") -> state_mod.Workspace: # --------------------------------------------------------------------------- # layout persistence # --------------------------------------------------------------------------- -def test_a_fresh_workspace_opens_the_default_arrangement(workspace): +def test_a_fresh_workspace_opens_the_default_arrangement(workspace, monkeypatch): + """On a *configured* machine. A workspace that is still mid-setup opens the + setup window as well -- see the test below, and `state.opening_windows`.""" + from ui import models as models_mod + + monkeypatch.setattr(models_mod, "first_run_needed", lambda: False) space = workspace_for() assert set(space.layout.windows) == set(registry.defaults()) +def test_a_workspace_that_is_still_mid_setup_opens_the_window_that_fixes_it( + workspace, monkeypatch +): + """Widened from "no token" to "no token or no project": a workspace with a + token and nothing to charge a run to opens four windows, three of them empty + for a reason nothing on screen was saying.""" + from ui import models as models_mod + + monkeypatch.setattr(models_mod, "first_run_needed", lambda: True) + space = workspace_for() + assert "setup" in space.layout.windows + + def test_the_layout_persists_per_project(workspace): a = workspace_for("alpha") a.open("funnel") @@ -55,7 +73,10 @@ def test_a_project_id_with_path_separators_cannot_escape_the_layout_directory(wo assert ".." not in path.name -def test_an_unreadable_layout_file_falls_back_to_the_default(workspace): +def test_an_unreadable_layout_file_falls_back_to_the_default(workspace, monkeypatch): + from ui import models as models_mod + + monkeypatch.setattr(models_mod, "first_run_needed", lambda: False) state_mod.layout_dir().mkdir(parents=True, exist_ok=True) state_mod.layout_path("proj").write_text("{ not json", encoding="utf-8") assert set(workspace_for("proj").layout.windows) == set(registry.defaults()) diff --git a/tests/test_ui_theme.py b/tests/test_ui_theme.py index bbb5773..e3a6f3c 100644 --- a/tests/test_ui_theme.py +++ b/tests/test_ui_theme.py @@ -52,13 +52,92 @@ def test_lab_is_started_with_the_flag_that_loads_it(): assert 'JUPYTER_CONFIG_DIR' in source -def test_the_selected_theme_is_a_light_one(): +def test_the_checked_in_selection_matches_the_checked_in_sheet(): """`custom.css` cannot register a named theme, so the sheet re-tokens a base - -- and it re-tokens the light one. Leaving JupyterLab Dark selected would - put dark defaults under cream overrides.""" + and the base has to match the palette -- dark defaults under cream overrides + is Lab's own chrome keeping the wrong theme in every corner the sheet does + not name. The repository copy is the light one; `install()` is what writes a + matching pair into a workspace.""" overrides = json.loads((config_dir() / "overrides.json").read_text(encoding="utf-8")) theme = overrides["@jupyterlab/apputils-extension:themes"]["theme"] - assert "Dark" not in theme + assert theme == jupyter_theme.LAB_BASE["light"] + + +def test_every_palette_names_a_base_to_re_token(): + """A palette with no base would install `custom.css` over whichever theme was + selected last, which is the mismatch above with no way to notice it.""" + assert set(jupyter_theme.LAB_BASE) == set(tokens.PALETTES) + + +def test_installing_a_palette_writes_a_matching_pair(workspace): + """The two files are read at different times by different loaders, and a + workspace where they disagree is the failure this pairing exists to stop.""" + result = jupyter_theme.install("dark") + assert result["error"] is None + + sheet = jupyter_theme.target().read_text(encoding="utf-8") + assert tokens.DARK["paper"] in sheet + assert tokens.COLOUR["paper"] not in sheet + + overrides = json.loads( + (jupyter_theme.target().parent.parent / "overrides.json").read_text(encoding="utf-8") + ) + assert overrides["@jupyterlab/apputils-extension:themes"]["theme"] == "JupyterLab Dark" + + +def test_installing_seeds_a_workspace_that_has_no_config_directory(workspace): + """The recommended layout keeps the workspace out of the checkout, and + `JUPYTER_CONFIG_DIR` points into the workspace -- so `--custom-css` and + `--ServerApp.config_file` have both been aimed at files that do not exist on + every install that took the advice.""" + result = jupyter_theme.install("light") + + assert result["error"] is None + server_config = jupyter_theme.target().parent.parent / "jupyter_server_config.py" + assert server_config.is_file(), "the framing-header config has to reach the workspace" + assert jupyter_theme.target().is_file() + + +def test_installing_does_not_overwrite_an_edited_server_config(workspace): + """`custom.css` is generated and is rewritten every time. The other two are + documents somebody may have edited, so they are seeded and then left.""" + jupyter_theme.install("light") + server_config = jupyter_theme.target().parent.parent / "jupyter_server_config.py" + server_config.write_text("# mine\n", encoding="utf-8") + + jupyter_theme.install("dark") + assert server_config.read_text(encoding="utf-8") == "# mine\n" + + +def test_installing_keeps_the_other_overrides(workspace): + """`overrides.json` carries the handoff's 88-column ruler. Rewriting the file + to hold one key would drop it, and nothing would say so.""" + jupyter_theme.install("light") + path = jupyter_theme.target().parent.parent / "overrides.json" + document = json.loads(path.read_text(encoding="utf-8")) + document["@jupyterlab/fileeditor-extension:plugin"] = {"editorConfig": {"rulers": [88]}} + path.write_text(json.dumps(document), encoding="utf-8") + + jupyter_theme.install("dark") + after = json.loads(path.read_text(encoding="utf-8")) + assert after["@jupyterlab/fileeditor-extension:plugin"]["editorConfig"]["rulers"] == [88] + assert after["@jupyterlab/apputils-extension:themes"]["theme"] == "JupyterLab Dark" + + +def test_an_unwritable_workspace_still_lets_lab_start(workspace, monkeypatch): + """Unstyled Lab beats no Lab. The caller reports the error rather than + failing on it -- `tools/lab.py` puts it in the start record.""" + monkeypatch.setattr(jupyter_theme, "write", _boom) + result = jupyter_theme.install("dark") + assert result["error"] and "Boom" in result["error"] + + +class _Boom(Exception): + pass + + +def _boom(*_args, **_kwargs): + raise _Boom("Boom") def test_the_ruler_the_handoff_asks_to_leave_alone_is_left_alone(): diff --git a/tests/test_ui_tokens.py b/tests/test_ui_tokens.py index d065254..7d8cdda 100644 --- a/tests/test_ui_tokens.py +++ b/tests/test_ui_tokens.py @@ -53,12 +53,19 @@ def test_the_colours_are_the_ones_the_handoff_specifies(name, value): def test_every_state_accent_is_a_real_token(): assert set(tokens.STATE_ACCENT) == {"ok", "attention", "broken", "neutral"} assert set(tokens.STATE_ACCENT.values()) <= set(tokens.COLOUR.values()) + # Keyed rather than valued, so the mapping means the same thing in a palette + # it was not written against. + assert set(tokens.STATE_ACCENT_KEYS) == set(tokens.STATE_ACCENT) + for theme in tokens.PALETTES: + assert set(tokens.STATE_ACCENT_KEYS.values()) <= set(tokens.palette(theme)) def test_every_series_colour_is_a_real_token(): assert set(tokens.SERIES.values()) <= set(tokens.COLOUR.values()) for name in tokens.SERIES: assert f"--grad-series-{name}:" in tokens.css_variables() + for theme in tokens.PALETTES: + assert set(tokens.SERIES_KEYS.values()) <= set(tokens.palette(theme)) def test_no_chart_series_borrows_a_chromatic_state_accent(): @@ -79,6 +86,14 @@ def test_no_chart_series_borrows_a_chromatic_state_accent(): tokens.STATE_ACCENT["broken"], } assert set(tokens.SERIES.values()) & chromatic == set() + # And in every palette, which is the version of the claim that survives a + # second one being added: a dark `link` that happened to land on the dark + # `broken` would put a state's colour in a chart without changing a rule. + for theme in tokens.PALETTES: + active = tokens.palette(theme) + accents = {active[tokens.STATE_ACCENT_KEYS[s]] for s in ("ok", "attention", "broken")} + series = {active[key] for key in tokens.SERIES_KEYS.values()} + assert series & accents == set(), theme @pytest.mark.parametrize( @@ -97,6 +112,147 @@ def test_chart_fills_are_drawn_from_the_series_ramp(selector): assert "--grad-series-" in block, f"{selector} does not use a series colour: {block}" +# --------------------------------------------------------------------------- +# the second palette +# --------------------------------------------------------------------------- +def relative_luminance(hex_colour: str) -> float: + """WCAG 2.1 relative luminance, for the contrast check below.""" + value = hex_colour.lstrip("#") + if len(value) == 3: + value = "".join(c * 2 for c in value) + channels = [] + for i in (0, 2, 4): + c = int(value[i : i + 2], 16) / 255 + channels.append(c / 12.92 if c <= 0.03928 else ((c + 0.055) / 1.055) ** 2.4) + r, g, b = channels + return 0.2126 * r + 0.7152 * g + 0.0722 * b + + +def contrast(a: str, b: str) -> float: + la, lb = relative_luminance(a), relative_luminance(b) + lo, hi = sorted((la, lb)) + return (hi + 0.05) / (lo + 0.05) + + +def test_both_palettes_carry_exactly_the_same_tokens(): + """A key in one and not the other is a rule that renders as `unset` -- which + is not a visible failure, it is the *inherited* colour, so the element looks + plausible and is wrong.""" + assert set(tokens.DARK) == set(tokens.COLOUR) + + +@pytest.mark.parametrize("theme", sorted(tokens.PALETTES)) +def test_every_ground_carries_legible_text_in_every_palette(theme): + """The rule the light palette got to satisfy by inspection. + + Inspection does not survive a second palette: `on-series-third` was white on + `link`, which is 5.6:1 in cream and 2.4:1 in the dark, and the first anyone + would have known is an unreadable spend meter at night. + """ + active = tokens.palette(theme) + failures = [] + for ground, text in tokens.FOREGROUND.items(): + ratio = contrast(active[ground], active[text]) + if ratio < 4.5: + failures.append(f"{ground}/{text} = {ratio:.2f}:1") + for series, text in tokens.SERIES_FOREGROUND.items(): + ratio = contrast(active[tokens.SERIES_KEYS[series]], active[text]) + if ratio < 4.5: + failures.append(f"series-{series}/{text} = {ratio:.2f}:1") + assert not failures, f"{theme}: " + ", ".join(failures) + + +def hue_degrees(hex_colour: str) -> float: + import colorsys + + value = hex_colour.lstrip("#") + r, g, b = (int(value[i : i + 2], 16) / 255 for i in (0, 2, 4)) + return colorsys.rgb_to_hsv(r, g, b)[0] * 360 + + +@pytest.mark.parametrize("theme", sorted(tokens.PALETTES)) +def test_the_three_state_accents_stay_distinguishable(theme): + """"One accent per state" is only information if the states do not converge. + + Measured as **hue** separation and deliberately not as contrast ratio. The + first version of this test used the luminance formula above and failed on a + perfectly good pair -- the dark palette's teal and its yellow are 1.48:1 and + are not remotely confusable, because a contrast ratio says how legible one + is *on* the other and nothing about telling two fills apart side by side. + Yellow, teal and crimson are read by hue; that is the thing to hold. + """ + active = tokens.palette(theme) + hues = { + state: hue_degrees(active[tokens.STATE_ACCENT_KEYS[state]]) + for state in ("ok", "attention", "broken") + } + for a, b in (("ok", "attention"), ("ok", "broken"), ("attention", "broken")): + gap = abs(hues[a] - hues[b]) % 360 + assert min(gap, 360 - gap) >= 30, f"{theme}: {a} and {b} share a hue" + + +def test_the_accents_keep_their_hues_across_the_two_palettes(): + """The vocabulary is "yellow needs you, teal passed, red broke". A dark theme + that renegotiated that would be a different design rather than the same one + at night -- so the values may move for legibility and the hues may not.""" + for state in ("ok", "attention", "broken"): + key = tokens.STATE_ACCENT_KEYS[state] + gap = abs(hue_degrees(tokens.COLOUR[key]) - hue_degrees(tokens.DARK[key])) % 360 + assert min(gap, 360 - gap) <= 20, state + + +def test_the_dark_palette_is_actually_dark(): + """Stated as an assertion because the failure mode is subtle: a palette that + inverted the text and forgot a ground reads as light with white text.""" + dark = tokens.palette("dark") + for ground in ("paper", "paper-raised", "paper-sunk", "desk"): + assert relative_luminance(dark[ground]) < 0.08, ground + assert relative_luminance(dark["ink"]) > 0.5 + + +def test_the_emphasis_ground_does_not_become_the_brightest_thing_on_screen(): + """The whole reason `fill` exists. The app bar, the status bar and every + table head are `background: var(--grad-fill)`, and they were + `var(--grad-ink)` -- so a palette that only swapped ink and paper would have + given the dark theme a white app bar and white table headers.""" + dark = tokens.palette("dark") + assert relative_luminance(dark["fill"]) < relative_luminance(dark["ink"]) + # And still distinct from the page it sits on, or the bar stops being one. + assert contrast(dark["fill"], dark["paper"]) > 1.15 + + +def test_the_hard_shadow_never_becomes_a_glow(): + """`SHADOW_SHELL` is an 8px offset block. Drawn in `ink` it is near-black on + cream and near-*white* in the dark palette, which is a glow.""" + assert "var(--grad-shadow-ink)" in tokens.SHADOW_SHELL + for theme in tokens.PALETTES: + assert relative_luminance(tokens.palette(theme)["shadow-ink"]) < 0.05, theme + + +def test_the_switch_is_one_attribute_and_ships_in_the_same_sheet(): + """Both palettes travel in one stylesheet because `ui/app.py` adds it once, + at import, with `shared=True` -- there is no second injection to make, so a + theme change is an attribute on `` and the cascade does the rest. That + is also what keeps it inside the design's motion rule: an attribute flip is + an instant state swap, not a transition.""" + sheet = tokens.stylesheet() + assert f':root[{tokens.THEME_ATTRIBUTE}="dark"]' in sheet + for name, value in tokens.DARK.items(): + assert f"--grad-{name}: {value};" in sheet, name + # The non-colour half is emitted once: a second copy would be a second place + # for the handle width to disagree with `layout.py`. + assert sheet.count("--grad-handle:") == 1 + + +def test_an_unknown_theme_falls_back_rather_than_failing(): + """What a settings file written by a newer version looks like from an older + one. The answer is the design's default, not a stylesheet that will not + generate and takes the window with it.""" + assert tokens.palette("solarized") == tokens.COLOUR + assert tokens.palette(None) == tokens.COLOUR + assert tokens.palette("DARK") == tokens.DARK + + # --------------------------------------------------------------------------- # the structural rules # --------------------------------------------------------------------------- @@ -128,7 +284,14 @@ def test_no_shadow_in_the_system_has_a_blur(): def test_the_structural_border_is_two_pixels_of_ink(): - assert tokens.BORDER_STRUCTURAL == f"2px solid {tokens.COLOUR['ink']}" + """Through the custom property rather than the literal. + + It used to interpolate `COLOUR['ink']` at import, which put `2px solid + #14100C` in the sheet -- so `--grad-border` was pinned to the light palette + by an f-string and no re-declaration of `--grad-ink` could move it. The + claim is unchanged; what it resolves through is.""" + assert tokens.BORDER_STRUCTURAL == "2px solid var(--grad-ink)" + assert "--grad-border: 2px solid var(--grad-ink);" in tokens.css_variables() def test_the_minimum_pane_matches_the_layout_model(): diff --git a/tests/test_vcs.py b/tests/test_vcs.py new file mode 100644 index 0000000..3a3852b --- /dev/null +++ b/tests/test_vcs.py @@ -0,0 +1,295 @@ +"""Versioning the workspace: what it refuses, and when it commits. + +The ledger is append-only, so the thing this protects is not the run records -- +it is `notes/`, a project's `MEMORY.md`, the pipeline code and a report's +`.tex`, all of which are ordinary files that an ordinary mistake can truncate. + +Two refusals carry most of the value and both are tested against a real +repository rather than a mock: versioning the *installation* would put research +on the same branch as upstream's releases, and adopting a repository somebody +else made would mean committing to a history that is not ours. +""" + +from __future__ import annotations + +import subprocess + +import pytest + +from core import paths, vcs, version + + +def git_available() -> bool: + return version.git("--version", cwd=paths.root()) is not None + + +pytestmark = pytest.mark.skipif(not git_available(), reason="git is not on PATH") + + +def initialised(workspace): + result = vcs.initialise() + assert result["error"] is None, result + return result + + +# --------------------------------------------------------------------------- +# what it refuses +# --------------------------------------------------------------------------- +def test_it_refuses_to_version_the_installation(workspace, monkeypatch): + """The default workspace *is* the checkout, and Grad's source is already + versioned there. Auto-committing would put a user's notebooks on the same + branch as upstream's releases -- which is what makes an update a merge, and + is the whole reason `workspace move` exists.""" + monkeypatch.setattr(paths, "install_dir", lambda: paths.root()) + + result = vcs.initialise() + assert result["created"] is False + assert "installation folder" in result["error"] + assert "workspace move" in result["fix"] + assert not (paths.root() / ".git").exists() + + +def test_it_refuses_to_adopt_somebody_elses_repository(workspace): + """A workspace can already be inside somebody's own versioning. Quietly + taking it over would mean committing to a history that is not ours.""" + subprocess.run(["git", "init"], cwd=workspace, capture_output=True, check=True) + + result = vcs.initialise() + assert result["created"] is False + assert "did not create" in result["error"] + assert vcs.enabled() is False + + +def test_a_workspace_inside_a_repository_is_not_a_repository(workspace, monkeypatch): + """`rev-parse --git-dir` answers yes from any subdirectory, so the check has + to be that the workspace is the *top*. Committing from inside somebody's + repository would sweep up whatever else it holds.""" + subprocess.run(["git", "init"], cwd=workspace, capture_output=True, check=True) + inner = workspace / "inner" + inner.mkdir() + monkeypatch.setenv("GRAD_ROOT", str(inner)) + + assert vcs.is_repository() is False + + +def test_an_unversioned_workspace_checkpoints_nothing(workspace): + """Initialisation is deliberate, so everything before it is a no-op rather + than an implicit `git init` in somebody's folder.""" + result = vcs.checkpoint("a run was collected") + assert result["committed"] is False + assert result["error"] is None + assert not (workspace / ".git").exists() + + +# --------------------------------------------------------------------------- +# what it does +# --------------------------------------------------------------------------- +def test_initialising_marks_the_repository_and_commits(workspace): + result = initialised(workspace) + + assert result["created"] is True + assert vcs.enabled() is True + assert (workspace / ".gitignore").is_file() + assert vcs.status()["commits"] == 1 + + +def test_initialising_twice_is_not_an_error(workspace): + initialised(workspace) + again = vcs.initialise() + assert again["already"] is True + assert again["error"] is None + + +def test_a_checkpoint_commits_what_changed(workspace): + initialised(workspace) + (workspace / "notes").mkdir(exist_ok=True) + (workspace / "notes" / "finding.md").write_text("the lr schedule was off by one", "utf-8") + + result = vcs.checkpoint("verdict bug on run-1") + assert result["committed"] is True + assert result["commit"] + assert vcs.status()["dirty"] == [] + + subjects = [entry["subject"] for entry in vcs.history()] + assert any("verdict bug on run-1" in s for s in subjects) + + +def test_a_checkpoint_with_nothing_to_commit_is_quiet(workspace): + """Called after every collect, so the common case is "nothing changed" and + it must not produce an empty commit per run.""" + initialised(workspace) + first = vcs.checkpoint("nothing happened") + assert first["committed"] is False + assert first["error"] is None + assert vcs.status()["commits"] == 1 + + +def test_the_message_names_the_project(workspace): + """The log is read to answer what an afternoon established, and the project + is half of that answer.""" + from core import budget + + budget.create("proj-x", title="width vs depth", budget={"gpu_usd": 10.0}) + budget.set_current("proj-x") + initialised(workspace) + (workspace / "notes").mkdir(exist_ok=True) + (workspace / "notes" / "a.md").write_text("x", "utf-8") + + vcs.checkpoint("collected run-9 (succeeded)") + assert vcs.history()[0]["subject"] == "proj-x: collected run-9 (succeeded)" + + +# --------------------------------------------------------------------------- +# what it tracks +# --------------------------------------------------------------------------- +def test_the_jsonl_ledgers_are_tracked_and_the_indexes_are_not(workspace): + """The JSONL ledgers are the source of truth and diff line by line, which is + the whole reason this repository is worth having. The SQLite files beside + them are derived from those ledgers and would only ever be conflicts.""" + initialised(workspace) + (workspace / "ledger" / "runs.jsonl").write_text('{"id": "run-1"}\n', encoding="utf-8") + (workspace / "ledger" / "ledger.sqlite").write_bytes(b"\x00binary") + vcs.checkpoint("a run") + + tracked = version.git("ls-files", cwd=workspace) or "" + assert "ledger/runs.jsonl" in tracked + assert "ledger.sqlite" not in tracked + + +@pytest.mark.parametrize( + "relative", + [ + "data/papers/2001.08361.tex", + "figures/001.png", + "ledger/runs/run-1/checkpoint.pt", + "data/lab/lab.json", + ".env", + "kaggle.json", + ], +) +def test_the_things_that_must_never_be_committed_are_not(workspace, relative): + """Two categories in one list: large and re-fetchable, and secret. The second + matters more than the first even with no remote -- a credential in a commit + survives the file being deleted.""" + initialised(workspace) + path = workspace / relative + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text("x", encoding="utf-8") + vcs.checkpoint("everything") + + tracked = (version.git("ls-files", cwd=workspace) or "").splitlines() + assert relative not in tracked + + +def test_a_workspace_that_already_has_a_gitignore_still_excludes_secrets(workspace): + """The gap the first version left. A workspace can already have a + `.gitignore` -- somebody's own, or one left by a checkout it used to be -- + and skipping the write there meant `.env` and `kaggle.json` were not + excluded and the first checkpoint committed them. A credential in a commit + survives the file being deleted.""" + (workspace / ".gitignore").write_text("# mine\n*.tmp\n", encoding="utf-8") + initialised(workspace) + + (workspace / ".env").write_text("VOYAGE_KEY=sk-real", encoding="utf-8") + (workspace / "scratch.tmp").write_text("x", encoding="utf-8") + (workspace / "notes").mkdir(exist_ok=True) + (workspace / "notes" / "a.md").write_text("kept", encoding="utf-8") + vcs.checkpoint("after") + + tracked = (version.git("ls-files", cwd=workspace) or "").splitlines() + assert ".env" not in tracked + assert "notes/a.md" in tracked + # And the rules that were already there still apply. + assert "scratch.tmp" not in tracked + assert "# mine" in (workspace / ".gitignore").read_text(encoding="utf-8") + + +def test_initialising_twice_does_not_write_the_block_twice(workspace): + initialised(workspace) + vcs._write_ignore(workspace / ".gitignore") + + body = (workspace / ".gitignore").read_text(encoding="utf-8") + assert body.count(vcs.IGNORE_BEGIN) == 1 + + +def test_a_git_that_runs_and_refuses_is_not_a_success(workspace, monkeypatch): + """`is None` means "never ran". A non-zero return code means it ran and said + no, and conflating the two configured and committed into a directory that + was not a repository.""" + class Refused: + returncode = 1 + stdout = "" + stderr = "fatal: cannot mkdir" + + monkeypatch.setattr(vcs, "_result", lambda *a, **k: Refused()) + out = vcs.initialise() + assert out["created"] is False + assert out["error"] + + +def test_a_failed_add_does_not_commit_whatever_was_staged(workspace, monkeypatch): + initialised(workspace) + + class Refused: + returncode = 1 + stdout = "" + stderr = "fatal: unable to index file" + + monkeypatch.setattr(vcs, "_result", lambda *a, **k: Refused()) + out = vcs.checkpoint("should not land") + assert out["committed"] is False + assert "unable to index file" in out["error"] + + +def test_the_current_project_pointer_is_machine_local(workspace): + """Which project is selected is one machine's choice, not a fact about the + research. Two checkouts should not fight over it.""" + initialised(workspace) + (workspace / "ledger" / ".current_project").write_text("proj-a", encoding="utf-8") + vcs.checkpoint("selected a project") + + assert ".current_project" not in (version.git("ls-files", cwd=workspace) or "") + + +# --------------------------------------------------------------------------- +# it may never fail a caller +# --------------------------------------------------------------------------- +def test_a_broken_git_does_not_fail_the_command_that_called_it(workspace, monkeypatch): + """`checkpoint` runs just after a collect has written the run record. A + wedged git turning a successful collect into a failed command would be + strictly worse than an uncommitted workspace.""" + initialised(workspace) + + def explode(*_args, **_kwargs): + raise OSError("git is on fire") + + monkeypatch.setattr(vcs, "_result", explode) + result = vcs.checkpoint("collected run-1") + + assert result["committed"] is False + assert "git is on fire" in result["error"] + + +def test_the_collect_path_checkpoints_without_being_able_to_fail(workspace, monkeypatch): + """The wrapper the three call sites share.""" + from core import submit + + seen = [] + monkeypatch.setattr( + "core.vcs.checkpoint", lambda reason, **_: seen.append(reason) or {"committed": True} + ) + submit.checkpoint_workspace("collected run-2 (succeeded)") + assert seen == ["collected run-2 (succeeded)"] + + def explode(*_args, **_kwargs): + raise RuntimeError("nope") + + monkeypatch.setattr("core.vcs.checkpoint", explode) + submit.checkpoint_workspace("collected run-3") # must not raise + + +def test_status_reports_the_refusal_rather_than_pretending(workspace, monkeypatch): + monkeypatch.setattr(paths, "install_dir", lambda: paths.root()) + state = vcs.status() + assert state["repository"] is False + assert "installation folder" in state["error"] diff --git a/tools/evolve.py b/tools/evolve.py index e3b2abc..0fc7536 100644 --- a/tools/evolve.py +++ b/tools/evolve.py @@ -129,7 +129,7 @@ "metered in ledger/quota.jsonl under the `evolve.mutate` stage and bounded by the\n" "project's token allocation. --mutator shinka switches to ShinkaEvolve, which\n" "refuses unless the installed release exposes a per-generation entry point.\n\n" - "--remote {ssh|hf_jobs|kaggle} --remote-spec evaluates every candidate on\n" + "--remote {ssh|hf_jobs|kaggle|modal} --remote-spec evaluates every candidate\n" "real hardware. It refuses unless that spec's preflight is complete and passing --\n" "tests, dry run, and a real smoke run on that hardware -- because a search is a\n" "loop with no human in it and the environment it lands in has to be proven once,\n" @@ -149,7 +149,8 @@ BACKEND_SSH = "ssh" BACKEND_HF = "hf_jobs" BACKEND_KAGGLE = "kaggle" -REMOTE_BACKENDS = (BACKEND_SSH, BACKEND_HF, BACKEND_KAGGLE) +BACKEND_MODAL = "modal" +REMOTE_BACKENDS = (BACKEND_SSH, BACKEND_HF, BACKEND_KAGGLE, BACKEND_MODAL) #: The checks a remote campaign's spec must have passed. Not `[preflight] checks` #: from the config, and that is the point: a machine configured to skip `smoke` @@ -697,6 +698,24 @@ def _remote_target(args: argparse.Namespace, cfg: config_mod.Config) -> dict[str _kaggle_hours_gate(cfg, args, sub, accelerator=accelerator, kind=kind) return target + if args.remote == BACKEND_MODAL: + from tools import modal as modal_tool # noqa: PLC0415 - optional deps + + gpu = modal_tool.resolve_gpu(None, sub, cfg) + # Refused before generation 0, for the reason the HF branch below gives: + # an unpriced accelerator makes the campaign's projected cost a fiction, + # and the campaign budget gate is the only thing between a search and an + # allocation. + rate = modal_tool.gpu_rate(gpu, cfg) + if rate is None: + raise ConfigError( + f"Modal GPU {gpu!r} has no rate in [modal.gpu_rates], so a campaign on it " + "cannot be priced", + fix=f'add `"{gpu}" = ` under [modal.gpu_rates] in config/grad.toml', + ) + target.update({"gpu": gpu, "rate_usd_per_hour": rate}) + return target + from tools import jobs as jobs_tool # noqa: PLC0415 - optional deps flavor = sub.target.get("flavor") or cfg.get("hf", "default_flavor", "a10g-small") @@ -810,7 +829,7 @@ def _remote_note(target: dict[str, Any] | None) -> dict[str, Any]: # Only what this backend actually resolved. A record carrying `host: null` # on a Kaggle campaign reads as a host that could not be found rather than # as a dimension that does not apply. - for key in ("host", "rate_usd_per_hour", "accelerator", "accelerator_kind", "flavor"): + for key in ("host", "rate_usd_per_hour", "accelerator", "accelerator_kind", "flavor", "gpu"): if target.get(key) is not None: note[key] = target[key] return note @@ -1325,6 +1344,25 @@ def _run_on_backend( accelerator=remote.get("accelerator"), **common, ) + if backend == BACKEND_MODAL: + from tools import modal as modal_tool # noqa: PLC0415 + + return modal_tool.evaluate_candidate( + remote["sub"], cfg, artifacts=artifacts, gpu=remote.get("gpu"), **common + ) + + if backend != BACKEND_HF: + # Explicit rather than a fall-through, and this is a bug that already + # happened once in the making: `modal` was added to `REMOTE_BACKENDS` + # before it was added here, and the effect of the old `return + # jobs_tool...` at the end of this function was that `--remote modal` + # ran the whole campaign on Hugging Face Jobs. Silently, with the right + # hardware name in the log. + raise ConfigError( + f"no candidate evaluator is wired up for backend {backend!r}", + fix=f"--remote {'|'.join(REMOTE_BACKENDS)}", + ) + from tools import jobs as jobs_tool # noqa: PLC0415 return jobs_tool.evaluate_candidate( diff --git a/tools/lab.py b/tools/lab.py index e7aa18f..17957eb 100644 --- a/tools/lab.py +++ b/tools/lab.py @@ -85,6 +85,22 @@ def _jupyter_config_dir() -> Path: return paths.root() / "config" / "jupyter" +def _install_theme() -> dict[str, Any]: + """Generate Lab's stylesheet for the workspace's palette. Never raises. + + Imports `ui` lazily and tolerates its absence: `tools/lab.py` is a CLI and + runs on installs without the `ui` extra, where there is no palette to apply + and no desktop app to match. + """ + try: + from core import settings # noqa: PLC0415 + from ui import jupyter_theme # noqa: PLC0415 + + return jupyter_theme.install(settings.theme()) + except Exception as exc: # noqa: BLE001 - see the docstring + return {"theme": None, "written": [], "seeded": [], "error": f"{type(exc).__name__}: {exc}"} + + def _read_state() -> dict[str, Any]: return jsonl.read_json(_state_path()) or {} @@ -188,6 +204,14 @@ def cmd_start(args: argparse.Namespace) -> dict[str, Any]: log = _log_path() log.parent.mkdir(parents=True, exist_ok=True) + # Before the server, because both files below are read at startup: the sheet + # by `--custom-css` and the base-theme selection by Lab's own settings + # loader. This is also what puts them in the workspace at all -- see + # `ui/jupyter_theme.py:install`, which seeds a config directory the + # recommended split has never had one in. Failure is reported, not raised: a + # Lab server that starts unstyled beats one that does not start. + theming = _install_theme() + env = { **os.environ, # Read by config/jupyter/jupyter_server_config.py, so the framing @@ -246,6 +270,10 @@ def cmd_start(args: argparse.Namespace) -> dict[str, Any]: "log": str(log), "started_at": time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime()), "listening": _listening(port), + # Reported rather than silent, because the failure it can carry is + # invisible: an unstyled Lab looks like a working Lab until you notice + # it is not the same application as the window around it. + "theme": theming, } jsonl.write_json(_state_path(), record) if not record["listening"]: diff --git a/tools/ledger.py b/tools/ledger.py index 42d3598..73567ee 100644 --- a/tools/ledger.py +++ b/tools/ledger.py @@ -252,6 +252,12 @@ def cmd_verdict(args: argparse.Namespace) -> dict[str, Any]: # archive holding every deviation and no verdict would be a record of what was # measured with the part that says what it meant left out. submit_lib.archive_quietly(args.run_id) + # The second of the three workspace checkpoints. A verdict is the moment a + # measurement becomes a finding, which makes it the one commit anybody + # reading the log later is looking for. + submit_lib.checkpoint_workspace( + f"verdict {args.verdict} on {args.run_id} ({args.quantity})" + ) return {"verdict": record, "remaining_unjudged": len(ls.run(args.run_id).unjudged_deviations())} diff --git a/tools/modal.py b/tools/modal.py new file mode 100644 index 0000000..61ea25b --- /dev/null +++ b/tools/modal.py @@ -0,0 +1,1065 @@ +"""grad-modal -- submit, watch, and collect Modal Sandboxes (HANDOFF §6, §7). + +The fourth backend, and the first one whose billing model the `[spend]` ceilings +were already the right instrument for: Modal charges per second against a +published rate table, so none of Kaggle's hours machinery is needed here. A +dollar ceiling measures something real again. + +Four decisions are load-bearing. + +**A Sandbox, not a Function.** Modal's headline API is a decorated function in a +deployed app, which would mean the research pipeline had to be written for +Modal. A Sandbox takes a container image, a command and a GPU -- which is +exactly the shape `core/submission.py` already resolves -- so a spec that runs +on HF Jobs runs here with a different `[target] platform` and nothing else. +`Sandbox.from_id` is the other half: `submit` and `collect` are separate CLI +invocations in separate processes, and the sandbox id in the ledger handle is +what reconnects them. + +**The credential never leaves this process.** Every other backend here hands a +secret to a child -- an environment variable, a config file written for the +duration of a call. Modal's client takes the token pair as *arguments* and sends +them as gRPC headers, so `modal.Client.from_credentials` is the strongest form +of §9 available anywhere in this project: there is nothing to scrub because +nothing is exported. `MODAL_TOKEN_ID` is deliberately never set. + +**Results come back through a Volume, because a sandbox's filesystem does not +outlive it.** A finished sandbox cannot be `exec`'d into, so `cat metrics.json` +works only while the job is still running -- which is precisely when there is +nothing to read. The run writes into a mounted Volume instead, under a directory +named for the run, and `collect` reads the Volume afterwards from a process the +sandbox knows nothing about. + +**The image must still be digest-pinned.** `Image.from_registry` takes a +registry reference and `core/submission.py` already requires a digest, so the +preflight hash keeps meaning what it says: the same hash is the same image. +Modal's own image-building DSL (`pip_install`, `run_commands`) is deliberately +not exposed -- an image assembled from a Python expression has no digest to hash +until it is built, and a preflight record keyed by a hash that does not cover +the environment is a preflight record that certifies nothing. +""" + +from __future__ import annotations + +import argparse +import shlex +import time +from pathlib import Path +from typing import Any + +from core import ( + budget, + config as config_mod, + credentials, + gates, + ledger_store as ls, + submit as submit_lib, +) +from core.cli import Cli, main +from core.config import Config +from core.errors import ConfigError, EXIT_RUNNING, GradError, UpstreamError, UsageError +from core.submission import Submission, parse_override + +cli = Cli( + "grad-modal", + "Submit and collect Modal Sandboxes. Refuses to submit without a passing " + "preflight, an open expectation, and headroom under both spend ceilings.", + epilog=( + "gate refusals have their own exit codes (4 preflight, 5 expectation, 6 spend,\n" + "7 stale run) so a refusal is never confused with an upstream failure.\n\n" + "Modal bills per second, so the [spend] ceilings are the gate here and there is\n" + "no hours allowance to exhaust -- exit 13 never comes from this backend.\n\n" + "A Sandbox's maximum lifetime is 24 hours and Modal enforces it: a spec whose\n" + "[estimate] hours exceeds that is refused rather than started." + ), +) + +PLATFORM = "modal" + +#: Modal's own hard ceiling on a Sandbox's lifetime. Not a policy of ours: the +#: container is killed at this point whatever it was doing, so a run planned past +#: it is a run that cannot finish. +MODAL_MAX_HOURS = 24.0 + +#: Where the run's outputs go inside the sandbox, as an environment variable the +#: pipeline can read. Named rather than positional so a pipeline that wants to +#: write a checkpoint next to its metrics has somewhere documented to put it. +OUT_ENV = "GRAD_OUT_DIR" + + +# --------------------------------------------------------------------------- +# backend +# --------------------------------------------------------------------------- +def _modal() -> Any: + """The Modal SDK, or a configuration error naming the extra to install.""" + try: + import modal # noqa: PLC0415 + except ImportError as exc: + raise ConfigError( + "the modal SDK is not installed, so Modal cannot be reached", + fix="pip install -e '.[modal]' # or: python -m pip install modal", + ) from exc + # The exact surface this module uses, not just the top-level names. Checked + # the way `tools/jobs.py:_hub` checks for `run_job`/`inspect_job`, and for + # the same reason: a version floor in `pyproject.toml` is a hint to the + # resolver, and this is the thing that actually decides whether a submission + # can work. Without it an older SDK fails at `Client.from_credentials` with + # an AttributeError several frames into a submit that has already written a + # ledger record. + needed = { + "Client": ("from_credentials",), + "Sandbox": ("create", "from_id"), + "Image": ("from_registry",), + "Volume": ("from_name",), + "App": ("lookup",), + } + for attr, methods in needed.items(): + target = getattr(modal, attr, None) + if target is None: + raise ConfigError( + f"the installed modal has no {attr}; this is not a version this understands", + fix="python -m pip install -U modal", + ) + missing = [m for m in methods if not hasattr(target, m)] + if missing: + raise ConfigError( + f"the installed modal's {attr} has no {', '.join(missing)}, " + "so this backend cannot reach it", + fix="python -m pip install -U modal", + ) + return modal + + +def _client() -> Any: + """An authenticated client, built from credentials fetched at the moment of use. + + `from_credentials` rather than the environment, and that is the whole + argument of §9 in one call: the token pair is passed as arguments and sent as + gRPC headers, so it is never in `os.environ` for the agent to read and never + in a child's environment for a subprocess to inherit. Nothing here needs + scrubbing because nothing is exported. + """ + modal = _modal() + token_id = credentials.get(credentials.MODAL_TOKEN_ID, required=False) + token_secret = credentials.get(credentials.MODAL_TOKEN_SECRET, required=False) + missing = [ + name + for name, value in ( + (credentials.MODAL_TOKEN_ID, token_id), + (credentials.MODAL_TOKEN_SECRET, token_secret), + ) + if not value + ] + if missing: + raise ConfigError( + f"no Modal credential stored: {', '.join(missing)}", + fix=" && ".join(f"python -m tools.jobs credential set {n}" for n in missing), + ) + try: + return modal.Client.from_credentials(token_id, token_secret) + except Exception as exc: # noqa: BLE001 - the SDK raises a wide family here + raise ConfigError( + f"Modal refused the stored credentials: {exc}", + fix=( + "mint a fresh token pair at modal.com/settings/tokens, then " + f"python -m tools.jobs credential set {credentials.MODAL_TOKEN_ID}" + ), + ) from exc + + +# --------------------------------------------------------------------------- +# hardware and money +# --------------------------------------------------------------------------- +def resolve_gpu(flag: str | None, sub: Submission, cfg: Config) -> str: + """Which accelerator this run asks for: the flag, the spec, then the config. + + The same resolution order every backend here uses, and the same reason: a + flag is a decision about one submission, a spec is a decision about the + pipeline, and the config is a decision about the machine. + """ + chosen = flag or sub.target.get("gpu") or cfg.get("modal", "default_gpu", "H100") + return str(chosen).strip() + + +def gpu_rate(gpu: str, cfg: Config) -> float | None: + """Dollars per hour for one accelerator, or None if it is not priced. + + None is not zero and the callers treat it as a refusal rather than a + bargain: an unpriced GPU is one whose spend the ceiling cannot bound, and + `[spend]` is the only gate this backend has. + + A count suffix is stripped before the lookup -- Modal spells eight H100s + `H100:8` -- and multiplies the rate, because eight cards cost eight times as + much and a table with an entry per count would be a table that goes stale one + row at a time. + """ + rates = cfg.get("modal", "gpu_rates", {}) or {} + name, _, count = str(gpu).partition(":") + rate = (rates or {}).get(name.strip()) + if rate is None: + return None + if not count.strip(): + return float(rate) + try: + multiplier = max(1, int(count)) + except ValueError: + # Not one card. `H100:eight` is a spec nobody can price, and pricing it + # as a single card would book an eight-GPU run at an eighth of its cost + # -- silently, and in the direction the ceiling cannot catch. + return None + return float(rate) * multiplier + + +def _rate_or_refuse(gpu: str, cfg: Config) -> float: + rate = gpu_rate(gpu, cfg) + if rate is None: + known = ", ".join(sorted(cfg.get("modal", "gpu_rates", {}) or {})) or "(none)" + raise ConfigError( + f"no price is configured for Modal GPU {gpu!r}, so this run's cost cannot be bounded", + fix=f"add it under [modal.gpu_rates] in config/grad.toml. Priced now: {known}", + ) + return rate + + +def _timeout_seconds(sub: Submission, cfg: Config) -> int: + """How long the sandbox may live, from the spec's own estimate. + + Modal kills a Sandbox at its timeout, so this is the number that decides + whether a run finishes. It is the estimate plus a margin rather than the + estimate, because an estimate that was exactly right is the one case a job + would be killed for being on time. + + A spec asking for more than Modal's 24-hour ceiling is refused here rather + than started: the alternative is a run that trains for a day and is killed + with nothing collected. + """ + hours = float(sub.estimate.get("hours") or 0.0) + if hours <= 0: + raise ConfigError( + f"{sub.spec_path} has no `[estimate] hours`, so the sandbox has no timeout to set", + fix="add `hours = ` under [estimate] in the spec", + ) + margin = float(cfg.get("modal", "timeout_margin", 1.25) or 1.25) + ceiling = min(float(cfg.get("modal", "max_hours", MODAL_MAX_HOURS)), MODAL_MAX_HOURS) + wanted = hours * margin + if wanted > ceiling: + raise ConfigError( + f"this spec asks for {hours:.2f}h which needs a {wanted:.2f}h sandbox, and Modal's " + f"ceiling is {ceiling:.2f}h -- the container would be killed mid-run", + fix="reduce `[estimate] hours`, checkpoint and resume across runs, or use a bigger GPU", + ) + return int(wanted * 3600) + + +# --------------------------------------------------------------------------- +# the sandbox +# --------------------------------------------------------------------------- +def _command_for(sub: Submission) -> list[str]: + if sub.target.get("command"): + return [str(c) for c in sub.target["command"]] + return ["python", sub.entrypoint.name, *sub.argv] + + +def _job_env(sub: Submission, out_dir: str) -> dict[str, str]: + env = {str(k): str(v) for k, v in (sub.target.get("env") or {}).items()} + # Into the Volume, not the container filesystem: a sandbox's disk is gone the + # moment it exits, and `collect` runs afterwards by construction. + env["GRAD_METRICS_FILE"] = f"{out_dir}/{Path(sub.metrics_file).name}" + env[OUT_ENV] = out_dir + return env + + +def _wrapped_command(command: list[str], sub: Submission, out_dir: str) -> list[str]: + """The command, plus the copy that makes a well-behaved pipeline unnecessary. + + `GRAD_METRICS_FILE` tells the pipeline where to write, and every pipeline in + this project respects it. A pipeline that does not -- one written for another + harness, one that hardcodes `metrics.json` beside itself -- would produce a + run that succeeded and collected nothing, which is the most expensive way to + fail: the money is spent and the result is gone. + + So the metrics file is copied into the Volume afterwards if it is sitting in + the working directory, and **the exit code is preserved across the copy**. + `cp` failing must not turn a failed run into a successful one or the reverse, + which is what `rc=$?` and the final `exit $rc` are for. + """ + name = Path(sub.metrics_file).name + inner = shlex.join(command) + out = shlex.quote(out_dir) + metrics = shlex.quote(name) + script = ( + f"mkdir -p {out}; {inner}; rc=$?; " + f"if [ -f {metrics} ]; then cp -f {metrics} {out}/ 2>/dev/null || true; fi; " + "exit $rc" + ) + return ["sh", "-c", script] + + +def _image(sub: Submission, modal: Any, cfg: Config, env: dict[str, str]) -> Any: + """The digest-pinned registry image, with the pipeline copied in. + + `copy=True` rather than the default mount: a mounted directory is attached at + startup and is a property of the *client* that started the sandbox, and this + client exits as soon as `submit` returns. Baking the code into a layer is + what makes the run survive its submitter. + """ + workdir = str(cfg.get("modal", "workdir", "/grad/pipeline")) + image = modal.Image.from_registry(sub.image) + image = image.add_local_dir(str(sub.spec_path.parent), workdir, copy=True) + if env: + image = image.env(env) + return image + + +def _volume(modal: Any, cfg: Config, client: Any) -> tuple[Any, str]: + name = str(cfg.get("modal", "volume_name", "grad-runs")) + volume = modal.Volume.from_name(name, create_if_missing=True, client=client) + return volume, name + + +def _run_dir(cfg: Config, run_id: str) -> str: + mount = str(cfg.get("modal", "mount_path", "/grad/out")).rstrip("/") + return f"{mount}/{run_id}" + + +def _create_sandbox( + sub: Submission, + cfg: Config, + *, + client: Any, + gpu: str, + command: list[str], + timeout_s: int, + run_id: str, +) -> tuple[Any, str]: + """Start the sandbox and return it with the id the ledger will hold.""" + modal = _modal() + volume, volume_name = _volume(modal, cfg, client) + mount = str(cfg.get("modal", "mount_path", "/grad/out")).rstrip("/") + out_dir = _run_dir(cfg, run_id) + env = _job_env(sub, out_dir) + app = modal.App.lookup( + str(cfg.get("modal", "app_name", "grad")), create_if_missing=True, client=client + ) + sandbox = modal.Sandbox.create( + *_wrapped_command(command, sub, out_dir), + app=app, + image=_image(sub, modal, cfg, env), + gpu=gpu, + timeout=timeout_s, + workdir=str(cfg.get("modal", "workdir", "/grad/pipeline")), + volumes={mount: volume}, + client=client, + ) + sandbox_id = getattr(sandbox, "object_id", None) + if not sandbox_id or not isinstance(sandbox_id, str): + # Never `str(sandbox)`. The fallback looks harmless and writes + # `` into the ledger handle, which + # `collect` then hands to `Sandbox.from_id` -- so the run cannot be + # collected, goes stale, and blocks every later submission through the + # §6 gate. Failing here instead lets the caller mark the submission + # failed while the sandbox is still the only thing that exists. + raise UpstreamError( + "Modal returned a sandbox with no id, so this run could never be collected", + fix="retry; if it persists, check the Modal dashboard and `python -m pip install -U modal`", + ) + # Detached, so the sandbox outlives this CLI. `submit` returns in seconds and + # the run takes hours; without this the client-side connection is the thing + # holding it, and `collect` in a later process could not reach it. + _detach(sandbox) + return sandbox_id, volume_name + + +def _detach(sandbox: Any) -> None: + """Release the client-side connection without stopping the sandbox. + + Best effort: an SDK without `detach` leaves a connection that dies with this + process anyway, which is the outcome `detach` asks for politely. + """ + try: + sandbox.detach() + except Exception: # noqa: BLE001 - see the docstring + pass + + +def _reattach(sandbox_id: str, client: Any) -> Any: + modal = _modal() + try: + return modal.Sandbox.from_id(sandbox_id, client=client) + except Exception as exc: # noqa: BLE001 + raise UpstreamError( + f"Modal could not find sandbox {sandbox_id}: {exc}", + fix="check the run in the Modal dashboard; if it is gone, `ledger abandon` the run", + ) from exc + + +def _state_of(sandbox: Any) -> tuple[str, int | None]: + """`(state, exit_code)`, where state is one of running/completed/failed. + + `poll()` returns None while the sandbox is alive and the exit code once it is + not, which is the whole state machine -- there is no queued state to wait + through, because a Sandbox either starts or fails to. + """ + try: + code = sandbox.poll() + except Exception as exc: # noqa: BLE001 + raise UpstreamError( + f"Modal would not report the sandbox's state: {exc}", + fix="retry, or check the run in the Modal dashboard", + ) from exc + if code is None: + return "running", None + return ("completed" if int(code) == 0 else "failed"), int(code) + + +def _poll(sandbox: Any, *, deadline: float, interval: float) -> tuple[str, int | None]: + while True: + state, code = _state_of(sandbox) + if state != "running" or time.time() >= deadline: + return state, code + time.sleep(max(1.0, interval)) + + +def _logs(sandbox: Any) -> str: + """Whatever the sandbox said, best effort and never fatal. + + A collected run whose logs could not be fetched is still a collected run -- + the metrics are in the Volume, which is the part the ledger needs. + """ + parts = [] + for stream in ("stdout", "stderr"): + try: + handle = getattr(sandbox, stream, None) + text = handle.read() if handle is not None else "" + if text: + parts.append(f"----- {stream} -----\n{text}") + except Exception as exc: # noqa: BLE001 - see the docstring + parts.append(f"----- {stream} unavailable: {type(exc).__name__}: {exc} -----") + return "\n".join(parts) + + +def _download_outputs(cfg: Config, run_id: str, dest: Path, client: Any) -> list[str]: + """Copy this run's directory out of the Volume. Returns what arrived. + + Reads the Volume rather than the sandbox, for the reason in the module + docstring: by the time `collect` runs, the sandbox that wrote these files no + longer exists to be read from. + """ + modal = _modal() + volume, _ = _volume(modal, cfg, client) + prefix = _run_dir(cfg, run_id).lstrip("/") + mount = str(cfg.get("modal", "mount_path", "/grad/out")).strip("/") + # Paths inside a Volume are relative to its mount point, so the mount prefix + # comes back off before anything is asked for. + inside = prefix[len(mount):].strip("/") if prefix.startswith(mount) else prefix + dest.mkdir(parents=True, exist_ok=True) + arrived: list[str] = [] + try: + entries = list(volume.iterdir(f"/{inside}", recursive=True)) + except Exception: # noqa: BLE001 - an empty or missing directory is a real outcome + return arrived + for entry in entries: + remote = getattr(entry, "path", None) + if not remote: + continue + relative = str(remote).split(inside, 1)[-1].strip("/") + if not relative: + continue + # The same guard `evaluate_candidate` puts on candidate filenames, for + # the same reason: these names come back from the Volume, which is + # written by the job, and `dest / "../../x"` resolves outside the run's + # artifacts directory. A traversal here writes to the researcher's disk. + if Path(relative).is_absolute() or ".." in Path(relative).parts: + continue + target = dest / relative + target.parent.mkdir(parents=True, exist_ok=True) + try: + with open(target, "wb") as fh: + for chunk in volume.read_file(str(remote)): + fh.write(chunk) + except Exception: # noqa: BLE001 - a directory entry, or a file that vanished + continue + arrived.append(relative) + return arrived + + +# --------------------------------------------------------------------------- +# submit +# --------------------------------------------------------------------------- +def _submit_args(p: argparse.ArgumentParser) -> None: + p.add_argument("--spec", required=True, help="path to the submission spec") + p.add_argument( + "--expect", + help="expectation id to bind to this run. REQUIRED unless --smoke: " + "no pre-registration, no submission", + ) + p.add_argument("--set", dest="overrides", action="append", default=[], metavar="KEY=VALUE") + p.add_argument("--gpu", help="Modal GPU, e.g. H100 or H100:8 (overrides the spec)") + p.add_argument("--task", help="task id for the ledger (defaults to the spec directory name)") + p.add_argument("--project", help="project to charge this run to (defaults to the current one)") + p.add_argument( + "--smoke", + action="store_true", + help="the gate-exempt, hard-capped one-step check from §6. Cannot train anything.", + ) + p.add_argument("--no-digest", action="store_true", help=argparse.SUPPRESS) + + +@cli.command("submit", "submit a sandbox (gated) or a smoke check (capped)", setup=_submit_args) +def cmd_submit(args: argparse.Namespace) -> dict[str, Any]: + cfg = config_mod.load() + sub = Submission.load( + args.spec, + overrides=dict(parse_override(o) for o in args.overrides), + resolve_digest=not args.no_digest, + ) + if args.gpu: + sub.target["gpu"] = args.gpu + + project_id = budget.resolve(args.project) + gpu = resolve_gpu(args.gpu, sub, cfg) + + if args.smoke: + if args.expect: + raise UsageError( + "--smoke does not take --expect: a smoke check is not a result and binds no prediction", + fix="drop --expect, or drop --smoke", + ) + result = run_smoke(sub, cfg, gpu=gpu, project=project_id) + from tools import preflight # noqa: PLC0415 + + preflight.record_check_result(sub.hash(), "smoke", result) + if not result.get("ok"): + raise GradError( + "smoke_failed", + result.get("reason", "the smoke check failed on the real target"), + exit_code=9, + fix=result.get("fix") or "read the smoke log under ledger/runs/", + detail=result, + ) + return {"smoke": result, "submission_hash": sub.hash()} + + command = _command_for(sub) + # Resolved before the gates so a misconfigured GPU or an impossible timeout + # is a configuration error rather than a gate refusal -- they have different + # exit codes because they need different actions. + rate = _rate_or_refuse(gpu, cfg) + timeout_s = _timeout_seconds(sub, cfg) + + summary = submit_lib.check(sub, args.expect, cfg, project=project_id) + # Then the backend and the credential, before any record exists: a missing + # package or an absent token is a configuration problem, not an in-flight + # run, and it must not leave a phantom estimate sitting on the ceiling. + client = _client() + + run_id, _ = submit_lib.record_submission( + sub, + expectation_id=args.expect, + platform=PLATFORM, + target={"gpu": gpu, "platform": PLATFORM, "rate_usd_per_hour": rate, + "timeout_s": timeout_s}, + command=command, + task=args.task, + project=project_id, + cfg=cfg, + ) + + try: + sandbox_id, volume_name = _create_sandbox( + sub, cfg, client=client, gpu=gpu, command=command, + timeout_s=timeout_s, run_id=run_id, + ) + except Exception as exc: # noqa: BLE001 - the SDK raises a wide family here + submit_lib.finish( + run_id, + status="submit_failed", + results={}, + cost_usd_actual=0.0, + artifacts_dir=submit_lib.artifacts_dir(run_id), + expectation=None, + extra={"error": str(exc)}, + ) + raise UpstreamError( + f"Modal refused the submission: {exc}", + fix="check the token, the GPU name, and that the image digest is pullable", + ) from exc + + submit_lib.attach_handle( + run_id, + {"sandbox_id": sandbox_id, "gpu": gpu, "volume": volume_name, + "run_dir": _run_dir(cfg, run_id), "timeout_s": timeout_s}, + ) + return { + "run_id": run_id, + "sandbox_id": sandbox_id, + "gpu": gpu, + "rate_usd_per_hour": rate, + "timeout_hours": round(timeout_s / 3600, 2), + "project": project_id, + "gates": summary, + "next": f"python -m tools.modal collect {run_id} --json", + } + + +# --------------------------------------------------------------------------- +# smoke +# --------------------------------------------------------------------------- +def run_smoke( + sub: Submission, cfg: Config, *, gpu: str | None = None, project: str | None = None +) -> dict[str, Any]: + """The §6 carve-out: gate-exempt, hard-capped, still ledgered. + + Same shape as every other backend's, and the caps come from the same place + (`gates.check_smoke_caps`) so "one step, minutes of wall clock, cents of + money" means the same thing here as on HF Jobs. Blocks, because it is bounded + to minutes by construction and preflight needs the answer. + """ + gpu = gpu or sub.target.get("smoke_gpu") or resolve_gpu(None, sub, cfg) + rate = _rate_or_refuse(gpu, cfg) + caps = gates.check_smoke_caps(sub, cfg, rate_usd_per_hour=rate, target_name=f"gpu {gpu!r}") + + project = project or budget.current_project() + # Everything that can fail for a configuration reason resolves before the + # ledger record exists, or a phantom in-flight estimate sits on the monthly + # ceiling for a run that never reached the platform -- and then goes stale + # and blocks every later submission. + client = _client() + + run_id = submit_lib.record_smoke_run( + sub, cfg=cfg, platform=PLATFORM, + target={"gpu": gpu, "platform": PLATFORM, "rate_usd_per_hour": rate}, + caps=caps, command=_command_for(sub), project=project, + ) + artifacts = submit_lib.artifacts_dir(run_id) + + command = _smoke_command(sub, caps) + try: + sandbox_id, _ = _create_sandbox( + sub, cfg, client=client, gpu=gpu, command=command, + timeout_s=int(caps["timeout_s"]), run_id=run_id, + ) + except ConfigError: + raise + except Exception as exc: # noqa: BLE001 + submit_lib.finish( + run_id, status="submit_failed", results={}, cost_usd_actual=0.0, + artifacts_dir=artifacts, expectation=None, extra={"error": str(exc)}, + ) + return {"ok": False, "reason": f"smoke submission failed: {exc}", + "fix": "check the Modal token and that the image digest is pullable", + "run_id": run_id} + + submit_lib.attach_handle(run_id, {"sandbox_id": sandbox_id, "gpu": gpu}) + sandbox = _reattach(sandbox_id, client) + state, code = _poll( + sandbox, + deadline=time.time() + float(caps["timeout_s"]), + interval=float(cfg.get("modal", "poll_interval_s", 20) or 20), + ) + logs = _logs(sandbox) + (artifacts / "smoke.log").write_text(logs, encoding="utf-8") + + ok = state == "completed" + elapsed = submit_lib.elapsed_hours(ls.run(run_id)) + cost = round(min(elapsed, float(caps["timeout_s"]) / 3600) * rate, 4) + submit_lib.finish( + run_id, + status="completed" if ok else "failed", + results={}, + cost_usd_actual=cost, + artifacts_dir=artifacts, + expectation=None, + extra={"sandbox_state": state, "exit_code": code, "smoke": True, + "cost_basis": "wall_clock"}, + ) + return { + "ok": ok, + "run_id": run_id, + "sandbox_id": sandbox_id, + "state": state, + "exit_code": code, + "gpu": gpu, + "cost_usd": cost, + "caps": caps, + "log": str(artifacts / "smoke.log"), + "output": "\n".join(logs.splitlines()[-25:]), + "reason": None if ok else f"the smoke sandbox ended {state} (exit {code})", + "fix": None if ok else f"read {artifacts / 'smoke.log'} -- this is the environment the real run would have used", + "scope": "remote; the only check that exercises the real image, data path, and hardware", + } + + +def _smoke_command(sub: Submission, caps: dict[str, Any]) -> list[str]: + """One step, real per-device batch size -- the same shape `tools/jobs.py` + builds, because §6's argument is about the check and not about the backend: + smoking at batch 2 does not test the thing that most often kills the run.""" + return [*_command_for(sub), "--steps", str(caps["steps"]), "--smoke"] + + +# --------------------------------------------------------------------------- +# evolve candidates +# --------------------------------------------------------------------------- +#: How many bytes of a candidate's output are kept, matching the other backends. +CANDIDATE_OUTPUT_BYTES = 8000 + + +def evaluate_candidate( + sub: Submission, + cfg: Config, + *, + candidate_id: str, + files: dict[str, str], + command: list[str], + timeout_s: int, + artifacts: Path, + gpu: str | None = None, +) -> dict[str, Any]: + """Run one evolve candidate in a Sandbox, and read its metrics back. + + **The delivery path is the simplest of the four backends, and that is + Modal's doing rather than ours.** `gpu.py` copies the pipeline to a host, + `kaggle.py` packs it into the notebook, and `jobs.py` smuggles the candidate + through a base64 environment variable because on HF Jobs the pipeline is in + the image and a candidate by definition is not. Modal builds the image + client-side, so a candidate is a second `add_local_dir` layered over the + first -- no encoding, no size ceiling, no prelude to unpack it. + + Like the other adapters: no ledger row, cost measured rather than estimated, + and a transport failure reported as one rather than as a candidate that + scored nothing. A campaign is a search, and a candidate that could not be + *delivered* says nothing about the mutation that produced it. + """ + import tempfile # noqa: PLC0415 - only the candidate path stages files + + gpu = gpu or resolve_gpu(None, sub, cfg) + artifacts.mkdir(parents=True, exist_ok=True) + started = time.time() + rate = gpu_rate(gpu, cfg) or 0.0 + + def _failed(message: str, **extra: Any) -> dict[str, Any]: + return { + "ok": False, + "exit_code": None, + "output": "", + "error": message, + "cost_usd": round((time.time() - started) / 3600.0 * rate, 4), + "gpu": gpu, + "where": f"modal:{candidate_id}", + **extra, + } + + try: + client = _client() + modal = _modal() + except GradError as exc: + return _failed(exc.message) + + with tempfile.TemporaryDirectory(prefix="grad-candidate-") as staging: + root = Path(staging) + for name, body in files.items(): + # Written under the staging root and never outside it: a candidate's + # filenames come from a model, and a `..` in one would otherwise be a + # path this process writes to on the researcher's machine. + target = (root / name).resolve() + if root.resolve() not in target.parents: + return _failed(f"candidate file {name!r} escapes the staging directory") + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(body, encoding="utf-8") + + workdir = str(cfg.get("modal", "workdir", "/grad/pipeline")) + out_dir = f"{str(cfg.get('modal', 'mount_path', '/grad/out')).rstrip('/')}/{candidate_id}" + try: + volume, _ = _volume(modal, cfg, client) + app = modal.App.lookup( + str(cfg.get("modal", "app_name", "grad")), create_if_missing=True, client=client + ) + image = _image(sub, modal, cfg, {**_job_env(sub, out_dir), "GRAD_CANDIDATE": candidate_id}) + # Layered *after* the pipeline, so a candidate replaces the file it + # is a mutation of rather than sitting beside it. + image = image.add_local_dir(str(root), workdir, copy=True) + sandbox = modal.Sandbox.create( + *_wrapped_command(command, sub, out_dir), + app=app, + image=image, + gpu=gpu, + timeout=int(timeout_s), + workdir=workdir, + volumes={str(cfg.get("modal", "mount_path", "/grad/out")).rstrip("/"): volume}, + client=client, + ) + except Exception as exc: # noqa: BLE001 - a refused sandbox is not a bad mutation + return _failed(f"the candidate could not be submitted: {exc}") + + state, code = _poll( + sandbox, + deadline=time.time() + int(timeout_s), + interval=float(cfg.get("modal", "poll_interval_s", 20) or 20), + ) + logs = _logs(sandbox) + (artifacts / "candidate.log").write_text(logs, encoding="utf-8") + _detach(sandbox) + + ok = state == "completed" + return { + "ok": ok, + # A real exit code, unlike HF Jobs, which reports only a state: Modal's + # `poll()` returns the process's own status, so this is measured. + "exit_code": code, + "output": logs[-CANDIDATE_OUTPUT_BYTES:], + "error": None if ok else f"the candidate's sandbox ended {state} (exit {code})", + "cost_usd": round((time.time() - started) / 3600.0 * rate, 4), + "gpu": gpu, + "sandbox_state": state, + "where": f"modal:{getattr(sandbox, 'object_id', candidate_id)}", + } + + +# --------------------------------------------------------------------------- +# status and collect +# --------------------------------------------------------------------------- +def _status_args(p: argparse.ArgumentParser) -> None: + p.add_argument("run_id") + + +@cli.command("status", "what a submitted run is doing right now", setup=_status_args) +def cmd_status(args: argparse.Namespace) -> dict[str, Any]: + """Poll one run without collecting it.""" + r = ls.run(args.run_id) + handle = r.get("handle") or {} + sandbox_id = handle.get("sandbox_id") + if not sandbox_id: + return {"run_id": r.id, "state": "no_handle", "collected": bool(r.collected)} + sandbox = _reattach(sandbox_id, _client()) + state, code = _state_of(sandbox) + return { + "run_id": r.id, + "sandbox_id": sandbox_id, + "state": state, + "exit_code": code, + "gpu": handle.get("gpu"), + "elapsed_hours": round(submit_lib.elapsed_hours(r), 3), + "collected": bool(r.collected), + } + + +def _collect_args(p: argparse.ArgumentParser) -> None: + p.add_argument("run_id") + p.add_argument("--wait", action="store_true", help="block until the sandbox finishes") + p.add_argument("--timeout", type=int, default=900, help="seconds, with --wait") + + +@cli.command("collect", "fetch outputs, compute deviations, write the run record", setup=_collect_args) +def cmd_collect(args: argparse.Namespace) -> dict[str, Any]: + """Closes the loop the model would otherwise close from memory. + + Writes results, actual cost and the deviations array, and leaves `verdict` + unset -- that is `ledger.py verdict`'s job, and judgement must not be able to + overwrite the record it judges. + """ + r = submit_lib.require_uncollected(args.run_id) + handle = r.get("handle") or {} + sandbox_id = handle.get("sandbox_id") + if not sandbox_id: + raise GradError( + "no_handle", + f"run {r.id} has no Modal sandbox id; it never reached the platform", + exit_code=3, + fix=f"python -m tools.ledger abandon {r.id} --reason '...' --json", + ) + + cfg = config_mod.load() + client = _client() + sandbox = _reattach(sandbox_id, client) + deadline = time.time() + (args.timeout if args.wait else 0) + state, code = _poll( + sandbox, deadline=deadline, + interval=float(cfg.get("modal", "poll_interval_s", 20) or 20), + ) + if state == "running": + raise GradError( + "still_running", + f"sandbox {sandbox_id} is still running", + exit_code=EXIT_RUNNING, + fix=f"python -m tools.modal collect {r.id} --wait --timeout 3600 --json", + detail={"run_id": r.id, "state": state}, + ) + + artifacts = submit_lib.artifacts_dir(r.id) + artifacts.mkdir(parents=True, exist_ok=True) + (artifacts / "sandbox.log").write_text(_logs(sandbox), encoding="utf-8") + + results: dict[str, Any] = {} + samples: dict[str, list[Any]] = {} + metrics_error = None + downloaded = _download_outputs(cfg, r.id, artifacts, client) + metrics_path = artifacts / Path(r.get("metrics_file") or "metrics.json").name + try: + results, samples = submit_lib.read_metrics(metrics_path) + except GradError as exc: + metrics_error = exc.message + + expectation = None + if r.get("expectation_id"): + try: + expectation = ls.expectation(r["expectation_id"]) + except GradError: + expectation = None + + cost, cost_warning = _actual_cost(r, handle, cfg) + record = submit_lib.finish( + r.id, + status="completed" if state == "completed" else "failed", + results=results, + cost_usd_actual=cost, + artifacts_dir=artifacts, + expectation=expectation, + samples=samples, + extra={ + "sandbox_state": state, + "exit_code": code, + "metrics_error": metrics_error, + "downloaded": downloaded, + "cost_warning": cost_warning, + "cost_basis": "wall_clock", + }, + ) + unjudged = [d for d in record["deviations"] if d.get("in_range") is not True] + return { + "run": record, + "artifacts": str(artifacts), + "downloaded": downloaded, + "cost_warning": cost_warning, + "needs_verdict": unjudged, + "next": ( + f"python -m tools.ledger verdict {r.id} --quantity {unjudged[0]['quantity']} " + "--verdict bug|real|inconclusive --note '...' --json" + ) if unjudged else None, + } + + +def _actual_cost(r: ls.Run, handle: dict[str, Any], cfg: Config) -> tuple[float, str | None]: + """What this run cost, and an honest note about how that was arrived at. + + **Wall clock from our own record, not Modal's accounting**, and the warning + says so on every run rather than only when something went wrong. Modal bills + per second from container start to exit; what is measurable here is the + interval between the ledger's `submitted_at` and now, which includes the + image pull and however long the run sat between finishing and being + collected. It is an upper bound, and the direction is the safe one for a + ceiling -- but it is not a measurement and the record must not imply it is. + + Bounded by the sandbox's own timeout, because the container cannot have run + longer than Modal would let it: collecting a week later must not book a + week of H100 time against the project. + """ + gpu = str(handle.get("gpu") or (r.get("target") or {}).get("gpu") or "") + rate = gpu_rate(gpu, cfg) + if rate is None: + return 0.0, ( + f"no rate configured for {gpu!r}, so this run is booked at $0 and its cost is " + "not bounded by the ceiling -- price it under [modal.gpu_rates] before the next one" + ) + elapsed = submit_lib.elapsed_hours(r) + timeout_h = float(handle.get("timeout_s") or 0) / 3600 or None + if timeout_h: + elapsed = min(elapsed, timeout_h) + return round(elapsed * rate, 4), ( + "cost is wall clock from submission priced against [modal.gpu_rates], not Modal's own " + "billing. It is long in one direction (it includes the image pull and any delay before " + "collection) and short in another (GPU time only; the CPU and memory the sandbox held " + "are not counted). Collect promptly if the number matters." + ) + + +# --------------------------------------------------------------------------- +# account and ceilings +# --------------------------------------------------------------------------- +def _account_args(p: argparse.ArgumentParser) -> None: + p.add_argument("--check", action="store_true", help="verify the stored token pair authenticates") + + +@cli.command("account", "which Modal workspace the stored credentials reach", setup=_account_args) +def cmd_account(args: argparse.Namespace) -> dict[str, Any]: + """Answer "is this wired up" without submitting anything. + + `--check` is the useful half: `credential status` says a secret is *stored*, + which is a different claim from a secret that *works*, and the gap between + them is only ever discovered at the worst moment otherwise. + """ + stored = { + name: bool(credentials.get(name, required=False)) + for name in (credentials.MODAL_TOKEN_ID, credentials.MODAL_TOKEN_SECRET) + } + out: dict[str, Any] = {"stored": stored, "authenticated": None} + if not args.check: + out["next"] = "python -m tools.modal account --check --json" + return out + try: + client = _client() + modal = _modal() + modal.App.lookup( + str(config_mod.load().get("modal", "app_name", "grad")), + create_if_missing=True, + client=client, + ) + out["authenticated"] = True + except ConfigError: + raise + except Exception as exc: # noqa: BLE001 + out["authenticated"] = False + out["error"] = f"{type(exc).__name__}: {exc}" + return out + + +@cli.command("gpus", "which accelerators are priced, and at what") +def cmd_gpus(_: argparse.Namespace) -> dict[str, Any]: + """The rate table, which is also the list of what may be asked for. + + An unpriced GPU is refused at submit rather than assumed free, so this is not + decoration: it is the set of hardware this installation can bound the cost of. + """ + cfg = config_mod.load() + rates = cfg.get("modal", "gpu_rates", {}) or {} + return { + "gpus": dict(sorted(rates.items())), + "default": cfg.get("modal", "default_gpu", "H100"), + "max_hours": min(float(cfg.get("modal", "max_hours", MODAL_MAX_HOURS)), MODAL_MAX_HOURS), + "note": "dollars per hour; a count suffix like H100:8 multiplies the rate", + } + + +def _ceilings_args(p: argparse.ArgumentParser) -> None: + p.add_argument("--project", help="which project's allocation to report (defaults to current)") + + +@cli.command("ceilings", "spend headroom for this backend", setup=_ceilings_args) +def cmd_ceilings(args: argparse.Namespace) -> dict[str, Any]: + """The same two ceilings `tools/jobs.py ceilings` reports, never conflated: + the machine's rolling spend, and this project's own allocation (§15).""" + cfg = config_mod.load() + window = int(cfg.get("spend", "window_days", 30)) + stale = [r.id for r in ls.stale_runs(cfg=cfg)] + payload: dict[str, Any] = { + "platform": PLATFORM, + "per_job_usd": cfg.get("spend", "per_job_usd"), + "monthly_usd": cfg.get("spend", "monthly_usd"), + "rolling": {k: v for k, v in ls.rolling_spend(window).items() if k != "runs"}, + "in_flight_runs": [r.id for r in ls.in_flight()], + "stale_runs": stale, + "blocked": bool(stale), + "note": ( + "Modal bills per second against [modal.gpu_rates], so the [spend] ceilings are " + "the only gate here -- there is no hours allowance and exit 13 never comes from " + "this backend." + ), + } + project_id = budget.resolve(args.project) + if project_id and budget.exists(project_id): + payload["project"] = budget.status(project_id) + else: + payload["project"] = {"project": project_id, "bounded": False} + return payload + + +if __name__ == "__main__": + main(cli) diff --git a/tools/nb.py b/tools/nb.py index 8504a2a..9578770 100644 --- a/tools/nb.py +++ b/tools/nb.py @@ -99,6 +99,14 @@ def _start_kernel(name: str, kernel_name: str) -> dict[str, Any]: stdout=fh, stderr=subprocess.STDOUT, stdin=subprocess.DEVNULL, + # Explicit rather than inherited, because this kernel has two + # parents. Started from the agent's Bash it would inherit + # `agent.interpreter_env`'s copy; started from the notebook window + # it inherits the desktop app's environment, which has none -- and + # "reading a paper crashes in the notebook but not in the shell" is + # a difference nobody should have to discover. See + # `core/spawn.py:utf8_env`. + env={**os.environ, **spawn.utf8_env()}, **spawn.detached(), ) conn.with_suffix(".pid").write_text(str(proc.pid), encoding="utf-8") diff --git a/tools/setup.py b/tools/setup.py index dc5f1d8..9a670a7 100644 --- a/tools/setup.py +++ b/tools/setup.py @@ -350,6 +350,15 @@ def cmd_host(args: argparse.Namespace) -> dict[str, Any]: "needs_kaggle_account": True, "fix": "python -m tools.kaggle account --set --json", }, + # Two credentials, and both are required: a token id without its secret + # authenticates nothing, so a workspace holding one half is not "partly + # ready", it is unready with a misleading panel. `missing` naming both is + # what stops someone going round the loop twice. + "modal": { + "credentials": (credentials.MODAL_TOKEN_ID, credentials.MODAL_TOKEN_SECRET), + "needs_host": False, + "fix": "python -m tools.jobs credential set modal_token_id # and modal_token_secret", + }, } diff --git a/tools/task.py b/tools/task.py index a0bdf89..03a3476 100644 --- a/tools/task.py +++ b/tools/task.py @@ -145,6 +145,12 @@ def cmd_start(args: argparse.Namespace) -> dict[str, Any]: stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + # Explicit for the reason `tools/nb.py` gives at its kernel spawn: this + # supervisor runs *the agent's own command*, and it has two possible + # parents -- the agent's Bash, which carries UTF-8 Mode, and the tasks + # window, which does not. A command that reads a paper must not depend + # on which button started it. See `core/spawn.py:utf8_env`. + env={**os.environ, **spawn.utf8_env()}, # Detached, so it outlives this CLI -- which exits in a moment and is the # whole point. See `core/spawn.py` for why this is not DETACHED_PROCESS. **spawn.detached(), diff --git a/tools/workspace.py b/tools/workspace.py index 5181290..faa211f 100644 --- a/tools/workspace.py +++ b/tools/workspace.py @@ -320,5 +320,55 @@ def _remove(source: Path, names: list[str]) -> list[str]: return removed +# --------------------------------------------------------------------------- +# version control +# --------------------------------------------------------------------------- +def _vcs_args(p: argparse.ArgumentParser) -> None: + p.add_argument( + "action", + choices=("init", "status", "log", "commit"), + help="init: start versioning this workspace; commit: checkpoint now", + ) + p.add_argument("--message", default="manual checkpoint", help="the commit subject") + p.add_argument("--limit", type=int, default=20, help="how many checkpoints `log` shows") + + +@cli.command("vcs", "keep a local history of the research", setup=_vcs_args) +def cmd_vcs(args: argparse.Namespace) -> dict[str, Any]: + """Version the workspace, locally, with no remote. + + `init` is deliberate and one-time: creating a repository inside somebody's + folder is a side effect they did not ask for. Everything after it is + automatic -- a run collected, a verdict recorded and a project's documents + regenerated each leave a commit, because those are the moments the system + already treats as meaningful. + + There is no `push` and no remote, and that is a decision rather than an + omission: a research workspace holds the pipeline, the data pointers and + whatever a notebook has printed, and publishing it is not a thing to make + one flag away. See `core/vcs.py`. + """ + from core import vcs # noqa: PLC0415 + + if args.action == "init": + result = vcs.initialise() + if result.get("error"): + raise UsageError(result["error"], fix=result.get("fix")) + return { + **result, + "next": "nothing — collect, verdict and project sync now checkpoint on their own", + } + if args.action == "commit": + if not vcs.enabled(): + raise UsageError( + "this workspace is not versioned", + fix="python -m tools.workspace vcs init --json", + ) + return vcs.checkpoint(args.message) + if args.action == "log": + return {"root": str(vcs.root()), "checkpoints": vcs.history(args.limit)} + return vcs.status() + + if __name__ == "__main__": main(cli) diff --git a/ui/app.py b/ui/app.py index bc44240..5ae0023 100644 --- a/ui/app.py +++ b/ui/app.py @@ -712,7 +712,18 @@ async def rewind_to(self, index: int) -> dict[str, Any]: # whole, and a message claiming the memory went back would be describing # the one outcome `core/rewind.py` exists to make impossible to miss. resumed = bool(anchor) and agent.rewind_supported() - drops = self._drops_turn(anchor) if resumed and plan["turns"] == 1 else None + # The first prompt after the anchor: the one this rewind is dropping, and + # therefore the point the files should go back to. Read once and used + # twice, because the two uses want different conditions -- the SDK only + # accepts `resume_drops_turn` for a single-turn rewind, while restoring + # files to the *earliest* dropped prompt is right for any number of them. + dropped_prompt = self._drops_turn(anchor) + drops = dropped_prompt if resumed and plan["turns"] == 1 else None + # Files first, because this one is a control request and the two after it + # are not: `rewind_files` needs a live client, and the next statement + # closes it. Ordering the other way round would have made this the only + # half of a rewind that silently never ran. + restored = await self._rewind_files(dropped_prompt) # The client goes before anything is written, so a rewind cannot leave a # live conversation running against a transcript that has moved out from # under it. `start()` builds the replacement on the next turn. @@ -738,7 +749,10 @@ async def rewind_to(self, index: int) -> dict[str, Any]: self.settled = [ *plan["keep"], rewind.record( - dropped=plan["dropped"], resumed=resumed, anchor=anchor if resumed else None + dropped=plan["dropped"], + resumed=resumed, + anchor=anchor if resumed else None, + files=restored, ), ] self._persist() @@ -767,11 +781,59 @@ async def rewind_to(self, index: int) -> dict[str, Any]: ) else: message = f"rewound {what} on screen — there is no conversation to put back" - return {"ok": True, "message": message, "prompt": plan["prompt"], "resumed": resumed} + # Only said when it happened. A rewind that restored nothing says nothing + # about files rather than reporting an absence: the common case is a + # conversation that edited none, and "no files were restored" reads as a + # failure of something that was never attempted. + if restored: + message += "; files it edited are back as they were" + return { + "ok": True, + "message": message, + "prompt": plan["prompt"], + "resumed": resumed, + "files": restored, + } + + async def _rewind_files(self, dropped_prompt: str | None) -> bool: + """Put the files back to what they were before the dropped prompt ran. + + Best effort by construction, and the return value is the whole point: + the caller words its result on it rather than announcing a restore it did + not perform. Every reason this can decline is a real and ordinary one -- + an SDK too old to checkpoint, a rewind with no live conversation to name + a prompt in, a session that has already been closed -- and none of them + is a reason to refuse to rewind the transcript. + + Narrower than it sounds, which the message the caller builds is careful + about: the CLI checkpoints around its own editing tools, so this returns + work the agent did with `Write` and `Edit`. What a `Bash` command wrote + stays written, and that includes everything a submitter did on a backend. + The ledger is append-only for exactly that reason -- an undo that reached + into `ledger/runs.jsonl` would be erasing evidence, not work. + """ + import agent # noqa: PLC0415 - imported here so the UI can load without the SDK + + client = self.client + if client is None or not dropped_prompt or not agent.checkpointing_supported(): + return False + try: + await client.rewind_files(dropped_prompt) + except Exception: # noqa: BLE001 - see the docstring + log.debug("the files did not move with the rewind", exc_info=True) + return False + return True - def _drops_turn(self, anchor: str) -> str | None: + def _drops_turn(self, anchor: str | None) -> str | None: """The uuid of the prompt this rewind means to discard, for the CLI's check. + `anchor` is optional because rewinding to the *first* prompt of a session + keeps nothing, so there is no last-entry-of-the-last-kept-turn to anchor + on. That is the "start over" rewind, and it is the one where restoring + files matters most -- so a missing anchor means "the first prompt in the + conversation" rather than "no prompt at all", which is what it used to + mean and why `rewind_to(0)` moved the transcript and left the work. + Read out of the SDK's own transcript rather than captured live, and that is the cheaper half of a real trade. Capturing it would mean asking every session for `replay-user-messages` and carrying `UserMessage` objects @@ -793,8 +855,10 @@ def _drops_turn(self, anchor: str) -> str | None: return None # The first prompt *after* the anchor: the SDK's rule of thumb is that # `resume_session_at` names the last entry kept and `resume_drops_turn` - # the prompt of the turn immediately following it. - found = False + # the prompt of the turn immediately following it. With no anchor there + # is nothing before the first prompt, so `found` starts true and the + # answer is the first user message in the conversation. + found = anchor is None for message in messages: if found and getattr(message, "type", None) == "user": return getattr(message, "uuid", None) diff --git a/ui/desktop.py b/ui/desktop.py index 08003c5..4a17ea0 100644 --- a/ui/desktop.py +++ b/ui/desktop.py @@ -772,11 +772,26 @@ def window_args() -> dict[str, Any]: A size is always returned; a position only when there is a saved one that lands on a screen that exists right now. + + **`text_select` is here because pywebview's default is `False`, and its + default is not a preference -- it injects + `body { user-select: none; cursor: default }` into the page.** Everything in + the workspace is text somebody might need to copy: a run id to paste into a + command, a traceback to search for, a metric out of the ledger, the agent's + own answer. None of it could be selected in the desktop app, and all of it + could in the browser UI, which is why this survived -- development happens + in a browser and the injected rule is not in any stylesheet to grep for. + + Turning it on restores what `ui/tokens.py` already assumed. The sheet puts + `user-select: none` on exactly three things -- the title bar, the split + handle, and everything while a drag is in flight -- which is a set that only + makes sense if selection is *on* everywhere else. It was written against the + browser, where it is. """ saved = read_geometry() width = max(MIN_SIZE[0], int(saved.get("width") or DEFAULT_SIZE[0])) height = max(MIN_SIZE[1], int(saved.get("height") or DEFAULT_SIZE[1])) - args: dict[str, Any] = {"width": width, "height": height} + args: dict[str, Any] = {"width": width, "height": height, "text_select": True} if saved.get("maximized"): args["maximized"] = True if "x" in saved and "y" in saved: diff --git a/ui/jupyter_theme.py b/ui/jupyter_theme.py index 82926bb..cf2e759 100644 --- a/ui/jupyter_theme.py +++ b/ui/jupyter_theme.py @@ -31,6 +31,7 @@ import sys from pathlib import Path +from typing import Any from core import paths from ui import tokens @@ -54,8 +55,19 @@ def repo_path() -> Path: return Path(__file__).resolve().parents[1] / "config" / "jupyter" / "custom" / "custom.css" -def stylesheet() -> str: - c = tokens.COLOUR +#: Which JupyterLab base each palette re-tokens. `custom.css` cannot register a +#: named theme, so it overrides whichever base is selected -- and the base has to +#: match, or dark defaults sit under cream overrides in every corner the sheet +#: below does not name. There are a lot of those corners: the file browser, the +#: command palette, the status bar, every menu. +LAB_BASE: dict[str, str] = { + "light": "JupyterLab Light", + "dark": "JupyterLab Dark", +} + + +def stylesheet(theme: str | None = None) -> str: + c = tokens.palette(theme) return f"""{BANNER} /* ------------------------------------------------------------------ tokens */ @@ -220,8 +232,8 @@ def stylesheet() -> str: .jp-RenderedHTMLCommon table {{ border-collapse: collapse; font-family: {tokens.FONT_MONO}; }} .jp-RenderedHTMLCommon thead th {{ - background: {c['ink']}; - color: {c['paper']}; + background: {c['fill']}; + color: {c['fill-ink']}; text-align: left; padding: 7px 9px; font-size: 10px; @@ -235,17 +247,17 @@ def stylesheet() -> str: /* ----------------------------------------------------------------- chrome */ .jp-SideBar, .jp-FileBrowser, .jp-DirListing {{ background: {c['paper-sunk']}; }} -.jp-DirListing-item.jp-mod-selected {{ background: {c['ink']}; color: {c['paper']}; }} +.jp-DirListing-item.jp-mod-selected {{ background: {c['fill']}; color: {c['fill-ink']}; }} .jp-Toolbar-item .jp-ToolbarButtonComponent:hover {{ background: {c['paper-sunk']}; }} .lm-TabBar-tab.lm-mod-current {{ - background: {c['ink']} !important; - color: {c['paper']} !important; + background: {c['fill']} !important; + color: {c['fill-ink']} !important; border: 0; }} .lm-TabBar-tab {{ background: {c['paper-sunk']}; border-right: 1px solid {c['rule-soft']}; }} .jp-Statusbar, #jp-bottom-panel {{ - background: {c['ink']}; - color: {c['paper']}; + background: {c['fill']}; + color: {c['fill-ink']}; font-family: {tokens.FONT_MONO}; font-size: 11px; }} @@ -259,17 +271,115 @@ def stylesheet() -> str: .cm-operator, .cm-punctuation {{ color: {c['muted']}; }} .cm-editor .cm-ruler {{ border-right: 1px dashed {c['rule-mid']}; }} .cm-cursor {{ border-left: 2px solid {c['ink']}; }} -.cm-editor .cm-selectionBackground {{ background: {c['attention']} !important; }} +.cm-editor .cm-selectionBackground {{ background: {c['attention']} !important; + color: {c['on-attention']} !important; }} """ -def write(path: Path | None = None) -> Path: +def write(path: Path | None = None, theme: str | None = None) -> Path: path = path or repo_path() path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(stylesheet(), encoding="utf-8") + path.write_text(stylesheet(theme), encoding="utf-8") return path +def install(theme: str | None = None) -> dict[str, Any]: + """Put the sheet and the base-theme selection where Lab will read them. + + Called by `tools/lab.py` before the server starts, and it closes two things + at once. + + The first is the theme: `custom.css` re-tokens a base rather than + registering one, so the base has to match the palette or Lab's own chrome -- + file browser, command palette, menus, every corner the sheet does not name + -- keeps the defaults of the wrong one. + + The second predates the theme and is the larger of the two. + `JUPYTER_CONFIG_DIR` is `paths.root()/config/jupyter` -- the *workspace* -- + and only the checkout has that directory in it. The installer asks for a + workspace separate from the code and gives good reasons for it, and every + install that took the advice has been starting Lab with `--custom-css` + aimed at a file that does not exist and `--ServerApp.config_file` at another + one. That is stock JupyterLab inside Grad's chrome, and the server config + that sets the framing headers never loading -- exactly the seam this module + was written to close, on the recommended layout. + + So the shipped files are seeded when they are absent, the way + `core/paths.py:_shipped` resolves the other three code-adjacent paths, and + only `custom.css` is rewritten every time: it is generated, and the other two + are documents someone may have edited. + + Never raises: a workspace whose config directory cannot be written is one + where Lab should still start, unstyled. + """ + import shutil # noqa: PLC0415 + + from core import jsonl # noqa: PLC0415 + + chosen = str(theme or tokens.DEFAULT_THEME).lower() + if chosen not in LAB_BASE: + chosen = tokens.DEFAULT_THEME + result: dict[str, Any] = { + "theme": chosen, "written": [], "seeded": [], "skipped": None, "error": None, + } + try: + config_dir = target().parent.parent + shipped_dir = repo_path().parent.parent + config_dir.mkdir(parents=True, exist_ok=True) + + # Seeded, not overwritten. `jupyter_server_config.py` reads + # GRAD_UI_ORIGIN and GRAD_LAB_PORT so the framing headers and the port + # cannot drift apart; a workspace without it has no framing policy at + # all, which is a security-relevant absence rather than a cosmetic one. + if shipped_dir != config_dir: + for name in ("jupyter_server_config.py", "overrides.json"): + source, destination = shipped_dir / name, config_dir / name + if source.is_file() and not destination.exists(): + shutil.copyfile(source, destination) + result["seeded"].append(str(destination)) + + # The workspace *is* the checkout in a default install, and then + # `target()` is the tracked, generated `custom.css` that + # `test_ui_theme.py` asserts equals `stylesheet()`. Writing the dark + # sheet there would dirty the repository and fail that test for anyone + # who had switched theme -- so the pair is left exactly as committed. + # + # Left *as a pair*, which is the part worth stating: skipping only the + # sheet while still switching the base theme would put dark JupyterLab + # defaults under a cream override, which is the mismatch `LAB_BASE` + # exists to prevent. Both are skipped, and the caller is told, because a + # notebook that stays cream in a dark workspace is otherwise a silent + # disagreement. Keeping the research in its own folder -- which the + # installer already recommends and the README already argues for -- is + # what makes a dark Lab available. + if config_dir == shipped_dir: + result["skipped"] = ( + "the workspace is the installation, where config/jupyter/custom/custom.css is " + "a tracked generated file; JupyterLab keeps the light theme here" + ) + return result + + sheet = target() + write(sheet, chosen) + result["written"].append(str(sheet)) + + # Merged rather than replaced: `overrides.json` carries the handoff's + # 88-column ruler and whatever else has been set in it, and rewriting the + # file to hold one key would silently drop the rest. + overrides_path = config_dir / "overrides.json" + current = jsonl.read_json(overrides_path) + if not isinstance(current, dict): + current = jsonl.read_json(shipped_dir / "overrides.json") or {} + themes = dict(current.get("@jupyterlab/apputils-extension:themes") or {}) + themes["theme"] = LAB_BASE[chosen] + current["@jupyterlab/apputils-extension:themes"] = themes + jsonl.write_json(overrides_path, current) + result["written"].append(str(overrides_path)) + except Exception as exc: # noqa: BLE001 - see the docstring + result["error"] = f"{type(exc).__name__}: {exc}" + return result + + def main(argv: list[str] | None = None) -> int: argv = sys.argv[1:] if argv is None else argv if "--write" in argv: diff --git a/ui/models.py b/ui/models.py index dfbfecf..0c3b91f 100644 --- a/ui/models.py +++ b/ui/models.py @@ -746,6 +746,18 @@ def sessions_model(current: str | None = None) -> dict[str, Any]: "Kaggle kernels — the free GPU/TPU backend; useless without the username", "backend", ), + # The ninth and tenth. Both halves are secret and either one alone + # authenticates nothing, so they are two rows rather than one -- the panel + # shows what is stored, and "half a token pair" is a state worth being able + # to see. + "modal_token_id": ( + "Modal sandboxes — H100s and up, billed by the second; needs the secret too", + "backend", + ), + "modal_token_secret": ( + "the other half of the Modal pair; neither authenticates alone", + "backend", + ), "voyage_key": ("the reranker and the local index's embeddings (costs credits)", "retrieval"), "openrouter_key": ( "optional second rail for the reranker; Voyage is used by default", @@ -843,6 +855,124 @@ def setup_needed() -> bool: return not stored +#: The first-run sequence: what a machine that has never been set up still needs, +#: in the order it needs it, and what each one buys. +#: +#: `blocking` is the whole design of this. Two of the three genuinely stop the +#: app being useful -- with no token nothing authenticates, and with no project +#: every run is charged to nobody and no ceiling bounds anything. A backend does +#: not: the kernel, the funnel and the ledger all work on this machine alone, and +#: putting "configure a GPU backend" in front of someone who opened the app to +#: read a paper would be the wizard this project already decided not to write. +FIRST_RUN_STEPS: tuple[dict[str, Any], ...] = ( + { + "id": "token", + "title": "authenticate", + "why": "every model call goes through your Claude subscription; nothing runs without it", + "blocking": True, + "opens": "setup", + "step": "token", + }, + { + "id": "project", + "title": "create a project", + "why": "every run, ceiling and report is filed against one — without it spend is unbounded", + "blocking": True, + "opens": "projects", + "step": None, + }, + { + "id": "backend", + "title": "choose where runs execute", + "why": "optional: the notebook, the funnel and the ledger all work on this machine alone", + "blocking": False, + "opens": "setup", + "step": "backends", + }, +) + + +def first_run_needed() -> bool: + """Is this machine mid-setup? The cheap half of `first_run`. + + Separate because the caller is `ui/state.py:opening_windows`, which decides + what is on screen before anything is drawn, and its docstring promises the + decision costs a credential-store read and nothing else. `first_run` reads + the *backend* readiness too, and that loads a `Config` -- which is not free, + and, worse, repopulates the config cache. `Workspace.switch_root` clears + that cache on purpose when the workspace moves, and a reader on the layout + path put it straight back, leaving the new workspace resolving the old + one's configuration. + + So the arrangement asks only what it acts on. The two blocking conditions + are exactly the ones that decide whether the setup window opens; the backend + is reported in the panel and never opens anything. + """ + from core import budget as budget_mod + + if setup_needed(): + return True + current, _ = _safe(budget_mod.current_project, None) + if not current: + return True + exists, _ = _safe(lambda: budget_mod.exists(current), False) + return not exists + + +def first_run() -> dict[str, Any]: + """What a fresh install still needs, and whether to say so at all. + + **Derived, never stored, and that is deliberate.** The obvious design is a + `first_run_done` flag in the settings overlay, and it is wrong in both + directions: someone who dismisses the panel before creating a project never + sees it again on a machine that still has no project, and someone who moves + to a new workspace gets no panel because a *different* workspace once set a + flag. Reading the three conditions means the panel is present exactly while + it is true, and disappears by being satisfied rather than by being dismissed. + + So there is nothing to reset and no state to migrate: `active` goes false the + moment a token and a project exist, which is the moment the panel has + finished being useful. + + Never raises. This runs on the build path of the one window whose job is to + be usable when nothing else is. + """ + from core import budget as budget_mod + + done: dict[str, bool] = {} + done["token"] = not setup_needed() + + def _has_project() -> bool: + # Both halves: a workspace can hold projects with none selected, and the + # selection file can name a project that was closed. Either way there is + # nothing for a run to be charged to. + current = budget_mod.current_project() + return bool(current) and budget_mod.exists(current) + + done["project"], _ = _safe(_has_project, False) + + def _has_backend() -> bool: + from core import config as config_mod + from tools import setup as setup_tool + + return any(b.get("ready") for b in setup_tool.readiness(config_mod.load())) + + done["backend"], _ = _safe(_has_backend, False) + + steps = [{**step, "done": bool(done.get(step["id"]))} for step in FIRST_RUN_STEPS] + remaining = [s for s in steps if not s["done"]] + return { + "steps": steps, + "done": len([s for s in steps if s["done"]]), + "total": len(steps), + # Only the blocking ones decide whether this is a machine mid-setup. A + # workspace with a token and a project is set up; an unconfigured backend + # is a choice, not an unfinished step. + "active": any(s["blocking"] and not s["done"] for s in steps), + "next": remaining[0] if remaining else None, + } + + def setup_model() -> dict[str, Any]: """What is configured, what is not, and what each answer would buy. @@ -957,6 +1087,13 @@ def setup_model() -> dict[str, Any]: # Nothing here blocks the app; this is what the appbar and the first-run # arrangement ask about. "complete": token["ready"] and any(b["ready"] for b in backends), + # What a machine that has never been set up still needs. Folded into this + # model rather than fetched separately by the window, so the panel and + # the steps below it are one snapshot -- two reads could disagree, and + # "authenticate" ticking green above a token step that still says + # missing is exactly the kind of disagreement a first-run panel must not + # produce. + "first_run": _safe(first_run, {})[0] or {}, "error": cfg_error or cred_error or backend_error, } diff --git a/ui/render.py b/ui/render.py index eff5323..01dbb65 100644 --- a/ui/render.py +++ b/ui/render.py @@ -33,7 +33,6 @@ import html import logging from pathlib import Path -from typing import Any from core import paths from ui import tokens @@ -43,7 +42,8 @@ #: `(resolved path) -> (mtime, size, html)`. One entry per notebook; a workspace #: with a hundred of them and every one opened is still a few megabytes, and the #: alternative is re-rendering the same unchanged file on every redraw. -_CACHE: dict[str, tuple[float, int, str]] = {} +#: path -> (mtime, size, theme, document). See `notebook_html`. +_CACHE: dict[str, tuple[float, int, str, str]] = {} class NotAllowed(Exception): @@ -102,15 +102,24 @@ def notebook_html(name: str) -> str: """A complete, script-free HTML document for one notebook.""" path = resolve(name) stat = path.stat() + theme = _theme() cached = _CACHE.get(str(path)) - if cached and cached[0] == stat.st_mtime and cached[1] == stat.st_size: - return cached[2] + # The palette is in the cache key, not just in the render. A cache keyed on + # mtime alone would go on serving the cream document after a theme switch, + # for exactly as long as nobody edited the notebook. + if cached and cached[:3] == (stat.st_mtime, stat.st_size, theme): + return cached[3] body = _body(path) - document = _document(name, body) - _CACHE[str(path)] = (stat.st_mtime, stat.st_size, document) + document = _document(name, body, theme) + _CACHE[str(path)] = (stat.st_mtime, stat.st_size, theme, document) return document +def _theme() -> str: + """The workspace's palette, or the default. See `tokens.resolved_theme`.""" + return tokens.resolved_theme() + + def _body(path: Path) -> str: """The notebook as HTML, or a readable explanation of why not. @@ -140,7 +149,7 @@ def _body(path: Path) -> str: return body -def _document(name: str, body: str) -> str: +def _document(name: str, body: str, theme: str | None = None) -> str: """Wrap the fragment in the workspace's own typography. nbconvert's own stylesheet is Lab's, and dropping it into a pane that is @@ -148,7 +157,7 @@ def _document(name: str, body: str) -> str: palette below is `ui/tokens.py`'s, read from it rather than copied, so the render cannot drift away from the rest of the app. """ - c = tokens.COLOUR + c = tokens.palette(theme) return f""" diff --git a/ui/shell.py b/ui/shell.py index bcd01d9..6468071 100644 --- a/ui/shell.py +++ b/ui/shell.py @@ -22,13 +22,20 @@ from typing import Any -from ui import desktop, kit, registry +from ui import desktop, kit, registry, tokens from ui.state import POLL_SECONDS, Workspace def build(workspace: Workspace) -> None: """Assemble the whole shell for one connected client.""" - from nicegui import ui + from nicegui import context, ui + + # Held for everything that has to reach this browser from outside a render. + # `context.client` resolves through the slot stack, which is empty in a + # spawned task -- see `Workspace.paint_theme`, whose first version was + # written without this and silently never repainted anything. + workspace.client = context.client + apply_theme(workspace.theme()) roots: dict[str, Any] = {} bars: dict[str, Any] = {} @@ -275,6 +282,57 @@ def _appbar(workspace: Workspace, windows: Any, projects: Any, workspaces: Any) ) +def apply_theme(theme: str) -> None: + """Put the palette on ``, where the dark block's selector looks. + + Both palettes are in the stylesheet already -- `ui/app.py` adds it once, at + import, with `shared=True`, so there is nothing to re-inject and the switch + is this attribute and the cascade. That is also what keeps it inside the + design's motion rule: an attribute flip is an instant state swap. + + Written with `ui.add_body_html` on the way in and `run_javascript` on a + change, because those are two different moments: at build time there is no + socket yet and a `run_javascript` would be sent to nobody, and after build + there is no head left to add to. Both write the same attribute. + + Never raises. A theme is decoration, and a workspace that will not open + because it could not read one is a worse outcome than a cream window. + """ + import logging # noqa: PLC0415 + + from nicegui import ui # noqa: PLC0415 + + name = str(theme or tokens.DEFAULT_THEME) + try: + ui.add_body_html( + f'' + ) + except Exception: # noqa: BLE001 - see the docstring + logging.getLogger("grad.ui").debug("theme not applied at build", exc_info=True) + + +def switch_theme(workspace: Workspace, theme: str) -> None: + """Persist the choice and move the live page to it, in that order. + + The order matters on the one path that can fail: if the write refuses -- an + unwritable app directory, a name a newer version wrote -- the page stays on + the palette that is still recorded, rather than showing one that will be + gone on the next launch. + + **Synchronous, and that is the fix rather than a tidy-up.** This was a + coroutine handed to `Workspace.spawn`, which meant the repaint ran in a + spawned task -- where NiceGUI's slot stack is empty, `context.client` cannot + be resolved, and `ui.run_javascript` raises before it reaches a socket. The + write and the notice both succeeded, so the status bar said "theme: dark" + over a workspace that was still cream. There is nothing here to await: + `set_theme` writes a file and `paint_theme` fires a message, so making it a + coroutine bought only the task that broke it. + """ + workspace.set_theme(theme) + workspace.paint_theme(theme) + + def _used_share(session: dict[str, Any]) -> float: ceiling = session.get("ceiling_usd") or 0.0 if not ceiling: @@ -569,6 +627,13 @@ async def _browse(workspace: Workspace, field: Any) -> None: ("full", "FULL", "⌥3", "the focused window, the rest at the edge"), ) +#: The palettes, and what each is for. Ordered light-first because that is the +#: default and the list should not read as though the app has an opinion. +THEME_ROWS = ( + ("light", "LIGHT", "cream paper, ink rules — the design as drawn"), + ("dark", "DARK", "the same design at night; figures stay on white"), +) + def _windows_menu(ui: Any, workspace: Workspace) -> kit.Menu: """`⋯` and `⌘K`: which windows are open, and how they are arranged. @@ -616,6 +681,23 @@ def _draw_windows_menu(workspace: Workspace, body: Any, menu: kit.Menu) -> None: row = kit.menu_row(chord, caption, hint, title=hint) row.on("click", lambda _=None, p=name: (workspace.preset(p), menu.close())) + # One row rather than a settings page. The theme is the only thing in + # the app with exactly two values and no consequences, and putting it + # behind the setup window would make a two-second decision a + # four-click one. `setup` still shows it, because `setup show` is + # where "what is this workspace wired to" is answered. + kit.text("APPEARANCE", "grad-caption").style("margin-top: 14px") + current = workspace.theme() + for name, caption, hint in THEME_ROWS: + row = kit.menu_row( + "■" if name == current else "□", caption, hint, + open=name == current, title=hint, + ) + row.on( + "click", + lambda _=None, t=name: (switch_theme(workspace, t), menu.redraw()), + ) + kit.text( "drag a title bar to move a window · drop it on another to swap them", "grad-caption", diff --git a/ui/splash.py b/ui/splash.py index 80221af..cdf3cc4 100644 --- a/ui/splash.py +++ b/ui/splash.py @@ -65,6 +65,19 @@ # --------------------------------------------------------------------------- # the parent's side # --------------------------------------------------------------------------- +def _theme() -> str: + """The workspace's palette, or the default. Never raises. + + Called on the launch path before anything else has loaded, so every failure + -- no app directory yet, a settings file half-written, an import that is not + there in a stripped install -- has to answer "light" rather than stop a + launch this module exists to make feel faster. + """ + from ui import tokens # noqa: PLC0415 + + return tokens.resolved_theme() + + def start(*, timeout_s: float = MAX_SECONDS) -> None: """Put the mark on screen. Returns immediately; never raises. @@ -95,6 +108,14 @@ def start(*, timeout_s: float = MAX_SECONDS) -> None: # that vanishes the instant it is started, which is a confusing thing to # meet while debugging one. "--watch-stdin", + # Passed down rather than read in the child, and the reason is the whole + # design of this module: the child exists to be on screen in about a + # tenth of a second, and reading the setting there would put + # `core.settings` and `core.appdata` on that path. The parent is already + # importing them. A splash that flashed cream in front of a dark + # workspace would be the one frame the whole theme is judged on. + "--theme", + _theme(), ] try: from core import paths, spawn # noqa: PLC0415 @@ -179,11 +200,13 @@ def _centre(window: Any, width: int, height: int) -> None: window.geometry(f"{width}x{height}+{x}+{y}") -def show(*, timeout_s: float = MAX_SECONDS, watch_stdin: bool = False) -> int: +def show( + *, timeout_s: float = MAX_SECONDS, watch_stdin: bool = False, theme: str = "light" +) -> int: """The window itself. Runs the Tk loop until something says to stop.""" import tkinter as tk # noqa: PLC0415 - the child's whole reason to exist - from ui.tokens import COLOUR # noqa: PLC0415 + from ui import tokens # noqa: PLC0415 # Read from the palette, not spelled here. `tests/test_ui_tokens.py` enforces # that rule across the package and it applies to a window drawn in Tk exactly @@ -191,8 +214,25 @@ def show(*, timeout_s: float = MAX_SECONDS, watch_stdin: bool = False) -> int: # brand yellow is the first thing to drift when the palette changes. The # module is pure Python with no dependencies of its own, so the cost of # importing it in this process is a parse. - ink, paper, brand = COLOUR["ink"], COLOUR["paper"], COLOUR["attention"] - muted = COLOUR["muted"] + # + # `palette()` rather than `COLOUR` since there are two: an unknown name + # resolves to the default there, so a `--theme` from a newer version is a + # cream splash rather than a KeyError in front of a launch. + colour = tokens.palette(theme) + # `fill`, not `ink`. The border and the ground behind the card are the + # emphasis ground -- in the dark palette `ink` is near-white, and a 2px + # near-white frame around a dark card is the same inversion bug the CSS + # `fill` token exists to prevent. + # + # Three names where there used to be one, for the same reason the stylesheet + # grew `fill` and `on-attention`: `ink` was doing three jobs here -- the + # frame around the card, the text on the card, and the nabla on the yellow + # mark -- and the three move in different directions when the ground + # inverts. The frame stays dark, the text goes light, and the nabla does not + # move at all because the yellow under it did not. + frame, paper, brand = colour["fill"], colour["paper"], colour["attention"] + ink, on_brand = colour["ink"], colour["on-attention"] + muted = colour["muted"] width, height = 340, 232 gone = threading.Event() @@ -205,7 +245,7 @@ def show(*, timeout_s: float = MAX_SECONDS, watch_stdin: bool = False) -> int: # It also keeps it out of the taskbar, where a second Grad entry that # disappears on its own would be its own small confusion. root.overrideredirect(True) - root.configure(bg=ink) + root.configure(bg=frame) root.attributes("-topmost", True) _centre(root, width, height) @@ -228,7 +268,7 @@ def show(*, timeout_s: float = MAX_SECONDS, watch_stdin: bool = False) -> int: if image is None: # No Pillow, no PNG, or a Tk without the image reader. Deliberately not # a second hand-drawn nabla -- see `desktop.splash_png`. - mark.configure(text="∇", fg=ink, font=("Segoe UI", 44, "bold"), width=3, height=1) + mark.configure(text="∇", fg=on_brand, font=("Segoe UI", 44, "bold"), width=3, height=1) mark.pack(pady=(26, 14)) tk.Label( @@ -325,8 +365,12 @@ def main(argv: list[str] | None = None) -> int: action="store_true", help="close when the pipe from the launching process does", ) + # Not validated against a list here: `tokens.palette` resolves an unknown + # name to the default, which is the behaviour that matters -- a splash is + # not the place to refuse to start over a theme name. + parser.add_argument("--theme", default="light", help="which palette to draw in") args = parser.parse_args(argv) - return show(timeout_s=args.timeout, watch_stdin=args.watch_stdin) + return show(timeout_s=args.timeout, watch_stdin=args.watch_stdin, theme=args.theme) if __name__ == "__main__": diff --git a/ui/state.py b/ui/state.py index a2c7cf7..354e22e 100644 --- a/ui/state.py +++ b/ui/state.py @@ -84,18 +84,29 @@ def load_layout(project: str | None) -> layout_mod.Layout: def opening_windows() -> tuple[str, ...]: """What opens when this workspace has never been arranged. - The mock's four -- unless the agent has no credentials, in which case those - four windows are four windows that cannot do anything, and the first thing - on screen should be the one that fixes it. + The mock's four -- unless this machine is mid-setup, in which case those four + windows are four windows that cannot do anything, and the first thing on + screen should be the one that fixes it. Only reached when there is no saved layout, so this costs a credential-store - read once per fresh workspace and nothing thereafter. And only the *token* is - checked (`models.setup_needed`): an unconfigured backend means no remote - training, which is a real limitation and not a reason to put a wizard in - front of someone who opened the app to read a ledger. + read once per fresh workspace and nothing thereafter -- and + `models.first_run_needed` rather than the full `first_run` is what keeps + that true, because the full one loads a `Config` and this path must not. + + **Widened from "no token" to "no token *or* no project".** The token was the + right question when it was the only blocking one, and it stopped being so + when projects became the thing every run, ceiling and report is filed + against: a workspace with a token and no project opens four windows, three + of which are empty because there is nothing to file against, and nothing on + screen says that is the reason. The panel at the top of the setup window + explains both. + + Still narrow in the way that matters: an unconfigured *backend* does not + open this window. Remote training is a real limitation and not a reason to + put a wizard in front of someone who opened the app to read a ledger. """ try: - if models.setup_needed(): + if models.first_run_needed(): return ("setup", *registry.defaults()) except Exception: # noqa: BLE001 - never the reason a workspace will not open log.debug("could not decide the opening arrangement", exc_info=True) @@ -196,6 +207,17 @@ def __init__(self, session: Any, project: str | None) -> None: #: destroyed. The window seeds the box from this on build and clears it #: on send. self.chat_draft: str = "" + #: The NiceGUI client this workspace was built for, set by `shell.build`. + #: + #: Held because `context.client` is resolved from the slot stack, which + #: is a contextvar and therefore empty in any spawned task -- so anything + #: that has to reach the browser from outside a render has no other way + #: to find it. See `paint_theme`, which is the bug that established this. + self.client: Any | None = None + #: The palette last pushed to that client, or None. Distinct from + #: `theme()`, which is what the *file* says: the two disagreeing is + #: precisely the failure this pair exists to make visible. + self.painted_theme: str | None = None #: Wakes that have arrived and not yet been turned into a prompt. #: #: A queue rather than a direct call, because the two ends are in @@ -883,6 +905,74 @@ def cycle_effort(self) -> str: ) return level + def theme(self) -> str: + """Which palette this workspace draws in. Never raises. + + Read on the build path of every page, so an unreadable overlay has to be + a cream window rather than a window that does not open. + """ + from ui import tokens # noqa: PLC0415 + + return tokens.resolved_theme() + + def set_theme(self, name: str) -> str: + """Record the choice. Returns what is now in effect. + + Refuses nothing silently: an unknown name raises out of `settings` and + the caller keeps the page on the palette that is still recorded, which + is the only arrangement where the screen and the file agree. + """ + from core import settings as settings_mod # noqa: PLC0415 + + settings_mod.set_theme(name) + self.say(f"theme: {name}") + return name + + def paint_theme(self, name: str) -> bool: + """Move the *live page* to a palette. Returns whether it got there. + + **Through the client this workspace was built with, never through + `ui.run_javascript`.** That helper is `context.client.run_javascript`, + and `context.client` is resolved from NiceGUI's slot stack -- which is a + contextvar, and therefore *empty in any task that was spawned*. The + first version of this switched the theme from a coroutine handed to + `spawn`, so the write succeeded, the status bar said "theme: dark", and + the repaint raised `The current slot cannot be determined` into a debug + log nobody was reading. The setting changed and the screen did not. + + `kit.run_js` documents the neighbouring version of this trap -- a live + slot that the handler has just deleted -- and the answer is the same + either way: hold the client, which `shell.build` has and which outlives + every element in it. + + Fire-and-forget on purpose. `run_javascript` returns an + `AwaitableResponse` that sends when it is *not* awaited and waits for a + reply when it is, and there is no reply worth waiting for here: setting + an attribute cannot fail in a way this side could act on, and awaiting + it would add a one-second timeout to a click. + + The return value is not decoration -- `tests/test_ui_theme.py` asserts + it, because "the setting was written" was exactly the assertion that + passed while the feature did nothing. + """ + import json # noqa: PLC0415 + + from ui import tokens # noqa: PLC0415 + + client = self.client + if client is None: + return False + try: + client.run_javascript( + "document.documentElement.setAttribute(" + f"{json.dumps(tokens.THEME_ATTRIBUTE)}, {json.dumps(str(name))})" + ) + except Exception: # noqa: BLE001 - a repaint is never worth a traceback + log.debug("could not repaint the theme", exc_info=True) + return False + self.painted_theme = name + return True + def say(self, message: str | None) -> None: """A one-line notice in the status bar: what a button just did.""" self.notice = message diff --git a/ui/tokens.py b/ui/tokens.py index 74b3e0d..ba472a0 100644 --- a/ui/tokens.py +++ b/ui/tokens.py @@ -56,11 +56,169 @@ "ink-rule": "#4A443A", "row-alt": "#FDFAF2", "attention-row": "#FFFBE8", + # -- role tokens ------------------------------------------------------- + # Everything above names a *colour*. The seven below name a *job*, and they + # exist because the light palette gets to conflate jobs that a dark one + # cannot. In cream-and-ink, `ink` is simultaneously the text on paper, the + # fill of the app bar, and the text on a yellow chip. Invert the ground and + # those three want to move in different directions: the text goes light, the + # app bar must stay dark or it becomes the brightest thing on screen, and + # the text on yellow must not move at all, because the yellow did not. + # + # So a fill and its foreground are named as a pair, and `test_ui_tokens.py` + # holds every pair to 4.5:1 *in both palettes*. That is the property that + # makes a second palette safe to add: a contrast rule you can only check by + # eye is a contrast rule that is already broken somewhere you have not + # looked. + # + # In this palette each one equals the colour it replaced, so the light + # stylesheet renders exactly as it did before they existed. + #: The emphasis ground: app bar, status bar, table head, focused title bar, + #: active button. "Inverted from the page", which is ink here and a raised + #: dark grey in the dark palette -- not the light one `ink` becomes. + "fill": "#14100C", + #: Text and hairline borders on `fill`. + "fill-ink": "#F7F3E8", + #: Text on the yellow. Fixed across both palettes, because `attention` is + #: fixed across both -- a brand mark that changed value with the theme would + #: stop being one. + "on-attention": "#14100C", + #: Text on the crimson. The `#fff` the handoff names ("`#A3122F` fill, white + #: text"), given a name so the dark sheet cannot be searched for stray + #: literals and find the one that is deliberate. + "on-broken": "#FFFFFF", + #: Text on `verified-tint`, which is a *tint* and not the fill: the fill + #: stays teal in both palettes and carries `verified-ink`, while the tint + #: inverts with the ground and needs a foreground that inverts with it. + "verified-tint-ink": "#04302C", + #: The hard shadow. Ink here; **not** `ink` in the dark palette, where an + #: 8px offset block of near-white is a glow rather than a shadow. + "shadow-ink": "#14100C", + #: What shows through behind the JupyterLab iframe before it paints. Was a + #: `#fff` literal, which is a white flash on every retile in a dark theme. + "iframe-ground": "#FFFFFF", + #: One per chart series, because a series fill is a fill like any other and + #: the segment labels sit inside it. `base` tracks `paper` in both palettes + #: (the series is `ink`, so its foreground is whatever ink is legible on); + #: the other two are pinned dark and light respectively by what their fills + #: do when the ground inverts. + "on-series-base": "#F7F3E8", + "on-series-alt": "#14100C", + "on-series-third": "#FFFFFF", +} + +#: The dark palette: the same keys, none added and none missing. +#: +#: Not a computed inversion. Inverting lightness mechanically gives a blue-grey +#: screen and a muddy yellow, and the two colours this design is *about* -- +#: `attention` and `verified` -- are the two an inversion damages most. These +#: are chosen against the same rules the light table was: warm greys rather than +#: neutral ones, one accent per state, and a foreground for every fill. +#: +#: `attention`, `verified` and `broken` keep their hues. They are the vocabulary +#: -- "yellow needs you, teal passed, red broke" -- and a theme that renegotiated +#: them would be a different design rather than the same one at night. +DARK: dict[str, str] = { + "ink": "#EFE8DA", + "paper": "#17140F", + "paper-raised": "#211D16", + "paper-sunk": "#100E0A", + "desk": "#0A0806", + "rule-soft": "rgba(239,232,218,0.16)", + "rule-mid": "rgba(239,232,218,0.32)", + "attention": "#FFD400", + # Lifted from #12A594: the light palette's teal is a *fill* under dark text, + # and here it also has to read as a stroke on a dark ground -- the band, the + # progress fill and the `ok` chip's border all draw with it. + "verified": "#1FC7B3", + "verified-ink": "#04302C", + "verified-tint": "#0E2E29", + "broken": "#D8324F", + "broken-tint": "#2C0F17", + "broken-ink": "#FF9AAB", + "broken-ink-2": "#FF8299", + "link": "#E8845C", + "muted": "#A79E8B", + "muted-2": "#8F8776", + "literal": "#45D6C2", + "hatch-a": "#1C1913", + "hatch-b": "#252118", + "ink-rule": "#4A443A", + "row-alt": "#1B1712", + "attention-row": "#241F0E", + # The emphasis ground goes *up* from paper here, not down to near-black. + # Dark UI convention and the handoff's own logic agree for once: elevation + # reads as light, and an app bar darker than the window it sits on would be + # a hole rather than a bar. + "fill": "#2E2822", + "fill-ink": "#EFE8DA", + "on-attention": "#14100C", + "on-broken": "#FFFFFF", + "verified-tint-ink": "#8AE6D6", + "shadow-ink": "#000000", + "iframe-ground": "#1E1B16", + "on-series-base": "#17140F", + "on-series-alt": "#14100C", + # `link` is light enough here that white on it is 2.4:1. The other two + # foregrounds are unchanged by the inversion; this one had to move. + "on-series-third": "#2A1008", +} + +#: The two palettes by name. `light` is the handoff's and is the default +#: everywhere -- a theme setting that has never been touched resolves to it. +PALETTES: dict[str, dict[str, str]] = {"light": COLOUR, "dark": DARK} +DEFAULT_THEME = "light" + +#: Every ground in the system, and the token that is legible on it. +#: +#: This is the table that makes a second palette safe. The rule "one accent per +#: state" is enforceable because `STATE_ACCENT` is total; the rule "text on a +#: fill can be read" needs the same treatment, and until there were two palettes +#: it did not have it -- the light one is legible by inspection and inspection +#: does not scale to a ground somebody inverts. +#: +#: `test_ui_tokens.py` holds every pair here to WCAG 4.5:1 **in both palettes**, +#: which is what caught `on-series-third`: white on the light palette's `link` +#: is 5.6:1 and white on the dark one's is 2.4:1, and nothing else would have +#: said so until a spend meter was unreadable at night. +FOREGROUND: dict[str, str] = { + "paper": "ink", + "paper-raised": "ink", + "paper-sunk": "ink", + "desk": "ink", + "row-alt": "ink", + "attention-row": "ink", + "fill": "fill-ink", + "attention": "on-attention", + "broken": "on-broken", + "verified": "verified-ink", + "verified-tint": "verified-tint-ink", + "broken-tint": "broken-ink", + "iframe-ground": "ink", +} + +#: The same claim for the chart ramp, kept separate because the keys on the left +#: are series names rather than palette entries. +SERIES_FOREGROUND: dict[str, str] = { + "base": "on-series-base", + "alt": "on-series-alt", + "third": "on-series-third", } # The four state accents, and the rule that governs them. `one accent per state, # never two in the same element` is a property of this mapping being total: a # state that is not here has no accent, rather than borrowing one. +#: +#: Named by palette *key* rather than by value, so the mapping means the same +#: thing in both palettes -- "ok is whatever `verified` is here". Resolved +#: against the light palette below for the callers that want a colour. +STATE_ACCENT_KEYS: dict[str, str] = { + "ok": "verified", + "attention": "attention", + "broken": "broken", + "neutral": "ink", +} + STATE_ACCENT: dict[str, str] = { "ok": COLOUR["verified"], "attention": COLOUR["attention"], @@ -82,12 +240,17 @@ # None of these three are chromatic accents, so a chart can be read as a chart # and a yellow fill can go back to meaning "this needs you". # `test_ui_tokens.py` holds the two mappings disjoint. -SERIES: dict[str, str] = { - "base": COLOUR["ink"], - "alt": COLOUR["muted"], - "third": COLOUR["link"], +# Keyed the same way `STATE_ACCENT_KEYS` is, and for the same reason: the claim +# "a series is never a state accent" is about the *mapping*, so it has to be +# checkable in whichever palette is on screen rather than in one of them. +SERIES_KEYS: dict[str, str] = { + "base": "ink", + "alt": "muted", + "third": "link", } +SERIES: dict[str, str] = {name: COLOUR[key] for name, key in SERIES_KEYS.items()} + # --------------------------------------------------------------------------- # type # --------------------------------------------------------------------------- @@ -104,13 +267,28 @@ # --------------------------------------------------------------------------- # structure # --------------------------------------------------------------------------- -BORDER_STRUCTURAL = f"2px solid {COLOUR['ink']}" -BORDER_HAIRLINE = f"1px solid {COLOUR['rule-soft']}" -BORDER_SECONDARY = f"1px dashed {COLOUR['rule-mid']}" -BORDER_PENDING = f"2px dashed {COLOUR['ink']}" - -SHADOW_SHELL = f"8px 8px 0 {COLOUR['ink']}" -SHADOW_CARD = f"6px 6px 0 {COLOUR['ink']}" +# Written as `var()` references rather than as interpolated hex, and that is the +# one change that made a second palette possible at all. +# +# These used to read `f"2px solid {COLOUR['ink']}"`, evaluated at import -- so +# `--grad-border` reached the browser as a literal `2px solid #14100C` and no +# amount of re-declaring `--grad-ink` further down could move it. Every +# structural rule in the sheet is built from these four, which meant every +# border and both shadows were pinned to the light palette by an f-string. +# +# Indirection through the custom property costs nothing (the browser resolves it +# at use) and buys the whole feature: one `:root` block per theme, and every +# derived value follows. +BORDER_STRUCTURAL = "2px solid var(--grad-ink)" +BORDER_HAIRLINE = "1px solid var(--grad-rule-soft)" +BORDER_SECONDARY = "1px dashed var(--grad-rule-mid)" +BORDER_PENDING = "2px dashed var(--grad-ink)" + +# `shadow-ink`, not `ink`. The offset block is near-black in both palettes: in +# the dark one `ink` is near-white, and an 8px white slab under every window is +# a glow, which is the opposite of what a shadow is for. +SHADOW_SHELL = "8px 8px 0 var(--grad-shadow-ink)" +SHADOW_CARD = "6px 6px 0 var(--grad-shadow-ink)" RADIUS = "0" STRIPE_WIDTH = "6px" # cell / ledger-entry state stripe @@ -127,10 +305,55 @@ BLINK = "gradblink 1.1s steps(1) infinite" -def css_variables() -> str: - """`:root` block. Everything else in the stylesheet reads from here.""" - lines = [f" --grad-{name}: {value};" for name, value in COLOUR.items()] - lines += [f" --grad-series-{name}: {value};" for name, value in SERIES.items()] +def palette(theme: str | None = None) -> dict[str, str]: + """One palette by name, falling back to light rather than raising. + + An unknown name is what a settings file written by a newer version looks + like from an older one, and the answer to that is the design's default -- + not a stylesheet that fails to generate and takes the window with it. + """ + return PALETTES.get(str(theme or DEFAULT_THEME).lower(), COLOUR) + + +def resolved_theme() -> str: + """The palette this workspace is set to, or the default. Never raises. + + One copy, because there were three: `ui/render.py`, `ui/splash.py` and + `ui/state.py` each had the same try-settings-except-default, and one of them + fell back to a literal `"light"` rather than to `DEFAULT_THEME` -- which is + the drift `_check_number`'s docstring warns about, arriving on schedule. + + Here rather than in `core/settings.py` because the fallback is a *design* + fact: an unreadable setting resolves to the palette the design ships, and + `palette()` already makes the same choice for an unknown name. + """ + try: + from core import settings # noqa: PLC0415 - keeps `core` off the import path + + return settings.theme() + except Exception: # noqa: BLE001 - a theme is never worth failing to draw + return DEFAULT_THEME + + +def colour_variables(theme: str | None = None) -> str: + """Just the colours, for one palette. The part that differs between themes.""" + active = palette(theme) + lines = [f" --grad-{name}: {value};" for name, value in active.items()] + lines += [ + f" --grad-series-{name}: {active[key]};" for name, key in SERIES_KEYS.items() + ] + return "\n".join(lines) + + +def css_variables(theme: str | None = None) -> str: + """`:root` block. Everything else in the stylesheet reads from here. + + The non-colour half is emitted once, with the light palette, because none of + it varies: a border is two pixels in both themes and its colour is a `var()` + reference resolved at use. That split is what lets `themed_variables` emit a + second block containing only the table that actually changes. + """ + lines = [colour_variables(theme)] lines += [ f" --grad-font-sans: {FONT_SANS};", f" --grad-font-mono: {FONT_MONO};", @@ -148,6 +371,33 @@ def css_variables() -> str: return ":root {\n" + "\n".join(lines) + "\n}" +#: The attribute the shell sets on ``, and the selector the dark block is +#: scoped by. One attribute rather than a class because `ui/static/tiling.js` +#: and the Lab iframe both need to read the current theme without knowing what +#: else is on the element. +THEME_ATTRIBUTE = "data-grad-theme" + + +def themed_variables() -> str: + """The override block for every palette that is not the default. + + Both palettes ship in one stylesheet, which is what makes switching instant + and reload-free: `ui/app.py` adds the sheet once, at import, with + `shared=True`, so there is no second injection to make -- the switch is one + attribute on the document element and the cascade does the rest. That also + satisfies the design's motion rule for free, since an attribute flip is an + instant state swap rather than a transition. + """ + blocks = [] + for name in PALETTES: + if name == DEFAULT_THEME: + continue + blocks.append( + f':root[{THEME_ATTRIBUTE}="{name}"] {{\n{colour_variables(name)}\n}}' + ) + return "\n\n".join(blocks) + + def _quasar_reset() -> str: """Undo the four Quasar defaults that contradict the design. @@ -172,6 +422,24 @@ class each. Radius, ripple, elevation and the uppercase button transform are .grad-app .q-field__native, .grad-app .q-field__input { font-family: var(--grad-font-sans); color: var(--grad-ink); } .grad-app .q-tab { text-transform: none; letter-spacing: 0; } + +/* The one rule in here that is deliberately *not* scoped to the app root. + Quasar renders a select's popup into a portal at `` level, so it is + not inside `.grad-app` and nothing above reaches it -- which is why the + squared corners declared for `.grad-app .q-menu` have never applied to the + three selects in this app. That was invisible while Quasar's white default + sat under a cream design; against a dark one it is a white card in the + middle of the screen. `ui.run(dark=False)` stays as it is: Quasar's dark + mode would fight every token here, and this is all it was needed for. */ +.q-menu { + border-radius: 0 !important; box-shadow: none !important; + background: var(--grad-paper-raised); color: var(--grad-ink); + border: var(--grad-border); +} +.q-menu .q-item { font-family: var(--grad-font-mono); font-size: 12px; } +.q-menu .q-item.q-manual-focusable--focused, +.q-menu .q-item:hover { background: var(--grad-paper-sunk); } +.q-menu .q-item.q-item--active { background: var(--grad-fill); color: var(--grad-fill-ink); } """ @@ -204,8 +472,8 @@ def _base() -> str: line-height: 1.55; } .grad-app a { color: var(--grad-link); text-decoration: underline; text-underline-offset: 2px; } -.grad-app a:hover { color: var(--grad-ink); background: var(--grad-attention); } -.grad-app ::selection { background: var(--grad-attention); color: var(--grad-ink); } +.grad-app a:hover { color: var(--grad-on-attention); background: var(--grad-attention); } +.grad-app ::selection { background: var(--grad-attention); color: var(--grad-on-attention); } .grad-app :focus-visible { outline: 2px solid var(--grad-ink); outline-offset: 2px; } .grad-mono { font-family: var(--grad-font-mono); } @@ -239,8 +507,8 @@ def _shell() -> str: height: calc(100vh - 28px); margin: 14px; overflow: hidden; } .grad-appbar { - display: flex; align-items: stretch; background: var(--grad-ink); - color: var(--grad-paper); border-bottom: var(--grad-border); flex: 0 0 auto; + display: flex; align-items: stretch; background: var(--grad-fill); + color: var(--grad-fill-ink); border-bottom: var(--grad-border); flex: 0 0 auto; } .grad-appbar-cell { display: flex; align-items: center; gap: 10px; padding: 10px 14px; @@ -248,12 +516,12 @@ def _shell() -> str: font-family: var(--grad-font-mono); font-size: 12px; } .grad-appbar-cell.right { border-right: 0; border-left: 1px solid var(--grad-ink-rule); } -.grad-appbar-cell.brand { border-right: 2px solid var(--grad-paper); } +.grad-appbar-cell.brand { border-right: 2px solid var(--grad-fill-ink); } .grad-mark { width: 22px; height: 22px; background: var(--grad-attention); - border: 2px solid var(--grad-paper); display: flex; align-items: center; + border: 2px solid var(--grad-fill-ink); display: flex; align-items: center; justify-content: center; font-family: var(--grad-font-mono); font-size: 13px; - font-weight: 700; color: var(--grad-ink); + font-weight: 700; color: var(--grad-on-attention); } .grad-wordmark { font-family: var(--grad-font-mono); font-size: 15px; font-weight: 700; letter-spacing: 0.22em; } @@ -267,10 +535,10 @@ def _shell() -> str: the stylesheet they sit. */ .grad-appbar .grad-btn.grad-appbar-btn { font-family: var(--grad-font-mono); font-size: 11px; font-weight: 700; - padding: 3px 8px; border: 1.5px solid var(--grad-paper); cursor: pointer; + padding: 3px 8px; border: 1.5px solid var(--grad-fill-ink); cursor: pointer; background: transparent; color: inherit; } -.grad-appbar .grad-btn.grad-appbar-btn:hover { background: var(--grad-paper); color: var(--grad-ink); } +.grad-appbar .grad-btn.grad-appbar-btn:hover { background: var(--grad-fill-ink); color: var(--grad-fill); } /* The `⋯` button carries a menu, so it gets the caret's job: a little wider than a word button, and legible as a target rather than as punctuation. @@ -300,9 +568,9 @@ def _shell() -> str: } /* Open is the state worth reading off the list at a glance, so it is the one that gets ink -- the mark alone is too quiet at eleven rows. */ -.grad-menu-row.open { background: var(--grad-ink); color: var(--grad-paper); } +.grad-menu-row.open { background: var(--grad-fill); color: var(--grad-fill-ink); } .grad-menu-row.open .mark, .grad-menu-row.open .hint { opacity: 0.7; } -.grad-menu-row.open:hover { background: var(--grad-broken); color: #fff; } +.grad-menu-row.open:hover { background: var(--grad-broken); color: var(--grad-on-broken); } /* A session is called whatever was first asked in it, so its name gets the row and the count beside it gets what it needs -- the reverse of a window list, where the names are one word and the hints are the sentence. */ @@ -342,12 +610,12 @@ def _shell() -> str: opacity: 0.55; font-size: 11px; width: 100%; overflow: hidden; white-space: nowrap; text-overflow: ellipsis; } -.grad-step.open { background: var(--grad-ink); color: var(--grad-paper); } +.grad-step.open { background: var(--grad-fill); color: var(--grad-fill-ink); } .grad-step.open .mark, .grad-step.open .hint { opacity: 0.7; } .grad-statusbar { display: flex; align-items: center; gap: 14px; padding: 0 12px; - background: var(--grad-ink); color: var(--grad-paper); + background: var(--grad-fill); color: var(--grad-fill-ink); font-family: var(--grad-font-mono); font-size: 11px; height: 30px; flex: 0 0 auto; border-top: var(--grad-border); } @@ -359,7 +627,7 @@ def _shell() -> str: replaced, 0.89 relative luminance against 0.69); what makes it recede is dropping the fill, and with it the hue's claim on the eye. */ .grad-statusbar .count { - border: 1.5px solid var(--grad-paper); padding: 1px 6px; font-weight: 700; + border: 1.5px solid var(--grad-fill-ink); padding: 1px 6px; font-weight: 700; } """ @@ -389,19 +657,19 @@ def _tiling() -> str: flex: var(--grad-fraction, 1) 1 0; overflow: hidden; } .grad-handle { - flex: 0 0 var(--grad-handle); background: var(--grad-ink); cursor: col-resize; + flex: 0 0 var(--grad-handle); background: var(--grad-fill); cursor: col-resize; display: flex; flex-direction: column; align-items: center; justify-content: center; gap: 3px; user-select: none; } .grad-handle.row { cursor: row-resize; flex-basis: var(--grad-handle); flex-direction: row; } -.grad-handle span { width: 2px; height: 2px; background: var(--grad-paper); display: block; } +.grad-handle span { width: 2px; height: 2px; background: var(--grad-fill-ink); display: block; } .grad-handle.dragging { background: var(--grad-broken); } .grad-window { display: flex; flex-direction: column; min-height: 0; flex: 1 1 auto; background: var(--grad-paper); overflow: hidden; } -.grad-window.focused .grad-titlebar { background: var(--grad-ink); color: var(--grad-paper); } -.grad-window.focused .grad-titlebar .grad-winctl { color: var(--grad-paper); } +.grad-window.focused .grad-titlebar { background: var(--grad-fill); color: var(--grad-fill-ink); } +.grad-window.focused .grad-titlebar .grad-winctl { color: var(--grad-fill-ink); } .grad-titlebar { display: flex; align-items: center; gap: 10px; padding: 0 10px; height: var(--grad-titlebar); flex: 0 0 var(--grad-titlebar); @@ -425,7 +693,7 @@ def _tiling() -> str: /* The Lab iframe lives outside the pane tree (see ui/static/tiling.js): a reparented iframe is destroyed and recreated by the browser, which would reload JupyterLab -- kernel and all -- on every retile. */ -.grad-iframe-host { position: absolute; border: 0; background: #fff; z-index: 5; } +.grad-iframe-host { position: absolute; border: 0; background: var(--grad-iframe-ground); z-index: 5; } .grad-iframe-anchor { flex: 1 1 auto; min-height: 0; } /* Dragging a title bar to retile. Three pieces of feedback, and none of them @@ -442,7 +710,7 @@ def _tiling() -> str: } .grad-drag-ghost { position: fixed; z-index: 41; pointer-events: none; - background: var(--grad-ink); color: var(--grad-paper); + background: var(--grad-fill); color: var(--grad-fill-ink); font-family: var(--grad-font-mono); font-size: 11px; font-weight: 700; text-transform: uppercase; letter-spacing: 0.14em; padding: 3px 8px; } @@ -453,9 +721,9 @@ def _tiling() -> str: outline: 2px dashed var(--grad-ink); outline-offset: -2px; opacity: 0.6; } .grad-window .grad-titlebar.grad-swap-target { - background: var(--grad-attention); color: var(--grad-ink); + background: var(--grad-attention); color: var(--grad-on-attention); } -.grad-window .grad-titlebar.grad-swap-target .grad-winctl { color: var(--grad-ink); } +.grad-window .grad-titlebar.grad-swap-target .grad-winctl { color: var(--grad-on-attention); } body.grad-dragging { cursor: grabbing; } /* Text selection and iframe hit-testing both eat a drag that crosses a pane. */ body.grad-dragging * { user-select: none !important; } @@ -472,11 +740,11 @@ def _controls() -> str: cursor: pointer; line-height: 1; } .grad-btn:hover { background: var(--grad-paper-sunk); } -.grad-btn.primary { background: var(--grad-attention); } +.grad-btn.primary { background: var(--grad-attention); color: var(--grad-on-attention); } .grad-btn.primary:hover { background: var(--grad-attention); filter: brightness(0.94); } .grad-btn.ok { background: var(--grad-verified); color: var(--grad-verified-ink); } -.grad-btn.danger { background: var(--grad-broken); color: #fff; } -.grad-btn.active { background: var(--grad-ink); color: var(--grad-paper); } +.grad-btn.danger { background: var(--grad-broken); color: var(--grad-on-broken); } +.grad-btn.active { background: var(--grad-fill); color: var(--grad-fill-ink); } .grad-btn.dashed { border: var(--grad-pending); background: transparent; opacity: 0.75; } .grad-btn[disabled], .grad-btn.disabled { background: var(--grad-paper-sunk); opacity: 0.5; pointer-events: none; @@ -487,7 +755,7 @@ def _controls() -> str: `.active`'s paper text in place -- paper on paper, illegible. Keep the inverted ink fill and most of its contrast. */ .grad-btn.active[disabled], .grad-btn.active.disabled { - background: var(--grad-ink); color: var(--grad-paper); opacity: 0.85; + background: var(--grad-fill); color: var(--grad-fill-ink); opacity: 0.85; } .grad-btn.ghost { border: 0; background: transparent; padding: 6px 8px; } .grad-btn.ghost:hover { background: var(--grad-paper-sunk); } @@ -501,9 +769,9 @@ def _controls() -> str: white-space: nowrap; text-transform: uppercase; letter-spacing: 0.06em; } .grad-chip.ok { background: var(--grad-verified); color: var(--grad-verified-ink); border-color: var(--grad-ink); } -.grad-chip.attention { background: var(--grad-attention); color: var(--grad-ink); } -.grad-chip.broken { background: var(--grad-broken); color: #fff; } -.grad-chip.solid { background: var(--grad-ink); color: var(--grad-paper); } +.grad-chip.attention { background: var(--grad-attention); color: var(--grad-on-attention); } +.grad-chip.broken { background: var(--grad-broken); color: var(--grad-on-broken); } +.grad-chip.solid { background: var(--grad-fill); color: var(--grad-fill-ink); } .grad-chip.outline { background: transparent; } .grad-chip.dashed { border: var(--grad-pending); background: transparent; } .grad-chip .dot { width: 7px; height: 7px; background: currentColor; } @@ -545,17 +813,17 @@ def _data() -> str: .grad-bar .seg { display: flex; align-items: center; justify-content: center; font-family: var(--grad-font-mono); font-size: 10px; font-weight: 700; overflow: hidden; white-space: nowrap; } -.grad-bar .seg.base { background: var(--grad-series-base); color: var(--grad-paper); } -.grad-bar .seg.chat { background: var(--grad-series-base); color: var(--grad-paper); } -.grad-bar .seg.tool { background: var(--grad-series-alt); color: var(--grad-ink); } -.grad-bar .seg.opus { background: var(--grad-series-third); color: #fff; } -.grad-bar .seg.broken { background: var(--grad-broken); color: #fff; } +.grad-bar .seg.base { background: var(--grad-series-base); color: var(--grad-on-series-base); } +.grad-bar .seg.chat { background: var(--grad-series-base); color: var(--grad-on-series-base); } +.grad-bar .seg.tool { background: var(--grad-series-alt); color: var(--grad-on-series-alt); } +.grad-bar .seg.opus { background: var(--grad-series-third); color: var(--grad-on-series-third); } +.grad-bar .seg.broken { background: var(--grad-broken); color: var(--grad-on-broken); } .grad-bar.thin { height: 12px; border-width: 1.5px; } .grad-table { width: 100%; border-collapse: collapse; font-family: var(--grad-font-mono); font-size: 12px; } .grad-table thead th { - background: var(--grad-ink); color: var(--grad-paper); text-align: left; + background: var(--grad-fill); color: var(--grad-fill-ink); text-align: left; padding: 7px 9px; font-size: 10px; letter-spacing: 0.12em; text-transform: uppercase; font-weight: 700; white-space: nowrap; } @@ -586,8 +854,8 @@ def _data() -> str: font-family: var(--grad-font-mono); font-size: 11px; font-weight: 700; flex: 0 0 18px; } .grad-status-square.ok { background: var(--grad-verified); color: var(--grad-verified-ink); } -.grad-status-square.attention { background: var(--grad-attention); color: var(--grad-ink); } -.grad-status-square.broken { background: var(--grad-broken); color: #fff; } +.grad-status-square.attention { background: var(--grad-attention); color: var(--grad-on-attention); } +.grad-status-square.broken { background: var(--grad-broken); color: var(--grad-on-broken); } /* Ledger band strip: predicted band, observed tick, falsifier bounds. */ .grad-band { position: relative; height: 30px; border: 1.5px solid var(--grad-ink); @@ -668,7 +936,7 @@ def _data() -> str: .grad-diff { font-family: var(--grad-font-mono); font-size: 12px; line-height: 1.65; background: var(--grad-paper-raised); border: 1.5px solid var(--grad-ink); } .grad-diff div { padding: 1px 9px; white-space: pre-wrap; } -.grad-diff .add { background: var(--grad-verified-tint); color: var(--grad-verified-ink); } +.grad-diff .add { background: var(--grad-verified-tint); color: var(--grad-verified-tint-ink); } .grad-diff .del { background: var(--grad-broken-tint); color: var(--grad-broken-ink-2); } .grad-diff .meta { background: var(--grad-paper-sunk); opacity: 0.7; } @@ -706,8 +974,8 @@ def _data() -> str: background: repeating-linear-gradient(135deg, var(--grad-hatch-a) 0 8px, var(--grad-hatch-b) 8px 16px); display: flex; align-items: center; justify-content: center; } -.grad-figure .tag { position: absolute; left: 0; bottom: 0; background: var(--grad-ink); - color: var(--grad-paper); font-family: var(--grad-font-mono); +.grad-figure .tag { position: absolute; left: 0; bottom: 0; background: var(--grad-fill); + color: var(--grad-fill-ink); font-family: var(--grad-font-mono); font-size: 10px; padding: 3px 7px; } """ @@ -741,7 +1009,7 @@ def _chat() -> str: .grad-msg.user .bubble { max-width: 88%; border: var(--grad-border); background: var(--grad-paper-raised); padding: 11px; font-size: 14px; } .grad-msg.grad .bubble { padding-left: 23px; font-size: 14px; } -.grad-avatar { width: 16px; height: 16px; background: var(--grad-attention); +.grad-avatar { width: 16px; height: 16px; background: var(--grad-attention); color: var(--grad-on-attention); border: 1.5px solid var(--grad-ink); display: inline-flex; align-items: center; justify-content: center; font-family: var(--grad-font-mono); font-size: 10px; font-weight: 700; } @@ -768,9 +1036,9 @@ def _chat() -> str: .grad-card > .head { display: flex; align-items: center; gap: 9px; padding: 6px 10px; font-family: var(--grad-font-mono); font-size: 11px; font-weight: 700; text-transform: uppercase; letter-spacing: 0.1em; } -.grad-card > .head.attention { background: var(--grad-attention); color: var(--grad-ink); } -.grad-card > .head.ink { background: var(--grad-ink); color: var(--grad-paper); } -.grad-card > .head.broken { background: var(--grad-broken); color: #fff; } +.grad-card > .head.attention { background: var(--grad-attention); color: var(--grad-on-attention); } +.grad-card > .head.ink { background: var(--grad-fill); color: var(--grad-fill-ink); } +.grad-card > .head.broken { background: var(--grad-broken); color: var(--grad-on-broken); } .grad-card > .body { padding: 11px; background: var(--grad-paper-raised); } .grad-card.gate { border-color: var(--grad-broken); } @@ -836,8 +1104,8 @@ def _chat() -> str: * question the pane was opened to ask. Paper rather than ink, because the chip * sits on the card's ink head. */ .grad-card.tool > .head .grad-chip.ok { - background: transparent; color: var(--grad-paper); - border: 1.5px solid var(--grad-paper); opacity: 0.7; + background: transparent; color: var(--grad-fill-ink); + border: 1.5px solid var(--grad-fill-ink); opacity: 0.7; } /* The agent statusline: always on screen, and the switch for the reasoning @@ -873,7 +1141,7 @@ def _chat() -> str: .grad-statusline .context.warn { opacity: 1; border: 1.5px solid var(--grad-ink); } .grad-statusline .context.attention { - opacity: 1; background: var(--grad-attention); color: var(--grad-ink); } + opacity: 1; background: var(--grad-attention); color: var(--grad-on-attention); } /* The two parts of the bar that are controls rather than reports, so they are the parts drawn as such. Effort sits to the left of the reasoning switch: both are about the agent's thinking, and this one changes what it does while @@ -888,7 +1156,7 @@ def _chat() -> str: .grad-statusline .effort.set { border-style: solid; opacity: 1; } .grad-statusline .effort:hover { background: var(--grad-paper-raised); } .grad-chat.reasoning-on .grad-statusline .reasoning { - background: var(--grad-ink); color: var(--grad-paper); } + background: var(--grad-fill); color: var(--grad-fill-ink); } /* The compaction marker: a rule across the transcript, not a message. Nothing was said at this point -- what happened is that everything above it stopped @@ -1010,6 +1278,7 @@ def stylesheet() -> str: return "\n".join( [ css_variables(), + themed_variables(), _base(), _shell(), _tiling(), diff --git a/ui/windows/setup.py b/ui/windows/setup.py index 2b2c3db..52a4bad 100644 --- a/ui/windows/setup.py +++ b/ui/windows/setup.py @@ -57,6 +57,8 @@ def render(workspace: Any) -> None: kit.empty("Setup could not read this machine's configuration.") return + _first_run(workspace, model) + active = workspace.selection.get("setup.step") or steps[0]["id"] if active not in {s["id"] for s in steps}: active = steps[0]["id"] @@ -81,6 +83,79 @@ def render(workspace: Any) -> None: _installation(workspace) +# --------------------------------------------------------------------------- +# 0. the first run +# --------------------------------------------------------------------------- +def _first_run(workspace: Any, model: dict[str, Any]) -> None: + """Three things to do, in order, on a machine that has never been set up. + + **Not a tour, and deliberately not one.** A guided overlay would have to + dim the workspace and float a callout with a drop shadow over it -- three + separate contradictions of a design whose motion rule is "instant state + swaps" and whose shadow rule is "no blur anywhere", enforced by + `tests/test_ui_tokens.py`. It would also have to know where a window *is*, + in a workspace whose whole premise is that you drag windows wherever you + want them, and it would have to be dismissable -- at which point it teaches + nobody who dismissed it. + + A list of what is not done yet has none of those problems. It is drawn in + the ordinary components, it survives being read twice, and it goes away by + being satisfied rather than by being closed. See `ui/models.py:first_run` + for why the state behind it is derived rather than stored. + + Rows are buttons: the point of putting this here is that the next action is + one click away rather than one paragraph away. + """ + run = model.get("first_run") or {} + if not run.get("active"): + return + + with kit.el("div", "grad-note").style("margin: 0 0 12px"): + with kit.row("", gap=9): + kit.text("FIRST RUN", "grad-label", tag="span") + kit.spacer() + kit.text(f"{run.get('done', 0)} of {run.get('total', 0)} done", "grad-caption", tag="span") + kit.text( + "Grad is not configured yet. These are the things it needs, in order.", + "grad-caption", + ).style("margin: 6px 0 9px") + + for step in run.get("steps") or []: + _first_run_row(workspace, step) + + +def _first_run_row(workspace: Any, step: dict[str, Any]) -> None: + done = bool(step.get("done")) + # A filled square for done and an empty one for not, the same mark the + # windows menu uses -- one vocabulary for "this is in effect". + row = kit.menu_row( + "■" if done else "□", + step.get("title", ""), + step.get("why", ""), + open=done, + title=step.get("why", ""), + # A finished step has nothing to click, and a `