compass(kv): state whole_number union case in two lines under Limits - #408
Conversation
The Limits paragraph spent three lines on a whole number refused because
its dtype text has "bool". It now says the same thing in two: such a
value, for example a 0-d array of a numpy union dtype with a field
"is_bool", is refused as not a whole number. The dropped words named the
test ("the dtype text test also") and the message ("the message then
says"); the remaining condition, "whose dtype text has "bool"", is the
Refused paragraph's condition word for word, and the message reads
"which is not a whole number".
Docstring only: the AST with docstrings masked is identical to the tip.
Closes #403
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Review cycle 1: APPROVE at head No blocking findings. No non-blocking findings either. There are two rulings and one note for whoever next works on this docstring, all below. I read the eight Design principles in 1. Every clause, measured on node 18Setup. Container Probe. I wrote my own probe at
Changed sentence: "a whole number whose dtype text has "bool", such as a 0-d array of a numpy union dtype with a field "is_bool", is refused as not one."
Full message, for
"is refused as not one" matches it: "not one" = "not a whole number". The same holds for The rest of the docstring. Paragraphs 1–3 and Limits sentence 2 are byte-identical to the tip (compared as parsed paragraphs). Every clause in them still measures true on this tree:
Format. The new lines are 75 and 76 columns wide. 2. AST identical with docstrings maskedI masked the first-statement string constant of every module, class and function, then compared
To show the masking is not vacuous, three code mutants make the masked AST differ from the tip, and a docstring-only mutant does not:
3. Rulings on the two flagged points(a) The lying The case is an
Neither "can escape" nor "such as" covers it.
So, read with "has" meaning the characters, sentence 1 is literally false for this object. But three things make it not a finding:
Guarding it would take a caveat about lying dunders across every paragraph. That is the opposite of principle 3, "Add only what is necessary, and nothing more", and nothing in ATOM produces such a dtype. No issue is needed. (b) "word for word" in the commit message: nothing is needed on the branch. The claim is loose. The Refused paragraph says "anything whose The branch must not be amended: AI_DEV_RULES says "PR branches only gain commits: no force-push, no rebase, no amend". A fix-up commit would only add a second message that is squashed away. The rule "PRs land squashed ... one commit per task with a written message" means the lander writes the squash message from scratch. The lander should not carry the "word for word" sentence into it. "the same condition as the Refused paragraph" is accurate. 4. ponytail-review (diff:
|
| tree | passed | skipped | xfailed | GATE_CPU_RC |
|---|---|---|---|---|
developer: merged d46e0c0de (tree 8f54c94b4, tip 37fba4df0) |
5276 | 155 | 3 | 0 |
reviewer: merged 3652b3ec3 (tree 901ef6de8, tip 3eb94cfa7) |
5276 | 155 | 3 | 0 |
| delta | 0 | 0 | 0 |
Gate 1 holds on the tree that will land.
Note for the next task in this area (not a finding)
An int subclass holding 8 whose dtype.__str__ returns a non-str is refused with the exact message. The TypeError from str() is caught, so it becomes this refusal instead of escaping.
This makes no sentence false: Limits sentence 2 says such an exception "can escape", not that it must. It is unchanged from the tip. Like (a), it needs a dunder that breaks its protocol.
Reviewed head: 0a152711fe9890d4e44b1ff1c4155e534361ae6a. Verdict: APPROVE. The gate above covers the tree that lands on tip 3eb94cfa7.
…le, not to one (#395) The backend and runner guards only asserted that their module walk was non-empty. A narrowed walk that still found something passed. So did a walk made non-recursive: on today's flat packages it drops no cases, but it hides a breach planted one directory down. Two tests per package: - test_the_walk_returns_every_module_under_the_root: runs the real walk (_backend_modules / _runner_modules) over a tree the test builds (__init__.py, a.py, sub/b.py) and asserts the exact expected set. - test_every_module_the_walk_returns_is_a_case: asserts the parametrised cases equal the walk. A nested breach under glob, rglob("__init__.py"), rglob("[!_]*.py"), a slice in the walk, and a slice or an inlined glob at parametrize each turn one of them red by name. On the tip, the same plants leave the suite green. The runner's engine-import exemption matched model_runner.py by file name at any depth. It now matches the path under the package, so runner/sub/model_runner.py importing the engine is refused. Today's test ids are unchanged. The three landed sites with the same non-empty guard (test_spec_schema, test_ir_data_model, test_clock_lp_identity) wait on #223's ruling. Gate (node 18, CPU tier, combined with #408 on 3eb94cf): 5280 passed (+4), 155 skipped, 3 xfailed, GATE_CPU_RC=0. Part of #232 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ile name (#407) SITES in test_runner_rpc_surface.py keyed broadcast sites by the bare file name engine_core.py. atom/diffusion/engine/engine_core.py has the same name. A waited broadcast padded into that file, at a line inside PrefillEngineCore's range, turned the #394 pin red for a call no RapidServe core makes. Site.file now holds the path relative to the repo root. Its consumers are updated: - the #394 pin's waited filter; - get_num_blocks; - the forward counter keys; - the docstring citation check, which strips atom/model_engine/, so a same-named file elsewhere keeps a "/" and cannot satisfy a citation. The three consumer files keep 94 identical ids, all passing. The citation fix also closes a second collision that was silent at the tip. An unwaited exit at a cited line of a same-named file passed the citation test there; it now fails. Changed lines: +11/-7, within the brief's 2x stop. Gate (node 18, CPU tier, combined with #395 and #408 on 3eb94cf): 5280 passed, 155 skipped, 3 xfailed, GATE_CPU_RC=0. Closes #401 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…usal comment (#402) The exit bullet in atom/compass/runner/__init__.py said that "the graphs and five KV tensors it deletes stay held". This runner allocates neither: - allocate_kv_cache allocates nothing; - capture_cudagraph captures nothing. The refusal comment in model_runner.py says the opposite, and that comment is the loss list a test holds against ModelRunner.exit's body. Nothing tested the bullet's copy of the list. The bullet now says exit (engine_core.py:260) never reaches ModelRunner.exit, and points at the comment on the RPC_SURFACE check for what that loses here. The rest of the docstring is unchanged, and test_runner_rpc_surface.py passes whole (69). The review found that an eagle3 speculative config builds and loads its drafter in ModelRunner.__init__, before forward refuses it. That means the comment's loss list omits a real deletion (self.drafter). Filed as #405, outside this file set. Gate (node 18, CPU tier, combined with #395, #408 and #407 on 3eb94cf): 5280 passed, 155 skipped, 3 xfailed, GATE_CPU_RC=0. Closes #398 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Closes #403
Change
atom/compass/kv/handoff.py,whole_number's docstring only: the Limits paragraph's first sentence goes from three lines to two, in the form the #391 cycle-2 reviewer proposed in N1.Net -1 line (119 -> 118). Line widths 75 and 76.
ruff checkandblack --checkon the file: both pass.Dev record
Every clause of the whole docstring was probed on node 18, on the merged tree
d46e0c0de(tip37fba4df0+ head0a152711f;merge-treeequals the head's tree8f54c94b4), withatom.__file__under the staged root. numpy 2.4.6, torch 2.10.0+rocm7.2.4, Python 3.12.3. The probe is the #391 cycle-2 reviewer's, extended; it is atagent_scratch/compass_dev/issue-403/probe.py. "Branch fired" is from an instrumented copy of the function. The last column is a copy of the function with the dtype-text test removed.The three things the brief asked about:
tp_size is <repr>, which is not a whole number; a parallel width or rank is a count, so it is refused rather than converted. The probe compares it character for character. For example:tp_size is array(8, dtype=(numpy.int64, [('is_bool', '<i8')])), which is not a whole number; ....dtypehas "bool" in its text").probe2.py).Not a finding, recorded for the reviewer. An
intsubclass whosedtype.__str__returns astrsubclass that holds the text "bool", but whoseinanswers False, is accepted. That contradicts "a whole number whose dtype text has "bool" ... is refused" if "text" is read as the characters. The tip's sentence makes the same claim ("the dtype text test also refuses a whole number whose dtype text has "bool"") and gets the same result, so this PR does not introduce it. A dunder that lies can falsify any clause here, the tip's "No value that is accepted changes on the way" included; Limits sentence 2 covers dunders that raise, not dunders that lie.Correction to the commit message. It says the kept condition is the Refused paragraph's "word for word". It is the same condition, but not the same words: the Refused paragraph says "anything whose
dtypehas "bool" in its text". The branch only gains commits, so this is corrected here rather than amended.AST with docstrings masked: identical to the tip (
masked AST head == tip: True). Unmasked it differs, as expected. To show the masking is not vacuous, three code mutants make the masked AST differ from the tip: droppingOverflowError,"bool"->"Bool", and changing the message text. A docstring-only mutant does not.Probe table (node 18, merged tree
d46e0c0de)Columns: clause | input |
whole_number| branch fired | copy without the dtype-text test.Limits sentence 1 (the changed sentence)
inanswers False, text 'bool'The message ("is refused as not one")
Unchanged paragraphs: first paragraph, Refused, Limits sentence 2
__int__raises RuntimeErrordtypeproperty raises RuntimeError (int sub 8)__ne__raises (int sub 8)__str__raises__ne__returns obj whose__bool__raises__str__returns str sub whose__contains__raisesdtyperaises AttributeError (int sub 8)dtyperaises AttributeError('bool') (int sub 8)Docstring and AST
Second paragraph: the launch-path clauses (
probe2.py, same tree)--tensor-parallel-sizeaction type--data-parallel-sizeaction type--pipeline-parallel-sizeaction type--data-parallel-size-localaction type-tp 8 -dp 2: types-tp 8.0ATOM_DP_RANK[is] parsed with int'"ATOM_DP_RANK": lambda: int(os.getenv("ATOM_DP_RANK", "0")),Configraises on a string width'assert 1 <= self.tensor_parallel_size <= 8if self.data_parallel_size < 1:Gate 1 (node 18,
xiaobizh_n18_cpu)The merged tree
d46e0c0de(git merge-tree --write-tree 37fba4df0 0a152711f=8f54c94b4, the head's own tree), staged bygit archiveinto the container's/tmp, with stamps written. The gate'scommit:line readsd46e0c0de (stamp),.compass-changedisatom/compass/kv/handoff.py, andatom.__file__is under the staged root. The tree's ownscripts/compass/gate_cpu.shran once, unpiped, undertimeout -k 10 3000.GATE_CPU_RC37fba4df0(from the brief)d46e0c0deWall time 188 s. No timing flake fired.
🤖 Generated with Claude Code