WIP - #137
Conversation
mypy 1.18 rejects `if x is Any:` as `comparison-overlap` when `x` is declared as a union of concrete types. The comparison is correct at runtime: `get_args` on an annotation such as `dict[str, Any]` hands back the `typing.Any` object itself, so the converters meet it as a value. Only the declared parameter types disagree, since they do not mention `Any`. Route the comparison through `is_annotation_any`, whose parameter is typed `object` — the honest domain of what typing introspection returns. The converters' declared unions stay as they are, and the runtime behavior is identical: the same `is` comparison, one call deeper. Until now this error failed the build of every target whose closure reaches `reboot/api.py` under a fresh mypy run, which is how it was found: Bazel's remote cache had been serving stale mypy results, so CI never re-ran mypy over these files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A `description=` was only set on `McpMethodOptions.description`, so a Reader, Writer, Transaction or Workflow that was not also an MCP tool didn't have it. It is now set on `MethodOptions.description` for every method, and MCP tool and resource descriptions read from there. `McpMethodOptions.description` is deprecated but still read as a fallback, so protos that already set it keep their descriptions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The first of several changes, split so that each can be reviewed on its own and so the dashboard can be tried out while the rest is written. It is not a documented feature yet: nothing opens a page by itself, so seeing one means passing `--open-dashboard` or visiting the URL. `rbt dev run` now starts a second Reboot application alongside the developer's, with its own state store, holding what the dashboard needs. The companion watches the developer's `api/` directory and records what those files declare, so the dashboard can describe an application. It also serves the page itself. The page reads that schema reactively and renders one section per state type: its fields, and each method's kind, whether it constructs, whether it is reachable over MCP, its signature and the errors it raises. Dashboard state, such as which detail views are open and which are closed, is saved in Reboot state, so it survives a hot reload and an `rbt dev run` restart. Auto-open is complete but off. `_AUTO_OPEN_DASHBOARD` is False, so only `--open-dashboard` opens a page. We don't reopen the dashboard if the developer already has it open, and we use the `Presence` library to determine whether they do. Note that presence does not drain through a DevPod workstation's port forward, which is filed separately. This will eventually supersede the inspect dashboard at `/__/inspect`, which lists state instances and their values. It does not replace it yet and both exist meanwhile: this describes an application's API, its state types, their fields and their methods, and cannot yet show the data behind them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A method can say what it does, but a state type is the sum of its state and its methods, and its name alone does not say what it is for. `Type` now takes a description, which the dashboard shows beside the state type's name and file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The manylinux images build every CPython with `--disable-shared` and delete even the static `libpython` archives. The `reboot-dev-reboot` genrule links `reboot_native.node` with `-lpython3.10`, a flag emitted by `python3.10-config --ldflags --embed`. That link has never been able to succeed inside these images. CI stayed green only while Bazel's remote cache served the genrule's outputs. The first cache miss made every platform fail deterministically. On x86_64 that miss came from a runner hardware swap: it changed the `lscpu` portion of `the_environment.txt`, and with it the whole cache scope. Point `python`/`python3` at a python-build-standalone CPython 3.10, which ships `libpython3.10.so`. It is the same build `reboot/nodejs/prepare_environment.sh` downloads. `pip`/`pip3` stay on the manylinux interpreter, whose layout `auditwheel` and the wheel builds expect. `python3` and `pip` therefore deliberately name different installations. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`rbt dashboard` needed `--api-directory`, naming a directory the `.rbtrc` already names for `rbt generate`. Two places to say the same thing is two places to change it, and nothing tells you when only one of them moves -- the dashboard just watches a directory the rest of the tooling has stopped using. So it reads what `rbt generate` was told instead, through a new `ArgumentParser.dot_rc_arguments`, which returns what the `.rbtrc` gives any subcommand. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPEVMhyDRxZEuH8eiykv99
Work that waits on nothing -- parsing, hashing, encoding -- never gives the event loop a chance of its own, so a servicer doing it over a collection holds its process for as long as the whole collection takes, and everything else it serves waits that long. `concurrently` is the wrong tool, because there is nothing to overlap. Measured over twelve parses of a 45KB file, it left the loop unable to answer for 24ms at a stretch -- 15ms even limited to one at a time, since its tasks are scheduled together and the loop drains several before looking at anything else -- and cost 30% more wall-clock in task machinery. An `asyncio.sleep(0)` in the loop measures best, at 6ms, but invites the question of why it is there and not somewhere else. This answers it: the yield falls out of how the work was grouped, which is a decision the caller has to make anyway, and the collection bounds it the way `concurrently`'s does. It takes elements rather than awaitables, because nothing is being run: the work stays in the caller's body, where it can go on mutating whatever it likes. Note an `async for` alone will not do -- `await` on something that resolves without suspending never reaches the event loop at all, which is why the yield has to live in here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPEVMhyDRxZEuH8eiykv99
The API files say which state types exist. They say nothing about
which file implements one, and the name does not say either --
`servicers.py` may implement several state types while being named
after none of them. What does say is the application:
Application(servicers=[AccountServicer, BankServicer, ...])
so this reads the entry point, resolves each registered servicer back
to the file defining it, and asks that class what it services.
Read rather than imported. Importing an application means having its
generated code, its dependencies and its `sys.path`, and the dashboard
is meant to work before any of that exists -- the same reason the API
files are read the way they are.
Driven by the API rather than by the filesystem: the API is what says
which state types there are to look for, so a state type appearing or
disappearing is what sets this going. `until_changes` suspends the
workflow in between, so it wakes when the declarations move rather
than on a timer.
Recorded one state per state type, so that working out one state
type's implementation neither waits on nor overwrites another's. A
state type the application registers no servicer for is recorded as
such rather than left looking unanalyzed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QPEVMhyDRxZEuH8eiykv99
The walk parsed every file the application reaches on every save, and an edit changes one of them. It now records what each file was found to hold along with a digest of the bytes it held, and parses one again only when those bytes differ. A digest and not `st_mtime_ns`, which is only as fine as the kernel's coarse clock: measured here, 163 of 200 consecutive rewrites of a file shared an mtime, so a save landing in the same tick as a read would have left that file looking untouched for good. Reading and hashing 50 files costs 1.6ms against 74ms to parse them, so asking exactly is still nearly all of the saving. Reachability is still worked out from the application every time, but over what is already held rather than by parsing: a file that stops being imported drops out however recently it changed, and one that starts being imported is parsed for the first time. A file that will not parse is left unrecorded, so it is tried again on the next save. `File` is where the analysis will attach: it says what a file holds, and asking whether that is still true is the question a hash of each method will answer one level finer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPEVMhyDRxZEuH8eiykv99
Scaffolding for analyzing them: what a servicer is made of, and a digest of each method that says whether analyzing it again would say anything new. Nothing is analyzed yet -- a `Method` is a name and a digest, and what it calls is the field that follows. The digest is over `ast.dump` without attributes, so it is of what the method says rather than how it is laid out: reformatting it, writing a comment in it, or pushing it down the file with an edit above leave it alone. That is what will keep an application of a thousand state types from re-analyzing everything on every save, one level finer than the file digest already does for parsing. Methods are recorded in the order they are written, which is the order somebody reading the file meets them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPEVMhyDRxZEuH8eiykv99
116c2fd to
0b2ace5
Compare
Which state type a servicer services takes type information: the name in front of `.Servicer` may be spelled any way an import can bind a name. So any class extending a dotted name ending in `.Servicer` is a servicer, recorded under the name the developer wrote, and type information will later replace that name with the fully qualified state type. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPEVMhyDRxZEuH8eiykv99
An iteration now carries a `Files` value through everything it does, immutable, like the `Analysis` that carries one file through it: `known` is what the previous iteration analyzed, `parsed` what this one has parsed and not yet analyzed, `analyzed` what it has finished, and `pending` the frontier: every file reached, whose imports are not yet followed, entering once and leaving once. A file depends on the file behind every one of its imports: those are the files that can change what this one means. For now every import is taken as used, since tools like `ruff` keep unused imports out of real code; narrowing to the imports whose names are used can come later if this proves too eager. Each `File` records its dependencies by the digest each had when it was read, and a known file is kept only while its own digest and every dependency's still match; otherwise it is parsed and analyzed again. A digest read once is recorded, so nobody reads the same bytes twice in one iteration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPEVMhyDRxZEuH8eiykv99
A relative import is resolved to a path where it is collected, since that is the one place the importing file's own directory is known. From there it is followed to its file like any other module, spelled as a path, with `os.sep` telling the two spellings apart, and files are deduped by their absolute path since a file can now be reached under two spellings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPEVMhyDRxZEuH8eiykv99
`_read` and `_try_find_file_of` make OS calls, and they were made with the event loop held, so a slow disk stalled every dashboard request for as long as the disk took. Both now go through `aiofiles`, which runs the call in a thread -- the way file operations are done everywhere else in the repo -- and everything between `files()` and the two of them becomes `async` to carry the `await` down. Parsing still holds the interpreter: `ast.parse` is CPU-bound, so no thread frees the loop from it, and `cooperatively` already bounds it to a file at a time. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPEVMhyDRxZEuH8eiykv99
A save landing while an iteration reads produces a torn snapshot: one file read before the save, another after. The watch is armed before anything is read, so the save's event is already waiting when the iteration finishes and the next one begins at once -- where a file kept against a stale dependency digest fails its check and is analyzed again. The digests recorded per dependency are what make the tear detectable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPEVMhyDRxZEuH8eiykv99
bf4cb0c to
b731827
Compare
0a7c6b1 to
2d5d1b6
Compare
| def __init__(self) -> None: | ||
| self._process: Optional[asyncio.subprocess.Process] = None | ||
| self._ids = itertools.count(1) | ||
| self._replies: dict[int, asyncio.Future[Any]] = {} |
There was a problem hiding this comment.
Rename _replies to _responses.
| # For every file handed to the server, the version number the | ||
| # protocol wants counted up per file, and a digest of the | ||
| # text sent, so that handing the text it already has sends | ||
| # nothing. |
There was a problem hiding this comment.
Reword this comment:
For each file that we "sync" to Pyright we keep around a version number (required by Pyright) and a digest (to skip syncing the file if its text has not changed).
| self._process.terminate() | ||
| await self._process.wait() | ||
|
|
||
| async def hand( |
There was a problem hiding this comment.
Rename function to sync.
| filename: str, | ||
| text: str, |
| """Hands the server one file's text, so that answers about | ||
| the file are about exactly this text. Handing the text the | ||
| server already has sends nothing.""" |
There was a problem hiding this comment.
Reword the docstring:
Sync the specified text with Pyright so it uses that text to resolve any subsequent queries. If the text for filename has not changed then nothing is sent to Pyright.
| """Returns where the name at a position in the text the file | ||
| was last handed as is defined, and `None` when pyright does | ||
| not know.""" |
There was a problem hiding this comment.
Let's also have definition_at raise InvalidArgument if filename hasn't been synced.
Also reword this docstring:
Returns the definition for the name at the position in the text of the specified filename, which must have been synced with Pyright already or an InvalidArgument is raised, or None when Pyright can not determine the definition (e.g., it is missing files it needs to find the definition, such as files that might need to be generated by the build system).
| """Returns where the name at a position in the text the file | ||
| was last handed as is defined, and `None` when pyright does | ||
| not know.""" | ||
| reply = await self._request( |
There was a problem hiding this comment.
| reply = await self._request( | |
| response = await self._request( |
| method: str, | ||
| params: dict[str, Any], |
There was a problem hiding this comment.
Please make these kwargs.
| b'Content-Length: %d\r\n\r\n%s' % (len(body), body) | ||
| ) | ||
|
|
||
| async def _read_replies(self) -> None: |
There was a problem hiding this comment.
Rename this meethod to just _read.
| # Server-to-client requests must be answered or the | ||
| # server waits on us the way we wait on it. |
There was a problem hiding this comment.
Update the comment to include that we know that a body is a server-to-client request if it has an id and a method.
477ea26 to
515b226
Compare
69596bb to
60d3d63
Compare
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
515b226 to
36daa6c
Compare
36daa6c to
06ab732
Compare
No description provided.