Skip to content

feat(ecosystem): PS-140 named a fix and the one command a reader would run did nothing - #746

Open
ywatanabe1989 wants to merge 1 commit into
developfrom
feat/ps140-repair-tail
Open

feat(ecosystem): PS-140 named a fix and the one command a reader would run did nothing#746
ywatanabe1989 wants to merge 1 commit into
developfrom
feat/ps140-repair-tail

Conversation

@ywatanabe1989

Copy link
Copy Markdown
Collaborator

feat(ecosystem): PS-140 named a fix and the one command a reader would run did nothing

PS-140 reports an ERROR when a cross-package gate guards with

module = pytest.importorskip(module_name)      # the FULL dotted path

because that SKIPS when a submodule is missing, so a rename inside an
installed peer reports as an absence and the run reads green. The gate exists
to catch that and cannot.

The rule then names the fix in prose. For a file that does not yet exist, the
generator emits it correctly. For an EXISTING file -- all 25 affected repos --
regenerating changes nothing: install-cross-package-gate owns only the import
list between the sentinels and deliberately preserves the tail below the
closing sentinel, so a leaf that hand-strengthened its assertion is not
silently reverted. Measured: a --dry-run against scitex-str re-emitted the
broken body verbatim.

So the defect lives in the preserved half and the obvious remedy cannot reach
it. That is what this adds a remedy for.

scitex-dev ecosystem install-cross-package-gate <dist> --force --repair-tail

OFF BY DEFAULT. The preservation policy is correct and this is an opt-in to
touching the region it protects.

WHAT IT REWRITES -- the GUARD, and nothing else:

# Arrange
root = module_name.split(".")[0]
pytest.importorskip(root)
# Act
module = importlib.import_module(module_name)
# Assert
assert module is not None            <- whatever was there, untouched

SURGICAL RATHER THAN CANNED, and that choice is load-bearing. Two shapes are
deployed. scitex-logging and scitex-str carry the stock body; scitex-io carries
a hand-written one with a different test name and a strictly stronger assertion
(assert getattr(mod, "__name__", None) == module_name) -- and it is STILL
BROKEN, because strengthening what a test asserts does nothing about whether it
runs. Swapping in a canned body would fix scitex-io by discarding the
improvement someone deliberately made; declining on it would leave it broken
forever. Rewriting only the guard does neither.

It DECLINES, loudly and by name, on any tail whose shape it cannot prove from
the AST. A repair that guesses is worse than an error message: a wrong rewrite
of a test gate is invisible in exactly the way the original defect is.

VERIFIED BEHAVIOURALLY, and the control is the part that counts:

the ORIGINAL body, given "json._this_submodule_does_not_exist"  ->  SKIPS
the REPAIRED body, same input                    ->  ModuleNotFoundError
a genuinely absent root                          ->  still SKIPS

That last one is not a nicety. PS-140's own prose: banning the skip "would
convert a legitimate absence into a hard failure -- a gate that cannot PASS, in
place of one that cannot FAIL."

Asserting the BROKEN behaviour explicitly is what establishes that the first
assertion measures this change rather than something incidental about json.

Test inputs are TRANSCRIBED FROM THE FLEET -- scitex-logging's and scitex-io's
actual bodies as of 2026-08-23 -- not invented samples. A repair that works on
a tidied-up sample and not on what is deployed would pass a test file and fix
nothing.

TWO LEGIBILITY DEFECTS FIXED HERE TOO, both introduced by this change and
caught before shipping:

  • the dry-run printed "# would PRESERVE N line(s) ... verbatim" while the
    guard was being rewritten. That line's whole job is to prove the
    preservation policy held, so leaving it would have made the reassurance
    itself the thing hiding a change to it. It now says REPAIR, and so does
    the write summary.
  • the first version left # Arrange empty and put the precondition under
    # Act. STX-TQ002 passes either way -- it checks that the three markers
    appear in order, not that anything sits under them -- so no linter would
    have caught it. The skip is a precondition; the repair now places it where
    the reclaimed alias line was.

POPULATION this unblocks, measured against the REMOTES via the contents API
(76 org repos, both branches):

branch     OLD   NEW   ABSENT
develop     25    26     25
main        27    24     25

A develop fix does NOT reach a leaf whose scheduled CI runs on main, so each
sweep PR must say which branch it fixed.

Reported by scitex-hub, who hit it in their own gate and measured the
conversion cost first: 12 modules, 12 import cleanly, 0 newly failing, 0 roots
absent. That measurement is what makes hard-import-instead-of-skip safe rather
than merely correct.

VERIFIED: full suite 7341 passed, 58 skipped, 1 xfailed, exit 0.

…d run did nothing

PS-140 reports an ERROR when a cross-package gate guards with

    module = pytest.importorskip(module_name)      # the FULL dotted path

because that SKIPS when a submodule is missing, so a rename inside an
installed peer reports as an absence and the run reads green. The gate exists
to catch that and cannot.

The rule then names the fix in prose. For a file that does not yet exist, the
generator emits it correctly. For an EXISTING file -- all 25 affected repos --
regenerating changes nothing: `install-cross-package-gate` owns only the import
list between the sentinels and deliberately preserves the tail below the
closing sentinel, so a leaf that hand-strengthened its assertion is not
silently reverted. Measured: a --dry-run against scitex-str re-emitted the
broken body verbatim.

So the defect lives in the preserved half and the obvious remedy cannot reach
it. That is what this adds a remedy for.

    scitex-dev ecosystem install-cross-package-gate <dist> --force --repair-tail

OFF BY DEFAULT. The preservation policy is correct and this is an opt-in to
touching the region it protects.

WHAT IT REWRITES -- the GUARD, and nothing else:

    # Arrange
    root = module_name.split(".")[0]
    pytest.importorskip(root)
    # Act
    module = importlib.import_module(module_name)
    # Assert
    assert module is not None            <- whatever was there, untouched

SURGICAL RATHER THAN CANNED, and that choice is load-bearing. Two shapes are
deployed. scitex-logging and scitex-str carry the stock body; scitex-io carries
a hand-written one with a different test name and a strictly stronger assertion
(`assert getattr(mod, "__name__", None) == module_name`) -- and it is STILL
BROKEN, because strengthening what a test asserts does nothing about whether it
runs. Swapping in a canned body would fix scitex-io by discarding the
improvement someone deliberately made; declining on it would leave it broken
forever. Rewriting only the guard does neither.

It DECLINES, loudly and by name, on any tail whose shape it cannot prove from
the AST. A repair that guesses is worse than an error message: a wrong rewrite
of a test gate is invisible in exactly the way the original defect is.

VERIFIED BEHAVIOURALLY, and the control is the part that counts:

    the ORIGINAL body, given "json._this_submodule_does_not_exist"  ->  SKIPS
    the REPAIRED body, same input                    ->  ModuleNotFoundError
    a genuinely absent root                          ->  still SKIPS

That last one is not a nicety. PS-140's own prose: banning the skip "would
convert a legitimate absence into a hard failure -- a gate that cannot PASS, in
place of one that cannot FAIL."

Asserting the BROKEN behaviour explicitly is what establishes that the first
assertion measures this change rather than something incidental about `json`.

Test inputs are TRANSCRIBED FROM THE FLEET -- scitex-logging's and scitex-io's
actual bodies as of 2026-08-23 -- not invented samples. A repair that works on
a tidied-up sample and not on what is deployed would pass a test file and fix
nothing.

TWO LEGIBILITY DEFECTS FIXED HERE TOO, both introduced by this change and
caught before shipping:

  * the dry-run printed "# would PRESERVE N line(s) ... verbatim" while the
    guard was being rewritten. That line's whole job is to prove the
    preservation policy held, so leaving it would have made the reassurance
    itself the thing hiding a change to it. It now says REPAIR, and so does
    the write summary.
  * the first version left `# Arrange` empty and put the precondition under
    `# Act`. STX-TQ002 passes either way -- it checks that the three markers
    appear in order, not that anything sits under them -- so no linter would
    have caught it. The skip is a precondition; the repair now places it where
    the reclaimed alias line was.

POPULATION this unblocks, measured against the REMOTES via the contents API
(76 org repos, both branches):

    branch     OLD   NEW   ABSENT
    develop     25    26     25
    main        27    24     25

A develop fix does NOT reach a leaf whose scheduled CI runs on main, so each
sweep PR must say which branch it fixed.

Reported by scitex-hub, who hit it in their own gate and measured the
conversion cost first: 12 modules, 12 import cleanly, 0 newly failing, 0 roots
absent. That measurement is what makes hard-import-instead-of-skip safe rather
than merely correct.

VERIFIED: full suite 7341 passed, 58 skipped, 1 xfailed, exit 0.
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