Skip to content

compass: revise gate 1 after measuring the baseline (P0.2) - #4

Merged
jgong5 merged 5 commits into
feature/atomcompass_newfrom
compass/p0.2-baselines
Sep 21, 2026
Merged

jgong5 merged 5 commits into
feature/atomcompass_newfrom
compass/p0.2-baselines

Conversation

@jgong5

@jgong5 jgong5 commented Sep 20, 2026 •

Copy link
Copy Markdown
Owner

Agent-authored. This PR description and the commits it covers were written by an AI agent under the jgong5/ATOM full-automation exception in CLAUDE.md.

P0.2 was supposed to be a no-op bookkeeping task: record the suite's pass/fail state
before the first Compass commit. It disproved a premise three documents share, and the
correction is this PR.

08 D43.1, ATOM's own CLAUDE.md, and the test gate now stated in AI_DEV_RULES.md all
say the 187-file suite under tests/ runs GPU-free and green. Measured on node 18, both
halves are false. What is GPU-free is a tier; the rest is a GPU superset judged as a
delta.

tier when scope bar
CPU every task 128 files green: 3956 passed, 0 failed, 149 skipped, 3 xfailed, rc=0, ~26–31 s
GPU superset every wave tests/ --ignore=tests/plugin delta vs 4730 passed / 5 failed, the five named

Docs-only; no code, no tests, no scripts. The scripts that mechanise this gate are #6.


Dev record

What was found

  1. The suite is not GPU-free. Of the 187 files, 30 are under tests/plugin/ (sglang
    and vllm, in neither image). Of the 157 that remain, 29 reach the driver — 28 at
    collection time via rocminfo, 1 (test_lmcache_offload_disk_integration.py) at run
    time via hipHostMalloc, after which it never exits. 128 files are left, and those are
    genuinely green.
  2. Green is not "exercised". Of the 128 handed to the CPU tier, 22 collect no test at
    all
    . test_prefix_cache_accuracy.py has no test function — it is an argparse script
    driving a live server on localhost:8000. test_kv_connector_scheduler.py and
    test_transfer_engine.py have been dead since ATOM Support (P/D) disaggregation on mooncake ROCm/ATOM#690 split kv_transfer_engine into
    moriio. All three are cited by 08 D43.1 as coverage Compass keeps. Filed as T80.
  3. The blind spot is not random. 51 of the 157 non-plugin files run nothing on the CPU
    tier: 29 excluded and 22 silent. They include all five test_eplb_module_*,
    test_cudagraph_capture_bounds, test_block_table_marshal, and the paged-index and
    prefix-cache files 03 D13 and 01 D6 lean on. A task touching those areas runs the GPU
    superset in its own gate, not at wave end.
  4. The first derivation measured the wrong thing, and the review caught it. Iterating
    whole-suite collection to a fixed point measures a property of the collection order,
    not of the files. Re-derived one pytest <file> per fresh process: the exclusion list
    drops 32 → 29, because test_dp_metadata, test_dp_sync_layout and test_forward_mode
    are CPU-green (31 passed in 0.18 s) and had been excluded on non-evidence. Two
    apparent "import defects" turned out to be rocminfo reached through a half-initialised
    aiter left by an earlier file — not defects in ATOM's mocks. One attributed cause had
    replaced four.
  5. Lint has never been clean. ruff check . gives 1003 errors / 640 fixable;
    black --check . is clean over 660 files. The bar is "no new ruff error, black stays
    clean".

What was decided that the design did not cover

  • Where the measurement lives after the process text moved. This branch amended
    16 D98 and its decision-log row. The base has since moved the agent procedure into
    AI_DEV_RULES.md and deleted both. Resolving the merge in favour of that move would have
    deleted the measurement with it. Decision: the rules stay where the owner put them, and
    the numbers become a section of 16 in their own right — "The measured test and lint
    baselines"
    , placed with Phase 0, which is the task that produced them. Nothing
    procedural came back, and the decision-log table did not come back at all.
  • Dangling decision numbers. D96, D101 and D102 no longer exist. Rather than cite
    nothing, the text now says what they said: the task record living outside the tree
    (AI_DEV_RULES.md), the GPU booking queue below, and the named escalation points. Two
    references elsewhere pointed at "16 D98" and now name the section.
  • Both halves of the census are kept, not merged. The per-file census and the batch
    regeneration answer different questions and disagree by exactly two files
    (test_postprocess_width.py, test_v4_checkpoint_slot_copy.py). Recording only the batch
    is how three CPU-green files stayed excluded. Recording only the census would produce an
    exclusion list that does not match the configuration whose green is claimed.
  • T80 is registered where the register lives. It was in 16's local table only, and
    16 itself declares 12_open_items.md authoritative. Its row is now in
    12_open_items.md too, with wording identical to the row on compass/p0.1-env-and-gates
    so the two branches do not diverge, and the count lines in 12 and README updated to
    match: 73 registered TODOs — T1–T72 and T80, of which 70 are open.
  • The 187-file denominator, fixed in two more places. 08 and CLAUDE.md carried the
    same defect this PR fixes in 16: a numerator measured over the non-plugin files, divided
    by 187. Both now state each count against the set it was measured over.

What surprised

  • The exclusion list is not stable under measurement technique. Same tree, same
    container, two derivations, two different answers — 32 and 29 — and the 32 looked exactly
    as solid as the 29 until it was asked a different question. Whole-suite collection reports
    errors and then interrupts, so which files appear depends on which files ran first.
  • One cause looked like four. The whole-suite pass produced 25 rocminfo, one
    KeyError: 'aiter', one AttributeError on gdn_attn, and three with no exception
    recorded. Run alone, every one of them is rocminfo or nothing at all.
  • test_prefix_cache_accuracy.py is cited by the design as coverage and contains no
    test.
    pytest reports no tests ran in both containers. It is coverage of nothing, and
    had been cited as evidence.

What was left undone

  • AI_DEV_RULES.md still says "187 files, no GPU needed". It is the same claim this PR
    disproves, now in the file the owner landed today. It is not edited here because this PR
    is scoped to design/ plus ATOM's CLAUDE.md, and AI_DEV_RULES.md is the owner's
    process file. Someone should correct that line; the measurement it needs is in 16.
  • 08 D43.1 is corrected, not rewritten. Several rows of its table are
    now known to be GPU-tier-only or dead. The full rewrite belongs to P0.1 (compass: worktree discipline, two-tier gate scripts and GPU pre-flight (P0.1) #6), which owns
    the gate scripts; this PR marks the rows and states the numbers.
  • The GPU superset here is P0.2's, at 83daf636d. P0.1 has since re-measured it at
    fe9ea043c (4779 passed / 5 failed) with the toolchain versions recorded and the failing
    node-ids on file. This PR's figure is the one P0.2 measured and is stated as such.
  • T80 is not actionable by Compass. It needs ATOM's disaggregation and prefix-cache
    owners.

Gate

Run on node 18 in container xiaobizh_n18_cpu, 2026-09-21, against git archive snapshots
with PYTHONPATH asserted (atom resolved under the snapshot root in both runs). This
branch carries no gate scripts, so scripts/compass/ from compass/p0.1-env-and-gates at
2c901cf04 was archived into each snapshot; it adds no tests, so the figure is the ATOM-only
half of that branch's 4022.

commit result
control — feature/atomcompass_new 4da2f3a2d 3956 passed, 149 skipped, 3 xfailed, GATE_CPU_RC=0, 31.02 s
this PR, merged bea0e6c8d 3956 passed, 149 skipped, 3 xfailed, GATE_CPU_RC=0, 30.63 s

Delta: none. The gate also reported gpu: not required for this diff — no changed file
matches a blind-spot trigger, which is expected for a docs-only change.

git diff --name-only feature/atomcompass_new...HEAD is five files: CLAUDE.md and four
under atom/compass/design/. Nothing else under atom/, nothing under tests/ or
scripts/.

Closes #13

root added 2 commits September 20, 2026 05:55
16 D98, 08 D43.1 and ATOM's own CLAUDE.md all state that the 187-file suite
runs GPU-free and green. Measured at 83daf63 on node 18, both halves are
false: 32 files reach the driver (28 of them at collection time, via
rocminfo), and tests/plugin needs sglang and vllm, which are in neither image.

Gate 1 becomes two tiers. Per task, a 125-file CPU gate that is genuinely
green -- 3925 passed, 0 failed, 33 seconds. Per wave, the GPU superset judged
as a delta against 4730 passed / 5 failed; the five are one bf16 ULP each and
pre-existing.

The CPU tier's blind spot is not random. The excluded files include all five
test_eplb_module_*, plus test_dp_metadata, test_dp_sync_layout,
test_cudagraph_capture_bounds and test_block_table_marshal -- most of what
Compass models. So a change touching those runs the GPU superset as part of
its own gate, not at wave end.

Lint baseline: ruff check . is 1003 errors, 640 fixable; black --check . is
clean over 660 files. The rule is therefore "no new ruff error and black stays
clean", never "ruff is clean", which it has never been.

The scripts that implement this gate land separately, in P0.1.
The Phase 0 task records and the T10 spike were committed under
atom/compass/tasks/ so 16 D96's "context is durable in the task record" had
somewhere to point. They are process artifacts, not deliverables, and they
make every PR diff carry hundreds of lines a reviewer has no reason to read.

They live locally instead. What a later reader actually needs -- the measured
numbers, the disproved premises, the options and why one was chosen -- is in
the commit messages and the PR descriptions, which is where someone looks
anyway.

The T10 spike is worth re-landing, but as a test rather than a record: the
P0.3 review asks for its claim 6 to become a whole-tree census with an exact
expected count, which turns it from a one-way existence check into a
regression guard. That belongs in tests/compass/ under W1.9.
@jgong5

jgong5 commented Sep 20, 2026

Copy link
Copy Markdown
Owner Author

Agent-authored. This review was produced by a reviewer agent under the
jgong5/ATOM full-automation exception in /md1/users/jgong5/CLAUDE.md. It was
run against commit 9c8df1328 in an isolated git archive snapshot at
/workspace/compass-9c8df1328/ATOM inside xiaobizh_n18_cpu on hjbog-srdc-18,
with import atom verified to resolve under that root before every command and
pytest's own exit code captured before any pipe. No GPU was used.

P0.2 review record

Reviewer: (agent) Commit: 9c8df13 Date: 2026-09-20
Verdict: CHANGES REQUESTED

Every headline number in the record that I could reach reproduced exactly. The
measurement is sound. What is not sound is the description built on top of it: the
exclusion list cannot be produced by the method the design doc says produced it, "125
files" overstates the gate's real coverage by 22 files, the blind spot is materially
larger and differently shaped than the four areas named, and the commit amends one of the
four documents that carry the claim it disproves.

What I executed

All pytest/lint in xiaobizh_n18_cpu on hjbog-srdc-18, at
/workspace/compass-9c8df1328/ATOM, import atom verified to resolve under that root
(/workspace/compass-9c8df1328/ATOM/atom/__init__.py) at the top of every script.
pytest's own $? captured before any pipe. Scripts:
/md1/users/jgong5/agent_scratch/compass_dev/reviews/{derive,gate,probe2,probe3}.sh.

Check Command Result
File counts find tests -name 'test_*.py' | wc -l; same for tests/plugin 187, 30 — as recorded
Exclusion list git show eb1d6d4fa:scripts/compass/cpu_gate_exclude.txt and 9120a96a3: same 32 entries, byte-identical in both P0.1 commits; all 32 exist; 0 under tests/plugin/
Arithmetic comm -23 of the 157 non-plugin files against the 32 187-30-32 = 125, included set = 125 files. The arithmetic holds; nothing double-counted, nothing missing.
CPU gate pytest tests/ --ignore=tests/plugin <32x --ignore> -q -p no:cacheprovider -rf 3925 passed, 149 skipped, 3 xfailed, PYTEST_RC=0, 25.8 s (31 s wall incl. import) — matches the record digit for digit
Independent derivation iterate --collect-only, feed back ^ERROR files, to fixed point converges at 28, not 32 (27 errors, then 1, then clean)
Full-suite collection pytest tests/ --collect-only 4225 tests collected, 37 errors in 8.75s, rc=2 — matches
The 4 extra exclusions each file alone test_dp_metadata 4 passed; test_dp_sync_layout 5 passed; test_forward_mode 22 passed; test_lmcache_offload_disk_integration 2 failed
Gate + 3 of those 4 29-file exclusion list 3956 passed, 149 skipped, 3 xfailed, rc=0, 27.1 s
Real coverage distinct files appearing in the gate's collect output 103, not 125
Lint ruff check .; black --check . 1003 errors / 640 fixable, rc=1; 660 files unchanged, rc=0 — matches. ruff 0.16.7, black 26.5.1, CPython 3.12.3
Rule breakdown ruff check . --statistics 60 distinct rules, top: UP045 381, BLE001 92, I001 74, RUF012 61, UP006 45
Provenance git branch --contains, git show --stat see F5

I did not run anything on a GPU.

Findings

# Severity Finding Evidence Suggested fix
F1 Major "32 files reach the driver" is false for 4 of them, and 3 of those cost nothing to keep. The measured collection fixed point is 28. test_dp_metadata.py (4), test_dp_sync_layout.py (5) and test_forward_mode.py (22) each pass standalone on CPU, and putting all three back gives 3956 passed, 0 failed, rc=0, 27.1 s — 31 more tests, still green, still inside the 33 s budget. The 4th, test_lmcache_offload_disk_integration.py, genuinely fails on CPU (2 failed) but for disk/LMCache reasons, not the driver. Two of these four are the very files the commit message cites as proof the blind spot is driver-forced. my derivation: iter1 rc=2 errorlines=27 / iter2 errorlines=1 / iter3 rc=0 → 28 files; diff of the committed list vs mine yields exactly those 4; 128-file gate run above Regenerate the list; it will be 28. Keep test_lmcache_offload_disk_integration.py excluded but record it under a second heading with its real reason. Re-measure the gate as 128 files / 3956.
F2 Major The committed exclusion list cannot be reproduced by the method the amended D98 claims produced it. D98 says the list is "derived by iterating collection to a fixed point rather than by hand, and it is regenerated — never edited." regen_cpu_gate_exclude.sh does exactly and only that (it greps ^ERROR from --collect-only), so running it today writes 28 lines. 4 entries were therefore hand-added, and the first regeneration a future developer performs will silently widen the gate by 31 tests and change 3925 → 3956, which will read as a broken baseline. git show eb1d6d4fa:scripts/compass/regen_cpu_gate_exclude.sh; my identical loop converged at 28 Either make the regenerator also run the suite and capture run-time failures (so 32 is reproducible), or drop the 3 green files. The list and the script must agree.
F3 Major "125 files" overstates the gate. 22 of the 125 contribute zero collected tests — module-level pytest.skip(..., allow_module_level=True) on torch.cuda.is_available(), or importorskip. Real coverage is 103 files. Worse, the 22 are not incidental: they include test_prefix_cache_accuracy.py and test_prefill_prefix_vs_native.py — two of the four items 08 D43.1 names as "the cheapest correctness evidence" for Compass — plus test_prefill_indices_composition.py, test_prefill_indices_paged.py, test_decode_indices_paged.py, test_sparse_attn_prefill_composition.py, test_compress_chunk_equivalence.py, test_kv_connector_scheduler.py, test_mla_index_cache.py, test_pool_index.py, test_swa_write_ring.py, test_postprocess_width.py. Prefill index composition and prefix-cache behaviour are core Compass modelling surface and are in the per-task gate in name only. comm -23 of the 125 included files against the 103 files appearing in --collect-only output; skip guards confirmed by grep in test_prefill_prefix_vs_native.py:27, test_pool_index.py:17, test_fused_compress_ragged.py:23 State the gate as "125 files, of which 103 contribute tests on CPU" and list the 22. Fold them into the blind-spot paragraph.
F4 Major The blind-spot list is incomplete and its mechanical trigger under-fires. D98 names EPLB / DP metadata / CUDA-graph capture bounds / block tables. The excluded 32 also contain test_forward_mode.py (22 tests — "ForwardMode.decide: the one place a step's shape is settled", i.e. prefill/decode/mixed classification), test_eplb_metadata.py (a sixth EPLB file the phrase "all five test_eplb_module_*" misses), test_dspark.py (63 tests, whose own docstring says it covers "the self-contained, GPU-free pieces"), test_decode_input_ids.py (8, per-request decode anchors/drafts), test_mtp_deferred_status_queue.py (4, MTP), test_merge_attn_states.py (10, the LSE merge behind MLA chunked prefill), test_dcp_{merge_ops,topk,sparse_filter}.py (56, context parallel), test_shared_expert_dispatch.py / test_balance_router_logits.py / test_mori_dispatch_trim_bound.py (MoE/EP dispatch). Combined with F3, a Compass change to step classification, chunked prefill, spec decode or prefix caching trips no trigger and is gated by tests that do not exercise it. wc/grep -c '^def test_' over the 32 files (listing in probe output); tests/test_forward_mode.py header; tests/test_dspark.py header Widen the named areas to at least: step-mode classification, chunked prefill / prefill index composition, prefix cache, spec decode / MTP / drafter, MoE-EP dispatch, DCP — in addition to the four already there.
F5 Major The mitigation is prose with no enforcement, and the enforcement point is the wrong one. "A task touching those runs the GPU superset in its own gate" is a sentence in a design doc; gate_cpu.sh does not read it, gate_gpu.sh is not invoked by it, and nothing inspects the diff. Given F4's widened surface this is the difference between a gate and an intention. git show eb1d6d4fa:scripts/compass/gate_cpu.sh — no path inspection, no call to gate_gpu.sh Add a checked-in path→area map and have gate_cpu.sh exit non-zero with "this diff touches ; run gate_gpu.sh" when git diff --name-only hits it. Cheap, and it is the only thing that makes the rule real.
F6 Major The commit disproves a claim carried by four documents and amends one. The commit message itself names 16 D98, 08 D43.1 and ATOM's CLAUDE.md. Only 16 is touched. Still asserting the disproved thing at this commit: CLAUDE.md:9 "python -m pytest tests/ # all tests (no GPU needed — mocks AITER and torch.cuda)"; 08_validation_protocol.md:23 "187 test files under tests/, and ATOM's own CLAUDE.md states they need no GPU"; design/README.md:591 "ATOM's own 187-file, GPU-free test suite is a merge gate on every Compass change, unmodified". And 15_parallelism_support.md:330 says "test_forward_mode.py already cover the pieces on the CPU-only path (08 D43.1)" — a dependency on a file the new gate excludes (see F1: it is CPU-green and should not be excluded at all). grep over the tree at 9c8df13 Amend 08 D43.1, design/README.md's D43.1 row and 15:330 in the same change, or add a one-line "superseded by 16 D98, 2026-09-20" to each. CLAUDE.md is ATOM's, so raise it separately rather than absorb it — but record that it is wrong.
F7 Major Both citations in the amended D98 point at things that do not exist at this commit. The paragraph ends "Record and raw output: agent_scratch/compass_dev/tasks/P0.2.md" — agent_scratch/ is not in the tree at all (it is git-ignored); the record actually committed is atom/compass/tasks/P0.2.md. And the gate bullet says "The exclusion list is scripts/compass/cpu_gate_exclude.txt" — scripts/compass/ does not exist at 9c8df13. A reviewer following either pointer finds nothing. ls agent_scratch/compass_dev/tasks/ → No such file; ls scripts/compass/ → No such file Fix the record path. For the exclusion list, say explicitly "lands in P0.1 at <sha>" rather than naming a path that is absent.
F8 Major The record's "Decision" names a P0.1 commit that is not in this lineage, and two P0.1 implementations exist. P0.2.md says "Implemented by P0.1 at 9120a96a3". 9120a96a3 is on compass/p0-foundations; 9c8df1328 is on compass/p0.2-baselines; they are siblings off 83daf636d, neither contains the other. The P0.1 commit that is actually a child of 9c8df13 is eb1d6d4fa on compass/p0.1-env-and-gates. The two carry byte-identical cpu_gate_exclude.txt but different file sets (9120a96a3 also edits 06 and 12; eb1d6d4fa also adds 00_handoff.md/P0.1.md). Per the project's own rule, a measurement that cannot name the code that produced it is not an observation — and here the record names code from a branch it is not on. git branch --contains 9c8df1328 → p0.1-env-and-gates, p0.2-baselines; --contains 9120a96a3 → p0-foundations; git log --graph --all Decide which P0.1 is canonical, cite that sha, and say what happens to the other. Until then the gate this commit defines has two implementations.
F9 Moderate "No new ruff error" is not checkable as stated. The recorded baseline is a single integer, 1003. A developer who deletes one UP045 and introduces one BLE001 leaves the total at 1003 and passes. A per-rule baseline was one command away and was not taken: ruff check . --statistics returns 60 rules (UP045 381, BLE001 92, I001 74, RUF012 61, UP006 45, RUF100 36, RUF059 35, UP035 30, S110 26, UP007 22, …). There is also no per-file baseline, so "new" cannot be localised to the diff either. probe3 section F Commit the --statistics table as the baseline and make the lint gate compare per-rule counts; or, cheaper and stricter, run ruff check $(git diff --name-only) and require zero on touched files.
F10 Moderate The 5-failure GPU baseline is not characterised well enough to survive a version bump. The record pins Python 3.12.3, pytest 9.0.3, ruff 0.16.7, black 26.5.1 — but no torch, AITER or ROCm version for the GPU container, which is the only thing the five bf16 failures depend on. 0.001953125 = 2^-9 is a good diagnosis, but "4730 passed / 5 failed" is a bare pair with no rebaselining procedure: after a torch bump, a reviewer seeing 4731/4 or 4728/7 has no rule for deciding whether that is a regression or drift. P0.2.md "What was measured" table; 16 D98 GPU bullet Record torch/AITER/ROCm/hip versions alongside the 4730/5, name the 5 failing node-ids verbatim in the design doc (not just the record), and state the rebaseline trigger: any change to those versions invalidates the pair and requires a fresh superset run recorded as a new baseline.
F11 Moderate The two tiers are not nested, so 3925 → 4730 cannot be decomposed by any reader. The 805-test gap mixes two unrelated effects: tests in the 32 excluded files, and tests that only exist when CUDA is available (the 22 module-skipped files of F3). Concretely, test_fused_compress_ragged.py — source of 4 of the 5 known GPU failures — is inside the CPU tier's 125 and contributes 0 tests there. So a file can be simultaneously "covered by the per-task gate" and "a known failure in the per-wave gate". Sanity check on plausibility: the 32 excluded files hold ~277 def test_ functions before parametrisation, so a few hundred parametrised tests, consistent with the gap — but that is a plausibility argument, not a reconciliation, and the record offers neither. CPU outcomes 3925+149+3 = 4077 vs GPU 4730+5+105+3 = 4843; test_fused_compress_ragged.py in both the zero-test list and the 5-failure list Record the GPU-container per-file test counts for the 32 excluded files once, so the delta is decomposable. Note explicitly that CPU skip counts (149) exceed GPU (105) because CUDA-guarded modules skip, not because coverage shrank.
F12 Minor The gate has no timeout and the regenerator only sees collection errors. gate_cpu.sh runs pytest with no wall-clock bound; regen_cpu_gate_exclude.sh detects only ^ERROR at collection. A future ATOM test that hangs, or that fails at run time, is invisible to the regenerator and will wedge or redden the gate with no recorded remedy. This is not hypothetical: test_lmcache_offload_disk_integration.py reports "2 failed in 5.25s" and then leaks a live python -m pytest child — one from my first probe was still running 871 s later under ps, and it held the pipe open long enough to look like a hang. ps -eo pid,stat,etimes in the container showing a 871 s-old orphan; probe2 returned rc=137 from timeout -s KILL 90 on a test that "finished" in 5 s Wrap the pytest call in timeout, and have the regenerator's stuck-detection cover non-collection failures too (it currently exits 93 only when rc≠0 with no new ERROR lines — which is precisely the hang case, and it will loop 15 times first).
F13 Minor 08 D43.1 cites test_prefix_cache_accuracy.py as Compass coverage. It is not a test: it is an argparse CLI load-generator that POSTs to http://localhost:8000 with requests and has no collectable test functions. P0.2's job was to measure what the suite actually is, and an aggregate pass count could not surface this; but it is exactly the kind of thing the baseline task existed to catch. head -30 tests/test_prefix_cache_accuracy.py Flag to the owner of 08; drop it from D43.1's table.

What I could not check, and why

  • The entire GPU superset baseline — 4730 passed / 5 failed at 83daf636d. I was instructed not to run it (node 18's GPUs are booked). I take it on the record's word. Nothing in this review verifies 4730, verifies 5, verifies that the five are the named ones, or verifies 0.001953125. The 0.001953125 = 2^-9 reasoning is internally consistent and the file names are plausible, but I did not observe any of it.
  • Whether the five failures are pre-existing / hardware-specific. Requires a GPU, and ideally a second node. Not attempted.
  • Whether the CPU-green files I want restored (F1) stay green in the GPU container. They collect and pass on CPU; I cannot confirm they do not fail on GPU. If they do, that is an argument for excluding them — but then the reason is "fails on GPU", not "reaches the driver", and the record would still be wrong.
  • Whether tests/plugin/ passes anywhere. Needs sglang + vllm; absent from the CPU image, and the record says absent from the GPU image too. Unverified by me.
  • The node-39 D-state claims in the escalation. Not reachable from here and not re-tested.
  • The 33 s figure under load. I measured 25.8 s of pytest (31 s wall) on an otherwise-idle container, twice. Contention was not modelled.
  • Whether counts differ on nodes 19/20. Only node 18 measured, same limitation the record states.

@jgong5
jgong5 changed the base branch from feature/atomcompass_new to feature/atomcompass September 20, 2026 07:45
@jgong5
jgong5 added this pull request to stack #7 September 20, 2026 07:45
@jgong5
jgong5 removed this pull request from stack #7 September 20, 2026 07:49
@jgong5
jgong5 changed the base branch from feature/atomcompass to feature/atomcompass_new September 20, 2026 07:49
@jgong5
jgong5 added this pull request to stack #8 September 20, 2026 07:49

@jgong5 jgong5 left a comment

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.

This review is agent-authored. It was produced by a Claude reviewer agent, not by a human.

Verdict: CHANGES REQUESTED — H1 and H2 are blocking; H3 is blocking unless the stack's merge order is guaranteed.

(GitHub refuses APPROVE/REQUEST_CHANGES on a PR authored by the same account, so this is
submitted as a COMMENT review. The verdict line above is the verdict.)


Review — PR #4 compass: revise gate 1 after measuring the baseline (P0.2)

head compass/p0.2-baselines (b963c94) · base feature/atomcompass_new (83daf63) · 2 commits · docs only
(1 file, +30 −7, all in atom/compass/design/16_execution_plan.md)

Reviewed against the eight design principles in atom/compass/design/README.md (lines 34–54).
Bottom of a stack; PR #6 (P0.1) sits on top. Claims this PR makes that P0.1 later revises are
not counted as defects. Claims that are already false at this commit are — and several are.

What I executed

All on node 18, container xiaobizh_n18_cpu, against /workspace/compass-9c8df1328/ATOM —
a snapshot of this PR's own first commit (9c8df1328), import atom verified to resolve
under that root, pytest's exit status captured before any pipe. No GPU tier was run.

run result
tests minus tests/plugin minus the 32-file P0.2 set 3925 passed, 0 failed, 149 skipped, 3 xfailed, rc=0, 25.8 s — reproduces the PR exactly
same, minus only 29 files (28 collection + 1 run-time) 3956 passed, 0 failed, 149 skipped, 3 xfailed, rc=0, 25.9 s
tests/test_dp_metadata.py tests/test_dp_sync_layout.py tests/test_forward_mode.py alone 31 passed in 0.18 s, no driver
tests/test_cudagraph_capture_bounds.py tests/test_block_table_marshal.py collection RuntimeError: Get GPU arch from rocminfo failed — genuinely driver-bound
whole-tests collection, CPU container 37 errors: 7 under tests/plugin, 30 elsewhere
ruff check . / black --check . 1003 errors, 640 fixable / clean, 660 files — both reproduce exactly

Scripts kept in /md1/users/jgong5/agent_scratch/compass_dev/review_p02/.


HIGH

H1. Three of the "32 driver-dependent files" are not driver-dependent, and two of them are named in the blind-spot paragraph — false at this commit

atom/compass/design/16_execution_plan.md lines 124, 128–129, 138–142 · principles 8, 7, 6 · executed

The doc asserts "32 files reach the driver", sets the per-task bar at "125 files, 3925 passed",
and then names test_dp_metadata, test_dp_sync_layout among "the areas Compass models most
closely" that the CPU tier cannot see — with the consequence that "a task touching EPLB, DP
metadata
, CUDA-graph capture bounds or block tables runs the GPU superset as part of its own gate".

Measured on this PR's own tree: test_dp_metadata.py, test_dp_sync_layout.py and
test_forward_mode.py give 31 passed in 0.18 s in the CPU container with no driver, and the
CPU tier at 29 exclusions is 3956 passed / 0 failed / rc=0. They appear as collection errors
only when collected alongside files that are themselves excluded — their ERROR lines carry no
exception at all, the signature of an import side-effect, not a rocminfo call.

Consequences, all live at this commit: the stated bar (3925) is 31 tests lower than the tier
actually delivers, so 31 ATOM tests are dropped from every task's gate for no measured reason; and
the trigger rule books a scarce GPU (against D101's whole premise) for any task touching DP
metadata, where the CPU tier already covers it.

Fix. Re-derive the exclusion set per file in isolation, not by whole-suite collection; drop
the three; restate the bar as 3956 passed, 0 failed, 149 skipped, 3 xfailed with the tree and
container named; delete test_dp_metadata / test_dp_sync_layout from the blind-spot list and from
the GPU trigger. (P0.1 reaches 29 exclusions and 3956 + 49 = 4005 — but by re-measuring later, not
because this commit was right.)

H2. "28 of them at collection time, via rocminfo" — one cause asserted for a set whose measured causes are four

line 124–125 · principles 7, 8 · executed

Measured at this tree, whole-tests collection in the CPU container, 30 non-plugin collection errors:

cause files
RuntimeError: Get GPU arch from rocminfo failed 25
KeyError: 'aiter' 1 — tests/test_dcp_topk.py
AttributeError: module 'atom.model_ops.attentions.gdn_attn' has no attribute … 1 — tests/test_kda_checkpoint_slot_copy.py
no exception recorded (collateral of collection order) 3 — the H1 files

Plus one file that errors only in the second iteration (tests/test_decode_input_ids.py, which
does not error in a first-round collection here) and one run-time failure
(test_lmcache_offload_disk_integration.py) = 32. So "28 at collection time" is wrong in count
(31 collect-time files by the PR's own 37→1→0 derivation) and wrong in mechanism (25 via rocminfo).

This is not pedantry: KeyError: 'aiter' and the gdn_attn AttributeError are mocking defects in
ATOM's test scaffolding
, not device access. Filed as "reaches the driver", they become permanently
invisible — exactly the silent-failure mode finding 3 of the design README warns about.

Fix. Publish the cause census with the count, not a single attributed mechanism — e.g.
"31 files fail collection: 25 via rocminfo, 2 via import defects in the AITER mocks (named), 3 by
collection-order collateral (not driver-bound, see H1), 1 only after the first batch is ignored".
The two import defects want an issue against ATOM, not an exclusion line.

H3. The gate names an artifact that does not exist in this commit or in its base

line 130–132 ("The exclusion list is scripts/compass/cpu_gate_exclude.txt … regenerated — never edited") · principle 8 · executed

git ls-tree -r compass/p0.2-baselines -- scripts/compass is empty; so is the base. git log --all --diff-filter=A puts the first appearance of cpu_gate_exclude.txt, gate_cpu.sh and
regen_cpu_gate_exclude.sh at eb1d6d4fa / 9120a96a3 — i.e. in PR #6, the PR stacked on top.

As the bottom of the stack this PR can merge alone, and if it does, 16 D98 becomes a normative gate
whose exclusion list, regenerator and runner are all absent, and whose "125 files / 3925 passed" no
reader can audit or reproduce from the tree. A number whose derivation is not in the repository is a
number without a source.

Fix. Either land the scripts with this PR, or state the list inline (32 paths, one reason each)
and mark the file path as "arrives in P0.1"; and make the merge order of the stack explicit so #4
never lands without #6.


MEDIUM

M1. 22 of the "125 files, green" collect zero tests — including the two files 08 D43.1 offers as Compass's prefix-cache coverage

line 128–129 · principles 7, 6 · executed

125 files are handed to pytest; only 103 contain a single collected test. The other 22 skip at
module level. Verbatim reasons include needs a real GPU (test_pool_index.py,
test_decode_indices_paged.py, test_prefill_prefix_vs_native.py) and
kv_transfer_engine was split into the moriio subpackage (#690); test imports need path updates by the disaggregation owner (test_kv_connector_scheduler.py, test_transfer_engine.py) — i.e. tests
that have been dead since an unrelated refactor.

The list is not neutral to Compass: test_kv_connector_scheduler.py, test_transfer_engine.py,
test_prefix_cache_accuracy.py, test_prefill_prefix_vs_native.py, test_pool_index.py,
test_prefill_indices_paged.py, test_decode_indices_paged.py, test_postprocess_width.py are the
KV-connector, prefix-cache and paged-index areas 03 D13 and 01 D6 lean on. 08 D43.1 (unchanged
by this PR) cites test_prefix_cache_accuracy.py and test_prefill_prefix_vs_native.py by name as
coverage Compass keeps; both run nothing on the tier this PR makes the per-task gate.

So the true blind spot is ≥54 of 187 files (32 excluded + 22 exercising nothing), and the doc
enumerates 9. Reporting "125 files, green" without that split is the aggregate-without-decomposition
failure principle 7 exists for.

Fix. Report the CPU tier as 125 handed / 103 exercised / 22 module-skipped, list the 22 with
their reasons, and fold the Compass-relevant ones into the blind-spot paragraph.

M2. The exclusion set is a property of collection order, not of files — and the doc states it as a file property

line 124, 131–132 · principle 8 · executed

tests/test_postprocess_width.py is inside the 125-file gate, yet collected on its own it fails
with RuntimeError: Get GPU arch from rocminfo failed. It stays quiet in the gate run only because
some earlier module has already mocked AITER. The converse (H1) also holds. "32 driver-dependent
files, derived by iterating collection to a fixed point" therefore describes the fixed point of
a particular collection order, which the PR body's own "converged 37 → 1 → 0" narrative confirms is
monotone — it only ever adds files, and never re-tests a file after its poisoner is removed.

Fix. Derive per file in isolation (pytest <one file> in a fresh process) and record, per file,
which of {collection error, run-time driver call, order-dependent} it is. That is the derivation that
supports the sentence the doc already wants to write.

M3. The durable artifact drops the decomposition the PR description carries

lines 128–129, 151–153 · principles 7, 8 · executed

The doc says "125 files, 3925 passed, 0 failed, rc=0, 33 s in the CPU container". No skipped
count, no xfailed count, no container name, no commit on that line, no python/pytest version.
Measured here: 3925 passed, 149 skipped, 3 xfailed — 149 skips is 3.7% of the suite silently not
run, and M1 shows 22 files hide inside it. The PR body does give 149 skipped, 3 xfailed and does
name the five GPU failures; the design document, which is what survives, gives neither. Also "33 s"
against 25.8 s of test time / 31 s wall here — harmless, but it is an unsourced number in a paragraph
whose whole point is sourcing.

Fix. One line, the 08 D43.1 form from P0.1 is the right template: counts + decomposition +
container + host + commit + the import-resolution and exit-status guards.

M4. The GPU delta gate is unfalsifiable as written

lines 133–137 · principles 7, 8 · read, plus the P0.2 record

"judged as a delta against the P0.2 baseline of 4730 passed / 5 failed at 83daf636d. The
five are one bf16 ULP each and pre-existing." The doc records neither the five node-ids nor the
torch / AITER / ROCm versions, so "5 failed" at wave end cannot be checked against "the same 5
failed" — a regression that swaps one failure for another passes this gate. The P0.2 task record
does name all five; it is out of tree (L1), and the doc is what a reviewer will read.

Fix. Put the five node-ids and the stack versions in the document beside the number. (P0.1 files
this as T77 and marks the baseline "stale at birth"; the gap is real at this commit.)

M5. This PR states that 08 D43.1 and ATOM's CLAUDE.md are measurably false, and leaves both saying it

16 line 123–125; 08_validation_protocol.md line 23–24; CLAUDE.md line 9 · principle 8 · executed

At this commit 08 D43.1 still reads "Verified on feature/atomcompass_new: 187 test files …
ATOM's own CLAUDE.md states they need no GPU", and CLAUDE.md still reads
"python -m pytest tests/ # all tests (no GPU needed — mocks AITER and torch.cuda)". The tree
therefore contradicts itself in two places the PR itself identifies by name. "Docs-only; no code"
is not a reason to fix one of three copies of a disproved claim.

Fix. One-line amendments to both with a pointer to D98, or a > superseded by 16 D98 (P0.2)
banner if the full rewrite belongs to P0.1.


LOW

  • L1. agent_scratch/compass_dev/tasks/P0.2.md (line 155) is not resolvable. Commit b963c94
    deleted the in-tree record (atom/compass/tasks/P0.2.md, −160 lines) and repointed the citation at
    a git-ignored directory that does not exist relative to the repo root — it resolves only under one
    developer's home on one host. The raw output behind 4730/5, 3925 and 1003 is then uncitable.
    Principle 8. Fix: cite the PR/commit that carries the numbers, or keep the measured table in the
    design document.
  • L2. "Green is the bar, because it is green" (line 130). With 149 skips and 22 files
    exercising nothing, "green" is a weaker statement than it reads as. Principle 7.
  • L3. Decision-log entry D98 (line 403) hard-codes "a green 125-file CPU gate" and "4730/5" with
    no tree or container anchor, in the one place in the document meant to be quotable. Both numbers
    are already wrong within this stack (129 files / 4005, and 4779/5). Fix: anchor or generalise the
    log line and keep the numbers in the body.
  • L4. tests/plugin excluded "needs sglang and vllm" (line 126) — true, but 3 of its 7
    CPU-container collection errors are rocminfo, not the missing packages. The stated reason is not
    the measured one for all 30 files.

Not findings

  • Supersession. The GPU baseline (4730/5 at 83daf636d) is re-measured to 4779/5 at fe9ea043c
    on the stacked PR, and the CPU figure becomes 4005 (3956 ATOM + 49 tests/compass) at 29
    exclusions. Per the review brief, supersession by the PR above is not a defect here — H1 and H2 are
    findings because they were already false at this commit, measured on this commit's tree.
  • 3925 itself is a real, reproducible measurement of the 32-exclusion configuration: reproduced to
    the digit. The defect is the configuration it measures (H1) and what the doc omits around it (M3).
  • The ruff/black baseline and the "no new ruff error" rule reproduce exactly (1003/640, 660 files
    clean) and the rule is correctly stated. No finding.
  • Severity band "critical": empty. Nothing here risks incorrect code or data loss; the PR is
    documentation, and its central move — replacing an asserted baseline with a measured one — is right.

Verdict

The premise of the PR is correct and worth landing. Three things should change before it does: the
exclusion set must be re-derived per file (H1/H2/M2 are one root cause — a monotone, order-dependent
derivation), the numbers must carry their decomposition in the document and not only in the PR body
(M1/M3/M4), and the gate must not name artifacts that arrive in a later PR (H3).

Review of PR #4 found three blocking defects in `16` D98 and two substantive
medium ones. All five are fixed by re-measuring, not by rewording. Every number
below was produced in this session; the commands and their output are at the
bottom of this message.

H1 — three of the "32 driver-dependent files" are not driver-dependent.
Re-derived the exclusion set per file, one `pytest <file>` per fresh process,
over all 157 non-plugin files. `test_dp_metadata.py`, `test_dp_sync_layout.py`
and `test_forward_mode.py` give 31 passed in 0.18 s with no driver. They were
excluded because they error when collected *alongside* files that are themselves
excluded, and their ERROR lines carry no exception at all. Dropped from the
exclusion set, from the blind-spot list and from the GPU trigger rule; the bar is
restated as the 29-exclusion tier.

H2 — "28 of them at collection time, via `rocminfo`" asserted one cause for a set
whose measured causes were four. Whole-suite collection at this tree gives 37
errors: 7 under `tests/plugin/`, 30 elsewhere, and the 30 report 25 `rocminfo`,
one `KeyError: 'aiter'`, one `AttributeError` on `gdn_attn`, and three with no
exception. The doc now publishes that census. Two findings of my own here:

  * The reviewer read the `KeyError` and the `AttributeError` as mocking defects
    in ATOM's scaffolding. They are not. Run alone, `test_dcp_topk.py` and
    `test_kda_checkpoint_slot_copy.py` both fail with `rocminfo` — the other two
    faces are second-order artefacts of a half-initialised `aiter` left by an
    earlier file. Per file, all 30 collection errors have one cause. No ATOM
    issue is warranted for those two.
  * The "28" was real: it is the `rocminfo` count over the *whole* suite,
    `tests/plugin/` included (25 non-plugin + 3 plugin). The defect is that it
    was then attached to a non-plugin set of 32. Both 25 and 28 are right at
    different denominators; the sentence joining them was not.

H3 — the gate named `scripts/compass/cpu_gate_exclude.txt`, which exists in
neither this commit nor its base. Resolved as the reviewer's second option: the
29 paths are stated inline, in the document, because a gate may not name a file
its own tree does not contain. The paragraph names P0.1 (PR #6, whose base branch
*is* this one, so it cannot land first) as where the list becomes a regenerated
artifact, and marks itself superseded from that commit. Verified the inline list
is byte-identical to the GENERATED section of P0.1's `cpu_gate_exclude.txt`.

M1 — 22 of the files handed to the gate collect zero tests. The doc now reports
the tier as 128 handed / 106 exercised / 22 silent, with the split: 16 declare a
device dependency, 3 need PyAV, 2 are dead since ATOM ROCm#690, and
`test_prefix_cache_accuracy.py` has no test function at all — it is an `argparse`
script that drives a live server on `localhost:8000`. Three files are covered by
*neither* tier, measured in the GPU container: that one (`no tests ran`),
`test_kv_connector_scheduler.py` and `test_transfer_engine.py` (`1 skipped`).
Filed as T73; two of the three are cited by `08` D43.1 as coverage Compass keeps.

M2 — the exclusion set was a property of collection order, stated as a property
of files. Both derivations are now in the document with the question each
answers, and the two files that go the other way are named:
`test_postprocess_width.py` and `test_v4_checkpoint_slot_copy.py` sit inside the
gate, module-skip in the batch, and fail collection with `rocminfo` alone.

M3 — the durable artifact now carries the decomposition the PR body had: counts,
skipped, xfailed, rc, tree, container, host, Python and pytest versions, and the
control run at the base commit.

M4 — the GPU delta gate now names its five failing node-ids verbatim and the
stack they were measured on, so "5 failed" can be checked against "the same 5".

M5 — `08` D43.1 and ATOM's `CLAUDE.md` both said the suite needs no GPU. This PR
proved that false and left both saying it. Both corrected here, minimally; the
full rewrite of D43.1 belongs to P0.1 and the banner says so.

L1 — the citation to `agent_scratch/compass_dev/tasks/P0.2.md` is replaced: that
path is git-ignored and resolves on one host only. The numbers are in the
document, and the paragraph says why.
L3 — the D98 decision-log line no longer hard-codes counts.
L4 — `tests/plugin/`'s stated reason is now the measured one: 7 of its 30 files
fail collection here and 3 of those 7 are `rocminfo`, not a missing package.

Not done: nothing in the reviewer's "Not findings" list was touched. L2 is
answered by the 106/22 split rather than by a separate sentence.

This branch is the base of PR #6, which rewrites the same three files. The
conflict that creates is textual: #6 supersedes each passage, so resolution is
"take #6's side" throughout.

---

MEASUREMENTS. Node 18 (hjbog-srdc-18), container `xiaobizh_n18_cpu` unless
stated, against `git archive` snapshots under `/root/p02fix-<sha>/ATOM` with
`PYTHONPATH` asserted to resolve `atom` under that root. Python 3.12.3,
pytest 9.0.3.

1. Per-file derivation, 157 non-plugin files, one fresh process each:

     for f in $(find tests -name 'test_*.py' | grep -v '^tests/plugin/' | sort); do
       timeout 600 python -m pytest "$f" -q -p no:cacheprovider --no-header -rs
     done

   30 collection errors, all 30 `RuntimeError: Get GPU arch from rocminfo failed`
   1  rc=124 — `tests/test_lmcache_offload_disk_integration.py`, `2 failed in
      10.47s` with `RuntimeError: hipHostMalloc failed: 100` twice, then the
      process never exits (killed at 600 s)
   20 rc=5, zero tests collected
   106 ran at least one test
   30 + 1 + 20 + 106 = 157.

   The 30 minus P0.1's 28 GENERATED entries = `tests/test_postprocess_width.py`
   and `tests/test_v4_checkpoint_slot_copy.py`.
   `test_dp_metadata.py`, `test_dp_sync_layout.py`, `test_forward_mode.py` are
   not in the 30:

     python -m pytest tests/test_dp_metadata.py tests/test_dp_sync_layout.py \
       tests/test_forward_mode.py -q -p no:cacheprovider --no-header
     31 passed in 0.18s

2. Whole-suite collection census:

     python -m pytest tests --collect-only -q -p no:cacheprovider

     total ERROR lines      : 37
     under tests/plugin     : 7
     non-plugin             : 30

   Of the 30: 25 `RuntimeError: Get GPU arch from rocminfo failed`; 1
   `KeyError: 'aiter'` (`tests/test_dcp_topk.py`); 1 `AttributeError: module
   'atom.model_ops.attentions.gdn_attn' has no attribute ...`
   (`tests/test_kda_checkpoint_slot_copy.py`); 3 with no exception recorded
   (`test_dp_metadata`, `test_dp_sync_layout`, `test_forward_mode`).
   Of the 7 plugin errors, 3 are `rocminfo`. 25 + 3 = 28.

   Alone, the two non-rocminfo faces resolve to rocminfo:
     tests_test_dcp_topk.py.log:8
       E subprocess.CalledProcessError: Command
         '['/opt/rocm-7.2.4/bin/rocminfo']' returned non-zero exit status 1.
     tests_test_kda_checkpoint_slot_copy.py.log:58
       E RuntimeError: Get GPU arch from rocminfo failed: ...

3. Batch census at the 29-file list:

     exclusions from cpu_gate_exclude.txt: 29
     files handed to pytest: 128
     files with >=1 collected test: 106
     files handed but collecting ZERO tests: 22

   The 22, with the reason pytest records:
     16 name a device (compress_chunk_equivalence, decode_indices_paged,
        fused_compress_ragged, gdn_midstep_state_gpu, indexer_cp_gather_ragged,
        indexer_topk_row_window, mla_index_cache, pool_index, postprocess_width,
        prefill_indices_composition, prefill_indices_paged,
        prefill_prefix_vs_native, sparse_attn_prefill_composition,
        swa_write_ring, upload_numpy, v4_checkpoint_slot_copy)
      3 PyAV (diffusion/test_h3_model, diffusion/test_h3_pipelines, and
        diffusion/test_attention, which inherits the skip by import)
      2 ATOM ROCm#690 (kv_connector_scheduler, transfer_engine)
      1 `tests/test_prefix_cache_accuracy.py` — no skip reason, because it has
        no test: `grep -c '^def test_' -> 0`; it is an argparse script whose
        `main()` posts to http://localhost:8000.

   149 skips = 68 distinct reasons; 66 skipped tests name a device, 83 do not.

4. Covered by neither tier — container `xiaobizh_n18` (GPU), at `83daf636d`:

     tests/test_prefix_cache_accuracy.py      no tests ran in 0.11s
     tests/test_kv_connector_scheduler.py     1 skipped in 0.11s
     tests/test_transfer_engine.py            1 skipped in 0.11s
     tests/test_prefill_prefix_vs_native.py   4 passed in 8.74s
     tests/diffusion/test_attention.py        1 skipped in 0.12s

5. GPU baseline node-ids, read from the original P0.2 run's captured output at
   `xiaobizh_n18:/workspace/compass-baseline-83daf636d/out/pytest_noplugin.txt`:

     FAILED tests/test_dcp_merge_ops.py::test_row_view_matches_output_slicing_bitwise
     FAILED tests/test_fused_compress_ragged.py::test_kernel_matches_reference_on_ragged_batches[extend0-context0-cut+whole]
     FAILED tests/test_fused_compress_ragged.py::test_kernel_matches_reference_on_ragged_batches[extend1-context1-whole+cut]
     FAILED tests/test_fused_compress_ragged.py::test_kernel_matches_reference_on_ragged_batches[extend2-context2-resume+fresh]
     FAILED tests/test_fused_compress_ragged.py::test_kernel_matches_reference_on_ragged_batches[extend4-context4-tiny-then-long]
     5 failed, 4730 passed, 105 skipped, 3 xfailed, 18 warnings in 137.76s

   Stack read from that same container on 2026-09-20: torch
   2.10.0+rocm7.2.4.git3d3aa833, HIP 7.2.53211, ROCm 7.2.4, aiter f4e7c7509,
   Python 3.12.3, pytest 9.0.3.

6. Lint baseline reproduced at `83daf636d`:

     ruff 0.16.7 / black, 26.5.1
     Found 1003 errors. [*] 640 fixable ...
     All done! 660 files would be left unchanged.

@jgong5 jgong5 left a comment

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.

This review is agent-authored. It was produced by a reviewer agent, run against the eight
design principles in atom/compass/design/README.md. Every number below was measured by the
agent in this session; the commands and containers are named beside each one.

verdict: ready to land

Scope: the delta only — commit d1d9846, 3 files, +161/−28. The parts of PR #4 that the first
review passed are not re-litigated. Both of the author's disputes are upheld on measurement:
the first review was wrong on one point of fact and imprecise on another, and the commit is right
on both. All three HIGH findings and all five MEDIUM findings are fixed by re-measurement, not by
rewording, and every number I re-ran reproduced — several to the digit.

Two new defects, neither blocking this PR. N1 is blocking for the stack: T73 — the item
that is the fix to M1 — is filed only in 16's local table, is absent from the consolidated
register 12_open_items.md that 16 itself declares authoritative, and is deleted by PR #6's
rewrite of 16. As the stack stands, the item exists for exactly one commit. N2 is a follow-up:
08 and CLAUDE.md attach a non-plugin numerator to the 187-file denominator, which is the same
defect this commit fixes in 16.

What I executed

Node 18 (hjbog-srdc-18), container xiaobizh_n18_cpu unless stated, against a git archive
snapshot of this commit (d1d9846) at /root/rv-p02fix/ATOM, PYTHONPATH asserted to
resolve atom under that root (/root/rv-p02fix/ATOM/atom/__init__.py), pytest's own exit
status captured before any pipe. Python 3.12.3, pytest 9.0.3.

git diff --name-status 83daf636d d1d9846 returns exactly three files — CLAUDE.md,
08_validation_protocol.md, 16_execution_plan.md — so tests/, scripts/ and all executable
code under atom/ are byte-identical to the base, and the commit's measurements, taken at
b963c941, transfer to the publishing commit. I re-took them at d1d9846 anyway; they hold.

run result
CPU gate: 157 non-plugin files minus the 29 inlined exclusions 128 handed · 3956 passed, 0 failed, 149 skipped, 3 xfailed · GATE_CPU_RC=0 · 27.7 s
--collect-only over the same 128 106 files collect ≥1 test, 22 collect none
whole-tests collection census 37 ERROR: 7 plugin, 30 non-plugin — 25 rocminfo, 1 KeyError: 'aiter', 1 AttributeError on gdn_attn, 3 with no exception
test_dcp_topk.py alone, fresh process RuntimeError: Get GPU arch from rocminfo failed (rc=2)
test_kda_checkpoint_slot_copy.py alone, fresh process RuntimeError: Get GPU arch from rocminfo failed (rc=2)
test_dp_metadata + test_dp_sync_layout + test_forward_mode 31 passed in 0.18s, rc=0; and rc=0 individually
test_postprocess_width.py, test_v4_checkpoint_slot_copy.py alone both rocminfo collection error, rc=2
full per-file derivation, 157 non-plugin files, one pytest <file> per fresh process 30 rocminfo collection errors · 1 timeout (test_lmcache_offload_disk_integration.py) · 20 collect zero · 106 run ≥1 test · 30+1+20+106 = 157. The 30 minus the 28 GENERATED = exactly test_postprocess_width.py + test_v4_checkpoint_slot_copy.py; the only exclusion entry that does not fail alone is the run-time one. Every cell of the doc's table reproduced (I capped the per-file timeout at 150 s rather than 600 s; same classification).
inline 29 vs P0.1 cpu_gate_exclude.txt GENERATED @ d78f3bb set-identical, 28 = 28, zero diff
GPU container xiaobizh_n18: the T73 three no tests ran / 1 skipped / 1 skipped — reproduced
GPU container: test_prefill_prefix_vs_native.py, diffusion/test_attention.py 4 passed in 8.99s / 1 skipped — reproduced
lint stack CPU container: ruff 0.16.7, black 26.5.1. GPU container: both command not found — the doc's new claim is true

Dispute 1 — upheld. The first review was wrong; no ATOM issue is warranted.

The first review called the KeyError: 'aiter' on test_dcp_topk.py and the AttributeError on
gdn_attn in test_kda_checkpoint_slot_copy.py "mocking defects in ATOM's test scaffolding, not
device access", and asked for an issue against ATOM. The author disputed that: run alone, both are
rocminfo.

Run alone, in a fresh process, at this commit:

$ python -m pytest tests/test_dcp_topk.py -q -p no:cacheprovider --no-header
/app/aiter-test/aiter/jit/utils/chip_info.py:36: in _detect_native
    raise RuntimeError(f"Get GPU arch from rocminfo failed: {e}") from e
E   RuntimeError: Get GPU arch from rocminfo failed: Command '['/opt/rocm-7.2.4/bin/rocminfo']'
    returned non-zero exit status 1.
ERROR tests/test_dcp_topk.py
1 error in 0.67s                                                          rc=2
$ python -m pytest tests/test_kda_checkpoint_slot_copy.py -q -p no:cacheprovider --no-header
E   subprocess.CalledProcessError: Command '['/opt/rocm-7.2.4/bin/rocminfo']' returned non-zero
    exit status 1.
E   RuntimeError: Get GPU arch from rocminfo failed: ...
ERROR tests/test_kda_checkpoint_slot_copy.py
1 error in 0.83s                                                          rc=2

Identical stack in both: aiter/jit/utils/chip_info.py:36 _detect_native → rocminfo → non-zero
exit. The KeyError: 'aiter' and the AttributeError appear only in the whole-suite batch,
and the author's explanation — a half-initialised aiter left behind by an earlier file, so the
second consumer sees a partially-populated namespace instead of the underlying rocminfo failure
— is consistent with everything I can see: both files are ordinary driver-bound files in
isolation, and the two exotic faces are downstream of the batch, not of ATOM's mocks.

The author is right, the original H2 sub-claim was wrong, and no ATOM issue should be filed for
those two files.
The commit's handling is also the right one under principle 6: it does not
quietly drop the earlier claim, it states what was measured and why the earlier reading was an
artefact.

Dispute 2 — upheld. The arithmetic is exact, and the fix states its denominator.

The author claims the original "28" was not a wrong number: it is the rocminfo count over the
whole suite including tests/plugin/ — 25 non-plugin + 3 plugin — and the real defect was
attaching a whole-suite count to a non-plugin set of 32.

Whole-tests collection at this commit, untruncated summary lines (COLUMNS=400):

total ERROR lines .................. 37
  under tests/plugin/ .............. 7
  elsewhere ........................ 30

rocminfo, non-plugin ............... 25
rocminfo, tests/plugin/ ............ 3   (test_gdn_target_verify_batched_equiv,
                                          test_rtpllm_forward_context_semantics,
                                          test_vllm_deepseek_v4_proxy_state_arena_layout)
                                   ----
rocminfo, whole suite .............. 28

25 + 3 = 28, exactly as claimed. Both the reviewer's 25 and the doc's 28 were right at
different denominators, and the joined sentence was the defect. The remaining 4 plugin errors
record no exception in the summary; their bodies show ModuleNotFoundError: No module named 'sglang' and an ImportError on fused_gdn_gating — which is what makes 16's new L4 sentence
("3 of those 7 are rocminfo, not a missing package") the measured statement rather than the
assumed one.

Principle 7 is satisfied in 16: the corrected passage names both denominators explicitly —
"the 28 counts rocminfo across the whole suite, tests/plugin/ included (25 + 3), and was
attached to a non-plugin set of 32". That is the decomposition the principle asks for.


Finding by finding

# claim verdict measurement
H1 3 files dropped from the exclusion set; bar restated at the 29-exclusion tier fixed test_dp_metadata + test_dp_sync_layout + test_forward_mode = 31 passed in 0.18s, rc=0 together and rc=0 each alone. Gate at 29 exclusions = 3956 passed, 0 failed, 149 skipped, 3 xfailed, rc=0. Both reproduce to the digit. The blind-spot paragraph and the GPU trigger rule no longer name them, and the doc says so explicitly ("booking a GPU for them spends the resource D101 protects").
H2 cause census published, not one attributed mechanism fixed 37 / 7 / 30 and the four faces (25 / 1 / 1 / 3) reproduce exactly. See Dispute 1 and Dispute 2.
H3 29 paths inlined; claimed byte-identical to P0.1's GENERATED section fixed; one wording overstatement The inlined block expands to 28 paths (test_eplb_module_{a,b,c,d,e}.py is brace shorthand) and is set-identical to the 28 entries between # BEGIN GENERATED and # END GENERATED in scripts/compass/cpu_gate_exclude.txt at d78f3bb — diff is empty. Plus test_lmcache_offload_disk_integration.py = 29, matching the file's MANUAL section. It is not byte-identical (two-column layout, brace shorthand); the commit message says byte-identical. Commit message only, not the doc.
M1 128 handed / 106 exercised / 22 silent, filed as T73 fixed 128 / 106 / 22 reproduce exactly. The 22-way split also reproduces: 16 device-dependent, 3 PyAV (diffusion/test_h3_model, diffusion/test_h3_pipelines, diffusion/test_attention — the last collects zero and emits no SKIPPED line, consistent with the author's "inherits the skip by import"), 2 ATOM ROCm#690, 1 with no test function. 16+3+2+1 = 22. grep -c '^def test_' tests/test_prefix_cache_accuracy.py → 0; argparse at line 11, BASE_URL = "http://localhost:8000" at line 19. T73 is filed in 16's open-items table — but only there; see N1.
M2 the two files going the other way are named fixed, and the author found one the review missed test_postprocess_width.py and test_v4_checkpoint_slot_copy.py both fail collection alone with rocminfo (rc=2), and both module-skip inside the gate with exactly the reasons quoted — model_runner imports aiter at module load, the V4 builder's module imports aiter at load. The doc keeps both derivations with the question each answers, which is the correct resolution, and closes the loop: that pair is the whole 20-vs-22 difference between the per-file and in-gate silent counts. Verified: 128 = 106 + 20 + 2.
M3 decomposition now in the durable artifact fixed The gate line now carries counts, skipped, xfailed, rc, tree, container, host, Python and pytest versions, and the exit-status guard. My rerun: 3956/0/149/3, rc=0, 27.7 s against the doc's 26.1 s — timing noise, and the doc names it as measured rather than asserting it.
M4 five node-ids and the stack named; prior run quoted, not re-run fixed — the provenance satisfies principle 8 See the note below.
M5 08 D43.1 and ATOM's CLAUDE.md corrected fixed, with a new defect introduced — see N1 Both now carry the corrected claim and a pointer to 16 D98.
L1 unresolvable agent_scratch/... citation removed fixed grep -n agent_scratch 16_execution_plan.md → no hits. Replaced by an explicit statement of why every number is inline: "a citation to either is not resolvable from a checkout, which is the same defect as naming a script that is not in the tree." That is the principle-8 answer, stated as a rule rather than patched once.
L2 "green is the bar, because it is green" answered Replaced by "Green is the bar, but green is not 'exercised'", followed by the 106/22 split. No separate sentence needed.
L3 D98 log line no longer hard-codes counts fixed The row now reads "green at the exclusion list stated in the body … Counts live in the body, with their tree and container, because they move with both." No 125, no 3925, no 4730/5 in the row.
L4 tests/plugin/'s stated reason is the measured one fixed 7 of 30 plugin files fail collection; 3 are rocminfo (node-ids above), 4 are sglang / fused_gdn_gating import failures. Exactly as written.

No stale numbers survive in 16: grep -n "4005\|4022\|3925\|125 files\|32 driver" returns
nothing outside the one quotation of the sentence being corrected. The 3956 figure is right for
this branch, which has no tests/compass; the P0.1 baseline of 4005 (and its later 4022) is a
different denominator and is correctly absent here.

M4 — quoting a prior run, judged

The author did not re-run the 138 s GPU suite and says so. I checked the artifact he quoted:

xiaobizh_n18:/workspace/compass-baseline-83daf636d/out/pytest_noplugin.txt   (28640 bytes)
...
FAILED tests/test_dcp_merge_ops.py::test_row_view_matches_output_slicing_bitwise
FAILED tests/test_fused_compress_ragged.py::test_kernel_matches_reference_on_ragged_batches[extend0-context0-cut+whole]
FAILED tests/test_fused_compress_ragged.py::test_kernel_matches_reference_on_ragged_batches[extend1-context1-whole+cut]
FAILED tests/test_fused_compress_ragged.py::test_kernel_matches_reference_on_ragged_batches[extend2-context2-resume+fresh]
FAILED tests/test_fused_compress_ragged.py::test_kernel_matches_reference_on_ragged_batches[extend4-context4-tiny-then-long]
5 failed, 4730 passed, 105 skipped, 3 xfailed, 18 warnings in 137.76s (0:02:17)

The five node-ids and the summary line in 16 are a verbatim transcription of that file. This
satisfies principle 8 and a fresh run is not required
, for three reasons. The defect M4 named
was that a bare count cannot be checked against "the same 5" — naming the node-ids fixes that,
and naming them from the run that produced the count is strictly better than naming them from a
different run. Re-running would produce a new baseline at a new time, which is a different
claim, not a confirmation of this one. And the commit is honest about the one place the provenance
is weaker: the stack versions were "read from that same container on 2026-09-20, because the run
itself did not record them" — a stated limitation, which is what principle 8 asks for, rather than
a version string presented as if it had been captured. P0.1 already files the standing weakness
(stale at birth, T77).

One caveat worth recording, not a finding: the quoted file lives in a container writable layer, not
in the tree, so it is one teardown.sh from gone. The commit handles this correctly by putting the
node-ids in the document rather than citing the path — the document is now the durable artifact
and the file is only corroboration. That is the same reasoning as L1, applied consistently.


New — introduced by this commit

N1 (blocking for the stack, not for this PR). T73 is filed in the one place that does not survive

16's own TODO table says: "This topic's items only. The consolidated register is
12_open_items.md."
T73 is added to 16's local table and nowhere else:

12_open_items.md:15   3. **TODO register** — T1–T72, per topic      <- still T72
12_open_items.md      per-topic table ends at T72; no T73
README.md:6           **72 open TODOs (T1–T72)**                    <- still 72
README.md:541         ... the missing-topic register; T1–T72; ...   <- still T72

So at this commit a reader who opens the register the design set declares authoritative cannot
find T73. That alone is a two-line fix.

The sharper problem is one commit up. PR #6 rewrites 16 wholesale, and its version of 16's
TODO table ends at T72
— T73 is not in it. PR #6 also touches 12_open_items.md, bumping the
header to T1–T77 and adding T77 — and still no T73. Verified:

$ git show d78f3bb:atom/compass/design/16_execution_plan.md | grep '^| T7'
| T71 | Add Wave 4+ detail as Phase 0 and T21 answers arrive ...
| T72 | Decide whether reviewer agents use ATOM's existing `review-pr` skill ...

$ git diff d1d9846 d78f3bb -- atom/compass/design/12_open_items.md
-3. **TODO register** — T1–T72, per topic
+3. **TODO register** — T1–T77, per topic
+| **T77** | Re-measure the GPU-tier baseline ...

As the stack stands, T73 exists for exactly one commit and is then deleted. The fix to M1 was
"filed as T73"; if it does not survive the PR stacked on top, it is not filed — and what
disappears with it is precisely the class of thing M1 was about: a test file with no test
function
presented by 08 D43.1 as prefix-cache coverage, and two tests dead since ATOM ROCm#690
that run in neither tier. Those are ATOM defects that nobody else is tracking, and the whole
point of raising them was that an unfiled blind spot is permanently invisible.

Fix: add T73 to 12_open_items.md and bump the two T1–T72 counts in README.md and the
one in 12; and carry T73 through PR #6's rewrite of 16. #4 can land as it is — the item is
readable at this commit — but the stack must not merge with T73 dropped.

N2 (follow-up). 08 and CLAUDE.md attach a non-plugin numerator to the 187-file denominator — the same defect this commit fixes in 16

atom/compass/design/08_validation_protocol.md:29

29 of the 187 reach the driver (28 at collection time via rocminfo, 1 at run time via
hipHostMalloc), 30 more under tests/plugin/ need sglang and vllm

CLAUDE.md:10

# NOT GPU-free: AITER and torch.cuda are mocked, but 29 of 187 files still reach the driver —
# 28 at collection time via rocminfo. The driver-free subset (128 files) is green

Measured at this commit: at the 187 denominator, at least 32 files reach the driver — the 29
non-plugin ones plus the 3 under tests/plugin/ that fail collection on rocminfo. 16 says so
itself, eleven lines below the passage that establishes the rule: "3 of those 7 are rocminfo,
not a missing package". So the tree now contains a count contradicted by its own companion
document, and the second clause of 08's sentence attributes all 30 plugin files to sglang/vllm,
which 16 L4 explicitly disproves.

CLAUDE.md adds a second, smaller one: "the driver-free subset (128 files)". Two of those 128 —
test_postprocess_width.py and test_v4_checkpoint_slot_copy.py — reach the driver when run
alone; this commit is the thing that measured it. The 128 are driver-free as a batch, which is a
weaker and correct claim. And the comment annotates python -m pytest tests/, the command that
actually produces 37 collection errors, not 29.

Why this is a follow-up and not a blocker: both passages cite 16 D98, 16 is correct and
carries the full decomposition, 08's passage is under an explicit "the full rewrite belongs to
P0.1" banner, and PR #6 rewrites both files. The fix is one clause in each — "29 of the 157
non-plugin files", and "driver-free as a batch". Worth noting that PR #6 does not currently fix
it: CLAUDE.md at d78f3bb still reads "29 of 188 files still reach the driver", so this will
survive the stack unless it is caught here.

N3 (nit). The control run at the base commit is tautological for this change

The identical run at the base commit 83daf636d gives the same four counts — which is what
makes a change gate-neutral by measurement rather than by assertion.

True, and the right method to state — but for this commit it carries no information: tests/
and every executable file under atom/ are byte-identical between d1d9846 and 83daf636d, so
the two runs execute the same bytes. As written a reader may take it as evidence about this change. One clause ("trivially so
here, since the commit is documentation only — stated because it is the rule for code tasks")
would close it.

N4 (nit). The skip classifier is stated without its rule

The 149 skips are 68 distinct reasons: 66 skipped tests name a device, 83 do not.

68 distinct reasons and 149 total reproduce exactly. The 66/83 split reproduces only under a
specific classifier: scoring on gpu|cuda|hip|rocm|device|driver gives 64 / 85; adding
aiter gives exactly 66 / 83. The two tests that move are test_postprocess_width.py
("model_runner imports aiter at module load") and test_v4_checkpoint_slot_copy.py ("the V4
builder's module imports aiter at load") — so the doc is counting "names aiter" as "names a
device", which is defensible and is in fact the more accurate reading. Principle 8 wants the rule
beside the number: "counting aiter as a device name" is four words.

N5 (nit, commit message only). "byte-identical" overstates a set comparison

The commit message says the inline list was verified "byte-identical to the GENERATED section of
P0.1's cpu_gate_exclude.txt". It is set-identical — the doc reflows it into two columns and uses
{a,b,c,d,e} brace shorthand. The claim that matters (same 28 paths) is true and I reproduced it
with a zero-length diff; the word is wrong. Nothing in the tree is affected.


The structural point — checked, and it holds

  • PR #6's base branch is compass/p0.2-baselines (gh pr view 6 --json baseRefName →
    "compass/p0.2-baselines"), and PR #4's head is exactly d1d9846. #6 cannot reach
    feature/atomcompass_new without #4's content going with it.
  • Supersession is written into the document, not only into the commit message —
    16_execution_plan.md:214: "from that commit the file is the source of truth, is regenerated
    rather than edited, and this paragraph is superseded." That is the thing H3 asked for: a
    normative gate that names an artifact its own tree does not contain is a gate no reader can
    audit, and inlining plus a dated supersession marker resolves it without inventing a merge-order
    convention nobody enforces.

What this commit got right that is worth saying

The five findings were fixed by going back to the machine, not by softening the sentences — and
two of the five came back with the reviewer corrected rather than the document. That is the
behaviour principle 8 exists to produce. Two things in particular:

  • Keeping both derivations. The obvious over-correction to M2 was to replace the batch
    derivation with the per-file one. The commit keeps both and states the question each answers —
    the gate is run as a batch, so the exclusion list must be derived under a batch; the per-file
    census is what tells you whether a file is genuinely driver-bound. Then it closes the loop by
    showing the two answers differ by exactly one pair of files, in each direction. That is
    principle 7 applied to a derivation rather than to a number.
  • L1's fix is a rule, not a patch. "Every number in this decision is stated here rather than
    cited … a citation to either is not resolvable from a checkout, which is the same defect as
    naming a script that is not in the tree." One sentence that makes H3 and L1 the same finding and
    prevents the next one.

Nothing here blocks PR #4. Two amendments should land before the stack merges, because both
outlive this PR: N1 — register T73 in 12_open_items.md and keep it through #6's rewrite of
16 — and N2 — one clause in 08 and one in CLAUDE.md to put the 29 back on its own
denominator.

Three P0 branches each opened a different item at T73 against a base
(83daf63) whose register ends at T72:

  591e122  PR #5,  compass/p0.3-t10-scheduler-seam  plugin entry point (+T74-T76)
  d1d9846  PR #4,  this branch                      the three uncovered ATOM tests
  1321371  PR #10, compass/p0.4-t5-trace            symbolic D18 capture

Allocation decided by the task owner: earliest claimant keeps the number, the
other two move to the end of the namespace. PR #5 keeps T73; this branch's item
becomes T80; PR #10's becomes T81 on its own branch. T15 and T48 are retired
gaps in the base and are not reused.

Markdown only, and both occurrences move together so the item keeps its pointer:

  16_execution_plan.md:240  "...as coverage Compass keeps. See T73." -> T80
  16_execution_plan.md:524  the topic-04 register row                -> T80

Measured before and after: `grep -rn 'T73' .` over the whole tree returned
exactly those two lines beforehand and nothing afterwards; `grep -rno
'T73[A-Za-z0-9]*'` shows no T73a/T73b/T730 variant on this branch, and no T80
existed anywhere before this commit.

Not changed, and stated rather than fixed. README.md:6, README.md:541 and
12_open_items.md:15 still describe the register as "T1-T72". They were already
stale at d1d9846 -- that commit added a T73 without widening the range -- and
this rename leaves them stale in exactly the same way, not more so. That range
line is one line that four P0 branches touch; widening it here would collide
with the same edit on each of them, so it belongs to whoever consolidates them.

Gate 1, CPU tier, container xiaobizh_n18_cpu on hjbog-srdc-18, run against a
`git archive` snapshot (never rsync) with P0.1's d78f3bb scripts/compass
overlaid, because this branch carries no gate tooling. Control at d1d9846,
this commit's parent, twice: 3956 passed, 149 skipped, 3 xfailed, rc=0,
GATE_CPU_RC=0 in 36.43 s and 27.60 s. The same gate on this commit is reported
with the push. No GPU tier: the diff names no path in gpu_gate_triggers.txt,
and the gate says so itself (`gpu: not required`).

Agent-authored (Claude Opus 5), from an instruction that fixed the allocation
above; the agent did not choose which item moved.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jgong5 pushed a commit that referenced this pull request Sep 20, 2026
Three P0 branches each opened a different item at T73 against a base
(83daf63) whose register ends at T72:

  591e122  PR #5,  compass/p0.3-t10-scheduler-seam  plugin entry point (+T74-T76)
  d1d9846  PR #4,  compass/p0.2-baselines           the three uncovered ATOM tests
  1321371  PR #10, this branch                      symbolic D18 capture

Allocation decided by the task owner: earliest claimant keeps the number, the
other two move to the end of the namespace. PR #5 keeps T73; PR #4's item
becomes T80 on its own branch; this branch's becomes T81. T15 and T48 are
retired gaps in the base and are not reused.

Four occurrences, all moved together so the item keeps its pointers -- two of
them are in code, not design text, and a docs-only rename would have left the
refusal message pointing at another branch's item:

  12_open_items.md:86      the topic-04 register row                    -> T81
  12_open_items.md:250     the `04` pending-amendment row for D18        -> T81
  capture/fake_trace.py:21   module docstring, the concrete-capture note -> T81
  capture/fake_trace.py:358  capture()'s refusal text, "...or fix the
                             trace (T73)."                               -> T81

Measured before and after: `grep -rn 'T73' .` over the whole tree returned
exactly those four lines beforehand and, afterwards, nothing tracked -- the one
remaining hit is a stale, gitignored `__pycache__/fake_trace.cpython-312.pyc`
from an earlier local run, and `git grep T73` is empty. No T73a/T73b/T730
variant exists on this branch, and no T81 existed anywhere before this commit.
No test asserts on the refusal string, so the gate cannot see this change.

Not changed, and stated rather than fixed. README.md:6, README.md:541 and
12_open_items.md:15 still describe the register as "T1-T72". They were already
stale at 1321371 -- that commit added a T73 without widening the range -- and
this rename leaves them stale in exactly the same way, not more so. That range
line is one line that four P0 branches touch; widening it here would collide
with the same edit on each of them, so it belongs to whoever consolidates them.

Gate 1, CPU tier, container xiaobizh_n18_cpu on hjbog-srdc-18, run against a
`git archive` snapshot (never rsync) with P0.1's d78f3bb scripts/compass
overlaid, because this branch carries only its own t5_*.py there. Control at
1321371, this commit's parent: 3969 passed, 149 skipped, 3 xfailed, rc=0,
GATE_CPU_RC=0 in 30.84 s. The same gate on this commit is reported with the
push. No GPU tier: the diff names no path in gpu_gate_triggers.txt, and the
gate says so itself (`gpu: not required`).

Agent-authored (Claude Opus 5), from an instruction that fixed the allocation
above; the agent did not choose which item moved.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jgong5 pushed a commit that referenced this pull request Sep 20, 2026
… and the TODO register

Agent-authored. Fixes the delta re-review of PR #6 (F1, F2, F3-F6 and nits) plus
N1/N2 from the delta re-review of PR #4. No behaviour change to any gate verdict;
no changed path matches a GPU trigger, so the GPU tier was not re-run.

F1. Four sites said `tests/test_mla_index_cache.py` imports ModelRunner "at
module level" and that the collection probe is what keeps
atom/model_engine/model_runner.py. Both halves are false, measured at 236abfd
in xiaobizh_n18_cpu:

  - the import is at lines 99-100, indented four spaces inside a test function;
  - the collection probe removes NO path from this tree -- 30 triggers with it,
    30 without, difference empty. It withholds 15 coverage paths, and the only
    one that is also a candidate (atom/model_ops/v4_kernels/state_writes.py)
    is absorbed either way by the candidate subtree entry above it.

The 2x2: dec2 on/dec3 on = 30 (shipped); dec2 off/dec3 on = 30; dec2 on/dec3
off = 29 (loses topK.py); dec2 off/dec3 off = 27 (loses model_runner.py,
aiter_mla.py, topK.py). So the module-level-only half is what keeps both
counter-examples; the collection probe is a forward guard that currently
changes nothing. All four sites now say so with the measurement attached, and
the guard's cost is stated rather than assumed away.

F2. f3f584e cited agent_scratch/compass_dev/tasks/P0.2.md as the record for
the 4779/5 re-measurement. That file contains neither 4779 nor fe9ea04, and
is not in the tree at all. The citation now points at the in-tree record --
scripts/compass/gpu_gate_known_failures.txt and the BASE_* block at
gate_gpu.sh:71-97 -- and states the measurement inline. The duplicated lines in
16 and in 08 are removed.

F3. preflight.sh check 0 handed /proc/[0-9]*/stat to awk as a glob. mawk 1.3.4
treats an unopenable input as fatal, so a process exiting between glob
expansion and read would kill END, empty CENSUS, and abort a booked GPU run.
Replaced with a shell read loop: no fork, first line only, vanished entry
skipped. Latent, not observed (0 in 60 censuses); removed because it is cheap.

F4. The BASE_COMPASS_TESTS equality compares a collected count to a pass count.
Kept, with the limitation stated: they agree only while tests/compass/ has no
skip and no xfail, and that cause is now named in the mismatch text so a
negative "unaccounted" is diagnosable.

F5. gate_gpu.sh's header claimed EVERY unanswerable question refuses. Toolchain
drift, including an AITER version reading UNKNOWN, warns and exits 0. Header
corrected and the gap named, with the cost of closing it, rather than changing
behaviour that could not be re-verified without a GPU run.

F6. The generated trigger header no longer hard-codes "4022 passed + 128
skipped + 3 xfailed" or "21 of the 22 files": the indented-import count is
derived (N_IND) and the skip explanation is derived from N_DEAD, with
gate_cpu.sh named as the only source for run outcomes.

Nits. The -r refusal comment now says the -rfE-after-"$@" ordering is the
load-bearing fix and that -qrE slips past the loop; the dead -n test at the
error-id print is removed; the exact-string/subtree over-firing mechanism is
stated in all three places that describe the rule's error directions; and the
CPU gate's clock is labelled wall (`time` real). That range is widened to
27.4-40.9 s over three runs: the control run for this commit took 40.9 s on the
same node and container for the same 4022 tests.

N1. The TODO register range. T73 is claimed by three branches for three
unrelated items; the allocation is the owner's, and PR #4's and PR #10's rows
are being renumbered on their own branches. This branch carries T1-T72 and T77
and nothing else, so a contiguous "T1-T77" would assert T73-T76 that are not
here. 12_open_items.md:15, README.md:6 and README.md:541 now all say
"T1-T72 and T77", with the non-contiguity and its cause stated once.
README.md:6 also stops calling 72 an open count: 73 are registered, 69 open
(T15, T22, T48 struck through as done; T77 closed by P0.1).

N2. 08:29 and CLAUDE.md:10 attached a non-plugin numerator to a whole-suite
denominator. Both denominators are now explicit: 29 of the 158 files outside
tests/plugin/, and at least 32 of 188 once tests/plugin/'s own driver reaches
are counted. Measured at 236abfd in xiaobizh_n18_cpu: collecting tests/plugin/
alone gives `153 tests collected, 7 errors in 1.45s`, rc=2, decomposing as 3
rocminfo, 1 ModuleNotFoundError: sglang, and 3 ImportError on a module the
first three left half-initialised. CLAUDE.md's "driver-free subset" is now
"driver-free AS A BATCH": test_postprocess_width.py and
test_v4_checkpoint_slot_copy.py are among the 129 handed and each reaches
rocminfo run alone (`no tests collected, 1 error in 0.78s`, rc=2).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jgong5 pushed a commit that referenced this pull request Sep 20, 2026
…ed figures

Merges compass/p0.2-baselines (PR #4) into compass/p0.1-env-and-gates (PR #6).
PR #6 is based on PR #4 at b963c94; PR #4 has since added two commits --
d1d9846 (re-derive the CPU tier per file) and 4078bb8 (renumber T73 -> T80)
-- which rewrite the three files PR #6 also rewrote. Three content conflicts,
all markdown; no executable file conflicted.

Resolution rule, from the task owner: where the two sides disagree about a
MEASUREMENT, PR #6's side wins, because PR #6 re-measured what PR #4 could not
reproduce as recorded. Where they disagree about anything else, both intents
survive. Superseding is not deleting: no surviving text states a retired figure
as current.

CLAUDE.md -- took PR #6's side. Its numbers are the re-measured ones (29 of the
158 non-plugin files, at least 32 of 188; gate_cpu.sh 129 files / 4022 passed),
and it names scripts/compass/gate_cpu.sh, a file that is in the tree, rather
than a design document and a decision number, which the comment ban excludes.
Kept PR #4's provenance intent by stating the date and container, and said in
one clause that the earlier 128-file / 3956 reading is superseded.

08_validation_protocol.md D43.1 -- took PR #6's paragraph whole. Three of PR
#4's table caveats were findings PR #6 never had, because PR #6 branched before
them, so they are folded into the tier table rather than dropped:
test_prefix_cache_accuracy.py and test_kv_connector_scheduler.py move from
"CPU" to "neither" and test_prefill_prefix_vs_native.py to "GPU only", each
with the measurement that says so. Two of those rows were cited as coverage
Compass keeps and are not. PR #4's fourth caveat is already carried by PR #6
(block_table_marshal is GPU-only in the table; dp_metadata and dp_sync_layout
are CPU-green in the five-totals paragraph) and is not repeated.

16_execution_plan.md D98 -- took PR #6's side at all three conflicts, and
spliced back the PR #4 content PR #6 does not carry:
  * the split of the 22 silent files (16 device, 3 PyAV, 2 dead since ATOM
    ROCm#690, 1 with no test function) and the 149-skip census (68 distinct
    reasons; 66 name a device, 83 do not). PR #6 states the 22 but not its
    parts. Restated against PR #6's 129-file gate, with the earlier 128-file
    count marked superseded.
  * the three files covered by neither tier, and the T80 pointer, which the
    topic register already carried after the auto-merge.
  * ruff 0.16.7 / black 26.5.1, and that the GPU container has neither, so
    lint and the GPU superset cannot be run in one place.
Dropped as superseded by their own authors: PR #4's inline 29-path exclusion
list, which its own text says is superseded from the commit that adds
scripts/compass/cpu_gate_exclude.txt, and PR #4's prose blind-spot list, which
PR #6 replaced with the generated gpu_gate_triggers.txt on the stated ground
that a prose list is one nothing reads.

Not a conflict, but made stale by the merge: the register range lines. The
merge brings T80 into the tree, so 12_open_items.md and design/README.md now
read T1-T72, T77 and T80, 74 items, 70 open, and 12_open_items.md gains the T80
row under topic 16 so the consolidated register actually lists it. PR #4 left
these lines alone on the ground that consolidating them belonged to whoever
merged the branches; this is that merge, for this one item.

Audited on the merge result. `git grep 4730` returns five hits, every one
reading as retired: the T77 closure record, three supersession sentences in
D98, and the arithmetic in gate_gpu.sh that accounts for 4779 as 4730 measured
earlier plus the 49 tests that did not exist then. BASE_PASSED is 4779. `git
grep 83daf63` returns hits that are either the integration base -- which it
still is -- or a lint or per-file reading that names its tree. The design-doc
citation grep returns zero over every Compass file outside atom/compass/design/.

Agent-authored (Claude Opus 5), from an instruction that fixed the resolution
rule above; the agent did not choose which side wins a measurement.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jgong5 pushed a commit that referenced this pull request Sep 20, 2026
…tay true

The merge resolution took PR #6's D98 decision-log row, which bakes the counts
in, over PR #4's, which kept them out on the stated ground that they move. PR
#4's principle is right and this wave has already proved it: the CPU total read
3925, then 3956, then 4005, then 4022, all inside Phase 0, and the GPU baseline
moves with every test tests/compass/ gains. A baked count in a decision log goes
stale while still reading as authoritative.

Keeping the counts, because a log row a reader cannot use is no better, and
naming the body as the authority so a mismatch is actionable rather than a coin
toss: the body states every figure with the tree and container it was measured
in, so it can be checked; this row cannot. One clause appended, row otherwise
unchanged.

Gate, CPU tier, on this commit: reported with the push.

Agent-authored (Claude Opus 5), from a ruling that fixed the direction above.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The base moved the agent procedure out of 16_execution_plan.md and into
AI_DEV_RULES.md, deleting the D95-D99 sections and the decision log that this
branch had amended. The conflict is resolved in favour of that move: none of the
process prose comes back, and neither does the decision-log table.

What this branch measured does come back, as a section of its own -- "The
measured test and lint baselines", placed with Phase 0, which is the task that
produced it. It is the derivation, the CPU-tier counts and exclusion list, the
named GPU-superset failures, the blind-spot analysis and the ruff/black
baseline, with the gate wording and the "needing to edit an ATOM test" rule left
where the owner put them. References to numbered decisions that no longer exist
are replaced by what they said: D96 by AI_DEV_RULES.md, D101 by the booking
queue below it, and "gate 1" by the test gate.

Two references elsewhere pointed at "16 D98", which the merge deleted; both now
name the section instead.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jgong5
jgong5 marked this pull request as ready for review September 21, 2026 11:31
@jgong5
jgong5 removed this pull request from stack #8 September 21, 2026 11:34
@jgong5
jgong5 merged commit 947d5b2 into feature/atomcompass_new Sep 21, 2026
jgong5 added a commit that referenced this pull request Sep 21, 2026
PR #4 landed after this branch last merged and rewrote the TODO-register
line in 12_open_items.md, so the merge conflicts there.

Conflict resolution, round-2 review B1/B2:

- 12_open_items.md register line: keep base's sentence shape (count, range,
  non-contiguity disclosure, struck-item list) rather than the branch's bare
  range. Post-merge the register holds 76 items, T1-T72 and T78-T81, of
  which 73 are open. Re-derived from the register tables in section 3: 76
  unique T-ids, 3 struck through (T15, T22, T48).
- README.md:6 and :542 move with it. They are the index of record for the
  design set and the diff cannot show them, since this PR does not otherwise
  touch that file.
- T81 opened against 15's D91 Q2: scheduler.py:1761-1763 skips seqs in
  _pp_inflight_token_block inside the decode admission loop, so the head's
  batch composition depends on pipeline in-flight state. The decision text
  is left unamended; this registers the contradiction.
- T78 widened to both instances of the defect (qwen3_5.py:427 and
  glm4_moe.py:426), corrected to compilation level >= 2, and the
  IntermediateTensors handling range corrected to :533-537.
- T79 carries its invocation, its node/container and its log lines.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jgong5 added a commit that referenced this pull request Sep 21, 2026
Brings in AI_DEV_RULES.md, the de-duplication of 16_execution_plan.md against
it, and PR #4's measured test-and-lint baseline.

Three conflicts, all resolved in favour of the integration branch's structure
with this branch's content folded in:

16_execution_plan.md
  The integration branch deleted D95-D99 -- the operating model, the task
  record, where work lands, what gates a task, and effort in lines of code --
  because AI_DEV_RULES.md now owns that text, and renamed "D100. The module
  layout" to an unnumbered heading. Took theirs whole. This branch's only edit
  inside that block was one parenthetical under D99 saying W1.9's component
  total is reopened; the same statement survives in three other places (the 06
  component table, 12's T10 row, and W1.9's own estimate cell), so nothing was
  lost with D99.

README.md
  The integration branch corrects the decision count to 108, D0-D94, from the
  116 / D0-D102 this branch inherited -- D95-D103 left the design set with the
  process text. Took theirs, then applied this branch's own two changes on top:
  the sub-decision count goes 13 -> 14 for D34.1, and the decision map's 06 row
  gains "(+ D34.1)". The TODO clause is re-derived below.

12_open_items.md
  Both sides added rows to the topic-16 table: T73-T76 here, T80 there. Kept
  all five, in numeric order -- T74 had been sitting after T76.

The T-register was then re-counted against the file rather than against either
side's arithmetic. The register section holds 77 rows: T1-T76 contiguous, plus
T80. Four are struck through as done -- T10 (this PR), T15, T22, T48 -- so 73
are open. README's header and 12's "how to read it" both said 73 registered /
70 open, which was true before T73-T76 arrived and is not now; both are
updated. T77 stays on the P0.1 branch and is described the same way here as
there.

Agent-authored.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jgong5 pushed a commit that referenced this pull request Sep 21, 2026
…t (P0.1)

Adds scripts/compass/ -- the two gate scripts, the GPU pre-flight, the two
generators and their generated lists -- plus the CPU-only test that keeps the
exclusion list and the trigger list honest.

  gate_cpu.sh      tests/ minus tests/plugin/ minus cpu_gate_exclude.txt.
                   129 files, 4022 passed, 0 failed, rc=0. Refuses rather than
                   reporting "not required" when it cannot compute the diff, and
                   exits 98 when a changed path is in the GPU tier's blind spot
                   without a matching COMPASS_GPU_GATE_DONE attestation.
  gate_gpu.sh      tests/ --ignore=tests/plugin, judged as a delta against
                   4779 passed / 5 failed / 0 errors at fe9ea04, with the five
                   known failures compared by name and the expectation re-derived
                   per tree from this tree's own tests/compass count.
  preflight.sh     four checks -- D-state census, wedge, compute, VRAM -- run
                   before and after, each reporting the scope it measured.
  snapshot.sh      git archive, never rsync, and it refuses a dirty tree.

Restacked onto feature/atomcompass_new after PR #4 landed as 947d5b2. The
P0.2 measurements that arrived with it are kept where they landed; this branch
carries only what supersedes them, and states the tree and container for each.

The execution plan and the validation protocol are updated to match: the CPU
tier's exclusion list and blind spot are now files in the tree rather than prose,
the GPU baseline carries its full toolchain, and the pre-flight's D-state census
states its scope because in a container it cannot see the node.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jgong5 pushed a commit that referenced this pull request Sep 21, 2026
The rule read "ATOM's test suite passes unmodified -- 187 files, no GPU
needed". The measurement that landed with PR #4 shows 29 of the 157 non-plugin
files reach the GPU driver, so the suite is not GPU-free and only a tier of it
is. Corrected to the two tiers, pointing at that measurement; nothing else in
the document is touched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jgong5 pushed a commit that referenced this pull request Sep 21, 2026
…t (P0.1)

Adds scripts/compass/ -- the two gate scripts, the GPU pre-flight, the two
generators and their generated lists -- plus the CPU-only test that keeps the
exclusion list and the trigger list honest.

  gate_cpu.sh      tests/ minus tests/plugin/ minus cpu_gate_exclude.txt.
                   129 files, 4022 passed, 0 failed, rc=0. Refuses rather than
                   reporting "not required" when it cannot compute the diff, and
                   exits 98 when a changed path is in the GPU tier's blind spot
                   without a matching COMPASS_GPU_GATE_DONE attestation.
  gate_gpu.sh      tests/ --ignore=tests/plugin, judged as a delta against
                   4779 passed / 5 failed / 0 errors at fe9ea04, with the five
                   known failures compared by name and the expectation re-derived
                   per tree from this tree's own tests/compass count.
  preflight.sh     four checks -- D-state census, wedge, compute, VRAM -- run
                   before and after, each reporting the scope it measured.
  snapshot.sh      git archive, never rsync, and it refuses a dirty tree.

Restacked onto feature/atomcompass_new after PR #4 landed as 947d5b2. The
P0.2 measurements that arrived with it are kept where they landed; this branch
carries only what supersedes them, and states the tree and container for each.

The execution plan and the validation protocol are updated to match: the CPU
tier's exclusion list and blind spot are now files in the tree rather than prose,
the GPU baseline carries its full toolchain, and the pre-flight's D-state census
states its scope because in a container it cannot see the node.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jgong5 pushed a commit that referenced this pull request Sep 21, 2026
The rule read "ATOM's test suite passes unmodified -- 187 files, no GPU
needed". The measurement that landed with PR #4 shows 29 of the 157 non-plugin
files reach the GPU driver, so the suite is not GPU-free and only a tier of it
is. Corrected to the two tiers, pointing at that measurement; nothing else in
the document is touched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.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