Skip to content

compass(tests): say what the kv, capture and backend tests check, not which task wrote them - #375

Merged
jgong5 merged 1 commit into
feature/atomcompass_newfrom
compass/issue-320-test-task-refs
Sep 24, 2026
Merged

jgong5 merged 1 commit into
feature/atomcompass_newfrom
compass/issue-320-test-task-refs

Conversation

@jgong5

@jgong5 jgong5 commented Sep 24, 2026

Copy link
Copy Markdown
Owner

Agent-authored (developer). I read the README's eight Design principles and AI_DEV_RULES.md at 8c0a2e6ee first.

Closes #320

No blocking issues. Comments and docstrings only, in four test files: +35/−38, inside the 30–50 estimate counted as changed sentences, and under 2x counted as changed lines. With docstrings masked, every file's AST is identical to the tip's. No test is renamed. The suite delta is 0.

What changed

Each rewrite says what the code does, or deletes a clause that only narrated the task.

file site (tip line) was now
test_kv_remote_prefill.py :15 "the defect this cut exists to close was a connector that behaved..." "the defect these tests exist to catch is a connector that behaves..."
:108 "The check the named result makes" "The check the field-set test makes"
:186 "The named result: the emitted set equals..." "The emitted set equals..."
:370 "The named result's other half" "The module's second claim" (the module docstring's two claims: the blob, then the suspension)
test_kv_simulated_connector.py :55, :59 "The two link speeds / transfer sizes of the named result" "Two link speeds / Two transfer sizes"
:126 "The named result: four cells..." "Four cells..."
test_backend_kv_geometry.py :52 "the device readings are another task's" "no device is read for it"
test_capture_real_model.py :150 "the device readings are another task's" "no device is read for it" (the file's own ARCH block: "Nothing here reads a device")
:1073 "one of the two repair routes the design record carries for this" "one repair to this site"
:2251 "the repair route the design record names for site two ... could have landed" "a repair to site two ... could have gone in"
:2485 "-- which is the point, because the repair is the next task and this is how it will be known to have worked" deleted
:2523 "that is this task's one production line" "it returns t.shape[0] as it is" (true at atom/utils/forward_context.py:430)
:2682, :2862 "TP2 is the one/width the result is named for" the clause dropped; the existing reason, a real rather than simulated width, kept
:2764 "This is the one the production change is for" "This is the one closed in ATOM's own code"
:2821 "the one line under atom/ this task changed" "ATOM's own _rows, which returns t.shape[0] unconverted"

The census curated 14 of these. :2682, :2862 and :2764 are three more of the same kind that the census regex does not reach ("the result is named for", "the production change"). No issue number was touched. There were no "owner's ruling" markers in this file set.

Coordination with #364. test_a_call_through_either_binding_reaches_the_sentinel (tip :2209) and test_the_capture_refuses_a_width_that_torch_would_specialise (tip :2597) are not edited. No census site falls inside either. The nearest hunk is at :2250, 14 lines below #364's last hunk in its own worktree.

File set. The body of #320 names test_kv_budget*.py. The census's S4 row, which the 14 sites come from, names test_kv_remote_prefill.py and test_kv_simulated_connector.py instead. I edited the census's four files and kept test_kv_budget.py and test_kv_budget_engine.py in the census and AST checks. Neither carries a task reference at the tip.

Gate 3: the named result

Census. census.py from #222 is unchanged (md5 688ef157...). I re-ran it on the worktree at the tip 8c0a2e6ee and at the head, filtered to the six files: test_kv_budget.py, test_kv_budget_engine.py, test_kv_remote_prefill.py, test_kv_simulated_connector.py, test_capture_real_model.py and test_backend_kv_geometry.py.

raw untagged rows curated sites tagged
tip 8c0a2e6ee 32 14 (11 raw rows, plus :150, :52, :2485 and :2821, which the second-pass words find) 0
head 20 0 0

The 20 raw rows left at the head, all read in context:

rule rows
E6, "the record" meaning the capture record or KV budget record the code writes 19: test_capture_real_model.py :32, :39, :45, :886, :1006, :1127, :1192, :1848, :1902, :1936, :1946 (a test string), :2123, :2147, :2159, :2244, :2570, :2595; test_kv_budget.py:2; test_kv_budget_engine.py:178
E6, "decision" as a code concept 1: test_capture_real_model.py:380, "a new operator on ATOM's forward is a classification decision somebody makes"

:2595 is inside #364's width-1 test. It is E6 either way.

The census's second-pass words (task, cut, sweep, reviewer, cycle, owner, brief, filed), plus "named result", "design record", "landed", "ruling", "inherited", "inert pin", "named for" and "production change/line", grep to two hits at the head. Both are test_backend_kv_geometry.py :16 and :216, "the heads cannot be cut". That is E6, KV heads split across ranks.

AST check. For each of the six files, I compared ast.dump at the tip and at the head with every module, class and function docstring replaced by a placeholder. Comments are not in the AST.

file masked unmasked
test_kv_simulated_connector.py IDENTICAL differs
test_kv_remote_prefill.py IDENTICAL differs
test_capture_real_model.py IDENTICAL differs
test_backend_kv_geometry.py IDENTICAL same (comment only)
test_kv_budget.py, test_kv_budget_engine.py IDENTICAL same (untouched)

Controls: BLOCK_SIZE = 64 → 65 reads DIFFERENT; a docstring edit reads IDENTICAL. No test is renamed.

Gate 1: the CPU tier, as a delta

Node 18, xiaobizh_n18_cpu. Both trees were staged by git archive into /tmp/i320gates/{control,branch} inside the container, with .compass-commit and .compass-changed written from the same ref. The tarball md5 matched on both ends. Each ran its own scripts/compass/gate_cpu.sh with PYTHONPATH set to its root, under timeout -k 10 3000, one at a time and not piped.

  • git merge-tree --write-tree 8c0a2e6ee 21a9dd3a8 → e8fb1c4b3704679f1db26528c03fc0187a244c21, which is 21a9dd3a8^{tree}. The branch side is a git commit-tree of that tree, bce8a76b6.
side stamp atom.__file__ result GATE_CPU_RC
control 8c0a2e6ee /tmp/i320gates/control/ATOM/atom/__init__.py 5268 passed, 155 skipped, 3 xfailed 0
branch bce8a76b6 /tmp/i320gates/branch/ATOM/atom/__init__.py 5268 passed, 155 skipped, 3 xfailed 0

Node-id delta, from each side's --junitxml: 5426 ids on each side. None only in the control, none only in the branch, and no outcome changed. No timing-class test failed or skipped differently, so nothing needed a re-run.

ruff check and black --check on the four files: RUFF_RC=0 and BLACK_RC=0 on both sides.

Left undone

  • History narration is out of scope, as in the census (§3, "not counted"), so these are unchanged: "An earlier version of this file..." (test_capture_real_model.py:2249), the module docstring's "One line of ATOM changed for it" (:95), and "the defect this connector's predecessor was fixed for" (test_kv_remote_prefill.py:379).
  • "the whole result" (test_capture_real_model.py:2833 at the tip) is left as it is. It reads as the capture's result, the symbol staying free, and does not point at a task.

🤖 Generated with Claude Code

… which task wrote them

Rewrites the task and design-record references in four test files so each
sentence says what the code does: "the named result", "this cut", "this
task's one production line", "another task's", "the next task", "the design
record carries/names", and "the result is named for". Comments and
docstrings only; no test logic, name or string that a test reads changes.

Closes #320

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
about the allocation and only then decides to suspend it -- four steps in one
loop, in that order, and the defect this cut exists to close was a connector
that behaved correctly at each step and wrongly across them. So the request
loop, in that order, and the defect these tests exist to catch is a connector

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.

Agent-authored (reviewer, cycle 1). Non-blocking. "the defect these tests exist to catch" is wider than what holds it.

The module docstring says two claims live here. This paragraph is about the second, the suspension. The first-claim tests do not catch a connector that behaves correctly at each step and wrongly across them. Those are test_the_blob_carries_the_field_set_the_router_relays, test_dropping_one_field_from_the_blob_is_refused_by_name and the other blob tests at :185–:330. That defect is caught by the scheduler_with(...) tests from :367 on.

Read at module scope, "these tests" is the same present-tense overreach that #322's and #325's reviews caught. The paragraph's context makes the intended reading likely, which is why this is non-blocking.

Suggested: "the defect the suspension tests exist to catch is a connector that behaves correctly at each step and wrongly across them."

Rule: AI_DEV_RULES.md, "Say what the code does." Principle 8: "Every claim carries its measurement."

**Site three**, `assert_shape_contract`'s `_rows`. This is the one the
production change is for, and it is the only one that could not be closed
from outside ATOM: `int(t.shape[0])` converts a dimension the assertion
**Site three**, `assert_shape_contract`'s `_rows`. This is the one closed in

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.

Agent-authored (reviewer, cycle 1). Non-blocking, ponytail. shrink: "This is the one closed in ATOM's own code, and it is" restates the clause after it. "the only one that could not be closed from outside ATOM" already says it, and the next sentence ("It no longer converts") says how. Keep: "Site three, assert_shape_contract's _rows, the only one that could not be closed from outside ATOM: ...". That is one line.

The rewrite itself is true against the tip: atom/utils/forward_context.py:430 returns t.shape[0] unconverted.

Principle 3: "Prioritise simplicity. Add only what is necessary, and nothing more."

@jgong5

jgong5 commented Sep 24, 2026

Copy link
Copy Markdown
Owner Author

Agent-authored (reviewer, cycle 1). Before reviewing, I read the README's eight Design principles and AI_DEV_RULES.md at 373f1fc5c.

Verdict: APPROVE 21a9dd3a8a232153fbd7a21640910eec819e4d1d. No blocking findings. There are three non-blocking findings: two inline and one below. Gates 1–3 hold for this task, and this approval is gate 4.

What I checked

1. Every changed line (+35/−38, comments and docstrings only). I checked each rewrite against the code at the tip. None is now false.

  • _rows "returns t.shape[0] as it is / unconverted" (:2520, :2817). True: atom/utils/forward_context.py:430 is return None if t is None else t.shape[0].
  • "TP2 is a width that is real rather than simulated" (:2679). True: the module docstring says "the group here reports width 2 because it has width 2", and it says apply_simulated_tp is not what produced it.
  • "no device is read for it" (test_backend_kv_geometry.py:52, test_capture_real_model.py:149). True: both are constants (64 << 30 and 64).
  • "The module's second claim" (test_kv_remote_prefill.py:370). True: the module docstring says "Two claims live here", and the second is the suspension.
  • "the field-set test" / "the drop test" (:108). True: :185 and :192 both call assert_relays_every_field.
  • "Two link speeds", "Two transfer sizes" and "Four cells" (test_kv_simulated_connector.py). True: PEAKS has 2 entries, BLOCK_COUNTS has 2, and the test is parametrized over both.
  • "the one closed in ATOM's own code" (:2763). True, but redundant (see the ponytail line).
  • The :2482 deletion leaves "so a repair to either one fails here", which still holds: both sites are pinned by value and by frame.

No undecided status was dropped. The six files have no "owner"/"ruling"/"undecided" marker at the tip or the head. No issue number was touched: there is no #n in the file set at either commit.

2. Census and AST, re-run on node 18. I used census.py, md5 688ef157… (unchanged), under Python 3.12.3. Each tree was a git archive snapshot, filtered to the six files.

tree raw untagged tagged
tip 373f1fc5c (same six files as 8c0a2e6ee) 31 0
head 21a9dd3a8 20 0
merged 29b96e650 20 0

The tip's 31 rows break down as 11 real + 20 E6. The 11 real rows are :1074, :2251, :2252, :2523, remote :15/:108/:186/:370, and simulated :55/:59/:126. :2251 and :2252 are one sentence, so they make 10 sites. With the 4 second-pass sites (:150, :52, :2485, :2821), that is the PR's 14 curated.

The census's second-pass words plus the PR's extra phrases give 20 hits at the tip and 2 at the head (test_backend_kv_geometry.py:16 and :216). Both head hits are "heads cannot be cut", which is E6.

AST, with module, class and function docstrings masked, tip against head and tip against the merged tree: IDENTICAL for all six files. Controls: BLOCK_SIZE = 64→65 reads DIFFERENT, a docstring edit reads IDENTICAL, and a comment edit reads IDENTICAL. No def or class name differs, so no test is renamed.

3. Rulings.

  • The 20 leftover hits are all E6. I read each in context. The 19 "the record" hits are the capture record the script prints (:32, :39, :45, :886, :1006, :1127, :1192, :1848, :1936, :1946 [an f-string refusal], :2123, :2147, :2159, :2244, :2595), a guard as "the record of where the traced graph stops being valid" (:1902, :2570), and the KV budget record (test_kv_budget.py:2, test_kv_budget_engine.py:178). The 1 "decision" hit is :380, "a classification decision somebody makes", the code's own concept. None points at a task.
  • The kept history lines are "An earlier version of this file" (:2247), "One line of ATOM changed for it" (:95) and "this connector's predecessor was fixed for" (test_kv_remote_prefill.py:379). All three fall under the census's "Not counted, and not proposed: History narration … It needs an owner ruling to be in scope". Keeping them is correct for S4.
  • Other indirect phrases also sit outside the census regex and outside this brief: "the two-site account" (:1076), "The two sites were recorded as independent … exactly as recorded" (:2506, :2516), and "the whole result" / "the comparison that carries the result" (:2829, :2859). The last now reads as the file's own result. These are worth S7's phrase list looking at, not this PR.

4. The #372 overlap. git merge-tree --write-tree 21a9dd3a8 5c89542a9 gives 07431439e, clean. The merged capture file differs from #372's head only by #375's hunks.

At the tip, #372's hunks are :2214–:2239 (sentinel test) and :2631 (width-1 test). #375's nearest hunks are :2251 and :2682. Neither test body is edited. tip + #372 also merges clean (eff0a2bec).

5. Gate (merged tree, once). I re-read the tip at 373f1fc5c.

  • git merge-tree --write-tree 373f1fc5c 21a9dd3a8 → 29b96e650df990927f39bd3d4fa6bae5234e679a. That is not 21a9dd3a8^{tree} (e8fb1c4b3), because the tip moved by compass(rules): the reviewer checks gates 1-3 as they apply per task, so approval never waits for the per-wave GPU tier (#369) #371 (AI_DEV_RULES.md, ±2).
  • Stamp: git commit-tree → 9a8b5a953. I staged it by git archive into /tmp/r375gates/merged/ATOM inside xiaobizh_n18_cpu, not the shared mount. The tar md5 matched on both ends. .compass-commit and .compass-changed were written from the same ref. I ran the tree's own gate_cpu.sh under timeout -k 10 3600, unpiped.
  • atom.__file__ = /tmp/r375gates/merged/ATOM/atom/__init__.py. The gate printed commit: 9a8b5a953 (stamp) and gpu: not required.
  • 5268 passed, 155 skipped, 3 xfailed; GATE_CPU_RC=0 PASSED. junit: 5426 cases, 0 failures, 0 errors. This matches the developer's control and branch counts exactly.
  • The timing classes all passed on this run: TestTheRegionIsNotCopiedPerChunk (4/4), TestNoSizeAtWhichACallStopsBeingOne[minimax] (2/2) and test_freezing_twice_is_additive_and_harmless.
  • ruff check and black --check on the four files: rc 0 and rc 0.

Findings (all non-blocking)

  1. Inline, test_kv_remote_prefill.py:15. "the defect these tests exist to catch" is wider than the tests that hold it. The blob tests do not catch a cross-step defect. Suggested: "the suspension tests". Rule: AI_DEV_RULES, "Say what the code does."
  2. Inline, test_capture_real_model.py:2763. Ponytail shrink: (below).
  3. PR body: the tip's raw census count is 31, not 32. The re-run gives 31, and the body's own decomposition (11 real + 20 E6) sums to 31. The curated 14 and the head's 20/0 are right. Principle 8: "Every claim carries its measurement. A number without a source is a defect."

ponytail-review

test_capture_real_model.py L2763: shrink: "This is the one closed in ATOM's own code, and it is" restates "the only one that could not be closed from outside ATOM". Drop it.

net: -1 lines possible.

Gates for this task:

  • Gate 1: delta 0 on the merged tree (above).
  • Gate 2: not applicable. The PR adds no behaviour, and the AST is identical.
  • Gate 3: the named result holds: census 0 curated at the head, AST IDENTICAL, suite delta 0.
  • Gate 4: this APPROVE.

No blocking issues.

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