test: pin TODO-002 retirement contract (depends on #432)#438
Draft
FileSystemGuy wants to merge 1 commit into
Draft
test: pin TODO-002 retirement contract (depends on #432)#438FileSystemGuy wants to merge 1 commit into
FileSystemGuy wants to merge 1 commit into
Conversation
Pins the contract that ties the three deferred-runtime-cross-check stubs together (3.3.5 trainingDistributedDataAccessibility, 3.3.7 trainingNodeCapabilityConsistency, 4.7.4 checkpointSimultaneousRwSupport), so a partial retirement breaks loudly. Five parametrized assertions, expanded across the three (rule_id, class, method) triples (11 cases total): 1. @rule decoration remains in place and discover_rules() finds it - catches "I deleted the method" or "I forgot the decorator after upgrading the body". 2. Method source still contains the TODO-002 marker - the marker IS the deferral signal; dropping it is the explicit retirement act. Failure message walks the retirer through the 7-point retirement checklist documented in the module docstring. 3. Method source does not contain log_violation - catches the inconsistent half-retirement: violation call added but TODO-002 marker still present. 4. SCHEMA_ERROR_RULE_MAP retains the 4.7.4 schema anchors (simultaneous_write / simultaneous_read / multi_host) - locks the schema anchor in place so retiring the runtime stub can't accidentally take the schema-anchor coverage with it. 5. No surprise schema anchors appear for rules that didn't declare one - forward-looking: 3.3.5 / 3.3.7 have no schema anchor today; if someone adds one as defense-in-depth, the test fails until they register it in EXPECTED_SCHEMA_ANCHORS, locking it in as part of the contract. The split from the existing per-method pinning tests (TestChkpt05DeferredFollowUp in test_checkpointing_check_phase2.py, test_3_3_5_* / test_3_3_7_* in test_training_check_retrofit.py) is deliberate: those test the current body emits info; this file tests the contract across all three rules, so partial retirement still trips one of the cross-rule invariants. DRAFT: depends on PR #432 (FileSystemGuy-rules-validator) landing first — main does not yet have rule_registry.py or system_yaml_schema_checks.py, both of which this test imports. Will become mergeable cleanly after #432 merges.
|
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
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.
Summary
Adds a tripwire-style contract test that locks in the retirement plan for the three deferred-runtime-cross-check stubs tagged
TODO-002:3.3.5 trainingDistributedDataAccessibilityTrainingCheckdistributed_data_accessibility_check3.3.7 trainingNodeCapabilityConsistencyTrainingChecknode_capability_consistency_check4.7.4 checkpointSimultaneousRwSupportCheckpointingChecksimultaneous_rw_supportEach stub currently emits an INFO-level note containing the marker
TODO-002, returnsTrue, and contributes no violation, becausesummary.jsondoesn't expose the per-host data the real cross-check would need. For4.7.4the structural anchor isSystemYamlSchemaCheck.SCHEMA_ERROR_RULE_MAP's three field-level entries (simultaneous_write,simultaneous_read,multi_host);3.3.5/3.3.7have no schema anchor and rely entirely on the deferred runtime stub.What the test pins
Five parametrized assertions (11 cases total) covering both the runtime side and the schema-anchor side:
@rule(...)is still on the method anddiscover_rules()still lists the ID. Catches "I deleted the method" or "I forgot to redecorate after upgrading the body" — without this,mlpstorage rules-coveragesilently drops the rule.TODO-002marker is present in the method source — the marker is the deferral signal. Removing it is the explicit retirement act; when it disappears, the failure message walks the retirer through the 7-point retirement checklist in the module docstring.log_violationwhile deferred — catches the half-retirement: violation call added butTODO-002marker still present.4.7.4SCHEMA_ERROR_RULE_MAPentries in place so retiring the runtime stub can't accidentally take the schema-anchor coverage with it.3.3.5/3.3.7have no anchor today; if someone adds one as defense in depth, the test fails until they register it inEXPECTED_SCHEMA_ANCHORS, making it part of the contract.Why a separate file from the existing pin tests
The existing per-method pin tests (
TestChkpt05DeferredFollowUpintest_checkpointing_check_phase2.py,test_3_3_5_*/test_3_3_7_*intest_training_check_retrofit.py) test that the current body emits info. This file tests the contract across all three rules, so partial retirement still trips one of the cross-rule invariants. The split is intentional — anyone touching a stub has to come through this file, which doubles as the retirement checklist.Dependency on PR #432
This PR is intentionally a draft. The test imports symbols (
rule_registry,system_yaml_schema_checks.SCHEMA_ERROR_RULE_MAP) that don't exist onmainyet — they land with PR #432 (FileSystemGuy-rules-validator). Until that merges, CI on this branch will fail at import time.Sequencing:
main→rule_registry,SystemYamlSchemaCheck, and the three deferred stubs all land.main, mark Ready for review, merge.Test plan
FileSystemGuy-rules-validator(which has all dependencies):python3 -m pytest mlpstorage_py/tests/test_todo_002_retirement_contract.py -v→ 11 passed, 0 failed.mainonce PR Submission checker: complete Rules.md coverage + 'mlpstorage validate' subcommand #432 lands.