feat(#242): roster conformance for 7 models, retire the parity suite (S3, split from #367) - #369
Merged
Merged
Conversation
Epic #242 S3, split out of PR #367 (self-disclosed in #368). This lands the model-side roster work and nothing else: no ensemble deletions, no ADR amendment, no register edit, no C-132 wiring. WHY THE PARITY SUITE DIES HERE AND NOT SEPARATELY. test_datafactory_parity.py and the roster assert mutually exclusive facts -- the old suite requires loss_reg == "tobit" (:161), a non-empty loss_reg_sigma (:166-170) and loss_class == "focal" (:155-157); the roster foundation sets mse, no sigma and weighted_bce. They cannot both be green, so the configs and the deletion are one change. Its premise is also gone: S2 migrated the viewser trio, so there are no longer two trios differing only in data source. VIOLET_VISITOR IS EXEMPT, NOT RECONFIGURED. It is fenced by EXPERIMENT_IN_PROGRESS (commit e1674af), it is mid-experiment, and the maintainer has uncommitted work in its config_queryset.py. Its two config files are not touched. The roster still declares all eight -- being mid-experiment excuses a model's values, not its membership. THE FENCE IS RE-HOMED, NOT REMOVED. test_datafactory_parity.py was the only reader of EXPERIMENT_IN_PROGRESS anywhere in the repo, so deleting it would have made the marker dead code. The set-pin comes across verbatim in spirit: EXPERIMENTS_IN_PROGRESS = {"violet_visitor"}, asserted exactly, so both adding and removing an exemption is a reviewed edit. PR #367 instead asserted the marker is ABSENT, which converts an escape hatch into a prohibition. THE RUSTY_BUCKET REWIRING IS DEFERRED, and this is a consequence of the exemption rather than a preference: the ensemble declares expected_samples_per_model 16, the seven pinned members emit D×K = 4×4 = 16, and violet emits 8 (D=8, no head sampler). Swapping the eight temporary_* stand-ins in now fails the ADR-015 contract test -- correctly. The ensemble is rewired in the change that settles violet. Its modelset is left as development has it. Also here: conftest.get_head_sample_count / get_produced_sample_count (K defaults to 1, so every non-family model is byte-identical), and the four call sites that should compare against the produced width rather than D alone. That change makes test_pfe_production_readiness's violet_visitor_calibration case pass -- it was red on development because the expected width was computed as D when the artifact carries D×K. Departures from PR #367, deliberate: test_roster_is_fully_pinned dropped; the fleet-wide C-132 guard and rusty_bucket's classification_targets held back for the C-132 change (gated on views-pipeline-core#422); _load fails instead of skipping for roster members; cross-repo issue refs qualified; rusty_bucket's config_hyperparameters left alone (PR #367 stripped 14 lines of rationale including why the pool was thinned 1024 -> 128 for a 28.6 GB memory ceiling). Full suite: 7747 passed, 219 skipped, 6 xfailed, 1 xpassed, 0 failed.
CI caught what my local run could not: violet_visitor's committed config_queryset.py is still a pure viewser queryset -- no views-datafactory source, no africa_me_legacy. S2 (#365) migrated the other three viewser models and skipped the one that was mid-experiment. I asserted otherwise because I probed the WORKING TREE, where the maintainer has an uncommitted migration of exactly that file. That is the C-110 mistake: a green local result said nothing about the repository, because the thing I measured is not committed. test_uses_datafactory and test_africa_region now scope to PINNED_MODELS, alongside test_no_viewser_import, which was already exempt. The whole source migration is part of settling the model, not a separate obligation. test_declares_ged_features stays on all eight -- the GED feature names are source-independent and hold for the committed violet queryset too. Verified against committed state with tools/audit/verify_committed.sh rather than the working tree.
This was referenced Aug 10, 2026
Merged
Open
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.
Epic #242 S3, split out of #367 (self-disclosed in #368). Model-side roster work only — no ensemble deletions, no ADR amendment, no register edit, no C-132 wiring. Those follow in their own PRs.
Why the parity suite is deleted here rather than separately
My earlier review proposed landing the roster first and retiring
test_datafactory_parity.pyafterwards, with both suites green together as the evidence. That is impossible — they assert mutually exclusive facts::161loss_reg == {"tobit"}FOUNDATIONsetsmse:166-170non-emptyloss_reg_sigmatest_no_tobit_sigmarequires it absent:155-157loss_class == "focal"FOUNDATIONsetsweighted_bceIts premise is also gone: S2 (#365) migrated the viewser trio, so there are no longer two trios differing only in data source.
violet_visitor is exempt, not reconfigured
Its two config files are not touched. It is fenced by
EXPERIMENT_IN_PROGRESS(e1674af9), mid-experiment, and there is uncommitted maintainer work in itsconfig_queryset.py. The roster still declares all eight — being mid-experiment excuses a model's values, not its membership.The fence is re-homed, not removed.
test_datafactory_parity.pywas the only reader ofEXPERIMENT_IN_PROGRESSanywhere in the repo, so deleting it would have made the marker dead code. The set-pin comes across:EXPERIMENTS_IN_PROGRESS = {"violet_visitor"}, asserted exactly, so adding or removing an exemption is a reviewed edit — plus a non-vacuity guard, since pins over an empty parametrisation assert nothing. #367 instead asserted the marker is absent, converting an escape hatch into a prohibition.Violet is exempt from the value pins (family, composition, gate, seed,
FOUNDATION) and from the viewser-import check — it declares a datafactory source and still imports viewser, a pre-S2 leftover whose removal is an edit to a fenced config. It remains subject to every structural check: grid, targets, gate channel, region, GED features, and metadata. Those are not what a loss experiment churns.The rusty_bucket rewiring is deferred
A consequence of the exemption, not a preference. The ensemble declares
expected_samples_per_model: 16; the seven pinned members emit D×K = 4×4 = 16; violet emits 8 (D=8, no head sampler). Swapping the eighttemporary_*stand-ins in now failstest_ensemble_configs.py::test_declared_modelset_and_sample_counts_match_reality— correctly. Its modelset is left asdevelopmenthas it, and the ensemble is rewired in the change that settles violet. The reason is recorded in the test file so the next reader does not re-derive it.D×K
conftest.get_head_sample_count/get_produced_sample_count— K defaults to 1, so every non-family model is byte-identical — plus the four call sites that should compare against the produced width rather than D alone.Side effect worth naming: this makes
test_pfe_production_readiness'sviolet_visitor_calibrationcase pass. It was red ondevelopmentbecause the expected width was computed as D while the artifact carries D×K.Departures from #367
test_roster_is_fully_pinneddropped (asserted the fence is absent)rusty_bucket'sclassification_targetsheld back — gated on views-pipeline-core#422 merging and a release being pinned here (CI installsviews_pipeline_core==3.0.0exactly, so the>=3.0.0,<4.0.0range will not pull it)_loadfails instead ofpytest.skipfor roster members — skip-on-absent is green-by-silenceviews-hydranet#246,views-pipeline-core#422) — bare#numbers resolve to unrelated issues hereensembles/rusty_bucket/configs/config_hyperparameters.pyleft alone — Epic #242 S3+S4: HydraNet roster reconfig + roster-conformance + D×K sample-count contract #367 collapsed it to one line, stripping 14 lines of rationale including why the pool was thinned 1024 → 128 (a 28.6 GB memory ceiling, 2026-07-20)Verification
ruff check .cleantest_ensemble_guard_uses_supported_vocabulary) isstrict=False, reads the installed pipeline-core rather than repo files, and is untouched by this branchmodels/violet_visitor/path in the diff