Skip to content

#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
masterfrom
1445-derive-launch-set
Open

#1445: derive the launch set from the registry — delete the hand list and the serialization point with it#1013
PaulFidika wants to merge 1 commit into
masterfrom
1445-derive-launch-set

Conversation

@PaulFidika

Copy link
Copy Markdown
Contributor

Closes pgw#1445.

LAUNCH_SET_NAMES was a serialization point: every ModelType PR had to edit the same
hand-maintained list, so two could never be in flight concurrently without one going
stale. Measured on pgw#1432 alone — origin/master moved out from under that branch
twice, 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 same
way.

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 — 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 (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 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, not by inspection. Each names the specific
problem rather than printing a list diff:

arm message
GREEN passed
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
extra-in-export ['ghost-family'] are named by the export document but are in neither MODEL_TYPES nor LORA_OVERLAYS
overlay-before-base the export interleaves LoRA overlays with base types
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.

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 3D
lane's #996 and every future model-types PR stop conflicting here.

Verification

  • 80 passed across tests/test_model_defaults_export.py + tests/test_model_defaults.py
  • uv run mypy tests/test_model_defaults_export.py → clean; ruff → clean
  • The test_eager_bridge_dtype_pgw1447.py mypy errors in a full-tree run are
    pre-existing: present at pristine origin/master with this change removed, and this
    diff touches exactly one file.

… 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.
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