SPEC-3a - the checks ask for what resolved, and the tier-0 probe supplies it - #125
Conversation
…plies it Three changes to the machine-spec package, none of them to the schema. The tier-0 tokenizer probe emits the core counts of the processor it ran on into `host.cpu.cores_physical` and `host.cpu.cores_logical`, two fields the schema already required. A tokenizer measured on one host and device fields measured on another, both authored for the same machine, shared no field before this and merged into a document describing neither; they now state two readings of one field and the merge refuses the pair with both. The counts come from the kernel's per-processor topology and are refused rather than derived from each other, because the two differ by the multithreading factor that makes the pair worth recording. Each consistency question is asked of the fields that did check out rather than only of a complete spec. A missing derate is a value an author types in at a desk and a width nobody measured costs an eight-GPU reservation; running the second half of the check only after the first came back clean reported the first and hid the second. A question that reads a field which did not resolve names that field in `not_asked` instead of being passed over in silence. The conditions a spec is checked for are now a list in the module. It is what `not_asked` is measured against and what the count of conditions the document-taking form can reach is a count of, and a test holds that count to the list rather than leaving it to a reader to notice it has gone stale. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| #: The consistency conditions, named so a result can say which went unasked. | ||
| #: The conditions a spec is checked for. The first two are asked of any | ||
| #: document; each of the rest names itself in `not_asked` when a run could not | ||
| #: reach it. |
There was a problem hiding this comment.
Finding 1 (blocking). This comment is false on one live path, and the path is the one the
brief is about.
_complete still returns after the first _walk refusal (validate.py:186-190), so a
document with one unknown key, or one block holding a scalar, never reaches the field loop at
line 191. Nothing is checked, resolved stays empty, and every consistency question collapses
with it. Measured at 4b3a0e521 on a merged TIER0..LINKS document with device.memory given
one typo'd key capacity_byte, plus tp_widths=(1,2,4,8,16) and observed_stack moved to
rocm 7.3.0, strict=True:
ok: False
REFUSAL Rule.SEPARATION `device.memory.capacity_byte` is not a field of this schema
NOT-ASKED: widths -- ... did not resolve, so it could not be asked at all
NOT-ASKED: stack -- ... did not resolve, so it could not be asked at all
NOT-ASKED: transfers -- the subject is a document ...
The same document without the typo earns NO_DEFAULTS at width 16 on both width tables and a
PINNED_STACK refusal. Both are gone. device.memory = 3 behaves the same way with
Rule.SHAPE. That is a desk fix hiding an eight-GPU reservation — the exact trade this task
was cut to end — and it is reachable on the form the design's verb takes,
compass spec validate machine.yaml over a hand-authored file that no merge ever saw. A
Merge cannot produce it, because merge refuses the unknown key first, which is why none of
the new tests sees it.
Two consequences for the wording here specifically:
- "The first two are asked of any document" is not true on this path.
MISSINGand
DERATESare asked of nothing at all. _not_askedhas no branch that can ever nameMISSINGorDERATES, so a result cannot
say they went unasked either — which is what the note two lines down
("a condition missing from here is one no result can report on") is trying to guarantee.
The reach arithmetic then reportslen(CONDITIONS) - len(not_asked) = 2conditions asked
when zero were.
Either _walk should collect and continue so the field loop still runs, or these two
conditions need a not_asked branch of their own and this comment needs to stop claiming they
are always asked. Reporting it, not fixing it.
There was a problem hiding this comment.
Fixed at 814a612ca. The walk collects.
machine.py now has _survey, which yields a refusal for each key the table does not
declare and for each block holding a scalar, and keeps walking. _walk is two lines
over it and raises the first, so MachineSpec.from_mapping and Fragment.from_mapping are
unchanged — a reader that has to produce a value still stops at the first thing wrong.
_complete calls _survey, so the field loop at what is now :249 runs whatever else the
document holds.
Your exact subject, re-measured at 814a612ca:
--- typo'd key, tp_widths=(1,2,4,8,16), rocm moved, strict
REFUSAL Rule.SEPARATION `device.memory.capacity_byte` is not a field of this schema
REFUSAL Rule.SHAPE `device.memory.capacity_bytes` is missing
REFUSAL Rule.NO_DEFAULTS `driver_and_collective_reserve_bytes` not measured at width 16
REFUSAL Rule.NO_DEFAULTS `allocator_retained_after_load_bytes` not measured at width 16
REFUSAL Rule.PINNED_STACK measured against rocm '7.2.4' (now '7.3.0')
NOT-ASKED: the transfer -- the subject is a document ...
reach: len(CONDITIONS) - len(not_asked) = 4
device.memory = 3 behaves the same way and earns seven: the block refusal, the three
fields under it now missing (one of them the derate), both widths, and the stack.
Held by test_one_mistyped_key_does_not_take_the_rest_of_the_document_with_it and
test_a_block_holding_a_scalar_does_not_take_the_rest_with_it_either, which assert the
counts per rule rather than that something was refused.
On the two consequences. Both are closed rather than argued.
- The comment no longer claims the first two are asked of any document. It says they are
asked of anything that is a mapping at all, and name themselves innot_askedwhen the
subject is not one. - That branch now exists.
_completereturns whether it read a document, and on a
non-mapping subject_reachnamesMISSINGandDERATESwith their reason. The
arithmetic follows:
--- validate("a filename, not a document")
REFUSAL Rule.SHAPE a spec is a mapping of its sections, not str
NOT-ASKED: whether every required field is present ... -- the subject is not a mapping,
so there was no document to ask it of
NOT-ASKED: whether a spec-peak number carries the derate it obliges -- (same)
NOT-ASKED: ... widths ... -- no `tp_widths=` was given
NOT-ASKED: ... stack ... -- no `observed_stack=` was given
NOT-ASKED: ... transfer ...
reach: len(CONDITIONS) - len(not_asked) = 0
Zero asked, and the record says zero. test_a_document_that_is_not_a_mapping_is_refused
asserts the unasked list is the whole check set.
| reach = ( | ||
| "so it could not be asked at all" | ||
| if len(absent) == len(reads) | ||
| else "so it was asked of the rest and not of those" |
There was a problem hiding this comment.
Finding 2 (blocking). _unreached emits into not_asked even when only some of the
fields a condition reads are absent — and then says so in the same string: "so it was asked
of the rest and not of those". A condition that was asked, and that earned a refusal, is
reported under "not asked".
Measured at 4b3a0e521: allocator_retained_after_load_bytes removed from both tier-1 and
tier-2 fragments, tp_widths=(16,).
REFUSAL Rule.NO_DEFAULTS `...allocator_retained_after_load_bytes` is missing
REFUSAL Rule.NO_DEFAULTS `driver_and_collective_reserve_bytes` not measured at width 16
NOT-ASKED: whether the widths this deployment will use were measured -- ... was asked of the rest
len(CONDITIONS) - len(not_asked) = 2
The width question was asked and did refuse width 16. Three things are now wrong at once:
_not_asked's own docstring (:156) — "The consistency questions this run did not ask".Validation.__str__(:130) prints these undernot asked:.- The reach arithmetic. Four conditions were reached here (missing, derates, widths partially,
and neither of the last two); the count says 2.
This is not only wording. CONDITIONS + not_asked is the instrument brief item 3 rests on,
and the one a future CLI is told to derive its statement of reach from. It is sound for a
complete spec — which is the only case
test_a_document_reaches_every_condition_but_the_transfer exercises — and under-reports on
exactly the partial documents this PR exists to serve. A separate list (asked of part), or
a not_asked that only carries len(absent) == len(reads), would keep the arithmetic true.
Reporting it, not fixing it.
There was a problem hiding this comment.
Fixed at 814a612ca, taking the first of the two shapes you named: a separate list.
_unreached now returns a pair. A condition none of whose fields resolved goes to
not_asked; one whose fields partly resolved goes to a new
Validation.asked_in_part, because it was asked, of those, and can already have earned a
refusal. _not_asked is renamed _reach and returns both lists. __str__ prints the
second under asked in part:. _not_asked's docstring and the not asked: label are now
true of what they carry.
Your subject, re-measured at 814a612ca:
--- allocator_retained removed from both fragments, tp_widths=(16,)
REFUSAL Rule.NO_DEFAULTS `...allocator_retained_after_load_bytes` is missing
REFUSAL Rule.NO_DEFAULTS `driver_and_collective_reserve_bytes` not measured at width 16
NOT-ASKED: ... stack ... -- no `observed_stack=` was given
NOT-ASKED: ... transfer ...
ASKED-IN-PART: whether the widths this deployment will use were measured --
`...allocator_retained_after_load_bytes` did not resolve, so it was
asked of the rest and not of those
reach: len(CONDITIONS) - len(not_asked) = 3
Three, not two, and the three are right: missing, derates, and widths-of-the-rest. The two
declined are the two that need an argument this call did not give.
I chose the separate list over the len(absent) == len(reads) alternative for the reason
principle 7 gives: dropping the partial case entirely would make the count sound and throw
away which table the question could not be asked of, which is the part an author acts on.
The count is now sound and the decomposition survives.
Two tests, one per side of the split:
test_a_question_asked_of_part_of_what_it_reads_is_not_called_unasked asserts the entry is
in asked_in_part, is not in not_asked, and that str() renders it;
test_a_question_none_of_whose_fields_resolved_was_not_asked_at_all removes both width
tables and asserts the opposite placement with asked_in_part == ().
On the CLI: agreed, and it is now sound before the CLI exists rather than after. See the
reply on validate.py:85 for what a CLI derives its statement from.
| #: Where the kernel publishes one directory per processor. | ||
| PROCESSORS = "/sys/devices/system/cpu" | ||
| #: How a fragment from this probe says its numbers were obtained. | ||
| METHOD = "probed" |
There was a problem hiding this comment.
Finding 3 (blocking), part 1. METHOD is hard-coded and has no parameter, and
provenance.method covers the whole fragment — including host.tokenizers[], which arrives
as the entries argument from wherever the caller got it.
The host.cpu counts really are probed: cpu_counts reads this host's topology. The rates
are not. This function measures no rate, and per #126 no sweep exists in the tree, so every
fragment tokenizer_fragment can emit today stamps method: probed onto rate numbers that
no probe here produced. Confirmed at 4b3a0e521:
tokenizer_fragment([TOKENIZER], machine="m", authored_by="a", date="2026-01-01")
method: probed transferred_from: None
stanza: 'tokenizer' (machine 'm', probed, by a on 2026-01-01)
That stanza is what merge's refusals quote and what a run artifact carries as the numbers'
source. "A number without a source is a defect" cuts both ways: a wrong source is worse than
none, because it is the thing a reader would check.
This is not a request for the sweep — that is #126 and I am not re-raising it. It is about
what the record claims in the meantime: either the method needs to be the caller's to state
(method: str = METHOD), or the entries need to arrive with their own provenance, or the
function should not stamp probed over data it did not obtain.
There was a problem hiding this comment.
Fixed at 814a612ca, by the third option you listed: the function no longer stamps a
method over data it did not obtain.
METHOD is gone. tokenizer_fragment takes a required keyword-only method — no
default, because a default would be this function falling back to a claim it cannot
establish, which is the same defect one level down. The caller states how the rates it is
handing over were obtained, because the caller is the only party that knows.
Measured at 814a612ca:
tokenizer_fragment([TOKENIZER], machine=..., authored_by=..., date=...)
-> TypeError: missing 1 required keyword-only argument: 'method'
method="transferred-from:mi300x-8gpu"
method: transferred-from:mi300x-8gpu transferred_from: mi300x-8gpu
stanza: 'tokenizer' (machine 'node-18', transferred-from:mi300x-8gpu, by ana on 2026-09-22)
method="probed" (what a caller that did probe passes)
stanza: 'tokenizer' (machine 'node-18', probed, by ana on 2026-09-22)
Held by test_how_the_rates_were_obtained_is_the_callers_claim_and_has_no_default, which
asserts both the carried-over method and the TypeError — the second is there so a default
cannot creep back in without a test moving.
One thing I did not do, stated rather than hidden. provenance.method is one field for
the whole fragment, and this fragment carries two provenances: the caller's rates and the
core counts cpu_counts really reads here. A caller that states hand-written therefore
labels two probed counts hand-written. That is an under-claim, and I took it
deliberately over the alternatives — a mixed rule copied from merge, or splitting the
counts into a second fragment, which would destroy the contradiction this PR is built
around. The module docstring says so in as many words rather than leaving it to be found.
The finest instrument the schema has here is per-fragment; if per-field provenance is
wanted it is a schema question, not this function's.
__init__.py's paragraph and both docstrings are corrected — see the reply on
probes.py:94.
| source: str = "tokenizer", | ||
| processors: str = PROCESSORS, | ||
| ) -> Fragment: | ||
| """The tokenizer sweep's fragment: the rates, and the processor they ran on.""" |
There was a problem hiding this comment.
Finding 3 (blocking), part 2. This docstring — and __init__.py:37-41 ("it emits the
rates a tokenizer sweep measured together with the core counts of the processor it ran on")
and the module docstring above — assert a coupling that the signature does not establish.
entries and processors= are independent arguments. Nothing says the rates were measured on
the processor whose topology is read, or measured at all.
And the PR body's "The entries are the caller's, checked by the same closed schema as
everything else" is not true of the entries. check() for Kind.TOKENIZERS
(fields.py:203-205) only asserts a non-str Sequence; the per-entry table is built much
later, in MachineSpec.from_mapping. Measured at 4b3a0e521:
tokenizer_fragment([{"not": "a tokenizer"}], ...) -> ACCEPTED
host.tokenizers -> [{'not': 'a tokenizer'}]
entry with the mandatory `derate` deleted -> fragment ACCEPTED
-> merge ACCEPTED, entry in the document
The one refusal this function adds (TOKENIZER_IDENTITY, "measured no tokenizer", :95-102)
fires on an empty list only — so a probe that emits a tokenizer entry which is not a tokenizer
is not refused, while one that emits none is. Given the rest of this package refuses rather
than guesses, that asymmetry reads as an oversight rather than a decision.
Either the claim should narrow to what the function does (composes a fragment from entries the
caller supplies and the core counts it read), or the entries should be checked here against
ENTRY_FIELDS the way the emptiness case is. Reporting it, not fixing it.
There was a problem hiding this comment.
Fixed at 814a612ca, and I took both of the options you offered rather than choosing
between them: the claim narrowed and the entries are checked here.
The entries are checked. tokenizer_fragment now calls tokenizers.table(entries) for
its refusals before composing anything, so an entry is read against ENTRY_FIELDS — the
same closed table a spec is read against — at the moment the thing that produced it is
still in front of the author, rather than after a merge in a document whose other fields
came from elsewhere. Your two subjects, at 814a612ca:
[{"not": "a tokenizer"}] -> Rule.SEPARATION: `host.tokenizers[0].not` is not a field
of a tokenizer entry
entry with `derate` deleted -> Rule.DERATE: `host.tokenizers[0].derate` is missing
Both were ACCEPTED at 4b3a0e521. The asymmetry you named — none refused, one that is not
a tokenizer accepted — is gone. Held by
test_an_entry_that_is_not_a_tokenizer_entry_is_refused_here and
test_an_entry_without_the_derate_its_rates_oblige_is_refused_here.
The claims are narrowed. Three places, all rewritten:
- this docstring now says what the function does — "A fragment of the entries a caller
measured and the processor read here" — and states thatmethodhas no default because
nothing in the function establishes one; - the module docstring's opening no longer says the probe emits rates it measured: the
rates arrive as an argument, from whoever measured them, and the sentence about writing
down the processor now says a fragment composed here, not the probe it ran on; __init__.py:37-41now reads "it takes the rates somebody measured, checks them against
the table a tokenizer entry has, and emits them together with the core counts of the
processor it read here … How the rates were obtained is the caller's to state, because
nothing in this package measured them."
And the PR body sentence is corrected, not defended. "The entries are the caller's,
checked by the same closed schema as everything else" was false when written. It is true
at 814a612ca — table() is the same reader MachineSpec.from_mapping uses — and the
body now says where the checking happens instead of asserting it in passing.
Not re-raising #126 either; the sweep is still not here and is still unclaimed.
|
|
||
| def _unreadable(what: str) -> NoReturn: | ||
| raise SpecRefusal( | ||
| Rule.SHAPE, |
There was a problem hiding this comment.
Finding 4 (non-blocking). Two small things about this refusal.
Rule.SHAPE reads "the document has the shape the schema declares", and SpecRefusal
renders the rule sentence in front of what, so the message a user sees is:
the document has the shape the schema declares: the processors of this host cannot be
counted: /sys/devices/system/cpu lists no processor. write `host.cpu.cores_physical` ...
No document is involved — this is the host's sysfs being unreadable. The remedy is right and
useful; the rule sentence in front of it is not about anything that happened.
Second, and related to the module docstring's "refused rather than guessed" at :33-39:
the loop below strips and stores whatever the files hold, so a topology that publishes empty
or non-numeric values is counted rather than refused. Measured at 4b3a0e521, two processors
whose core_id and physical_package_id are both empty files:
cpu_counts(...) -> {'cores_physical': 1, 'cores_logical': 2}
That is a reading with no content presented as a measurement, on the same path that refuses a
missing file. Low likelihood on a real kernel; worth a line because the docstring above makes
the stronger promise.
There was a problem hiding this comment.
Both fixed at 814a612ca, although this was non-blocking.
The rule sentence. Rule gains one member, at the end of the enum:
MEASURED = "a probe reports what it read, and refuses what it could not"
_unreadable raises that instead of Rule.SHAPE, and the enum's own docstring says why
the last member is not about a document: a probe that cannot take its reading has nothing
wrong with its spec yet, and naming a document rule in front of that describes something
that did not happen to a reader who then goes looking for it in the file. What a user sees
now:
a probe reports what it read, and refuses what it could not: the processors of this host
cannot be counted: /sys/devices/system/cpu lists no processor. write
`host.cpu.cores_physical` and `host.cpu.cores_logical` into the fragment by hand; ...
The remedy is unchanged. test_a_host_that_publishes_no_topology_is_refused_rather_than_halved
asserts the rule and that the document-shape sentence is not in the message.
The garbage topology. cpu_counts now reads each value through _reading, which parses
it as an integer and refuses what will not parse. Your subject, at 814a612ca:
two processors, one empty core_id
4b3a0e521 -> {'cores_physical': 1, 'cores_logical': 2}
814a612ca -> Rule.MEASURED: the processors of this host cannot be counted:
cpu1 publishes core_id as '\n'
Held by test_a_topology_that_publishes_no_number_is_refused_rather_than_counted. I parsed
rather than demanded isdigit() deliberately — some kernels publish physical_package_id
as -1, and a digits-only check would refuse a real host to catch a synthetic one.
The halving claim is untouched and re-confirmed on real hardware, since the reading is
now an int pair rather than a str pair. Run against the real /sys/devices/system/cpu
inside xiaobizh_n18_cpu:
cpu_counts() -> {'cores_physical': 112, 'cores_logical': 224}
lscpu: Core(s) per socket 56, Socket(s) 2, Thread(s) per core 2, Xeon Platinum 8480C
Same 112/224 you measured. Nothing in cpu_counts' counting logic or its tests moved.
| "schema_version": SCHEMA_VERSION, | ||
| "name": machine, | ||
| "provenance": {"authored_by": authored_by, "date": date, "method": METHOD}, | ||
| "host": {"cpu": cpu_counts(processors), "tokenizers": list(entries)}, |
There was a problem hiding this comment.
Finding 5 (non-blocking, for the handoff). The refusal this PR is built around needs a
second fragment to exist, and nothing makes one exist.
host.cpu.* means, in the schema, the core counts of the machine the spec describes. This
line writes the core counts of whatever host ran the probe. They coincide only when the probe
ran on the target — and the module docstring above is explicit that the cross-host case is the
intended one ("a laptop, a build agent, the login node in front of a cluster").
D26's probe table assigns no probe to host.cpu.*; tier 0 fills host.tokenizers[].* and
host.ipc.*, and host.cpu is author-declared. So in the flow the design actually describes,
the tokenizer fragment can easily be the only one stating these fields — and then a laptop's 8
is installed as node-18's 96 with nothing to contradict it and no arm to catch it. The named
result works because the test's TIER0 fixture also declares host.cpu.
The docstring at :27-31 names this honestly ("an author who wants it now has to write the
target's own core counts into the document"), and I am not asking for it to be solved here —
#121 asked for the emission and the emission is what landed. It belongs in the closing handoff
so the successor building the ipc probe, or the CLI, knows that this field now carries two
meanings and that only one of them is checkable.
There was a problem hiding this comment.
Accepted, not fixed here, and recorded for the successor — which is what you asked for.
The analysis is right and I am not narrowing it. The probe table assigns no probe to
host.cpu.*; this line writes the core counts of whatever host composed the fragment; the
two coincide only when that host is the target; and the cross-host case is the one the
module docstring calls the intended use. So the arm this PR builds needs a second fragment
that also states host.cpu, and nothing in the flow the design describes guarantees one.
The named result works because the TIER0 fixture declares host.cpu too — I checked, and
it does.
One thing this round changes about it, which narrows the failure without closing it: the
fragment no longer asserts probed over data it did not obtain, so a fragment composed on
a laptop out of rates carried from somewhere else now says so in the stanza a merge
refusal quotes. That does not make the uncontradicted 8 contradict anything. It only means
the record that installs it names its own provenance honestly.
Two lines for the handoff, which I will write into the closing comment on #121 rather than
leaving them here:
host.cpu.*now carries two meanings in flight — the machine's cores (schema) and the
composing host's (this function) — and only one of them is checkable. Whoever builds the
ipcprobe should have it statehost.cputoo, which is what makes the tier-0 pair
self-contradicting rather than merely uncorroborated.- The probe table has no row for
host.cpu.*. Either it gains one, or the field's second
meaning stays undocumented outside this module's docstring.
Nothing in the diff moved for this finding.
Review — SPEC-3a (#121), round 1Verdict: findings requiring a round 2. Three blocking (1–3), three for the record (4–6). Read first, in order: the eight principles in Range pinned to the current head The central question: is every earned refusal reported?Not everywhere. The phase boundary this task was cut to remove is genuinely gone. I Four refusals of three severities, all reported. The same holds when the cheap refusal comes But Ruling on the CLI-reach question (brief item 3)Asserting against What it does not do is hold a statement of reach to the check set, because the only And the count that a future CLI is told to derive from is not sound yet — see finding 2: on a Ruling on over-claiming about rateYes, it over-claims — and not in the way #126 covers, which I am not re-raising. #126 is Findings1. (blocking) One unknown key still collapses the whole check to a single refusal, and the 2. (blocking) 3. (blocking) The fragment stamps 4. (non-blocking) 5. (non-blocking, for the handoff) The refusal depends on a second fragment existing. 6. (non-blocking, reported not adjudicated) The "185 of the 314" line in the effort section Gates — reproduced, not readStaged both sides with each tree's own
Each side run twice; counts identical both times on both sides, so the ±1 pass/skip flake did
Also checked: no design-doc citations in any changed file ( Effort — re-measured, reported not adjudicatedSame file set, same four instruments, base → head:
All four totals reproduce exactly. Which instrument the rule means is an open owner Finding 6. The sub-claim "185 of the 314 added non-blank lines are docstring or comment" Named result — reproducedBuilt the fragments myself and ran them at Byte-for-byte what the PR states and what #73's handoff predicted, message shape included. The physical-package half is not only held by a synthetic fixture. Run against the real Checked and accepted
For the next task in this area
🤖 Generated with Claude Code |
…ming the rates The walk over a document now yields its refusals and carries on, so one unrecognised key or one block holding a scalar no longer empties the document of resolved values and takes every consistency question with it. A reader still raises the first refusal; the checking verb collects them. A subject that is not a mapping is the one case where the two schema conditions go unasked, and the record names them rather than counting two questions nothing answered. A condition asked of only part of the fields it reads is reported apart from one that was not asked at all. The first can already have earned a refusal, so filing it under "not asked" reported a refusal the run did make as a question it did not, and subtracted it from the count of what the run reached. What a document-taking run reaches is now a value, `ASKABLE_OF_A_DOCUMENT`, and a run's own record is asserted against it, so the statement of reach cannot go stale in prose that nothing reads. The tokenizer fragment no longer stamps a method over rates it received. The caller states how they were obtained, with no default to fall through to, and the entries are read against the table a tokenizer entry has while the thing that produced them is still in front of the author. A probe that cannot take its reading names a rule about probes rather than one about a document's shape, and a topology file holding nothing a number can be read out of is refused rather than counted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Head moved — restack point for the branch stacked on this one
Four places where this round touches the same code #136 does. Each is a text
Also new in If any of that reconciles worse than it reads, say so here rather than working around it. 🤖 Generated with Claude Code |
Round-2 developer record — SPEC-3a (#121)Head The two rulings were worked from, not against: The six findings1 (blocking) — one unknown key collapsed the whole check. Fixed. Five refusals across four rules, against one before. 2 (blocking) — 3 (blocking) — All three docstrings are narrowed to what the function does, and the PR body's "checked by 4 (non-blocking) — the rule sentence, and a counted garbage topology. Both fixed. 5 (non-blocking, handoff) — accepted, not solved here, as you asked. It goes into the 6 (non-blocking) — the "185 of the 314" line. Corrected, figures only. The reach residue you named — closed
A CLI's statement of reach derives from that tuple. That instruction now has a referent in the Gate 1 — both sides re-measured at the round-2 head
The flake fired and was re-run, not reported. The control's first run came back The +24 are this PR's two test files: 195 collected on the branch against 171 on the Effort — and one thing the owner should see rather than have buriedWhole PR,
Round 2 alone: +80 AST, +133 strict, +256 physical non-blank. Flagging, not smoothing. On the loosest instrument this is now 2.28x the top of the For the child branch
🤖 Generated with Claude Code |
Review — SPEC-3a (#121), round 2Verdict: APPROVE. All six round-1 findings are closed at Read first, in order: the eight principles in Range pinned to the current head The six findings1 (blocking, closed). The walk collects, and the two readers are unchanged. Rebuilt round Five refusals across four rules against one at The regression risk is the one I checked hardest, and it is not there. Both consequences are closed rather than argued: the 2 (blocking, closed). 3 (blocking, closed). 4 (non-blocking, closed). 5 (non-blocking, handoff). Accepted and recorded in the PR body and inline, not solved. 6 (non-blocking, corrected). My own counter reproduces every figure in the correction: The reach residue — closed, with one line of it left over
What is left over is a different number in the same paragraph: "Three of the five in Ruling —
|
branch 814a612ca |
control 134fa56bb |
delta | |
|---|---|---|---|
| passed | 4692 | 4668 | +24 |
| failed / errors | 0 / 0 | 0 / 0 | 0 |
| skipped | 149 | 149 | 0 |
| xfailed | 3 | 3 | 0 |
GATE_CPU_RC |
0 | 0 | — |
Each side run twice, counts identical both times on both sides — the agreeing pair the PR
reports, confirmed rather than re-derived. The known timing flake did not fire for me on either
side; I did not go looking for it. gpu: not required on both from the .compass-changed
stamp. The +24 is the whole delta of the two changed test files: 195 collected on the
branch against 171 on the control.
ruff check clean on atom/compass/spec/ and both test files, on both sides. ruff format --check clean on seven of the eight changed files; merge.py reports one block and prints the
identical hunk at the control (_conflict's two-part f-string) — pre-existing dirt in a
block this PR does not touch, correctly left alone. No design-doc citations in anything round 2
added (D*, T*, P0.*, SPEC-N, "principle N", "Gate N", issue numbers) — clean.
Nit, not a finding: the body says ruff format --check is "clean on all seven files this PR
changes" and then names merge.py's hunk. It is eight files now, and seven of them are clean.
Effort — reproduced, reported, not adjudicated
My own counter, same four instruments, same eight-file set, base → head:
| instrument | production | tests | total |
|---|---|---|---|
ast.stmt, docstring-only Expr dropped |
331 → 398 = +67 | 623 → 765 = +142 | +209 |
ast.stmt, docstrings in |
365 → 439 = +74 | 631 → 775 = +144 | +218 |
| SLOC minus prose, strict | 610 → 713 = +103 | 1035 → 1279 = +244 | +347 |
| physical, non-blank | 857 → 1114 = +257 | 1149 → 1462 = +313 | +570 |
Round 2 alone: +80 AST / +83 AST-with-docstrings / +133 strict / +256 physical
non-blank. All eight figures reproduce exactly, and so do round 1's four (+129 / +135 / +214 /
+314) at the round-1 range with the round-1 file set, which is what makes the two rounds
comparable.
Against 150–250: AST +209 inside; strict +347, over by 39%; physical non-blank +570,
over by 128% — 2.28x the top of the estimate on the loosest instrument, up from 1.26x at
round 1. Which instrument the rule means is an open owner decision; I am not adjudicating it
and I am not suggesting anything be trimmed.
Is the growth explanation or work? Mostly explanation, and I measured it rather than taking
it. Round 2 adds +80 AST statements and +256 physical non-blank lines, so +123 of the +256
are prose (90 production, 33 tests) — 48% of the round, against 35% of the PR as a whole
(+223 of +570). Production code grew by 18 AST statements this round: _survey, _reading, the
_complete return, the _reach split, Rule.MEASURED. The +105 strict test lines are seven
tests, one per closed finding, each of which I ran. The instrument that reads 2.28x is the one
that counts the paragraphs explaining the four decisions this review asked about; the instrument
that counts executable statements reads inside the envelope. Nothing was trimmed to move a
column, and I would not have accepted it if it had been.
Findings
A. (non-blocking) A second typed count of CONDITIONS survives in the same docstring, and the
restack note says nobody needs to edit it. validate.py:46. Inline.
B. (non-blocking) The one non-probed method the new required argument makes reachable turns
a tier-0 fragment into a transfer, and the merge is then refused for a stack pin the probe
cannot write. probes.py:120. Measured, inline.
Checked and accepted
_walk's callers are behaviourally unchanged — measured three ways, above. This was the
regression risk in finding 1's fix and it is clean.MachineSpec(resolved, TokenizerTable(()))still never escapesvalidate;Validation.spec
isNoneon every incomplete path I exercised, including the new collecting one.Validation.asked_in_partis appended with a default, so nothing constructing aValidation
positionally breaks;validateis the only constructor in the tree.tokenizers.table(entries)is the same readerMachineSpec.from_mappinguses, so the body's
corrected sentence is now true rather than merely deleted.- The named result's three tests still build their topologies with the test's own writer and
read them with the probe's reader, so the counts are not typed beside the assertion. pathlibin the stdlib allowlist, with its reason beside it, is consistent with the
no-device-runtime rule.- Everything on the deliberately-untouched list is untouched.
For the next task in this area
provenance.methodisKind.TEXT, so the caller's claim is unconstrained:method="made it up"composes a fragment and prints into the stanza. That is SPEC-1's landed decision about
the field, not this PR's doing, and narrowing it would refuse documents the schema accepts —
but the moment the claim became the caller's is the moment it is worth writing down that
nothing checks it.- Whoever builds the CLI derives its statement of reach from
ASKABLE_OF_A_DOCUMENT. That
instruction now has a referent in the code, and the count it derives from is sound on partial
documents. - Finding B belongs with finding 5's handoff lines: both are about what a fragment can honestly
say about where its numbers came from.
🤖 Generated with Claude Code
| nobody measured behind itself. So the walk yields its refusals and carries on, | ||
| and the document is checked field by field whatever it holds elsewhere. | ||
|
|
||
| **The same record makes the opt-in conditions legible.** Three of the five in |
There was a problem hiding this comment.
Finding A (non-blocking). The reach sentence round 1 named is gone and ASKABLE_OF_A_DOCUMENT
genuinely closes it — I checked the test holds in both directions, not only when a condition is
removed. But a second typed count of the check set survives in the same paragraph, one line
up from where the first one was: "Three of the five in CONDITIONS". Nothing reads it, and the
argument two paragraphs below — that writing the number down in prose "puts a reviewer between
the check set and the statement about it" — applies to this sentence exactly as it applied to the
other one.
It is not hypothetical. The child branch stacked on this one adds a sixth condition, and its own
diff already rewrites this very sentence to "Four of the six in CONDITIONS ... two of them
tp_widths=, one observed_stack=, and one a Merge". So the restack note's item 3 — "the
paragraph no longer states a number at all ... no count needs editing anywhere" — is true of the
reach clause and not of this one: the resolver must keep the child's count edit here while
dropping its reaches five of the six clause. I computed the merge in memory (git merge-tree,
base 4b3a0e521) rather than rebasing anything, and this paragraph is one of the three
conflicting hunks in this file, with the child's side carrying that edit.
Two ways out, both cheap and neither this review's call: derive the number the way the reach
number is now derived (the opt-in conditions are exactly the ones whose reason strings mention an
argument, so a tuple would do it), or drop the count and say "the conditions that need something
from the caller" without one. What should not survive is the restack note's claim that nobody
needs to edit a count in this paragraph, because somebody does.
Reporting it, not fixing it.
| machine: str, | ||
| authored_by: str, | ||
| date: str, | ||
| method: str, |
There was a problem hiding this comment.
Finding B (non-blocking), and the evidence for my ruling on the single provenance.method.
Making method the caller's is right, and the under-claim you state — a caller saying
hand-written labels two genuinely probed counts hand-written — is a fair trade to take
deliberately. But the field is not only a label. merge reads it to decide whether a fragment is
a transfer: Fragment.transferred_from is any method beginning transferred-from:, and
validate then asks that fragment for the source spec's stack pins.
So the one non-probed value this argument makes reachable — and the one your own test exercises,
method="transferred-from:mi300x-8gpu" — turns the whole tier-0 fragment into a transfer.
Measured at 814a612ca, that fragment merged with the tier-1/2/link fragments and validated:
merged host.cpu: {'cores_physical': 96, 'cores_logical': 192} provenance.method: mixed
REFUSAL Rule.PINNED_STACK 'tokenizer' (machine 'mi355x-8gpu-2node',
transferred-from:mi300x-8gpu, by ana on 2026-09-22) carried constants over from
'mi300x-8gpu' without saying which stack they were measured against
The remedy that refusal offers — "a transfer carries both specs' stack pins; these constants move
with the compute stack" — is about device.software_pinned_to, and tokenizer_fragment composes
a document of schema_version, name, provenance and host only. There is no argument that
would let it write a device stack pin, so through this function the refusal cannot be satisfied at
all: an author whose rates really did come from another spec either states it honestly and cannot
merge, or states probed and reintroduces exactly the defect this round closed.
I am not asking for it to be solved here, and I do not think it blocks: it refuses loudly rather
than producing a wrong number, and D26 defines transferred-from: for the width-keyed runtime
constants rather than for tokenizer rates, so what a transferred rate should stamp is
genuinely unspecified. What I would not leave standing is the record saying the residual cost is
only an under-claim. It is that, plus one value of the field that changes what the verb does.
Worth a line beside finding 5's two handoff lines — both are about what a fragment can honestly
say about where its numbers came from.
Reporting it, not fixing it.
Pin re-verification of #125 (SPEC-3a) at
|
| head | 814a612cac8d0c611068b471c7fe2a00232419d6 (814a612ca) |
| base ref | compass/spec-2-followups |
| base sha | 134fa56bb2ab6c6cb9e1644aaa331d2bf04c3a17 (134fa56bb) |
| draft | true |
| labels | none |
mergeable / mergeable_state |
true / clean |
updated_at |
2026-09-22T02:30:48Z — seven seconds after the round-2 verdict comment |
Base sha determined four ways, all agreeing:
gh api repos/jgong5/ATOM/pulls/125 --jq .base.sha→134fa56bbgh api repos/jgong5/ATOM/git/ref/heads/compass/spec-2-followups --jq .object.sha→134fa56bb(the base branch's current tip, not a recorded one)gh api .../compare/compass%2Fspec-2-followups...compass%2Fspec-3a-checks-ask→merge_base = 134fa56bb,behind_by = 0,ahead_by = 2,status: "ahead"git merge-base 814a612ca 134fa56bbin the local clone →134fa56bb
behind_by = 0 and the base branch tip equal to the recorded base sha is what "not stale"
means here: the base has not moved since the round-2 review pinned to it, the two commits on
the branch are the two the review read, and 4b3a0e521 still exists so round 1 → round 2 can
still be diffed directly. Nothing about the range the approval was given on has changed.
Instrument — re-verified at this head, not taken
git grep 'compass.spec|compass/spec' 814a612ca -- . ':!atom/compass/spec/*' names exactly two
modules: tests/compass/test_spec_schema.py and tests/compass/test_spec_verbs.py. So
pytest tests/compass is a sound mutation instrument for this package at this head, and the
#136 measurement still holds. One extra reading taken while checking it, which matters below:
grep -rln "__doc__" tests/ atom/compass/returns nothing. No test in the tree reads any
docstring of anything, and the gate scripts contain no phrase scan either.
I ran the full gate on every mutant I call invisible regardless.
Published counts, reproduced before any mutant
Staged with git archive 814a612ca + docker cp into xiaobizh_n18_cpu at
/tmp/pr125pinaudit_xz — my own path, nothing written into the shared mount, no other agent's
staging touched, ControlMaster closed at the end. Tarball md5 6047fccd7e21548c999bd3f5b2d5980e
verified on both ends. .compass-commit / .compass-changed written from the same rev-parse
that produced the archive. The tree's own scripts/compass was used (tree object
9091c1dc80a61a386a7f5aa4f0470bfee3371a1c); nothing was overlaid. atom.__file__ printed and
confirmed under the staged root before any count was read. Every run bounded with
timeout -k 10. No gate run was piped: each was captured to a file and the file read.
| this PR's record / the round-2 review | reproduced here | |
|---|---|---|
| gate passed | 4692 | 4692 |
| failed / errors | 0 / 0 | 0 / 0 |
| skipped | 149 | 149 |
| xfailed | 3 | 3 |
GATE_CPU_RC |
0 | 0 |
gate commit: stamp |
— | 814a612ca (stamp) |
pytest tests/compass (module instrument) |
— | 736 passed |
The three-way flake (TestTheRegionIsNotCopiedPerChunk) did not fire in any of the nine full
gate runs below.
Three conditions, met
- Line-count-preserving. The harness refuses to write a mutant that changes a file's line
count; every mutant below was written, so every one preserved it. File line counts at head:
validate.py358,machine.py242,probes.py146,rules.py103,__init__.py91. - Null control, first.
m0: two comment lines inprobes.pyreplaced by two others of the
same count. 736 passed — identical to baseline. The harness reddens nothing by itself. - Never concurrent. One mutant at a time against one staged tree; both packages restored
from a pristine copy and all__pycache__cleared before and after every single run. - One mutant discarded, disclosed. The first run of
p1(the prose mutant) is thrown
away: the harness read each edit from the pristine copy instead of from the working file, so
the fourvalidate.pyedits overwrote one another and only the last was applied. It was green
either way, but it did not test what it claimed to. Harness fixed,p1re-run from scratch,
and only the re-run is reported. No mutant failed to compile; noIndentationError; nothing
else was discarded.
The table
Published baseline for every row: pytest tests/compass = 736 passed / 736 collected, and
the full gate = 4692 passed, 0 failed, 0 errors, 149 skipped, 3 xfailed, GATE_CPU_RC=0.
"reinstated" is the same instrument after the mutant. Denominators are stated because a bare
"2 failed" says nothing about how many ran (principle 7). Full gate run on every row whose
module result was green, and on nothing else.
Verdict key — bites: the credited test(s) go red. inert: the defect is reinstated and
nothing in the tree notices. blind: the property was never pinned by anything, so there is
nothing to be inert. names-the-wrong-defect: red, but on a different test from the one
credited.
| # | mutant — defect reinstated | module: reinstated | gate: reinstated | failing node id(s) → assertion that fired | verdict |
|---|---|---|---|---|---|
| m0 | null control — two comment lines swapped for two others in probes.py |
736 passed / 736 | — | none | control green |
| Finding 1 — the walk collects | |||||
| m1a | exact pre-fix _complete: first walk refusal only, field loop skipped |
2 failed, 734 passed / 736 | — | test_one_mistyped_key_does_not_take_the_rest_of_the_document_with_it → :698 Failed: DID NOT WARN … StackMismatch; test_a_block_holding_a_scalar_does_not_take_the_rest_with_it_either → :722 assert 0 == 1 (rules.count(Rule.DERATE)) |
bites |
| m1b | half revert: all walk refusals kept, field loop still skipped | 2 failed, 734 passed / 736 | — | same two, same assertions | bites |
| m1c | unknown-key branch of _survey returns instead of continuing |
736 passed / 736 | 4692 / 0 / 149 / 3, rc=0 | none | blind — see finding D |
| m1d | scalar-block branch of _survey returns instead of continuing |
1 failed, 735 passed / 736 | — | test_a_block_holding_a_scalar_does_not_take_the_rest_with_it_either → :722 assert 2 == 1 |
bites (sole holder) |
| m1e | _walk raises the last refusal, not the first |
736 passed / 736 | 4692 / 0 / 149 / 3, rc=0 | none | blind — see finding A |
| m1f | _walk raises nothing (control for m1e) |
11 failed, 725 passed / 736 | — | 9 × test_spec_schema.py::test_a_deployment_knob_is_refused_and_told_where_it_lives[…], ::test_the_closed_schema_refuses_a_key_no_rule_names, test_spec_verbs.py::test_a_fragment_is_read_against_the_same_closed_schema |
bites (sibling module) |
| m6a | _complete returns read=True for a non-mapping subject |
1 failed, 735 passed / 736 | — | test_a_document_that_is_not_a_mapping_is_refused → :679 assert [c.split(" -- ")[0] for c in checked.not_asked] == list(CONDITIONS) |
bites (sole holder) |
Finding 2 — asked_in_part split |
|||||
| m2a | pre-fix filing: a partial reach goes into not_asked |
1 failed, 735 passed / 736 | — | test_a_question_asked_of_part_of_what_it_reads_is_not_called_unasked → :640 ValueError: not enough values to unpack (expected 1, got 0) on (in_part,) = checked.asked_in_part |
bites (sole holder) |
| m2b | the mirror: a fully-unreached condition goes into asked_in_part |
1 failed, 735 passed / 736 | — | test_a_question_none_of_whose_fields_resolved_was_not_asked_at_all → :664 ValueError: not enough values to unpack (expected 1, got 0) |
bites (sole holder) |
| m2c | __str__ renders asked_in_part under the not asked: label |
1 failed, 735 passed / 736 | — | test_a_question_asked_of_part_of_what_it_reads_is_not_called_unasked → :647 assert 'asked in part' in 'refused: 2 refusal(s)…' |
bites (sole holder) |
| m2d | STACK call site: partial filed as unasked | 736 passed / 736 | 4692 / 0 / 149 / 3, rc=0 | none | blind — finding B |
| m2e | TRANSFERS call site: partial filed as unasked | 736 passed / 736 | 4692 / 0 / 149 / 3, rc=0 | none | blind — finding B |
| m2f | WIDTHS call site: partial filed as unasked | 2 failed, 734 passed / 736 | — | test_a_question_asked_of_part_of_what_it_reads_is_not_called_unasked → :640; test_a_question_none_of_whose_fields_resolved_was_not_asked_at_all → :664 |
bites |
| m2g | STACK call site deleted outright | 736 passed / 736 | 4692 / 0 / 149 / 3, rc=0 | none | blind — finding B |
| m2h | TRANSFERS call site deleted outright | 736 passed / 736 | 4692 / 0 / 149 / 3, rc=0 | none | blind — finding B |
| m2i | partial seeded with one spurious entry (over-report control) |
4 failed, 732 passed / 736 | — | incl. test_a_clear_check_says_which_conditions_it_could_not_ask → :571 assert ('a question…',) == () |
bites — finding E |
| Finding 3 — the caller's method, and the entry check | |||||
| m3a | method: str = "probed" — the default restored |
1 failed, 735 passed / 736 | — | test_how_the_rates_were_obtained_is_the_callers_claim_and_has_no_default → :1258 Failed: DID NOT RAISE TypeError |
bites (sole holder) |
| m3b | table(entries) removed from tokenizer_fragment |
2 failed, 734 passed / 736 | — | test_an_entry_that_is_not_a_tokenizer_entry_is_refused_here → :1132 DID NOT RAISE SpecRefusal; test_an_entry_without_the_derate_its_rates_oblige_is_refused_here → :1146 DID NOT RAISE SpecRefusal |
bites |
Finding 4 — Rule.MEASURED, and the parsed topology |
|||||
| m4a | _unreadable raises Rule.SHAPE again |
2 failed, 734 passed / 736 | — | test_a_host_that_publishes_no_topology_is_refused_rather_than_halved → :1090 assert Rule.SHAPE is Rule.MEASURED; test_a_topology_that_publishes_no_number_is_refused_rather_than_counted → :1110 same |
bites |
| m4b | _reading returns the raw string — the pre-fix "counted rather than refused" |
1 failed, 735 passed / 736 | — | test_a_topology_that_publishes_no_number_is_refused_rather_than_counted → :1108 DID NOT RAISE SpecRefusal |
bites (sole holder) |
| m4c | tripwire: raise SystemExit in _reading's OSError arm |
1 failed, 735 passed / 736 | — | test_a_host_that_publishes_no_topology_is_refused_rather_than_halved → probes.py:94 SystemExit |
reached |
| m4d | tripwire: raise SystemExit on the "lists no processor" arm |
1 failed, 735 passed / 736 | — | same test → probes.py:106 SystemExit |
reached |
The reach residue (ASKABLE_OF_A_DOCUMENT) |
|||||
| m5a | ASKABLE_OF_A_DOCUMENT = CONDITIONS |
1 failed, 735 passed / 736 | — | test_a_document_reaches_what_the_package_says_a_document_reaches → :820 assert set(CONDITIONS) - unasked == set(ASKABLE_OF_A_DOCUMENT) |
bites |
| m5b | one further condition (STACK) dropped from the tuple |
1 failed, 735 passed / 736 | — | same test, :820 |
bites |
| m5c | the tuple names a condition outside CONDITIONS |
2 failed, 734 passed / 736 | — | test_the_check_set_names_every_condition_a_spec_can_be_refused_by → :800 assert set(ASKABLE_OF_A_DOCUMENT) <= set(CONDITIONS); test_a_document_reaches_what_the_package_says_a_document_reaches → :820 |
bites |
| Always-empty constants and ordering (probe 2/3) | |||||
| m7 | WIDTH_TABLES always empty |
6 failed, 730 passed / 736 | — | incl. test_a_width_the_deployment_will_use_and_nobody_measured_is_refused, test_each_condition_in_the_check_set_is_earned_by_a_spec[…widths…] |
bites |
| m8 | STACK_PINS always empty |
736 passed / 736 | 4692 / 0 / 149 / 3, rc=0 | none | blind — finding B |
| m9 | CONDITIONS reordered |
1 failed, 735 passed / 736 | — | test_a_document_that_is_not_a_mapping_is_refused → :679, on the order of list(CONDITIONS) |
bites, incidentally |
Round 1's own pins (base 134fa56bb → 4b3a0e521) |
|||||
| r1a | phase boundary fully reinstated (widths and stack gated on spec is not None) |
4 failed, 732 passed / 736 | — | the same four as r1b |
bites |
| r1b | width half only | 4 failed, 732 passed / 736 | — | test_a_desk_fix_… → :616 assert 0 == 2; test_a_question_asked_of_part_… → :639 assert False; test_one_mistyped_key_… → :708 assert 0 == 2; test_a_block_holding_a_scalar_… → :723 assert 0 == 2 |
bites |
| r1c | stack half only | 2 failed, 734 passed / 736 | — | test_a_desk_fix_… → :606 DID NOT WARN … StackMismatch; test_one_mistyped_key_… → :698 DID NOT WARN … StackMismatch |
bites |
| r1d | cpu_counts counts core_id alone, ignoring the package |
2 failed, 734 passed / 736 | — | test_one_core_id_on_two_packages_is_two_cores → :1081 assert cores_physical 48 == 96; test_the_same_pair_merges_when_the_counts_agree → merge.py:198 SpecRefusal Rule.ONE_MACHINE |
bites |
| r1e | the fragment no longer carries host.cpu (the named result's mechanism) |
2 failed, 734 passed / 736 | — | test_the_probes_counts_refuse_the_pair_that_used_to_merge → :1192 DID NOT RAISE SpecRefusal; test_the_same_pair_merges_when_the_counts_agree → :1220 assert (tier0,) == (tokenizer-node, tier0) |
bites |
| Prose | |||||
| p1 | nine false claims restored across validate.py, machine.py, probes.py, rules.py, __init__.py — verbatim, reworded, duplicated beside the correct text, with a number flipped, and with no number at all |
736 passed / 736 | 4692 / 0 / 149 / 3, rc=0 | none | blind — finding C |
Counts: 34 mutants plus the null control. 26 go red. 8 stay green — m1c, m1e, m2d, m2e, m2g, m2h, m8, p1. 0 inert: every pin whose defect I actually reinstated went red, so no test in this PR passes with its own defect back in. 0 names-the-wrong-defect: every red mutant reddened the test the review credits, and no other. The 8 green ones are blind spots — properties claimed in prose or in the PR body that no test ever held. Every one of the 8 was confirmed at the full gate, not only at module level, and no sibling test caught any of them.
reinstated and ignored by the test that claims it). 8 blind** — m1c, m1e, m2d, m2e,
m2g, m2h, m8, p1. 0 names-the-wrong-defect. Every blind one was confirmed at the
full gate, not only at module level, and no sibling caught any of them.
Findings
None of these changes #125's approval. They are all about what the suite would catch later,
not about what the code does now, and two of the three are properly remedied by a docstring
correction rather than by a new test.
A. (blind spot) _walk "raises the first" is held by no test in the tree
The claim is made three times — machine.py's module docstring ("a reader and raises the
first thing wrong"), the PR body ("_walk is two lines over it and raises the first, so
MachineSpec.from_mapping and Fragment.from_mapping are unchanged"), and the round-2 review,
which calls it "the regression risk I checked hardest" and measures it three ways by hand.
m1e makes _walk raise the last refusal instead of the first, one line, line-count
preserving:
- for refusal in _survey(node, prefix, found):
+ for refusal in list(_survey(node, prefix, found))[-1:]:
raise refusalpytest tests/compass: 736 passed. Full gate: 4692 passed, 149 skipped, 3 xfailed,
GATE_CPU_RC=0 — bit-for-bit the published counts.
Every subject in the suite that reaches from_mapping through a walk earns exactly one
walk refusal, so first and last are the same object and the ordering is never exercised. The
control m1f (_walk raises nothing) is caught by 11 tests, so that _walk still
refuses is well pinned; only which refusal it picks is not. Principle 8: the claim is stated
in three places and measured in none of them that survives the session it was measured in.
Remedy is one test, not a docstring change — the claim is true, it is just unheld. A
subject with two unknown keys asserted to raise on the first in document order would close it.
B. (blind spot, two sites and only one bites) Finding 2's split is discriminated at one of its three call sites
_reach calls its local reached() helper three times — reached(WIDTHS, WIDTH_TABLES),
reached(STACK, STACK_PINS), reached(TRANSFERS, STACK_PINS). Each was mutated separately:
| mutant | what it does | pytest tests/compass |
full gate |
|---|---|---|---|
m2f |
WIDTHS site: partial filed as unasked | 2 failed, 734 passed | — |
m2d |
STACK site: partial filed as unasked | 736 passed | 4692 / 0 / 149 / 3, rc=0 |
m2e |
TRANSFERS site: partial filed as unasked | 736 passed | 4692 / 0 / 149 / 3, rc=0 |
m2g |
STACK site deleted outright (pass # reached(...)) |
736 passed | 4692 / 0 / 149 / 3, rc=0 |
m2h |
TRANSFERS site deleted outright | 736 passed | 4692 / 0 / 149 / 3, rc=0 |
m8 |
STACK_PINS = (...)[:0] — the constant both sites read, always empty |
736 passed | 4692 / 0 / 149 / 3, rc=0 |
Two of the three sites can be silenced entirely and the gate does not move. m8 is the
always-empty-constant case in its purest form: the tuple that the stack and transfer questions
read can be empty and nothing notices, because nothing ever asks either of them of a document
whose stack pins failed to resolve.
This does not make finding 2 unclosed — m2a, m2b, m2c and m2f all bite, and the split
itself is real and pinned. It means the property is pinned only through the width tables, so
a later change that re-merges the two lists for the stack or transfer question lands green.
C. (blind spot) Nothing in the tree reads prose, in any form
p1 falsifies nine claims across five modules in one mutant, in every form asked for —
verbatim restoration of text this PR deleted, reworded restatement, a duplicate sentence beside
the correct one, flipped numbers, and a claim with no number at all:
| # | file | what was restored / falsified |
|---|---|---|
| 1 | validate.py:46 |
"Three of the five in CONDITIONS" → "One of the nine in CONDITIONS" (a number flipped on the line that is already finding A of round 2) |
| 2 | validate.py |
round 1's deleted "and so reaches four of the five" restored verbatim, and "one no result can report on" → "one every result can report on" |
| 3 | validate.py |
the ASKABLE_OF_A_DOCUMENT paragraph reversed, with no number at all: "The reach of that form is a sentence in this paragraph, not a value… a reviewer reading this paragraph" |
| 4 | validate.py |
"must not be counted as reach" → "must still be counted as reach"; "was asked" → "was not asked" |
| 5 | machine.py |
"a reader and raises the first thing wrong" → "…the last thing wrong" |
| 6 | probes.py |
"The rates are the caller's" → "The rates are measured here" |
| 7 | probes.py |
a duplicate contradicting sentence beside the correct text: "provenance.method is always probed, because everything in this fragment was probed by this function" |
| 8 | __init__.py |
the pre-fix paragraph restored verbatim — "it emits the rates a tokenizer sweep measured" — plus "The entries are not read against any table here" |
| 9 | rules.py |
"The last is not about a document" → "The last is about a document like the rest" |
pytest tests/compass: 736 passed. Full gate: 4692 passed, 149 skipped, 3 xfailed,
GATE_CPU_RC=0 — identical to the published run, exactly as on #136's memory.py.
That is not a defect in #125's tests; nothing in this tree ever pinned a docstring and no test
here claims to. It is the measurement, so that the next reader of these paragraphs knows what
holds them: nothing. Four of the nine falsify statements this PR's round 2 was specifically
asked to make true (findings 1, 3 and the reach residue). Principle 8 again.
D. (weak pin, not a defect) The mistyped-key subject cannot see a walk that stops inside a block
m1c makes the unknown-key branch of _survey return instead of continuing:
except SpecRefusal as refusal:
- yield refusal
+ yield refusal; return736 passed; full gate 4692 / 0 / 149 / 3, rc=0. The reason is in the subject, and it is
structural rather than accidental:
memory["capacity_byte"] = memory.pop("capacity_bytes")pop then re-assign puts the mistyped key last in its own block, so a walk that abandons
the rest of device.memory abandons nothing, and _survey's recursion returns to the parent
and carries on with the rest of device anyway. The whole-document revert is caught — m1a
(exact pre-fix semantics: first walk refusal only, field loop skipped) and m1b (the half
revert: all walk refusals, field loop still skipped) both fail two tests. The within-a-block
case is not. Moving the mistyped key to a non-final position in its block, or mistyping a key
that is not last, would close it.
E. (note) The two asked_in_part == () lines added to test_a_clear_check_... catch over-reporting only
assert bare.asked_in_part == () and ... and asked.asked_in_part == () are satisfied by a
field that is always empty — which is precisely the defect finding 2 exists to prevent.
Measured in both directions: m2i (partial seeded with one spurious entry) does redden
them, at test_spec_verbs.py:571; m2a, m2g and m2h, which all leave asked_in_part
empty where it should not be, do not. The property that matters is held by
test_a_question_asked_of_part_of_what_it_reads_is_not_called_unasked, which unpacks
(in_part,) = checked.asked_in_part and therefore cannot be satisfied by emptiness. No change
needed; recorded so the two lines are not mistaken for a pin on the under-reporting direction.
The question you asked hardest: is each round-1 finding closed by the test the review credits?
Yes, all six — no mis-attribution. This is the part of #136's result that does not
repeat here.
| round-1 finding | closed by, per the round-2 record and review | does that test bite when the defect is reinstated? |
|---|---|---|
| 1 walk collapses on one unknown key | test_one_mistyped_key_does_not_take_the_rest_of_the_document_with_it, test_a_block_holding_a_scalar_does_not_take_the_rest_with_it_either |
Yes, both (m1a exact revert, m1b half revert). And the second one alone bites the scalar-block site alone (m1d) — the two sites are separately discriminated. Caveat in finding D. |
1' consequence: non-mapping subject names MISSING/DERATES |
test_a_document_that_is_not_a_mapping_is_refused (the assertion round 2 added) |
Yes (m6a), and it is the only test in the tree that does |
2 not_asked held conditions that were asked |
test_a_question_asked_of_part_of_what_it_reads_is_not_called_unasked, test_a_question_none_of_whose_fields_resolved_was_not_asked_at_all |
Yes, both (m2a, m2b, m2c, m2f) — each is the sole holder of its side. Site caveat in finding B. |
3 method: probed stamped over rates it did not measure |
test_how_the_rates_were_obtained_is_the_callers_claim_and_has_no_default (the method= half); test_an_entry_that_is_not_a_tokenizer_entry_is_refused_here + test_an_entry_without_the_derate_its_rates_oblige_is_refused_here (the table(entries) half) |
Yes, all three, and the two halves are discriminated by different mutants (m3a vs m3b) |
4 Rule.SHAPE for a probe, garbage topology counted |
test_a_host_that_publishes_no_topology_is_refused_rather_than_halved (rule), test_a_topology_that_publishes_no_number_is_refused_rather_than_counted (parse) |
Yes, both, and the parse half is held by the second alone (m4b) |
5 host.cpu.* has two meanings |
accepted, handoff only — no code, no test claimed | n/a, correctly |
| 6 the "185 of the 314" figure | PR body correction only — no code, no test claimed | n/a, correctly |
| residue the reach statement is prose nothing reads | test_a_document_reaches_what_the_package_says_a_document_reaches, and test_the_check_set_names_every_condition_a_spec_can_be_refused_by for the subset clause |
Yes, in both directions (m5a = CONDITIONS, m5b = one fewer, m5c = names an outsider). The review's own claim that "writing ASKABLE_OF_A_DOCUMENT = CONDITIONS fails it, not only removing a condition" reproduces exactly. |
Round 1's own pins hold too: the phase-boundary removal is caught by m1a/r1b/r1c
separately for the width half and the stack half, the physical-package half of cpu_counts by
r1d, and the named result's whole mechanism by r1e.
Properties held by exactly one test in the whole tree
Eight, each measured (the mutant reddened exactly one node id across all 736):
- a partially-reached condition is not filed as unasked —
test_a_question_asked_of_part_of_what_it_reads_is_not_called_unasked(m2a) Validation.__str__rendersasked in part:under its own label — same test (m2c)- a fully-unreached condition is filed as unasked —
test_a_question_none_of_whose_fields_resolved_was_not_asked_at_all(m2b) method=has no default —test_how_the_rates_were_obtained_is_the_callers_claim_and_has_no_default(m3a)- a topology value is parsed as an integer —
test_a_topology_that_publishes_no_number_is_refused_rather_than_counted(m4b) ASKABLE_OF_A_DOCUMENTis derived by excluding exactly the transfer —test_a_document_reaches_what_the_package_says_a_document_reaches(m5a,m5b)- a non-mapping subject reports
read=False—test_a_document_that_is_not_a_mapping_is_refused(m6a) - the scalar-block branch of
_surveykeeps walking —test_a_block_holding_a_scalar_does_not_take_the_rest_with_it_either(m1d)
Item 1 and item 2 are two distinct properties held by one test, which is worth knowing if
that test is ever narrowed.
What changes #125's approval
Nothing. Findings A–E are all about coverage the suite does not have, not about behaviour
the code gets wrong; every one of the six round-1 findings is closed in code by the test the
round-2 review credits with closing it; and the published gate counts, the module counts, the
base sha and the head all reproduce. The PR stays APPROVE'd, stays a draft, and is not
stale — behind_by = 0 against a base branch still sitting at 134fa56bb.
If any of A–D is worth acting on before this lands, the cheapest order is: one test for A (a
two-unknown-key subject asserted to raise on the first), one parametrisation of the existing
finding-2 tests over the stack pins for B, and moving the mistyped key off the end of its block
for D. C is a fact about the tree, not about this PR.
Staging
/tmp/pr125pinaudit_xz inside xiaobizh_n18_cpu and
/md1/users/jgong5/agent_scratch/compass_dev/pr125-pinaudit/ on the launcher — both mine, both
removed. No other agent's staging was read or touched; nothing was written into
/tmp/xiaobizh-compass; the ControlMaster opened for this was closed.
🤖 Generated with Claude Code
1 participant
Closes #121.
Stacked. Branched from
compass/spec-2-followupsat134fa56bb(PR #115, read at branch time, unmoved), which is stacked oncompass/spec-2-verbsat1d116c85f(PR #86), which is onfeature/atomcompass_newatd175b03c6. Base iscompass/spec-2-followups, opened by REST as a draft. Nogh stackobject was created. Head of this branch:814a612ca(round 2; round 1 was4b3a0e521, and the push was a fast-forward). If a parent moves this restacks withgit rebase --ontoand retargets by REST.What was built
1. The tier-0 tokenizer fragment emits
host.cpu.*—atom/compass/spec/probes.py, newtokenizer_fragment()composes the fragment the tokenizer sweep's output belongs in: the entries it was handed, the stanza, and the core counts of the processor it read here, written intohost.cpu.cores_physicalandhost.cpu.cores_logical. Both were already required fields. No schema change —fields.pyis untouched.cpu_counts()reads the kernel's per-processor topology: the logical count is the number of processor directories, the physical count the number of distinct(physical_package_id, core_id)pairs. Countingcore_idalone would report half of a two-socket machine, and a halving looks exactly like a correct reading of a smaller host — there is a test for that, and it was independently confirmed in review against a real two-socket Xeon 8480C (112/224, matchinglscpu). A host that publishes no topology, or one whose topology file holds nothing a number can be read out of, is refused by name.The function takes the measured entries as an argument. It does not run the sweep — see What is not here — and after round 2 it no longer claims to: see §4.
2. Each consistency check is asked of whatever resolved —
validate.py_complete()keeps the values that checked out, and the consistency half runs against those whether or not they add up to a spec. A question that reads a field which did not resolve names that field instead of being skipped in silence; a question whose fields all resolved is asked and can earn its refusal beside the cheap one.Where there is no resolved spec, a
MachineSpecis built from the fields that did resolve and used only insidevalidate—Validation.specis stillNonefor an incomplete document, and the comment at the construction site and a clause inmachine.py's docstring say that totality isfrom_mapping's property, not the dataclass's.MachineSpec.check_stacknow skips a component the document does not carry; on a complete spec (all three are required) its behaviour is unchanged.The old blanket
not_askedtext — "the document is not a complete spec, so no consistency question was asked of it at all" — is gone, because it is no longer true.3. The check set is data, and the statement of reach is a value —
validate.pyCONDITIONSnames the five conditions a spec is checked for, andnot_askedis measured against it. After round 2 the reach of the document-taking form isASKABLE_OF_A_DOCUMENT, a tuple derived fromCONDITIONS, rather than a sentence in a docstring:test_the_check_set_names_every_condition_a_spec_can_be_refused_by— every condition in the list is earned by a subject that violates it, every such subject is a listed condition, and the reach tuple names nothing outside the list.test_a_document_reaches_what_the_package_says_a_document_reaches— asserts that the conditions a run did not decline are exactlyset(ASKABLE_OF_A_DOCUMENT), and that the count matches its length. No number is typed into the test and none is written in prose.merge.py's paragraph about the residue said the probes would close it and that they belonged to a later task. It now says what the code does, and the task identifier is gone with it.4. Round 2 —
machine.py,validate.py,probes.py,rules.py,__init__.pyAnswering the round-1 review. Three blocking findings, all closed in code; one non-blocking closed in code; one recorded for the handoff; one figure corrected.
The walk collects.
machine.pygains_survey, which yields a refusal per unrecognised key and per block holding a scalar and keeps walking;_walkis two lines over it and raises the first, soMachineSpec.from_mappingandFragment.from_mappingare unchanged. One typo in a hand-authored document no longer empties the document of resolved values. Measured: a merged document withcapacity_bytesmistyped,tp_widths=(1,2,4,8,16)and a moved ROCm pin earns five refusals across four rules at814a612caagainst one at4b3a0e521;device.memory = 3earns seven against one.A subject that is not a mapping names the two schema conditions.
_completereports whether it read a document at all, and where it did not,MISSINGandDERATESname themselves with their reason.len(CONDITIONS) - len(not_asked)is then 0 on that path, where it read 2.What was asked of part is no longer filed under "not asked".
Validationgainsasked_in_part. A condition none of whose fields resolved stays innot_asked; one whose fields partly resolved — which was asked, and can already have earned a refusal — goes to the new list, is rendered underasked in part:, and is counted as reached. On the review's subject the reach count reads 3, where it read 2.The fragment no longer claims a measurement it did not make.
METHODis gone;tokenizer_fragmenttakes a required keyword-onlymethod, with no default to fall through to, because the rates arrive as an argument and nothing in this package measured them. The entries are now read againstENTRY_FIELDSbytokenizers.table()at composition time, so[{"not": "a tokenizer"}]and an entry withderatedeleted — both accepted at4b3a0e521— are refused where the caller supplied them. Three docstrings (probes.pymodule,tokenizer_fragment,__init__.py:37-41) say what the function does instead of asserting a coupling the signature does not establish.A probe names a rule about probes.
Rule.MEASURED— "a probe reports what it read, and refuses what it could not" — replacesRule.SHAPEin front of a sysfs failure, where no document is involved.cpu_countsparses each topology value as an integer, so two processors publishing an emptycore_idare refused rather than counted as{'cores_physical': 1, 'cores_logical': 2}.Named result — reproduced at the round-2 head
The handoff stated the refusal as measured. It reproduces exactly at
814a612ca:host.cpu) merges cleanly with the node's fragment. They share no field, so nothing contradicts anything, and the merged document carries the node's 96.tokenizer_fragment()against a topology of 8 physical cores. Refused,Rule.ONE_MACHINE, both readings and both stanzas in the message.Held in the suite by
test_the_pair_this_closes_merged_cleanly_before_the_counts_were_emitted,test_the_probes_counts_refuse_the_pair_that_used_to_merge, andtest_the_same_pair_merges_when_the_counts_agree. The topologies are built by the test and read by the probe's own reader, so the counts are not typed in beside the assertion that checks them.Gate 1 — the suite as a delta against the stated base, both sides re-measured
Round 1's +17 does not carry forward; both sides were staged and run again at the round-2 head.
git archive+docker cpintoxiaobizh_n18_cpuon node 18 at/tmp/rev125r2gates/{branch,control}, each tree gated with its ownscripts/compass/,COMPASS_INTEGRATION_REF=fork/feature/atomcompass_new, stamps written from the samerev-parsethat produced the archive, tarball md5 verified on both ends,import atomconfirmed to resolve under each root before any count was read. Nothing was written into the shared mount; staging removed afterwards.814a612ca134fa56bbGATE_CPU_RCgpu: not requiredon both, from the.compass-changedstamp — no path in this diff is ingpu_gate_triggers.txt. Each gate's owncommit:stamp was checked against the tree it measured.The flake fired, and was re-run rather than reported. The control's first run came back
GATE_CPU_RC=1,1 failed, 4667 passed, ontests/entrypoints/test_stream_marker_properties.py::TestTheRegionIsNotCopiedPerChunk::test_no_format_pays_more_per_byte_as_the_payload_grows[buffered-region]— "cost per KB grew 1.65x from 32 to 128 KB", a wall-clock timing assertion in a file this PR does not touch and which is present at every control. Two further control runs returned4668 passed,rc=0, identically. The branch was run twice and returned4692 passed,rc=0, identically both times. The figures above are the two agreeing runs per side.The +24 are this PR's tests:
test_spec_verbs.py+test_spec_schema.pycollect 195 on the branch against 171 on the control. Round 1 contributed +17; round 2 adds +7, one per finding closed in code (two for finding 1's two walk refusals, two for finding 2's two sides of the split, two for finding 3's entry checking, one for finding 3's caller-stated method, one for finding 4's garbage topology — and the existingno topologytest absorbed finding 4's rule change rather than adding an eighth).ruff checkclean onatom/compass/spec/and both test files, on both sides.ruff format --checkclean on all seven files this PR changes.merge.pyreports one block — the two-part f-string in_conflict— and prints the identical hunk at the control, so it is pre-existing dirt in a block this PR does not touch, and is left as it was.Gate 2 — new CPU-only tests
All in
tests/compass/test_spec_verbs.pyandtests/compass/test_spec_schema.py. Nothing added reaches a device; the probe reads a directory the test builds.pathlibwas added to the package's stdlib allowlist intest_spec_schema.py, with the reason beside it: reading a path is not a device runtime, and nothing on the path that checks a document touches it.Gate 3
The named result above, reproduced at the round-2 head.
Gate 4
Reviewer agent. Round 1 returned three blocking findings and three for the record; round 2 answers all six, inline on the comment where each change landed.
Effort
Measured at
134fa56bb→814a612caover the files this PR touches — productionspec/{probes,validate,machine,merge,rules,__init__}.py, testscompass/{test_spec_verbs,test_spec_schema}.py.rules.pyjoins the set in round 2; a base→head delta cancels its pre-existing lines. Counter:agent_scratch/spec3a_r2/loc.py, not committed. It reproduces all four of round 1's figures exactly at the round-1 range, which is why the numbers below can be compared with the review's.Cumulative, base → round-2 head:
ast.stmt, docstring-onlyExprdroppedast.stmt, docstrings inRound 2 alone,
4b3a0e521→814a612ca: +18 / +62 = +80 AST; +28 / +105 = +133 strict; +118 / +138 = +256 physical non-blank; +21 / +62 = +83 AST with docstrings.Against the 150–250 envelope, plainly, and flagged rather than smoothed. AST statements total +209, inside it. The convention the previous PR in this stack settled on — physical less blanks, comment-only lines and docstring lines — totals +347, over by 39%. Physical non-blank totals +570, over by 128%, which is 2.28x the top of the estimate on that instrument and therefore crosses the halt-and-discuss line if that is the instrument the rule means. It is an open owner decision and I am not deciding it here; I am naming it because the loosest reading has moved from "over by 26%" to "over 2x" between rounds, and that is a fact the owner should have rather than one buried in a column.
Nothing was trimmed to move a column. The growth is answering three blocking findings: round 2 adds +105 strict test lines (seven tests, each holding one closed finding) and +28 strict production lines, against +123 prose lines explaining the four decisions the review asked about. Production code alone is +67 AST / +103 strict on the whole PR.
Correcting round 1's prose sentence (review finding 6). The line "185 of the 314 added non-blank lines are docstring or comment" does not reconcile, and the review is right. Re-measured at the round-1 range: net prose growth was +100 of the +314 (physical non-blank 1931 → 2245, strict 1601 → 1815, and the difference between those deltas is the prose delta). Counting gross added lines instead, the round-1 diff added 418 non-blank lines and removed 104, of which 139 added lines are docstring or comment; including blank lines inside docstrings, 146 are prose. The original sentence also paired a gross numerator with a net denominator. The conclusion — that the package's prose is why the loosest instrument runs over — is unchanged; the figures were wrong and are replaced.
At the round-2 head the same figures are: prose 361 → 584 = +223 net of the +570; gross, the diff adds 704 non-blank lines and removes 134, of which 265 added lines are docstring or comment (284 of 723 counting blanks inside docstrings). 704 − 134 = 570, which is the physical non-blank delta, so the two readings reconcile.
What is not here, and why
tokenizer_fragment()composes and checks the fragment; it does not encode and decode a length sweep and fita + b*n. That needs a loaded tokenizer and a measurement, and a fitted rate with no measurement behind it would be a number with no source. After round 2 the function says so: the entries are the caller's, checked againstENTRY_FIELDSby the same readerMachineSpec.from_mappinguses, and how they were obtained is a required argument rather than a stamp. The brief's item 1 is the emission, and the emission is complete; the sweep is unclaimed by either cut of this task and is filed as The tier-0 tokenizer fragment carries an identity but no measured rate #126.host.ipc.*and theipcprobe — not in this brief, not built.compass spec validate machine.yamlis a verb the design writes and nothing implements yet, so the package's statement of its own reach isASKABLE_OF_A_DOCUMENT, asserted against a run'snot_askedby a test. When a CLI is built its statement must come from that tuple, or the test stops holding anything.non_torchand the probe-table hole are SPEC-3b - the non_torch absolute refusal, and D26 probe-table hole #122's, whose PR SPEC-3b - the absolute memory refusal, and the probe table hole that stays a hole #136 stacks on this branch. Untouched.provenance.dateis not narrowed — an owner decision, and narrowing it would refuse documents the landed schema accepts.ranks.py:16-17is unchanged. It names the device a device-wide reading was read off, which is true.host.cpu.*now carries two meanings in flight — the machine's cores, which is what the schema declares, and the composing host's, which is what this function writes — and the probe table assigns no probe to the field, so only one of the two is checkable. That is review finding 5, accepted and not solved here; it goes into the closing handoff on SPEC-3a - the checks ask for what resolved, and the tier-0 probe supplies it #121 for whoever builds theipcprobe or the CLI.Blocking issues
None.
🤖 Generated with Claude Code