compass(docs): drop the redundant RapidServe ordering clause and agree the substitution citations - #404
Conversation
…e the substitution citations Drop, from the overrides.py comment and design 01, the clause saying the Config refusal happens before any engine core is built, and the config.py:1737-1745 and llm_engine.py:43 citations. "Config raises" already implies the ordering, and each citation points at code the sentence already names (RAPIDSERVE_RUNNERS, Config). Cite the RapidServe runner substitution as config.py:1730-1736 everywhere (it was 1727-1736, 1729-1736 and 1730-1736), naming Config.__post_init__ where the sentence had only a line range. Correct the runner_qualname read in engine_core.py from :129 to :128. The overrides.py AST is unchanged with comments and docstrings masked. Closes #400 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Review cycle 1: PR #404 (issue #400), head Agent-authored review. I read the eight design principles in Verdict: APPROVE at 1. Citation checks, resolved at tip
|
| Where (head) | Citation | Code at the tip | Result |
|---|---|---|---|
| 01:49, 01:1417 | engine_core.py:128 |
:128 is config.runner_qualname,, the third argument of self.runner_mgr = AsyncIOProcManager( (:125). :129 is config,. |
exact |
| 01:51, 01:944, 02:40, overrides.py:117 | config.py:1730-1736 |
:1730 is if (, :1731 is self.enable_rapidserve, :1732 is and self.runner_qualname == "atom.model_engine.model_runner.ModelRunner", :1733 is ):, :1734 is self.runner_qualname = (, :1735 is "atom.model_engine.model_runner.RapidServeModelRunner", and :1736 is ). :1727-1729 is the comment above, and :1737 starts the RAPIDSERVE_RUNNERS refusal. |
spans exactly the substitution |
| 01:51, 02:39 | Config.__post_init__ |
@dataclass at :1504, then class Config: at :1505 and def __post_init__(self): at :1670. No other def sits between :1670 and :1730. |
correct enclosing method |
I found four sites that cite the substitution, not five. The PR body's table also lists four, while the issue comment says "all five places". Every site reads 1730-1736.
Missed-citation search over the whole head tree (git grep at 1d0811038):
1729-1736|1727-1736|1737-1745|engine_core\.py:129|llm_engine\.py:43\bhas zero hits anywhere, includingtests/andscripts/.config\.py:1[67]NNhas the four sites above, plus15:437config.py:1677-1681, which is a different refusal (megawithout EP).- A search for the ordering phrase (
before (any|either) (engine core|class),constructs any engine core,builds that Config) has zero hits. - Other
engine_core.py:12xcitations:01:120engine_core.py:125names theAsyncIOProcManagerconstruction, and02:37engine_core.py:125-130spans the whole call, which contains:128. Both are correct and outside the brief.
Rule: "Do not write 'checked' unless the check you ran is the one that answers the claim." Principle 8: "Every claim carries its measurement." Each claim is matched to the quote above it.
2. Paragraph re-reads (every edited paragraph, in full)
- 01:48-51 ("The seam needs no ATOM change."): grammatical. The two precedents now read file:range for RLHF and symbol plus range for RapidServe. Both parentheticals resolve.
- 01:934-947 (the struck
time.sleep(2)item). The paragraph ends: "Configkeeps a simulated runner from it:--enable-rapidserveselectsRapidServeModelRunneronly whenrunner_qualnameis still the default (config.py:1730-1736), and otherwiseConfigraisesValueErrorunlessrunner_qualnameis inRAPIDSERVE_RUNNERS. The cost, for a runner that list names, is two real seconds…" It is true as written:- With the default runner, the substitution installs a runner that is in the set (
config.py:2102RAPIDSERVE_RUNNERS = frozenset({"…RapidServeModelRunner"})). - With any other runner,
:1737-1738raises unless the runner is listed. - "Keeps … from it" still needs the ordering, and "
Configraises" carries it:Configis built atllm_engine.py:43before any core. So the removed clause took no truth with it, and nothing dangles.
- With the default runner, the substitution installs a runner that is in the set (
- 01:1416-1418 (Sources, "The seam"): single-token change, grammatical.
- 02:37-41: "
Config.__post_init__(config.py:1730-1736) swaps inRapidServeModelRunnerautomatically" is grammatical and true ("automatically" means underenable_rapidservewith the default qualname, and the range shows the condition). - overrides.py:109-120 (Scope comment): now ends "
Configtherefore raisesValueErrorforenable_rapidserve=Truewith any runner not inRAPIDSERVE_RUNNERS, this one included." It is grammatical and true, since the Compass runner is not in the frozenset.git diff -U0 b80ee8ac4 1d0811038 -- atom/compass/runner/overrides.pyhas 0 changed lines that do not start with#, which is consistent with the developer's masked-AST identity. The comment carries no design-doc references. Rule: "No design-doc references in code or runtime output."
3. ponytail-review
Comment and doc only, 3 files, +11/-13. The diff is itself the ponytail cut #397's review asked for (net: -2). It adds one symbol name at two bare ranges, which the brief requested ("Prefer naming the symbol where the doc's style allows it"). There is nothing further to delete, inline or shrink. Principle 3: "Add only what is necessary, and nothing more."
Lean already. Ship.
4. Gate 1 (node 18, xiaobizh_n18_cpu)
- Tip:
37fba4df0(compass(kv): whole_number Limits name the value, its dtype, or a returned object as where an escape comes from (#390) #391), re-read withgit ls-remotebefore gating and again before posting. - Merged tree:
git merge-tree --write-tree 37fba4df0 1d0811038gives2f3c56c55. That is not the head treefacede62a, because the tip moved. So I gated the merged tree. - Stamp:
git commit-tree 2f3c56c55 -p 37fba4df0 -p 1d0811038gives9d793a957, which is not pushed and has no ref..compass-changedholds the 3 PR files. - Staging:
git archiveof the stamp, a tarball, thendocker exec -i … catinto/tmp/r404gate/. The md5eeb563ad…matched on both ends. The shared mount was untouched. - Run: the tree's own
scripts/compass/gate_cpu.shwithtimeout -k 10 3000, unpiped. It started only afterpgrep -fc "[g]ate_cpu.sh"returned 0 (after compass(tests): SITES filters broadcast sites on the bare name engine_core.py, which a diffusion file shares #401's gates cleared).
| tree | commit: |
atom: |
passed | skipped | xfailed | failed | GATE_CPU_RC |
|---|---|---|---|---|---|---|---|
developer, merged = head at b80ee8ac4 |
1d0811038 |
/tmp/i400gate/ATOM/atom/__init__.py |
5276 | 155 | 3 | 0 | 0 |
reviewer, merged at 37fba4df0 |
9d793a957 (stamp) |
/tmp/r404gate/ATOM/atom/__init__.py |
5276 | 155 | 3 | 0 | 0 PASSED |
The run took 187.85 s. None of the three known flakes fired. The gate stands for tip 37fba4df0. If the tip moves again before landing, the lander's tree check applies.
Findings
- N1 (non-blocking, issue thread only). The compass: drop the redundant ordering clause and line citations #397 added, and make design-doc citations of the RapidServe substitution agree #400 delivery comment says the substitution "is cited as
config.py:1730-1736in all five places". There are four sites: 01:51, 01:944, 02:40 and overrides.py:117. The PR body's table correctly lists four. A one-word correction in the handoff comment is enough, and the diff needs no change. Principle 8: "A number without a source is a defect."
Gates 1-3 as they apply: gate 1 is green, as above. Gate 2 does not apply, because the change is comment and doc only and adds no behaviour. Gate 3's named result is met: every touched citation resolves, the masked AST is identical, and the delta is 0. This review is gate 4.
Closes #400
Drops the ordering clause and two line citations that #397 added, and makes the design docs cite the RapidServe runner substitution one way. Comment and doc only: 3 files, +11/-13.
Changes
atom/compass/runner/overrides.py(comment only), design 01 near L944.LLMEnginebuilds thatConfig(llm_engine.py:43) before either class" and "beforeLLMEngine.__init__constructs any engine core".config.py:1737-1745citation. Each passage now ends at "RAPIDSERVE_RUNNERS, this one included." / "is inRAPIDSERVE_RUNNERS."config.py:1730-1736everywhere. It had been cited three ways:1727-1736,1729-1736and1730-1736.Config.__post_init__, following theConfig.runner_qualname(atom/config.py:1595) style used in the same sentences.RapidServeModelRunner,runner_qualnameandConfig, so it only gets the corrected range.runner_qualnameread is now cited asengine_core.py:128, not:129, at 01:49 and 01:1417 (the Sources list repeats the same citation).Citation table (every citation touched, resolved at tip
b80ee8ac4)atom/config.py,atom/model_engine/engine_core.pyandatom/model_engine/llm_engine.pyare identical at the tip and the head.engine_core.py:128(was:129):128isconfig.runner_qualname,, the third argument ofself.runner_mgr = AsyncIOProcManager((:125-130).:129isconfig,.Config.__post_init__,config.py:1730-1736(was1729-1736):1730if (·:1731self.enable_rapidserve·:1732and self.runner_qualname == "atom.model_engine.model_runner.ModelRunner"·:1733):·:1734self.runner_qualname = (·:1735"atom.model_engine.model_runner.RapidServeModelRunner"·:1736). The enclosing def isdef __post_init__(self):at:1670.:1727-1729is the comment above the statement, which is why neither start line was exact.config.py:1730-1736(was1727-1736)Config.__post_init__(config.py:1730-1736) (wasconfig.py:1729-1736)config.py:1730-1736(unchanged, re-checked)01:945, overrides.py:120removedconfig.py:1737-1745:1737if self.enable_rapidserve and self.runner_qualname not in RAPIDSERVE_RUNNERS:. The sentence namesRAPIDSERVE_RUNNERS, andconfig.py:2102defines it.overrides.py:121removed with its clausellm_engine.py:43:43config = Config(model, **config_kwargs)The remaining sentences are still true.
ConfigraisesValueErrorunderenable_rapidserve=Truefor a runner not inRAPIDSERVE_RUNNERS(config.py:1737-1738).RAPIDSERVE_RUNNERSisfrozenset({"atom.model_engine.model_runner.RapidServeModelRunner"}), so the Compass runner is included.No test or script pins any string that changed. I grepped
tests/andscripts/for1729-1736,1727-1736,1737-1745,engine_core.py:129andllm_engine.py:43, and none of them match.Named result
overrides.pyAST is identical once comments and docstrings are masked.ast.dumpofb80ee8ac4:atom/compass/runner/overrides.pycompared with the head, after stripping the leading stringExprfrom every module, class and function body.701ad09fe38cf2ae. Head:701ad09fe38cf2ae.identical: True.RPC_SURFACE: dict[str, bool]todict[str, int]givesidentical: False.Gate 1 (node 18,
xiaobizh_n18_cpu)b80ee8ac4(compass(tests): pin RAPIDSERVE_RPCS to the RapidServe cores waited broadcasts #396), re-read fromfork/feature/atomcompass_newbefore gating.git merge-tree --write-tree b80ee8ac4 1d0811038givesfacede62a, which equals1d0811038^{tree}. The stamp is therefore the head itself:.compass-commit=1d0811038,.compass-changed= the 3 PR files.git archiveinto a tarball,docker exec -i … catinto/tmp/i400gate/inxiaobizh_n18_cpu, md5cc74986f…matching on both ends, thentar -x. The shared mount was not touched.scripts/compass/gate_cpu.sh, run withtimeout -k 10 3000, unpiped, and started only afterpgrep -fc "[g]ate_cpu.sh"returned 0.commit:atom:b80ee8ac4(given baseline)1d08110381d0811038 (stamp)/tmp/i400gate/ATOM/atom/__init__.pyThe delta is 0 on every count. The run took 187.3 s. None of the three known flakes fired, so no re-run was needed.
Dev record
Config.__post_init__only where a citation stood alone as a bare range (01:51, 02:39). At 01:944 and inoverrides.pythe sentence already names the symbols, so they keep only the range. The compass(docs): say that Config refuses enable_rapidserve with a non-RapidServe runner #397 reviewer judged the range to beoverrides.py's house style, since everyRPC_SURFACEentry citesengine_core.py:NNN.engine_core.py:125-130is the wholeAsyncIOProcManager(...)call, and it contains:128.engine_core.py:125cites theAsyncIOProcManagerconstruction, not the read.🤖 Generated with Claude Code