Skip to content

compass(tests): pin the citation check's atom/model_engine/ prefix strip - #413

Merged
jgong5 merged 1 commit into
feature/atomcompass_newfrom
compass/issue-411
Sep 24, 2026
Merged

jgong5 merged 1 commit into
feature/atomcompass_newfrom
compass/issue-411

Conversation

@jgong5

@jgong5 jgong5 commented Sep 24, 2026

Copy link
Copy Markdown
Owner

Closes #411

What changed

tests/compass/test_runner_rpc_surface.py only, +19 / -6 (net +13, brief 5-15):

  • The nested cite in test_every_site_the_package_docstring_cites_is_one_no_caller_waits_for moves to module level as _cite. Its body and its parameter name are unchanged, so the reviewer's M2 sed from compass(tests): key broadcast sites by their repo-relative path, not the base name engine_core.py #407 still applies verbatim.
  • New test test_a_site_in_a_same_named_file_elsewhere_is_not_a_cited_one. It takes a real cited site from SITES and moves it to atom/diffusion/engine/<same base name>. It then asserts that the original is cited and the moved one is not. The first assertion keeps the second from passing vacuously.

Nothing near test_what_the_comment_says_a_hole_at_exit_loses_is_what_exit_does is touched.

Mutant table

Measured on xiaobizh_n18_cpu, on the merged tree d358a0253 (tip 60186b80c + head e5b4d4007). Each plant ran on a copy that was restored afterwards, and diff -rq against the staged tree was checked empty. atom.__file__ resolved under the staged root.

  • new = test_a_site_in_a_same_named_file_elsewhere_is_not_a_cited_one
  • cite = test_every_site_the_package_docstring_cites_is_one_no_caller_waits_for
run tests result
unplanted new + cite 2 passed
unplanted whole file 70 passed
M2: _cite returns pathlib.Path(s.file).name (the reviewer's sed, line count 1236 kept) whole file 1 failed, 69 passed. FAILED ...::test_a_site_in_a_same_named_file_elsewhere_is_not_a_cited_one at :1194, assert 'engine_core.py:500' not in {...}
C1: unwaited call_func("exit") at atom/diffusion/engine/engine_core.py:260 cite 1 failed, :1177
C2: the same call at atom/model_engine/zz/engine_core.py:260 cite 1 failed, :1177
M2 + C1 cite 1 passed. This is the hole #411 names.
M2 + C1 new 1 failed, :1194, assert 'engine_core.py:260' not in {...}

Under M2 the failing assertion is the not in one (:1194), not the non-vacuity guard. The site next() picks depends on set order. Every cited site gives the same verdict, so the order does not matter.

Gate 1

Merged tree git merge-tree --write-tree 60186b80c e5b4d4007 = d358a0253. It was gated once, on xiaobizh_n18_cpu, with its own scripts/compass/gate_cpu.sh.

  • Staged by git archive of stamp commit 1ea8471ca (parents tip 60186b80c and head e5b4d4007). The tarball md5 ff5ab2fcd7574a379f5293f70a11edc2 matched on both ends.
  • .compass-changed = tests/compass/test_runner_rpc_surface.py.
  • The gate printed commit: 1ea8471ca (stamp) and atom: /tmp/i411/merged/ATOM/atom/__init__.py.
  • The run was bounded by timeout -k 10 3000, not piped, and ran alone.
tree passed skipped xfailed GATE_CPU_RC
tip (brief's baseline) 5280 155 3 0
merged d358a0253 5281 155 3 0 PASSED

Suite delta: +1 id, the new test. #402, which moved the tip from 4116a63a9 to 60186b80c during this work, changes only the package docstring and adds no test. No flake class fired.

ruff check and ruff format --check on the changed file are clean.

🤖 Generated with Claude Code

The docstring citation check keys a site by its path with
`atom/model_engine/` stripped, so a site in a same-named file elsewhere
keeps a `/` that no citation matches. Nothing held that: switching the
key back to the base name left every test green.

`cite` moves to module level as `_cite`, unchanged, and one test feeds it
a cited site moved to `atom/diffusion/engine/`, asserting the original is
cited and the moved one is not.

Closes #411

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
citation matches. A base name would let it satisfy the citation.
"""
site = next(s for n in UNWAITED for s in SITES[n])
moved = site._replace(file=f"atom/diffusion/engine/{pathlib.Path(site.file).name}")

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.

Non-blocking. The test catches a base-name _cite, but one other spelling of the strip survives it.

I measured this on xiaobizh_n18_cpu, on the merged tree d358a0253. Each mutant replaced the one return line in _cite; the line count stayed at 1236.

mutant whole file with a plant
R1: re.sub(r'^atom/[a-z_]+/', '', s.file) (strip any atom/<pkg>/) 70 passed C3: an unwaited call_func("exit") at atom/zz_plant/engine_core.py:260. Head: cite 1 failed at :1177. R1 + C3: 2 passed (cite and new).

So R1 reopens the collision for a same-named file one level under any sibling package. The moved path here is two levels down (atom/diffusion/engine/). Under R1 it keeps an engine/ segment, so R1 leaves it uncited. R1 still catches the real C1 file: R1 + C1 is red at :1177.

The brief's required finding is M2, and M2 is pinned. I reproduced it red by name at :1194. R1 is not the obvious "simplification". It does not hold APPROVE.

If you want it closed at zero lines, f"atom/diffusion/{pathlib.Path(site.file).name}" kills M2, R1 and rsplit('/', 1)[-1]. The cost is that the test no longer names the real same-named file. Declining is fine.

Principle 6, "Refuse rather than fall back. ... A guessed one is a defect." AI_DEV_RULES gate 4, "A check counts only once someone has seen it fire."

"""
site = next(s for n in UNWAITED for s in SITES[n])
moved = site._replace(file=f"atom/diffusion/engine/{pathlib.Path(site.file).name}")
cited = set(re.findall(CITATION, PACKAGE_DOC))

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.

Non-blocking, ponytail. L1192: shrink: cited = set(re.findall(CITATION, PACKAGE_DOC)) repeats L1174. Add a module-level CITED = set(re.findall(CITATION, PACKAGE_DOC)) beside CITATION and use it in both tests: net -1.

This is optional. The duplicate is one line, and both copies read the same constant.

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

@jgong5

jgong5 commented Sep 24, 2026

Copy link
Copy Markdown
Owner Author

Review cycle 1: APPROVE, head e5b4d400714c21972e158aae86be4ee7551516e0

There are no blocking findings. Two non-blocking findings are inline.

What was checked

Every changed line. The diff is tests/compass/test_runner_rpc_surface.py only, +19/−6, net +13, inside the brief's 5–15.

  • Hoisting cite to _cite is needed. The new test has to call the function the citation check uses. A copy inside the new test would pin the copy, not the check. M2 applied to _cite alone turns the new test red, which shows the test reaches the shared function. The body is byte-identical to the nested version.
  • The new test cannot pass vacuously.
    • If no unwaited site existed, next() would raise StopIteration, which is an error and not a pass.
    • The guard assert _cite(site) in cited holds the original to being cited. I saw it fire: under R2 below it fails at :1193.
    • The moved path cannot be cited by accident. CITATION is r"`([a-z_]+\.py:\d+)`", which admits no /, and the stripped moved path keeps one.
    • next() picks a site in set order, which changes from run to run. That does not matter: under M2 the second assertion fails for whichever site the first one accepted. I saw :260, and the developer saw :500.
  • The comments and docstrings are true.
    • # the docstring cites paths under atom/model_engine/: all six citations in the package docstring at the tip are atom/model_engine/engine_core.py and pp_engine_core.py lines.
    • The new docstring says only that prefix is stripped, so the moved site keeps a / that no citation matches. That holds for this _cite and this CITATION.
    • It also says a base name would let the moved site satisfy the citation. M2 confirms that.
    • Neither comment carries a design-doc reference. AI_DEV_RULES: "No design-doc references in code or runtime output."
  • Gate 2. AI_DEV_RULES: "The tests must exercise something the PR did not itself add." The new test drives the pre-existing _cite, SITES recovered from ATOM's source, and the package docstring.
  • ruff. ruff check and ruff format --check on the file are clean. That is ruff 0.16.7 on node 18.

Mutants

These ran on xiaobizh_n18_cpu, on a git archive of the merged tree d358a0253, with atom.__file__ under the staged root.

  • Each mutant replaced the one return line of _cite, keeping the line count at 1236.
  • Each copy was restored afterwards and checked identical with diff -rq.
  • Unplanted, the whole file gave 70 passed.
mutant run result
M2: pathlib.Path(s.file).name whole file 1 failed, 69 passed: test_a_site_in_a_same_named_file_elsewhere_is_not_a_cited_one, :1194, assert 'engine_core.py:260' not in {...}
M2 + C1: unwaited exit at atom/diffusion/engine/engine_core.py:260 cite + new 1 failed, 1 passed: cite green (this is the #411 hole), new red at :1194
R3 (mine): s.file.rsplit('/', 1)[-1] whole file 1 failed, 69 passed: new, :1194, assert 'engine_core.py:500' not in {...}
R2 (mine): s.file.removeprefix('atom/') whole file 2 failed, 68 passed: cite at :1177, and new at :1193 (the guard)
R1 (mine): re.sub(r'^atom/[a-z_]+/', '', s.file) whole file 70 passed. R1 survives.
C3 at the head: unwaited exit at atom/zz_plant/engine_core.py:260 cite 1 failed, :1177
R1 + C3 cite + new 2 passed: the hole R1 reopens
R1 + C1 cite + new 2 failed, cite at :1177 and new at :1193

The brief's named result is met:

  • M2 is red by name, and the head is green.
  • C1 and C2 are still caught. C1 is shown above under R1 and M2; C2 is in the developer's run and is unchanged by this diff.
  • The suite delta is +1 id.

Finding 1 (non-blocking, inline at L1191): R1 survives. It is not the obvious simplification, and M2, the required finding, is pinned, so it does not hold APPROVE. AI_DEV_RULES gate 4: "An inert pin on a required finding blocks APPROVE." The pin on M2 is not inert. A zero-line option is inline.

ponytail-review

  • L1192: shrink: cited = set(re.findall(CITATION, PACKAGE_DOC)) repeats L1174. Add a module-level CITED beside CITATION and use it in both tests. This is finding 2, non-blocking and inline.

net: -1 lines possible.

Gate 1

The tip moved from 60186b80c to 9a6317927 (#410) after the developer's gate. git merge-tree --write-tree 9a6317927 e5b4d4007 is 088e436e8, not d358a0253, so the developer's gate does not cover the tree that would land. I gated the new tree once.

  • Staging. git archive of the stamp commit 57cacb907, made with git commit-tree 088e436e8 -p 9a6317927 -p e5b4d4007.
    • .compass-commit and .compass-changed went in as virtual files; .compass-changed = tests/compass/test_runner_rpc_surface.py.
    • The tarball md5 c3950ec5 matched on both ends.
  • Run. On xiaobizh_n18_cpu, with the tree's own scripts/compass/gate_cpu.sh.
    • It printed commit: 57cacb907 (stamp) and atom: /tmp/r413/gate/ATOM/atom/__init__.py.
    • It was bounded by timeout -k 10 3000, not piped, and started only after pgrep -f "[g]ate_cpu.sh" came back empty.
tree passed skipped xfailed GATE_CPU_RC
tip 9a6317927 (tree 5f9ee034e, #410's gated merged tree) 5273 155 3 0
merged 088e436e8 5274 155 3 0 PASSED

Suite delta: +1 id, the new test. No flake class fired. The tip baseline is #410's gate on the same tree, 5f9ee034e, which is 9a6317927^{tree}.

For the next task in this area

_cite pins one spelling of the strip against the base name. A new pin is needed if the docstring ever cites a file outside atom/model_engine/ or in a subdirectory of it. CITATION admits no /, so such a citation would fail the check and could not be written as it stands.

@jgong5
jgong5 marked this pull request as ready for review September 24, 2026 04:25
@jgong5
jgong5 merged commit d803291 into feature/atomcompass_new Sep 24, 2026
jgong5 added a commit that referenced this pull request Sep 24, 2026
…ile citation test (#418)

Stripping any atom/<pkg>/ prefix (R1) survived
test_a_site_in_a_same_named_file_elsewhere_is_not_a_cited_one. That
test planted its site two levels below atom/, where R1 still left a
"/" behind. The site now sits at atom/diffusion/<name>, one level
below atom/, and the docstring names the atom/<pkg>/ strip next to the
base name.

These _cite mutants now fail the test by name:
- R1 and M2, from the #413 review;
- R3, taking the text after the last "/";
- two added in this review: strip atom/ plus one or two segments, and
  endswith.

Both citation tests now read one module-level CITED frozenset. A
missing runner package docstring now surfaces as a collection error at
import, not as 8 named failures. That is filed as a follow-up.

Gate (node 18, CPU tier, merged tree on 29006c6): 5275 passed,
155 skipped, 3 xfailed, GATE_CPU_RC=0.

Closes #416

Co-Authored-By: Claude Opus 5.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