Skip to content

compass(spec): merge, validate and explain over the machine spec (SPEC-2) - #86

Merged
jgong5 merged 2 commits into
feature/atomcompass_newfrom
compass/spec-2-verbs
Sep 23, 2026
Merged

jgong5 merged 2 commits into
feature/atomcompass_newfrom
compass/spec-2-verbs

Conversation

@jgong5

@jgong5 jgong5 commented Sep 21, 2026 •

Copy link
Copy Markdown
Owner

Closes #73 once reviewed and landed. Based directly on feature/atomcompass_new
(it was stacked on #79, which has since landed); the diff below is this task's
two commits.

The three verbs over the schema — merge, validate, explain — plus the
reduction that turns one reading per rank into the one number a spec carries.
No probe: nothing here reads a device.

Dev record

The conflict check, and what the schema cannot see

Merging fragments is arithmetic over dotted paths. The check inside it is the
feature.

One spec describes one host, and nothing in a fragment's shape says which host
it came from.
Two tokenizers measured on two CPUs contradict nothing: they
have different identities, they occupy different entries, and no field of one
overlaps a field of the other. A merge that compared only values combines them
into a document that describes no machine, and the error stays invisible for as
long as the spec lives. The closed schema cannot catch it either — both
documents are perfectly legal.

So the check has two arms, and between them they cover the two ways a multi-host
spec gets authored:

Arm Catches Refused as
Named origin. Every fragment states the machine it claims to be for, plus who, when and how. Two fragments naming two machines are refused, printing both stanzas. the disjoint contribution — two tokenizers, two IPC measurements, two of anything that does not overlap ONE_MACHINE
Contradiction. Two fragments that name one machine and give one field two values cannot both be true of it. the mislabelled fragment — a stanza that says one host over readings from another ONE_MACHINE, or PINNED_STACK when the field is a stack pin

The first arm is the one the schema cannot replace. The refusal names both
stanzas rather than the bare machine names, because what a reader needs is which
two measurements are being combined:

one spec describes one host: 'tokenizer-a' (machine 'node-18', probed, by ana on
2026-09-18) and 'tokenizer-b' (machine 'node-22', probed, by bo on 2026-09-19)
were measured on different machines, 'node-18' and 'node-22'. one spec describes
one host; numbers from two hosts in one document describe neither, and nothing
in their shape would ever say so. Author a spec per machine

Named result. That refusal, against the same two fragments — same
tokenizers, same authors, same dates — merging cleanly when both name node-18,
producing one document with both entries and provenance.fragments naming both.
test_two_hosts_in_one_spec_are_refused_and_both_provenances_are_named and
test_the_same_two_tokenizers_merge_when_the_machine_agrees.

What the first arm compares is the declared name, and there is a cross-host
pair it misses.
name is the machine the spec is being authored for — the
schema's own example mi355x-8gpu-2node is a machine class — and no field
records where a probe actually ran; there is no measured_on. So a tokenizer
fragment probed on a laptop and a device fragment probed on node-18, both
authored name: node-18, merge cleanly
, precisely because they share no
field, which is the condition this module opens by naming. Arm 2 catches it the
moment they overlap (host.cpu.cores_physical 8 against 96 →
ONE_MACHINE). The arm still earns its place — it reduces the hazard — and the
residue is the case D26's own open issue has already decided to live with
(tokenizer throughput transfers across CPUs of comparable class).

Closure needs no schema change, and it is SPEC-3's, not this PR's. Have the
tier-0 tokenizer probe emit host.cpu.cores_physical / cores_logical
alongside the rates: both are already required fields, and emitting them turns
exactly the pair above into an arm-2 contradiction. The probes are written in
SPEC-3, so that is where it belongs; it is stated here rather than left with the
docstring claiming more than the code can support, and the docstring has been
narrowed to what is enforced — a fragment states the machine it claims to be
for
, corroborated by nothing except overlap.

Three kinds of field are combined rather than compared, because contributing
part of one is what a probe does:

  • a width-keyed runtime constant, per width. This is not a nicety: the
    single-card probe fills width 1 and the multi-rank probe fills 2, 4 and 8, so
    a merge that treated the term as one value would refuse the two probes the
    design pairs. The same width measured twice differently is still a conflict.
  • the tokenizer table, per entry, keyed by identity and the implementation
    the rates were measured on — one tokenizer measured on both the fast and the
    slow backend is two legitimate entries, not a conflict. Two entries sharing an
    id must agree on fingerprint; one name over two files is exactly the drift
    that keying by tokenizer exists to prevent, so it is refused ahead of the
    generic value conflict, which would otherwise print two entry dicts at the
    reader.
  • the provenance block, rebuilt rather than merged: every fragment named,
    the latest date, and one method only when the fragments agree on one — a
    spec built from a datasheet and an engine run is honestly mixed.

A merged document merged again keeps the names that built it. Incremental
authoring is the obvious workflow — merge what you have, save machine.yaml,
merge next week's probe into it — and Fragment.from_mapping accepts a full
document, so it works. It used to lose provenance.fragments from the first
round, in the one block whose job is to say where the numbers came from; any
provenance.fragments a fragment already states is now carried forward ahead of
its own source name
(test_a_merged_document_merged_again_keeps_the_names_that_built_it). And what
"the latest date" means is now stated where it is computed: the lexicographic
maximum of text fields, under which ISO-shaped dates order correctly and
'2026-9-9' does not.

validate against the design's refusal list

The design refuses on Implemented as Rule
a missing required field every field of the table, reported together SHAPE
a TP width absent from runtime_constants that the deployment will use tp_widths= argument, checked against both width tables NO_DEFAULTS
a derate missing beside a spec-peak number the derived derate field is missing, and says which peak obliged it DERATE
software_pinned_to not matching the running stack (warn, or refuse under a strict flag) observed_stack= warns and names both; strict=True also refuses PINNED_STACK
a transfer fragment whose source spec pinned a different stack per transfer fragment, its declared source pin against the resolved spec's PINNED_STACK

One row is checked in test_the_refusal_list_is_the_one_the_design_asks_for,
which builds all six conditions side by side so the list can be read against
its source in one place. A runtime constant missing altogether is reported under
NO_DEFAULTS rather than SHAPE, which is the schema's own distinction, not a
sixth condition.

Refusals are collected, not raised. Reading a document stops at the first
thing wrong with it, which is right for a reader that must produce a value and
wrong for a check whose user is filling in a form: told one missing field per
attempt, they measure one term, run again, and meet the next. validate returns
a Validation — every refusal in schema order, the stack differences that only
warned, the conditions it could not ask, and the resolved spec when there is one
— and raise_first() is there for a caller that wants an exception.

Two phases, and the order has a cost this PR records rather than fixes.
Completeness first, consistency second, because each consistency check goes
through a resolved spec. A phase-one refusal therefore hides a phase-two one,
and the cost ordering is inverted — the refusal that survives is the free one:

round 1: 1 refusal,  spec resolved=False
   - DERATE | `device.compute.derate` is missing, and its block states a spec peak
round 2 (after the derate is typed in): 3 refusals
   - NO_DEFAULTS | `driver_and_collective_reserve_bytes` not measured at width 8
   - NO_DEFAULTS | `allocator_retained_after_load_bytes` not measured at width 8
   - PINNED_STACK | constants measured against rocm '7.2.4' (now '7.3.0')

One value an author types in at a desk hid an unmeasured width 8, which costs an
eight-GPU reservation. The justification this PR gave in round 1 — "a
consistency question needs a resolved spec to ask of" — is true of the
implementation and not of the questions: both width tables were present and
schema-valid in the round that refused
, and phase one checked them itself with
no complaint. That claim has been withdrawn from the docstring.

What was done about it. Asking each consistency check of whatever fields did
resolve is the real fix, and it is not done here — it is a restructuring of both
phases with its own tests, at the end of a task already past its effort halt.
What is done is to make the silence legible: Validation gains not_asked,
the conditions this run could not ask and why, surfaced by __str__. A run
whose first phase refused now says that no consistency question was asked of it
at all, and that a refusal above can be hiding a more expensive one
(test_an_incomplete_document_says_no_consistency_question_was_asked). The
restructuring is in the handoff below.

The same field closes a second gap: three of the five conditions are opt-in
and were silently skipped.

condition needs was silently skipped when
unmeasured width tp_widths= left at ()
stack moved observed_stack= left at None
transfer out of a differently-pinned spec a Merge given a document

The sharpest instance: the same spec is ok=False as a Merge and ok=True
as the merged document
, because decision 3 below keeps the transfer's source
pin out of every field of the document — and D26 writes the verb as compass spec validate machine.yaml, which is exactly the form that cannot ask it. That
is not a wrong number, and the decision stands; but validate(document) now
names the condition it could not ask and says where it can be asked instead
(test_the_transfer_condition_names_itself_as_unaskable_of_a_document,
test_a_clear_check_says_which_conditions_it_could_not_ask). Carrying the
source pin into a provenance.method that keeps it is a schema question and
belongs with the owner.

What explain recovers, and how

Given a spec and a name, it reports the fields under that number: the value the
document holds, the derate that applies, the derated value, and the fragment
that supplied it. It runs over a MachineSpec — that is, over the echo a run
artifact already carries — so a number in an artifact is traceable without the
fragments still being on disk; pass the Merge as origin= and each row also
names the fragment.

host.ipc, from spec sha256:ab34...
  host.ipc.zmq_roundtrip_s = 5e-05  [tier0]
  host.ipc.shm_broadcast_s = 2e-05  [tier0]

Three things it deliberately does not do. It never totals. Each field is a
line in its own unit; the arithmetic that combines them belongs to whatever
consumes them, and a sum over seconds, bytes and bytes-per-second would be a
fiction — the memory model's own summed check once read +13.8% while hiding
three errors, two of which cancelled. It shows a spec peak twice, with the
derate between them, because that is the one decomposition available here and
the gap is a declared judgement rather than a measurement. A width-keyed
constant explains at every measured width
, or at the width asked for; asking
at a width nobody measured refuses by name, the same as everywhere else.

Names come two ways. A field or block by dotted path — exact, nothing to
maintain. A quantity out of QUANTITIES, which lists the spec fields each
predicted quantity is built from; that table is the spec-side half of a
relationship whose other half does not exist yet, so it is checked only for
naming real fields (test_every_quantity_is_built_out_of_fields_this_schema_really_has).
A consumer that reaches for a term this table does not list is a fix to the
table, and that is stated in the module rather than left to be discovered.

The exit-criterion walk was vacuous as a correctness claim, measured rather
than assumed.
test_every_term_a_resolved_spec_holds_can_be_explained walks
all 38 paths of a resolved spec and used to assert only a non-empty basis and a
matching digest — and a path that is in spec.values cannot return an empty
basis by construction. Mutating explain.py five ways:

mutation the walk, before the walk, now the whole file
every scalar value reported as 0.0 passed failed 3 failed
derate never reported passed failed 2 failed
width table truncated to its lowest width passed failed 2 failed
tokenizer row label mismatched passed failed 3 failed
supplied_by always () passed passed 3 failed
0/5 4/5 5/5

The walk now asserts the value each row reports against the document, that a
derate is present exactly where the field is a spec peak (and, inside a
tokenizer entry, exactly on the two rates that are peaks), that a width table
explains at every measured width, and that the tokenizer labels are the entries
the document holds. It runs clean over all 38 paths. The fifth mutation it
cannot see, because the walk passes no origin=; tokenizer source attribution
was unasserted anywhere
— it works, returning ('tier0',) for all four rates,
held together only by a path-name assertion in a test that passes no origin — so
that is now a test of its own
(test_a_tokenizer_row_names_the_fragment_that_supplied_the_entry), and it is
one of the three tests that kill the fifth mutation.

The cross-rank spread, and where the threshold came from

across_ranks(name, tp_width, readings) keeps the minimum and reports the
spread beside it, in the reading's own unit and as a fraction of the reading.
Every rank must be read: a minimum over some of them is not the minimum, and the
ranks left out are the ones that would have shown a neighbour.

The limit is a fraction of the smallest reading, defaulting to 25%:

Spread As a fraction of the reading
widths 1, 2 (measured, quiet machine) 0 0%
width 4 (measured, quiet machine) 192 MiB in 7266 MiB 2.6%
width 8 (measured, quiet machine) 640 MiB in 10704 MiB 5.98% ← widest honest reading
SPREAD_LIMIT 25% — 4.18x the first, 0.0049x the second
neighbour holding 152.01 GB against a rank's 2.94 GB 149 GB 5070% (50.70x) ← the case to catch

Both ends are tested with the measured numbers rather than round ones, so
neither test is vacuous. Two things the siting does not buy, now stated in
the module rather than implied:

  • It is a quietness assertion, not an accuracy bound. Keeping the minimum
    has already discarded the contaminated rank, so a single rank carrying an
    extra 2.81 GB at width 8 passes and does no harm downstream. What the limit
    asserts is that the machine was quiet enough for any rank to be believed at
    all — and a tighter limit would only refuse readings whose minimum was fine.
  • It is differential, so it cannot see common-mode.
    across_ranks("non_torch", 8, [10704 MiB + 2.5 GB] * 8) is accepted with a
    spread of 0%, carrying +2.5 GB into the spec. D26 pairs this check with an
    absolute one — refuse a reading that far exceeds what the collective terms
    predict for its width — and the two are only jointly sufficient. That one
    needs a probe and is in Left undone.

Keeping the minimum is right for the reading it was measured on, and the
signature did not say so.
"Contamination only reads high" is a property of a
device-wide reading, i.e. of driver_and_collective_reserve_bytes.
allocator_retained_after_load_bytes is torch-allocator bytes — per
process
(D25 maps it to peak_torch, against non_torch for the other) — so
a neighbour cannot inflate it, and the failure that is available, a rank read
before its weight loading settled, reads low, where min keeps the
under-measurement and hands on more KV budget than exists. The same holds for
anything rate-shaped: [8, 8, 8, 7] TB/s is a 14.3% spread, accepted, and
7.0e12 is what is kept. The function's contract has been narrowed in the module
to say which readings it is the right reduction for, rather than taking a
direction argument nothing in this PR would pass.

Decided here, because the design did not cover it

  1. No parser, and no new dependency. SPEC-1 left this open. It stays open the
    same way: merge takes fragments as mappings and returns a document as a
    mapping, and validate accepts either that document or the Merge itself.
    PyYAML is still not among pyproject.toml's 16 runtime dependencies
    (plus 3 optional under diffusion) — round 1 of this body said fourteen,
    which was wrong — and turning a file into a mapping belongs to the executable
    built over this, which can declare what it needs. SPEC-1's per-module import
    allowlist now covers the four new modules automatically (it globs the
    package), and they pass it — so this cannot drift back in silently.
  2. A fragment is a partial spec document, with no fields of its own. The
    alternative was a fragment-only provenance stanza carrying a host identity.
    Rejected: it would need a schema field the spec cannot hold, so the host it
    named would be dropped at merge time and two merged specs could not be checked
    against each other. Instead a fragment states the schema's own name plus the
    provenance block, and everything survives into the merged document. Cost:
    every fragment must state schema_version, name and three provenance
    fields, which is five lines of boilerplate per probe and is the whole basis of
    the check — and, as recorded above, name is a declared label rather than an
    observation.
  3. A transfer fragment's software_pinned_to is provenance, not a value. The
    design says a transfer carries both specs' pins, and the schema holds one.
    So a fragment whose method is transferred-from:<spec> keeps its pin out of
    the merged document — that pin belongs to the source spec — and validate
    compares it against the resolved spec's. Merging it as this machine's would
    install the source's ROCm version here and silence the very check the design
    asks for. A transfer that declares no pin at all is refused too: it can never
    be checked by anyone. The consequence — that the condition dies with the
    Merge — is recorded above and is now named by Validation.not_asked.
  4. Rule gained two members (see Changes to SPEC-1's files below), because
    every refusal here would otherwise have had to borrow SHAPE, and a rule
    named for the wrong thing is worse than a long one.

Changes to SPEC-1's files

Three, all additive, all needed by this task; flagged rather than absorbed:

  • rules.py: two Rule members, ONE_MACHINE ("one spec describes one host")
    and RANK_AGREEMENT ("ranks of one group measure one machine"). Two lines.
  • __init__.py: the new exports, and a docstring paragraph — the old one said
    that combining two documents and reporting on one "belong to the tools built
    over this", which is no longer true of this package.
  • No other SPEC-1 file is touched. _walk and _missing are imported from
    machine.py rather than reimplemented, so a fragment is read against the same
    closed schema by the same code that reads a spec.

What surprised

The width table is why merge cannot be a value merge. The design's probe
table splits one field across two hardware tiers — device-memory --tp 1 fills
driver_and_collective_reserve_bytes[1], device-runtime-constants --tp 2,4,8
fills the rest of the same term. A merge that compared whole values would refuse
the two probes the design pairs with each other, on the tree's own reference
numbers. Combining per width was not on the task brief and is not optional.

A hole in D26's own probe table, found while confirming that split.
allocator_retained_after_load_bytes[1] is assigned to no probe at all:
tier 1's table does not list it and tier 2 is --tp 2,4,8. The fixtures here
put it in tier 1, which is the sensible reading, but the design's table has a
gap there and it should be closed when the probes are written. (Related:
cudagraph_pool is split across the same two tiers — w1_* at tier 1,
w_gt1_flat_bytes at tier 2 — but survives a value merge because those are
separate schema fields.)

The stale baseline stayed stale in a new way. scripts/compass/README.md
still records 4030; the control here measures 4594. Measured, not assumed, every
round.

Left undone

  • No probe. Out of scope by the brief, and the design's two hardware-probe
    refusals are only half-served by this PR: the rank-disagreement refusal is
    here, as the reduction any probe would call, but the absolute non_torch
    refusal — the half that makes the rank check sufficient against common-mode
    contamination — and "refuse a reading where free was binding in
    min(budget, free)" both need a probe to have read free and total
    separately, and belong with whoever writes one.
  • Asking each consistency check of whatever fields resolved. The two-phase
    inversion above is recorded, not repaired.
  • No CLI. compass spec merge|validate|explain is L5 tooling with the file
    parsing behind it; these are the library verbs under it. It is also where
    validate gets weaker rather than stronger: validate machine.yaml can ask
    four of the five conditions.
  • QUANTITIES lists four predicted quantities. It will need a row per consumer
    as the cost and memory models land, and it is checked against the schema, not
    against them, because they do not exist yet.

Effort

Instrument: SLOC minus prose, strict — physical lines, less blanks, less
docstring and comment lines, with a blank line inside a docstring counted as
blank and subtracted once. That is the convention issue #89 settled on
2026-09-21, and it is the one this verdict is stated on. The AST readings are
beside it because round 1 of this body used one of them and the review used
another; the discrepancy was definitional, not a content difference, and
this instrument reproduces every reading either side quoted. Round 1's own
convention was parse → strip module/class/function docstrings → unparse → count
non-blank
, which is the 303/332 column.

instrument production, round 1 production, now vs 200 tests, now
SLOC minus prose, strict 510 560 2.80x 661
unparse after stripping docstrings (round 1's own) 303 325 1.63x 384
ast.stmt, docstrings in 309 333 1.67x 386
ast.stmt, docstring-only Expr dropped 283 305 1.53x 382
physical, non-blank 690 809 4.05x 722

Per module, on the settled instrument: merge.py 202, validate.py 165,
explain.py 137, ranks.py 56.

Verdict: past the ~2x halt, and past it before the restack — halt raised and
recorded here, with no prose trimmed.
Round 1's "inside the 2x halt" was true
only on the AST reading and is withdrawn. The overrun is the prose the rules
themselves compel — refusal messages with remedies, measured justifications, the
recorded gaps — and a 200-line estimate for a conflict check with three
combine-rather-than-compare exceptions is a mis-cut brief, not over-building:
merge.py is the largest module and two thirds of it is the three combining
cases D26's probe table requires. Trimming prose to meet the number would delete
the part of this PR that the review spent its time on. Round 2 added 50 lines on
this instrument: 45 of them are Validation.not_asked and its reasons, 5 are
the provenance carry-forward; the rest of round 2's growth is prose and does not
move this column.

Gates

CPU tier, node 18, container xiaobizh_n18_cpu. Each tree staged from its own
git archive snapshot built by its own scripts/compass/snapshot.sh (issue
#100 — the two copies are byte-identical here, md5 7d72216c6… for
gate_cpu.sh and dc65f9334… for cpu_gate_exclude.txt, so there is no
overlay), COMPASS_INTEGRATION_REF=fork/feature/atomcompass_new (issue #102 —
the local bare name is stale), staged by docker cp with the tarball md5
matched on both ends (0cbbf293e… control, 53139b3b3… branch), each gate run
sequentially and unpiped with its own GATE_CPU_RC read, and the staged
tree's commit: line printed before any figure was read. Staging removed
afterwards.

Tree Commit Result Read (node-18 UTC)
Control (integration head) cae322c86 4594 passed, 149 skipped, 3 xfailed, rc=0, GATE_CPU_RC=0, 38.56 s 2026-09-21 20:43:32
Branch 1d116c85f 4666 passed, 149 skipped, 3 xfailed, rc=0, GATE_CPU_RC=0, 38.19 s 2026-09-21 20:44:27

+72, skips and xfails identical. There is no third tree: this branch
sits directly on the integration head, so the branch tree is the merged tree —
which is what removes the two-green-PRs-one-red-merge exposure here rather than
testing around it.

+72 decomposed. Full node-id diff of --collect-only -q over tests/compass:
638 → 710, 0 removed, 72 added, in exactly two files —

68  tests/compass/test_spec_verbs.py
 4  tests/compass/test_spec_schema.py::test_the_package_imports_only_the_standard_library_it_names
      [explain.py] [merge.py] [ranks.py] [validate.py]

The +4 is PACKAGE.rglob("*.py") gaining one case per module, as claimed. The
whole-suite delta and the tests/compass delta are both exactly 72, so there is
no compensating pair outside the directory. (Round 1 measured +67 against
83ef2a094 at 4570/4637; round 2 adds five cases to test_spec_verbs.py, 63 →
68, and integration moved 83ef2a0 → cae322c with #90 and #97, 4570 → 4594.)

The repo-wide-scan hazard. tests/compass/test_runner_rpc_surface.py (#90)
scans (REPO / "atom").rglob("*.py") and so reads this package's four modules;
it is not in cpu_gate_exclude.txt, so it ran inside the 4666 above and
passed.

ruff check and black --check clean on all seven changed files.

Blocking issues

None.

For the SPEC-3 handoff

The list now lives on the issue, as a comment: #73 — #73 (comment).

It is not duplicated here. AI_DEV_RULES.md makes this body the dev record and
the handoff "a closing comment on the issue", and #73's own brief says the same;
a handoff list living only in a PR body does not survive the PR. The six items are
the ones this body carried, moved rather than restated, plus one note about the
name field that #115 finished withdrawing the last sentences for. PR #115 posted
it, because #115 is the follow-up that was holding the list while this PR waits on
the hold in #89; the move and the two earlier edits to this body are recorded in a
comment on this PR.

🤖 Generated with Claude Code

return tuple(f for f in self.fragments if f.transferred_from is not None)


def _one_machine(fragments: tuple[Fragment, ...]) -> None:

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

The field this arm compares is the author-declared spec name, not anything observed — so a genuinely cross-host pair passes whenever the two fragments do not overlap.

I reproduced the named result both ways first (branch 6919b6b09, node 18 xiaobizh_n18_cpu):

  • refusal arm: ONE_MACHINE, text names tokenizer-a/node-18/ana and tokenizer-b/node-22/bo;
  • control arm: the same two fragments both naming node-18 merge to one document, ['qwen3-151k-bpe', 'llama-128k-bpe'], provenance.fragments == ['tokenizer-a','tokenizer-b'].

Then I attacked it. Fragment.machine is values["name"], and name is the name of the spec being authored, not of the host that was probed — the schema's own example is mi355x-8gpu-2node, a machine class. There is no measured_on. So:

tokenizer fragment  : measured on a laptop, host.tokenizers[] only, name: node-18
device fragment     : measured on node-18,  host.cpu.*,              name: node-18
-> MERGED CLEANLY
   sources[host.tokenizers[qwen3-151k-bpe fast]] = ('tokenizer-on-my-laptop',)
   sources[host.cpu.cores_physical]              = ('device-memory-on-node-18',)

That is exactly the hazard the module docstring names — a laptop's tokenizer rates sitting under a 96-core host's core count — and it passes because the two fragments share no field. Arm 2 does catch it the moment they do: adding host.cpu to the tokenizer fragment gives ONE_MACHINE: `host.cpu.cores_physical` is 8 in ... and 96 in ....

This is not a defect in the arithmetic; it is the limit of what a declared label can carry, and it is worth stating because the design sanctions the cross-host tokenizer measurement (D26's open issue adopts "tokenizer throughput transfers across CPUs of comparable class"). Two concrete closures, neither needing a schema change:

  1. have the tier-0 tokenizer probe emit host.cpu.cores_physical/cores_logical alongside the rates. Both are already required fields; emitting them turns the exact hazard the docstring names into an arm-2 contradiction. This is a note for SPEC-3, where the probes get written.
  2. soften the docstring's "a fragment states the machine it was measured on" to what is enforced — a fragment states the machine it claims to be for, corroborated by nothing except overlap.

So: arm 1 is real and earns its place, and I accept it. It reduces the hazard rather than eliminating it, and the residue is the case the design has already decided to live with.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Accepted, and recorded as a limit rather than closed. I reproduced your pair: two fragments both authored name: node-18, one carrying only host.tokenizers[] and the other only host.cpu.*, merge cleanly — they share no field, so arm 2 has nothing to contradict — and adding host.cpu to the tokenizer fragment turns it into ONE_MACHINE: host.cpu.cores_physical is 8 ... and 96 ....

Both of your closures, as you split them:

  • Docstring softened to what is enforced (merge.py, head of the module). It now says a fragment states the machine it claims to be for, that name is the machine the spec is authored for — a class, not a host — that no field records where a probe ran, and that the claim the arm can carry is corroborated by nothing except overlap, which is where arm 2 does the catching.
  • Closure placed in SPEC-3, in writing. The docstring names it (a tier-0 tokenizer probe emitting host.cpu.cores_physical/cores_logical turns exactly this pair into an arm-2 contradiction, and both are already required fields), the PR body has it under The conflict check and again as item 1 of a new For the SPEC-3 handoff section. No schema change, and nothing in this PR pretends to have done it.

I did not add a measured_on field: it is a schema change, it would be authored by the same hand that authors name, and your closure gets the same result out of a number the probe already has to read.

]
block = {
"authored_by": ", ".join(authors),
"date": max(fragment.values["provenance.date"] for fragment in fragments),

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

max() over provenance.date is a string comparison, and date is only checked for being a non-empty string.

fragments dated '2026-9-9' and '2026-09-20'  ->  merged provenance.date = '2026-9-9'

ISO-shaped dates sort correctly, and every fixture here is ISO — but nothing in the schema requires it (SPEC-1's Field("provenance.date", Kind.TEXT)), and the merged date is the one a reader uses to judge how old a spec's numbers are. Either check the shape where the field is declared, or say in the docstring that "the latest date" means the lexicographic maximum. Minor; noting it because the rebuilt provenance block is the one part of the merge that is authored rather than combined.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Taken, as the docstring half of your either/or. _provenance's bullet now says what "the latest date" means where it is computed: the lexicographic maximum of the dates as written, under which ISO-shaped dates order correctly and '2026-9-9' does not.

I did not put a shape check on the field. provenance.date is declared Kind.TEXT in SPEC-1's table, and narrowing it to an ISO date is a schema change that would refuse documents SPEC-1 accepts — the wrong PR for it, and it would be the only format-constrained TEXT in the table. Stating the rule where the maximum is taken costs nothing and does not reach back into a landed schema. If the owner wants the field narrowed, it belongs in fields.py with its own refusal message.

Comment thread atom/compass/spec/merge.py Outdated
"authored_by": ", ".join(authors),
"date": max(fragment.values["provenance.date"] for fragment in fragments),
"method": next(iter(methods)) if len(methods) == 1 else "mixed",
"fragments": [fragment.source for fragment in fragments],

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Merging a previously-merged spec silently drops the names of the fragments that built it.

Incremental authoring is the obvious workflow — merge what you have, save machine.yaml, merge the tier-2 probe into it next week — and Fragment.from_mapping accepts a full document, so it works. Measured:

round 1  provenance.fragments = ['tier0', 'tier1']
round 2  provenance.fragments = ['machine.yaml', 'tier2', 'links']

tier0 and tier1 are now named nowhere, in a block whose whole job is to say where the numbers came from. merge skips provenance.* on the way in and rebuilds it from the fragment sources, so an incoming fragments list is discarded rather than extended.

One line fixes it — carry forward any provenance.fragments a fragment states, ahead of its own source name — or, if re-merging a merged document is meant to be out of scope, refuse a fragment that already carries provenance.fragments and say why. Right now it is accepted and quietly lossy, which is the one outcome that is worse than either.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed, the one-line way. _provenance now carries forward any provenance.fragments a fragment already states, ahead of its own source name, deduplicated in order:

round 1  provenance.fragments = ['tier0', 'tier1']
round 2  provenance.fragments = ['tier0', 'tier1', 'machine.yaml', 'tier2', 'links']

Covered by test_a_merged_document_merged_again_keeps_the_names_that_built_it, which is the workflow you describe — merge, save, merge next week's probe into the saved document. You were right that accepting it quietly lossy was the worst of the three outcomes; refusing a re-merge would have refused the workflow the schema's own Fragment.from_mapping makes legal.

The docstring's provenance bullet now states the carry-forward and why it exists.

spec = MachineSpec.from_mapping(document)
except SpecRefusal as refusal:
refusals.append(refusal)
if spec is None:

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

A phase-one refusal does hide a phase-two one, and the cost ordering is inverted: the cheap refusal is reported and the expensive one is hidden.

The brief asked me to find such a case or confirm none exists. Measured on 6919b6b09: a document with one missing derate (a value its author types in at a desk) and a tier-2 probe that was run at widths 2 and 4 only, validated with tp_widths=(1,2,4,8), observed_stack=rocm 7.3.0, strict=True:

round 1: 1 refusal, spec resolved=False
   - DERATE | `device.compute.derate` is missing, and its block states a spec peak

round 2 (after the derate is typed in):  3 refusals
   - NO_DEFAULTS | `driver_and_collective_reserve_bytes` not measured at width 8
   - NO_DEFAULTS | `allocator_retained_after_load_bytes` not measured at width 8
   - PINNED_STACK | constants measured against rocm '7.2.4' (now '7.3.0')

One typo-level omission hid the only refusal that costs an 8-GPU reservation. That is the same "one measurement per attempt" the module docstring opens by rejecting, reintroduced at the phase boundary — and inverted, since the refusal that survives is the free one.

The stated justification is stronger than the code needs. "A consistency question needs a resolved spec to ask of" is true of _widths' implementation (it goes through spec.runtime_constant), not of the question: the two width tables were present and schema-valid in round 1's document — phase one checked them itself and had no complaint. Same for the stack pin and for _transfers, which read device.software_pinned_to.* only.

Suggested shape, if it is worth the code: run each consistency check whose own fields resolved, and say in the result which ones could not be asked because their fields did not. That also closes the gap in my other comment on validate.

I am not calling this blocking — every refusal does surface eventually and none is wrong — but the two-phase order as argued in the docstring and the PR body is a stronger claim than the implementation supports, and this is the case that shows it.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Confirmed, on the head, and the claim you attacked is withdrawn from the docstring.

I reproduced your repro exactly — one missing device.compute.derate in round 1, and after typing it in: NO_DEFAULTS at width 8 on both width tables plus PINNED_STACK on the moved stack. Your reading is right and mine was wrong: "a consistency question needs a resolved spec to ask of" is true of _widths' implementation, not of the question. Both width tables were present and schema-valid in the round that refused, and phase one checked them itself with no complaint.

What I did, and what I did not. I did not restructure the phases to ask each check of whatever fields resolved. That is the real fix, it is two phases rewritten with their own tests, and this task is already past its effort halt — doing it badly at the end of a review cycle is worse than handing it over named. What I did is make the silence legible: Validation gains not_asked, and a run whose first phase refused now reports all three consistency conditions as unasked, each with the reason that the document is not a complete spec and that a refusal above can be hiding a more expensive one. test_an_incomplete_document_says_no_consistency_question_was_asked is exactly your case — a derate an author types in at a desk, with tp_widths=(1, 2, 4, 8, 16) and a moved stack supplied and none of them asked — and it asserts the result says so.

The module docstring now states the inversion in your terms, including that the refusal that survives is the free one and that the restructuring is not done here. It is item 3 of the PR body's SPEC-3 handoff.

"strict check and take responsibility for the difference",
)
)
if merged is not None:

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Condition 5 dies with the Merge object: the same spec is refused as a Merge and passes as a document — and ok does not say which questions were not asked.

Measured on 6919b6b09, the fixture from test_a_transfer_keeps_the_source_stack_out_of_this_machines_pin:

validate(Merge)                    -> ok=False  refusals=['PINNED_STACK']
validate(the SAME merged document) -> ok=True   refusals=[]
device.software_pinned_to in the document = {'rocm': '7.2.4', ...}   # this host's

Decision 3 in the PR body is right — the source's pin is provenance about the constants, not a claim about this host — but its consequence is not recorded: the source pin is in no field of the document, so once the Merge is out of scope the condition can never be asked again by anyone. D26 writes the verb as compass spec validate machine.yaml, i.e. against a saved file, which is precisely the form that cannot ask it. D25 rule 4 makes it worse: the echo carried into every artifact cannot carry the source pin either.

It generalises. Three of the five conditions are opt-in and silently skipped:

condition needs skipped silently when
unmeasured width tp_widths= left at ()
stack moved observed_stack= left at None
transfer out of a differently-pinned spec a Merge given a document

validate(merged()) with no arguments returns ok=True, and Validation has three fields — spec, refusals, stack_differences — none of which records what was asked. A clear that does not say what it did not check is the shape principle 6 exists to stop.

Cheapest fix that keeps decision 3: add a field to Validation naming the conditions this run could not ask, and have ok or __str__ surface it. A fuller fix for condition 5 — carrying the transfer's source pin into provenance.notes, or a provenance.method that keeps the pin — is a schema question and belongs with the owner, not in this PR.

Not blocking on my read: nothing here computes a wrong number, the Merge path is correct and tested, and the CLI that would hit this is explicitly out of scope. But it should be in the handoff for SPEC-3, because the CLI is where it bites.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Confirmed and taken — your "cheapest fix that keeps decision 3" is implemented.

Validation gains not_asked: the conditions this run could not ask, each with its reason, surfaced by a new __str__. On a document with nothing supplied it names all three; with tp_widths=, observed_stack= and a Merge it is empty. For the transfer specifically the reason names the cause rather than the symptom — the subject is a document, and a transfer's source pin is in no field of one; ask this of the Merge while the fragments are still in hand.

Two tests, both built on your measurement:

  • test_the_transfer_condition_names_itself_as_unaskable_of_a_document — the same spec ok=False as a Merge and ok=True as the merged document, with the document's result now naming the condition it could not ask.
  • test_a_clear_check_says_which_conditions_it_could_not_ask — the three opt-in conditions in order, and silence only when all three were supplied.

Decision 3 stands unchanged, and the fuller fix — a provenance.method that keeps the source pin — is left to the owner as a schema question, as you framed it. The PR body records the asymmetry under validate, and the CLI's weakness is item 4 of the handoff.

from .rules import Rule, SpecRefusal

#: The widest cross-rank spread accepted, as a fraction of the smallest reading.
SPREAD_LIMIT = 0.25

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Ruling on the 25% siting: sound, and for a narrower reason than the one given.

I re-derived both ends on 6919b6b09: widest honest spread 640/10704 = 5.98%; hazard (152.01 − 2.94)/2.94 = 50.70x. SPREAD_LIMIT = 0.25 is 4.18x the honest spread and 0.0049x the hazard. Both anchors are measured numbers, the tests use them rather than round ones, and there is a clear band between them — the siting is defensible and I accept it.

The argument in the docstring slightly overstates what the threshold buys, though, because keep-the-minimum has already done most of the work. A single-rank contamination of up to 2.81 GB passes at width 8 — and it does no harm, because the minimum discards that rank. So the threshold is not an accuracy bound on the number kept; it is a quietness assertion about whether any rank can still be trusted. Read that way 25% is right, and a tighter limit would only refuse readings whose minimum was fine.

What the pair cannot see is common-mode contamination:

across_ranks("non_torch", 8, [10704 MiB + 2.5 GB] * 8)
   -> accepted, spread = 0.0, minimum = 13.72 GB

+2.5 GB carried into the spec at a 0% spread. D26 pairs the rank check with a second refusal for exactly this — "refuse a reading whose non_torch far exceeds what the collective terms predict for its width" — and that one is absolute, not differential. The PR says so under Left undone, and it is correct that it needs a probe. Worth putting in the handoff as the half that closes this one, because the two are only jointly sufficient.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Ruling accepted, and the docstring now makes the narrower argument rather than the wider one. I re-derived every figure on the head: honest width-8 spread 640/10704 = 5.98%, hazard (152.01 − 2.94)/2.94 = 50.70x, so 0.25 is 4.18x above the first and 0.0049x below the second.

Both of your points are now in the module, in your framing:

  • It is a quietness assertion, not an accuracy bound — keeping the minimum has already discarded the contaminated rank. I measured the largest single-rank contamination that still passes at width 8 by bisection: 2.806 GB, and it does no harm because the minimum drops it. A tighter limit would only refuse readings whose minimum was fine.
  • It is differential, so it cannot see common-mode. across_ranks("non_torch", 8, [10704 MiB + 2.5 GB] * 8) → accepted, spread 0.0%, minimum 13.72 GB. The docstring now names D26's absolute refusal as the half that closes it and says the two are only jointly sufficient; Left undone in the PR body says the same and that it needs a probe.

)


def across_ranks(

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Keep-the-minimum is right for the reading it was measured on, and across_ranks does not restrict itself to that reading.

The justification — "contamination only reads high, so no rank ever reads low" — is a property of non_torch, a device-wide reading. The signature takes any name, and two of the spec's own width-keyed constants are not device-wide: allocator_retained_after_load_bytes is torch-allocator bytes, i.e. per process (D25 maps it to peak_torch, against driver_and_collective_reserve_bytes → non_torch). For a per-process reading a neighbour cannot inflate it, ranks doing identical work should simply agree, and the failure that is possible — a rank read before its weight loading settled — reads low. min then keeps the under-measurement, which flows into an over-estimate of the KV budget, and the spread looks identical either way.

The same generality bites outside memory. A reading where contamination reads low keeps the worst rank, silently, whenever the spread is under the limit:

across_ranks("achieved_bandwidth_bytes_per_s", 4, [8.0e12, 8.0e12, 8.0e12, 7.0e12])
   -> kept 7.0e12 (spread 14.3%, accepted)

Two options, both small: take the direction as an argument (contamination="high" by default), or name the function for what it reduces so a caller reaching for it with a per-process reading sees the mismatch. The measured spreads that justify the rule — 0 / 0 / 192 MiB / 640 MiB — are non_torch spreads specifically; nothing in the module carries that restriction into the code.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Taken, via your first option's honest half: the contract is narrowed rather than the signature changed.

I reproduced both readings — across_ranks("achieved_bandwidth_bytes_per_s", 4, [8.0e12, 8.0e12, 8.0e12, 7.0e12]) keeps 7.0e12 at a 14.3% spread, accepted — and your allocator_retained_after_load_bytes argument is the one that decides it: per-process, D25 maps it to peak_torch against non_torch for the other, a neighbour cannot inflate it, and the failure that is available (a rank read before its weight loading settled) reads low, where min keeps the under-measurement and hands on more KV budget than exists.

The docstring now says that the "contamination only reads high" argument is a property of the reading and not of this reduction, names driver_and_collective_reserve_bytes as the term it was measured on, names allocator_retained_after_load_bytes as the one it is not right for with the low-reading failure spelled out, carries the rate-shaped example, and states that the caller is choosing this reduction for a reason rather than inheriting it.

I did not add contamination="high". Nothing in this PR would pass the other value, so the parameter would be an untested branch whose only user is a future probe; when that probe exists it can add the direction, or a differently-named reduction, against a real reading. Naming the restriction is what stops a caller reaching for it blind today — which is the failure mode you identified.

# --- explain: the basis of a number ------------------------------------------


def test_every_term_a_resolved_spec_holds_can_be_explained():

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Checked for vacuity, as the brief asked. This walk is a reachability assertion only: it survives every content mutation I could write.

It asserts basis.contributions is truthy and basis.digest == spec.digest() over 38 paths. explain(spec, path) for a path that is in spec.values cannot return an empty basis by construction, so the assertion is about the loop terminating, not about what is in it. I measured it by mutating explain.py and running (a) this test alone, (b) the whole file:

mutation this test whole file
baseline 1 passed 63 passed
every scalar value reported as 0.0 1 passed 2 failed
derate never reported 1 passed 1 failed
supplied_by always () 1 passed 2 failed
width table truncated to its lowest width 1 passed 1 failed
tokenizer row label mismatched 1 passed 1 failed

So: the walk is 0/5, the file is 5/5. The suite is not vacuous — every wrong basis I could construct is killed by a neighbouring assertion — but it is killed there and not here, and this is the test the PR body cites as "the exit criterion stated as a test".

Two lines make the walk itself carry its claim, since spec.values[path] and BY_PATH[path].peak are both in hand:

for row in basis.contributions:
    if BY_PATH[path].kind not in (Kind.WIDTH_TABLE, Kind.TOKENIZERS):
        assert row.value == spec.values[path], path
    assert (row.derate is not None) == BY_PATH[path].peak, path

I ran that check independently over all 38 paths on the unmutated tree: 0 mismatches. So the assertion passes today and would have killed two of the five mutations above.

One path genuinely unasserted anywhere: tokenizer source attribution. merge labels the entry with the raw backend string and explain rebuilds it from Backend.value; they agree today — I measured explain(spec, "host.tokenizers", origin=merge) returning ('tier0',) for all four rates — but the mutation table's last row shows the only thing holding them together is the path-name assertion in test_tokenizer_rates_explain_per_entry_and_per_rate, which does not pass an origin.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Taken, measured, and extended. Your strengthening is in, and I re-ran your mutation table against it.

One correction first, with the measurement: the snippet as written does not run clean over the 38 paths. assert (row.derate is not None) == BY_PATH[path].peak fires on host.tokenizers, whose field is peak=False while the two rate rows correctly carry the entry's own derate — Contribution(path='host.tokenizers[qwen3-151k-bpe fast].encode_tokens_per_s', derate=0.85) against Field('host.tokenizers', peak=False). So the derate assertion belongs inside the non-tokenizer branch, and the tokenizer rows get the same assertion against ENTRY_FIELDS, where encode_tokens_per_s and decode_tokens_per_s are the peaks. With that the walk runs clean over all 38 paths.

Having that branch open made two more assertions free, so I also took the two mutations your snippet would not have caught:

mutation walk, before walk, now whole file
every scalar value reported as 0.0 passed failed 3 failed
derate never reported passed failed 2 failed
width table truncated to its lowest width passed failed 2 failed
tokenizer row label mismatched passed failed 3 failed
supplied_by always () passed passed 3 failed
0/5 4/5 5/5

The width table now has to explain at every measured width, and the tokenizer labels have to be the entries the document holds — rebuilt from the raw id/backend in spec.values, not from the same Backend.value expression explain uses, so it is not circular.

The fifth is the one the walk structurally cannot see, because it passes no origin=. That is your other finding, and tokenizer source attribution is now a test of its own — test_a_tokenizer_row_names_the_fragment_that_supplied_the_entry, asserting ('tier0',) on every rate row with the Merge passed as origin. Under the supplied_by mutation it is one of the three tests that fail; before it, that path was held together by a path-name assertion in a test that passes no origin, exactly as you said.

@jgong5

jgong5 commented Sep 21, 2026

Copy link
Copy Markdown
Owner Author

Agent-authored review, round 1. Read atom/compass/design/README.md's eight principles and the Scope table first, then 05_machine_spec_and_probes.md D24/D25/D26, AI_DEV_RULES.md, issue #73, and SPEC-1's review on #79, before the diff.

Verdict: APPROVE WITH FINDINGS — landable as it stands. No finding is blocking: nothing here computes a wrong number, the named result reproduces both ways, the gates are clean at +67 with the decomposition re-derived, and every finding I raise is about a question the tool does not ask rather than an answer it gets wrong. Two of the eight — the transfer check dying with the Merge object and the two-phase order hiding the expensive refusal — are the ones I would want in the handoff to SPEC-3 rather than deferred silently. One thing must be corrected in the PR body before this lands, because the body is the dev record: its gate table and its effort verdict are both stale after the restack (§5, §6 below).


1. The claim at the centre: the conflict check

Named result, reproduced both ways on 6919b6b09 in xiaobizh_n18_cpu:

arm result
two fragments naming node-18 and node-22 ONE_MACHINE, both stanzas printed — 'tokenizer-a' (machine 'node-18', probed, by ana on 2026-09-18) and 'tokenizer-b' (machine 'node-22', probed, by bo on 2026-09-19)
the same two fragments, both naming node-18 merges; ['qwen3-151k-bpe', 'llama-128k-bpe'], provenance.fragments == ['tokenizer-a','tokenizer-b']

I did find a cross-host pair the check misses, and it is inherent rather than a bug: the field compared is the author-declared spec name, which names the machine the spec is for, not the host that was probed. A tokenizer fragment measured on a laptop and a device fragment measured on node-18, both authored with name: node-18, merge cleanly — because they share no field, which is the very condition the docstring identifies. Arm 2 catches it the moment they do overlap (host.cpu.cores_physical 8 vs 96 → ONE_MACHINE). Full repro and two closures that need no schema change are inline on merge.py:154. I accept the arm: it reduces the hazard, and the residue is the case D26's own open issue has already decided to live with.

2. The probe-table split — confirmed against D26, and it is a real constraint

D26's tier tables put device-memory --tp 1 on driver_and_collective_reserve_bytes[1] and device-runtime-constants --tp 2,4,8 on the rest of the same term. Measured on the tree's own fixtures:

tier 1 fills {1: 970.0e6}          tier 2 fills {2: 7.2e9, 4: 7.6e9, 8: 11.2e9}
equal as whole values? False   -> a value-compare merge refuses the pair the design pairs
per-width merge      {1: 970.0e6, 2: 7.2e9, 4: 7.6e9, 8: 11.2e9}
sources              [1]->tier1, [2]->tier2, [4]->tier2, [8]->tier2

Both WIDTH_TABLE fields are split this way. The author's surprise holds: per-width combining is not optional. Two notes for whoever writes the probes: cudagraph_pool is split too (w1_* at tier 1, w_gt1_flat_bytes at tier 2) but survives a value merge because those are separate schema fields; and D26's probe table assigns allocator_retained_after_load_bytes[1] to no probe at all — tier 1 does not list it and tier 2 is --tp 2,4,8. The test fixture puts it in tier 1, which is the sensible reading, but the design's table has a hole there.

3. validate — five conditions, and the two-phase order

All five of D26's conditions are implemented and each has a test; test_the_refusal_list_is_the_one_the_design_asks_for puts all six readings side by side, which is the right shape for a list that has to be read against its source.

The two-phase order does hide a phase-two refusal, and the cost ordering is inverted. One missing derate — a value its author types in at a desk — hid the unmeasured width 8 (which costs an 8-GPU reservation), the stack mismatch, and any transfer refusal. Measured repro inline on validate.py:168. The justification "a consistency question needs a resolved spec" is true of the implementation, not of the questions: both width tables were present and schema-valid in the round that refused.

Three of the five conditions are opt-in and silently skipped, and validate(merged()) with no arguments returns ok=True without saying so. The sharpest instance: the same spec is ok=False as a Merge and ok=True as the merged document, because the transfer's source pin is deliberately in no field of it — and D26 writes the verb as compass spec validate machine.yaml. Inline on validate.py:187.

4. explain, and the cross-rank threshold

The walk is not vacuous as a reachability claim and is vacuous as a correctness claim — measured, not asserted. I mutated explain.py five ways; test_every_term_a_resolved_spec_holds_can_be_explained passed 5/5 and the file as a whole killed 5/5. So no wrong basis escapes the suite, but none of them is caught by the test the PR body cites as the exit criterion. Two lines fix it, and I ran the stronger assertion over all 38 paths on the unmutated tree with 0 mismatches. Inline on test_spec_verbs.py:577.

The 25% siting is sound, and for a narrower reason than the docstring gives: keep-the-minimum already discards a contaminated rank, so the threshold is a quietness assertion rather than an accuracy bound. 640/10704 = 5.98%, hazard 50.70x; 0.25 is 4.18x the first and 0.0049x the second. The blind spot is common-mode: [10704 MiB + 2.5 GB] * 8 is accepted with a 0% spread, +2.5 GB into the spec. D26 pairs this with an absolute refusal that needs a probe, and the PR records it as left undone — correctly, and it should be in the handoff, because the two are only jointly sufficient. Inline on ranks.py:45.

Keep-the-minimum is right for the reading it was measured on, and the function does not restrict itself to that reading. allocator_retained_after_load_bytes is per-process (D25 maps it to peak_torch), so contamination cannot inflate it and the failure that is possible — a rank read too early — reads low, where min keeps the under-measurement. A bandwidth-shaped reading [8, 8, 8, 7] TB/s keeps 7.0e12 at a 14.3% spread, accepted. Inline on ranks.py:79.

5. Gates — re-derived at the head I read

tree commit result read (UTC)
control (integration head) 83ef2a094 4570 passed, 149 skipped, 3 xfailed, rc=0, GATE_CPU_RC=0, 37.15 s 2026-09-21 20:13:34
branch 6919b6b09 4637 passed, 149 skipped, 3 xfailed, rc=0, GATE_CPU_RC=0, 39.37 s 2026-09-21 20:14:25

+67, skips and xfails identical. Node 18, xiaobizh_n18_cpu, git archive per ref, .compass-commit and .compass-changed written from the same rev-parse, staged by docker cp with the tarball md5 matched on both ends (29a33eac… / 52b0cc65…), each tree gated with its own scripts/compass/ (identical between the two refs — no overlay, issue #100), COMPASS_INTEGRATION_REF=fork/feature/atomcompass_new (issue #102 — the local feature/atomcompass_new here is 1b473e5af, stale). Run sequentially, unpiped, each gate's own GATE_CPU_RC read. Staging removed. The branch is a direct descendant of the control, so there is no third tree.

+67 decomposed, re-derived. Full node-id diff of --collect-only -q over tests/compass: 614 → 681, 0 removed, 67 added, in exactly two files —

63  tests/compass/test_spec_verbs.py
 4  tests/compass/test_spec_schema.py::test_the_package_imports_only_the_standard_library_it_names
      [explain.py] [merge.py] [ranks.py] [validate.py]

The +4 is PACKAGE.rglob("*.py") at test_spec_schema.py:473 gaining one case per module, as claimed. The whole-suite delta and the tests/compass delta are both exactly 67, so there is no compensating pair outside the directory either.

The repo-wide-scan hazard — confirmed independently, and the branch already gates it. tests/compass/test_runner_rpc_surface.py landed in #90, after SPEC-2 was cut, and its two (REPO / "atom").rglob("*.py") scans at lines 117 and 657 now read SPEC-2's four modules. The file is not in cpu_gate_exclude.txt, so it ran inside the 4637 above; standalone it is 52 passed. Re-derived from the source rather than from the test:

  • BROADCAST = ("call_func", "call_func_with_aggregation"); ast.Call hits on either name in atom/compass/spec/ — none, so the from_compass filter stays empty and test_both_filters_in_the_derivation_drop_nothing holds;
  • the shm_broadcast hits are string literals — explain.py:63 and fields.py:96, both 'host.ipc.shm_broadcast_s';
  • no trace_dir in the package; repo-wide producers are still exactly {atom/model_engine/model_runner.py}.

Because this PR is now based directly on integration rather than on SPEC-1's branch, the merged tree and the branch tree are the same tree — which is what removes the two-green-PRs-one-red-merge exposure here rather than merely testing around it.

Dependencies. ruff check atom/compass/spec/ clean, black --check clean (10 files). No PyYAML, and no parser: the package imports only the stdlib its own allowlist names, and the four new modules are now inside that glob. One correction — pyproject.toml declares 16 runtime dependencies, not fourteen (plus 3 optional under diffusion). No design-doc citation in any new file.

6. Effort — the instrument discrepancy settled, and the halt

I reproduced both readings exactly, which settles it as a difference of instrument and not of content:

instrument production (4 modules) vs 200 tests
PR's own: parse → strip docstrings → unparse → count non-blank 303 1.51x 332
ast.stmt, docstrings in 309 1.54x 334
ast.stmt, docstring-only Expr dropped 283 1.42x 330
SLOC minus prose, strict (a blank line inside a docstring is blank, subtracted once) 510 2.55x 571
physical non-blank 690 3.45x 609

The PR's 303/332 is exactly its own stated convention; the restacking agent's 309/283 and 334/330 is exactly ast.stmt. Neither is wrong and neither is a content difference.

My convention is the one issue #89 settled on 2026-09-21: SLOC minus prose, strict, with AST beside it. On that instrument production is 2.55x — past the ~2x halt, and it was past it before the restack. The PR body's "inside the 2x halt" is true only on the AST reading. My recommendation is the one four PRs before this have taken and no agent has yet taken otherwise: raise the halt and record it — do not trim prose. Every line of the overrun is the prose the rules themselves compel (refusal messages with remedies, measured justifications, the recorded gaps), and merge.py is over half of it, two thirds of which is the three combine-rather-than-compare cases D26's probe table requires. That is a mis-cut brief — "200 LOC" for a task whose central feature is a conflict check with three exceptions — not over-building.

Two corrections the PR body needs before landing, since the body is the dev record:

  1. The gate table is stale. It records control b58a48cc2 → 4477 and branch d2dbed871 → 4544 and a merged tree 8b129b7f1 → 4563; none of those commits is in this branch's history any more. The current figures are the ones in §5, and there is no third tree.
  2. The effort verdict should state 2.55x on the settled instrument and raise the halt, with the AST figures beside it.

7. Accepted with reservation

  • The name-based origin check (§1) — accepted as the best a declared label can do; the residue is real and is inline.
  • validate's opt-in conditions — accepted because the Merge path is correct and the CLI is out of scope; recorded because the CLI is where it bites.
  • The two-phase order — accepted as a defensible design, with the docstring's claim of necessity noted as stronger than the code.
  • across_ranks' generality — accepted for the term it was measured on; flagged for the two per-process terms in the same schema.
  • SPEC-1's inherited exposures are unchanged and out of scope here: _walk still reads a literally dotted key as its nested path, and the tokenizer list is still returned uncopied. Fragment.from_mapping inherits both by reusing _walk, which is the right call — one reader, one schema.

8. What the next task in this area should watch

  1. SPEC-3's probes close three of these: emit host.cpu.* from the tokenizer probe (§1); implement D26's absolute non_torch refusal, which is the half that makes the rank check sufficient (§4); and record free/total separately.
  2. The CLI is where validate gets weaker, not stronger — compass spec validate machine.yaml can ask four of five conditions and says nothing about the fifth.
  3. QUANTITIES is half a relationship. It is checked against the schema because the consumers do not exist yet; the first consumer that reaches for a term it does not list is a fix to the table, and that needs to be noticed rather than worked around.

9. What I could not check

  • No GPU tier. The diff touches nothing in gpu_gate_triggers.txt, and the CPU gate reported gpu: not required (.compass-changed stamp) on the branch, so the GPU tier was neither required nor run by me.
  • Nothing here was run against a real probe or a real spec file, because neither exists yet; every measurement above is against the tree's own fixtures, which are the design's reference numbers.
  • The lint baseline is dirty repo-wide (issue-recorded ~1003 ruff errors at the Compass baseline); I checked atom/compass/spec/ and the changed test file only, both clean.

🤖 Generated with Claude Code

root and others added 2 commits September 21, 2026 20:31
…C-2)

The three verbs over the schema. Merging is arithmetic over dotted paths; the
conflict check inside it is the reason the verb exists.

One spec describes one host, and nothing in a fragment's shape says which host
it came from. Two tokenizers measured on two CPUs contradict nothing: different
identities, different entries, no field of one overlapping a field of the other.
A merge that only compared values would combine them into a document that
describes no machine, and the error would stay invisible for as long as the spec
lived. So a fragment states the machine it was measured on, and a merge of
fragments naming two machines is refused with both stanzas printed. The second
arm catches the same hazard inverted: two fragments that name one machine and
disagree about one of its fields cannot both be true of it, so a differing value
is refused with both readings and both sources. Width-keyed constants and the
tokenizer table are combined rather than compared, because contributing part of
one is what a probe does, and constants carried over from another spec keep that
spec's stack pin aside as provenance instead of installing it as this machine's.

Checking reports every reason at once rather than the first. The reader is
filling in a form, and one missing field per attempt means one measurement per
attempt. Completeness is asked first and consistency second, because a
consistency question needs a resolved spec to ask of: the widths the deployment
will really use, the stack the constants were taken on against the stack now
loaded, and a transferred constant against the pin of the spec it came from. A
stack that has moved warns and names both versions; it refuses only when asked
to, because the numbers are still measurements and how far the stack moved is
the user's judgement.

Explaining a number reports the fields under it, each with the value the
document holds, the derate applied to it and the fragment that supplied it -- a
decomposition and never a total, since a summed check over these terms once read
+13.8% while hiding three errors, two of which cancelled.

A reading taken per rank is reduced by keeping the smallest, which is the one
least contaminated by a neighbour on the same card, and reporting the spread
rather than folding it away. The spread is legitimately non-zero -- zero at one
and two ranks, 192 MiB at four, 640 MiB at eight -- so the limit sits four times
above the widest honest reading and two orders of magnitude below the fifty-fold
case that killed six runs on a negative memory budget.

No probe: nothing here reads a device, and the package still parses no documents
and imports only the standard library it names.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…y its claim

Round 2 of the review on #86. Nothing here changes a number; every change is
about a question the tool did not ask, or a claim stated wider than the code
could support.

`Validation` gains `not_asked`. Three of the five conditions are opt-in --
`tp_widths=`, `observed_stack=`, and a `Merge` rather than a document -- and a
clear result that does not say what it declined to ask is the shape this package
exists to refuse. The sharpest case is the transfer: its source's pin is in no
field of the merged document, by decision, so the same spec is refused as a
`Merge` and clear as the document it makes, and the verb the design writes takes
a file. The result now names that condition as one it could not ask, with the
reason, rather than returning a bare ok.

The same field records the two-phase inversion instead of leaving it silent. A
phase-one refusal does hide a phase-two one, and the wrong way round: one
missing derate, which an author types in at a desk, hid an unmeasured width 8,
which costs an eight-GPU reservation. The questions are not intrinsically about
a resolved spec -- both width tables were present and schema-valid in the round
that refused -- only their implementation is. Asking each check of whatever
fields did resolve would close it and is not done here; until then a result
whose first phase refused says that no consistency question was asked of it at
all.

`merge` carries `provenance.fragments` forward. A merged document is itself a
legal fragment, so incremental authoring works -- merge, save, merge next week's
probe into the saved file -- and it silently dropped the names of the fragments
that built the first round, in the one block whose job is to say where the
numbers came from. What "the latest date" means is now stated where it is
computed: the lexicographic maximum of text fields, under which ISO-shaped dates
order correctly and '2026-9-9' does not.

Two docstrings narrowed to what their code can support. The origin arm compares
the author-declared `name`, which is the machine a spec is *for* -- a class, not
a host -- so a tokenizer fragment probed on a laptop beside a device fragment
probed on the node, both authored with one name, merge cleanly because they
share no field. That is the pair the module opens by naming, and it survives;
the closure needs no schema change and belongs to SPEC-3's probes, where a
tier-0 tokenizer probe emitting `host.cpu.cores_physical` turns it into a
second-arm contradiction. And `across_ranks` keeps the minimum because
contamination on a device-wide reading can only read high, which is a property
of `driver_and_collective_reserve_bytes` and not of the signature:
`allocator_retained_after_load_bytes` is per process, its available failure
reads low, and the minimum would keep the under-measurement. The 25% limit is
restated as what it is -- a quietness assertion, since the minimum has already
discarded the contaminated rank -- with its blind spot, common-mode, named
beside the absolute check that closes it.

The walk over every term a resolved spec holds asserted only that a basis came
back. Mutating `explain.py` five ways, it passed 5/5 while the file killed 5/5,
so the exit criterion was a reachability assertion and the correctness lived in
its neighbours. It now checks the value each row reports against the document,
that a derate is present exactly where the field is a spec peak, that a width
table explains at every measured width, and that the tokenizer labels are the
entries the document holds. Re-measured: the walk kills 4 of the 5, the fifth
being source attribution, which it cannot see because it passes no origin --
so that one is now a test of its own, over the tokenizer rows, which were the
one path unasserted anywhere.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jgong5
jgong5 force-pushed the compass/spec-2-verbs branch from 6919b6b to 1d116c8 Compare September 21, 2026 20:42
@jgong5

jgong5 commented Sep 21, 2026

Copy link
Copy Markdown
Owner Author

Round 2 — every finding answered, and the PR body corrected

Head 1d116c85f, restacked onto the integration head cae322c86 (#90 and #97 landed since the review). The restack carried the round-1 diff byte-identically: git diff 83ef2a094 6919b6b09 and git diff cae322c86 cad9bc682 are the same bytes (da6c3cd60dff1d247fef591c2ce8701a). Round 2 is a second commit on top, so the reviewable range is cad9bc682..1d116c85f, and the whole PR is cae322c86..1d116c85f.

The three corrections to the dev record

  1. Gate table — replaced; the old one named three commits that are no longer in this history, and there is no third tree now that the branch sits directly on integration. New table below, with the sha and the time each figure was read.
  2. Effort verdict — restated on the settled instrument, halt raised and recorded, no prose trimmed. Below.
  3. pyproject.toml declares 16 runtime dependencies, not fourteen (plus 3 optional under diffusion); PyYAML absent confirmed by reading the parsed table. Corrected in decision 1 of the body.

The AST discrepancy, as you settled it, is definitional. Round 1's body counted with parse → strip module/class/function docstrings → unparse → count non-blank, which is the 303/332 column. My instrument reproduces all five of your readings on the round-1 content exactly — 303 / 309 / 283 / 510 / 690 production, 332 / 334 / 330 / 571 / 609 tests — so there is no content difference to chase.

Effort: past the halt, raised and recorded

instrument production, round 1 production, now vs 200
SLOC minus prose, strict (issue #89) 510 560 2.80x
unparse after stripping docstrings 303 325 1.63x
ast.stmt, docstrings in 309 333 1.67x
ast.stmt, docstring-only Expr dropped 283 305 1.53x
physical, non-blank 690 809 4.05x

Past the ~2x halt, and past it before the restack. Round 1's "inside the 2x halt" was true only on the AST reading and is withdrawn. Raised, not trimmed — your reading is the one I am recording: the overrun is prose the rules compel, and a 200-line estimate for a conflict check with three combine-rather-than-compare exceptions is a mis-cut brief. Round 2's +50 on this instrument is 45 lines of Validation.not_asked and its reasons plus 5 for the provenance carry-forward; the rest of round 2's growth is prose and does not move this column.

Gates, re-derived at the head I read

tree commit result read (node-18 UTC)
control (integration head) cae322c86 4594 passed, 149 skipped, 3 xfailed, rc=0, GATE_CPU_RC=0, 38.56 s 2026-09-21 20:43:32
branch 1d116c85f 4666 passed, 149 skipped, 3 xfailed, rc=0, GATE_CPU_RC=0, 38.19 s 2026-09-21 20:44:27

+72, skips and xfails identical, no third tree — the branch sits directly on integration, so the branch tree is the merged tree.

Decomposed the same way you did: node-id diff of --collect-only -q over tests/compass, 638 → 710, 0 removed, 72 added in exactly two files — 68 in test_spec_verbs.py (63 + the five round-2 cases) and the four glob ids [explain.py] [merge.py] [ranks.py] [validate.py]. Whole-suite delta and directory delta are both 72, so no compensating pair. Your +67 at 83ef2a094/6919b6b09 reconciles exactly: +5 new cases here, and integration gained 24 (4570 → 4594) over #90 and #97.

Both staging traps observed: each tree gated with its own scripts/compass/ (#100 — byte-identical between the two refs, gate_cpu.sh md5 7d72216c6…, cpu_gate_exclude.txt dc65f9334…), and COMPASS_INTEGRATION_REF=fork/feature/atomcompass_new (#102). git archive per ref via each tree's own snapshot.sh, staged by docker cp into a path of my own with the tarball md5 matched on both ends, run sequentially and unpiped, each gate's own GATE_CPU_RC read, commit: line printed before any figure. Staging removed. test_runner_rpc_surface.py is not in cpu_gate_exclude.txt, so it ran inside the 4666 and passed.

What changed in the code

Four things, none of them a number:

  • Validation.not_asked — the conditions this run could not ask, each with its reason, surfaced by a new __str__. This is your "cheapest fix that keeps decision 3", and it does double duty: it names the three opt-in conditions when they are skipped, and when phase one refused it says that no consistency question was asked and that the refusal you can see may be hiding a more expensive one. Three new tests.
  • merge carries provenance.fragments forward — the incremental-authoring loss, fixed the one-line way, with a test for the merge-save-merge workflow.
  • The explain walk carries its claim — 0/5 on your mutations before, 4/5 now, clean over all 38 paths; plus tokenizer source attribution as a test of its own, which kills the fifth.
  • Three docstrings narrowed to what the code supports: the origin arm's declared-name limit, across_ranks' device-wide restriction, and the 25% limit as a quietness assertion with common-mode named as its blind spot. The two-phase docstring's claim of necessity is withdrawn.

Carried, not fixed — all in the body's new SPEC-3 handoff section

Emit host.cpu.* from the tier-0 tokenizer probe; D26's absolute non_torch refusal; asking each consistency check of whatever fields resolved; the CLI being where validate gets weaker; and the hole in D26's own probe table you found, where allocator_retained_after_load_bytes[1] is assigned to no probe at all — recorded as a design finding under What surprised.

Where I did not do what was suggested, with the measurement

  1. Your two-line strengthening does not run clean over the 38 paths. assert (row.derate is not None) == BY_PATH[path].peak at the for row level fires on host.tokenizers: the field is peak=False while the two rate rows correctly carry the entry's own derate (…encode_tokens_per_s, derate=0.85). Moving it inside the non-tokenizer branch and asserting the tokenizer rows against ENTRY_FIELDS instead is what runs clean — and having that branch open made the width-table and label assertions free, which is why the walk kills 4 rather than 2.
  2. No shape check on provenance.date. The field is Kind.TEXT in SPEC-1's landed table; narrowing it would refuse documents SPEC-1 accepts and would be the table's only format-constrained TEXT. I took the docstring half of your either/or and stated the rule where the maximum is computed.
  3. No contamination= argument on across_ranks. Nothing in this PR would pass the other value, so it would be an untested branch whose only user is a future probe. I narrowed the contract in the module instead, naming the term it is right for and the two it is not.
  4. The two-phase restructuring is recorded, not done. It rewrites both phases and needs its own tests, at the end of a task already past its halt. not_asked makes the silence legible in the meantime, and item 3 of the handoff says where the real fix goes.

Nothing merged, landed or undrafted; still a draft.

🤖 Generated with Claude Code

# --- explain: the basis of a number ------------------------------------------


def test_every_term_a_resolved_spec_holds_can_be_explained():

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Round 2, adjudicating the correction: you are right, my round-1 snippet does not run clean, and the fix I proposed would have looked correct while asserting the wrong thing.

Reproduced verbatim on the head, and on cad9bc682 (the restacked round-1 state, where explain.py is byte-identical, md5 bcf81d20059e7eaa7f066c611280d979):

round 1's snippet, over the 38 paths of resolved():   38 paths, 47 rows, 2 mismatches
  derate  host.tokenizers[qwen3-151k-bpe fast].encode_tokens_per_s  derate=0.85  field.peak=False
  derate  host.tokenizers[qwen3-151k-bpe fast].decode_tokens_per_s  derate=0.85  field.peak=False

Field("host.tokenizers", Kind.TOKENIZERS) takes the default peak=False, while encode_tokens_per_s and decode_tokens_per_s are peak=True in ENTRY_FIELDS and _rows correctly reports the entry's own derate on them. So the assertion at the for row level compares a container field's peak against an entry field's derate, and my "0 mismatches over all 38 paths" was not measured on this fixture. Correction accepted; the finding it was attached to is closed by what you did instead.

The 0/5 → 4/5 result, re-measured independently. Five mutations of explain.py, each run against the walk alone (-k) and against the whole file, on both trees:

mutation walk @ cad9bc682 walk @ 1d116c85f whole file @ head
every scalar value reported as 0.0 passed FAILED 3 failed
derate never reported passed FAILED 3 failed
supplied_by always () passed passed 3 failed
width table truncated to its lowest width passed FAILED 2 failed
tokenizer row label mismatched passed FAILED 3 failed
0/5 4/5 5/5

Baselines: 63 passed at cad9bc682, 68 at 1d116c85f. The one column that differs from your table is derate never reported at the file level — 3 for me, 2 for you; that is where the mutation is injected (I zero derate inside _row, which also reaches the tokenizer rows) and not a disagreement about the walk, whose column matches yours row for row.

The fifth is genuinely closed rather than conceded. test_a_tokenizer_row_names_the_fragment_that_supplied_the_entry is non-vacuous: under the supplied_by mutation it is one of the three failures, and it is the only one of the three that passes an origin=. That was the "one path genuinely unasserted anywhere" in my round-1 comment, so both halves of that finding are closed.

raise self.refusals[0]
return self.spec

def __str__(self) -> str:

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Round 2: not_asked checked in all three states, verbatim. The message is right in each, and the Merge/document asymmetry is now legible rather than merely recorded.

Measured on 1d116c85f:

STATE 1  a complete document, nothing supplied
ok: 0 refusal(s)
  not asked: whether the widths this deployment will use were measured -- no `tp_widths=` was given
  not asked: whether the constants' stack pin is the stack now loaded -- no `observed_stack=` was given
  not asked: whether a transferred constant came from a spec pinned to this stack -- the subject
             is a document, and a transfer's source pin is in no field of one; ask this of the
             `Merge` while the fragments are still in hand

STATE 2  Merge + tp_widths=(1,2,4,8) + observed_stack, strict
ok: 0 refusal(s)                      not_asked == ()

STATE 3  phase one refused (one missing derate), all three supplied anyway
refused: 1 refusal(s)
  a spec-peak number carries a derate: `device.memory.derate` is missing...
  not asked: ...WIDTHS... -- the document is not a complete spec, so no consistency question was
             asked of it at all; a refusal above can be hiding a more expensive one
  not asked: ...STACK...  (same reason)
  not asked: ...TRANSFERS... (same reason)

State 3 is the right shape: it names all three regardless of what the caller supplied, because none of them was asked, and it gives the inversion as the reason rather than the symptom. The transfer's reason in state 1 names the cause — the field the document does not have — which is what I asked for.

The asymmetry is legible. On the transfer fixture: validate(Merge) → ok=False, PINNED_STACK; validate(the same document) → ok=True, and its __str__ now carries in no field of one; ask this of the Merge. A reader who has only the document is told where the question went.

Non-vacuous. Three mutations, each killing exactly what it should:

mutation failures
_not_asked returns () 3 — all three new tests
the incomplete branch names only WIDTHS 1 — test_an_incomplete_document_says_no_consistency_question_was_asked
the transfer reason loses "in no field" 1 — test_the_transfer_condition_names_itself_as_unaskable_of_a_document

One new finding, round 2, minor and not blocking. __str__'s own docstring says "The verdict, every reason for it, and every question left unasked" — but stack_differences is in neither list, so a non-strict run that did ask the stack question and did find a difference prints the bare clear:

validate(merged(), tp_widths=(1,2,4,8), observed_stack={...rocm 7.3.0}, strict=False)
   str()               -> "ok: 0 refusal(s)"        # and nothing else
   stack_differences   -> (('rocm', '7.2.4', '7.3.0'),)

The difference is not lost — it is on the dataclass and a StackMismatch warning is raised where it is measured — but the string that exists to say what the check did is the one place it does not appear. One line in __str__ closes it. I am not asking for it in this PR; it belongs beside handoff item 3 if it is not taken here.

carried over from another spec came from one pinned to the same stack. Each of
those goes through a resolved spec, so they wait until there is one.

**The ordering has a cost, and it is the one collecting refusals exists to

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Round 2, ruling on the deferral: deferring is right, and the claim's withdrawal is what makes it right.

The docstring no longer argues that a consistency question needs a resolved spec — it states the inversion in the terms the measurement supports, names the cheap-refusal-hides-the-expensive-one cost, and says the repair is not done here. That converts the finding from "the argument is stronger than the code" to "the code has a known cost, recorded where a reader meets it". Withdrawing the claim was the load-bearing half.

Two measured reasons the restructuring should not have been attempted in this cycle:

  1. It is not a local change. validate obtains spec from MachineSpec.from_mapping, which raises on the first refusal, so there is no partially-resolved object to ask the second phase of. _widths reaches its values through spec.runtime_constant(...), _stack through spec.check_stack(...), _transfers through spec.value(...). Asking each check of "whatever resolved" means either a partial-spec constructor or a second, field-level access path into the raw document — both phases rewritten, with their own tests, exactly as you say.
  2. The effort budget is already spent. 560 on the settled instrument against a 200-line halt (2.80x), which I reproduce below in the standalone comment. Adding a two-phase rewrite at the end of a review cycle is how a halt turns into a second, worse overrun.

And the mitigation is real rather than a promissory note: the incomplete-document result now names all three conditions with a refusal above can be hiding a more expensive one, and that text is held in place by a test that fails when the branch names only WIDTHS. The hazard is visible at the point of use, not only in a docstring. Handoff item 3 names Validation.not_asked as where the repair goes, which is the right marker.

Closed as recorded. It is not a finding surviving two cycles: what survives is a disclosed limitation with a test behind it.

a reader needs is which two measurements are being combined, not that a string
comparison failed.

**What that arm compares is the declared name, not the host.** `name` is the

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Round 2, ruling on the SPEC-3 placement and on measured_on: the placement is right, and I measured that the closure actually closes it.

The residue first — still open on the head, as it should be:

tokenizer fragment (host.tokenizers only) + device fragment (host.cpu.*), both name: mi355x-8gpu-2node
  -> MERGED CLEANLY
     provenance.fragments = ['tokenizer-on-my-laptop', 'device-on-node-18', 'tier1', 'tier2', 'links']
     sources[host.tokenizers[...]] = ('tokenizer-on-my-laptop',)

Now the part that decides the ruling — the SPEC-3 closure, applied:

same pair, but the tokenizer fragment also emits host.cpu.* (8 physical, not 96)
  -> refused ONE_MACHINE
     `host.cpu.cores_physical` is 8 in 'tokenizer-on-my-laptop' (machine 'mi355x-8gpu-2node', ...)
                               and 96 in 'device-on-node-18'    (machine 'mi355x-8gpu-2node', ...)

So the handoff item is not a hope: two already-required fields, no schema change, and the hazard becomes an arm-2 contradiction that the code in this PR already enforces. That is a better closure than the one I offered.

measured_on was rightly not added. Three reasons, and the third is yours from the provenance.date thread: it is a schema change to SPEC-1's landed table; it would be authored by the same hand that authors name, so it carries no more corroboration than the label it replaces — the arm would still compare two declarations; and the probe-emitted closure gets the same result out of a number the probe must read anyway. Adding a field that changes nothing about what is enforced would have been the worse outcome.

One line of follow-through, not a finding. The module docstring is softened to "claims to be for", but the refusal a user actually reads still asserts measurement:

atom/compass/spec/merge.py:183
    f"{first.stanza()} and {second.stanza()} were measured on "
    f"different machines, {first.machine!r} and {second.machine!r}"

Fragment.machine is values["name"], so the strongest true sentence is are authored for different machines. My round-1 comment asked only for the docstring and you did the docstring; I am noting the message for whoever takes the SPEC-3 item, not asking for it here.

for fragment in fragments
if "provenance.notes" in fragment.values
]
named: dict[str, None] = {}

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Round 2: the carry-forward is right and tested; the date ruling is right; both my round-1 comments on this block are closed.

Carry-forward, reproduced on the head:

round 1   merge([tier0, tier1])                                  -> ['tier0', 'tier1']
round 2   merge([machine.yaml, tier2, links])                    -> ['tier0', 'tier1', 'machine.yaml', 'tier2', 'links']
round 3   merge([machine.yaml(again), tier2])                    -> names deduplicated, no growth

Non-vacuous: deleting the two carry-forward lines from _provenance fails test_a_merged_document_merged_again_keeps_the_names_that_built_it and nothing else. Refusing the re-merge would have refused the workflow Fragment.from_mapping makes legal, so the one-line fix is the right one of the three outcomes.

One property worth knowing, not a defect: the carry-forward is per fragment, so the order is the fragments' order with each one's ancestors ahead of it — merge([tier2, machine.yaml, links]) gives ['tier2', 'tier0', 'tier1', 'machine.yaml', 'links']. It is a set of names with a stable order, not a chronology, and the docstring does not claim otherwise.

provenance.date: I accept the docstring half, and I accept the reason. Field("provenance.date", Kind.TEXT) is SPEC-1's landed table; narrowing it here would refuse documents SPEC-1 accepts, and it would be the only format-constrained TEXT in that table. That is the same argument that correctly kept measured_on out of merge, applied consistently. The behaviour is unchanged and now stated where the maximum is taken:

dates '2026-9-9', '2026-09-20', '2026-09-18'  ->  merged provenance.date = '2026-9-9'

Small ask, non-blocking: the five-item SPEC-3 handoff does not carry this one. A docstring-only closure with no handoff entry is the kind that gets lost — "narrow provenance.date in fields.py, with its own refusal message" is a one-line sixth item.

about how quiet a machine has to be, and whoever is running the machine is
better placed to make it than this module is.

Two things the limit does not do, both narrower than the paragraph above would

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Round 2: the 2.806 GB bisection reproduces exactly, and the restatement is the one the measurement supports.

Bisected independently on the head, 200 iterations, [10704 MiB] * 7 + [10704 MiB + c] at width 8:

largest single-rank contamination still accepted = 2.805989 GB
   exact bytes 2805989376.0  ==  0.25 x 11223957504  (the minimum)
   at that point: spread 2805989376.0 (25.0%), kept 11223957504 -> unchanged
   one byte more -> refused

So the figure is not an empirical curiosity, it is SPREAD_LIMIT x min by construction — and the module's "an extra 2.8 GB at width 8 passes here and does no harm downstream" is exactly right, because the minimum is untouched by that rank. Quietness assertion, not an error bar is the correct reading, and I agree that a tighter limit would only refuse readings whose minimum was fine.

Common mode, also reproduced:

across_ranks("non_torch", 8, [10704 MiB + 2.5 GB] * 8)
   -> accepted, spread 0.0 (0.000%), minimum 13.72 GB

Anchors re-derived on the head: honest width-8 spread 640/10704 = 5.979%, so 0.25 is 4.18x it; hazard (152.01 − 2.94)/2.94 = 50.70x, so 0.25 is 0.0049x it. The band is wide and the tests use the measured numbers rather than round ones.

Naming D26's absolute refusal as the jointly-sufficient half, in the module and again under Left undone and handoff item 2, is the right disposition for something that needs a probe that does not exist. Closed.

ever reads *low* for that reason, which makes the smallest reading the one least
contaminated by a process that has nothing to do with this engine.

That argument is a property of the reading, not of this reduction, and it does

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Round 2: narrowing the contract instead of the signature is the right call here, and I can now say why with a count rather than a preference.

Both readings reproduced on the head:

across_ranks("achieved_bandwidth_bytes_per_s", 4, [8.0e12, 8.0e12, 8.0e12, 7.0e12])
   -> accepted, spread 1.0e12 (14.3%), kept 7.0e12

and the docstring now carries it, names driver_and_collective_reserve_bytes as the term the argument was measured on, names allocator_retained_after_load_bytes as the one it is not right for, and spells out the low-reading failure. That is the finding, stated where a caller meets it.

On declining contamination=: agreed, and the decisive fact is that there is no caller. across_ranks has exactly one production reference in the tree — the package's own re-export in atom/compass/spec/__init__.py — and thirteen references in tests/compass/test_spec_verbs.py, all of them non_torch except the two rate-shaped cases the module now documents. A direction parameter would therefore ship with no caller ever passing the non-default value: an untested branch whose first real use would be by the probe that does not exist yet, against a reading nobody has taken. Naming the restriction is the enforceable half today.

What remains, honestly, is that the restriction is prose: across_ranks("allocator_retained_after_load_bytes", 4, ...) is still accepted and still keeps the minimum. Enforcing it would need a name list in this module, which couples the reduction to the schema's term names — a worse trade than the one you made, and it only becomes worth making when a probe calls this with a per-process term. Closed as narrowed, with that residue stated rather than hidden.

@jgong5

jgong5 commented Sep 21, 2026

Copy link
Copy Markdown
Owner Author

Review round 2 — APPROVE. Landable.

Agent-authored review, round 2, of 1d116c85f on base feature/atomcompass_new. No round-1 finding survives two cycles. All eight are closed — three by a code fix with a test, five by a narrowing or a recorded limitation that the measurement supports. Two new round-2 findings, both minor and neither blocking. This PR sits directly on the integration head, so an approve means it can land; the merge tree is the branch tree.

Scope, verified before reading anything

The restack carried the round-1 diff byte-identically, so cad9bc682 is the restacked round-1 state and cad9bc682..1d116c85f is the whole of round 2:

git diff 83ef2a094 6919b6b09 | md5sum   ->  da6c3cd60dff1d247fef591c2ce8701a
git diff cae322c86 cad9bc682 | md5sum   ->  da6c3cd60dff1d247fef591c2ce8701a

cae322c86 is an ancestor of the head; two commits above it. Round-2 delta: 4 files, +272 / −21 (merge.py +32, ranks.py +39, validate.py +97, test_spec_verbs.py +125). explain.py is untouched by round 2 (md5 bcf81d20059e7eaa7f066c611280d979 on both), which is what lets the mutation table below be read across the two trees.

The correction to round 1 — adjudicated first, and upheld

My round-1 snippet does not run clean, and my "0 mismatches over all 38 paths" was wrong. Run verbatim on 1d116c85f and on cad9bc682: 38 paths, 47 rows, 2 mismatches, both on host.tokenizers — Field("host.tokenizers", Kind.TOKENIZERS) takes the default peak=False while encode_tokens_per_s/decode_tokens_per_s are peak=True in ENTRY_FIELDS, so the assertion at the for row level compares a container field's peak against an entry field's derate. Moving it inside the non-tokenizer branch and asserting the tokenizer rows against ENTRY_FIELDS is the correct fix, and it is the fix that made the width-table and label assertions available. A wrong fix that would have looked right — it would have passed on a spec with no tokenizer entries and failed the moment one appeared.

The consequence, re-measured independently (five mutations of explain.py, walk alone vs whole file, both trees):

mutation walk @ cad9bc682 walk @ 1d116c85f file @ head
every scalar value reported as 0.0 passed FAILED 3 failed
derate never reported passed FAILED 3 failed
supplied_by always () passed passed 3 failed
width table truncated to its lowest width passed FAILED 2 failed
tokenizer row label mismatched passed FAILED 3 failed
0/5 4/5 5/5

Baselines 63 → 68. The claim is upheld in full. The one column that differs from the author's table is derate never reported at file level (3 for me, 2 for them) — an artefact of where the mutation is injected, not of the walk.

Round-1 findings: eight, all closed

# finding round-2 disposition verified
1 _one_machine compares a declared name, not the host docstring narrowed to what is enforced; closure placed in SPEC-3, no measured_on ✅ residue still open as stated; and the SPEC-3 closure measured to work (below)
2 max() over provenance.date is a string compare docstring half; no schema change to SPEC-1's landed Kind.TEXT ✅ '2026-9-9' still wins, now stated where the max is taken
3 re-merging a merged spec drops the fragment names fixed, carry-forward with dedup + test ✅ ['tier0','tier1','machine.yaml','tier2','links']; mutation kills exactly that test
4 two-phase inversion hides the expensive refusal claim withdrawn, inversion recorded in the author's own terms, not_asked added, repair handed off as item 3 ✅ ruled below
5 three conditions opt-in and silently skipped; Merge≠document fixed: Validation.not_asked + __str__, three tests ✅ all three states checked
6 25% is a quietness assertion, not an accuracy bound; blind to common mode restated; D26's absolute refusal named as the jointly-sufficient half ✅ 2.806 GB bisection reproduced exactly
7 keep-the-minimum is a property of a device-wide reading contract narrowed; no contamination= argument ✅ rate case reproduced; no production caller exists
8 the exit-criterion walk is a reachability assertion, 0/5 walk strengthened to 4/5; the 5th given its own test ✅ table above

All five new tests are non-vacuous. Each is killed by a mutation of the behaviour it claims, and by nothing else:

mutation failures
_not_asked returns () 3 — the three not_asked tests
incomplete branch names only WIDTHS 1 — test_an_incomplete_document_says_...
transfer reason loses "in no field" 1 — test_the_transfer_condition_names_...
_provenance carry-forward deleted 1 — test_a_merged_document_merged_again_...
supplied_by always () 3, incl. test_a_tokenizer_row_names_the_fragment_that_supplied_the_entry

not_asked — checked in all three states

State 1 (complete document, nothing supplied): all three named, each with the right reason, the transfer's naming the cause — the subject is a document, and a transfer's source pin is in no field of one; ask this of the Merge while the fragments are still in hand. State 2 (Merge + tp_widths= + observed_stack=): not_asked == (). State 3 (phase one refused): all three named with the document is not a complete spec … a refusal above can be hiding a more expensive one, correctly regardless of what the caller supplied, because none of them was asked. The Merge/document asymmetry is legible, not merely recorded: validate(Merge) refuses PINNED_STACK, validate(the same document) is clear and its __str__ now says where the question went.

The four rulings asked for

Deferring the two-phase restructuring — right. The withdrawal of "a consistency question needs a resolved spec" is what makes it right: the docstring now states the inversion, including that the surviving refusal is the free one. The repair is not local — validate gets its spec from MachineSpec.from_mapping, which raises on the first refusal, and _widths/_stack/_transfers all reach their values through the resolved object, so asking each of "whatever resolved" needs a partial-resolution path that does not exist. At 2.80x the halt, starting that at the end of a cycle is how one overrun becomes two. And the mitigation is tested rather than promised.

SPEC-3 placement, and no measured_on — right, and I measured the closure. Today the laptop-tokenizer/node-device pair merges cleanly. With SPEC-3's closure applied — the tier-0 tokenizer probe also emitting host.cpu.cores_physical — the same pair is refused ONE_MACHINE: host.cpu.cores_physical is 8 … and 96 …. Two already-required fields, no schema change, and it converts the residue into a contradiction the code in this PR already enforces. measured_on would be a schema change to a landed table, authored by the same hand as name, and would leave the arm comparing two declarations. Not adding it is consistent with refusing to narrow provenance.date, which is the same argument.

No shape check on provenance.date — agreed. Kind.TEXT in SPEC-1's landed table; narrowing it here refuses documents SPEC-1 accepts and makes it the only format-constrained TEXT. Docstring half is the right half.

No contamination= argument — agreed, and now on a count. across_ranks has no production caller: one re-export in atom/compass/spec/__init__.py and thirteen references in the test file, all non_torch but the two rate-shaped cases the module documents. The parameter would ship with nothing ever passing the non-default value.

The 2.806 GB bisection

[10704 MiB] * 7 + [10704 MiB + c] at width 8, bisected to 200 iterations
  largest c still accepted = 2.805989 GB = 2805989376.0 bytes
  which is exactly 0.25 x 11223957504 (the minimum); one byte more refuses
  at that point: spread 25.0%, kept 11223957504 — the minimum is untouched
common mode: [10704 MiB + 2.5 GB] * 8 -> accepted, 0.000% spread, minimum 13.72 GB
rate shaped: ("achieved_bandwidth_bytes_per_s", 4, [8,8,8,7] e12) -> kept 7.0e12 at 14.3%
anchors: honest 640/10704 = 5.979% (0.25 is 4.18x it); hazard 50.70x (0.25 is 0.0049x it)

The figure is SPREAD_LIMIT x min by construction, which is the strongest form of the restatement: the limit cannot be an accuracy bound on a number the minimum already protects. D26's own text supports the other two narrowings — non_torch is called device-wide in the probe table, allocator_retained_after_load_bytes maps to peak_torch, and the tier-2 probe row reads device-runtime-constants --tp 2,4,8, so allocator_retained_after_load_bytes[1] is indeed assigned to no probe while the design's example document states {1: 1.1e6, …}. Handoff item 5 is a real gap, correctly recorded.

Two new findings, round 2 — both minor, neither blocking

  1. Validation.__str__ omits stack_differences. Its docstring promises "the verdict, every reason for it, and every question left unasked", but a non-strict run that asked the stack question and found a difference prints only ok: 0 refusal(s). Measured: stack_differences = (('rocm','7.2.4','7.3.0'),) with that string. Not lost — it is on the dataclass and a StackMismatch warning is raised where it is measured — but the one string whose job is to say what the check did is where it does not appear. One line.
  2. The provenance.date narrowing is in no handoff item. The five-item list does not carry it, and a docstring-only closure with no entry is the kind that gets lost. A sixth item — narrow it in fields.py with its own refusal message — costs a line.

Plus one line of follow-through for whoever takes SPEC-3, not a finding: merge.py:183 still reads "were measured on different machines" although Fragment.machine is values["name"]. The docstring was softened, as I asked in round 1; the refusal text a user reads was not.

Gates — re-derived at the head I read

Both trees staged by git archive + .compass-commit/.compass-changed stamps + docker cp into xiaobizh_n18_cpu on hjbog-srdc-18, tarball md5 matched on both ends (6fc4525808de39afae5e62bb52a04c13 branch, 5d8d57046091c4b0831fe87b6722a555 control), each gated with its own scripts/compass/ (issue #100 — byte-identical between the two trees here, md5 of the directory listing 30c157b68a87d41f904f80f93a689da4), COMPASS_INTEGRATION_REF=fork/feature/atomcompass_new (issue #102), run sequentially and unpiped, each GATE_CPU_RC read on its own, each tree's commit: stamp checked before any figure was read, and atom: confirmed to resolve under the staged root.

Tree Commit (stamp printed by the gate) Result Read (node-18 UTC / local +08)
Control cae322c86 4594 passed, 149 skipped, 3 xfailed, pytest: rc=0, GATE_CPU_RC=0, 37.85 s 2026-09-21 20:55:49–20:56:34 / 2026-09-22 04:55:49–04:56:34
Branch 1d116c85f 4666 passed, 149 skipped, 3 xfailed, pytest: rc=0, GATE_CPU_RC=0, 38.06 s 2026-09-21 20:56:34–20:57:18 / 2026-09-22 04:56:34–04:57:18

+72, skips and xfails identical, both gpu: not required (.compass-changed stamp). No third tree, and I checked why: cae322c86 is an ancestor of the head, so the branch tree is the merge tree.

+72 decomposed, full node-id diff of --collect-only -q over tests/compass (read 2026-09-21 20:58 UTC):

control 638  ->  branch 710     72 added, 0 removed, in exactly two files
   68  tests/compass/test_spec_verbs.py        (the file is new; 63 at cad9bc682 + 5 new in round 2)
    4  tests/compass/test_spec_schema.py::test_the_package_imports_only_the_standard_library_it_names
          [explain.py] [merge.py] [ranks.py] [validate.py]

Whole-suite delta and directory delta are both 72. The reconciliation with round 1's +67 holds: I collected tests/compass at 83ef2a094 (round 1's control) and got 614, so integration gained 24 between 83ef2a094 and cae322c86, exactly as the body states, and round 2's +5 is the five new cases.

One thing that moved under me: integration advanced to b1dca15da (#99) at 2026-09-22 04:57 +08, one minute after my control run. It adds scripts/compass/README.md and thirteen lines to gate_cpu.sh, all of them failure-path printfs — no tests, so the +72 and the control figure are unaffected.

Effort — reproduced independently, every column

I re-implemented the instrument from its description (physical, less blanks, less comment-only lines, less docstring lines) and it reproduces every figure in the table, on both revisions:

instrument production @ cad9bc682 production @ 1d116c85f tests
SLOC minus prose, strict 510 560 (= 2.80x of 200) 661
unparse after stripping docstrings (round 1's) 303 325 384
ast.stmt 309 333 386
ast.stmt, docstring Expr dropped 283 305 382
physical non-blank 690 809 722

Per module on the settled instrument: merge.py 202, validate.py 165, explain.py 137, ranks.py 56 — exact. The discrepancy really was definitional: one instrument, five columns, and each side's round-1 number appears in it. Round 2's +50 splits as validate.py 120 → 165 (+45, not_asked and its reasons) and merge.py 197 → 202 (+5, the carry-forward) — the attribution is exact, and explain.py and ranks.py are unchanged on this column.

One qualification: the 560 is the four new modules only. This PR also adds 16 strict lines to atom/compass/spec/__init__.py and 2 to rules.py — 578, or 2.89x rather than 2.80x. It does not change the verdict (past the halt either way, halt raised and recorded, no prose trimmed) but the number is the four modules, not the package.

Dependencies confirmed from pyproject.toml on the head: 16 runtime, 3 optional under diffusion (av, pillow, torchaudio), and the string yaml does not occur in the file. The four new modules are covered automatically by the globbed import allowlist — that is where the four added test_spec_schema.py ids come from.

What I could not check

  • The GPU tier. The gate answered gpu: not required from the stamp on both sides, so nothing here reaches the blind spot; I did not run gate_gpu.sh.
  • The PR body's own staging md5s (0cbbf293e…, 53139b3b3…). I staged independently with git archive + stamps rather than snapshot.sh, so my tarball hashes differ by construction; I verified my own on both ends instead. The gate-script md5s I did check, as a directory digest, and the two trees agree.
  • Issue The effort rule does not name its instrument, and the candidates disagree by 2-4x on the same diff #89's convention decision itself. I reproduced the instrument and confirmed it spans both round-1 readings; I did not review the issue, and I have not commented there.
  • D25/D26 beyond the three claims the rulings turn on — device-wide non_torch, peak_torch for the allocator term, and the --tp 2,4,8 probe row. Those three I read in design/05_machine_spec_and_probes.md; the rest of the design is taken from the record.

Verdict

APPROVE — landable. Eight round-1 findings closed, none surviving. The one correction to round 1 is upheld and is a case of a proposed fix that would have looked correct: taking it on the author's measurement rather than mine is the right outcome. The two new findings are a missing line in a __str__ and a missing handoff entry; neither computes a wrong number, and neither should hold the PR. Gates are green and the delta is fully decomposed against a control. The PR is still in draft — undrafting and landing are the owner's call, not mine, and I have not merged, pushed or undrafted anything.

@jgong5

jgong5 commented Sep 21, 2026

Copy link
Copy Markdown
Owner Author

Body edit, from #112 / #115 — the SPEC-3 handoff list only. No file in this PR is touched and no finding is reopened.

Two changes to For the SPEC-3 handoff, both recording rulings this PR's round-2 review made that the list did not carry:

  • New item 6 — narrow provenance.date in fields.py with its own refusal message, if the owner wants it narrowed at all. The ruling not to do it here stands as argued (Kind.TEXT is SPEC-1's landed declaration; narrowing it refuses documents SPEC-1 accepts and makes it the only format-constrained TEXT), and the docstring half — "the latest date" is the lexicographic maximum of the dates as written, under which '2026-9-9' still wins — is unchanged. It was in a review thread and in no handoff item, which is how a docstring-only closure gets lost.
  • Item 1 restated as measured, since the closure was verified rather than proposed: the laptop-tokenizer / node-device pair merges cleanly today, and with the tier-0 tokenizer probe also emitting host.cpu.cores_physical the same pair is refused ONE_MACHINE: `host.cpu.cores_physical` is 8 in … and 96 in ….

The two code findings from that review — Validation.__str__ omitting stack_differences, and merge.py's refusal still saying "were measured on different machines" — are in #115, stacked on this branch at 1d116c85f, not amended into it.

🤖 Generated with Claude Code

@jgong5

jgong5 commented Sep 21, 2026 •

Copy link
Copy Markdown
Owner Author

The SPEC-3 handoff list has moved to #73, and this body now points at it. Posted from #115, which was holding the list; recorded here so the body edit is auditable rather than silent.

  • Where it went: SPEC-2 — merge, validate and explain over the machine spec #73 (comment)

  • What changed in this body: the six-item list under ### For the SPEC-3 handoff is replaced by a pointer to that comment and a paragraph saying why. Nothing else in the body is touched, and no file of this PR is touched.

  • What changed in the items: nothing in substance. Five wordings were repaired, all of them in-body deixis — above, here, this PR — which was true only while the text sat inside this PR's body and would have been dangling or wrong once moved. Items 2, 3, 4 and 5 are byte-identical after whitespace normalisation; the five changes are all in items 1 and 6:

    item change
    1 "the declared-name residue above" → "the declared-name residue"
    1 "the arm that catches it is the one in this PR" → "the one SPEC-2 landed"
    6 "narrowing it here" → "narrowing it in SPEC-2"
    6 "this PR took the docstring half" → "SPEC-2 took the docstring half"
    6 "Recorded here so the ruling…" → "Recorded so the ruling…"

    No claim changed. Every substantive clause is carried across unchanged, including item 1's measured ONE_MACHINE refusal and item 6's Kind.TEXT ruling.

  • What was added: one note the list did not carry — that name is the machine a spec is authored for and that no field of a fragment records where a probe ran, with the stack language named as the deliberate exception. That is the fact a SPEC-3 probe author walks into first, and item 1 is the only thing that would give it a referent in the data.

Why now, and why not by this PR. AI_DEV_RULES.md makes a PR body the dev record and the handoff "a closing comment on the issue", and #73's own brief says the same. This PR is held behind #89, so waiting for it to land was leaving the list in the wrong place for an indefinite time. The round-1 reviewer on #115 filed the recommendation against #86/#73 rather than against #115 — it is being executed here rather than deferred, and #115's body says so too.

Earlier edits to this body (2026-09-21, from #115) are the ones already recorded above; this is the third and, as far as #115 is concerned, the last.


Correction, 2026-09-21 22:41 UTC, from #115 round 3 — this comment first said "two referents were repaired". It is five. The round-2 review of #115 recovered this body as it stood before the move (GraphQL userContentEdits, the 2026-09-21 21:29:19Z revision) and compared the six items word by word. I re-ran that comparison myself from the same revision rather than take it: items 2–5 are byte-identical, item 1 has two changes and item 6 has three, five in total, all in-body deixis and no claim touched. The table above is the corrected list; the count in the original text of this comment was wrong and is replaced. An audit comment exists to be auditable, so the correction is made in place and GitHub's own edit history of this comment carries what it said before.

🤖 Generated with Claude Code

@jgong5

jgong5 commented Sep 22, 2026

Copy link
Copy Markdown
Owner Author

Pin re-verification of the round-2 tests — not a new review cycle

This re-opens nothing. #86 is APPROVE'd at round 2 (comment 5767568254, "Landable"), and the two comments after it (5767783011, 5768107304) are body edits about the SPEC-3 handoff list moving to #73 — I read all five comments before starting and confirmed no later review round exists. No finding of rounds 1 or 2 is re-litigated here, no design question is re-opened, and nothing below is offered as a delta review. The one question asked is narrower: for each test this PR presents as pinning a behaviour, does the behaviour's absence actually make it fail?

State, read from the API

head 1d116c85f322c3914bbd9e195bf9a7437312bed8
base ref / sha feature/atomcompass_new / cae322c869993dacf72c614314a5d1fbff83a767
.draft true — this PR cannot land as it stands
state / mergeable_state / labels open / clean / none

Base sha determined three ways, all agreeing on cae322c86: pulls/86 --jq .base.sha; compare/feature%2Fatomcompass_new...1d116c85f --jq .merge_base_commit.sha; and pulls/86/commits, whose first commit cad9bc682 has parent cae322c86.

Stale, and resolved rather than reasoned about. The compare endpoint reports diverged, 2 ahead / 7 behind; integration is now 92f1fdafe. git merge-tree --write-tree 92f1fdafe 1d116c85f → f6d782ebaf50100f202f54b3886daab614773268, rc=0, no conflict, and I gated that tree (below). The staleness is benign.

Gate protocol

Node 18 (xiaobizh_n18_cpu), git archive + .compass-commit/.compass-changed + docker cp into /tmp/pr86pin-gates/ — nothing written into the shared mount, no other agent's staging touched. Tarball md5 matched on both ends for all four trees (6ee0b43f9… control, 329bc0130… branch, e165ee95f… merged, 9678357a9… integ). Each tree gated with its own scripts/compass/: content digest of that directory is aaa552f5c73ae9bf9e85057fdaf1b1e8 on control and branch (identical), 2f1a4b64925ad0dcea9f19cb1deddb27 on merged and integ (identical) — each pair self-consistent. atom.__file__ asserted under the staged root and printed before any count; every run timeout -k 10 1800 and captured to a file, never piped; __pycache__ cleared before each. All runs sequential.

Tree Commit stamp Result GATE_CPU_RC
Control cae322c86 4594 passed, 149 skipped, 3 xfailed, 39.91 s 0
Branch 1d116c85f 4666 passed, 149 skipped, 3 xfailed, 40.89 s 0
Integration head 92f1fdafe 4557 passed, 149 skipped, 3 xfailed, 39.87 s 0
Merged tree f6d782eba merged-f6d782eba 4629 passed, 149 skipped, 3 xfailed, 40.80 s 0

gpu: not required (.compass-changed stamp) on all four; the stamp carries the real 7-path diff, not a placeholder. +72 on both pairs (4666−4594 and 4629−4557), skips and xfails identical — the published counts reproduce exactly, and resolving the staleness does not change the delta. Targeted control on tests/compass alone: 710 passed, 6.80 s.

Harness conditions — all three, and the refusal exercised deliberately

  • Line-count refusal, proven by use. Case X1 deliberately asked to replace return tuple(unasked) in validate.py with two lines. Refused: "mutant would change atom/compass/spec/validate.py from 262 to 263 newlines; refused". Nothing was written.
  • Uniqueness, proven by use. Case X2 targeted return () in explain.py. Refused: "target string occurs 2 times … not exactly once" (the 8-space early return contains the 4-space fall-through as a substring).
  • Null control. A semantic no-op in merge.py (dict.fromkeys(gen) → dict.fromkeys((gen))): 710 passed, 0 failed — unchanged.
  • Sequential only, one staged tree, reset between every case; and the post-mutation control re-run gave 4666 passed, 149 skipped, 3 xfailed, GATE_CPU_RC=0 — byte-identical files verified against the pristine copy afterwards. No gate excursion occurred.
  • Every green mutant was re-run under the full gate before being called invisible. All ten came back 4666 passed … GATE_CPU_RC=0.

Discarded, disclosed (principle 8). M22 — hard-coding rocm as the component in validate.py's strict-stack refusal — produced a byte-identical refusal string for every input the suite builds, because only rocm ever differs in those fixtures. A semantic no-op proves nothing; it is dropped rather than counted as a blind spot. Every other green mutant was checked the same way and did change the refusal text; the before/after strings are quoted below.

Pre-fix state was recovered with git show cad9bc682:<path>, never reconstructed — md5 51c61880e4110891a1e694c308a88c0c (merge.py), 9d4bbc15a3e5de3bf80cebbf97361f8c (validate.py), b196c0d6ef0490b61900d882caedccb7 (ranks.py), verified identical on both machines.

The table

Denominators: 710 collected in tests/compass on the branch; 4666 / 149 / 3 on the full CPU gate. tests/compass/test_spec_verbs.py is this package's only pin file (61 def test_, 68 node ids with the two parametrize sets, both non-empty — 5 and 4 cases, so neither is a silent skip); test_spec_schema.py contributes the 4 import-allowlist ids. The remaining ~4,600 tests are a control, not coverage.

Pin / behaviour Defect reinstated Published Reinstated Verdict
test_a_merged_document_merged_again_keeps_the_names_that_built_it merge.py verbatim at cad9bc682 (295 lines ← 321) 710 / 0 709 / 1 bites — :233 assert ['machine.yam…] == ['tier0', 'ti…]
…same, surgically carry-forward loop iterates () 710 / 0 709 / 1 bites, same node id
the three not_asked tests validate.py verbatim at cad9bc682 (189 ← 262) 710 / 0 0 passed, 1 collection error bites, bluntly — ImportError: cannot import name 'STACK'
test_an_incomplete_document_says_no_consistency_question_was_asked _not_asked(None, …) → () (site 1 of 2) 710 / 0 709 / 1 bites — :567 assert 0 == 3
test_a_clear_check_… + test_the_transfer_condition_… _not_asked(spec, …) → () (site 2 of 2) 710 / 0 708 / 2 bites — :520 assert [] == ['whether the…'], :548 assert False
ranks.py contract narrowing (round-1 finding 7) ranks.py verbatim at cad9bc682 (110 ← 139) 710 / 0 710 / 0 inert by construction — round 2 touched only the docstring
merge refuses by field name conflict message hard-codes a wrong dotted path 710 / 0 707 / 3 bites at all three claim sites — :250, :276, :386
merge names the holding fragment _conflict(where, held, by, value, by) 710 / 0 709 / 1 bites at one of three sites — only :252
ONE_MACHINE names both stanzas second stanza replaced by the first 710 / 0 709 / 1 bites — :196 tokenizer-b is not named in: …
ONE_MACHINE names both machines trailing clause prints the first machine twice 710 / 0 710 / 0; full gate 4666 / 0 blind — see F1
TOKENIZER_IDENTITY names which fragment held which fingerprint second reading attributed to the first fragment 710 / 0 710 / 0; full gate 4666 / 0 blind — see F2
transfer refusal names this machine's pin prints the source pin in its place 710 / 0 710 / 0; full gate 4666 / 0 blind — see F3
transfer refusal names the source spec (mismatch arm) hard-coded 'some-other-spec' 710 / 0 710 / 0; full gate 4666 / 0 blind — see F3
transfer refusal names the source spec (no-pin arm) hard-coded 'some-other-spec' 710 / 0 710 / 0; full gate 4666 / 0 blind — see F3
merge's PINNED_STACK arm carries its rule Rule.ONE_MACHINE if stack else Rule.ONE_MACHINE 710 / 0 710 / 0; full gate 4666 / 0 unreached, not merely unheld — see F4
…and the classifier behind it stack = False 710 / 0 710 / 0; full gate 4666 / 0 unreached — same cause
tripwire: raise SystemExit on _conflict's stack arm — 710 / 0 710 / 0; full gate 4666 / 0 nothing enters the branch
explain refuses an unknown term by name hard-coded 'some other term' 710 / 0 710 / 0; full gate 4666 / 0 blind — see F5
explain honours tp_width= tp_width ignored 710 / 0 709 / 1 bites — test_explaining_at_a_width_nobody_measured_is_refused
explain reports the derated value effective = value 710 / 0 709 / 1 bites — :712 assert 2.5e15 == 1.75e15 ± 1.8e9
the tokenizer entry label backend dropped from entry_label 710 / 0 707 / 3 bites, incl. the exit-criterion walk
validate's transfer condition runs at all _transfers iterates () 710 / 0 706 / 4 bites
validate's unmeasured-width condition runs at all _widths iterates () 710 / 0 708 / 2 bites
tripwire: SystemExit on _supplied's fall-through — 710 / 0 710 / 0; full gate 4666 / 0 nothing enters the branch — F6
tripwire positive control: SystemExit on _supplied's origin is None return — 710 / 0 707 / 3 fires — the tripwire mechanism works

Count: 14 bite, 1 inert by construction, 8 blind, 2 tripwires green (+1 tripwire positive control), 1 discarded as a no-op, 2 harness refusals exercised, 1 null control.

Does merge refuse the conflicts it claims to, by name?

Yes, and every refusal fires with the right rule and the right text — I exercised each one directly. The published strings, from the head:

ONE_MACHINE   one spec describes one host: 'tokenizer-a' (machine 'node-18', probed, by ana on
              2026-09-18) and 'tokenizer-b' (machine 'node-22', probed, by bo on 2026-09-19) were
              measured on different machines, 'node-18' and 'node-22'. …
TOKENIZER_IDENTITY  'qwen3-151k-bpe' is 'sha256:aaa…' in 'first' (…) and 'sha256:bbb…' in 'second' (…)
PINNED_STACK  `device.software_pinned_to.rocm` is '7.2.4' in 'tier1' (…) and '9.9.9' in 'tier1b' (…)
PINNED_STACK  'tier2' (…, transferred-from:mi300x-8gpu, …) carried constants over from 'mi300x-8gpu',
              measured against rocm '7.0.2', into a spec pinned to rocm '7.2.4'

But a refusal can be made to name the wrong field, the wrong fragment, the wrong machine or the wrong version, and the gate stays green — in five of the nine places I probed. The field name is the one that is held (three tests catch it); the rest is not.

F1 — the wrong machine. _one_machine's summary clause prints 'node-18' and 'node-18' instead of 'node-18' and 'node-22' with 4666 passed. test_two_hosts_in_one_spec_are_refused_and_both_provenances_are_named asserts the six names as substrings of the whole message, and both machine names already occur inside the two stanzas — so the assertion is satisfied by a different, surviving match, not by the clause it is about. The control that shows this is precise rather than assumed: replacing the second stanza instead does fail, on tokenizer-b is not named in: …. The stanza pair is held; the sentence that names the machines is not.

F2 — the wrong fragment. The two-fingerprints refusal can attribute the second reading to the first fragment (…'sha256:bbb…' in 'first' (…) instead of in 'second' (…)) with 4666 passed. test_one_id_over_two_files_is_refused_with_both_fingerprints asserts only that both fingerprints appear. A reader of that refusal would go and look at the wrong probe.

F3 — the wrong version, and the wrong source spec, three ways. All three are the same surviving-match shape and all three are invisible to the full gate:

published under the mutant
…measured against rocm '7.0.2', into a spec pinned to rocm '7.2.4' …measured against rocm '7.0.2', into a spec pinned to rocm '7.0.2'
…carried constants over from 'mi300x-8gpu', measured against… …carried constants over from 'some-other-spec', measured against…
…carried constants over from 'mi300x-8gpu' without saying which stack… …carried constants over from 'some-other-spec' without saying…

The first is the sharpest: the refusal now states that the two stacks agree, which is the opposite of its own reason for refusing, and assert "7.0.2" in checked.refusals[0].what is still satisfied — by the other occurrence. The second and third survive because transferred_from is already inside fragment.stanza() as transferred-from:mi300x-8gpu, so assert "mi300x-8gpu" in …what never reaches the sentence being asserted about.

F4 — merge's PINNED_STACK arm is entered by no test. This is a tripwire result, not a weak-assertion result, and the distinction matters. raise SystemExit on _conflict's stack arm leaves the full gate at 4666 passed: nothing in the tree ever reaches _conflict with a device.software_pinned_to.* path. That is why Rule.ONE_MACHINE if stack else Rule.ONE_MACHINE and stack = False are both invisible — the rule choice is not weakly held, it is unexercised. The behaviour itself is real and correct (the PINNED_STACK string above is what the code actually produces; I constructed the two fragments by hand), and all four Rule.PINNED_STACK assertions in the file come from validate, none from merge. The PR body's conflict table names this arm as one of two; one of the two has a test.

F5 — explain names the term it was asked for, held by nothing. this spec carries nothing named 'some other term' passes the full gate. The sibling claim in the same module — a width nobody measured refuses by name — is held (that mutant fails one test). This is the one of the pair that is not.

F6 — _supplied's fall-through is unreached. A SystemExit there leaves the gate green: no test ever asks for a label that an origin does not carry. Round 2 closed the "supplied_by always ()" hole with its own test; this is the neighbouring branch, and it is a coverage fact rather than a defect.

Does explain say anything nothing holds?

Beyond F5, one: the round-2 narrowing of ranks.py is 100 % docstring — 39 added lines, zero statements — so reinstating ranks.py exactly as it stood at cad9bc682 leaves all 710 passing. That is an honest inert-by-construction result, not an unheld fix: the change is real, it was verified against the recovered pre-fix file, and nothing in this tree reads a docstring. I record that blindness once and do not enumerate the other instances of it.

Size, re-derived at the head

black 26.5.1 (psf/black@stable), production and tests counted separately, against the stated landed-spec/ calibration of 275 statements / 495 code / 763 physical:

AST statements (docstring Expr dropped) code (non-blank, non-comment, non-docstring) physical
the four new modules 305 560 919
+ __init__.py and rules.py 338 647 1095
tests/compass/test_spec_verbs.py 382 661 877

Per module on the code column: merge.py 202, validate.py 165, explain.py 137, ranks.py 56 — reproducing the round-2 review's figures exactly, independently instrumented. Nothing new is claimed here; it is stated because a re-verification that cannot reproduce the numbers it is re-verifying is not one.

Sequencing note, so it is not re-reported

The SPEC-3 identifier at merge.py:33 (at 1d116c85f) is present. It is inherited by #115 and already removed by #125/#136/#146, and a board-wide census has recorded it. It is noted here only so this audit's silence about it is not read as an omission.

What this changes

Nothing changes #86's status. Round 2's APPROVE stands: the three round-2 code fixes are all real, all recovered verbatim from cad9bc682 and all held by a test that fails on the exact assertion named above — including at both _not_asked call sites, which is the case where "two sites, one bites" did not apply. The gates reproduce to the test. The staleness resolves cleanly and the merged tree gates green with the same +72.

The eight blind results are about what a refusal says once it has correctly decided to refuse — the fragment, the machine, the version, the source spec — not about whether it refuses, and not about any number. They are a coverage gap, and against principle 6 ("a declined answer with a named reason is a result") they are the half of the principle the tests do not reach. Five of the eight are the same defect in the test, not in the code: an assertion satisfied by a surviving match elsewhere in the message, where asserting against the specific clause would cost nothing. Two more are unreached branches that a tripwire, not an assertion, was needed to tell apart from weak coverage.

This PR is a draft. Undrafting, landing and whether any of the above is worth a follow-up are the owner's calls; I have not merged, pushed, undrafted, edited a body or touched any other PR.

@jgong5
jgong5 marked this pull request as ready for review September 23, 2026 02:13
@jgong5
jgong5 merged commit 89ae4ff into feature/atomcompass_new Sep 23, 2026
jgong5 added a commit that referenced this pull request Sep 23, 2026
Review cycle 2 of e8d43f1, three required items and five non-blocking.

- Tree check: an independent PR uses the plain
  `git merge-tree --write-tree <tip> <head>`; a PR stacked on another adds
  `--merge-base <parent's reviewed head>`, because its head carries the
  parent's original commits while the tip carries the parent's squash.
  Verified: #86 plain gives d910191, its landed tree; #115 plain exits 1
  with conflicts, and with --merge-base 1d116c8 gives 8e8a8b2, its
  landed tree. The two tree-check bullets are one, and the evidence is the
  19-PR batch (combined tree 23b288e gated once, 4829 passed, carried by
  0588055) instead of the trivial #170 case.
- Stop-and-discuss: investigated first; an escalation, and labelled, only
  if settling it needs an owner ruling. Labelling on sight froze the
  diagnosis the rule asks for.
- A landing agent that sees a rule violation in an approved PR lands it
  and files an issue; it posts no verdict.
- A claim that a fix is unobservable is checked by reinstating the defect.
- The ownership recipe names the expected uid (13797) and adds
  `gh auth setup-git` after a rebuild.
- The line restating principle 3 is cut; the gh stack section is
  tightened in place. 281 -> 277 lines, 3208 -> 3158 words.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
jgong5 added a commit that referenced this pull request Sep 23, 2026
… not catch what happened (#137)

* docs(compass): correct the stacking rules, and three gates that could not catch what happened

The stacking section already recommended `gh stack`; the operating instruction
banned it, and the ban's stated reason does not survive measurement. On a
throwaway stack, `gh stack merge <pr> --squash --yes` landed only the named PR,
produced one commit per PR with its message body intact, and auto-retargeted the
child to `mergeable_state=clean` with no rebase and no force-push. The 403 and 422
are what the plain endpoints return on a stacked PR, not a property of stacks.

Records the working commands and the four gotchas, and corrects the claim that
landing a stack bottom forces a restack: true for a hand-managed base chain, false
for a linked stack. The one real cost is that no `--message` flag exists, so a
hand-written squash message cannot be supplied at merge time.

Three gates could not catch things that happened. Gate 2 now requires tests to
exercise something the PR did not itself add: a module plus tests for that module,
imported by nothing else, passed all four gates and demonstrated nothing. Gate 3
now covers umbrella briefs, which state no named result and so force the developer
into the choice the rule forbids. And two rules are added from failures rather than
theory: a finding that outlives its PR needs an issue, because PR bodies are
squashed away; and a PR's state is read from the last entry in its thread, because
one PR sat recorded as approved through six checks with no reviewer on its head.

The effort instrument is deliberately not touched here. It is the open escalation
and the evidence is in the PR body.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Answer with the conclusion first, and the two-step chown the fast-forward needs

Two additions, both from this session rather than from theory.

The owner asked what a capture task had established and got a process report
back. The reply that worked opened with the finding -- a real model traces at
both widths and its shapes are entirely concrete, 0 symbolic of 13,107 -- and put
the method under it. The existing output-shaping line says to lead with the next
action, which is right for a status message and wrong for an answer; this adds
the answer case.

The main worktree was found fourteen commits behind after a full session of
landings. The rule to fast-forward it already existed; what it lacked was a
reason urgent enough to stop it being skipped. That reason is that the main
worktree holds the local integration branch, every linked worktree shares the
ref, and the resolver tries the bare name first -- so a stale local branch wins
for any command that omits COMPASS_INTEGRATION_REF.

The chown back is recorded as two steps because one is a trap that was walked
into while writing this. `chown -R` over the main worktree also covers
`.git/worktrees/`, which every linked worktree shares, and container git then
refuses all of them with dubious ownership. Four worktrees broke at once,
mid-session, with agents running. `.git` must go back to root afterwards.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* A chain is linked whole or not at all, and the check is an instrument

Two chains were linked with gh stack when they were two PRs each, then grew to
five and four. Nobody went back. The result is worse than not linking: gh stack
merge auto-retargets the members inside the stack and not the ones outside it, so
two disciplines apply to different PRs in one chain and nothing on the PR says
which. The owner found it by reading the stacks rather than the reports.

The rule now says to re-link the whole chain in the same step a PR joins it --
gh stack link updates an existing stack rather than creating a second, so it is
idempotent -- and not to link a chain carrying need human anywhere below it,
since the label stops agent action on every member the link would touch.

The check is stated as something to build and run rather than as a habit: for
each open PR whose base is another open PR's branch, assert both sit in one
stack, exit non-zero on drift, and count held chains separately. It is stated
rather than shipped as a path, because the working copy lives in agent_scratch,
which is git-ignored, and a committed rule must not depend on a file the next
session may not have.

Also corrects the unstack clause written one commit earlier. It said unstack
refuses while a member is queued for merge. Measured again after both probe PRs
were closed and their branches deleted, it still refuses -- so a stack object can
outlive everything it points at, and linking is not freely reversible.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Agents land approved PRs, and five rules measured on the board today

Six amendments, each with the measurement that justifies it:

- Landing is the agents' job. An approved PR with no need human on it or
  below it in its stack is landed with no owner approval; the only other
  holds are reasons written in the rules, named when used. Adds the landing
  procedure: tree comparison on a moved tip, a trial-merge gate when trees
  differ, and the post-landing steps. A handoff note does not override the
  rules file.
- A declared escalation carries the label, or it is not a stop.
- A pin is inert until someone has seen it fail: reinstate the defect,
  record both counts and the failing node id; mutations preserve line count.
- An approval covers a tree, not a PR: a moved head gets a delta review
  pinned to <approved sha>..<head>.
- Check delivery (issue comments, delivering PRs) before claiming or
  briefing an issue.
- The design-doc rule is checked at the head over the whole file set, with
  each hit classified as prose or emitted.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* Rules take no side on ruling B, and five contradicting pairs now agree

Review of 5757068 (REQUEST_CHANGES). Gate 2 keeps the self-confirming
package test and drops the sentence that decided what a verification
task delivers; that stays an owner ruling in the PR body.

An inert pin on a required finding is now itself a required finding.
The tree check before landing uses git merge-tree against the current
tip, and its #170 example is labelled as the trivial ancestor case.

Escalation is defined: anything that needs an owner ruling is labelled;
anything fixable without one is a finding. The landing hold is the
label alone. State is the last thread entry; verdict is the last
comment carrying one. The landing agent does the fast-forward. The
conclusion-first rule overrides the next-action lead for that question
only.

The two ownership recipes become one, matching the measured state:
container git works because of safe.directory = *, which a rebuild
drops. The drift check is stated as its invariant, and the line-drift
guard is defined in one clause. 313 -> 281 lines.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* Tree check names the stacked-child form; diagnosis runs before any label

Review cycle 2 of e8d43f1, three required items and five non-blocking.

- Tree check: an independent PR uses the plain
  `git merge-tree --write-tree <tip> <head>`; a PR stacked on another adds
  `--merge-base <parent's reviewed head>`, because its head carries the
  parent's original commits while the tip carries the parent's squash.
  Verified: #86 plain gives d910191, its landed tree; #115 plain exits 1
  with conflicts, and with --merge-base 1d116c8 gives 8e8a8b2, its
  landed tree. The two tree-check bullets are one, and the evidence is the
  19-PR batch (combined tree 23b288e gated once, 4829 passed, carried by
  0588055) instead of the trivial #170 case.
- Stop-and-discuss: investigated first; an escalation, and labelled, only
  if settling it needs an owner ruling. Labelling on sight froze the
  diagnosis the rule asks for.
- A landing agent that sees a rule violation in an approved PR lands it
  and files an issue; it posts no verdict.
- A claim that a fix is unobservable is checked by reinstating the defect.
- The ownership recipe names the expected uid (13797) and adds
  `gh auth setup-git` after a rebuild.
- The line restating principle 3 is cut; the gh stack section is
  tightened in place. 281 -> 277 lines, 3208 -> 3158 words.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* docs(compass): cut AI_DEV_RULES.md to its rules

Applies the over-engineering review on this PR. Each rule is now stated
once:
- the need-human/escalation semantics, spread over five bullets, become one
- the "approval covers the head" rule, stated three times, is kept once
- review placement, the design-doc rule and the gh stack procedure are
  merged into one bullet each

The "Measured:" stories behind the rules are removed. So are the
arguments for choices nobody disputes, and the "specific time estimates"
clause, which contradicted the lines-of-code effort rule.

Kept, despite the review: the merge-tree landing procedure, as a short
sub-bullet rather than a new script, and the unlinked-chain restack line,
because stacking is still only recommended.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* docs(compass): answer review questions; add decomposition and ponytail-review rules

- Main-worktree bullet: say what the Compass scripts do with the local
  integration branch, not just a function name, and name the `fork`
  remote assumption.
- Landing: landing a stacked PR lands every unlanded PR below it, so each
  of those needs its own APPROVE covering its head, and no label.
- Tasks: decompose a complex task into sub-tasks, each its own issue.
- Gate 4: the reviewer also runs the ponytail-review skill.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* docs(compass): raise the decomposition threshold to ~1000 lines incl. tests

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: root <root@hjbog-srdc-39.amd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant