diff --git a/.gitignore b/.gitignore index e2d7c32..31038d1 100644 --- a/.gitignore +++ b/.gitignore @@ -52,11 +52,22 @@ reports/**/claims.tex # client's transcript is keyed by, so it is a credential in everything but name; # the session files are the conversations themselves, and the layouts and kernel # pid are one machine's, not the repository's. +# +# These now live under `core/appdata.py`'s directory instead -- outside the +# workspace entirely, which is a better answer than ignoring them. The entries +# stay because a checkout that predates the move still has the files until +# `migrate_legacy` runs, and an un-ignored credential is not something to leave +# resting on a startup path having been taken. data/ui_storage_secret data/ui_session-*.jsonl data/layouts/ data/kernel/ +# JupyterLab's own runtime state, written into the config directory it is +# pointed at rather than into data/. Generated on first start, per machine. +config/jupyter/lab/ +config/jupyter/migrated + # Credentials never live in the workspace (HANDOFF §9). This is belt and braces. .env *.pem diff --git a/README.md b/README.md index 7c9d2c0..676eb7a 100644 --- a/README.md +++ b/README.md @@ -62,9 +62,14 @@ mirror of Anthropic's limit.** The meter says so on screen. pip install -e ".[agent,notebook,retrieval,remote,ui,math,dev]" ``` -Optional extras, each pinned and each independently skippable: `lab` (the -embedded JupyterLab, pinned exactly because the 3→4 break is what killed the -Tabnine extension), `wiki` (RepoWiki), `evolve` (ShinkaEvolve). +`ui` brings the embedded JupyterLab with it (the `lab` extra, pinned exactly +because the 3→4 break is what killed the Tabnine extension). That is not a +convenience: the notebook window's interior *is* Lab, so an app installed +without it ships a window whose only content is a button that fails. + +Optional extras, each pinned and each independently skippable: +`lab-extensions` (LSP, git — `python-lsp-server[all]` is heavier than everything +else here put together), `wiki` (RepoWiki), `evolve` (ShinkaEvolve). The core — ledger, preflight, gates, submitters — needs only the standard library plus a file lock. Everything heavier is optional and imported at the @@ -100,26 +105,47 @@ the only thing that forced a shell open beside the app on a fresh machine. The value goes down a pipe rather than in an argument — an argv is visible to anything that can list processes. -### Retrieval without an institutional email - -Tier 1 defaults to **Ai2's Asta** (`asta-tools.allen.ai`) rather than to the -Semantic Scholar REST API. It is the same corpus and it exposes the same -`snippet_search` the funnel is built around, but Semantic Scholar -[no longer issues API keys to free-domain email addresses][s2-keys], which -leaves a personal account on a shared anonymous pool that is rate limited often -enough that "no results" and "no key" are hard to tell apart. - -Asta is reached over streamable HTTP without adopting MCP as an architecture, -which is what §5 already said about that endpoint. Its key is optional and -raises limits rather than unlocking anything. Set `[retrieval] tier1` to `s2` -or `both` to change it, or pass `--tier1` per search. - -**The endpoint, the transport and the tool names are from Ai2's documentation; -the shape of each tool's result is not verified against the live service.** -`core/http.py:_rows` reads both plausible shapes and raises on anything else, -because a search that quietly returns nothing reads as "the literature has -nothing on this". - +### Retrieval without an institutional email, and without waiting + +Tier 1 defaults to **Papers with Code** (`paperswithcode.co/api/v1`) — the +catalogue as revived by Hugging Face, not the `.com` site Meta shut down. It is +anonymous and read-only: no account, no key, nothing to store. The endpoints are +those of [`huggingface/pwc-cli`][pwc-cli], which is the reference client for +this API. + +The choice is about latency, and the numbers are measured rather than assumed: + +| tier 1 | one search | key | text | +| --- | --- | --- | --- | +| `pwc` *(default)* | **1–2 s** | none | title only; abstracts fetched separately | +| `asta` | ~121 s, and ~283 s to report a backend failure | optional | full-text snippets | +| `s2` | fast when it answers | institutional addresses only | full-text snippets | + +Stage 0 turns one question into ~6 queries and each goes to two endpoints, so +Asta's per-call latency is twenty minutes of discovery before anything is +ranked — and every caller gives up first: the agent's own Bash tool backgrounds +a command at 120 s, and a shell `timeout` kills it. Semantic Scholar +[no longer issues API keys to free-domain email addresses][s2-keys], leaving a +personal account on a shared anonymous pool that is rate limited often enough +that "no results" and "no key" are hard to tell apart. + +**What the default costs.** Asta is the only one of the three with genuine +full-text snippets, which is what §5 designed stage-3 triage around. Under `pwc`, +triage reads the *abstract* instead — fetched from arXiv in one batched request +for the whole candidate pool (`core/http.py:arxiv_abstracts`), because nearly +every row is an arXiv paper and `id_list` takes a hundred ids at once. A +candidate that ends up with no abstract is reranked on its title, and the trace +says how many did. `pwc`'s expansion is also a *dense neighbour* rather than a +citation edge, and `neighbours` reports it as one rather than claiming the +citation graph §5 asks for. + +Set `[retrieval] tier1` to `asta`, `s2`, `both` (the two Semantic Scholar doors) +or `all`, or pass `--tier1` per search. Two wall clocks bound a run either way: +`request_deadline_s` caps one request, and `stage1_budget_s` caps the whole of +stage 1 — when it is spent the funnel keeps what it retrieved and records how +many queries it actually searched. + +[pwc-cli]: https://github.com/huggingface/pwc-cli [s2-keys]: https://www.semanticscholar.org/product/api ## Run @@ -258,12 +284,39 @@ autouse fixture in `tests/conftest.py` replaces `core.http._httpx`, because a suite that reaches the network does not fail, it *hangs*. Implemented but not exercised against a live service: the HF Jobs backend, the -SSH backend, Asta, Semantic Scholar, the OpenRouter reranker, Voyage embeddings, -the two Haiku funnel stages, Context7, ShinkaEvolve, and RepoWiki. They are -written against the documented interfaces and fail with actionable errors rather -than tracebacks, but a real credential and a real run are what will find the +SSH backend, Semantic Scholar, the OpenRouter reranker, Voyage embeddings, the +two Haiku funnel stages, Context7, ShinkaEvolve, and RepoWiki. They are written +against the documented interfaces and fail with actionable errors rather than +tracebacks, but a real credential and a real run are what will find the mismatches. +Tier-1 discovery is no longer in that list. A live run of the default funnel +returns 118 candidates from two rankings in 3.1 seconds, fills 99 abstracts in +one arXiv request, and hands 15 survivors to triage. + +Papers with Code and Asta are both exercised against the live services now, and +what that found is worth recording, because none of it could have been reasoned +out from the documentation: + +* **Asta's `search_papers_by_relevance` takes `keyword`** where `snippet_search` + takes `query`, so tier 1 lost that endpoint on every search — the server + answers in about a second with a validation error, which the slower endpoint's + latency hid. +* **Asta answers `tools/call` with an event stream it holds open**, pinging every + 15 seconds. `httpx`'s timeout is per socket read, so every ping reset it: a + buffered read waited for a close the server never promised, and discovery did + not fail, it never returned. The client now streams the response, stops at the + reply to its own request, and enforces a total deadline. +* **Asta wraps its hits under `result`** (singular), a key `_rows` did not know — + so even with the argument fixed, every hit was discarded as an unrecognised + envelope. And its `limit` must be ≤ 100, which `--no-expand` exceeded by + dividing the candidate ceiling across one query instead of six. +* **Papers with Code returns no abstract with a search row**, which is why + `arxiv_abstracts` exists. + +The *shapes* the remaining tools answer in are still read defensively, and an +unrecognised one still raises rather than returning an empty list. + **Two of [HANDOFF-2 §23](HANDOFF-2.md)'s open questions are now closed:** - **Context7's REST endpoints** (§23 item 2) are verified against the live API: diff --git a/agent.py b/agent.py index f0c6f8d..49e1038 100644 --- a/agent.py +++ b/agent.py @@ -22,14 +22,17 @@ import argparse import asyncio +import dataclasses import json +import logging import os import sys +import time from pathlib import Path from typing import Any import hooks -from core import config as config_mod, credentials, paths, quota_log +from core import appdata, config as config_mod, credentials, paths, quota_log from core.errors import EXIT_PROJECT_BUDGET BUILTIN_TOOLS = ["Read", "Write", "Edit", "Bash", "Glob", "Grep"] @@ -73,22 +76,59 @@ def build_options(cfg: Any, *, permission_mode: str | None = None, resume: str | "PreToolUse": [sdk.HookMatcher(matcher="Bash", hooks=[hooks.pre_tool_use])], "Stop": [sdk.HookMatcher(hooks=[hooks.stop])], } - return sdk.ClaudeAgentOptions( - resume=resume, - model=cfg.model_for("research"), - system_prompt=system_prompt(), - allowed_tools=BUILTIN_TOOLS, - disallowed_tools=DENIED_TOOLS, - permission_mode=mode, - cwd=str(paths.root()), - hooks=hook_matchers, + options: dict[str, Any] = { + "resume": resume, + "model": cfg.model_for("research"), + "system_prompt": system_prompt(), + "allowed_tools": BUILTIN_TOOLS, + "disallowed_tools": DENIED_TOOLS, + "permission_mode": mode, + "cwd": str(paths.root()), + "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 # a whole `AssistantMessage` is finished. With it the same turn also # emits `StreamEvent`s carrying token deltas. `TextStream` is what turns # the two into one transcript -- see the warning in its docstring. - include_partial_messages=True, - ) + "include_partial_messages": True, + } + options.update(thinking_option(cfg, sdk)) + return sdk.ClaudeAgentOptions(**options) + + +def thinking_option(cfg: Any, sdk: Any) -> dict[str, Any]: + """Ask for the reasoning as text, when the installed SDK can be asked. + + Capturing thinking blocks is not enough to *have* any: Opus 4.7+ defaults + `display` to "omitted" and sends them with a signature and no text. So the + chat window's reasoning switch had nothing to reveal no matter how correctly + the stream was read -- the bug was one flag away from the feature, and it + looked exactly like a toggle that did nothing. + + Feature-detected rather than assumed, for the same reason `agent.py probe` + exists: this option is newer than the permission mode and the SDK's shape has + changed between releases. An SDK without it gets the options it understands + and a session with no reasoning, which is what it would have had anyway. + """ + display = str(cfg.get("agent", "reasoning", "summarized")).lower() + if display not in ("summarized", "omitted"): + display = "summarized" + try: + fields = {f.name for f in dataclasses.fields(sdk.ClaudeAgentOptions)} + except TypeError: + # Not a dataclass. `dataclasses.fields` raises rather than returning + # empty, and this whole function exists to tolerate an SDK whose options + # object is not the shape we expect -- so the release that changes it to + # an ordinary class or a TypedDict must degrade to "no reasoning + # settings", exactly as an SDK without the field already does, rather + # than take the app down before the first turn. + return {} + if "thinking" not in fields: + return {} + # `adaptive` rather than a fixed budget: the model decides how much thinking + # a turn is worth, which is the right call for a session that ranges from + # "what is in the ledger" to a campaign design. + return {"thinking": {"type": "adaptive", "display": display}} def preflight_environment() -> dict[str, Any]: @@ -222,6 +262,7 @@ async def drive_turn( stream: Any, *, on_chunk: Any = None, + on_session_id: Any = None, session: str | None = None, ) -> dict[str, Any]: """One turn, for every surface that runs one. @@ -236,6 +277,13 @@ async def drive_turn( `on_chunk` is called with each newly-visible piece of text; the UI passes nothing because its renderer reads `stream.blocks` on a timer instead. + + `on_session_id` is called the moment the SDK names this conversation, and it + exists because the return value is not reached on every path this function + can take. A turn that is *interrupted* raises, so a caller reading the id + off the return value learned it only for turns that finished -- and an + interrupted turn is precisely the one after which the client is rebuilt, so + that was the case where losing the id cost the whole conversation. """ refusal = check_turn_budget() if refusal: @@ -258,6 +306,8 @@ async def drive_turn( # A resumed conversation can be given a new id, so the latest wins. candidate = getattr(message, "session_id", None) if isinstance(candidate, str) and candidate: + if candidate != sdk_session_id and on_session_id is not None: + on_session_id(candidate) sdk_session_id = candidate # The *last* usage seen, recorded once after the loop -- not one # record per message. `ResultMessage` arrives last and carries the @@ -324,6 +374,37 @@ def _delta_of(message: Any) -> str: return text if isinstance(text, str) else "" +def _thinking_delta_of(message: Any) -> str: + """The reasoning a partial-message stream event carries, if any. + + The mirror of `_delta_of`, and a separate function rather than a parameter on + it: the two feed different halves of the transcript, and the whole reason + `_delta_of` filters as hard as it does is that mixing them makes the stream + say something the settled message does not. + """ + event = getattr(message, "event", None) + if not isinstance(event, dict) or event.get("type") != "content_block_delta": + return "" + delta = event.get("delta") + if not isinstance(delta, dict) or delta.get("type") != "thinking_delta": + return "" + text = delta.get("thinking") + return text if isinstance(text, str) else "" + + +def _thinking_of(message: Any) -> str: + """The reasoning in a finished message. + + A `ThinkingBlock` carries `.thinking`, not `.text`, which is exactly why + `_text_of` misses it -- and why the reasoning needed a second pair of + accessors rather than a looser filter on the first. + """ + content = getattr(message, "content", None) + if not isinstance(content, list): + return "" + return "".join(getattr(b, "thinking", "") or "" for b in content) + + class TextStream: """One turn's visible text, assembled from deltas *and* finished messages. @@ -343,22 +424,29 @@ class TextStream: `feed` returns only the text that has not been shown yet, so a CLI can print its return value directly; `text` is the whole answer so far, for a UI that re-renders from it. + + The two accessors are parameters because the *reasoning* half of a turn + arrives the same way and has the same trap: `thinking_delta` events followed + by a `ThinkingBlock` containing all of them. One class, given the other pair + of accessors, is what keeps the no-duplication rule stated once. """ - def __init__(self) -> None: + def __init__(self, delta_of: Any = None, whole_of: Any = None) -> None: self.text = "" #: The tail of `text` contributed by deltas since the last finished #: message -- the part a finished message is entitled to overwrite. self._streamed = "" + self._delta_of = delta_of or _delta_of + self._whole_of = whole_of or _text_of def feed(self, message: Any) -> str: - delta = _delta_of(message) + delta = self._delta_of(message) if delta: self.text += delta self._streamed += delta return delta - text = _text_of(message) + text = self._whole_of(message) # A message with no text at all -- a tool result, a system message, the # final result -- must leave a half-streamed block alone. if not text: @@ -521,6 +609,13 @@ def tool_block(use: dict[str, Any]) -> dict[str, Any]: "rows": rows[:MAX_ROWS], "status": "running", "result": "", + # Wall clock rather than `time.monotonic`, because this is written into + # the session file and read back in another process, where a monotonic + # reading from a previous boot means nothing at all. What reads it is the + # tasks window, which reports how long the call in flight has been + # running -- the difference between a spinner and knowing a forty-minute + # job is still going. + "started": time.time(), } @@ -539,23 +634,42 @@ class TurnStream: happened. Text assembly is delegated to `TextStream` unchanged, including its rule that a finished message replaces the deltas that built it. + Reasoning is a third kind of block, kept beside the prose rather than folded + into it. It is *not* part of `text`: the answer and the working are different + claims, the transcript's `text` is what the agent said, and a UI that wants + the working can ask for the blocks. Which is exactly what the chat window's + statusline toggles. + `feed` returns what a CLI should print next; a UI re-renders from `blocks`. """ def __init__(self) -> None: self.blocks: list[dict[str, Any]] = [] self._text = TextStream() + self._think = TextStream(_thinking_delta_of, _thinking_of) #: The block the current run of prose is accumulating into, if open. self._open: dict[str, Any] | None = None + #: The same, for the current run of reasoning. + self._open_thought: dict[str, Any] | None = None #: Tool blocks by call id, so a result can find the call it answers. self._calls: dict[str, dict[str, Any]] = {} @property def text(self) -> str: - """The turn's prose, tool cards left out -- what a plain transcript says.""" + """The turn's prose, tool cards and reasoning left out -- what a plain + transcript says.""" return "".join(b["text"] for b in self.blocks if b["kind"] == "text") + @property + def thinking(self) -> str: + """The turn's reasoning, for a caller that wants only that half.""" + return "".join(b["text"] for b in self.blocks if b["kind"] == "thinking") + def feed(self, message: Any) -> str: + # Reasoning first, because that is the order the API sends it in: a + # message carrying both a `ThinkingBlock` and a `TextBlock` reasoned + # before it answered, and `blocks` is read top to bottom. + self._feed_thinking(message) printed = self._feed_text(message) for use in _tool_uses(message): block = tool_block(use) @@ -563,8 +677,12 @@ def feed(self, message: Any) -> str: self._calls[block["id"]] = block # A tool call ends the run of prose above it: whatever the agent says # next belongs *below* the card, because that is when it said it. + # The reasoning run ends with it, for the same reason -- interleaved + # thinking resumes *after* the call, not inside the block above it. self._text = TextStream() self._open = None + self._think = TextStream(_thinking_delta_of, _thinking_of) + self._open_thought = None printed += _tool_line(block) for result in _tool_results(message): block = self._calls.get(result["id"]) @@ -583,6 +701,8 @@ def note(self, text: str) -> None: if not text: return self._text = TextStream() + self._think = TextStream(_thinking_delta_of, _thinking_of) + self._open_thought = None self._open = {"kind": "text", "text": text} self.blocks.append(self._open) @@ -604,6 +724,19 @@ def _feed_text(self, message: Any) -> str: self._open["text"] = self._text.text return chunk + def _feed_thinking(self, message: Any) -> None: + """The same shape as `_feed_text`, over the reasoning accessors. + + Nothing is returned: `feed`'s return value is what a CLI prints, and the + reasoning is not that. It is a block a UI can choose to draw. + """ + self._think.feed(message) + if self._think.text: + if self._open_thought is None: + self._open_thought = {"kind": "thinking", "text": ""} + self.blocks.append(self._open_thought) + self._open_thought["text"] = self._think.text + def _tool_line(block: dict[str, Any]) -> str: subject = f" {block['title']}" if block["title"] else "" @@ -677,6 +810,38 @@ async def run_probe() -> int: # --------------------------------------------------------------------------- +def _configure_logging() -> None: + """Log to a file under the app directory, and say so nowhere on screen. + + The desktop app is a GUI process with no console to print a traceback into, + so before this the only record of a window that failed to render was a + `log.exception` written to a handler that did not exist. Rotating, because + this is the one file here nobody will ever remember to delete. + + stderr keeps its handler when there *is* a console: the CLI is the same + entry point, and swallowing its output to a file nobody asked for would be + worse than the problem being solved. + """ + from logging.handlers import RotatingFileHandler # noqa: PLC0415 + + root = logging.getLogger() + if any(isinstance(h, RotatingFileHandler) for h in root.handlers): + return + appdata.ensure() + try: + handler = RotatingFileHandler( + appdata.logs_dir() / "grad.log", maxBytes=2_000_000, backupCount=3, encoding="utf-8" + ) + except OSError: # a locked or unwritable log must not stop the app opening + return + handler.setFormatter( + logging.Formatter("%(asctime)s %(levelname)-7s %(name)s: %(message)s") + ) + root.addHandler(handler) + if root.level == logging.NOTSET or root.level > logging.INFO: + root.setLevel(logging.INFO) + + def main() -> None: parser = argparse.ArgumentParser( prog="grad", @@ -689,24 +854,47 @@ def main() -> None: parser.add_argument( "--port", type=int, - default=8080, - help="port for --ui; move it when something else already holds 8080", + default=None, + help="pin the --ui port; by default 8080, or the next free port above it", ) parser.add_argument("--check", action="store_true", help="report environment and auth posture, then exit") args = parser.parse_args() + _configure_logging() + for name in appdata.migrate_legacy(): + logging.getLogger("grad").info("moved data/%s into %s", name, appdata.app_dir()) + if args.check: print(json.dumps(preflight_environment(), indent=2)) return if args.probe: raise SystemExit(asyncio.run(run_probe())) if args.ui: + from core import instance # noqa: PLC0415 + + # One workspace at a time. Two would fight over the layout file, the + # transcript directory and the Lab server's recorded origin, and the + # second would bind a different port -- which is exactly the mismatch + # that stops Lab embedding. See `core/instance.py`. + try: + instance.acquire() + except instance.AlreadyRunning as running: + if instance.show_running(running.info): + return + raise SystemExit( + f"{running}\nIt is not answering on that port. If it is wedged, end the " + "`python` process holding it and start again." + ) from None + from ui.app import run as run_ui # noqa: PLC0415 # A non-default port also moves the app's origin, and the embedded Lab # scopes its `frame-ancestors` to that origin -- so `tools.lab` needs # `--ui-origin http://127.0.0.1:` to match, or the iframe is blocked. - run_ui(port=args.port) + try: + run_ui(port=args.port) + finally: + instance.release() return prompt = " ".join(args.prompt) if args.prompt else None diff --git a/config/jupyter/jupyter_server_config.py b/config/jupyter/jupyter_server_config.py index dae6073..9039a6c 100644 --- a/config/jupyter/jupyter_server_config.py +++ b/config/jupyter/jupyter_server_config.py @@ -27,6 +27,9 @@ """ import os +import pathlib +import re +import sys # The Grad UI's origin. `tools/lab.py` sets this when it launches the server, so # the two cannot drift apart; the default matches ui/app.py's default port. @@ -58,13 +61,46 @@ "X-Frame-Options": "", }, } -c.ServerApp.allow_origin = _APP_ORIGIN +# The same pair of hosts as the CSP above, and for the same reason. `allow_origin` +# takes exactly one origin, so setting it to `127.0.0.1:` rejects the +# websocket when the app is opened on `localhost:` -- and *that* failure is +# the confusing half: the page renders, the frame loads, and only the kernel +# connection dies. `allow_origin_pat` is the regex form, which is how both can be +# named without widening this to `*`. +_ORIGINS = [_APP_ORIGIN] + ([_LOCALHOST_ALIAS.strip()] if _LOCALHOST_ALIAS else []) +c.ServerApp.allow_origin_pat = "|".join(re.escape(origin) for origin in _ORIGINS) c.ServerApp.allow_credentials = True # The websocket the kernel connection rides on. c.ServerApp.allow_remote_access = False c.ServerApp.disable_check_xsrf = False +# Where jupyter-lsp looks for language servers, stated rather than discovered. +# +# `LanguageServerManager.node_roots` is a traitlet with a `@default` generator, +# and that generator ends by running `npm prefix -g` to find a custom global +# prefix (jupyter_lsp/types.py, `_default_node_roots`). It is a `subprocess.run` +# with no `creationflags`, and on Windows `npm` is `npm.cmd` -- a batch file, so +# Windows runs it through `cmd.exe`. That is the black window titled *npm +# prefix* that appears a second or two after the Lab tab starts. +# +# Setting the trait here means the `@default` generator never runs, so the probe +# never happens. The list below is what the generator would have produced minus +# that last step: the server's own root, JupyterLab's staging directory, and the +# environment prefix (conda puts `node_modules` in `$PREFIX/lib` on POSIX and +# directly in `%PREFIX%` on Windows). `extra_node_roots` is searched first and is +# left alone, so a user who installs a language server somewhere unusual still +# has the documented way to say so. +_NODE_ROOTS = [str(pathlib.Path.cwd())] +try: + from jupyterlab import commands as _lab_commands + + _NODE_ROOTS.append(str(pathlib.Path(_lab_commands.get_app_dir()) / "staging")) +except Exception: # noqa: BLE001 - a missing staging dir is not a reason to fail startup + pass +_NODE_ROOTS += [str(pathlib.Path(sys.prefix) / "lib"), str(pathlib.Path(sys.prefix))] +c.LanguageServerManager.node_roots = _NODE_ROOTS + # Kernel ownership discipline (§19): Lab has its own kernel manager and # `tools/nb.py` spawns its own detached kernels. Two owners over one notebook # reproduces the "works in the kernel that grew it" failure `nb verify` exists diff --git a/core/appdata.py b/core/appdata.py new file mode 100644 index 0000000..e10a7fa --- /dev/null +++ b/core/appdata.py @@ -0,0 +1,321 @@ +"""Where the *app's* state lives, which is not where the *research* lives. + +Two roots, and the split between them is the whole point of this module. + +`core/paths.py` resolves the **workspace**: the ledger, the notebooks, the notes +and figures a report cites. That is the user's research. It is versioned beside +the code that produced it, and the README's claim that "every number in a report +traces to a run record" is only checkable because the record sits next to the +number. None of it moves here, and a helper that would move it does not belong +in this file. + +This module resolves the **installation**: state belonging to this copy of Grad +on this machine. The window layout, the Lab server's port and token, kernel +connection files, the HTTP cache, the cookie-signing secret, chat transcripts, +logs. Every one of them is regenerable, machine-specific, or private, and none +of them describes a result. In a repository they are noise at best -- the Lab +token and the storage secret are a leak. + +**Transcripts are the awkward case, and they are why `workspace_state_dir` +exists.** They are private, so they want to be here; but `ui/app.py:rebind` +documents that switching the workspace root switches which conversation is on +screen, and a single flat directory would quietly break that -- one transcript +pile shared by every folder you ever opened. So the per-workspace state is +namespaced by the root it belongs to: readable stem, plus a hash, because two +different folders can share a name and `D:/work/grad` must not collide with +`C:/old/grad`. + +`GRAD_APP_DIR` overrides everything, which is what lets the test suite point an +installation at a temp directory the way `GRAD_ROOT` already points a workspace +at one. + +On POSIX this is one directory rather than the three XDG would ask for +(`XDG_STATE_HOME`, `XDG_CACHE_HOME`, `XDG_DATA_HOME`). That is deliberate: the +Windows install is the one that matters here, it has exactly one +`%LOCALAPPDATA%\\Grad`, and keeping the two platforms the same shape means a +path bug reproduces on either. +""" + +from __future__ import annotations + +import hashlib +import logging +import os +import shutil +from pathlib import Path + +log = logging.getLogger("grad.appdata") + +#: Subdirectories of the app root, created together by `ensure`. +#: +#: `cache` is deliberately absent. Its only consumer -- `core/http.py:_cache_path` +#: -- creates it on demand, and creating it here would make `migrate_legacy`'s +#: "the destination already exists, leave it alone" guard fire on every single +#: run, so a legacy `data/cache` would never move at all. +_SUBDIRS = ("state", "logs", "workspaces") + + +def app_dir() -> Path: + """This installation's private directory. + + `GRAD_APP_DIR` first, then the platform's per-user application data. The + Windows branch reads `LOCALAPPDATA` rather than joining `~` blindly, because + a roaming profile moves it and the literal path would be wrong exactly on + the machines where it matters. + """ + env = os.environ.get("GRAD_APP_DIR") + if env: + return Path(env).resolve() + if os.name == "nt": + base = os.environ.get("LOCALAPPDATA") + root = Path(base) if base else Path.home() / "AppData" / "Local" + return (root / "Grad").resolve() + return (Path.home() / ".local" / "state" / "grad").resolve() + + +def state_dir() -> Path: + """Small persistent files: layouts, Lab's port and token, the instance lock.""" + return app_dir() / "state" + + +def logs_dir() -> Path: + return app_dir() / "logs" + + +def cache_dir() -> Path: + """Regenerable downloads. Safe to delete; nothing cites it.""" + return app_dir() / "cache" + + +def _slug(value: str) -> str: + """A readable, filesystem-safe stem. Never the whole name -- see `_key`.""" + keep = "".join(c if c.isalnum() or c in "._-" else "-" for c in value) + while ".." in keep: + keep = keep.replace("..", ".") + return keep.strip("._-")[:32] or "workspace" + + +def _key(root: Path) -> str: + """A stable directory name for a workspace root. + + Stem *and* digest. The stem alone collides -- every checkout called `grad` + would share one directory, which is precisely the "one transcript pile" the + module docstring rules out. The digest alone is unreadable, and someone will + eventually have to look in here and work out which folder a directory + belongs to. Case-folded first because Windows paths are case-insensitive and + `D:\\Grad` and `d:\\grad` are the same workspace. + + **Resolved first, and that is the load-bearing line.** The digest is taken + over the path's *text*, so two spellings of one directory are two different + workspaces to this function: `D:/work/grad` and `D:/work/./grad`, a relative + path and its absolute form, a symlink and its target. Every reader arrives + through `paths.root()`, which resolves; a caller that passes a root + explicitly -- `migrate_legacy` is the one that does -- may not have. Without + this line that caller writes into a key nothing ever reads, which is the + same silent failure as a migration landing in the wrong directory: the + source is gone, the destination is real, and the app opens on defaults with + nothing to explain it. + """ + resolved = Path(root).resolve() + text = str(resolved).casefold() + digest = hashlib.sha256(text.encode("utf-8")).hexdigest()[:12] + return f"{_slug(resolved.name)}-{digest}" + + +def workspace_state_dir(root: Path | None = None) -> Path: + """App state that belongs to one workspace, such as its chat transcripts. + + Defaults to the current root. Imported lazily for the same reason + `core/paths.py` imports `core/workspace.py` lazily: almost everything + imports this module, and the dependency would otherwise run in a circle. + """ + if root is None: + from core import paths # noqa: PLC0415 + + root = paths.root() + path = app_dir() / "workspaces" / _key(Path(root)) + return path + + +def lock_path() -> Path: + """The single-instance lock. See `core/instance.py`.""" + return state_dir() / "instance.json" + + +def ensure() -> None: + """Create the app directories. Cheap, idempotent, and safe to call early.""" + for name in _SUBDIRS: + (app_dir() / name).mkdir(parents=True, exist_ok=True) + + +# --------------------------------------------------------------------------- +# migration +# --------------------------------------------------------------------------- +#: `data/` in a workspace -> which directory it belongs in now. Only +#: entries that are unambiguously app state; `data/papers`, `data/corpus.sqlite` +#: and `data/mnist` are cited or downloaded research and are deliberately absent. +#: +#: The bucket is *which* of the three roots, and it has to match what the code +#: that reads the file actually resolves -- a migration that lands somewhere +#: nothing reads is worse than none, because the old copy is gone and the app +#: silently starts from defaults. `layouts` and `kernel` are per-workspace +#: (`ui/state.py:layout_dir`, `tools/nb.py:_conn_path`); `lab` is one server per +#: installation and `cache` is regenerable, so both are installation-wide. +_INSTALL, _WORKSPACE, _CACHE = "install", "workspace", "cache" +_LEGACY: tuple[tuple[str, str], ...] = ( + ("layouts", _WORKSPACE), + ("kernel", _WORKSPACE), + ("lab", _INSTALL), + ("cache", _CACHE), +) + +#: Loose files rather than directories, matched by glob under `data/` itself. +#: Transcripts are the conversations, so they are private, and they are keyed to +#: the workspace for the reason `ui/sessions.py:sessions_dir` gives. +#: +#: `data/nb_verify.json` is *not* here on purpose. It records which notebooks +#: verified clean on a fresh kernel, which is what the CITABLE chip and +#: `report check` rest on -- evidence about the research rather than state about +#: the machine -- so it stays in the workspace with the notebooks it describes. +_LEGACY_FILES: tuple[tuple[str, str], ...] = ( + ("ui_session-*.jsonl", _WORKSPACE), + ("ui_storage_secret", _INSTALL), +) + + +def _bucket_dir(bucket: str, name: str, base: Path) -> Path: + """Where a legacy entry lands, which must be exactly where the code that + reads it resolves. `data/cache` is the whole cache directory rather than a + child of it, because `core/http.py` reads `cache_dir()` itself.""" + if bucket == _WORKSPACE: + return workspace_state_dir(base) / name + if bucket == _CACHE: + return cache_dir() + return state_dir() / name + + +def _relocate(source: Path, target: Path) -> bool: + """Copy a directory's contents to `target`, then remove the originals. + + Deliberately *not* `shutil.move`. Moving a directory wholesale is one + operation with two outcomes on Windows, and the bad one loses files: a + single open handle inside -- the Lab server holding its own log, which is + the normal state of affairs when this runs -- makes the rename fail, and the + copy-then-delete fallback it degrades into can delete some sources after + copying them and then abort on the locked one. That leaves files neither + here nor there, which is the one result a migration must never produce. + + So: copy everything first, into a staging directory beside the target; + promote it only once every file has arrived; and only then remove the + sources, each independently and never before its copy exists. A locked file + is left where it is, which is the safe direction -- a duplicate is a tidying + problem, a deletion is not. + """ + staging = target.with_name(f"{target.name}.incoming") + try: + if staging.exists(): + shutil.rmtree(staging, ignore_errors=True) + staging.mkdir(parents=True, exist_ok=True) + for item in source.iterdir(): + if item.is_dir(): + shutil.copytree(item, staging / item.name, dirs_exist_ok=True) + else: + shutil.copy2(item, staging / item.name) + except OSError as exc: + log.debug("could not stage %s: %s", source, exc) + shutil.rmtree(staging, ignore_errors=True) + return False + try: + staging.rename(target) + except OSError as exc: + log.debug("could not promote %s: %s", staging, exc) + shutil.rmtree(staging, ignore_errors=True) + return False + # Only now, and only what verifiably arrived. + for item in source.iterdir(): + landed = target / item.name + if not landed.exists(): + continue + try: + if item.is_dir(): + shutil.rmtree(item) + else: + item.unlink() + except OSError: # held open; the copy is safe, so leaving it is fine + log.debug("left %s in place; it is in use", item) + try: + source.rmdir() # only succeeds if everything above was removed + except OSError: + pass + return True + + +def _relocate_files(legacy: Path, pattern: str, bucket: str, base: Path) -> list[str]: + """Move loose files sitting directly in `data/`, matched by glob. + + A separate pass because these are not directories and the directory entries + are not globs. The transcripts are the reason it exists: they are the + conversations themselves, they sit at the top of `data/` rather than in a + subdirectory of their own, and a migration that moved the layouts but left + them behind would open a workspace with its whole history apparently gone -- + still on disk, still private, and no longer anywhere the app looks. + + Copy-then-delete, and never the other way, for the reason in `_relocate`. + """ + target_dir = workspace_state_dir(base) if bucket == _WORKSPACE else state_dir() + moved: list[str] = [] + for source in sorted(legacy.glob(pattern)): + if not source.is_file(): + continue + target = target_dir / source.name + if target.exists(): + continue + try: + target_dir.mkdir(parents=True, exist_ok=True) + shutil.copy2(source, target) + except OSError as exc: + log.debug("could not migrate %s: %s", source, exc) + continue + try: + source.unlink() + except OSError: # copied safely; a duplicate beats a deletion + log.debug("left %s in place; it is in use", source) + moved.append(source.name) + return moved + + +def migrate_legacy(root: Path | None = None) -> list[str]: + """Move app state out of a workspace that predates this split. + + Non-fatal by construction: this runs at startup, and a workspace on a + read-only mount or a directory held open by a running Lab server must not + stop the app from opening. A destination that already exists is left alone + rather than merged -- the new location is the live one by then, and merging + would resurrect stale state over it. + + Returns what moved, for the caller to log. + """ + from core import paths # noqa: PLC0415 + + # Resolved, so the destinations computed below are keyed exactly as the + # readers key them. `paths.root()` already resolves; an explicit argument + # has no such guarantee. See `_key`. + base = Path(root).resolve() if root is not None else paths.root() + legacy = base / "data" + if not legacy.is_dir(): + return [] + ensure() + moved: list[str] = [] + for pattern, bucket in _LEGACY_FILES: + moved += _relocate_files(legacy, pattern, bucket, base) + for name, bucket in _LEGACY: + source = legacy / name + if not source.is_dir(): + continue + target = _bucket_dir(bucket, name, base) + if target.exists(): + continue + target.parent.mkdir(parents=True, exist_ok=True) + if _relocate(source, target): + moved.append(name) + return moved diff --git a/core/config.py b/core/config.py index 7a5f60f..03469e4 100644 --- a/core/config.py +++ b/core/config.py @@ -43,6 +43,13 @@ "retrieval": { "s2_base": "https://api.semanticscholar.org/graph/v1", "asta_base": "https://asta-tools.allen.ai/mcp/v1", + # Papers with Code, as revived by Hugging Face. The `.com` site Meta + # shut down is gone; this is the `.co` one, and its v1 API is anonymous, + # read-only and documented by `github.com/huggingface/pwc-cli`. + "pwc_base": "https://paperswithcode.co/api/v1", + # arXiv's Atom API, used for one thing: abstracts in bulk. See + # `arxiv_abstracts`. + "arxiv_base": "https://export.arxiv.org/api/query", "openrouter_base": "https://openrouter.ai/api/v1", "rerank_model": "voyageai/rerank-2.5", "embed_model": "voyage-4", @@ -64,17 +71,44 @@ "triage_top": 15, "cache_ttl_s": 604800, "request_timeout_s": 60, + # The ceiling on ONE request end to end, as distinct from + # `request_timeout_s`, which httpx applies per socket read. + # + # The distinction is the whole bug it exists to stop. Asta answers a + # `tools/call` with an event stream and holds it open while it works, + # sending `: ping` comments every 15s. Every ping resets the per-read + # timeout, so a 60s read timeout never fires no matter how long the + # server takes, and a buffered read of that stream waits for a close + # that may never come. A total deadline is the only thing that bounds + # it. 300s because a live `search_papers_by_relevance` measured 121s + # and a bound below the real latency is just an outage. + "request_deadline_s": 300, + # Wall clock for the whole of stage 1, as distinct from the per-request + # deadline above. Stage 0 turns one question into six queries and each + # goes to two endpoints, so a five-minute request deadline is a one-hour + # stage -- and the caller kills it long before the endpoints that work + # can contribute anything. When this is spent the funnel stops issuing + # tier-1 calls, keeps what it retrieved, and writes into the trace how + # many queries it actually searched. + "stage1_budget_s": 300, "min_request_interval_s": 1.1, # unauthenticated S2 is ~1 req/s - # Which tier-1 client does discovery: "asta", "s2", or "both". + # Which tier-1 client does discovery: "pwc", "asta", "s2", "both" + # (the two Semantic Scholar doors) or "all". # - # Asta by default because it is the one that *works*. Both reach the - # same Semantic Scholar corpus and both expose snippet search, but S2's - # own API stopped issuing keys to free-domain email addresses, leaving a - # personal account on the shared anonymous pool -- which is rate limited + # Papers with Code by default because it is the one that *answers*. + # Measured against the live services: pwc returns in 1-2s; Asta takes + # ~121s for a search and ~283s to report that its own backend refused a + # connection, and stage 0 multiplies that by six queries and two + # endpoints, so every caller gives up before discovery finishes. S2's + # own API stopped issuing keys to free-domain addresses, leaving a + # personal account on the shared anonymous pool, which is rate limited # often enough that "no results" and "no key" are hard to tell apart. - # Asta's key is optional. "both" is for comparing them, and it doubles - # the request count for candidates that mostly fuse back together. - "tier1": "asta", + # + # The trade is real and worth knowing: Asta is the only one of the three + # with genuine full-text snippets, which is what §5 designed stage-3 + # triage around. Under pwc, triage reads the abstract -- fetched from + # arXiv in one batched request, see `core/http.py:arxiv_abstracts`. + "tier1": "pwc", }, # HANDOFF-2 §18 listed the REST paths as unverified (§23 item 2). They are # now verified against the live API: `/api/v2/libs/search` returns @@ -139,6 +173,17 @@ "model": "claude-opus-5", "permission_mode": "dontAsk", "max_turns": 0, # 0 = unbounded + # Whether the reasoning arrives as *text*, which is what the chat + # window's statusline switches on and off. + # + # This is not the same question as whether the model thinks. Opus 4.7+ + # defaults `display` to "omitted" and sends thinking blocks with a + # signature and no text, so a client that captures reasoning correctly + # still has nothing to show -- which is exactly what a toggle over an + # empty transcript looks like. "summarized" is the only value that + # actually produces text; "omitted" is the SDK's own default and is here + # so turning the feature off is a config edit rather than a code one. + "reasoning": "summarized", }, "hosts": {}, } @@ -330,6 +375,13 @@ def load(path: Path | None = None, *, reload: bool = False) -> Config: ("smoke", "max_steps"), ("smoke", "max_wall_clock_s"), ("smoke", "max_cost_usd"), + # Both bound a wall clock, and both are read straight into arithmetic on + # `time.monotonic()`. Unvalidated, a string here is a TypeError from inside + # a retrieval loop and a negative is a deadline that has already expired -- + # every search failing instantly with a timeout message, which reads as an + # outage at the endpoint rather than as a typo in a config file. + ("retrieval", "request_deadline_s"), + ("retrieval", "stage1_budget_s"), ) diff --git a/core/http.py b/core/http.py index 64d4434..c9b1c80 100644 --- a/core/http.py +++ b/core/http.py @@ -51,7 +51,9 @@ def identifier_of(paper: dict[str, Any]) -> Any: return None -def candidate_id(identifier: Any, title: Any = "", text: Any = "") -> str | None: +def candidate_id( + identifier: Any, title: Any = "", text: Any = "", *, namespace: str = "s2" +) -> str | None: """The key the funnel fuses candidates on, or None when there is not one. Both tier-1 clients mint ids in one `s2:` namespace, deliberately: it is one @@ -70,13 +72,20 @@ def candidate_id(identifier: Any, title: Any = "", text: Any = "") -> str | None when there is not, which fuses genuine duplicates and separates genuine distinctions; and None when there is neither, because a hit with no id, no title and no text has nothing to rank and nothing to cite. + + `namespace` is what keeps that sharing honest once there is more than one + corpus. Asta and S2 share `s2:` because they are the same index and the same + ids; Papers with Code is a different catalogue with its own numbering, and + giving it the same prefix would fuse two unrelated papers whose ids happened + to collide. The digest fallback is namespaced too, so a title-only hit from + one corpus does not silently absorb a title-only hit from another. """ if identifier not in (None, ""): - return f"s2:{identifier}" + return f"{namespace}:{identifier}" material = f"{title or ''}\n{str(text or '')[:400]}".strip() if not material: return None - return "s2:t-" + hashlib.sha256(material.encode("utf-8")).hexdigest()[:16] + return f"{namespace}:t-" + hashlib.sha256(material.encode("utf-8")).hexdigest()[:16] def _httpx() -> Any: @@ -236,6 +245,287 @@ def neighbours(self, paper_id: str, *, direction: str = "citations", limit: int return out +# --------------------------------------------------------------------------- +# Papers with Code -- the corpus that answers in seconds +# --------------------------------------------------------------------------- +class PapersWithCode: + """Tier 1, and the default: the ML/AI catalogue behind `paperswithcode.co`. + + **Why this replaced Asta as the default.** Asta serves the right corpus and + is the only one of these with genuine full-text snippets, but measured + against the live service it answers a `search_papers_by_relevance` in ~121 + seconds and takes ~283 seconds to report that its own backend refused a + connection. Stage 0 turns one question into six queries and each goes to two + endpoints, so that is twenty minutes of discovery before anything is ranked, + and every caller -- the agent's Bash tool at 120s, a shell `timeout`, a + person -- gives up first. This answers in one to two seconds. A retriever + that returns is worth more than a better one that does not. + + **Two search modes, and they are genuinely different rankings.** `keyword` + is lexical and `semantic` is dense, which is exactly the pair `corpus.rrf` + exists to fuse -- so the two verbs the funnel already calls per query map + onto them without the funnel knowing anything changed. + + **What is given up, stated plainly.** Search rows carry no abstract, so §5's + "triage on ~500 words of the paper itself" becomes "triage on the abstract", + and the abstracts are fetched separately -- see `arxiv_abstracts`, which + fills the whole pool in one request because nearly every row here is an + arXiv paper. `related` is a *dense neighbour* rather than a citation edge, + and `neighbours` says so rather than presenting it as the citation graph + §5 asks for. + + Anonymous and read-only: no account, no key, nothing to store. The + endpoints and their parameters are read off `huggingface/pwc-cli`, which is + the reference client for this API. + """ + + #: The largest page this API will return, checked against the live service. + MAX_PAGE = 100 + + #: `funnel verb -> the API's search mode`. The funnel calls both per query + #: and fuses them, which is the whole point of there being two. + MODES = {"snippet_search": "semantic", "paper_search": "keyword"} + + def __init__(self, cfg: Config) -> None: + self.base = str(cfg.get("retrieval", "pwc_base")).rstrip("/") + self.timeout = float(cfg.get("retrieval", "request_timeout_s", 60)) + self.ttl = float(cfg.get("retrieval", "cache_ttl_s", 604800)) + self.interval = float(cfg.get("retrieval", "min_request_interval_s", 1.1)) + + def _get(self, path: str, params: dict[str, Any] | None = None) -> Any: + key = f"pwc:{self.base}:{path}:{json.dumps(params or {}, sort_keys=True)}" + hit = _cached(key, self.ttl) + if hit is not None: + return hit + _throttle("pwc", self.interval) + httpx = _httpx() + try: + resp = httpx.get( + f"{self.base}/{path.lstrip('/')}", + params=params, + headers={"Accept": "application/json", "User-Agent": "grad/1"}, + timeout=self.timeout, + ) + except Exception as exc: # noqa: BLE001 + raise UpstreamError( + f"Papers with Code request failed: {exc}", + fix="retry, or run with --local-only to search papers already ingested", + ) from exc + if resp.status_code == 429: + raise UpstreamError( + "Papers with Code rate-limited the request", + fix="wait and retry; this API is anonymous, so there is no key to raise it", + ) + if resp.status_code >= 400: + raise UpstreamError( + f"Papers with Code returned {resp.status_code}: {resp.text[:200]}", + fix=( + "the endpoint may have moved: check github.com/huggingface/pwc-cli and " + "set [retrieval] pwc_base in config/grad.toml" + ), + ) + try: + data = resp.json() + except Exception as exc: # noqa: BLE001 + raise UpstreamError( + f"Papers with Code returned a body that is not JSON: {resp.text[:200]}", + fix="retry; if it persists the API contract has changed", + ) from exc + _store(key, data) + return data + + def _search(self, query: str, mode: str, limit: int) -> list[dict[str, Any]]: + data = self._get( + "papers/search", + { + "q": query, + "page": 1, + "page_size": max(1, min(self.MAX_PAGE, int(limit))), + "mode": mode, + }, + ) + return _normalise_pwc(_pwc_rows(data, f"papers/search ({mode})"), f"pwc.{mode}") + + def snippet_search(self, query: str, limit: int = 20) -> list[dict[str, Any]]: + """The dense side. Named for the verb the funnel calls, not for what it + returns: there are no snippets here, and `arxiv_abstracts` is what makes + the candidates readable enough to rerank.""" + return self._search(query, self.MODES["snippet_search"], limit) + + def paper_search(self, query: str, limit: int = 20) -> list[dict[str, Any]]: + """The lexical side.""" + return self._search(query, self.MODES["paper_search"], limit) + + def neighbours( + self, paper_id: str, *, direction: str = "citations", limit: int = 20 + ) -> list[dict[str, Any]]: + """Related work, and **not** the citation graph. + + The rows this returns carry `provenance: "dense"` and a similarity + score: they are nearest neighbours in an embedding space, not papers + that cite or are cited by the seed. That still buys recall -- §5's point + is that expansion reaches papers no query string does -- but calling it + a citation edge would put a claim in the trace that the data does not + support, and a ledger entry's basis is the thing that must not be + overstated. The backward direction is refused for the same reason it is + on Asta: there is no endpoint for it, and answering it with the forward + one would double-count a single direction under two names. + """ + if direction != "citations": + return [] + data = self._get( + f"papers/{_quote(paper_id)}/related", + {"limit": max(1, min(self.MAX_PAGE, int(limit)))}, + ) + return _normalise_pwc(_pwc_rows(data, "related"), "pwc.related") + + +def _quote(value: Any) -> str: + """A path segment. Ids here are arXiv ids and integers, but they reach a URL + from a search result rather than from a constant.""" + from urllib.parse import quote # noqa: PLC0415 + + return quote(str(value), safe="._-") + + +def _pwc_rows(data: Any, what: str) -> list[dict[str, Any]]: + """The hits inside a response: `{"results": [...]}`, or a bare list. + + `related` answers with a list and `search` with an envelope, so both are + read. An unrecognised shape raises for the reason `_rows` does: `ok: true` + with no results reads as "the literature has nothing on this". + """ + if isinstance(data, list): + return [item for item in data if isinstance(item, dict)] + if isinstance(data, dict): + for key in ("results", "items", "data"): + if isinstance(data.get(key), list): + return [item for item in data[key] if isinstance(item, dict)] + raise UpstreamError( + f"Papers with Code's {what} returned no recognisable list of hits: " + f"{sorted(data)[:8] if isinstance(data, dict) else type(data).__name__}", + fix=( + "the API contract has changed -- compare it against " + "github.com/huggingface/pwc-cli and update core/http.py:_pwc_rows" + ), + ) + + +def _normalise_pwc(items: list[dict[str, Any]], source: str) -> list[dict[str, Any]]: + out = [] + for item in items: + row = _pwc_row(item, source=source) + if row is not None: + out.append(row) + return out + + +def _pwc_row(item: dict[str, Any], *, source: str) -> dict[str, Any] | None: + """One hit, in the vocabulary the funnel already fuses and reranks. + + The arXiv id leads, and that is deliberate: it is the field that survives + into `external`, that `paper_ingest` takes, and that `arxiv_abstracts` + fetches on -- so a candidate, its abstract and the ingest of its PDF all + name the same paper. The catalogue's own id is the fallback for the rows + that are not arXiv preprints. + """ + arxiv = str(item.get("arxiv_id") or "").strip() + identifier = arxiv or item.get("id") or item.get("route_identifier") + key = candidate_id(identifier, item.get("title"), "", namespace="pwc") + if key is None: + return None + published = str(item.get("published") or "") + return { + "id": key, + # What `neighbours` expands from: the API takes either, and the arXiv id + # is the one that means something outside this catalogue. + "paper_id": arxiv or str(item.get("id") or ""), + "title": item.get("title"), + "year": published[:4] or None, + # No full text here -- see the class docstring. `abstract` is filled in + # afterwards, and the reranker reads whichever of the two is present. + "snippet": "", + "abstract": str(item.get("abstract") or ""), + "section": "", + "source": source, + "citations": item.get("citation_count"), + "external": {"ArXiv": arxiv} if arxiv else {}, + "url": item.get("url_abs") or "", + } + + +# --------------------------------------------------------------------------- +# arXiv -- one request, every abstract +# --------------------------------------------------------------------------- +#: How many ids arXiv will take in one `id_list`. The whole reason to use this +#: rather than a per-paper lookup: a pool of a hundred candidates costs one +#: request instead of a hundred. +ARXIV_BATCH = 100 + + +def arxiv_abstracts( + arxiv_ids: Sequence[str], *, cfg: Config, timeout: float | None = None +) -> dict[str, str]: + """Abstracts for a batch of arXiv ids, as `{id: abstract}`. + + This exists because the fast corpus does not carry abstracts in its search + results and the reranker and stage-3 triage both read them: a candidate pool + of titles alone is a measurably worse funnel. Fetching them one at a time + would cost a request per candidate and undo the reason the corpus was + changed, so this uses `id_list`, which takes a hundred at once. + + Never raises. A missing abstract is a candidate that ranks on its title, + which is what would have happened anyway -- so a failure here degrades the + ranking rather than the run, and the caller records it as a warning. + """ + ids = [str(i).strip() for i in arxiv_ids if str(i or "").strip()][:ARXIV_BATCH] + if not ids: + return {} + key = f"arxiv:abstracts:{json.dumps(sorted(ids))}" + hit = _cached(key, float(cfg.get("retrieval", "cache_ttl_s", 604800))) + if isinstance(hit, dict): + return hit + _throttle("arxiv", max(3.0, float(cfg.get("retrieval", "min_request_interval_s", 1.1)))) + httpx = _httpx() + try: + resp = httpx.get( + str(cfg.get("retrieval", "arxiv_base")), + params={"id_list": ",".join(ids), "max_results": len(ids)}, + headers={"User-Agent": "grad/1"}, + timeout=timeout or float(cfg.get("retrieval", "request_timeout_s", 60)), + ) + resp.raise_for_status() + out = _parse_arxiv_atom(resp.text) + except Exception: # noqa: BLE001 - see the docstring: this degrades, it does not fail + return {} + _store(key, out) + return out + + +def _parse_arxiv_atom(xml: str) -> dict[str, str]: + """`{bare arxiv id: abstract}` out of an Atom feed. + + The id in the feed is a URL with a version suffix (`.../abs/1706.03762v7`) + and the id asked for has neither, so it is reduced to the bare form -- the + caller looks its candidates up by what Papers with Code gave it. + """ + import xml.etree.ElementTree as ET # noqa: PLC0415 + + namespace = {"atom": "http://www.w3.org/2005/Atom"} + try: + root = ET.fromstring(xml) + except ET.ParseError: + return {} + out: dict[str, str] = {} + for entry in root.findall("atom:entry", namespace): + raw = (entry.findtext("atom:id", "", namespace) or "").rsplit("/", 1)[-1] + bare = raw.split("v")[0] if raw else "" + summary = " ".join((entry.findtext("atom:summary", "", namespace) or "").split()) + if bare and summary: + out[bare] = summary + return out + + # --------------------------------------------------------------------------- # Asta -- the same corpus, through a door that opens # --------------------------------------------------------------------------- @@ -244,6 +534,18 @@ def neighbours(self, paper_id: str, *, direction: str = "citations", limit: int #: `initialize` result and this follows it. MCP_PROTOCOL_VERSION = "2025-06-18" +#: The largest `limit` Asta's tools accept, from the service: *"The limit +#: parameter must be between 1 and 100 inclusive."* +#: +#: Clamped here rather than left to callers because it is the service's +#: constraint, not the funnel's. `paper_search.py` divides its candidate ceiling +#: across the expanded queries, so skipping stage 0 -- one query instead of six +#: -- asks for six times as many per call, and the funnel's own `--no-expand` +#: path therefore refused *every* tier-1 call with a validation error. Asking +#: for the most the service will give is the honest reading of "as many as you +#: can"; raising `--candidates` should not be a way to get zero results. +MAX_LIMIT = 100 + class Asta: """Ai2's scientific corpus over MCP, at `asta-tools.allen.ai`. @@ -278,6 +580,10 @@ class Asta: def __init__(self, cfg: Config) -> None: self.base = str(cfg.get("retrieval", "asta_base")).rstrip("/") self.timeout = float(cfg.get("retrieval", "request_timeout_s", 60)) + #: Total wall clock for one request, enforced here rather than by httpx. + #: See `_post`: the per-read timeout above cannot bound a stream that is + #: being kept alive. + self.deadline = float(cfg.get("retrieval", "request_deadline_s", 300)) self.ttl = float(cfg.get("retrieval", "cache_ttl_s", 604800)) self.interval = float(cfg.get("retrieval", "min_request_interval_s", 1.1)) try: @@ -313,23 +619,57 @@ def _headers(self) -> dict[str, str]: return headers def _post(self, body: dict[str, Any]) -> Any: + """One JSON-RPC message out, one back -- under a deadline this enforces. + + **The response is streamed rather than buffered, and that is the whole + fix.** Asta answers `tools/call` with an event stream and holds it open + while it works, sending `: ping` comments every 15 seconds. `httpx`'s + `timeout` is per socket read, so every ping reset it: a buffered + `httpx.post` waited for a close that the server had no obligation to + send, the read timeout could never fire no matter how long it took, and + the funnel's first tier-1 call simply never returned. Discovery was + unreachable, and the failure had no error to go with it. + + Two things bound it now. `_mcp_payload` stops at the reply to *this* + request rather than at the end of the stream -- the answer is what was + asked for, and reading past it is waiting for a close nobody promised -- + and `deadline` caps the whole exchange in case the reply never comes. + """ _throttle("asta", self.interval) httpx = _httpx() + deadline = time.monotonic() + self.deadline try: - resp = httpx.post(self.base, json=body, headers=self._headers(), timeout=self.timeout) + with httpx.stream( + "POST", self.base, json=body, headers=self._headers(), timeout=self.timeout + ) as resp: + # Assigned on `initialize` and echoed from then on. A server that + # does not use sessions simply never sends it. + session = resp.headers.get("mcp-session-id") + if session: + self._session = session + if resp.status_code >= 400: + self._refuse(resp) + # A notification gets 202 Accepted and an empty body; there is + # nothing to parse and nothing to wait for. + if resp.status_code == 202: + return None + return _mcp_payload(resp, request_id=body.get("id"), deadline=deadline) + except UpstreamError: + # Already the shaped refusal, with the fix that belongs to it. Left + # alone rather than re-wrapped as a transport failure -- "Asta rate + # limited the request" and "Asta request failed" send you to two + # different places. + raise except Exception as exc: # noqa: BLE001 raise UpstreamError( f"Asta request failed: {exc}", fix="retry, or run with --local-only to search papers already ingested", ) from exc - # Assigned on `initialize` and echoed from then on. A server that does - # not use sessions simply never sends it. - session = resp.headers.get("mcp-session-id") - if session: - self._session = session - - if resp.status_code == 401 or resp.status_code == 403: + def _refuse(self, resp: Any) -> None: + """Turn a 4xx/5xx into the refusal that says what to do about it.""" + resp.read() + if resp.status_code in (401, 403): raise UpstreamError( f"Asta rejected the request ({resp.status_code})", fix=( @@ -346,19 +686,13 @@ def _post(self, body: dict[str, Any]) -> Any: f"python -m tools.jobs credential set {credentials.ASTA_KEY}" ), ) - if resp.status_code >= 400: - raise UpstreamError( - f"Asta returned {resp.status_code}: {resp.text[:200]}", - fix=( - "the endpoint may have moved: check allenai.org/asta/resources/mcp and " - "set [retrieval] asta_base in config/grad.toml" - ), - ) - # A notification gets 202 Accepted and an empty body; there is nothing - # to parse and nothing to wait for. - if resp.status_code == 202 or not (resp.content or b"").strip(): - return None - return _mcp_payload(resp) + raise UpstreamError( + f"Asta returned {resp.status_code}: {resp.text[:200]}", + fix=( + "the endpoint may have moved: check allenai.org/asta/resources/mcp and " + "set [retrieval] asta_base in config/grad.toml" + ), + ) def _call(self, method: str, params: dict[str, Any] | None = None) -> Any: self._id += 1 @@ -433,11 +767,19 @@ def tool(self, name: str, arguments: dict[str, Any]) -> Any: def snippet_search(self, query: str, limit: int = 20) -> list[dict[str, Any]]: """Full-text excerpts. The reason to prefer this over a metadata search: ~500 words of the paper itself is what stage 3 triages on.""" - data = self.tool("snippet_search", {"query": query, "limit": limit}) + data = self.tool("snippet_search", {"query": query, "limit": _limit(limit)}) return _normalise(_rows(data, "snippet_search"), "asta.snippet") def paper_search(self, query: str, limit: int = 20) -> list[dict[str, Any]]: - data = self.tool("search_papers_by_relevance", {"query": query, "limit": limit}) + # `keyword`, not `query`. The two tools disagree: `snippet_search` takes + # `query` and this one takes `keyword`, and sending the wrong one is not + # a soft failure -- the server answers in ~1s with a pydantic validation + # error ("keyword Field required"), which `tool` raises as an + # UpstreamError, so tier 1 lost this endpoint entirely on every search + # while snippet_search's slowness hid it. Checked against tools/list. + data = self.tool( + "search_papers_by_relevance", {"keyword": query, "limit": _limit(limit)} + ) return _normalise(_rows(data, "search_papers_by_relevance"), "asta.paper") def neighbours( @@ -453,20 +795,41 @@ def neighbours( """ if direction != "citations": return [] - data = self.tool("get_citations", {"paper_id": paper_id, "limit": limit}) + data = self.tool("get_citations", {"paper_id": paper_id, "limit": _limit(limit)}) return _normalise(_rows(data, "get_citations"), "asta.citations") -def _mcp_payload(resp: Any) -> Any: +def _limit(value: Any) -> int: + """A `limit` the service will accept. See `MAX_LIMIT`.""" + try: + wanted = int(value) + except (TypeError, ValueError): + return 20 + return max(1, min(MAX_LIMIT, wanted)) + + +def _mcp_payload(resp: Any, *, request_id: Any = None, deadline: float | None = None) -> Any: """One JSON-RPC message out of a streamable-HTTP response. The same request may be answered with `application/json` or with an SSE - stream, at the server's discretion, so both are handled. For a stream the - *last* `data:` frame carrying a result is taken: progress notifications - share the channel with the answer. + stream, at the server's discretion, so both are handled. + + For a stream this **stops at the reply to `request_id`** rather than reading + to the end. That is not an optimisation: the server may keep the stream open + after answering, pinging every 15 seconds, and a reader that waits for the + close waits forever -- which is exactly how a 60-second read timeout failed + to bound a call that never returned. The reply is the answer; there is + nothing after it worth waiting for. + + Progress notifications share the channel, so frames are filtered to those + carrying a `result` or an `error`, and the last one seen is the fallback for + a server that answers without echoing the id. """ content_type = (resp.headers.get("content-type") or "").lower() if "text/event-stream" not in content_type: + resp.read() + if not (resp.content or b"").strip(): + return None try: return resp.json() except Exception as exc: # noqa: BLE001 @@ -476,15 +839,57 @@ def _mcp_payload(resp: Any) -> Any: ) from exc answer: Any = None - for line in resp.text.splitlines(): - if not line.startswith("data:"): - continue + #: `data:` lines seen since the last event boundary. An SSE event may carry + #: its payload across several of them, joined with newlines -- which is not + #: an exotic corner of the spec but the ordinary way a server emits a JSON + #: body large enough to wrap. Parsing each line on its own throws away every + #: such message as unparseable JSON, so a long enough answer looked exactly + #: like a stream that carried no result at all. + chunks: list[str] = [] + + def _frame() -> Any: + """The completed event's payload, or None if it is not one of ours.""" + if not chunks: + return None try: - frame = json.loads(line[5:].strip()) + value = json.loads("\n".join(chunks)) except json.JSONDecodeError: + return None + if not isinstance(value, dict) or not ("result" in value or "error" in value): + return None + return value + + for line in resp.iter_lines(): + if deadline is not None and time.monotonic() > deadline: + raise UpstreamError( + "Asta held the stream open past its deadline without answering", + fix=( + "retry; raise [retrieval] request_deadline_s in config/grad.toml if the " + "corpus is genuinely this slow, or use --local-only meanwhile" + ), + ) + line = str(line).rstrip("\r") + if line.startswith("data:"): + # Exactly one leading space is part of the framing, not the data. + chunks.append(line[6:] if line.startswith("data: ") else line[5:]) continue - if isinstance(frame, dict) and ("result" in frame or "error" in frame): - answer = frame + if line: + continue # `event:`, `id:`, `retry:`, or a `:` comment + frame = _frame() + chunks.clear() + if frame is None: + continue + answer = frame + if request_id is None or frame.get("id") == request_id: + return answer + # A stream that ends without a trailing blank line still delivered its last + # event; dropping it would fail on precisely the well-formed response that + # arrived in one piece and closed. + frame = _frame() + if frame is not None: + answer = frame + if request_id is None or frame.get("id") == request_id: + return answer if answer is None: raise UpstreamError( "Asta's event stream carried no result", @@ -521,18 +926,32 @@ def _mcp_result(result: Any) -> Any: return text +#: The keys a tool's payload may wrap its hits under, in the order they are +#: tried. `result` is first because it is the one a live call actually returns +#: -- `search_papers_by_relevance` answers `{"result": [{"paperId": …, "title": +#: …}]}` -- and its absence is why tier 1 found nothing even once the argument +#: name was right: every hit was thrown away as an unrecognised envelope, and +#: the funnel raised rather than returning them. +#: +#: Singular, and *not* the JSON-RPC `result` field. By the time this runs, `tool` +#: has already unwrapped the RPC envelope and the tool's own content block, so +#: this is the payload's own key -- they are only spelled the same. +ROW_KEYS = ("result", "data", "results", "snippets", "papers", "citations", "items") + + def _rows(data: Any, tool_name: str) -> list[dict[str, Any]]: """The list of hits inside a tool's payload, whatever it is wrapped in. - Unverified against the live service, so this reads the S2 REST shape and the - obvious flattenings of it. An unrecognised payload raises: a search that - quietly returns nothing reads as "the literature has nothing on this", and - that is a conclusion nobody should draw from a schema change. + Partly verified against the live service now: `search_papers_by_relevance` + uses `result`. The rest are the S2 REST shape and the obvious flattenings of + it, still unverified. An unrecognised payload raises: a search that quietly + returns nothing reads as "the literature has nothing on this", and that is a + conclusion nobody should draw from a schema change. """ if isinstance(data, list): candidates = data elif isinstance(data, dict): - for key in ("data", "results", "snippets", "papers", "citations", "items"): + for key in ROW_KEYS: if isinstance(data.get(key), list): candidates = data[key] break diff --git a/core/instance.py b/core/instance.py new file mode 100644 index 0000000..6b4c147 --- /dev/null +++ b/core/instance.py @@ -0,0 +1,172 @@ +"""One Grad at a time, and a way to reach the one that is already running. + +Two mechanisms, because they answer different questions and only one of them +can be trusted. + +**The lock decides.** On Windows that is a named mutex, on POSIX an `flock` over +a file in the app directory. Both are held by the kernel for as long as the +process lives and both are released when it dies -- including when it is killed, +which is the case a pid file gets wrong. A pid file left behind by a crash says +"already running" forever, and the fix is always the same undignified thing: +telling a user to go and delete a file before their app will open. + +**The state file only describes.** `instance.json` carries the port the running +instance is serving on, so a second launch can hand over to it instead of dying +silently. It is written *after* the lock is taken and it is never consulted to +decide whether to start -- a stale one is an inconvenience, not a lockout. + +Why the port has to be discoverable at all: the app picks the first free port at +or above 8080, so the second launch cannot assume 8080 and cannot guess. Without +this file, double-clicking the shortcut while Grad sits in the notification area +would do nothing at all, which reads exactly like a broken shortcut. +""" + +from __future__ import annotations + +import json +import logging +import os +import urllib.error +import urllib.request +from typing import Any + +from core import appdata + +log = logging.getLogger("grad.instance") + +#: The mutex name is global to the user's session, not the machine: two people +#: on one Windows box via fast user switching are two installations, and +#: `Local\\` scopes the name to the session. `Global\\` would have one of them +#: refuse to start because the *other* was running. +_MUTEX_NAME = r"Local\GradientAgent.Grad.SingleInstance" + +#: How long to wait for the running instance to answer. It is a local process +#: raising a window; if it has not answered in this long it is wedged, and the +#: honest thing is to say so rather than hang the launcher. +_SHOW_TIMEOUT_S = 3.0 + + +class AlreadyRunning(Exception): + """Raised by `acquire` when another instance holds the lock.""" + + def __init__(self, info: dict[str, Any] | None) -> None: + self.info = info or {} + port = self.info.get("port") + where = f" on port {port}" if port else "" + super().__init__(f"Grad is already running{where}.") + + +class _Lock: + """The held lock. One per process; `release` is idempotent.""" + + def __init__(self) -> None: + self._handle: Any = None + self._fh: Any = None + + def acquire(self) -> bool: + if os.name == "nt": + return self._acquire_windows() + return self._acquire_posix() + + def _acquire_windows(self) -> bool: + import ctypes # noqa: PLC0415 + from ctypes import wintypes # noqa: PLC0415 + + ERROR_ALREADY_EXISTS = 183 + kernel32 = ctypes.WinDLL("kernel32", use_last_error=True) + kernel32.CreateMutexW.argtypes = [wintypes.LPCVOID, wintypes.BOOL, wintypes.LPCWSTR] + kernel32.CreateMutexW.restype = wintypes.HANDLE + handle = kernel32.CreateMutexW(None, True, _MUTEX_NAME) + if not handle: + # Cannot create the mutex at all. Refusing to start over a failure + # of the guard itself would be worse than the duplicate it guards + # against, so this reports "acquired" and lets the app open. + log.debug("CreateMutexW failed: %s", ctypes.get_last_error()) + return True + if ctypes.get_last_error() == ERROR_ALREADY_EXISTS: + kernel32.CloseHandle(handle) + return False + self._handle = handle + return True + + def _acquire_posix(self) -> bool: + import fcntl # noqa: PLC0415 + + appdata.ensure() + path = appdata.state_dir() / "instance.lock" + fh = open(path, "a+b") # noqa: SIM115 - held for the process lifetime + try: + fcntl.flock(fh.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB) + except OSError: + fh.close() + return False + self._fh = fh + return True + + def release(self) -> None: + if self._handle is not None: + import ctypes # noqa: PLC0415 + + ctypes.WinDLL("kernel32").CloseHandle(self._handle) + self._handle = None + if self._fh is not None: + self._fh.close() + self._fh = None + + +_held = _Lock() + + +def read_state() -> dict[str, Any]: + """What the running instance published, or `{}`. Never raises.""" + try: + return json.loads(appdata.lock_path().read_text(encoding="utf-8")) + except (OSError, ValueError): + return {} + + +def publish(port: int) -> None: + """Record where this instance is serving, for the next launch to find.""" + appdata.ensure() + payload = {"pid": os.getpid(), "port": int(port)} + try: + appdata.lock_path().write_text(json.dumps(payload), encoding="utf-8") + except OSError: # the app works without it; only the handover degrades + log.debug("could not publish instance state") + + +def clear() -> None: + try: + appdata.lock_path().unlink(missing_ok=True) + except OSError: + pass + + +def acquire() -> None: + """Take the single-instance lock, or raise `AlreadyRunning`.""" + if not _held.acquire(): + raise AlreadyRunning(read_state()) + + +def release() -> None: + _held.release() + clear() + + +def show_running(info: dict[str, Any] | None = None) -> bool: + """Ask the instance that is already up to raise its window. + + Returns whether it answered. A `False` here is the difference between "your + app is on screen now" and "something is holding the lock but not serving", + and the launcher says different things for the two. + """ + state = info if info is not None else read_state() + port = state.get("port") + if not port: + return False + url = f"http://127.0.0.1:{int(port)}/__grad/show" + try: + with urllib.request.urlopen(url, timeout=_SHOW_TIMEOUT_S) as response: # noqa: S310 + return 200 <= response.status < 300 + except (urllib.error.URLError, OSError, ValueError): + return False diff --git a/core/paths.py b/core/paths.py index a6072be..b193d71 100644 --- a/core/paths.py +++ b/core/paths.py @@ -106,17 +106,25 @@ def config_path() -> Path: def cache_dir() -> Path: - return _p("data", "cache") + """Regenerable downloads, which are an installation's business rather than a + workspace's -- so this is the one path here that resolves outside the root. + See `core/appdata.py` for the split.""" + from core import appdata # noqa: PLC0415 + + return appdata.cache_dir() def ensure_workspace() -> None: - """Create the directories the CLIs write into. Cheap and idempotent.""" + """Create the directories the CLIs write into. Cheap and idempotent. + + `cache_dir` is absent on purpose: it lives under the app directory now, and + `appdata.ensure` is what creates that side. + """ for d in ( ledger_dir(), preflight_dir(), data_dir(), papers_dir(), - cache_dir(), notes_dir(), notebooks_dir(), figures_dir(), diff --git a/core/spawn.py b/core/spawn.py new file mode 100644 index 0000000..b2061c2 --- /dev/null +++ b/core/spawn.py @@ -0,0 +1,85 @@ +"""Spawning a child process without flashing a console window. + +One Windows detail, in one place, because it is invisible on the platform most +of this was written on and unmissable on the one it runs on. + +A console-subsystem executable -- `python.exe`, `jupyter.exe`, `tasklist` -- +inherits its parent's console when there is one. When there is *not*, Windows +allocates a fresh one, and a fresh console is a black window that appears over +whatever you were looking at. The desktop app is precisely the case with no +console to inherit: `ui.run(native=True)` is a GUI process, and every button in +the workspace runs a CLI. Starting JupyterLab flashed one window for the CLI and +another for the `tasklist` that checks whether a previous server is still alive. + +`CREATE_NO_WINDOW` is the flag for "console app, no window". It is **mutually +exclusive with `DETACHED_PROCESS`** -- passing both fails with +`ERROR_INVALID_PARAMETER` rather than being redundant -- so `detached()` is a +separate function rather than an argument. + +**Which of the two a long-lived child gets is not a matter of taste.** +`DETACHED_PROCESS` reads like the stronger promise -- no console at all, rather +than one that is merely hidden -- and it is the weaker one in the only way that +matters here, because a console is *inherited*. A detached child has none to +lend, so the first console program *it* starts is given a fresh one, and a fresh +console is the black window this module exists to prevent. That is not +hypothetical: the Lab server was started detached, and `jupyter-lsp` runs +`npm prefix -g` to locate language servers (`npm` is `npm.cmd` on Windows, so +that is `cmd.exe`). The window appeared a second after the Lab tab opened, from +a grandchild nobody here wrote. + +So `detached()` asks for `CREATE_NO_WINDOW` too: the child gets a console of its +own -- not the parent's, so closing a terminal cannot signal it -- and it is +never shown, and every console descendant inherits that invisibility. The +"outlives its parent" half of the promise is carried by +`CREATE_NEW_PROCESS_GROUP`, which is what actually keeps a Ctrl+C to our group +away from it; process lifetime was never tied to the console. + +Everything here is a no-op off Windows, where none of it is a problem. +""" + +from __future__ import annotations + +import os +import subprocess +from typing import Any + +WINDOWS = os.name == "nt" + +#: "This is a console application; do not give it a window." +NO_WINDOW = getattr(subprocess, "CREATE_NO_WINDOW", 0) if WINDOWS else 0 +#: "A console of its own, never shown, and its own process group." Not +#: `DETACHED_PROCESS`, which the two flags above cannot be combined with anyway +#: -- and which is what gave *grandchildren* windows. See the module docstring. +DETACHED = ( + NO_WINDOW | getattr(subprocess, "CREATE_NEW_PROCESS_GROUP", 0) +) if WINDOWS else 0 + + +def quiet() -> dict[str, Any]: + """Keyword arguments for a child that should not open a window. + + For `subprocess.run`, `subprocess.Popen` and `asyncio.create_subprocess_exec` + alike -- they all take `creationflags`, and all of them ignore it off + Windows because the flag resolves to zero there. + """ + return {"creationflags": NO_WINDOW} if NO_WINDOW else {} + + +def detached() -> dict[str, Any]: + """Keyword arguments for a child that must outlive its parent, quietly. + + Quietly for its whole subtree, which is the part that is easy to get wrong: + see the module docstring for why this is `CREATE_NO_WINDOW` rather than + `DETACHED_PROCESS`. + + `start_new_session` off Windows is the same idea by another mechanism: the + child leads its own process group, so a signal to ours does not reach it. + """ + if WINDOWS: + return {"creationflags": DETACHED} + return {"start_new_session": True} + + +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/install.ps1 b/install.ps1 new file mode 100644 index 0000000..d4231eb --- /dev/null +++ b/install.ps1 @@ -0,0 +1,185 @@ +<# +.SYNOPSIS + Install Grad on Windows: a virtual environment, the dependencies, and a + shortcut that opens the workspace without a console window. + +.DESCRIPTION + This does not produce a single self-contained .exe, and the reason is worth + stating rather than discovering. `claude-agent-sdk` does not call the API + directly -- it spawns the `claude` CLI and speaks stream-JSON over its + stdio. So the agent's brain is a separate native binary holding its own + subscription auth, and no amount of PyInstaller bundling can absorb it. The + same is true of JupyterLab, which is a Python environment with kernels and + is the point of the app rather than an implementation detail of it. + + What is achievable, and what this does, is an install with one visible + entry point: a Start Menu shortcut that launches `pythonw.exe` (no console), + holds the single-instance lock, and lives in the notification area until you + quit it. + +.PARAMETER InstallExtras + Extras to install. Defaults to the set the desktop app needs. + +.PARAMETER NoShortcut + Skip creating the Start Menu and Desktop shortcuts. + +.PARAMETER Python + Python to build the environment with. Defaults to whatever `py -3` or + `python` resolves to. + +.EXAMPLE + powershell -ExecutionPolicy Bypass -File .\install.ps1 +#> + +[CmdletBinding()] +param( + [string] $InstallExtras = "ui,notebook,agent,lab", + [switch] $NoShortcut, + [string] $Python = "" +) + +$ErrorActionPreference = "Stop" +Set-StrictMode -Version Latest + +$Root = Split-Path -Parent $MyInvocation.MyCommand.Path +$VenvDir = Join-Path $Root ".venv" +$AppData = Join-Path $env:LOCALAPPDATA "Grad" + +function Write-Step($message) { Write-Host "==> $message" -ForegroundColor Cyan } +function Write-Warn($message) { Write-Host " ! $message" -ForegroundColor Yellow } +function Write-Ok($message) { Write-Host " + $message" -ForegroundColor Green } + +# -------------------------------------------------------------------------- +# 1. Python +# -------------------------------------------------------------------------- +Write-Step "Locating Python 3.11 or newer" + +function Resolve-Python { + param([string] $Explicit) + if ($Explicit) { return $Explicit } + # `py -3` first: the launcher knows about every install, where `python` on + # PATH may be the Microsoft Store stub that only opens the Store page. + if (Get-Command py -ErrorAction SilentlyContinue) { + $probe = & py -3 -c "import sys; print(sys.executable)" 2>$null + if ($LASTEXITCODE -eq 0 -and $probe) { return $probe.Trim() } + } + if (Get-Command python -ErrorAction SilentlyContinue) { return "python" } + return "" +} + +$PythonExe = Resolve-Python -Explicit $Python +if (-not $PythonExe) { + throw "No Python found. Install 3.11+ from https://www.python.org/downloads/ and re-run." +} + +$Version = & $PythonExe -c "import sys; print('%d.%d' % sys.version_info[:2])" +$Parts = $Version.Split('.') +if ([int]$Parts[0] -lt 3 -or ([int]$Parts[0] -eq 3 -and [int]$Parts[1] -lt 11)) { + throw "Python $Version is too old; Grad needs 3.11 or newer." +} +Write-Ok "Python $Version at $PythonExe" + +# -------------------------------------------------------------------------- +# 2. The environment +# -------------------------------------------------------------------------- +Write-Step "Creating the virtual environment" +if (-not (Test-Path $VenvDir)) { + & $PythonExe -m venv $VenvDir + Write-Ok "created $VenvDir" +} else { + Write-Ok "reusing $VenvDir" +} + +$VenvPython = Join-Path $VenvDir "Scripts\python.exe" +$VenvPythonW = Join-Path $VenvDir "Scripts\pythonw.exe" +if (-not (Test-Path $VenvPython)) { throw "The virtual environment has no python.exe: $VenvPython" } + +Write-Step "Installing Grad and its dependencies (this takes a few minutes)" +& $VenvPython -m pip install --upgrade pip --quiet +# Editable, because this repository *is* the install: the workspace, the +# prompts and the skills are read from here at runtime. +& $VenvPython -m pip install -e "$($Root)[$($InstallExtras)]" +if ($LASTEXITCODE -ne 0) { throw "pip install failed." } +Write-Ok "installed extras: $InstallExtras" + +# -------------------------------------------------------------------------- +# 3. The things this installer cannot install +# -------------------------------------------------------------------------- +Write-Step "Checking what has to be present but cannot be vendored" + +$Claude = Get-Command claude -ErrorAction SilentlyContinue +if ($Claude) { + Write-Ok "claude CLI at $($Claude.Source)" +} else { + Write-Warn 'The "claude" CLI was not found on PATH.' + Write-Warn "The agent spawns it as a subprocess -- without it, the workspace" + Write-Warn "opens but no turn can run. Install it, then re-run this script:" + Write-Warn " npm install -g @anthropic-ai/claude-code" +} + +# WebView2 is what pywebview renders into. It ships with Windows 11 and recent +# 10, so this is a check rather than a step -- but a missing runtime shows up as +# a window that opens blank, which is not a self-explaining failure. +$WebView2 = @( + "HKLM:\SOFTWARE\WOW6432Node\Microsoft\EdgeUpdate\Clients\{F3017226-FE2A-4295-8BDF-00C3A9A7E4C5}", + "HKLM:\SOFTWARE\Microsoft\EdgeUpdate\Clients\{F3017226-FE2A-4295-8BDF-00C3A9A7E4C5}" +) | Where-Object { Test-Path $_ } | Select-Object -First 1 + +if ($WebView2) { + Write-Ok "WebView2 runtime present" +} else { + Write-Warn "WebView2 runtime not detected. The desktop window needs it:" + Write-Warn " https://developer.microsoft.com/microsoft-edge/webview2/" + Write-Warn "Without it, run the browser fallback: python agent.py --ui (then open the port)." +} + +# -------------------------------------------------------------------------- +# 4. The shortcut +# -------------------------------------------------------------------------- +if (-not $NoShortcut) { + Write-Step "Creating shortcuts" + + New-Item -ItemType Directory -Force -Path $AppData | Out-Null + $IconPath = Join-Path $AppData "grad.ico" + # Drawn by the app itself, so the shortcut and the notification area cannot + # show two different marks. See ui/desktop.py:write_icon. + & $VenvPython -c "from ui import desktop; desktop.write_icon(r'$IconPath')" 2>$null + if ($LASTEXITCODE -ne 0 -or -not (Test-Path $IconPath)) { + Write-Warn "could not render the icon; the shortcut will use Python's" + $IconPath = $VenvPythonW + } + + $Shell = New-Object -ComObject WScript.Shell + $Targets = @( + (Join-Path ([Environment]::GetFolderPath('Programs')) "Grad.lnk"), + (Join-Path ([Environment]::GetFolderPath('Desktop')) "Grad.lnk") + ) + foreach ($LinkPath in $Targets) { + $Link = $Shell.CreateShortcut($LinkPath) + # pythonw.exe, not python.exe: the console-subsystem interpreter would + # put a black window behind the app for its whole lifetime. This is the + # same reasoning core/spawn.py applies to every child process. + $Link.TargetPath = $VenvPythonW + $Link.Arguments = "`"$(Join-Path $Root 'agent.py')`" --ui" + $Link.WorkingDirectory = $Root + $Link.IconLocation = $IconPath + $Link.Description = "Grad - a personal research agent for mathematics and machine learning" + $Link.Save() + Write-Ok $LinkPath + } +} + +# -------------------------------------------------------------------------- +# 5. Where things live +# -------------------------------------------------------------------------- +Write-Host "" +Write-Step "Done" +Write-Host " Workspace (your research, versioned): $Root" +Write-Host " App state (layouts, logs, Lab token): $AppData" +Write-Host "" +Write-Host " Start it from the Start Menu, or:" +Write-Host " $VenvPythonW `"$(Join-Path $Root 'agent.py')`" --ui" +Write-Host "" +Write-Host " It opens on port 8080, or the next free port above it, and stays in" +Write-Host " the notification area when you close the window. Quit from there." +Write-Host "" diff --git a/install.sh b/install.sh new file mode 100644 index 0000000..d810d2f --- /dev/null +++ b/install.sh @@ -0,0 +1,165 @@ +#!/usr/bin/env bash +# Install Grad: a virtual environment, the dependencies, and a `grad` launcher. +# +# This is the portable half of the installer. It covers three situations, and it +# is worth being clear about which one you are in, because they do not all get +# the same app: +# +# * Git Bash / MSYS on Windows -- the full desktop app. Identical result to +# install.ps1, except that the Start Menu shortcut is that script's job: +# creating a .lnk needs COM. Run install.ps1 if you want the shortcut. +# * WSL -- the CLI and the browser UI. The native window needs WebView2, which +# is on the Windows side of the boundary, so `--ui` falls back to a browser. +# * Linux / macOS -- the CLI and the browser UI. The workspace, the ledger, +# the gates and the notebooks all work; the native window and the +# notification-area icon are Windows features and are skipped. +# +# What no installer on any platform can do is vendor the `claude` CLI. The SDK +# spawns it as a subprocess and speaks stream-JSON over its stdio, so it is a +# separate native binary with its own auth. This script checks for it and tells +# you how to get it. + +set -euo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +VENV="$ROOT/.venv" +EXTRAS="${GRAD_EXTRAS:-ui,notebook,agent,lab}" + +step() { printf '\033[36m==> %s\033[0m\n' "$1"; } +ok() { printf ' \033[32m+\033[0m %s\n' "$1"; } +warn() { printf ' \033[33m!\033[0m %s\n' "$1"; } +die() { printf ' \033[31mx\033[0m %s\n' "$1" >&2; exit 1; } + +case "$(uname -s)" in + MINGW*|MSYS*|CYGWIN*) PLATFORM="windows" ;; + Darwin) PLATFORM="macos" ;; + *) PLATFORM="linux" ;; +esac +if [ "$PLATFORM" = "linux" ] && grep -qi microsoft /proc/version 2>/dev/null; then + PLATFORM="wsl" +fi + +# --------------------------------------------------------------------------- +# 1. Python +# --------------------------------------------------------------------------- +step "Locating Python 3.11 or newer" +PYTHON="" +for candidate in "${GRAD_PYTHON:-}" python3.13 python3.12 python3.11 python3 python; do + [ -n "$candidate" ] || continue + command -v "$candidate" >/dev/null 2>&1 || continue + if "$candidate" -c 'import sys; raise SystemExit(0 if sys.version_info >= (3, 11) else 1)' 2>/dev/null; then + PYTHON="$candidate" + break + fi +done +[ -n "$PYTHON" ] || die "No Python 3.11+ found. Install one and re-run, or set GRAD_PYTHON." +ok "$($PYTHON -c 'import sys; print("Python %d.%d at %s" % (*sys.version_info[:2], sys.executable))')" + +# --------------------------------------------------------------------------- +# 2. The environment +# --------------------------------------------------------------------------- +step "Creating the virtual environment" +if [ ! -d "$VENV" ]; then + "$PYTHON" -m venv "$VENV" + ok "created $VENV" +else + ok "reusing $VENV" +fi + +# Windows venvs put the interpreter in Scripts/, POSIX in bin/. Git Bash sees +# the Windows layout, so this cannot key off the shell. +if [ -x "$VENV/Scripts/python.exe" ]; then + VPY="$VENV/Scripts/python.exe" + VPYW="$VENV/Scripts/pythonw.exe" +else + VPY="$VENV/bin/python" + VPYW="" +fi +[ -x "$VPY" ] || die "The virtual environment has no interpreter at $VPY" + +step "Installing Grad and its dependencies (this takes a few minutes)" +"$VPY" -m pip install --upgrade pip --quiet +# Editable, because this repository *is* the install: the workspace, the prompts +# and the skills are read from here at runtime. +"$VPY" -m pip install -e "$ROOT[$EXTRAS]" || die "pip install failed." +ok "installed extras: $EXTRAS" + +# --------------------------------------------------------------------------- +# 3. What cannot be vendored +# --------------------------------------------------------------------------- +step "Checking what has to be present but cannot be vendored" +if command -v claude >/dev/null 2>&1; then + ok "claude CLI at $(command -v claude)" +else + warn "The 'claude' CLI was not found on PATH." + warn "The agent spawns it as a subprocess -- without it the workspace opens" + warn "but no turn can run. Install it and re-run:" + warn " npm install -g @anthropic-ai/claude-code" +fi + +case "$PLATFORM" in + windows) ok "native desktop window and notification-area icon available" ;; + wsl) + warn "WSL: the native window needs WebView2 on the Windows side, so --ui" + warn "runs in browser mode. Open the port it prints in a Windows browser." ;; + *) + warn "$PLATFORM: the native window and tray icon are Windows features." + warn "--ui runs in browser mode; everything else is unaffected." ;; +esac + +# --------------------------------------------------------------------------- +# 4. The launcher +# --------------------------------------------------------------------------- +step "Creating the launcher" +BIN_DIR="${GRAD_BIN_DIR:-$HOME/.local/bin}" +mkdir -p "$BIN_DIR" +LAUNCHER="$BIN_DIR/grad" + +# pythonw.exe where there is one: on Windows the console interpreter would keep +# a black window open behind the app for its whole lifetime, which is the same +# problem core/spawn.py solves for every child process. +if [ -n "$VPYW" ] && [ -x "$VPYW" ]; then + RUNNER="$VPYW" +else + RUNNER="$VPY" +fi + +cat > "$LAUNCHER" <> ~/.bashrc" ;; +esac + +# --------------------------------------------------------------------------- +# 5. Where things live +# --------------------------------------------------------------------------- +if [ "$PLATFORM" = "windows" ]; then + APP_STATE="%LOCALAPPDATA%\\Grad" +else + APP_STATE="$HOME/.local/state/grad" +fi + +printf '\n' +step "Done" +printf ' Workspace (your research, versioned): %s\n' "$ROOT" +printf ' App state (layouts, logs, Lab token): %s\n' "$APP_STATE" +printf '\n' +printf ' Start the workspace: grad --ui\n' +printf ' Ask a single question: grad "what is in the ledger?"\n' +printf '\n' +if [ "$PLATFORM" = "windows" ]; then + printf ' For a Start Menu shortcut, run install.ps1 as well:\n' + printf ' powershell -ExecutionPolicy Bypass -File %s\n\n' "$ROOT/install.ps1" +fi diff --git a/pyproject.toml b/pyproject.toml index b0fcef7..0dd2129 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -29,7 +29,7 @@ remote = ["keyring>=25.0", "huggingface-hub>=0.24"] # 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 # coexists with a `@ui.page` route. On 2.x the shell raises at page build. -ui = ["nicegui>=3.0", "pywebview>=5.0", "nbformat>=5.10", "nbconvert>=7.16"] +ui = ["nicegui>=3.0", "pywebview>=5.0", "nbformat>=5.10", "nbconvert>=7.16", "grad[lab]"] math = ["sympy>=1.13", "mpmath>=1.3"] # HANDOFF-2 §19: the extension set is *declared*, not accumulated, and every pin # is exact rather than a floor. The JupyterLab 3->4 break is what killed the @@ -37,8 +37,15 @@ math = ["sympy>=1.13", "mpmath>=1.3"] # the Lab tab down. "Connect an arbitrary extension" means: add a pin here, # reinstall, restart. Read any *server* extension before adding it -- it runs in # the Lab process with your filesystem rights and can reach the credential store. -lab = [ - "jupyterlab==4.4.7", +# +# Split in two, because these answer different questions. The notebook window is +# one of the twelve and its interior *is* Lab, so shipping the app without a Lab +# server ships a window whose only content is a button that fails -- `ui` now +# depends on this extra for that reason. The extensions are a preference and stay +# opt-in: `python-lsp-server[all]` alone is heavier than everything else in this +# file put together. +lab = ["jupyterlab==4.4.7"] +lab-extensions = [ "jupyterlab-lsp==5.1.1", "python-lsp-server[all]==1.12.2", "jupyterlab-git==0.51.1", diff --git a/tests/conftest.py b/tests/conftest.py index ce358b1..615d642 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -28,6 +28,29 @@ def workspace(tmp_path, monkeypatch): config._cache.clear() +@pytest.fixture(autouse=True) +def isolate_app_dir(tmp_path, monkeypatch): + """Point the *installation* at a temp directory too, for every test. + + `GRAD_ROOT` isolates the workspace; it does not isolate `core/appdata.py`, + which resolves outside the root by design -- the Lab server's port and + token, the window layouts, the chat transcripts, the instance lock. Without + this, running the suite writes into the developer's real + `%LOCALAPPDATA%\\Grad`: it would overwrite the `lab.json` of a Lab server + they have open, and tests would read each other's leftovers through it + rather than starting clean. + + Autouse and separate from `workspace`, because the app directory is reached + by modules that have no workspace at all. + """ + # A *sibling* of the workspace, never a child. `workspace` points GRAD_ROOT + # at `tmp_path`, so nesting the app directory inside it would make the one + # property this split exists for -- app state is not in the workspace -- + # untestable, and true in production but false in every test. + monkeypatch.setenv("GRAD_APP_DIR", str(tmp_path.parent / f"{tmp_path.name}-appdata")) + yield + + @pytest.fixture(autouse=True) def clean_process_state(): """Module-level registries outlive a fixture, so they are emptied around diff --git a/tests/test_asta.py b/tests/test_asta.py index 8c9ebe8..ea4fcf3 100644 --- a/tests/test_asta.py +++ b/tests/test_asta.py @@ -28,17 +28,48 @@ class FakeResponse: + """A streamed response, because that is what the client now opens. + + `read()` and `iter_lines()` are the two halves of `httpx`'s streaming API the + client uses, and they are not interchangeable: the JSON path buffers, and the + event-stream path must *not*, because the server keeps the stream open after + answering. `lines` counts how far a test's reader actually got, which is what + makes "it stopped at the reply" assertable. + """ + def __init__(self, status_code=200, payload=None, text="", content_type="application/json", - headers=None): + headers=None, hang=False): self.status_code = status_code self._payload = payload if payload is not None else {} self.text = text or (json.dumps(self._payload) if payload is not None else "") self.headers = {"content-type": content_type, **(headers or {})} + self.content = b"" + #: A server that never closes the stream: `iter_lines` keeps yielding + #: keepalive comments after the body, the way Asta's does. + self.hang = hang + self.lines = 0 + + def read(self): self.content = self.text.encode() + return self.content + + def iter_lines(self): + for line in self.text.splitlines(): + self.lines += 1 + yield line + while self.hang: + self.lines += 1 + yield ": ping" def json(self): return self._payload + def __enter__(self): + return self + + def __exit__(self, *_): + return False + def rpc(result) -> dict: return {"jsonrpc": "2.0", "id": 1, "result": result} @@ -58,8 +89,8 @@ def transport(monkeypatch): class FakeHttpx: @staticmethod - def post(url, json=None, headers=None, timeout=None): - posts.append({"url": url, "body": json, "headers": headers}) + def stream(method, url, json=None, headers=None, timeout=None): + posts.append({"url": url, "method": method, "body": json, "headers": headers}) if queue: return queue.pop(0) return FakeResponse(payload=rpc({"protocolVersion": "2025-06-18"})) @@ -438,12 +469,14 @@ def test_the_tier_one_selector_builds_the_clients_the_config_names(workspace): from tools import paper_search cfg = config_mod.load(reload=True) + assert [n for n, _ in paper_search.tier1_clients(cfg, "pwc")] == ["pwc"] assert [n for n, _ in paper_search.tier1_clients(cfg, "asta")] == ["asta"] assert [n for n, _ in paper_search.tier1_clients(cfg, "s2")] == ["s2"] assert [n for n, _ in paper_search.tier1_clients(cfg, "both")] == ["asta", "s2"] + assert [n for n, _ in paper_search.tier1_clients(cfg, "all")] == ["pwc", "asta", "s2"] assert paper_search.tier1_clients(cfg, "none") == [] # The default, when nothing overrides it. - assert [n for n, _ in paper_search.tier1_clients(cfg)] == ["asta"] + assert [n for n, _ in paper_search.tier1_clients(cfg)] == ["pwc"] def test_an_unknown_tier_one_source_lists_the_real_ones(workspace): @@ -452,4 +485,141 @@ def test_an_unknown_tier_one_source_lists_the_real_ones(workspace): with pytest.raises(UsageError) as exc: paper_search.tier1_clients(config_mod.load(reload=True), "scholar") - assert "asta" in (exc.value.fix or "") + assert "pwc" in (exc.value.fix or "") + + +# --------------------------------------------------------------------------- +# the stream that is held open -- why the funnel's first call never returned +# --------------------------------------------------------------------------- +def sse(*frames: dict) -> str: + return "".join(f"event: message\ndata: {json.dumps(f)}\n\n" for f in frames) + + +def handshake(queue) -> None: + queue.append(FakeResponse(payload=rpc({"protocolVersion": "2025-06-18"}))) + queue.append(FakeResponse(status_code=202, text="")) + + +def test_the_read_stops_at_the_reply_rather_than_at_the_close(workspace, transport): + """The bug this closes. Asta answers `tools/call` with an event stream and + keeps it open afterwards, pinging every 15s. `httpx`'s timeout is per read, + so every ping reset it and a buffered read waited for a close the server + never promised -- the call did not fail, it never returned, and tier 1 was + simply unreachable with no error to go with it.""" + _, queue = transport + handshake(queue) + # id 2: initialize was 1, and the notification between them carries none. + answer = FakeResponse( + text=sse( + {"jsonrpc": "2.0", "method": "notifications/progress", "params": {}}, + rpc(tool_result({"data": [{"paperId": "p1", "title": "Attention"}]})) | {"id": 2}, + ), + content_type="text/event-stream", + hang=True, + ) + queue.append(answer) + + rows = client().snippet_search("attention") + assert [r["title"] for r in rows] == ["Attention"] + assert answer.lines <= 8, "it kept reading past the answer it already had" + + +def test_a_stream_that_never_answers_is_bounded_by_the_deadline(workspace, transport): + """`request_timeout_s` is per socket read and a keepalive resets it, so the + only thing that can bound this is a total deadline. A search that hangs is + worse than one that fails: nothing points at its own cause.""" + from core import paths + + config = paths.root() / "config" / "grad.toml" + config.parent.mkdir(parents=True, exist_ok=True) + # Zero rather than negative: the deadline is `monotonic() + value`, so zero + # is already behind us by the first check and the stream is bounded on the + # first pass -- while staying a value the config validator accepts. A + # negative one tested the timeout through a setting that is now refused. + config.write_text("[retrieval]\nrequest_deadline_s = 0\n", encoding="utf-8") + + _, queue = transport + handshake(queue) + queue.append(FakeResponse(text="", content_type="text/event-stream", hang=True)) + + with pytest.raises(UpstreamError, match="deadline"): + client().snippet_search("attention") + + +def test_the_deadline_message_names_the_key_that_moves_it(workspace, transport): + from core import paths + + config = paths.root() / "config" / "grad.toml" + config.parent.mkdir(parents=True, exist_ok=True) + # Zero rather than negative: the deadline is `monotonic() + value`, so zero + # is already behind us by the first check and the stream is bounded on the + # first pass -- while staying a value the config validator accepts. A + # negative one tested the timeout through a setting that is now refused. + config.write_text("[retrieval]\nrequest_deadline_s = 0\n", encoding="utf-8") + + _, queue = transport + handshake(queue) + queue.append(FakeResponse(text="", content_type="text/event-stream", hang=True)) + with pytest.raises(UpstreamError) as exc: + client().snippet_search("attention") + assert "request_deadline_s" in (exc.value.fix or "") + + +def test_relevance_search_sends_the_argument_that_tool_actually_takes(workspace, transport): + """`snippet_search` takes `query` and this one takes `keyword`, and sending + the wrong one is not a soft failure: the server answers in ~1s with a + pydantic validation error, so tier 1 lost this endpoint on every search + while snippet_search's slowness hid it.""" + posts, queue = transport + handshake(queue) + queue.append(FakeResponse(payload=rpc(tool_result({"data": []})))) + + client().paper_search("efficient optimizers", limit=7) + arguments = posts[-1]["body"]["params"]["arguments"] + assert arguments == {"keyword": "efficient optimizers", "limit": 7} + + +def test_the_envelope_a_live_call_actually_uses_is_read(workspace, transport): + """`search_papers_by_relevance` answers `{"result": [...]}` -- singular, and + a key this did not know. Every hit was thrown away as an unrecognised + envelope, so tier 1 found nothing even once the argument name was right, and + the funnel raised rather than returning them. + + Not the JSON-RPC `result`: by the time `_rows` runs, the RPC envelope and the + tool's content block are both already unwrapped. They are only spelled the + same.""" + rows = call_with(workspace, transport, {"result": [ + {"paperId": "sha-1", "title": "Memory Efficient Optimizers with 4-bit States"}, + ]}) + assert [r["id"] for r in rows] == ["s2:sha-1"] + assert rows[0]["title"].startswith("Memory Efficient") + + +def test_a_still_unrecognised_envelope_names_the_keys_it_knows(workspace, transport): + from core import http as http_mod + + assert "result" in http_mod.ROW_KEYS + with pytest.raises(UpstreamError, match="no recognisable list"): + call_with(workspace, transport, {"payload": {"hits": []}}) + + +def test_a_limit_above_what_the_service_accepts_is_clamped(workspace, transport): + """*"The limit parameter must be between 1 and 100 inclusive."* The funnel + divides its candidate ceiling across the expanded queries, so skipping stage + 0 asks for six times as many per call -- and `--no-expand` therefore refused + every tier-1 call with a validation error. Raising `--candidates` should not + be a way to get zero results.""" + posts, queue = transport + handshake(queue) + queue.append(FakeResponse(payload=rpc(tool_result({"result": []})))) + + client().paper_search("efficient optimizers", limit=150) + assert posts[-1]["body"]["params"]["arguments"]["limit"] == http.MAX_LIMIT + + +def test_a_nonsense_limit_becomes_a_usable_one_rather_than_a_refusal(workspace, transport): + posts, queue = transport + handshake(queue) + queue.append(FakeResponse(payload=rpc(tool_result({"result": []})))) + client().snippet_search("attention", limit=0) + assert posts[-1]["body"]["params"]["arguments"]["limit"] == 1 diff --git a/tests/test_desktop_app.py b/tests/test_desktop_app.py new file mode 100644 index 0000000..70408d1 --- /dev/null +++ b/tests/test_desktop_app.py @@ -0,0 +1,499 @@ +"""Being a desktop app: where state lives, one instance, and a poll that yields. + +These cover the parts that only show up on a real machine -- a second +double-click of the shortcut, a Lab server left running from a previous port, a +socket that has stopped answering while the window is open. None of them starts +a server or a real process; §24's discipline holds. +""" + +from __future__ import annotations + +import asyncio +import os +import threading +from pathlib import Path + +import pytest + +from core import appdata, instance, paths +from tools import lab as lab_tool +from ui import desktop, models, render, sessions, state as state_mod + + +# --------------------------------------------------------------------------- +# where state lives +# --------------------------------------------------------------------------- +def test_app_state_resolves_outside_the_workspace(workspace): + """The whole point of the split: nothing the app writes for its own + convenience lands in a folder that gets committed.""" + root = paths.root() + for path in ( + appdata.state_dir(), + appdata.logs_dir(), + appdata.cache_dir(), + appdata.lock_path(), + appdata.workspace_state_dir(), + state_mod.layout_dir(), + ): + assert root not in path.parents, f"{path} is inside the workspace" + + +def test_research_paths_stay_in_the_workspace(workspace): + """The other half, and the one that matters more. A report's claim traces to + a run record because the record sits next to it; moving the ledger into + AppData would break that chain and make the repository not self-describing. + """ + root = paths.root() + for path in ( + paths.ledger_dir(), + paths.runs_path(), + paths.notebooks_dir(), + paths.notes_dir(), + paths.figures_dir(), + paths.papers_dir(), + ): + assert root in path.parents or path == root + + +def test_two_workspaces_get_different_app_state(workspace, tmp_path, monkeypatch): + """Transcripts and layouts are per-workspace. One flat directory would hand + every folder the same conversation and the same panes.""" + first = appdata.workspace_state_dir() + monkeypatch.setenv("GRAD_ROOT", str(tmp_path / "elsewhere")) + assert appdata.workspace_state_dir() != first + + +def test_workspaces_with_the_same_name_do_not_collide(tmp_path): + """`D:/work/grad` and `C:/old/grad` are different workspaces with one name, + which a readable-stem-only key would merge.""" + a = appdata.workspace_state_dir(tmp_path / "one" / "grad") + b = appdata.workspace_state_dir(tmp_path / "two" / "grad") + assert a != b + assert a.name.startswith("grad-") and b.name.startswith("grad-") + + +def test_one_directory_spelled_two_ways_is_one_workspace(tmp_path): + """The key is a digest of the path's *text*, so an unresolved spelling is a + different workspace to it. Readers all arrive through `paths.root()`, which + resolves; `migrate_legacy` takes a root argument and may not -- and a + migration keyed differently from its reader is the silent kind of loss.""" + target = tmp_path / "grad" + target.mkdir() + spellings = [ + target, + tmp_path / "." / "grad", + tmp_path / "grad" / "sub" / "..", + Path(str(target) + os.sep), + ] + keys = {appdata.workspace_state_dir(s) for s in spellings} + assert len(keys) == 1, keys + + +def test_a_migration_lands_where_an_unresolved_root_reads(tmp_path, monkeypatch): + """The whole point of resolving: `migrate_legacy` given a scruffy path must + write where a reader given the tidy one will look.""" + root = tmp_path / "ws" + (root / "data" / "layouts").mkdir(parents=True) + (root / "data" / "layouts" / "p.json").write_text("{}", encoding="utf-8") + + appdata.migrate_legacy(tmp_path / "ws" / "sub" / "..") + + monkeypatch.setenv("GRAD_ROOT", str(root)) + assert (state_mod.layout_dir() / "p.json").exists() + + +def test_the_workspace_key_is_stable_across_calls(tmp_path): + """It names a directory holding transcripts; a key that moved would orphan + them on every launch.""" + target = tmp_path / "grad" + assert appdata.workspace_state_dir(target) == appdata.workspace_state_dir(target) + + +def test_migration_moves_app_state_and_leaves_research_alone(workspace): + """An existing workspace predates the split. Its layouts should move; its + papers and datasets are cited or expensive and must not.""" + legacy = workspace / "data" + (legacy / "layouts").mkdir(parents=True, exist_ok=True) + (legacy / "layouts" / "proj.json").write_text("{}", encoding="utf-8") + (legacy / "papers").mkdir(parents=True, exist_ok=True) + (legacy / "papers" / "a.pdf").write_bytes(b"%PDF-") + + moved = appdata.migrate_legacy(workspace) + + assert "layouts" in moved + # Where `ui/state.py:layout_dir` actually reads. A migration that lands + # anywhere else is worse than none: the old copy is gone and the app opens + # on defaults with no error to explain it. + assert (appdata.workspace_state_dir() / "layouts" / "proj.json").exists() + assert not (legacy / "layouts").exists() + # Untouched, and named in the assertion so deleting it from _LEGACY is a + # deliberate act rather than a silent one. + assert (legacy / "papers" / "a.pdf").exists() + + +def test_every_migration_target_is_where_its_reader_looks(workspace): + """The bug this guards is silent by construction, so it is asserted against + the readers rather than against a remembered path.""" + from tools import nb as nb_tool + + (workspace / "data" / "layouts").mkdir(parents=True, exist_ok=True) + (workspace / "data" / "layouts" / "p.json").write_text("{}", encoding="utf-8") + (workspace / "data" / "kernel").mkdir(parents=True, exist_ok=True) + (workspace / "data" / "kernel" / "default.json").write_text("{}", encoding="utf-8") + (workspace / "data" / "lab").mkdir(parents=True, exist_ok=True) + (workspace / "data" / "lab" / "lab.json").write_text("{}", encoding="utf-8") + (workspace / "data" / "ui_session-1.jsonl").write_text("{}\n", encoding="utf-8") + + appdata.migrate_legacy(workspace) + + assert (state_mod.layout_dir() / "p.json").exists() + assert nb_tool._conn_path("default").exists() + assert lab_tool._state_path().exists() + assert (sessions.sessions_dir() / "ui_session-1.jsonl").exists() + + +def test_the_cache_is_actually_relocated(workspace): + """`ensure()` used to create the cache directory, so the "destination + already exists" guard fired on every run and `data/cache` never moved.""" + legacy = workspace / "data" / "cache" + legacy.mkdir(parents=True, exist_ok=True) + (legacy / "abc.json").write_text("{}", encoding="utf-8") + + assert "cache" in appdata.migrate_legacy(workspace) + assert (paths.cache_dir() / "abc.json").exists() + assert not legacy.exists() + + +def test_transcripts_are_migrated_with_everything_else(workspace): + """They sit loose at the top of `data/` rather than in a subdirectory, so a + pass that only walked directories left the whole conversation history + behind -- still on disk, still private, no longer anywhere the app looks.""" + (workspace / "data").mkdir(parents=True, exist_ok=True) + (workspace / "data" / "ui_session-20260815-abcd.jsonl").write_text("{}\n", encoding="utf-8") + (workspace / "data" / "ui_storage_secret").write_text("s3cret", encoding="utf-8") + # Evidence about the research rather than state about the machine: it stays + # with the notebooks it describes. + (workspace / "data" / "nb_verify.json").write_text("{}", encoding="utf-8") + + appdata.migrate_legacy(workspace) + + assert (sessions.sessions_dir() / "ui_session-20260815-abcd.jsonl").exists() + assert not (workspace / "data" / "ui_session-20260815-abcd.jsonl").exists() + assert (appdata.state_dir() / "ui_storage_secret").read_text(encoding="utf-8") == "s3cret" + assert (workspace / "data" / "nb_verify.json").exists() + + +def test_a_locked_file_is_copied_and_left_rather_than_lost(workspace, monkeypatch): + """The normal state when this runs: a Lab server is up and holding its own + log open. A wholesale directory move fails on that handle and its fallback + can delete some sources after copying them and abort on the locked one -- + leaving files neither here nor there. Copy, promote, then delete. + + The lock is simulated rather than taken. Holding a real handle only blocks + the unlink on Windows -- POSIX unlinks open files happily -- so a test built + on one would assert nothing at all on the platform it did not run on. + """ + legacy = workspace / "data" / "lab" + legacy.mkdir(parents=True, exist_ok=True) + (legacy / "lab.json").write_text('{"port": 8889}', encoding="utf-8") + held = legacy / "lab.log" + held.write_text("serving\n", encoding="utf-8") + + real_unlink = Path.unlink + + def refuse(self, *args, **kwargs): + if self.name == "lab.log": + raise PermissionError(32, "in use by another process") + return real_unlink(self, *args, **kwargs) + + monkeypatch.setattr(Path, "unlink", refuse) + + assert "lab" in appdata.migrate_legacy(workspace) + # Everything arrived, including the file that could not be removed. + assert (appdata.state_dir() / "lab" / "lab.json").exists() + assert (appdata.state_dir() / "lab" / "lab.log").read_text(encoding="utf-8") == "serving\n" + # And nothing was destroyed on the way: the locked original survives. + assert held.exists() + + +def test_a_failed_migration_leaves_the_sources_untouched(workspace, monkeypatch): + """Half a migration is worse than none. If the copy cannot finish, the + workspace must look exactly as it did before.""" + legacy = workspace / "data" / "layouts" + legacy.mkdir(parents=True, exist_ok=True) + (legacy / "proj.json").write_text('{"a": 1}', encoding="utf-8") + + def explode(*_args, **_kwargs): + raise OSError("disk full") + + monkeypatch.setattr(appdata.shutil, "copy2", explode) + assert appdata.migrate_legacy(workspace) == [] + assert (legacy / "proj.json").read_text(encoding="utf-8") == '{"a": 1}' + assert not (appdata.workspace_state_dir() / "layouts").exists() + assert not (appdata.workspace_state_dir() / "layouts.incoming").exists() + + +def test_migration_does_not_overwrite_the_live_location(workspace): + """A second run, or a workspace opened after the app has already written + layouts, must not have stale state resurrected over the current state.""" + (appdata.state_dir() / "layouts").mkdir(parents=True, exist_ok=True) + (appdata.state_dir() / "layouts" / "proj.json").write_text('{"live": 1}', encoding="utf-8") + legacy = workspace / "data" / "layouts" + legacy.mkdir(parents=True, exist_ok=True) + (legacy / "proj.json").write_text('{"stale": 1}', encoding="utf-8") + + appdata.migrate_legacy(workspace) + + assert "live" in (appdata.state_dir() / "layouts" / "proj.json").read_text(encoding="utf-8") + + +# --------------------------------------------------------------------------- +# one instance +# --------------------------------------------------------------------------- +def test_a_second_instance_cannot_take_the_lock(workspace): + """Two workspaces would fight over the layout file, the transcript directory + and Lab's recorded origin -- and the second would bind a different port, + which is exactly the mismatch that stops Lab embedding.""" + first = instance._Lock() + assert first.acquire() is True + second = instance._Lock() + try: + assert second.acquire() is False + finally: + first.release() + + +def test_the_lock_is_released_for_the_next_launch(workspace): + """Held by the OS, not by a pid file: a crash must not leave an app that + refuses to start until someone deletes a file.""" + first = instance._Lock() + assert first.acquire() is True + first.release() + second = instance._Lock() + assert second.acquire() is True + second.release() + + +def test_the_published_port_survives_for_the_handover(workspace): + """The app takes the first free port at or above 8080, so a second launch + cannot guess where the first one is listening.""" + instance.publish(8123) + assert instance.read_state()["port"] == 8123 + instance.clear() + assert instance.read_state() == {} + + +def test_a_missing_state_file_is_not_an_error(workspace): + assert instance.read_state() == {} + assert instance.show_running({}) is False + + +# --------------------------------------------------------------------------- +# ports and origins +# --------------------------------------------------------------------------- +def test_an_explicit_port_is_honoured_even_if_it_looks_busy(): + """`--port` is someone overriding the picker on purpose.""" + assert desktop.choose_port(9321) == 9321 + + +def test_the_chosen_port_walks_up_from_the_default(): + chosen = desktop.choose_port(None) + assert desktop.DEFAULT_PORT <= chosen < desktop.DEFAULT_PORT + desktop.PORT_SPAN + + +def test_a_lab_on_another_port_is_a_mismatch(monkeypatch): + """Lab bakes `frame-ancestors` at launch, so an app that has moved ports + cannot embed it -- and the browser calls that "refused to connect".""" + monkeypatch.setattr(models, "app_port", lambda: 8081) + assert models.origin_mismatch({"running": True, "ui_origin": "http://127.0.0.1:8080"}) is True + + +def test_the_localhost_alias_is_not_a_mismatch(monkeypatch): + """The Jupyter config allows both spellings on purpose, because a browser + treats them as different origins and which one the window opened on is not + Lab's business. Flagging it would put a banner on a working server.""" + monkeypatch.setattr(models, "app_port", lambda: 8080) + assert models.origin_mismatch({"running": True, "ui_origin": "http://localhost:8080"}) is False + assert models.origin_mismatch({"running": True, "ui_origin": "http://127.0.0.1:8080"}) is False + + +def test_a_stopped_lab_is_never_a_mismatch(monkeypatch): + """There is a different, better message for "not running"; two banners for + one condition is worse than one.""" + monkeypatch.setattr(models, "app_port", lambda: 9999) + assert models.origin_mismatch({"running": False, "ui_origin": "http://127.0.0.1:8080"}) is False + + +# --------------------------------------------------------------------------- +# quitting while work is in flight +# --------------------------------------------------------------------------- +def test_nothing_running_is_not_busy(workspace, monkeypatch): + monkeypatch.setattr(desktop, "_lab_busy", lambda: []) + report = desktop.busy_report() + assert report["busy"] is False + assert desktop.busy_sentence(report) == "Nothing is running." + + +def test_a_running_command_makes_quitting_a_question(workspace, monkeypatch): + """`nb verify` spawns a kernel detached and can hold a GPU for half an hour. + Losing it to a stray click on Quit is a real cost.""" + monkeypatch.setattr(desktop, "_lab_busy", lambda: []) + + class FakeTask: + label = "verify 03-optimizers.ipynb" + + monkeypatch.setattr("ui.tasks.running", lambda: [FakeTask()]) + report = desktop.busy_report() + assert report["busy"] is True + assert "verify 03-optimizers.ipynb" in desktop.busy_sentence(report) + + +def test_a_busy_lab_kernel_counts_too(workspace, monkeypatch): + """Lab's kernels are invisible to `ui/tasks.py` -- different owner, separate + process -- so they have to be asked about separately.""" + monkeypatch.setattr(desktop, "_lab_busy", lambda: ["python3"]) + monkeypatch.setattr("ui.tasks.running", lambda: []) + report = desktop.busy_report() + assert report["busy"] is True + assert "executing a cell" in desktop.busy_sentence(report) + + +def test_an_unreachable_lab_does_not_block_the_quit(workspace, monkeypatch): + """A wedged server must not hang the prompt that asks about it.""" + monkeypatch.setattr( + "tools.lab.lab_state", lambda: {"running": True, "port": 1, "token": "x"} + ) + assert desktop._lab_busy() == [] + + +# --------------------------------------------------------------------------- +# the poll +# --------------------------------------------------------------------------- +@pytest.mark.asyncio +async def test_the_poll_builds_models_off_the_event_loop(workspace): + """`tools/lab.py:_listening` probes a socket with a 0.4s timeout on this + path, while the notebook window is open. On the loop that is a 400ms freeze + of every window, the chat stream and the pane dragging.""" + space = state_mod.Workspace(_FakeSession(), "proj") + seen: dict[str, int] = {} + + def builder(_workspace): + seen["thread"] = threading.get_ident() + return {"value": 1} + + space.layout = _only("ledger", space) + monkey = dict(state_mod.MODEL_BUILDERS) + monkey["ledger"] = builder + state_mod.MODEL_BUILDERS.update(monkey) + try: + await space.poll() + finally: + state_mod.MODEL_BUILDERS.update({"ledger": lambda w: models.ledger_model()}) + + assert seen["thread"] != threading.get_ident() + + +@pytest.mark.asyncio +async def test_a_slow_poll_does_not_stack(workspace): + """A pass that outruns POLL_SECONDS is a slow disk or a hung port. Queueing + the next one turns one slow poll into an unbounded pile of them.""" + space = state_mod.Workspace(_FakeSession(), "proj") + space._ticking = True + await space.poll() # returns immediately rather than running a pass + assert space.models == {} + + +@pytest.mark.asyncio +async def test_the_loop_bound_window_is_not_offloaded(workspace): + """`tasks` reads the live SDK session rather than a file. There is no I/O to + move, and reading it from a worker thread while the loop mutates it is a + race for no gain.""" + assert "tasks" in state_mod.LOOP_BOUND + + +# --------------------------------------------------------------------------- +# the read-only render +# --------------------------------------------------------------------------- +def _notebook(workspace, name: str = "n.ipynb", source: str = "print(1)"): + import nbformat + + nb = nbformat.v4.new_notebook(cells=[nbformat.v4.new_code_cell(source)]) + paths.notebooks_dir().mkdir(parents=True, exist_ok=True) + target = paths.notebooks_dir() / name + nbformat.write(nb, target) + return target + + +def test_the_render_carries_no_script(workspace): + """It is stored output from a file that may have been cloned rather than + written here, shown in a sandboxed frame. The `basic` template is what makes + that possible; the `lab` one ships the JavaScript that draws it.""" + _notebook(workspace) + body = render.notebook_html("n.ipynb") + # Pygments splits the source across spans, so the cell is asserted by its + # tokens rather than by its text. + assert "print" in body and "highlight" in body + assert " None: + pass + + +def _only(window_id: str, space): + """A layout holding one window, so a poll rebuilds exactly one model.""" + from ui import layout as layout_mod + + return layout_mod.Layout.default([window_id]) diff --git a/tests/test_funnel.py b/tests/test_funnel.py index 52858cf..d1b8306 100644 --- a/tests/test_funnel.py +++ b/tests/test_funnel.py @@ -135,7 +135,8 @@ def test_a_healthy_stage_still_returns_its_payload(monkeypatch): "source, client, expected_fix", [ # Each door's dead end points at the one that is not a dead end. - ("s2", "SemanticScholar", "--tier1 asta"), + ("pwc", "PapersWithCode", "--tier1 asta"), + ("s2", "SemanticScholar", "--tier1 pwc"), ("asta", "Asta", "credential set asta_api_key"), ], ) @@ -191,7 +192,7 @@ def test_an_empty_run_still_writes_the_trace_the_funnel_view_reads( from tools import paper_search # The default source, so this covers the path a real run takes. - monkeypatch.setattr(http, "Asta", lambda cfg: _RateLimited()) + monkeypatch.setattr(http, "PapersWithCode", lambda cfg: _RateLimited()) paper_search.cli.run( ["search", "efficient optimizers", "--no-expand", "--no-triage", "--no-local", "--json"] ) @@ -202,7 +203,7 @@ def test_an_empty_run_still_writes_the_trace_the_funnel_view_reads( written = json.loads(traces[0].read_text(encoding="utf-8")) assert written["stages"]["1_retrieve"] == {"rankings": 0, "candidates": 0} # Which client was asked, so "no results" can be read against who was down. - assert written["stages"]["1_sources"] == ["asta"] + assert written["stages"]["1_sources"] == ["pwc"] assert any("rate-limited" in w for w in written["warnings"]) @@ -311,3 +312,191 @@ def test_the_fix_the_error_prints_is_a_command_that_exists(): assert credentials.CLAUDE_TOKEN in CREDENTIAL_NAMES assert credentials.CLAUDE_TOKEN in credentials.status() assert credentials.CLAUDE_TOKEN in haiku.AUTH_FIX + + +# --------------------------------------------------------------------------- +# stage 1: what a slow tier-1 costs, and what bounds it +# --------------------------------------------------------------------------- +class _Counted: + """A tier-1 client that counts calls and fails a chosen verb. + + `snippet_search` is the one that was down when this was written -- Asta's own + backend refusing a connection -- and the number that matters is not that it + failed but that it took 283 seconds to say so. + """ + + def __init__(self, fails=("snippet_search",), hits=1, seconds=0.0) -> None: + self.fails = set(fails) + self.hits = hits + self.seconds = seconds + self.calls: list[str] = [] + + def _call(self, verb: str, query: str, limit: int = 20): + import time + + self.calls.append(verb) + if self.seconds: + time.sleep(self.seconds) + if verb in self.fails: + raise UpstreamError(f"Asta's {verb} failed: ConnectionRefusedError", fix="retry") + return [ + {"id": f"s2:{query[:4]}-{i}", "paper_id": f"p{i}", "title": f"paper {i}", + "year": 2024, "snippet": "an excerpt", "source": "asta.paper", "external": {}} + for i in range(self.hits) + ] + + def snippet_search(self, query, limit=20): + return self._call("snippet_search", query, limit) + + def paper_search(self, query, limit=20): + return self._call("paper_search", query, limit) + + def neighbours(self, *_a, **_k): + return [] + + +def _expand_to(monkeypatch, queries): + """Stage 0, without Haiku: what matters here is how many queries stage 1 is + handed, because that is the multiplier on every tier-1 failure.""" + from core import haiku + + monkeypatch.setattr( + haiku, "expand", lambda *a, **k: {"queries": list(queries), "hyde": None} + ) + + +def test_a_dead_endpoint_is_dropped_after_one_failure_rather_than_per_query( + workspace, capsys, monkeypatch +): + """The run-killer. Stage 0 turns one question into six queries, and asking a + down endpoint once per query cost 283 seconds each time to learn the same + thing six times -- so the funnel was killed by its caller before the + endpoint that *was* working could return anything.""" + from core import http + from tools import paper_search + + client = _Counted(fails=("snippet_search",)) + monkeypatch.setattr(http, "PapersWithCode", lambda cfg: client) + _expand_to(monkeypatch, ["q1", "q2", "q3", "q4", "q5", "q6"]) + + code = paper_search.cli.run( + ["search", "efficient optimizers", "--no-triage", "--no-rerank", + "--no-local", "--no-citations", "--json"] + ) + payload = json.loads(capsys.readouterr().out) + + assert code == 0 + assert client.calls.count("snippet_search") == 1, "asked once, not once per query" + assert client.calls.count("paper_search") == 6, "the working endpoint is not punished" + assert payload["data"]["results"], "the run returns what the live endpoint found" + + +def test_what_was_dropped_is_written_into_the_trace(workspace, capsys, monkeypatch): + """A funnel that quietly searched one endpoint of two looks exactly like a + corpus that had little to say.""" + from core import http + from tools import paper_search + + monkeypatch.setattr(http, "PapersWithCode", lambda cfg: _Counted(fails=("snippet_search",))) + _expand_to(monkeypatch, ["q1", "q2"]) + paper_search.cli.run( + ["search", "efficient optimizers", "--no-triage", "--no-rerank", + "--no-local", "--no-citations", "--json"] + ) + capsys.readouterr() + + written = json.loads(next((paths.notes_dir() / "funnel").glob("*.json")).read_text("utf-8")) + assert written["stages"]["1_discovery"]["dropped"] == ["pwc.snippet_search"] + assert written["stages"]["1_discovery"]["queries_searched"] == 2 + assert any("dropped for the rest of this run" in w for w in written["warnings"]) + + +def test_stage_one_stops_when_its_wall_clock_is_spent_and_says_how_far_it_got( + workspace, capsys, monkeypatch +): + """`core/http.py` bounds one request; this bounds the run. Six queries over + two endpoints under a five-minute request deadline is a one-hour stage, and + the caller kills it long before it ends.""" + from core import http + from tools import paper_search + + config = paths.root() / "config" / "grad.toml" + config.parent.mkdir(parents=True, exist_ok=True) + config.write_text("[retrieval]\nstage1_budget_s = 0.05\n", encoding="utf-8") + + client = _Counted(fails=(), seconds=0.03) + monkeypatch.setattr(http, "PapersWithCode", lambda cfg: client) + _expand_to(monkeypatch, [f"q{i}" for i in range(20)]) + + code = paper_search.cli.run( + ["search", "efficient optimizers", "--no-triage", "--no-rerank", + "--no-local", "--no-citations", "--json"] + ) + payload = json.loads(capsys.readouterr().out) + + assert code == 0, "stopping early is not failing" + assert payload["data"]["results"], "what was retrieved is kept" + written = json.loads(next((paths.notes_dir() / "funnel").glob("*.json")).read_text("utf-8")) + searched = written["stages"]["1_discovery"]["queries_searched"] + assert 0 < searched < 20 + assert any("stage1_budget_s" in w for w in written["warnings"]) + + +def test_progress_reaches_stderr_so_a_pipe_can_see_the_run_moving( + workspace, capsys, monkeypatch +): + """Everything that runs this reads a pipe -- the tasks window streams the + tail, and the agent's own Bash gives up at 120s and backgrounds the command. + A run that prints nothing until its envelope is indistinguishable from a + hung one in both.""" + from core import http + from tools import paper_search + + monkeypatch.setattr(http, "PapersWithCode", lambda cfg: _Counted(fails=())) + _expand_to(monkeypatch, ["q1"]) + paper_search.cli.run( + ["search", "efficient optimizers", "--no-triage", "--no-rerank", + "--no-local", "--no-citations", "--json"] + ) + captured = capsys.readouterr() + assert "stage 1: pwc.paper_search" in captured.err + assert "stage 1" not in captured.out, "the --json contract stays one object on stdout" + + +def test_the_advice_matches_what_actually_failed(workspace): + """A live run failed with `ConnectionRefusedError` raised inside Asta's own + backend, and the fix said "discovery is rate limited" and pointed at a key. + No key affects that, and advice that cannot be followed is followed first.""" + from tools import paper_search + + limited = paper_search._tier1_fix(["asta"], ["asta.snippet_search: rate-limited"]) + assert "credential set asta_api_key" in limited + + refused = paper_search._tier1_fix( + ["pwc"], ["pwc.paper_search: ConnectionRefusedError inside the service"] + ) + assert "credential set" not in refused + assert "--local-only" in refused + + +def test_one_endpoint_being_down_does_not_hide_what_the_other_found( + workspace, capsys, monkeypatch +): + """The state of the service as this was written: `snippet_search` refuses + and `search_papers_by_relevance` works. A funnel that reports "every + retrieval call failed" in that situation is claiming the literature has + nothing on the question.""" + from core import http + from tools import paper_search + + monkeypatch.setattr(http, "PapersWithCode", lambda cfg: _Counted(fails=("snippet_search",), hits=3)) + _expand_to(monkeypatch, ["q1", "q2"]) + code = paper_search.cli.run( + ["search", "efficient optimizers", "--no-triage", "--no-rerank", + "--no-local", "--no-citations", "--json"] + ) + payload = json.loads(capsys.readouterr().out) + assert code == 0 + assert payload["ok"] is True + assert len(payload["data"]["results"]) == 6 + assert any("snippet_search" in w for w in payload["data"]["trace"]["warnings"]) diff --git a/tests/test_lab_and_wiki.py b/tests/test_lab_and_wiki.py index 701a6e1..9c81e66 100644 --- a/tests/test_lab_and_wiki.py +++ b/tests/test_lab_and_wiki.py @@ -16,7 +16,7 @@ import pytest -from core import jsonl, paths +from core import appdata, jsonl, paths from core.errors import ConfigError, GradError, UsageError from tools import lab, wiki @@ -66,7 +66,7 @@ def test_status_reports_not_running_before_a_start(workspace): def test_the_token_is_not_in_the_status_payload(workspace): """A status output is the sort of thing that ends up in a screenshot.""" jsonl.write_json( - paths.data_dir() / "lab" / "lab.json", + appdata.state_dir() / "lab" / "lab.json", {"port": 8889, "token": "super-secret", "pid": 1, "url": "http://127.0.0.1:8889/lab"}, ) payload = lab.cmd_status(argparse.Namespace(json=True)) @@ -77,7 +77,7 @@ def test_the_token_is_not_in_the_status_payload(workspace): def test_url_includes_the_token_because_the_iframe_needs_it(workspace): jsonl.write_json( - paths.data_dir() / "lab" / "lab.json", + appdata.state_dir() / "lab" / "lab.json", {"port": 8889, "token": "tok123", "pid": 1}, ) result = lab.cmd_url(argparse.Namespace(path="notebooks/a.ipynb", json=True)) @@ -223,8 +223,22 @@ def _server_app(monkeypatch, origin: str): source = (_repo_root() / "config" / "jupyter" / "jupyter_server_config.py").read_text( encoding="utf-8" ) - config = type("Config", (), {})() - config.ServerApp = type("ServerApp", (), {})() + + class _Section: + """`get_config()` returns a traitlets `Config`, which creates a section + the first time one is touched -- `c.ServerApp.ip = ...` needs no + declaration, and neither does `c.LanguageServerManager.node_roots`. A + stub that predeclares the sections it happens to know about turns a new + setting in the real config into an AttributeError here, which says + nothing about the setting and everything about the stub. + """ + + def __getattr__(self, name: str): + section = _Section() + setattr(self, name, section) # traitlets caches it; so does this + return section + + config = _Section() namespace: dict = {"get_config": lambda: config} monkeypatch.setenv("GRAD_UI_ORIGIN", origin) exec(compile(source, "jupyter_server_config.py", "exec"), namespace) @@ -299,3 +313,205 @@ def fake_run(argv, **kw): assert result["html"].endswith("index.html") html = (wiki.output_dir() / "index.html").read_text(encoding="utf-8") assert "core/budget.py" in html + + +# --------------------------------------------------------------------------- +# the app ships the server it embeds +# --------------------------------------------------------------------------- +def test_the_desktop_app_brings_the_lab_server_with_it(workspace): + """The notebook window's interior *is* Lab, so an app installed without a + Lab server ships a window whose only content is a button that fails. That + was the state: `lab` was an extra nobody's install line mentioned.""" + import tomllib + + doc = tomllib.loads((_repo_root() / "pyproject.toml").read_text(encoding="utf-8")) + extras = doc["project"]["optional-dependencies"] + assert "grad[lab]" in extras["ui"] + assert any(p.startswith("jupyterlab==") for p in extras["lab"]) + + +def test_the_extension_set_stays_opt_in(workspace): + """Heavier than everything else in the file put together, and a preference + rather than a requirement -- so it is a second extra rather than a reason + to make the first one enormous.""" + import tomllib + + doc = tomllib.loads((_repo_root() / "pyproject.toml").read_text(encoding="utf-8")) + extras = doc["project"]["optional-dependencies"] + assert "grad[lab-extensions]" not in extras["ui"] + for pin in extras["lab-extensions"]: + assert "==" in pin, f"{pin} is not pinned exactly" + + +def test_the_missing_jupyter_message_names_an_install_that_provides_it(workspace, monkeypatch): + from core.errors import ConfigError + + monkeypatch.setattr(lab.shutil, "which", lambda name: None) + with pytest.raises(ConfigError) as exc: + lab._executable() + assert "[ui]" in (exc.value.fix or "") + + +# --------------------------------------------------------------------------- +# the origin the iframe is framed from +# --------------------------------------------------------------------------- +def test_the_websocket_accepts_both_spellings_of_the_loopback_host(workspace, monkeypatch): + """`allow_origin` takes exactly one origin, and `127.0.0.1:8080` and + `localhost:8080` are different origins to a browser. Naming only one is the + confusing half of the failure: the page renders, the frame loads, and only + the kernel connection dies.""" + import re + + app = _server_app(monkeypatch, "http://127.0.0.1:8080") + pattern = app.allow_origin_pat + assert re.fullmatch(pattern, "http://127.0.0.1:8080") + assert re.fullmatch(pattern, "http://localhost:8080") + assert not re.fullmatch(pattern, "http://evil.example") + assert getattr(app, "allow_origin", "") != "*" + + +def test_a_server_running_on_the_wrong_origin_is_restarted(workspace, monkeypatch): + """Framing headers are fixed at launch, and a blocked frame is reported by + the browser as "127.0.0.1 refused to connect" -- which reads as a dead port + and sends you looking for a server that is running perfectly well.""" + from core import jsonl + + state = {"port": 8889, "pid": 4242, "ui_origin": "http://127.0.0.1:8080", "token": "t"} + jsonl.write_json(lab._state_path(), state) + monkeypatch.setattr(lab, "_listening", lambda port: True) + monkeypatch.setattr(lab, "_alive", lambda pid: True) + + class Restarted(Exception): + """A sentinel, so "it got as far as launching" is an assertion rather + than a mock of the launch itself.""" + + stopped: list[bool] = [] + monkeypatch.setattr(lab, "cmd_stop", lambda _: stopped.append(True)) + monkeypatch.setattr(lab, "_executable", lambda: (_ for _ in ()).throw(Restarted())) + + # Same origin: left alone, which is the one case worth not restarting. + same = lab.cmd_start(_start_namespace("http://127.0.0.1:8080")) + assert same["already_running"] is True + assert stopped == [] + + # Different origin: the server has to come down for the header to change. + with pytest.raises(Restarted): + lab.cmd_start(_start_namespace("http://127.0.0.1:9000")) + assert stopped == [True] + + +def _start_namespace(origin: str): + import argparse + + return argparse.Namespace(port=lab.DEFAULT_PORT, ui_origin=origin, force=False) + + +def test_the_window_asks_the_page_which_origin_it_is_on(workspace): + """`--port` moves the app, and pywebview may open `localhost` where the + config assumed `127.0.0.1`. The page knows the answer to both; guessing + covers neither.""" + import asyncio + + pytest.importorskip("nicegui", reason="the ui extra is not installed") + from ui.windows import notebook as notebook_window + + class Page: + @staticmethod + async def run_javascript(code, timeout=None): + assert "location.origin" in code + return "http://localhost:8099" + + assert asyncio.run(notebook_window._origin(Page)) == "http://localhost:8099" + + +def test_a_page_that_cannot_answer_falls_back_to_the_port_the_app_bound(workspace, monkeypatch): + import asyncio + + pytest.importorskip("nicegui", reason="the ui extra is not installed") + from ui import app as app_mod + from ui.windows import notebook as notebook_window + + class Gone: + @staticmethod + async def run_javascript(code, timeout=None): + raise RuntimeError("the client disconnected") + + monkeypatch.setattr(app_mod, "PORT", 9123) + assert asyncio.run(notebook_window._origin(Gone)) == "http://127.0.0.1:9123" + + +# --------------------------------------------------------------------------- +# no console windows +# --------------------------------------------------------------------------- +def test_a_long_lived_child_keeps_a_console_so_its_own_children_stay_quiet(workspace): + """`DETACHED_PROCESS` gives a child no console, so the first console program + *it* starts is given a fresh -- visible -- one. That is where the `npm + prefix` window came from: jupyter-lsp probing for language servers under a + Lab server we had started detached. + + `CREATE_NO_WINDOW` is the fix and is also mutually exclusive with + `DETACHED_PROCESS` (both together is ERROR_INVALID_PARAMETER, not + redundancy), so this asserts the swap rather than the coexistence. + """ + import subprocess + + from core import spawn + + if not spawn.WINDOWS: + assert spawn.detached() == {"start_new_session": True} + return + + flags = spawn.detached()["creationflags"] + assert flags & subprocess.CREATE_NO_WINDOW + assert not flags & subprocess.DETACHED_PROCESS + # The half of the promise the console was never carrying: a Ctrl+C to our + # group must not reach it. + assert flags & subprocess.CREATE_NEW_PROCESS_GROUP + + +def test_the_verify_kernel_is_spawned_through_the_same_one_definition(workspace): + """`tools/nb.py` had its own hand-rolled copy of the flags, which meant its + detached kernels had the same invisible-grandchild problem.""" + source = __import__("inspect").getsource(__import__("tools.nb", fromlist=["nb"])) + assert "**spawn.detached()" in source + assert "DETACHED_PROCESS" not in source + + +def test_the_lab_server_is_started_without_a_console(workspace, monkeypatch): + """Every button in the workspace runs a CLI, and `ui.run(native=True)` is a + GUI process with no console to lend -- so Windows gave each child a fresh + one, which is a black window over the workspace.""" + from core import spawn + + seen: dict = {} + + class Fake: + returncode = None + pid = 999 + + def poll(self): + return None + + def fake_popen(argv, **kwargs): + seen.update(kwargs) + return Fake() + + monkeypatch.setattr(lab, "_executable", lambda: "jupyter") + monkeypatch.setattr(lab, "_free_port", lambda preferred: preferred) + monkeypatch.setattr(lab, "_listening", lambda port: True) + monkeypatch.setattr(lab.subprocess, "Popen", fake_popen) + lab.cmd_start(_start_namespace("http://127.0.0.1:8080")) + + for key, value in spawn.detached().items(): + assert seen.get(key) == value + + +def test_the_workspaces_own_commands_are_started_without_a_console(workspace): + """`tasklist` for the liveness check and the CLI itself were two windows + per Lab start.""" + from core import spawn + from ui import tasks as tasks_mod + + source = __import__("inspect").getsource(tasks_mod) + assert source.count("**spawn.quiet()") >= 2 + assert spawn.quiet() == ({} if not spawn.WINDOWS else {"creationflags": spawn.NO_WINDOW}) diff --git a/tests/test_pwc.py b/tests/test_pwc.py new file mode 100644 index 0000000..7b5f4d3 --- /dev/null +++ b/tests/test_pwc.py @@ -0,0 +1,301 @@ +"""Papers with Code, and the arXiv batch that makes its rows readable. + +This replaced Asta as the funnel's default tier 1 for one reason, and it is a +measured one rather than a preference: Asta answers a search in ~121 seconds and +takes ~283 seconds to report that its own backend refused a connection, and +stage 0 multiplies that by six queries and two endpoints. Every caller gives up +first. This answers in one to two seconds. + +What it costs is the thing to keep honest, and most of what is asserted below is +about that: search rows carry **no abstract**, so `arxiv_abstracts` fills the +pool in one request, and `related` is a *dense neighbour* rather than a citation +edge and must not be presented as one. + +No network: the transport is faked, the same way `tests/test_asta.py` fakes it. +The shapes here are not guesses -- they are what the live API returned. +""" + +from __future__ import annotations + +import json + +import pytest + +from core import config as config_mod, http +from core.errors import UpstreamError + + +class FakeResponse: + def __init__(self, status_code=200, payload=None, text=""): + self.status_code = status_code + self._payload = payload + self.text = text or (json.dumps(payload) if payload is not None else "") + + def json(self): + if self._payload is None: + raise ValueError("not json") + return self._payload + + def raise_for_status(self): + if self.status_code >= 400: + raise RuntimeError(f"status {self.status_code}") + + +#: One row exactly as `papers/search` returned it live. Note what is *not* here. +LIVE_ROW = { + "id": "93318", + "title": "Towards Efficient Optimizer Design for LLM via Structured Fisher Approximation", + "arxiv_id": "2502.07752", + "source": "arxiv", + "authors": ["Wenbo Gong", "Meyer Scetbon"], + "published": "2025-02-11", + "url_abs": "https://arxiv.org/abs/2502.07752", + "citation_count": 6, +} + + +@pytest.fixture +def transport(monkeypatch): + """A fake catalogue. `queue` answers each GET in order.""" + gets: list[dict] = [] + queue: list[FakeResponse] = [] + + class FakeHttpx: + @staticmethod + def get(url, params=None, headers=None, timeout=None): + gets.append({"url": url, "params": params or {}, "headers": headers}) + return queue.pop(0) if queue else FakeResponse(payload={"results": []}) + + monkeypatch.setattr(http, "_httpx", lambda: FakeHttpx) + return gets, queue + + +def client(): + return http.PapersWithCode(config_mod.load(reload=True)) + + +# --------------------------------------------------------------------------- +# the two rankings +# --------------------------------------------------------------------------- +def test_the_two_verbs_are_two_genuinely_different_rankings(workspace, transport): + """Lexical and dense, which is the pair `corpus.rrf` exists to fuse -- so the + two calls the funnel already makes per query map onto them without the + funnel knowing anything changed.""" + gets, queue = transport + queue.append(FakeResponse(payload={"results": [LIVE_ROW]})) + queue.append(FakeResponse(payload={"results": [LIVE_ROW]})) + + client().snippet_search("efficient optimizers") + client().paper_search("efficient optimizers") + assert gets[0]["params"]["mode"] == "semantic" + assert gets[1]["params"]["mode"] == "keyword" + + +def test_a_row_arrives_in_the_vocabulary_the_funnel_already_fuses(workspace, transport): + _, queue = transport + queue.append(FakeResponse(payload={"results": [LIVE_ROW]})) + row = client().paper_search("efficient optimizers")[0] + + shared = {"id", "paper_id", "title", "year", "snippet", "abstract", "source", "external"} + assert shared <= set(row) + assert row["id"] == "pwc:2502.07752" + assert row["paper_id"] == "2502.07752" + assert row["year"] == "2025" + assert row["external"]["ArXiv"] == "2502.07752" + # The honest part: the catalogue's search does not return text. + assert row["snippet"] == "" + assert row["abstract"] == "" + + +def test_this_catalogue_does_not_share_the_semantic_scholar_namespace(workspace, transport): + """Asta and S2 share `s2:` because they are one index with one set of ids. + This is a different catalogue with its own numbering, and giving it the same + prefix would fuse two unrelated papers whose ids happened to collide.""" + _, queue = transport + queue.append(FakeResponse(payload={"results": [{"id": "991", "title": "Attention"}]})) + assert client().paper_search("attention")[0]["id"] == "pwc:991" + + +def test_a_page_larger_than_the_api_allows_is_clamped(workspace, transport): + gets, queue = transport + queue.append(FakeResponse(payload={"results": []})) + client().paper_search("attention", limit=500) + assert gets[0]["params"]["page_size"] == http.PapersWithCode.MAX_PAGE + + +# --------------------------------------------------------------------------- +# expansion, and what it is not +# --------------------------------------------------------------------------- +def test_related_work_is_reported_as_a_dense_neighbour_not_a_citation(workspace, transport): + """The rows carry `provenance: "dense"` and a similarity score: they are + nearest neighbours in an embedding space, not papers that cite the seed. + A ledger entry's basis is the thing that must not be overstated.""" + _, queue = transport + queue.append(FakeResponse(payload=[{**LIVE_ROW, "provenance": "dense", "similarity": 0.86}])) + row = client().neighbours("1706.03762")[0] + assert row["source"] == "pwc.related" + assert "citation" not in row["source"] + + +def test_the_backward_direction_is_refused_rather_than_faked(workspace, transport): + gets, _ = transport + assert client().neighbours("1706.03762", direction="references") == [] + assert gets == [], "no request should have been made at all" + + +def test_related_answers_with_a_bare_list_and_search_with_an_envelope(workspace, transport): + """Both are what the live API returns, and both have to be read.""" + _, queue = transport + queue.append(FakeResponse(payload=[LIVE_ROW])) + assert len(client().neighbours("1706.03762")) == 1 + + +def test_an_unrecognised_shape_raises_rather_than_returning_nothing(workspace, transport): + """`ok: true` with no results reads as "the literature has nothing on this", + which is the one conclusion a schema change must not manufacture.""" + _, queue = transport + queue.append(FakeResponse(payload={"papers_maybe": []})) + with pytest.raises(UpstreamError) as exc: + client().paper_search("attention") + assert "pwc-cli" in (exc.value.fix or "") + + +def test_rate_limiting_says_there_is_no_key_to_add(workspace, transport): + _, queue = transport + queue.append(FakeResponse(status_code=429, text="slow down")) + with pytest.raises(UpstreamError) as exc: + client().paper_search("attention") + assert "anonymous" in (exc.value.fix or "") + + +def test_a_moved_endpoint_names_the_config_key_that_moves_with_it(workspace, transport): + _, queue = transport + queue.append(FakeResponse(status_code=404, text="gone")) + with pytest.raises(UpstreamError) as exc: + client().paper_search("attention") + assert "pwc_base" in (exc.value.fix or "") + + +# --------------------------------------------------------------------------- +# the abstracts +# --------------------------------------------------------------------------- +ATOM = """ + + + http://arxiv.org/abs/2211.09760v1 + VeLO + While deep learning models have replaced + hand-designed features, these models are still trained. + + + http://arxiv.org/abs/1706.03762v7 + Attention Is All You Need + The dominant sequence transduction models. + +""" + + +def test_a_hundred_abstracts_cost_one_request(workspace, transport): + """The whole reason to use `id_list`: fetching them one at a time would cost + a request per candidate and undo the reason the corpus was changed.""" + gets, queue = transport + queue.append(FakeResponse(text=ATOM)) + out = http.arxiv_abstracts( + ["2211.09760", "1706.03762"], cfg=config_mod.load(reload=True) + ) + assert len(gets) == 1 + assert gets[0]["params"]["id_list"] == "2211.09760,1706.03762" + assert out["2211.09760"].startswith("While deep learning models") + # Flattened: the feed wraps them, and a reranker reading newlines as + # structure would be reading the feed's formatting. + assert "\n" not in out["2211.09760"] + + +def test_the_version_suffix_is_stripped_so_the_lookup_matches(workspace, transport): + """The feed answers `.../abs/1706.03762v7` and the caller asked for + `1706.03762`, which is what its candidates are keyed by.""" + _, queue = transport + queue.append(FakeResponse(text=ATOM)) + out = http.arxiv_abstracts(["1706.03762"], cfg=config_mod.load(reload=True)) + assert "1706.03762" in out + + +def test_a_failed_fetch_degrades_the_ranking_rather_than_the_run(workspace, transport): + """A candidate with no abstract ranks on its title, which is what would have + happened without this. What it must not do is raise.""" + _, queue = transport + queue.append(FakeResponse(status_code=503, text="down")) + assert http.arxiv_abstracts(["1706.03762"], cfg=config_mod.load(reload=True)) == {} + + +def test_nothing_to_fetch_is_not_a_request(workspace, transport): + gets, _ = transport + assert http.arxiv_abstracts([], cfg=config_mod.load(reload=True)) == {} + assert http.arxiv_abstracts(["", None], cfg=config_mod.load(reload=True)) == {} + assert gets == [] + + +# --------------------------------------------------------------------------- +# the funnel, over the whole of it +# --------------------------------------------------------------------------- +def test_the_pool_is_enriched_before_it_is_reranked(workspace, capsys, monkeypatch): + """Stage 2 reranks on `title + snippet-or-abstract` and stage 3 triages on + the same, so a pool of bare titles is a measurably worse funnel -- and a + funnel silently reranking titles looks exactly like one reranking + abstracts.""" + from tools import paper_search + + class Catalogue: + def snippet_search(self, query, limit=20): + return [] + + def paper_search(self, query, limit=20): + return [{ + "id": "pwc:2502.07752", "paper_id": "2502.07752", "title": "A paper", + "year": "2025", "snippet": "", "abstract": "", "section": "", + "source": "pwc.keyword", "external": {"ArXiv": "2502.07752"}, + }] + + def neighbours(self, *_a, **_k): + return [] + + monkeypatch.setattr(http, "PapersWithCode", lambda cfg: Catalogue()) + monkeypatch.setattr( + http, "arxiv_abstracts", lambda ids, **_k: {"2502.07752": "the real abstract"} + ) + paper_search.cli.run( + ["search", "optimizers", "--no-expand", "--no-rerank", "--no-triage", + "--no-local", "--no-citations", "--full", "--json"] + ) + payload = json.loads(capsys.readouterr().out) + assert payload["data"]["results"][0]["text"] == "the real abstract" + assert payload["data"]["trace"]["stages"]["1_abstracts"] == {"wanted": 1, "found": 1} + + +def test_candidates_left_without_an_abstract_are_reported(workspace, capsys, monkeypatch): + from tools import paper_search + + class Catalogue: + def snippet_search(self, query, limit=20): + return [] + + def paper_search(self, query, limit=20): + return [{ + "id": "pwc:1", "paper_id": "9999.99999", "title": "A paper", "year": "2025", + "snippet": "", "abstract": "", "section": "", "source": "pwc.keyword", + "external": {"ArXiv": "9999.99999"}, + }] + + def neighbours(self, *_a, **_k): + return [] + + monkeypatch.setattr(http, "PapersWithCode", lambda cfg: Catalogue()) + monkeypatch.setattr(http, "arxiv_abstracts", lambda ids, **_k: {}) + paper_search.cli.run( + ["search", "optimizers", "--no-expand", "--no-rerank", "--no-triage", + "--no-local", "--no-citations", "--json"] + ) + payload = json.loads(capsys.readouterr().out) + assert any( + "title alone" in w for w in payload["data"]["trace"]["warnings"] + ), "a funnel reranking titles must not look like one reranking abstracts" diff --git a/tests/test_streaming.py b/tests/test_streaming.py index ce51260..61d5f06 100644 --- a/tests/test_streaming.py +++ b/tests/test_streaming.py @@ -451,3 +451,128 @@ def test_the_options_actually_ask_for_partial_messages(monkeypatch): options = agent.build_options(config_mod.load()) assert isinstance(options, sdk.ClaudeAgentOptions) assert options.include_partial_messages is True + + +# --------------------------------------------------------------------------- +# the reasoning (`TurnStream`, the third kind of block) +# --------------------------------------------------------------------------- +def thinking(text: str) -> FakeStreamEvent: + return FakeStreamEvent( + {"type": "content_block_delta", "delta": {"type": "thinking_delta", "thinking": text}} + ) + + +def test_the_reasoning_is_a_block_of_its_own_and_not_part_of_the_answer(): + """The chat window's statusline switches this on and off, so it has to be + separable. `text` is what the agent *said*; the working is a different + claim and the transcript records it as one.""" + stream = agent.TurnStream() + stream.feed(thinking("the ceiling is the binding constraint")) + stream.feed(delta("Raise the ceiling.")) + stream.feed(FakeMessage([ + ThinkingBlock("the ceiling is the binding constraint"), + FakeBlock("Raise the ceiling."), + ])) + assert kinds(stream) == ["thinking", "text"] + assert stream.text == "Raise the ceiling." + assert stream.thinking == "the ceiling is the binding constraint" + + +def test_reasoning_is_not_printed_by_the_command_line(): + """`feed` returns what a CLI prints. The reasoning is a block a UI may draw, + not something the terminal session starts emitting.""" + stream = agent.TurnStream() + printed = stream.feed(thinking("hmm")) + stream.feed(delta("Yes.")) + assert printed == "Yes." + + +def test_a_finished_thinking_block_does_not_repeat_the_deltas_that_built_it(): + """The same trap `TextStream` exists for, one channel over: the SDK sends + `thinking_delta` events *and* the `ThinkingBlock` containing all of them.""" + stream = agent.TurnStream() + stream.feed(thinking("first ")) + stream.feed(thinking("second")) + stream.feed(FakeMessage([ThinkingBlock("first second")])) + assert stream.thinking == "first second" + assert kinds(stream) == ["thinking"] + + +def test_reasoning_resumes_below_a_call_rather_than_above_it(): + """Interleaved thinking: the order is the information here too. Reasoning + that happened *after* a command must not be appended to the block that was + open before it ran.""" + stream = agent.TurnStream() + stream.feed(FakeMessage([ThinkingBlock("check the ledger first")])) + stream.feed(FakeMessage([ToolUse("tu_1", "Bash", {"command": "ls"})])) + stream.feed(FakeMessage([ToolResult("tu_1", "one")])) + stream.feed(FakeMessage([ThinkingBlock("one entry, so the claim holds")])) + assert kinds(stream) == ["thinking", "tool", "thinking"] + assert [b["text"] for b in stream.blocks if b["kind"] == "thinking"] == [ + "check the ledger first", + "one entry, so the claim holds", + ] + + +def test_a_turn_that_only_reasoned_still_settles_as_something(): + """`Session.ask` keeps a turn when it produced blocks. Reasoning is blocks, + so an interrupted turn that had only got as far as thinking is not recorded + as a prompt that went unanswered.""" + stream = agent.TurnStream() + stream.feed(thinking("still working out what to run")) + assert stream.text == "" + assert stream.blocks and stream.blocks[0]["kind"] == "thinking" + + +def test_a_call_is_stamped_so_the_tasks_window_can_age_it(): + """Wall clock, not monotonic: it goes into the session file and is read back + in another process, where a monotonic reading means nothing.""" + import time + + stream = agent.TurnStream() + stream.feed(FakeMessage([ToolUse("tu_1", "Bash", {"command": "sleep 60"})])) + started = stream.blocks[0]["started"] + assert isinstance(started, float) + assert abs(started - time.time()) < 60 + + +def test_the_reasoning_is_asked_for_as_text_rather_than_assumed(monkeypatch): + """Capturing thinking blocks is not enough to have any: Opus 4.7+ defaults + `display` to "omitted" and sends them with a signature and no text. The chat + window's reasoning switch had nothing to reveal no matter how correctly the + stream was read -- one flag away from the feature, and indistinguishable + from a toggle that does nothing.""" + 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(reload=True)) + assert options.thinking == {"type": "adaptive", "display": "summarized"} + + +def test_reasoning_can_be_turned_off_from_the_config(workspace, monkeypatch): + sdk = pytest.importorskip("claude_agent_sdk", reason="the SDK is not installed") + from core import config as config_mod, paths + + config = paths.root() / "config" / "grad.toml" + config.parent.mkdir(parents=True, exist_ok=True) + config.write_text('[agent]\nreasoning = "omitted"\n', encoding="utf-8") + monkeypatch.setattr(agent, "system_prompt", lambda: "prompt") + options = agent.build_options(config_mod.load(reload=True)) + assert options.thinking == {"type": "adaptive", "display": "omitted"} + + +def test_an_sdk_without_the_option_still_builds_a_session(monkeypatch): + """Feature-detected rather than assumed, for the same reason the deny probe + exists: this option is newer than the permission mode, and the SDK's shape + has changed between releases.""" + import dataclasses + + from core import config as config_mod + + class OldOptions: + pass + + class OldSdk: + ClaudeAgentOptions = dataclasses.make_dataclass("ClaudeAgentOptions", ["model"]) + + assert agent.thinking_option(config_mod.load(reload=True), OldSdk) == {} diff --git a/tests/test_ui_shell.py b/tests/test_ui_shell.py index 936693f..9f3f500 100644 --- a/tests/test_ui_shell.py +++ b/tests/test_ui_shell.py @@ -91,9 +91,18 @@ def html_of(client: Client) -> str: @pytest.mark.parametrize("window", registry.ids()) def test_every_window_renders_on_an_empty_workspace(rendered, window): """The empty state is the state a new user sees, so it is the one most - worth proving renders at all.""" + worth proving renders at all. + + The element count is not enough on its own and never was: `shell._render` + turns a window that raises into a card saying so, deliberately, so that ten + working windows and one broken one is still a usable workspace -- and that + card is elements too. A `_Statusline` method that got dedented out of its + class took the whole chat window down, and this test watched it happen and + passed, because a failure card is bigger than ten elements. + """ client, _ = rendered([window]) assert len(client.elements) > 10 + assert "failed to render" not in html_of(client) def test_all_eleven_render_together(rendered): @@ -103,6 +112,7 @@ def test_all_eleven_render_together(rendered): assert "grad-shell" in markup assert "grad-tiles" in markup assert "grad-statusbar" in markup + assert "failed to render" not in markup def test_a_window_whose_render_raises_does_not_take_the_shell_down(rendered, monkeypatch): @@ -427,14 +437,103 @@ def test_a_new_turn_clears_the_tail_rather_than_stacking_onto_it(rendered): def test_the_status_line_names_the_call_in_flight(rendered): """A spinner says something is happening; naming the command says a - 40-minute job is running and which one.""" + 40-minute job is running and which one. + + The fallbacks changed with the statusline: `running …` was the only thing it + could say when no call was in flight, which covered "reasoning", "writing" + and "nothing has come back yet" with one word. Each is now named, because + each is a different answer to "why is nothing on screen". + """ from ui.windows.chat import _activity - assert _activity([]) == "running …" + assert _activity([]) == "waiting for the model" assert _activity([ {"kind": "tool", "name": "Bash", "title": "python -m tools.jobs run", "status": "running"}, ]) == "running Bash python -m tools.jobs run" - assert _activity([{"kind": "tool", "name": "Bash", "title": "ls", "status": "ok"}]) == "running …" + # A finished call is not an activity; what is happening is whatever came + # after it, and the reasoning is as specific as that gets. + assert _activity([ + {"kind": "tool", "name": "Bash", "title": "ls", "status": "ok"}, + {"kind": "thinking", "text": "one entry, so the claim holds"}, + ]) == "thinking" + assert _activity([ + {"kind": "thinking", "text": "working it out"}, + {"kind": "text", "text": "The answer is"}, + ]) == "writing" + + +def test_the_statusline_switches_the_reasoning_without_redrawing_the_transcript(rendered): + """A toggle that rebuilt the transcript would take its scroll position with + it, which is the same reason the poll never touches this window. So the + blocks are always in the DOM and a class decides whether they are painted. + + Clicked rather than called: the first version of this test drove + `Workspace.toggle_reasoning` and wrote the class by hand, which exercised + everything except the handler on the bar -- and the handler was the half + that was broken. + """ + client, space = rendered(["chat"]) + with client: + assert space.show_reasoning is False + roots = [ + e for e in client.elements.values() if "grad-chat" in getattr(e, "classes", []) + ] + assert roots, "the chat root carries the class the switch writes" + assert "reasoning-on" not in roots[0].classes + + bars = [ + e for e in client.elements.values() + if "grad-statusline" in getattr(e, "classes", []) + ] + assert len(bars) == 1 + + click(bars[0]) + assert space.show_reasoning is True + assert "reasoning-on" in roots[0].classes + # Said once, at the click: switching on something that reveals nothing + # is indistinguishable from a switch that does not work. + assert "no reasoning in this session yet" in (space.notice or "") + + click(bars[0]) + assert space.show_reasoning is False + assert "reasoning-on" not in roots[0].classes + + +def test_the_statusline_reports_the_call_in_flight_as_the_turn_moves(rendered): + """`sync` is the method the flush timer drives at 15 Hz, and it was the + other half dedented out of the class.""" + client, space = rendered(["chat"]) + with client: + bar = next( + e for e in client.elements.values() + if "grad-statusline" in getattr(e, "classes", []) + ) + line = chat_statusline(space, bar) + space.session.busy = True + line.sync([{"kind": "tool", "name": "Bash", "title": "pytest -q", "status": "running"}]) + assert "RUNNING" in html_of(client) + assert "running Bash pytest -q" in html_of(client) + + +def chat_statusline(space, bar): + """The `_Statusline` behind a rendered bar, via the handler bound to it.""" + for listener in bar._event_listeners.values(): # noqa: SLF001 - no public hook + if listener.type == "click" and listener.handler is not None: + return listener.handler.__self__ + raise AssertionError("the statusline has no click handler") + + +def test_the_session_picker_is_the_workspaces_own_menu(rendered): + """The last Quasar control in the workspace. A `select` has one string per + option, so "another window has this open" arrived as a ` · ` fragment glued + onto the title and looked exactly like the rows that can be opened.""" + client, space = rendered(["chat"]) + with client: + selects = [ + e for e in client.elements.values() if type(e).__name__ == "Select" + ] + assert selects == [] + assert "grad-session-btn" in html_of(client) def test_the_focused_window_is_marked(rendered): diff --git a/tests/test_ui_state.py b/tests/test_ui_state.py index ae6c9b6..9a24c28 100644 --- a/tests/test_ui_state.py +++ b/tests/test_ui_state.py @@ -325,7 +325,15 @@ async def test_switching_folder_reloads_the_project_and_its_layout(workspace, tm assert budget_mod.current_project() is None assert space.project is None assert [c.windows for c in space.layout.columns] != stacked - assert state_mod.layout_dir() == paths.data_dir() / "layouts" + # The layout followed the switch without living in the workspace. Both + # halves matter: it is keyed to the new root (so the arrangement above came + # back to its default rather than carrying over), and it resolves under the + # app directory (so a pane arrangement is never a file somebody commits). + from core import appdata + + assert state_mod.layout_dir() == appdata.workspace_state_dir() / "layouts" + assert appdata.app_dir() in state_mod.layout_dir().parents + assert paths.root() not in state_mod.layout_dir().parents @pytest.mark.asyncio diff --git a/tests/test_ui_tasks.py b/tests/test_ui_tasks.py index 125b792..4c2be10 100644 --- a/tests/test_ui_tasks.py +++ b/tests/test_ui_tasks.py @@ -331,3 +331,106 @@ async def test_run_tool_reports_a_command_that_printed_no_envelope(workspace): payload = await tasks_mod.run_tool(name) assert payload["ok"] is False assert "something went wrong" in payload["error"]["message"] + + +# --------------------------------------------------------------------------- +# the agent's own calls +# --------------------------------------------------------------------------- +class FakeSession: + """A `ui.app.Session` as far as the tasks model is concerned: the turn in + flight, and the turns that settled.""" + + def __init__(self, blocks=None, settled=None) -> None: + self.blocks = blocks or [] + self.settled = settled or [] + + +def call(cid, name="Bash", title="ls", status="running", result="", started=None): + import time as _t + + block = { + "kind": "tool", "id": cid, "name": name, "title": title, + "status": status, "result": result, + } + block["started"] = _t.time() if started is None else started + return block + + +def test_the_turn_in_flight_is_what_the_tasks_window_shows_first(): + """Every capability in this project is reached by a Bash into `tools/`, so + the agent's calls are the other half of "what is running on this machine". + Until this they were visible only in the transcript, which is the wrong + place to look once the conversation has scrolled on.""" + from ui import models + + session = FakeSession( + blocks=[{"kind": "text", "text": "checking"}, call("tu_2", title="pytest -q")], + settled=[{"role": "assistant", "blocks": [call("tu_1", status="ok", result="one\ntwo")]}], + ) + rows = models.agent_calls_model(session) + assert [r["id"] for r in rows] == ["tu_2", "tu_1"] + assert rows[0]["state"] == "running" + assert rows[1]["state"] == "ok" + + +def test_a_call_left_running_by_a_settled_turn_is_not_reported_as_running(): + """The turn died or was interrupted mid-call. Whatever it started is not + this app's to know about, and saying "running" of something nothing is + waiting for is the same lie as a tail that silently forgets.""" + from ui import models + + session = FakeSession(settled=[{"role": "assistant", "blocks": [call("tu_1")]}]) + row = models.agent_calls_model(session)[0] + assert row["state"] == "unfinished" + assert row["running"] is False + assert row["elapsed"] == "" + + +def test_only_a_live_call_is_given_a_clock(): + from ui import models + + session = FakeSession(blocks=[call("tu_1", started=None)]) + assert models.agent_calls_model(session)[0]["elapsed"] != "" + + +def test_a_call_from_a_transcript_written_before_calls_were_stamped_still_lists(): + """The session file outlives the version that wrote it. The row is worth + showing; the clock is the part that is not known.""" + from ui import models + + block = call("tu_1") + del block["started"] + assert models.agent_calls_model(FakeSession(blocks=[block]))[0]["elapsed"] == "" + + +def test_the_call_list_is_bounded_so_the_window_is_not_a_second_transcript(): + from ui import models + + settled = [ + {"role": "assistant", "blocks": [call(f"tu_{i}", status="ok")]} + for i in range(models.AGENT_CALLS * 2) + ] + assert len(models.agent_calls_model(FakeSession(settled=settled))) == models.AGENT_CALLS + + +def test_the_two_lists_are_counted_apart(): + """A task is a process this app started and can stop; a call is one the + agent made and only the agent can stop. Merging them would imply a STOP + button that does not exist.""" + from ui import models, tasks + + tasks.start("a wiki rebuild", "tools.wiki", "map") + model = models.tasks_model(agent=models.agent_calls_model(FakeSession(blocks=[call("tu_1")]))) + assert model["running"] == 1 + assert model["agent_running"] == 1 + assert [r["id"] for r in model["rows"]] != [c["id"] for c in model["agent"]] + + +def test_the_tasks_model_without_a_session_is_unchanged(): + """`tasks_model` is called from the poll and from tests with no session at + all; the agent half is additive.""" + from ui import models + + model = models.tasks_model() + assert model["agent"] == [] + assert model["agent_running"] == 0 diff --git a/tests/test_ui_turns.py b/tests/test_ui_turns.py new file mode 100644 index 0000000..afb93c9 --- /dev/null +++ b/tests/test_ui_turns.py @@ -0,0 +1,252 @@ +"""Interrupting a turn, and the turn after it (`ui.app.Session`). + +The bug this suite exists for, as reported: interrupt the agent, submit another +prompt, and the answer is invisible until you interrupt *again*. It has one +visible symptom and three separate causes, all of which end in the same place -- +`Session.busy` is still True, so the composer's guard silently refuses the next +prompt and nothing on screen says why: + + * the SDK refused the interrupt, and every exception was swallowed; + * the interrupt was accepted and the turn did not end; + * the interrupt landed *late*, on the turn issued after the one it was aimed + at, killing it the moment it started. + +None of the three needs the SDK to reproduce -- they are all about what this +class does with a client that behaves in a particular way -- so the client here +is a fake that can be told to behave in each of them. +""" + +from __future__ import annotations + +import asyncio + +import pytest + +app = pytest.importorskip("ui.app", reason="the ui extra is not installed") + +pytestmark = pytest.mark.asyncio + + +class FakeClient: + """A `ClaudeSDKClient` shaped just enough to drive one turn. + + `receive_response` yields nothing and waits until `finish` is set, which is + what an in-flight turn is; `interrupt` sets it, which is what a working + interrupt does. The variations are what the tests are about. + """ + + def __init__(self, *, refuses: bool = False, ignores: bool = False) -> None: + self.refuses = refuses + self.ignores = ignores + self.finish = asyncio.Event() + self.prompts: list[str] = [] + self.interrupts = 0 + self.closed = False + + async def __aenter__(self): + return self + + async def __aexit__(self, *_): + self.closed = True + # Whatever was waiting for a message is not going to get one. + self.finish.set() + return False + + async def query(self, prompt: str) -> None: + self.prompts.append(prompt) + self.finish.clear() + + async def receive_response(self): + await self.finish.wait() + return + yield # pragma: no cover - makes this an async generator + + async def interrupt(self) -> None: + self.interrupts += 1 + if self.refuses: + raise RuntimeError("not connected") + if not self.ignores: + self.finish.set() + + +@pytest.fixture +def session(monkeypatch): + """A `Session` whose client is a fake and whose notices are collected.""" + notices: list[str] = [] + made: list[FakeClient] = [] + kind: dict[str, dict] = {"kwargs": {}} + + made_session = app.Session("turns") + made_session.notify = notices.append + + async def start() -> None: + if made_session.client is None: + made_session.client = FakeClient(**kind["kwargs"]) + made.append(made_session.client) + + monkeypatch.setattr(made_session, "start", start) + made_session.notices = notices # type: ignore[attr-defined] + made_session.clients = made # type: ignore[attr-defined] + made_session.client_kind = kind # type: ignore[attr-defined] + return made_session + + +async def settled(_record) -> None: + return None + + +async def run_turn(session, prompt: str = "hello"): + """Start a turn and wait until it is genuinely in flight.""" + task = asyncio.create_task(session.ask(prompt, settled)) + for _ in range(200): + await asyncio.sleep(0) + if session.client is not None and session.client.prompts: + return task + raise AssertionError("the turn never reached the client") + + +# --------------------------------------------------------------------------- +# the three causes +# --------------------------------------------------------------------------- +async def test_an_interrupt_the_sdk_refuses_is_reported_and_still_ends_the_turn(session, monkeypatch): + """Every exception used to be swallowed here, which is precisely how + pressing STOP twice became the way to stop a turn: the first press failed in + silence and the second one happened to land. + + A refused interrupt is also the case that most needs the escalation: nothing + asked the turn to stop, so nothing will end it but taking the client down. + """ + monkeypatch.setattr(app, "INTERRUPT_GRACE_S", 0.05) + session.client_kind["kwargs"] = {"refuses": True} + task = await run_turn(session) + + assert session.interrupt() == "interrupting the turn…" + await asyncio.wait_for(task, timeout=5) + + assert session.busy is False + assert any("refused the interrupt" in n for n in session.notices) + + +async def test_a_turn_that_ignores_the_interrupt_is_taken_down_anyway(session, monkeypatch): + """`ui/tasks.py:cancel`'s escalation, applied to the one control that did + not have it. A session that stays busy forever refuses every prompt after + it, and says nothing about either.""" + monkeypatch.setattr(app, "INTERRUPT_GRACE_S", 0.05) + session.client_kind["kwargs"] = {"ignores": True} + task = await run_turn(session) + client = session.client + + session.interrupt() + await asyncio.wait_for(task, timeout=5) + + assert client.interrupts == 1, "the tool's own stop is asked for first" + assert client.closed is True + assert session.busy is False + assert any("did not stop" in n for n in session.notices) + + +async def test_the_next_turn_waits_for_a_pending_interrupt(session, monkeypatch): + """The late interrupt. Fire-and-forget, it outlives the turn it was aimed at + and lands on the one after it -- which then produces nothing and explains + nothing, and is the shape of the bug as reported.""" + monkeypatch.setattr(app, "INTERRUPT_GRACE_S", 0.05) + session.client_kind["kwargs"] = {"ignores": True} + first = await run_turn(session, "one") + + session.interrupt() + await asyncio.wait_for(first, timeout=5) + + second = await run_turn(session, "two") + # The interrupt is finished before the second turn is issued, so it cannot + # be the thing that ends it. + assert session._stopping is None or session._stopping.done() # noqa: SLF001 + assert session.client.interrupts == 0 + session.client.finish.set() + await asyncio.wait_for(second, timeout=5) + assert session.client.prompts == ["two"] + + +# --------------------------------------------------------------------------- +# what the control says +# --------------------------------------------------------------------------- +async def test_interrupting_nothing_says_so_rather_than_pretending(session): + assert session.interrupt() == "nothing is running" + + +async def test_a_second_press_does_not_stack_a_second_interrupt(session, monkeypatch): + monkeypatch.setattr(app, "INTERRUPT_GRACE_S", 0.05) + session.client_kind["kwargs"] = {"ignores": True} + task = await run_turn(session) + + session.interrupt() + assert session.interrupt() == "already stopping — the turn is being taken down" + await asyncio.wait_for(task, timeout=5) + assert session.clients[0].interrupts == 1 + + +# --------------------------------------------------------------------------- +# what survives it +# --------------------------------------------------------------------------- +async def test_the_client_is_rebuilt_after_an_interrupt_and_resumes_the_conversation(session): + """A fresh client cannot hold a message belonging to the turn that was + stopped -- which is the other way the next turn ended instantly with nothing + drawn. `resume` is what keeps that from costing the conversation, so the id + has to have been recorded by then.""" + task = await run_turn(session) + session.sdk_session_id = "sdk-1" + + session.interrupt() + await asyncio.wait_for(task, timeout=5) + assert session.client is None + + second = await run_turn(session, "again") + assert len(session.clients) == 2, "a new client, not the interrupted one" + session.client.finish.set() + await asyncio.wait_for(second, timeout=5) + assert session.sdk_session_id == "sdk-1" + + +async def test_the_sdk_session_id_is_recorded_from_a_turn_that_never_finished(session, monkeypatch): + """`drive_turn`'s return value is not reached on the interrupt path, and + that id is what the rebuilt client resumes from -- so reading it off the + return value lost the conversation on exactly the turns that end in a + rebuild.""" + import agent as agent_mod + + class Message: + session_id = "sdk-7" + + async def drive_turn(client, prompt, stream, *, on_session_id=None, **_): + on_session_id(Message.session_id) + raise RuntimeError("interrupted") + + monkeypatch.setattr(agent_mod, "drive_turn", drive_turn) + await session.ask("hello", settled) + assert session.sdk_session_id == "sdk-7" + + +async def test_a_partly_streamed_turn_keeps_what_it_streamed(session, monkeypatch): + """An interrupted turn is more legible with its half than without it, and + the transcript must not claim the prompt went unanswered.""" + import agent as agent_mod + + async def drive_turn(client, prompt, stream, **_): + stream.feed(_FakeAssistant("half an answer")) + raise RuntimeError("interrupted") + + monkeypatch.setattr(agent_mod, "drive_turn", drive_turn) + await session.ask("hello", settled) + + assert session.settled[0] == {"role": "user", "text": "hello"} + assert "half an answer" in session.settled[1]["text"] + assert "the session failed" in session.settled[1]["text"] + + +class _FakeAssistant: + def __init__(self, text: str) -> None: + self.content = [_FakeTextBlock(text)] + + +class _FakeTextBlock: + def __init__(self, text: str) -> None: + self.text = text diff --git a/tools/lab.py b/tools/lab.py index bf2002f..e7aa18f 100644 --- a/tools/lab.py +++ b/tools/lab.py @@ -27,8 +27,11 @@ deliberately unsandboxed. Lab stays on its own port and never shares the UI's storage secret. 3. *Pin everything.* The JupyterLab 3->4 break is what killed the Tabnine - extension. `pyproject.toml`'s `lab` extra pins JupyterLab itself and every - extension, so an unrelated `pip install -U` cannot take the app down. + extension. `pyproject.toml` pins JupyterLab in the `lab` extra and every + extension in `lab-extensions`, exactly rather than as a floor, so an + unrelated `pip install -U` cannot take the app down. The `ui` extra depends + on `lab`, because the notebook window's interior *is* Lab and an app that + ships that window without a server to put in it ships a button that fails. "Connect an arbitrary extension" therefore means: add a pin, reinstall, restart. """ @@ -45,7 +48,7 @@ from pathlib import Path from typing import Any -from core import jsonl, paths +from core import appdata, jsonl, paths, spawn from core.cli import Cli, main from core.errors import ConfigError, GradError @@ -68,11 +71,14 @@ def _state_path() -> Path: - return paths.data_dir() / "lab" / "lab.json" + """Under the app directory, not the workspace: this file carries the Lab + server's token, and a freshly minted secret does not belong in a folder + somebody will eventually commit.""" + return appdata.state_dir() / "lab" / "lab.json" def _log_path() -> Path: - return paths.data_dir() / "lab" / "lab.log" + return appdata.logs_dir() / "lab.log" def _jupyter_config_dir() -> Path: @@ -90,7 +96,10 @@ def _executable() -> str: return found raise ConfigError( "jupyter is not installed, so there is no Lab server to start", - fix="pip install -e '.[lab]' # pins jupyterlab and every extension", + fix=( + "pip install -e '.[ui]' # the desktop app ships the Lab server it embeds; " + "add ,lab-extensions for the pinned extension set" + ), ) @@ -121,7 +130,11 @@ def _alive(pid: int | None) -> bool: if not pid: return False if os.name == "nt": - out = subprocess.run( + # Through `spawn.run`: this is called on every `lab start` and `lab + # status`, and `tasklist` is a console program. Under the desktop app + # there is no console to inherit, so each check opened one -- a black + # window over the workspace for as long as it took to list one process. + out = spawn.run( ["tasklist", "/FI", f"PID eq {pid}", "/NH"], capture_output=True, text=True, check=False, ) @@ -153,9 +166,21 @@ def cmd_start(args: argparse.Namespace) -> dict[str, Any]: worse trade than re-reading the file after a restart. """ state = _read_state() - if not args.force and state.get("port") and _listening(int(state["port"])) and _alive(state.get("pid")): + running = bool( + state.get("port") and _listening(int(state["port"])) and _alive(state.get("pid")) + ) + # A server already up on the *right* origin is the one thing worth not + # restarting. On the wrong one it is worse than nothing: the framing headers + # are fixed at launch, so the iframe is blocked and the browser reports it as + # "127.0.0.1 refused to connect" -- which reads as a dead port and sends you + # looking for a server that is running perfectly well. Restarting is the only + # way to change a header that was decided at start time. + stale_origin = running and state.get("ui_origin") != args.ui_origin + if running and not args.force and not stale_origin: return {**state, "already_running": True, "next": "python -m tools.lab status --json"} + if stale_origin: + cmd_stop(argparse.Namespace()) executable = _executable() port = _free_port(args.port) @@ -190,28 +215,21 @@ def cmd_start(args: argparse.Namespace) -> dict[str, Any]: f"--ServerApp.config_file={_jupyter_config_dir() / 'jupyter_server_config.py'}", ] - creationflags = 0 - start_new_session = False - if os.name == "nt": - creationflags = getattr(subprocess, "DETACHED_PROCESS", 0) | getattr( - subprocess, "CREATE_NEW_PROCESS_GROUP", 0 - ) - else: - start_new_session = True - with open(log, "ab") as fh: - proc = subprocess.Popen( + # Detached, which on Windows also means *no console at all* -- a + # stronger promise than `CREATE_NO_WINDOW`, and not combinable with it. + # See `core/spawn.py`. + server = subprocess.Popen( argv, cwd=str(paths.root()), stdout=fh, stderr=subprocess.STDOUT, - stdin=subprocess.DEVNULL, env=env, - creationflags=creationflags, start_new_session=start_new_session, + stdin=subprocess.DEVNULL, env=env, **spawn.detached(), ) deadline = time.time() + 30 while time.time() < deadline and not _listening(port): - if proc.poll() is not None: + if server.poll() is not None: raise GradError( "lab_died", - f"the Lab server exited immediately (code {proc.returncode})", + f"the Lab server exited immediately (code {server.returncode})", exit_code=8, fix=f"read {log}", detail={"log": str(log)}, @@ -221,7 +239,7 @@ def cmd_start(args: argparse.Namespace) -> dict[str, Any]: record = { "port": port, "token": token, - "pid": proc.pid, + "pid": server.pid, "url": f"http://127.0.0.1:{port}/lab", "root_dir": str(paths.root()), "ui_origin": args.ui_origin, @@ -294,7 +312,7 @@ def cmd_extensions(_: argparse.Namespace) -> dict[str, Any]: def _run(argv: list[str]) -> dict[str, Any]: try: - out = subprocess.run( + out = spawn.run( argv, capture_output=True, text=True, timeout=120, env=env, check=False ) except subprocess.TimeoutExpired: @@ -325,7 +343,7 @@ def cmd_stop(_: argparse.Namespace) -> dict[str, Any]: jsonl.write_json(_state_path(), {}) return {"stopped": False, "note": "no Lab server was running"} if os.name == "nt": - subprocess.run(["taskkill", "/PID", str(pid), "/T", "/F"], capture_output=True, check=False) + spawn.run(["taskkill", "/PID", str(pid), "/T", "/F"], capture_output=True, check=False) else: import signal # noqa: PLC0415 diff --git a/tools/nb.py b/tools/nb.py index 332a647..8504a2a 100644 --- a/tools/nb.py +++ b/tools/nb.py @@ -27,7 +27,7 @@ from pathlib import Path from typing import Any -from core import config as config_mod, paths +from core import appdata, config as config_mod, paths, spawn from core.cli import Cli, main from core.errors import EXIT_CHECK_FAILED, ConfigError, GradError, NotFound, UsageError @@ -55,7 +55,17 @@ def _jupyter() -> Any: def _conn_path(name: str) -> Path: - d = paths.data_dir() / CONNECTION_DIR + """Per workspace, not per installation. + + Kernel names reach this unqualified -- `default` is the one most commands + get -- so a single directory shared by every workspace means opening a + second folder and running a cell reconnects to the *first* folder's kernel: + same name, same connection file, a live kernel on the other end with another + project's imports and another project's `cwd`. Verifying a notebook against + that is exactly the "works in the kernel that grew it" failure `nb verify` + exists to catch, arrived at from the opposite direction. + """ + d = appdata.workspace_state_dir() / CONNECTION_DIR d.mkdir(parents=True, exist_ok=True) return d / f"{name}.json" @@ -70,8 +80,11 @@ def _start_kernel(name: str, kernel_name: str) -> dict[str, Any]: for every cell, so the kernel must survive the CLI exiting. A `KernelManager`-owned kernel does not -- it is torn down with its manager, which is correct for a notebook server and useless here. So the connection - file is written first and `ipykernel_launcher` is spawned detached - (DETACHED_PROCESS on Windows, a new session elsewhere). + file is written first and `ipykernel_launcher` is spawned detached. + + Through `core/spawn.py` rather than by hand: the flags for "outlives me and + shows no window, for its children too" are subtle enough that a second copy + of them is a second thing to get wrong, and this one *was* the second copy. """ jc = _jupyter() conn = _conn_path(name) @@ -79,15 +92,6 @@ def _start_kernel(name: str, kernel_name: str) -> dict[str, Any]: jc.write_connection_file(fname=str(conn), kernel_name=kernel_name) log = conn.with_suffix(".log") - creationflags = 0 - start_new_session = False - if os.name == "nt": - creationflags = getattr(subprocess, "DETACHED_PROCESS", 0) | getattr( - subprocess, "CREATE_NEW_PROCESS_GROUP", 0 - ) - else: - start_new_session = True - with open(log, "wb") as fh: proc = subprocess.Popen( [sys.executable, "-m", "ipykernel_launcher", "-f", str(conn)], @@ -95,8 +99,7 @@ def _start_kernel(name: str, kernel_name: str) -> dict[str, Any]: stdout=fh, stderr=subprocess.STDOUT, stdin=subprocess.DEVNULL, - creationflags=creationflags, - start_new_session=start_new_session, + **spawn.detached(), ) conn.with_suffix(".pid").write_text(str(proc.pid), encoding="utf-8") return {"connection_file": str(conn), "kernel_name": kernel_name, "pid": proc.pid, "started": True} @@ -432,7 +435,7 @@ def cmd_stop(args: argparse.Namespace) -> dict[str, Any]: @cli.command("status", "which kernels have connection files") def cmd_status(_: argparse.Namespace) -> dict[str, Any]: - d = paths.data_dir() / CONNECTION_DIR + d = appdata.workspace_state_dir() / CONNECTION_DIR return { "kernels": [p.stem for p in d.glob("*.json")] if d.exists() else [], "figures_dir": str(paths.figures_dir()), diff --git a/tools/paper_search.py b/tools/paper_search.py index 68145b1..53b8009 100644 --- a/tools/paper_search.py +++ b/tools/paper_search.py @@ -17,6 +17,8 @@ import argparse import json import re +import sys +import time from typing import Any from core import config as config_mod, corpus, credentials, haiku, http, paths, quota_log @@ -40,28 +42,82 @@ #: `tier1` value -> the clients it selects, in the order they are queried. -TIER1_SOURCES = ("asta", "s2", "both", "none") +#: `both` predates `pwc` and still means what it meant: the two Semantic Scholar +#: doors, for comparing them. +TIER1_SOURCES = ("pwc", "asta", "s2", "both", "all", "none") + + +class _Budget: + """A wall clock over the whole of stage 1. + + `core/http.py` bounds one *request*; this bounds the run. They are different + numbers because stage 0 turns one question into six queries and each is put + to two endpoints, so a per-request deadline of five minutes is a + one-hour stage. What made that concrete: a live `snippet_search` took 283 + seconds to report that Asta's own backend had refused a connection, and the + funnel was killed by its caller long before the endpoints that *were* + working could contribute anything. + + Stopping early is not the same as failing. What has been retrieved is kept, + what was skipped is written into the trace, and the caller gets results -- + which is the whole difference between a slow funnel and a broken one. + """ + + def __init__(self, limit: float) -> None: + self.limit = max(0.0, limit) + self._started = time.monotonic() + + @property + def elapsed(self) -> float: + return time.monotonic() - self._started + + @property + def spent(self) -> bool: + return bool(self.limit) and self.elapsed >= self.limit + + +def _progress(message: str) -> None: + """A line of progress on stderr, where a `--json` contract cannot see it. + + The funnel prints nothing until its envelope, which is minutes later, and + everything that runs it reads a pipe: the tasks window streams the tail, and + the agent's own Bash call gives up at 120 seconds and moves the command to + the background. A run with no output is indistinguishable from a hung one in + all three places, and that is how a slow stage got read as a broken tool. + """ + print(message, file=sys.stderr, flush=True) def tier1_clients(cfg: Any, override: str | None = None) -> list[tuple[str, Any]]: """The discovery clients for this run, named so a trace can say which spoke. - Both reach the same Semantic Scholar corpus and both answer in the same - vocabulary (`core/http.py:_row`), so a candidate found by either fuses to - one entry. What differs is whether the door opens: S2's own API no longer - issues keys to free-domain addresses, so a personal account falls back to - the shared anonymous pool. + All three answer in the same vocabulary (`core/http.py:_row`, `_pwc_row`), + so the funnel does not know which tier a candidate came from and does not + have to. What differs is whether the door opens, and how fast: + + * **pwc** -- Papers with Code, as revived by Hugging Face. Anonymous, one to + two seconds, and two genuinely different rankings (lexical and dense) that + RRF was built to fuse. The default, because it is the one that answers. + * **asta** -- the same Semantic Scholar corpus as `s2`, through a door that + opens without an institutional address, and the only one of the three with + real full-text snippets. Measured live at 121s for a search and 283s to + report a backend failure, which is why it is no longer the default. + * **s2** -- the REST API directly. Its own keys are only issued to + institutional addresses, so a personal account falls back to the shared + anonymous pool, which is near-permanently rate limited. """ - chosen = str(override or cfg.get("retrieval", "tier1", "asta")).lower() + chosen = str(override or cfg.get("retrieval", "tier1", "pwc")).lower() if chosen not in TIER1_SOURCES: raise UsageError( f"unknown tier-1 source {chosen!r}", fix=f"one of: {', '.join(TIER1_SOURCES)}", ) out: list[tuple[str, Any]] = [] - if chosen in ("asta", "both"): + if chosen in ("pwc", "all"): + out.append(("pwc", http.PapersWithCode(cfg))) + if chosen in ("asta", "both", "all"): out.append(("asta", http.Asta(cfg))) - if chosen in ("s2", "both"): + if chosen in ("s2", "both", "all"): out.append(("s2", http.SemanticScholar(cfg))) return out @@ -120,32 +176,97 @@ def cmd_search(args: argparse.Namespace) -> dict[str, Any]: if tier1: per_query = max(5, ceiling // max(1, len(queries) * 2 * len(tier1))) + budget = _Budget(float(cfg.get("retrieval", "stage1_budget_s", 300))) + #: `(source, verb)` pairs that have already failed once. See `_Budget`. + dropped: set[tuple[str, str]] = set() + searched = 0 for query in queries: + if budget.spent: + break + searched += 1 for name, client in tier1: for verb in ("snippet_search", "paper_search"): + if (name, verb) in dropped or budget.spent: + continue + _progress(f"stage 1: {name}.{verb} q{searched}/{len(queries)}") try: hits = getattr(client, verb)(query, limit=per_query) except GradError as exc: + # Dropped for the rest of the run, not merely skipped for + # this query. A tier-1 endpoint that is down is down for + # every query, and finding that out costs a *request* -- + # a live snippet_search took 283 seconds to report that + # Asta's own backend had refused a connection, so trying + # it once per expanded query spent 28 minutes learning + # the same thing six times and got the run killed before + # the stages that were working could return anything. + dropped.add((name, verb)) trace.setdefault("warnings", []).append(f"{name}.{verb}: {exc}") upstream_failures.append(f"{name}.{verb}: {exc}") + _progress(f"stage 1: {name}.{verb} failed; not retried this run") continue rankings.append(hits) + _progress(f"stage 1: {name}.{verb} returned {len(hits)}") for hit in hits: candidates.setdefault(hit["id"], hit) - if not args.no_citations: + # Both caps are reported rather than left to be inferred from a thin + # result set: a funnel that quietly searched two of six queries looks + # exactly like a corpus that had little to say. + if dropped: + trace.setdefault("warnings", []).append( + "dropped for the rest of this run after one failure each: " + + ", ".join(sorted(f"{n}.{v}" for n, v in dropped)) + ) + if budget.spent: + trace.setdefault("warnings", []).append( + f"tier-1 discovery stopped after {budget.limit:.0f}s having searched " + f"{searched} of {len(queries)} queries — raise [retrieval] " + "stage1_budget_s, or --no-expand to search one query instead of six" + ) + trace["stages"]["1_discovery"] = { + "queries_searched": searched, + "queries": len(queries), + "dropped": sorted(f"{n}.{v}" for n, v in dropped), + "seconds": round(budget.elapsed, 1), + } + if not args.no_citations and not budget.spent: + # Twenty calls live under that single check -- five seeds by two + # clients by two directions -- so testing the budget once at the top + # bounds nothing: the stage can run minutes past `stage1_budget_s` + # doing expansion after it has already decided it is out of time. + # Rechecked per call, and the loop is left rather than skipped, so + # what is already retrieved is kept and the trace still records how + # far it got. seeds = [c for c in list(candidates.values())[:5] if c.get("paper_id")] + expanded = 0 for seed in seeds: + if budget.spent: + break for name, client in tier1: + # A verb that was dropped in discovery was dropped because + # this endpoint refused it or timed out. Asking the same + # client again, per seed, spends the remaining budget + # rediscovering a failure already recorded above. + if (name, "neighbours") in dropped or budget.spent: + continue for direction in ("citations", "references"): + if budget.spent: + break try: hits = client.neighbours( seed["paper_id"], direction=direction, limit=10 ) except GradError: + dropped.add((name, "neighbours")) continue + expanded += 1 rankings.append(hits) for hit in hits: candidates.setdefault(hit["id"], hit) + trace["stages"]["1_discovery"]["expansions"] = expanded + trace["stages"]["1_discovery"]["dropped"] = sorted( + f"{n}.{v}" for n, v in dropped + ) if not args.no_local: local = _local_ranked(args.question, hyde, cfg, trace) @@ -156,6 +277,7 @@ def cmd_search(args: argparse.Namespace) -> dict[str, Any]: fused = corpus.rrf(rankings, k=int(cfg.get("retrieval", "rrf_k", 60))) pool = [candidates[f["id"]] | {"rrf": f["rrf"]} for f in fused if f["id"] in candidates][:ceiling] + _fill_abstracts(pool, cfg, trace) quota_log.record( quota_log.STAGE_RETRIEVE, unit="quota", detail={"queries": len(queries), "candidates": len(pool)} ) @@ -177,7 +299,7 @@ def cmd_search(args: argparse.Namespace) -> dict[str, Any]: raise UpstreamError( "every retrieval call failed, so the search returned nothing: " + "; ".join(dict.fromkeys(upstream_failures)), - fix=_tier1_fix(trace["stages"]["1_sources"]), + fix=_tier1_fix(trace["stages"]["1_sources"], upstream_failures), ) return { "question": args.question, @@ -260,31 +382,105 @@ def apply_rerank(pool: list[dict[str, Any]], scored: list[dict[str, Any]]) -> li ] -def _tier1_fix(sources: list[str]) -> str: - """What to actually do when discovery is down, per source. +def _tier1_fix(sources: list[str], failures: list[str] | None = None) -> str: + """What to actually do when discovery is down, per source *and per failure*. This used to say "store a Semantic Scholar API key -- it is free", which stopped being true: Ai2 no longer accept key requests from free-domain email addresses, so for a personal account that instruction has no ending. Advice that cannot be followed is worse than no advice, because it is followed first and the real fix is found second. + + The Asta branch had the same problem one step down. It said "discovery is + rate limited, not broken" whatever had happened, and pointed at a key -- but + a live run failed with `ConnectionRefusedError` raised inside Asta's own + backend, which no key affects. So the advice is now chosen by what the + failures actually say rather than assumed. """ - if sources == ["s2"]: + joined = " ".join(failures or []) + local = "Meanwhile --local-only searches what is already ingested." + elsewhere = ( + "python -m tools.paper_search search '' --tier1 {} --json " + '(or set [retrieval] tier1 = "{}" in config/grad.toml)' + ) + # The cause first where there is one, because a rate limit has a fix of its + # own and it is not the same fix as an endpoint being down. + if "rate-limited" in joined or "429" in joined: + if "asta" in sources: + return ( + "retry -- discovery is rate limited, not broken. A key raises Asta's limits " + "and is requested from a form rather than reviewed: " + f"python -m tools.jobs credential set {credentials.ASTA_KEY}. {local}" + ) + if "pwc" in sources: + return ( + "retry -- discovery is rate limited, not broken. This catalogue is " + "anonymous, so there is no key that raises it; the same literature is " + "reachable more slowly through " + elsewhere.format("asta", "asta") + f". {local}" + ) + return ( + "retry -- Semantic Scholar's anonymous pool is shared and its own keys are only " + "issued to institutional addresses. Papers with Code is anonymous and answers in " + "about a second: " + elsewhere.format("pwc", "pwc") + f". {local}" + ) + if sources == ["pwc"]: + return ( + "this catalogue is anonymous, so there is no key to add and nothing to " + "configure -- it is down or unreachable. Retry, or reach the same literature " + "more slowly through " + elsewhere.format("asta", "asta") + f". {local}" + ) + if sources in (["s2"], ["asta"]): + # Both doors onto the Semantic Scholar corpus point at the one that is + # neither rate limited nor minutes slow. return ( - "Semantic Scholar's own API only issues keys to institutional addresses, so " - "this is the shared anonymous pool. Switch to Ai2's Asta, which serves the " - "same corpus and does not require one: " - "python -m tools.paper_search search '' --tier1 asta --json " - "(or set [retrieval] tier1 = \"asta\" in config/grad.toml)" + "Semantic Scholar's own API only issues keys to institutional addresses, and " + "Asta answers in minutes rather than seconds. Papers with Code is anonymous and " + "answers in about a second: " + elsewhere.format("pwc", "pwc") + f". {local}" ) return ( - "retry -- discovery is rate limited, not broken. A key raises Asta's limits and " - "is requested from a form rather than reviewed: " - f"python -m tools.jobs credential set {credentials.ASTA_KEY}. " - "Meanwhile --local-only searches what is already ingested." + "the message above is the service's own and names the endpoint that refused -- no " + f"key affects it. The endpoints fail independently, so retrying is worth it even " + f"when one of them is down. {local}" ) +def _fill_abstracts(pool: list[dict[str, Any]], cfg: Any, trace: dict[str, Any]) -> None: + """Give the candidates that have no text an abstract, in one request. + + Stage 2 reranks on `title + snippet-or-abstract` and stage 3 triages on the + same, so a pool of bare titles is a measurably worse funnel -- and the fast + corpus does not return abstracts with its search results. Nearly every row + it does return is an arXiv paper, and arXiv takes a hundred ids at once, so + the whole pool costs one call rather than one per candidate. + + Degrades rather than fails: a candidate with no abstract ranks on its title, + which is what would have happened without this. What it must not do is go + unrecorded -- a funnel silently reranking titles looks exactly like one + reranking abstracts, and only one of them is the retrieval §5 evaluates. + """ + wanted = { + c["external"]["ArXiv"]: c + for c in pool[: http.ARXIV_BATCH] + if not (c.get("snippet") or c.get("abstract")) + and isinstance(c.get("external"), dict) + and c["external"].get("ArXiv") + } + if not wanted: + return + _progress(f"stage 1: fetching {len(wanted)} abstracts from arXiv") + found = http.arxiv_abstracts(list(wanted), cfg=cfg) + for arxiv_id, abstract in found.items(): + if arxiv_id in wanted: + wanted[arxiv_id]["abstract"] = abstract + missing = len(wanted) - len(found) + trace["stages"]["1_abstracts"] = {"wanted": len(wanted), "found": len(found)} + if missing: + trace.setdefault("warnings", []).append( + f"{missing} of {len(wanted)} candidates were reranked and triaged on their " + "title alone — arXiv returned no abstract for them" + ) + + def _local_ranked(question: str, hyde: str | None, cfg: Any, trace: dict[str, Any]) -> list[dict[str, Any]]: """Tier 2, fused across FTS5 and vectors before joining the global pool.""" path = paths.corpus_sqlite() diff --git a/ui/app.py b/ui/app.py index cf4cab1..acaab5f 100644 --- a/ui/app.py +++ b/ui/app.py @@ -39,14 +39,32 @@ import logging import re import secrets +import sys from pathlib import Path from typing import Any -from core import config as config_mod, paths -from ui import katex, kit, sessions, shell, state as state_mod +from core import appdata, config as config_mod, instance, paths +from ui import desktop, katex, kit, render, sessions, shell, state as state_mod ROLES = ("user", "assistant") STATIC_URL = "/grad-static" +#: The port `run()` bound, so the rest of the app can name its own origin. The +#: embedded Lab scopes its framing headers to one origin and is started by a +#: button in the notebook window, which would otherwise have to assume the +#: default port and be wrong on every `--port`. Set at launch rather than read +#: from the environment because `ui.run` is where the number is decided. +PORT = 8080 + +#: How long the SDK's own interrupt is given to end the turn before the client +#: is taken down instead. The same shape as `ui/tasks.py:cancel` -- ask the thing +#: that knows how to stop cleanly, then stop it anyway -- and for the same +#: reason: a control that reports "interrupting…" and leaves the session busy +#: forever is worse than no control, because the composer then silently refuses +#: every prompt after it. +INTERRUPT_GRACE_S = 8.0 +#: How often `_stop_turn` checks whether the turn it asked to stop has settled. +SETTLE_POLL_S = 0.1 + # Where anything the transcript must not carry goes instead: this handler is the # app's own log, not user-visible text and not the persisted session file. log = logging.getLogger("grad.ui") @@ -73,6 +91,11 @@ def __init__(self, key: str = "default") -> None: # rather than at claim time means `most_recent` can tell "another window # is in this session" from "the window that was in it is gone". sessions.register(key) + #: Where a message that has nowhere else to go is put on screen. Set by + #: `build` to the workspace's status bar; None in tests and on the CLI. + #: An interrupt that failed used to be swallowed entirely, which is how + #: pressing STOP twice became the way to stop a turn. + self.notify: Any = None self.client: Any = None #: The turn in flight, as `agent.TurnStream` blocks: prose, and the tool #: calls between it. The chat window's timer draws from here, so a card @@ -89,7 +112,15 @@ def __init__(self, key: str = "default") -> None: #: Ours names the file; this one is what makes reopening continue rather #: than merely redisplay. See the note in `ui/sessions.py`. self.sdk_session_id: str | None = None - self._task: asyncio.Task[None] | None = None + #: Set while no turn is in flight, so an interrupt can wait for the turn + #: it asked to stop without polling `busy` -- which the *next* turn sets + #: back to True, and a waiter watching that flag would never wake. + self._idle = asyncio.Event() + self._idle.set() + #: The interrupt in progress, if one is. Held so the next turn can wait + #: for it: a fire-and-forget interrupt can outlive the turn it was aimed + #: at and land on the one after it. + self._stopping: asyncio.Task[None] | None = None async def start(self) -> None: if self.client is not None: @@ -234,6 +265,7 @@ async def rebind(self) -> None: await self.close() self.blocks = [] self.busy = False + self._idle.set() self.settled.clear() # Sessions live under the root's data directory, so the one that was # open belongs to the folder just left -- and the claim on it does too. @@ -252,10 +284,18 @@ async def ask(self, prompt: str, on_settle: Any) -> None: if self.busy: return self.busy = True + self._idle.clear() try: + # Before `start`, and before anything is sent. An interrupt is a + # control request to the CLI, and one still in flight is aimed at a + # turn that has already ended -- so without this it lands on the + # turn about to be issued and kills it the moment it starts, which + # is a turn that produces nothing and explains nothing. + await self._stopped() await self.start() except Exception: self.busy = False + self._idle.set() raise self.settled.append({"role": "user", "text": prompt}) @@ -271,7 +311,15 @@ async def ask(self, prompt: str, on_settle: Any) -> None: # rather than inline is what stops the two surfaces disagreeing about # whether the budget applies. result = await agent.drive_turn( - self.client, prompt, stream, session=self.session_id + self.client, + prompt, + stream, + # Recorded as it arrives rather than read off the return value, + # which an interrupted turn never reaches. That id is what lets + # the rebuilt client `resume` this conversation, so losing it on + # exactly the turns that end in a rebuild cost the whole thread. + on_session_id=self._remember_sdk_session, + session=self.session_id, ) if result.get("sdk_session_id"): self.sdk_session_id = result["sdk_session_id"] @@ -298,6 +346,10 @@ async def ask(self, prompt: str, on_settle: Any) -> None: ) finally: self.busy = False + # Woken here rather than at the end of the block: an interrupt that + # is waiting for this turn is waiting to stop *doing* things, and + # persisting and settling the transcript below are not that. + self._idle.set() # The first thing asked is what the session is about, and naming it # from the prompt beats leaving every session called "empty session" # until someone renames it by hand. An explicit title is never @@ -318,23 +370,118 @@ async def ask(self, prompt: str, on_settle: Any) -> None: self._persist() await on_settle(record) - def interrupt(self) -> None: - """Interrupt the turn in flight, if there is one. + def interrupt(self) -> str: + """Stop the turn in flight, and say what was done about it. Bound to a button and to Escape, so it fires when nothing is running. - The result has to be consumed: a bare `create_task` drops any SDK error - and Python logs "Task exception was never retrieved". + + This used to be one `create_task` around `client.interrupt()` with every + exception swallowed, and it had three failure modes that all looked the + same from the composer -- the turn stays busy, so the *next* prompt is + silently refused and nothing on screen says why. Pressing STOP a second + time appeared to fix it, which is the bug as reported: + + * the SDK refused the interrupt (not connected, control request + errored), and nothing said so; + * the interrupt was accepted but the turn did not end, and the session + stayed busy for the life of the app; + * the interrupt arrived late, after the turn had already ended, and + landed on whatever was issued next. + + So: the failure is reported, the turn is *made* to end, and the pending + interrupt is something the next turn waits for. The message is returned + rather than pushed, because the two callers already have somewhere to + put a line and disagree about where. + """ + if not self.busy: + return "nothing is running" + if self._stopping is not None and not self._stopping.done(): + return "already stopping — the turn is being taken down" + self._stopping = asyncio.create_task(self._stop_turn()) + return "interrupting the turn…" + + async def _stop_turn(self) -> None: + """Ask the SDK to stop, then make sure the turn actually stopped. + + The escalation is `ui/tasks.py:cancel`'s, for the same reason: the tool's + own stop verb is the one that ends things cleanly, and the blunt + instrument is what happens when it does not work. Here the blunt + instrument is dropping the client, which ends `receive_response` and lets + `ask` settle the partial turn the way any other failure settles. + + **The client is dropped either way**, and that is deliberate rather than + laziness about the happy path. The client owns the CLI subprocess and the + message stream, and an interrupted turn is precisely the case where what + is left in that stream is unclear -- a result the aborted turn never + emitted, or one nobody read. Reusing it made the *next* turn end + instantly on a message belonging to the last one, with nothing drawn. + A fresh client cannot have a stale message in it, and `resume` carries + the conversation across, so what is paid is a restart on a deliberate, + occasional action. """ - if not (self.busy and self.client and hasattr(self.client, "interrupt")): + # Captured once, and every step below acts on *this* client rather than + # on `self.client`. There are two awaits in here, each long enough for + # `ask` to have settled the turn and built a fresh client underneath -- + # and the fresh one belongs to a turn nobody asked to stop. Taking it + # down, or clearing the busy flag it set, stops a turn that started + # after the interrupt. + client = self.client + if client is not None and hasattr(client, "interrupt"): + try: + await client.interrupt() + except Exception as exc: # noqa: BLE001 - reported, never swallowed + log.warning("the SDK refused the interrupt", exc_info=exc) + self.say( + f"the SDK refused the interrupt ({type(exc).__name__}) — " + "stopping the turn the hard way" + ) + if not await self._idles_within(INTERRUPT_GRACE_S): + self.say(f"the turn did not stop within {INTERRUPT_GRACE_S:.0f}s — taking the client down") + # Whether it stopped when asked or had to be taken down, the client goes + # -- unless it is no longer ours to take down. + if self.client is not client: + return + await self.close() + if self.client is not client and self.client is not None: return + if self.busy and not await self._idles_within(INTERRUPT_GRACE_S): + # The turn outlived the client that was feeding it. Whatever it is + # waiting for is not going to arrive, and a composer that stays + # locked on the outcome of that is the failure this whole method is + # about -- so the flag is cleared and the fact is said out loud + # rather than left to be inferred from a session that never answers. + self.busy = False + self._idle.set() + self.say("the turn is still winding down — the composer is usable again") - async def _interrupt() -> None: - try: - await self.client.interrupt() - except Exception: # noqa: BLE001 - a failed interrupt must not kill the app - pass + async def _idles_within(self, seconds: float) -> bool: + """Has the turn settled inside this window? Never raises.""" + try: + await asyncio.wait_for(self._idle.wait(), timeout=seconds) + except asyncio.TimeoutError: + return False + return True + + async def _stopped(self) -> None: + """Wait for a pending interrupt, so it cannot land on the next turn.""" + pending, self._stopping = self._stopping, None + if pending is None or pending.done(): + return + # Bounded: `_stop_turn` bounds itself, and a hang here would be the very + # thing this method exists to prevent, one layer up. + await asyncio.wait([pending], timeout=INTERRUPT_GRACE_S * 2) + + def _remember_sdk_session(self, sdk_session_id: str) -> None: + self.sdk_session_id = sdk_session_id - self._task = asyncio.create_task(_interrupt()) + def say(self, message: str) -> None: + """A line for the status bar, when there is one to put it in.""" + log.info("%s", message) + if self.notify is not None: + try: + self.notify(message) + except Exception: # noqa: BLE001 - a notice must not kill a turn + log.exception("could not report: %s", message) def path(self) -> Path: return sessions.path_for(self.session_id) @@ -435,6 +582,52 @@ def build() -> None: ui.add_body_html(f'', shared=True) katex.install(nicegui_app) + @nicegui_app.get("/__grad/show") + def _show() -> dict[str, bool]: + """How a second launch hands over to this one. + + The launcher cannot raise another process's window, and on Windows it + may not even be allowed to try -- foreground rights belong to the + process that has them. So the running instance raises its own window, + and the only thing crossing the boundary is the request. Unauthenticated + like the rest of this port, and harmless: the whole effect is that a + window the user already owns becomes visible. + """ + return {"shown": desktop.show_window()} + + @nicegui_app.get("/__grad/notebook/{name}") + def _notebook(name: str) -> Any: + """One notebook, rendered read-only, for the pane's iframe. + + Served from this app rather than from Lab so the pane has something to + show whether or not a Lab server is running -- and so what it shows can + be sandboxed, which Lab cannot be. `ui/render.py` explains the rest; the + name is validated there, against a directory rather than a pattern. + """ + from fastapi.responses import HTMLResponse, PlainTextResponse # noqa: PLC0415 + + try: + body = render.notebook_html(name) + except render.NotAllowed: + return PlainTextResponse("no such notebook in this workspace", status_code=404) + except OSError as exc: + return PlainTextResponse(f"could not read it: {exc}", status_code=503) + return HTMLResponse( + body, + headers={ + # It is a document built from untrusted stored output and it + # needs nothing from anywhere: no scripts, no fetches, no + # framing by anyone but us. The iframe is sandboxed as well -- + # this is the half that holds if the sandbox attribute is ever + # dropped by an edit that looks unrelated. + "Content-Security-Policy": ( + "default-src 'none'; img-src data: blob:; style-src 'unsafe-inline'; " + f"frame-ancestors {desktop.origin(PORT)} http://localhost:{PORT}" + ), + "Cache-Control": "no-store", + }, + ) + @ui.page("/") def index() -> None: from nicegui import context # noqa: PLC0415 - page scope, not import scope @@ -442,6 +635,11 @@ def index() -> None: session = Session(_client_key()) session.adopt() workspace = state_mod.Workspace(session, _current_project()) + # The one place both exist. A turn's own failures already reach the + # transcript; this is for the things that happen *around* a turn -- an + # interrupt the SDK refused, a client that had to be taken down -- which + # have no turn to be written into. + session.notify = workspace.say # Per client, not `app.on_shutdown`: that would accumulate one handler # per connection and hold every session's subprocess open until the app # itself exits. @@ -525,7 +723,7 @@ def _storage_secret() -> str: Persisted rather than generated per launch so a restart does not orphan every transcript written before it. """ - path = paths.data_dir() / "ui_storage_secret" + path = appdata.state_dir() / "ui_storage_secret" if not path.exists(): path.parent.mkdir(parents=True, exist_ok=True) path.write_text(secrets.token_urlsafe(32), encoding="utf-8") @@ -536,14 +734,88 @@ def _storage_secret() -> str: return path.read_text(encoding="utf-8").strip() -def run(*, native: bool = True, port: int = 8080) -> None: +def _install_desktop(native: bool) -> None: + """Startup wiring that only makes sense once the loop and window exist. + + Registered as a NiceGUI startup handler rather than done before `ui.run`, + because both halves need things that do not exist yet at call time: the + event loop `desktop.request_quit` dispatches onto, and the pywebview window + whose close button is being reinterpreted. + """ + from nicegui import app as nicegui_app # noqa: PLC0415 + + @nicegui_app.on_startup + def _wire() -> None: + desktop.bind_loop(asyncio.get_running_loop()) + if not native: + # Browser mode has no window to hide and no tray to hide it to; the + # tab is the affordance and closing it is the user's business. + return + desktop.start_tray(on_restart_lab=_restart_lab_here) + window = getattr(nicegui_app.native, "main_window", None) + if window is None: + return + + def _on_closing() -> bool: + """False cancels the close, which is how pywebview spells "hide".""" + if desktop.hide_to_tray(): + return False + desktop.request_quit() + return False + + try: + window.events.closing += _on_closing + except Exception: # noqa: BLE001 - an un-hookable window just closes + log.debug("could not intercept the window close", exc_info=True) + + @nicegui_app.on_shutdown + def _unwire() -> None: + from core import instance # noqa: PLC0415 + + instance.release() + + +def _restart_lab_here() -> None: + """Restart Lab bound to this app's origin. Also the tray's menu entry.""" + from core import spawn # noqa: PLC0415 + + argv = [ + sys.executable, "-m", "tools.lab", "start", + "--ui-origin", desktop.origin(PORT), "--force", + ] + try: + spawn.run(argv, cwd=str(paths.root()), capture_output=True, text=True, timeout=90) + except Exception: # noqa: BLE001 - reported by the window's next poll + log.exception("could not restart Lab") + + +def run(*, native: bool = True, port: int | None = None) -> None: """`ui.run(native=True)` gives a real desktop window via pywebview, so the packaging question is answered without Electron or Tauri. Browser mode is - the fallback when pywebview misbehaves on Windows.""" + the fallback when pywebview misbehaves on Windows. + + `port=None` means "choose one" -- see `ui/desktop.py:choose_port` for why + that is a walk-up from 8080 rather than anything random. + """ from nicegui import ui + global PORT + + port = desktop.choose_port(port) + PORT = port + appdata.ensure() + # Here as well as in `agent.py:main`, because this is a public entry point: + # anything that imports `ui.app` and calls `run` -- a launch config, a test + # harness, a shortcut written before the CLI existed -- skips `main` + # entirely, and would then open on an empty app directory while the state it + # wanted sat unmigrated in the workspace. Idempotent, so running twice costs + # a stat per entry. + for name in appdata.migrate_legacy(): + log.info("moved data/%s into %s", name, appdata.app_dir()) paths.ensure_workspace() + instance.publish(port) build() + _install_desktop(native) # `window_size` is passed *only* in native mode, and that is not a # nicety: NiceGUI turns `native` on whenever a window size is given, so # passing it unconditionally made `native=False` unreachable and the diff --git a/ui/desktop.py b/ui/desktop.py new file mode 100644 index 0000000..d641b1e --- /dev/null +++ b/ui/desktop.py @@ -0,0 +1,438 @@ +"""The parts of being a desktop app that are not the workspace itself. + +Port selection, the notification-area icon, what the window's close button +means, and the one question that has to be asked before any of it shuts down: +*is something still running?* + +Three decisions are load-bearing here. + +**The port is chosen, not random.** 8080 if it is free, then 8081, 8082, and so +on. A random port would collide with nothing, which sounds strictly better until +you remember that JupyterLab bakes `frame-ancestors` into its CSP at *launch* +from the app's origin (`config/jupyter/jupyter_server_config.py`). A new random +port every launch is a new origin every launch, so a Lab server left running in +the background would be scoped to the port before last and refuse to embed -- +every single time. Walking up from a fixed base means the port is usually the +same one, so the surviving Lab usually still matches. `ui/windows/notebook.py` +handles the case where it does not. + +**Closing the window hides it.** The kernels, the Lab server and any running +tool survive, because that is what "background app" means and because closing a +window is not a decision to abandon a running experiment. Quitting is explicit, +from the tray menu, and it is the only path that takes the process down. + +**Quit asks when work is in flight.** Two independent sources of "busy", because +there are two kinds of kernel here and neither can see the other: JupyterLab's +own kernels, which only Lab knows about and which are read over its REST API, +and this app's subprocesses -- `nb verify` on a fresh kernel, a preflight, a +report build -- which only `ui/tasks.py` knows about. Losing either to a stray +click on Quit costs real GPU minutes. +""" + +from __future__ import annotations + +import asyncio +import json +import logging +import socket +import threading +import urllib.error +import urllib.request +from typing import Any, Callable + +log = logging.getLogger("grad.ui") + +#: Where port selection starts. Matches the historical default, so an existing +#: Lab server's recorded origin still matches on the common path. +DEFAULT_PORT = 8080 +#: How far to walk before giving up. Twenty consecutive busy ports is a machine +#: with a problem, not a machine that needs a twenty-first probe. +PORT_SPAN = 20 +#: Lab's REST API is local and already running; anything slower than this is a +#: server that is wedged, and a quit prompt must not hang on it. +_LAB_TIMEOUT_S = 1.5 + +#: Set by `run` so callbacks arriving on the tray thread can reach the UI loop. +_loop: asyncio.AbstractEventLoop | None = None +#: Set by the shell. Shows the "something is running" dialog on a live client. +_confirm_quit: Callable[[dict[str, Any]], Any] | None = None +_tray: Any = None +_quitting = threading.Event() + + +# --------------------------------------------------------------------------- +# ports +# --------------------------------------------------------------------------- +def port_is_free(port: int) -> bool: + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as probe: + try: + probe.bind(("127.0.0.1", port)) + except OSError: + return False + return True + + +def choose_port(preferred: int | None = None) -> int: + """The first free port at or above `preferred`. See the module docstring. + + An explicitly requested port is honoured even when it looks busy: `--port` + is someone overriding this function on purpose, and second-guessing them + would move the app somewhere they did not ask for and did not expect. + """ + if preferred is not None: + return preferred + for candidate in range(DEFAULT_PORT, DEFAULT_PORT + PORT_SPAN): + if port_is_free(candidate): + return candidate + # Nothing free in the span. Hand back the base and let `ui.run` produce the + # bind error, which names the port and is a better message than ours. + return DEFAULT_PORT + + +def origin(port: int) -> str: + return f"http://127.0.0.1:{port}" + + +# --------------------------------------------------------------------------- +# what is running +# --------------------------------------------------------------------------- +def _lab_busy() -> list[str]: + """Kernel ids Lab reports as executing. `[]` when Lab is not up. + + Read over Lab's REST API rather than by inspecting kernels directly: Lab + owns these, their connection files are its business, and `execution_state` + is exactly the field being asked about. + """ + from tools import lab as lab_tool # noqa: PLC0415 + + try: + state = lab_tool.lab_state() + except Exception: # noqa: BLE001 - a quit prompt must not fail on this + return [] + if not state.get("running") or not state.get("port"): + return [] + url = f"http://127.0.0.1:{int(state['port'])}/api/kernels" + request = urllib.request.Request(url) # noqa: S310 - fixed local scheme + token = state.get("token") + if token: + request.add_header("Authorization", f"token {token}") + try: + with urllib.request.urlopen(request, timeout=_LAB_TIMEOUT_S) as response: # noqa: S310 + kernels = json.loads(response.read().decode("utf-8")) + except (urllib.error.URLError, OSError, ValueError, TimeoutError): + return [] + if not isinstance(kernels, list): + return [] + return [ + str(k.get("name") or k.get("id")) + for k in kernels + if isinstance(k, dict) and k.get("execution_state") == "busy" + ] + + +def busy_report() -> dict[str, Any]: + """Everything that would be interrupted by quitting right now.""" + from ui import tasks as tasks_mod # noqa: PLC0415 + + try: + local = [t.label for t in tasks_mod.running()] + except Exception: # noqa: BLE001 + local = [] + kernels = _lab_busy() + return {"kernels": kernels, "tasks": local, "busy": bool(kernels or local)} + + +def busy_sentence(report: dict[str, Any]) -> str: + """One line naming what is running, for the confirmation dialog.""" + parts: list[str] = [] + kernels = report.get("kernels") or [] + tasks = report.get("tasks") or [] + if kernels: + noun = "kernel is" if len(kernels) == 1 else "kernels are" + parts.append(f"{len(kernels)} Lab {noun} executing a cell") + if tasks: + noun = "command" if len(tasks) == 1 else "commands" + parts.append(f"{len(tasks)} {noun} still running ({', '.join(tasks[:3])})") + return " and ".join(parts) if parts else "Nothing is running." + + +# --------------------------------------------------------------------------- +# the window +# --------------------------------------------------------------------------- +def _window() -> Any: + """The pywebview window, or None in browser mode.""" + try: + from nicegui import app as nicegui_app # noqa: PLC0415 + + return getattr(nicegui_app.native, "main_window", None) + except Exception: # noqa: BLE001 + return None + + +def show_window() -> bool: + window = _window() + if window is None: + return False + try: + window.show() + window.restore() + except Exception: # noqa: BLE001 - a window that will not raise is not fatal + log.debug("could not raise the window", exc_info=True) + return False + return True + + +def hide_window() -> None: + window = _window() + if window is None: + return + try: + window.hide() + except Exception: # noqa: BLE001 + log.debug("could not hide the window", exc_info=True) + + +#: The separate Lab window, once opened. One at a time; reopening focuses it. +_lab_window: Any = None + + +def native_available() -> bool: + """Whether there is a desktop window to open a second one beside.""" + return _window() is not None + + +def open_lab_window(url: str) -> bool: + """Show JupyterLab in a window of its own. Returns whether one is up. + + **This is the fix for the workspace feeling slow with Lab open.** As an + iframe, Lab shares a renderer process with the shell: one main thread laying + out a whole JupyterLab document *and* the pane tree, so Lab's work and + Grad's block each other, and the overlay in `ui/static/tiling.js` has to + measure a layout containing all of it on every reflow. A second webview is + a second renderer process -- Lab can be as busy as it likes and the + workspace keeps its frame rate. + + What is given up is the seam: Lab is no longer visually inside the pane. The + notebook window keeps the chrome that matters -- the verify banner, which is + the only source of citable state -- so nothing that gates a claim moves. + """ + global _lab_window + + if _lab_window is not None: + try: + # Navigated, not merely raised. Selecting a different notebook and + # clicking again used to show the window still displaying the old + # one -- and if that notebook's kernel had since been culled, what + # you got was a Lab sitting on a dead connection, which reads as + # "the Lab window loses its kernel" rather than as "this window was + # never told to move". + if getattr(_lab_window, "get_current_url", None) and url != _lab_window.get_current_url(): + _lab_window.load_url(url) + _lab_window.show() + _lab_window.restore() + return True + except Exception: # noqa: BLE001 - destroyed windows raise; fall through + _lab_window = None + try: + import webview # noqa: PLC0415 + except ImportError: + return False + try: + window = webview.create_window( + "Grad — JupyterLab", url, width=1280, height=900, resizable=True + ) + except Exception: # noqa: BLE001 - browser mode, or a backend that refuses + log.exception("could not open the Lab window") + return False + + def _forget() -> None: + global _lab_window + + _lab_window = None + + try: + window.events.closed += _forget + except Exception: # noqa: BLE001 - only costs us a stale handle + log.debug("could not track the Lab window's close", exc_info=True) + _lab_window = window + return True + + +def lab_window_open() -> bool: + return _lab_window is not None + + +def bind_confirm(callback: Callable[[dict[str, Any]], Any]) -> None: + """Registered by the shell: shows the quit confirmation on a live client.""" + global _confirm_quit + + _confirm_quit = callback + + +# --------------------------------------------------------------------------- +# quitting +# --------------------------------------------------------------------------- +def shutdown() -> None: + """Take the app down. The only path that ends the process. + + Lab is deliberately *not* stopped here. It is a detached server with its own + lifetime -- see `core/spawn.py` -- and a user who quit the workspace has not + necessarily finished with the notebook they left running in it. `tools/lab.py + stop` is how it ends. + """ + if _quitting.is_set(): + return + _quitting.set() + from core import instance # noqa: PLC0415 + + instance.release() + if _tray is not None: + try: + _tray.stop() + except Exception: # noqa: BLE001 + log.debug("tray would not stop", exc_info=True) + try: + from nicegui import app as nicegui_app # noqa: PLC0415 + + nicegui_app.shutdown() + except Exception: # noqa: BLE001 + log.debug("nicegui shutdown failed", exc_info=True) + + +def request_quit() -> None: + """Quit, asking first when something is running. + + Callable from the tray thread, which is why the dialog is dispatched onto + the UI loop rather than opened here: NiceGUI elements belong to the loop + that built them, and building one from pystray's thread would either be + ignored or corrupt the client's element tree. + """ + report = busy_report() + if not report["busy"] or _confirm_quit is None or _loop is None: + shutdown() + return + show_window() + try: + asyncio.run_coroutine_threadsafe(_ask_then_quit(report), _loop) + except Exception: # noqa: BLE001 - if the ask cannot be staged, do not quit + log.exception("could not raise the quit confirmation") + + +async def _ask_then_quit(report: dict[str, Any]) -> None: + if _confirm_quit is None: + return + try: + confirmed = _confirm_quit(report) + if asyncio.iscoroutine(confirmed): + confirmed = await confirmed + except Exception: # noqa: BLE001 + log.exception("quit confirmation failed") + return + if confirmed: + shutdown() + + +# --------------------------------------------------------------------------- +# the notification-area icon +# --------------------------------------------------------------------------- +def _icon_image(size: int = 64) -> Any: + """The tray glyph: the nabla, ink on the brand yellow. + + Drawn rather than loaded so there is no image file to lose track of in a + packaged install, and because a tray icon is sixteen logical pixels of flat + colour -- exactly what this design language already is. + """ + from PIL import Image, ImageDraw # noqa: PLC0415 + + ink, paper = (20, 16, 12), (255, 212, 0) + image = Image.new("RGBA", (size, size), (*paper, 255)) + draw = ImageDraw.Draw(image) + draw.rectangle([0, 0, size - 1, size - 1], outline=ink, width=max(2, size // 16)) + inset = size * 0.26 + draw.polygon( + [(inset, inset), (size - inset, inset), (size / 2, size - inset)], + fill=ink, + ) + return image + + +def write_icon(path: str) -> str: + """Save the mark as a multi-resolution `.ico`, for the installer's shortcut. + + Here rather than in the installer so the shortcut and the notification area + cannot drift apart -- there is one drawing of this glyph and both read it. + Windows picks a size per context (16px in the taskbar, 32px on the desktop, + 256px in the large-icon view), and an `.ico` carrying only one of them gets + scaled into the others. + """ + from pathlib import Path as _Path # noqa: PLC0415 + + target = _Path(path) + target.parent.mkdir(parents=True, exist_ok=True) + sizes = [(n, n) for n in (16, 24, 32, 48, 64, 128, 256)] + _icon_image(256).save(target, format="ICO", sizes=sizes) + return str(target) + + +def start_tray(*, on_restart_lab: Callable[[], Any] | None = None) -> Any: + """Put Grad in the notification area. Returns the icon, or None. + + Optional by construction: `pystray` is in the `ui` extra, but a machine + without a working tray (a bare Windows Server session, most Linux desktops + without an AppIndicator host) must still get a usable app. The only thing + lost is the way back from a hidden window, so `hide_to_tray` refuses to hide + when this returned None. + """ + global _tray + + try: + import pystray # noqa: PLC0415 + except ImportError: + log.info("pystray is not installed; the app will not show in the notification area") + return None + + def _menu() -> Any: + items = [ + pystray.MenuItem("Open Grad", lambda: show_window(), default=True), + ] + if on_restart_lab is not None: + items.append(pystray.MenuItem("Restart Lab for this window", lambda: on_restart_lab())) + items.append(pystray.Menu.SEPARATOR) + items.append(pystray.MenuItem("Quit Grad", lambda: request_quit())) + return pystray.Menu(*items) + + try: + icon = pystray.Icon("grad", _icon_image(), "Grad", _menu()) + # `run_detached` would be the tidier call, but it is not implemented on + # every backend; a daemon thread around `run` works on all of them and + # dies with the process either way. + threading.Thread(target=icon.run, name="grad-tray", daemon=True).start() + except Exception: # noqa: BLE001 - see the docstring + log.exception("could not start the tray icon") + return None + _tray = icon + return icon + + +def has_tray() -> bool: + return _tray is not None + + +def hide_to_tray() -> bool: + """Hide the window, if there is a way back to it. + + Returns whether it hid. Without a tray icon this refuses: a hidden window + with no icon and no taskbar entry is an app that is running, consuming a + port, holding the single-instance lock, and unreachable by any means short + of Task Manager. + """ + if not has_tray(): + return False + hide_window() + return True + + +def bind_loop(loop: asyncio.AbstractEventLoop) -> None: + global _loop + + _loop = loop diff --git a/ui/kit.py b/ui/kit.py index 4369423..d07c649 100644 --- a/ui/kit.py +++ b/ui/kit.py @@ -313,6 +313,93 @@ def blink_caret() -> Any: return el("span", "grad-caret") +class Menu: + """A dialog whose body is rebuilt each time it opens. + + `ui.dialog` builds its contents once. These menus list projects, folders, + open windows and stored sessions, and all four change *because of* what the + dialog does -- create a project and the list it was read from is already + stale, open a window and the mark beside its name is wrong. Redrawing on open + is cheaper than binding every row to the poll, and it cannot go stale between + the click and the dialog appearing. + + `draw` is handed the menu so a control *inside* it can call `redraw` after + changing what the menu is listing -- which is what lets the window menu stay + open across several toggles instead of closing after each one. + + It lives here rather than in `ui/shell.py`, where it was written, because the + chat window's session picker is the fourth of these: a Quasar `select` was + the one control in the workspace still carrying NiceGUI's own look, and it + could not say the two things a session row has to say -- that reopening one + only redisplays it, and that another window already has it. + """ + + def __init__(self, dialog: Any, draw: Any) -> None: + self._dialog = dialog + self._draw = draw + + def open(self) -> None: + self.redraw() + self._dialog.open() + + def redraw(self) -> None: + self._draw(self) + + def close(self) -> None: + self._dialog.close() + + +def menu(draw: Callable[[Any, Any], None], *, width: int = 460) -> Menu: + """A `Menu` over a card in the app's own paper, ready to be filled. + + `draw` is called with `(body, menu)` each time it opens; it is expected to + clear the body itself, because a redraw from inside the menu is the same + call. + """ + ui = _ui() + with ui.dialog() as dialog, el("div", "grad-app"): + body = el("div", "grad-card", style=f"background: var(--grad-paper); min-width: {width}px") + return Menu(dialog, lambda m: draw(body, m)) + + +def menu_row( + mark: str, + name: str, + hint: str, + *, + open: bool = False, + title: str = "", + wide: bool = False, + disabled: bool = False, +) -> Any: + """One row of a menu: a mark, a name, and what the row is for. + + A `