test(ADR-012): assert the lr_/by_ target prefixes — the guard the ADR asked for and never got - #375
Merged
Merged
Conversation
… the ADR asked ADR-012 proposed this guard itself -- "Existing tests can be extended to assert that all regression_targets use the lr_ prefix and all classification_targets use the by_ prefix" -- and it was never written. Nothing checked either prefix. Same family as views-models#367: a targets declaration no test inspected. #374 closed the metric half by loading every config through pipeline-core's CoreConfigSniffer; the sniffer validates the targets<->metrics PAIRING and never the target NAMES, so this closes the other half. Measured across every non-fixture source: 190 regression targets, all lr_; 24 classification targets, all by_. The convention already holds everywhere -- this pins it rather than repairing it. Fixtures are excluded via meta/fixtures.json, loaded not hardcoded (the rule tools/partitions/fileops.py and tools/catalogs/create_catalogs.py follow, pinned by TestFixtureSetConsistency). Nine fixture entities declare a deliberate synth_target. That exclusion is itself pinned, so a REAL model adopting synth_ still fails -- the exclusion is a declared fact, not a circular one. Own file rather than an addition to test_config_completeness.py: that file asks whether required keys are present, this asks whether the values are well-formed. Also records that both ADR-proposed guards now exist: - ADR-012's implementation note, with the three-month gap stated - ADR-009's conventions table gains the targets<->metrics row, attributing the rule to pipeline-core's sniffer rather than restating it, so the two cannot drift. The table previously mentioned no targets or metrics keys at all -- the gap was wider than one missing key. Verified the guard bites: injecting 'ln_sb_best' into bright_starship turns it red naming the model and the target; removing it turns it green. My first attempt at that check used the wrong quote style and silently changed nothing -- the guard was fine, the verification was the vacuous thing. Full suite: 7753 passed, 219 skipped, 6 xfailed, 1 xpassed, 0 failed.
This was referenced Aug 11, 2026
rusty_bucket: finalize the ~8 constituents, draw counts, and target/level (maintainer decision)
#146
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.
ADR-012 proposed this guard itself — "Existing tests (
test_config_completeness.py) can be extended to assert that allregression_targetsuse thelr_prefix and allclassification_targetsuse theby_prefix" — and it was never written. Nothing checked either prefix, for about three months.Same family as #367: a targets declaration that no test inspected. #374 closed the metric half by loading every config through pipeline-core's
CoreConfigSniffer. The sniffer validates the targets↔metrics pairing and never the target names, so this closes the other half.What the tree already looks like
Measured across every non-fixture source:
regression_targetslr_classification_targetsby_The convention already holds everywhere. This pins it; it does not repair anything.
Fixtures
Nine fixture entities declare a deliberate
synth_target. They are excluded viameta/fixtures.json, loaded rather than hardcoded — the ruletools/partitions/fileops.pyandtools/catalogs/create_catalogs.pyfollow, pinned byTestFixtureSetConsistency(C-61, #99).That exclusion is itself pinned by a test, so it stays a declared fact rather than a circular one: a real model adopting
synth_still fails.Placement
Own file, not an addition to
test_config_completeness.py. That file asks whether required keys are present; this asks whether the values are well-formed. Different question, different file.Both ADRs updated to say the guards exist
Verified it bites
Injecting
'ln_sb_best'intobright_starshipturns it red, naming the model and the offending target; removing it turns it green.My first attempt at that check used
"by_sb_best"while the file uses'by_sb_best', so the injection silently changed nothing and the "test" passed. The guard was fine — the verification was the vacuous thing. Worth recording, because a check that cannot fail looks exactly like a check that passed.Verification
ruff check .clean