Method call static analysis - #137
Open
benh wants to merge 24 commits into
Open
Conversation
benh
force-pushed
the
caller-static-analysis-3
branch
7 times, most recently
from
August 20, 2026 03:17
bf4cb0c to
b731827
Compare
rileysdev
force-pushed
the
riley/reboot-inspect
branch
from
August 20, 2026 07:11
0a7c6b1 to
2d5d1b6
Compare
benh
commented
Aug 20, 2026
benh
force-pushed
the
caller-static-analysis-3
branch
3 times, most recently
from
August 20, 2026 20:50
477ea26 to
515b226
Compare
rileysdev
force-pushed
the
riley/reboot-inspect
branch
2 times, most recently
from
August 21, 2026 01:16
69596bb to
60d3d63
Compare
benh
force-pushed
the
caller-static-analysis-3
branch
2 times, most recently
from
August 22, 2026 19:22
36daa6c to
06ab732
Compare
rileysdev
force-pushed
the
riley/reboot-inspect
branch
from
August 24, 2026 19:14
60d3d63 to
8d42a32
Compare
benh
force-pushed
the
caller-static-analysis-3
branch
2 times, most recently
from
August 24, 2026 23:08
17a9e99 to
f4b6885
Compare
rileysdev
force-pushed
the
riley/reboot-inspect
branch
4 times, most recently
from
August 25, 2026 00:48
eedbe2f to
c13f688
Compare
benh
marked this pull request as ready for review
August 25, 2026 00:50
Contributor
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
rileysdev
approved these changes
Aug 25, 2026
aviator-app
Bot
force-pushed
the
riley/reboot-inspect
branch
from
August 25, 2026 01:47
c13f688 to
7359802
Compare
rileysdev
force-pushed
the
riley/reboot-inspect
branch
from
August 25, 2026 01:47
7359802 to
d1a0eda
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Comment on lines
+775
to
+784
| HOWS_BY_CLASS_NAME = { | ||
| '_ConstructIdempotently': Call.How.CONSTRUCT, | ||
| '_Forall': Call.How.FORALL, | ||
| '_Idempotently': Call.How.CALL, | ||
| '_Schedule': Call.How.SCHEDULE, | ||
| '_SelfIdempotently': Call.How.CALL, | ||
| '_SelfSchedule': Call.How.SCHEDULE, | ||
| '_Spawn': Call.How.SPAWN, | ||
| '_Until': Call.How.UNTIL, | ||
| } |
Contributor
There was a problem hiding this comment.
_Reactively is missing from this table, so reactive calls made by a servicer method are dropped from the analysis entirely — recorded neither in calls nor in ambiguous.
The generator emits class _Reactively: nested directly inside WeakReference (see reboot/templates/reboot.py.j2#L5121-L5123), at the same nesting level as _Idempotently and _Schedule, and its stubs are async def <Method>(__this__, __context__, ...) — exactly the shape _takes_context_second recognizes. Confirmed in the golden: tests/reboot/greeter_rbt.golden.py#L21611-L21614.
Because _generated_definitions only descends into nested classes whose name is in HOWS_BY_CLASS_NAME, no MethodDefinition is ever recorded for _Reactively bodies. In _analyze_function, the call then takes the _rbt.py branch, the match definition: finds no MethodDefinition case, and falls through to the unconditional continue — ambiguous.append(...) is never reached, since that only happens when location is None or helper is None.
This is reachable from inside a servicer method: __context__ is annotated ExternalContext | ReaderContext | WorkflowContext, and the repo already has servicer-side examples, e.g. tests/reboot/bank.py#L970-L973:
async def test_reactive_method_call(self, context: ReaderContext, request: Empty) -> Empty:
account = Account.ref(self.state.account_ids[0])
async for _ in account.reactively().balance(context):Suggested fix: add a REACTIVELY value to ServicerInfo.Method.Call.How in rbt/dashboard/v1/dashboard.proto and map '_Reactively' to it here. (Mapping it to the existing Call.How.CALL would at least stop the silent drop, but loses the distinction.) Either way the names_by_how assertion in tests/reboot/dashboard/implementation_watcher_tests.py will need updating — its current expectation of {CALL, SCHEDULE, SPAWN, FORALL, UNTIL, CONSTRUCT} is itself evidence that reactive calls produce nothing today, and the test fixture's GENERATED template doesn't model _Reactively at all.
`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
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
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
benh
force-pushed
the
caller-static-analysis-3
branch
from
August 25, 2026 18:05
211ef36 to
a13a4e1
Compare
Current Aviator status
This pull request is currently open (not queued). How to mergeTo merge this PR, comment
See the real-time status of this PR on the
Aviator webapp.
Use the Aviator Chrome Extension
to see the status of your PR within GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For every method of a servicer, statically analyze it to determine what method calls it makes.