Skip to content

compass(tests): pin RAPIDSERVE_RPCS to the RapidServe cores waited broadcasts - #396

Merged
jgong5 merged 2 commits into
feature/atomcompass_newfrom
compass/issue-394
Sep 24, 2026
Merged

jgong5 merged 2 commits into
feature/atomcompass_newfrom
compass/issue-394

Conversation

@jgong5

@jgong5 jgong5 commented Sep 24, 2026

Copy link
Copy Markdown
Owner

Closes #394

What changed

One test in tests/compass/test_rapidserve_runner_config.py, +19 lines. test_runner_rpc_surface.py is not touched.

test_the_rpcs_named_are_the_waits_only_the_rapidserve_cores_make asserts:

set(config_module.RAPIDSERVE_RPCS) == waited - surface.BASE == rapid_only

It uses test_runner_rpc_surface.py's SITES, BASE, RAPID, ENGINE and _classes through a module import (import test_runner_rpc_surface as surface). test_runner_step_semantics.py already imports SITES from that module the same way. There is no second scan of the source:

  • waited is the names in SITES whose site is in engine_core.py, has waits, and has a line inside PrefillEngineCore or DecodeEngineCore. That is the refusal message's claim that "the RapidServe engine cores call ... and wait for each reply".
  • rapid_only is (set(SITES) & RAPID) - BASE. This is the reviewer's form: the broadcast names RapidServeModelRunner defines and ModelRunner does not.

Each half catches a mutant the other half misses (M3/M3b and M4 below).

Named result: mutants

Run on xiaobizh_n18_cpu on node 18, against the staged branch tree at 52551552a (atom.__file__ under the staged root). Each mutant edits one file in a copy of the tree, and the file is restored and cmp-checked afterwards. The whole file is run, 8 ids.

mutant file result the comparison that fails
M0: unmutated none 8 passed none
M1: drop "prefill_forward" from RAPIDSERVE_RPCS atom/config.py 1 failed, 7 passed RAPIDSERVE_RPCS == waited - BASE: extra in right 'prefill_forward'
M2: add "_bind_kv_cache_to_modules" (a RapidServeModelRunner method that is never broadcast) atom/config.py 1 failed, 7 passed RAPIDSERVE_RPCS == waited - BASE: extra in left
M2b: add a made-up name "never_broadcast" atom/config.py 1 failed, 7 passed RAPIDSERVE_RPCS == waited - BASE: extra in left
M3: plant call_func("drain_prefill_stream_pool", wait_out=True) in PrefillEngineCore copy of engine_core.py 1 failed, 7 passed RAPIDSERVE_RPCS == waited - BASE: extra in right 'drain_prefill_stream_pool'
M3b: the same with drain_decode_stream_pool in DecodeEngineCore copy of engine_core.py 1 failed, 7 passed RAPIDSERVE_RPCS == waited - BASE: extra in right
M4: plant call_func("_bind_kv_cache_to_modules", wait_out=True) in the base EngineCore copy of engine_core.py 1 failed, 7 passed waited - BASE == rapid_only: extra in right '_bind_kv_cache_to_modules'
C1 (control): plant an unwaited call_func("drain_prefill_stream_pool") in PrefillEngineCore copy of engine_core.py 8 passed, as intended none. The message names only waited calls.

The only failing id in every red row is the new test.

  • The pin is not vacuous. RAPIDSERVE_RPCS has 7 names, so an empty derivation fails the comparison. Every red row above shows it biting.
  • Why both halves are kept. Each half was evaluated alone on the same mutated copies. The reviewer's form alone holds on M3 and M3b, because the planted name is not a RapidServeModelRunner method. The engine-core half alone holds on M4. Both hold on M0.
  • Before this PR, M1 gave 7 passed (the compass(config): refuse enable_rapidserve with a runner that cannot answer its engine cores #386 review's X2).

Gate 1: the merged tree

git merge-tree --write-tree f89b1492c 52551552a gives tree 6f141d065. It was staged into xiaobizh_n18_cpu with git archive and docker exec -i ... tar -x at a private path, with the .compass-commit/.compass-changed stamps. It was run with the tree's own scripts/compass/gate_cpu.sh, and the gate printed atom: /tmp/i394/merged/ATOM/atom/__init__.py and commit: 5e40d0289 (stamp).

tree passed skipped xfailed GATE_CPU_RC
tip 10a664256 (brief) 5275 155 3 0
merged f89b1492c + 52551552a 5276 155 3 0
  • f89b1492c differs from 10a664256 only in atom/compass/AI_DEV_RULES.md, which no test or gate script reads.
  • The node-id delta is from --collect-only on both staged trees, with the gate's own ignore list: tip f89b1492c 5412, merged 5413. The only difference is + tests/compass/test_rapidserve_runner_config.py::test_the_rpcs_named_are_the_waits_only_the_rapidserve_cores_make, which passed in the gate's junit.

🤖 Generated with Claude Code

jgong5 and others added 2 commits September 24, 2026 02:08
…roadcasts (#394)

`RAPIDSERVE_RPCS` must equal two sets derived from source by
`test_runner_rpc_surface.py`'s `SITES`, `BASE` and `RAPID`:

- the names `PrefillEngineCore` and `DecodeEngineCore` broadcast with a
  wait, minus the names `ModelRunner` defines;
- the broadcast names `RapidServeModelRunner` defines, minus the names
  `ModelRunner` defines.

A name dropped from or added to the tuple, or a new waited broadcast in
either RapidServe core, now fails the test.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
for name, sites in surface.SITES.items()
for s in sites
if s.waits
and s.file == "engine_core.py"

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking. An observation, not a change request: s.file is a basename, so a second engine_core.py is filtered by this file's class line ranges.

Site.file is path.name (test_runner_rpc_surface.py, _call_sites), and the tree has two files with that name: atom/model_engine/engine_core.py and atom/diffusion/engine/engine_core.py. This filter passes sites from both, then checks their line numbers against PrefillEngineCore/DecodeEngineCore in the first one.

Measured (X4, xiaobizh_n18_cpu, copy of merged tree 265770218). I appended 700 comment lines to atom/diffusion/engine/engine_core.py and then a module-level call_func("diffusion_only_rpc", wait_out=True), which lands at line 939, inside PrefillEngineCore's range. Result: 1 failed, 7 passed, failing on this test at :69. That red is false, because no RapidServe core makes that call.

It is latent today. The diffusion file has 235 lines and no call_func, so it would have to grow past line 816 before this could fire. The precise fix, carrying the relative path in Site, is in test_runner_rpc_surface.py, outside this PR's file set, and the false red is loud rather than silent. So I am not asking for a change and not filing an issue. Principle 6 ("Refuse rather than fall back. A declined answer with a named reason is a result. A guessed one is a defect.") is the reason to record it: file identity is guessed from the basename here.

@jgong5

jgong5 commented Sep 24, 2026

Copy link
Copy Markdown
Owner Author

Review cycle 1: PR #396 (issue #394), head 52551552af5c0b3e2bc528c2d21bc6884886996c

Written by a reviewer agent.

Verdict: APPROVE 52551552a. Nothing is blocking. There is one non-blocking finding, about the PR body (F1 below), and one inline observation at :65 that asks for no change (#396 (comment)).

I read the eight design principles in atom/compass/design/README.md and AI_DEV_RULES.md at 03e20031f before the diff. Everything below was measured on xiaobizh_n18_cpu (node 18), on trees staged with git archive at /tmp/r396rv1. Every run confirmed atom.__file__ under the staged root.

1. The changed lines

  • How waited decides "inside". It uses lineno <= s.line <= end_lineno over the ClassDef nodes from _classes(ENGINE / "engine_core.py"). That is lexical, so it behaves as follows:

    • A nested def counts (X2 is red).
    • A decorator line sits above ClassDef.lineno, so it does not count. No broadcast lives in a decorator.
    • A subclass defined after DecodeEngineCore does not count (X3 stays green), nor does a helper outside the classes. The rapid_only half covers both as soon as RapidServeModelRunner defines the name (X1 and X3b are red). If no runner defines it, test_runner_rpc_surface.py::test_the_dispatched_names_outside_the_surface_belong_to_other_runners goes red (X1b).
    • The one real imprecision is the basename match at :65. See the inline comment. It is latent and gives a false red, never a silent pass.
  • The docstring is true of the assertion. Its first clause is lexical ("broadcast" from those class bodies). Calls a core inherits from EngineCore are covered by the second clause, which is exactly the M4 case.

  • The cross-test import has no coupling problem. Measured on merged tree 265770218:

    run result
    new test alone 1 passed
    file alone 8 passed
    surface, then file 77 passed
    file, then surface 77 passed
    step_semantics, file, surface 94 passed
    • A probe test asserted test_rapidserve_runner_config.surface is test_runner_rpc_surface and test_runner_step_semantics.SITES is test_runner_rpc_surface.SITES. sys.modules holds one key, test_runner_rpc_surface, so the source scan runs once per session.
    • Cost. Collecting this file alone went from 0.12 s to 3.11 s, because importing surface scans atom/. -X importtime gives 4.3 s cumulative for that import. In the gate the module is imported anyway, so the gate pays nothing extra.
  • ruff 0.16.7: check and format --check are clean on the file.

2. Mutants

I reproduced three of the developer's mutants and designed eight of my own (X6 is in section 3). Each one edits a copy of merged tree 265770218, and every touched file is restored and checked with cmp. "Red" means 1 failed, 7 passed, with the failing id tests/compass/test_rapidserve_runner_config.py::test_the_rpcs_named_are_the_waits_only_the_rapidserve_cores_make and the AssertionError at :69.

id mutant result
M0 unmutated (run first and last) 8 passed
R1 = dev M1 drop "prefill_forward" from RAPIDSERVE_RPCS red
R2 = dev M2b add "never_broadcast" red
R3 = dev M4 base EngineCore waits on _bind_kv_cache_to_modules red
X1 RapidServeModelRunner.drain_stream_pool, plus a waited broadcast from a module-level helper after DecodeEngineCore red (rapid_only half)
X1b the same helper broadcast, with no runner defining the name (file + surface) new test green; surface ...belong_to_other_runners red, 8 == 7
X2 waited broadcast in a nested def inside PrefillEngineCore._init_disagg red
X3 a subclass FastPrefillEngineCore(PrefillEngineCore) defined after DecodeEngineCore, with a waited broadcast of a new name 8 passed (outside the line ranges)
X3b X3, plus RapidServeModelRunner defines the name red (rapid_only half)
X4 waited broadcast at line 939 of a padded atom/diffusion/engine/engine_core.py red, and false (inline comment at :65)
X5 call_func_with_aggregation("drain_stream_pool") in DecodeEngineCore red (aggregation counts as waited)

The helper candidate from the brief (X1, X1b). The waited half does not see a helper, and it should not: the helper is not in a core's body. The pin still sees it through rapid_only whenever the name is a RapidServe method. When it is not, the surface file's leftover count sees it.

3. The double equality: justified, keep both halves

I evaluated each half alone, on the same mutants:

mutant both halves == rapid_only only == waited - BASE only
X6: a RapidServeModelRunner method broadcast without wait_out, and the constant updated to name it red 8 passed red
M4, file + surface red red new test passes; only surface's count goes red (8 == 7)
M3, file + surface red new test passes; surface red (8 == 7) red
  • The refusal message makes two claims: the cores "call ... and wait for each reply", and "only RapidServeModelRunner defines them". Each half pins one of them.
  • rapid_only alone lets the message name an RPC nobody waits on (X6).
  • waited alone misses a waited call a core inherits from EngineCore (M4). Once someone bumps the surface file's count from 7 to 8, nothing holds the constant.

F1. Non-blocking, about the PR body. Principle 8: "Every claim carries its measurement. A number without a source is a defect." The body credits the waited half with M3/M3b. At suite level, M3 is already red in test_runner_rpc_surface.py::test_the_dispatched_names_outside_the_surface_belong_to_other_runners (assert (7 == 7 and 8 == 7)) when only the rapid_only form is kept, so M3 is not what justifies that half. X6 is the mutant only the waited half holds. Please cite X6 in the squash message or the handoff instead of M3. This needs no code change.

4. ponytail-review

  • L58-67: the waited comprehension stays. X6 shows it is load-bearing, and it is already one comprehension.
  • L68-69: the double equality stays, as argued in section 3.
  • L9: the module import reuses the derivation, as the brief asked. It is not a second scan.

Lean already. Ship.

5. Gate 1: the merged tree

tree passed skipped xfailed GATE_CPU_RC
merged 265770218 5275 156 3 0
  • The one skip over the developer's 155 is the known noise guard. It is TestTheRegionIsNotCopiedPerChunk::test_no_format_pays_more_per_byte_as_the_payload_grows[kimi-incremental], skipped with "machine too noisy to measure: control ratio 0.59". I re-ran the class twice on the same tree with no other gate running: 4 passed both times. That gives 5276 passed, as expected. The new id passed in the gate's junit.
  • The tip moved again during review, to b7cd11d48 (compass(tests): say the refusal comment is what the exit test holds against ModelRunner.exit #399). That commit is docstring-only in test_runner_rpc_surface.py, in the docstring of test_what_the_comment_says_a_hole_at_exit_loses_is_what_exit_does. The merged tree is now 0eb26779e. It differs from the gated 265770218 only in those 2 docstring lines, and SITES, BASE, RAPID and _classes are unchanged.

🤖 Generated with Claude Code

jgong5 added a commit that referenced this pull request Sep 24, 2026
…unner, in overrides.py and design 01 (#397)

Since #386, Config raises ValueError when enable_rapidserve is set with
a runner_qualname not in RAPIDSERVE_RUNNERS. Two passages still
described that combination as reachable:
- the Scope comment above RPC_SURFACE in atom/compass/runner/overrides.py
  ("seven silent parks ... nothing in this module closes that gap");
- the time.sleep(2) entry in design 01 ("a simulated runner plus that
  flag would reach the sleep").

Both now say Config refuses the combination and name
RAPIDSERVE_RUNNERS. The overrides.py change is comment-only; its AST is
identical with comments and docstrings masked.

Named-result check: with the design doc's wrapped lines joined, "would
reach the sleep" occurs once at the base and zero times at the head, and
"silent parks" zero times in both files. A line-by-line grep cannot see
the wrapped phrase at either end.

Ordering: the review's probe keeps the real engine_core_mgr and records
engine-core construction and multiprocessing.Process. It shows
LLMEngine raises at Config (llm_engine.py:43) before any engine core is
constructed. A mutant that builds a PrefillEngineCore before Config
shows up in that probe. The developer's manager-level stub observes only
that neither core manager is called.

Gate (node 18, CPU tier, combined with #396 on b7cd11d): 5276 passed,
155 skipped, 3 xfailed, GATE_CPU_RC=0.

Closes #387

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@jgong5
jgong5 marked this pull request as ready for review September 24, 2026 02:54
@jgong5
jgong5 merged commit b80ee8a into feature/atomcompass_new Sep 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant