Skip to content

compass(spec): hold the probe tables, the reimport helper and the transfer stanzas against their defects #357

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jgong5 merged 1 commit into from
Sep 23, 2026
Merged

compass(spec): hold the probe tables, the reimport helper and the transfer stanzas against their defects #357

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions tests/compass/test_spec_verbs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,7 @@ def test_a_transfer_keeps_the_source_stack_out_of_this_machines_pin():
checked = validate(combination)
assert not checked.ok
assert checked.refusals[0].rule is Rule.PINNED_STACK
assert checked.refusals[0].what.startswith("'tier2' (machine ")

@jgong5 jgong5 Sep 23, 2026

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, R1-2: same as R1-1, in the stack-mismatch arm.

Principle 6, and the rule "A check counts only once someone has seen it fire."

Mutant CA2mL. The mismatch arm keeps fragment.source, and renders the rest of the stanza from merged.fragments[-1]. The text it produces, probed on node 18:

'tier2' (machine 'mi355x-8gpu-2node', probed, by a person on 2026-09-18) carried constants over from 'mi300x-8gpu', measured against rocm '7.0.2', into a spec pinned to rocm '7.2.4'

Result: head 064ac89bd, 1307 passed, 0 failed. The endswith below pins everything after the stanza, and this startswith pins everything up to (machine . That leaves exactly the method, author and date free.

The fix: extend the needle through the method, "'tier2' (machine 'mi355x-8gpu-2node', transferred-from:mi300x-8gpu, by a person on 2026-09-18) carried ". With that, startswith plus endswith together cover the whole string.

This is non-blocking for the same reason as R1-1: the #346 reviewer's CA2m is red at the head (1 F) and green at the tip.

assert checked.refusals[0].what.endswith(
"carried constants over from 'mi300x-8gpu', measured against rocm "
"'7.0.2', into a spec pinned to rocm '7.2.4'"
Expand Down Expand Up @@ -597,6 +598,7 @@ def test_a_saved_transfer_merged_again_names_the_merge_that_dropped_its_pin():
rest = fragments()[:2] + [fragment("links", LINKS)]
(refused,) = validate(merge([saved] + rest)).refusals
assert refused.rule is Rule.PINNED_STACK
assert refused.what.startswith("'t.yaml' (machine ")

@jgong5 jgong5 Sep 23, 2026

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, R1-1: the needle stops just before the part of the stanza that differs between fragments.

Principle 6: "A declined answer with a named reason is a result. A guessed one is a defect." The rule: "A check counts only once someone has seen it fire."

't.yaml' (machine holds the source name. (machine …) is the same for every fragment, because _one_machine enforces that. After it, the stanza renders the method, author and date, and in these fixtures the method is the field that tells fragments apart.

Mutant CA2L. The saved arm keeps fragment.source, but the rest of its stanza comes from merged.fragments[-1] ('links'). It is one line, the line count is kept, and the file is atom/compass/spec/validate.py. Here is the text it produces, probed on node 18:

't.yaml' (machine 'mi355x-8gpu-2node', probed, by a person on 2026-09-18) carried constants over from 'mi300x-8gpu', and its provenance names ...

It says a probed fragment carried constants over. That contradicts itself: the real stanza reads transferred-from:mi300x-8gpu.

Result: tests/compass, head 064ac89bd, 1307 passed, 0 failed. The null control is also 1307 passed. At the tip it is 1307 passed as well.

A fix of the same shape as your first_hand control a few lines below:

    assert refused.what.startswith(
        "'t.yaml' (machine 'mi355x-8gpu-2node', transferred-from:mi300x-8gpu, "
        "by a person on 2026-09-18) carried constants over from "
    )

The needle you have does what the brief asked, and it reddens the #346 reviewer's CA2 (0 F at the tip, 1 F at the head). So this finding is non-blocking. If it is not taken here, it needs a follow-up issue. The same gap is in the mismatch arm (R1-2).

assert "over from 'mi300x-8gpu', and its provenance names the" in refused.what
assert "fragments an earlier merge built it from" in refused.what
bare = fragment("tier2", TIER2, method="transferred-from:mi300x-8gpu")
Expand Down Expand Up @@ -1987,6 +1989,10 @@ def test_the_probe_question_reads_only_the_tables_a_probe_can_fall_short_on():
}


#: The one width table a probe falls short on, the retained bytes at width 1.
THE_HOLE = "device.runtime_constants.allocator_retained_after_load_bytes"


def reimported_validate(name):
"""`validate` imported again, the way a session imports it the first time.

Expand All @@ -2002,6 +2008,7 @@ def reimported_validate(name):
)
module = importlib.util.module_from_spec(loaded)
loaded.loader.exec_module(module)
assert importlib.import_module("atom.compass.spec.validate") is not module

@jgong5 jgong5 Sep 23, 2026

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, R1-3: the new assertion holds half of the docstring's sentence.

Principle 8: "Every claim carries its measurement."

The docstring (L2002) says the helper "does not register it". The assertion holds a narrower claim: that the helper does not register the module under the canonical name.

Mutant REGown. It changes exec_module(module) to exec_module(module); __import__('sys').modules[loaded.name] = module, which registers the module under its own name. The line count is kept.

Result: head 064ac89bd, 1307 passed, 0 failed. For comparison, your REG mutant, which registers under the canonical name, gives 2 F by both callers' ids, and it is green at the tip. I reproduced that too.

The consequence clause is held. "the instance the rest of the suite is holding is the one it started with" is exactly what import_module(...) is not module checks.

It cannot pass for the wrong reason in this file. The canonical module is imported at collection, by L57–68. I probed that "atom.compass.spec.validate" in sys.modules is True at the call, so import_module returns the suite's instance and never imports a fresh one.

Two one-line fixes: narrow the sentence to "does not register it under the package's name", or add assert loaded.name not in sys.modules. The first costs no import.

return module


Expand All @@ -2019,8 +2026,9 @@ def test_a_width_table_no_probe_is_named_for_is_refused_and_not_an_import_error(
monkeypatch.setattr(schema_module, "SCHEMA", schema_module.SCHEMA + (added,))
under_test = reimported_validate("validate_with_a_table_no_probe_is_named_for")
assert added.path in under_test.WIDTH_TABLES
# A table with no entry has no hole to report, so it is not a probe table.
assert added.path not in under_test.PROBE_TABLES
# A table with no entry has no hole to report, so it is not a probe table,
# and the table that has one still is.
assert under_test.PROBE_TABLES == (THE_HOLE,)
# The term is named where a caller asks about it, by the refusal this
# package exists to give.
with pytest.raises(SpecRefusal) as refused:
Expand All @@ -2045,7 +2053,8 @@ def test_a_probe_given_the_width_that_has_none_empties_the_probe_tables(monkeypa
(probes_module.SINGLE_CARD, probes_module.MULTI_RANK),
)
under_test = reimported_validate("validate_with_every_width_filled")
assert under_test.PROBE_TABLES == ()
# Emptied by the probe, and not empty before it.
assert (PROBE_TABLES, under_test.PROBE_TABLES) == ((THE_HOLE,), ())
checked = under_test.validate(merged().document, tp_widths=(16,))
assert not checked.ok
assert not any(
Expand Down