#1445: derive the launch set from the registry — delete the hand list and the serialization point with it - #1013
Open
PaulFidika wants to merge 1 commit into
Open
#1445: derive the launch set from the registry — delete the hand list and the serialization point with it#1013PaulFidika wants to merge 1 commit into
PaulFidika wants to merge 1 commit into
Conversation
… and the serialization point with it
Paul's minimum-hand-specification ruling, and the same shape the master-green lane
applied to `test_cli_entry_orders`: a list is what went stale, and a second list goes
stale the same way.
`LAUNCH_SET_NAMES` was a SERIALIZATION POINT. Every ModelType PR had to edit it, so two
could never be in flight concurrently without one going stale — measured on pgw#1432
alone, master moved out from under that branch twice, 5 commits each time, and the list
went stale both times. It went stale for FOUR lanes in a row: audio, LTX-2, pgw#1432,
and master itself.
It also failed in the worst available place. The pinned list lives in the full `tests`
job, which is red for unrelated reasons, so a lane that added a ModelType, saw `tests`
red, and diffed the COUNT against the known baseline concluded "not mine" and enqueued.
Only diffing failure NAMES caught it, and nothing forced anyone to.
WHAT IS ASSERTED IS NOT CIRCULAR, and that is the whole design. `export_document()`
builds its names as `list(defaults_vocabularies())` (`defaults_export.py:58`), so
checking the export against that mapping would test the emitter against its own input
and prove nothing. `MODEL_TYPES` and `LORA_OVERLAYS` are a DIFFERENT source — the
declaration tuples — so the export is checked against those.
WHAT IS DELIBERATELY NOT ASSERTED: the exact sequence within the base block. That order
is `defaults_vocabularies()` insertion order and no independent source defines it — the
mapping's order genuinely differs from the tuple's today. Pinning it would recreate the
hand list this deletes. The ordering PROPERTY the docstring actually promises — base
types first, then the overlays — IS checked, because that one is structural rather than
incidental. Saying which of the two is checkable is the honest half of the change.
RED/GREEN VERIFIED ON ALL FOUR ARMS, by mutating the registry the test reads rather than
by inspection, and each names the specific problem:
GREEN passed
RED missing-from-export ['internvl-u'] are registered in MODEL_TYPES/LORA_OVERLAYS
but the export document does not name them — they would ship
with NO SCHEMA
RED extra-in-export ['ghost-family'] are named by the export document but are in
neither MODEL_TYPES nor LORA_OVERLAYS
RED overlay-before-base the export interleaves LoRA overlays with base types
RED empty-registry the model-type registry collapsed to []
THE VACUITY ARM IS THE ONE THAT MATTERS MOST and it is why the non-emptiness checks come
FIRST: set equality against an empty registry is trivially true, so an import that
silently produced no types would have passed every other assertion in this test. It now
fails loudly on anchors that have been in the launch set since it existed.
Net -24 lines, and a new ModelType now has TWO registration sites instead of three. The
3D lane's #996 and every future model-types PR stop conflicting here.
Verified: 80 passed across the two defaults suites; `uv run mypy
tests/test_model_defaults_export.py` clean; ruff clean. The `test_eager_bridge_dtype_
pgw1447.py` mypy errors in the full run are pre-existing — they are present at pristine
master with this change removed, and this diff touches one file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes pgw#1445.
LAUNCH_SET_NAMESwas a serialization point: every ModelType PR had to edit the samehand-maintained list, so two could never be in flight concurrently without one going
stale. Measured on pgw#1432 alone —
origin/mastermoved out from under that branchtwice, 5 commits each time, and the list went stale on both. It went stale for four
lanes in a row: audio, LTX-2, pgw#1432, and master itself.
Paul's minimum-hand-specification ruling, and the same shape the master-green lane applied
to
test_cli_entry_orders: a list is what went stale; a second list goes stale the sameway.
It also failed in the worst available place. The pinned list lives in the full
testsjob, which is red for unrelated reasons — so a lane that added a ModelType, saw
testsred, and diffed the count against the known baseline concluded "not mine" and
enqueued. Only diffing failure names caught it, and nothing forced anyone to.
What is asserted is NOT circular — that is the whole design
export_document()builds its names aslist(defaults_vocabularies())(
defaults_export.py:58). So checking the export against that mapping would test theemitter against its own input and prove nothing.
MODEL_TYPESandLORA_OVERLAYSare adifferent source — the declaration tuples — so the export is checked against those.
What is deliberately NOT asserted: the exact sequence within the base block. That
order is
defaults_vocabularies()insertion order and no independent source defines it —the mapping's order genuinely differs from the tuple's today (the tuple runs
… Flux2Klein, Krea2, Anima, Ernie, QwenImage, ZImage …, the emitter runs… flux2-klein, qwen-image, z-image, krea-2, anima, ernie …). Pinning it would recreatethe hand list this deletes.
The ordering property the docstring actually promises — base types first, then the
overlays — is checked, because that one is structural rather than incidental. Saying
which of the two is checkable is the honest half of the change.
Red/green verified on all four arms
By mutating the registry the test reads, not by inspection. Each names the specific
problem rather than printing a list diff:
['internvl-u'] are registered in MODEL_TYPES/LORA_OVERLAYS but the export document does not name them — they would ship with NO SCHEMA['ghost-family'] are named by the export document but are in neither MODEL_TYPES nor LORA_OVERLAYSthe export interleaves LoRA overlays with base typesthe model-type registry collapsed to []The vacuity arm is the one that matters most, and it is why the non-emptiness checks
come FIRST. Set equality against an empty registry is trivially true, so an import that
silently produced no types would have passed every other assertion in this test. It now
fails loudly on anchors that have been in the launch set since it existed.
Effect
Net -24 lines, and a new ModelType now has two registration sites instead of
three — the tuple and
defaults_vocabularies(), both already fenced by pgw#1001. The 3Dlane's #996 and every future model-types PR stop conflicting here.
Verification
tests/test_model_defaults_export.py+tests/test_model_defaults.pyuv run mypy tests/test_model_defaults_export.py→ clean; ruff → cleantest_eager_bridge_dtype_pgw1447.pymypy errors in a full-tree run arepre-existing: present at pristine
origin/masterwith this change removed, and thisdiff touches exactly one file.