From 3e43b43ec8972923e09b44f9649049ccf3be4578 Mon Sep 17 00:00:00 2001 From: Curtis Anderson Date: Fri, 24 Jul 2026 07:52:02 -0700 Subject: [PATCH 1/2] =?UTF-8?q?test(checkpointing):=20RED=20=E2=80=94=20?= =?UTF-8?q?=C2=A74.7.1=20gap=20breach=20should=20warn,=20not=20invalidate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Special build for one submitter: a >30s write→read inter-phase gap in a split-mode checkpointing submission must be reported as a warning rather than a hard error, in both the submission checker (cache_flush_validation) and the reportgen-side rules checker (check_invocation_structure). --- .../tests/test_checkpointing_check_phase2.py | 38 +++++++++++-------- tests/unit/test_rules_checkers.py | 35 +++++++++++++++++ 2 files changed, 58 insertions(+), 15 deletions(-) diff --git a/mlpstorage_py/tests/test_checkpointing_check_phase2.py b/mlpstorage_py/tests/test_checkpointing_check_phase2.py index 4e7bd26e..e32f03bc 100644 --- a/mlpstorage_py/tests/test_checkpointing_check_phase2.py +++ b/mlpstorage_py/tests/test_checkpointing_check_phase2.py @@ -166,8 +166,12 @@ def test_split_mode_with_25s_gap_passes(self, tmp_path, mock_logger): assert result is True assert mock_logger.errors == [] - def test_split_mode_with_45s_gap_emits_4_7_1(self, tmp_path, mock_logger): - """Split-mode with 45s gap → [4.7.1 checkpointCacheFlushValidation] violation.""" + def test_split_mode_with_45s_gap_warns_not_fails(self, tmp_path, mock_logger): + """Split-mode with 45s gap → warn_violation, not a hard failure. + + Special-build relaxation: a §4.7.1 gap breach is reported as a + warning so the submission is not invalidated. + """ from mlpstorage_py.tests.conftest import build_submission root = build_submission( tmp_path, @@ -177,11 +181,13 @@ def test_split_mode_with_45s_gap_emits_4_7_1(self, tmp_path, mock_logger): ) check = _run_checkpointing_check(root, mock_logger) result = check.cache_flush_validation() - assert result is False - assert len(mock_logger.errors) >= 1 - assert mock_logger.errors[0].startswith("[4.7.1 checkpointCacheFlushValidation]"), \ - f"Expected [4.7.1 checkpointCacheFlushValidation]; got {mock_logger.errors[0]!r}" - assert "30-second limit" in mock_logger.errors[0] + assert result is True + assert mock_logger.errors == [] + assert any( + w.startswith("[4.7.1 checkpointCacheFlushValidation]") + and "exceeds 30-second limit" in w + for w in mock_logger.warnings + ), f"Expected 30-second-limit warning; got warnings={mock_logger.warnings!r}" def test_split_mode_missing_timestamps_emits_4_7_1(self, tmp_path, mock_logger): """Split-mode without write summary end_time → [4.7.1] 'missing end_time'.""" @@ -298,9 +304,10 @@ def test_split_mode_missing_invocation_end_time_falls_back_to_summary_end(self, teardown window is charged to the failover-callout budget — reproducing the original #782 bug on older results dirs. - 45s teardown + 25s real gap ⇒ legacy gap = 70s ⇒ FAIL (matches - pre-fix behavior). The INFO line must record the "write summary end" - origin so the submitter can see why the gap looks so large. + 45s teardown + 25s real gap ⇒ legacy gap = 70s ⇒ warning + (special-build relaxation: gap breaches never invalidate). The INFO + line must record the "write summary end" origin so the submitter can + see why the gap looks so large. """ from mlpstorage_py.tests.conftest import build_submission root = build_submission( @@ -313,12 +320,13 @@ def test_split_mode_missing_invocation_end_time_falls_back_to_summary_end(self, ) check = _run_checkpointing_check(root, mock_logger) result = check.cache_flush_validation() - assert result is False + assert result is True + assert mock_logger.errors == [] assert any( - e.startswith("[4.7.1 checkpointCacheFlushValidation]") - and "exceeds 30-second limit" in e - for e in mock_logger.errors - ), f"Expected 30-second-limit violation; got errors={mock_logger.errors!r}" + w.startswith("[4.7.1 checkpointCacheFlushValidation]") + and "exceeds 30-second limit" in w + for w in mock_logger.warnings + ), f"Expected 30-second-limit warning; got warnings={mock_logger.warnings!r}" # INFO line must record the fallback origin. assert any( "[4.7.1 checkpointCacheFlushValidation]" in m diff --git a/tests/unit/test_rules_checkers.py b/tests/unit/test_rules_checkers.py index c47db9eb..85c96e29 100755 --- a/tests/unit/test_rules_checkers.py +++ b/tests/unit/test_rules_checkers.py @@ -1004,6 +1004,41 @@ def test_invocation_end_time_bookend_preferred_over_proxy(self, mock_logger): # 07:18:15 - 07:18:05 = 10 s, from the bookends (not the 071800 summary). assert any("10.0s inter-phase gap" in i.message for i in closed) + def test_gap_over_30s_is_warning_not_invalid(self, mock_logger): + """Special-build relaxation: a >30s inter-phase gap must not + invalidate the submission — it is reported as a warning-severity + CLOSED issue instead of INVALID. + """ + write_run = self._run( + mock_logger, + num_reads=0, num_writes=10, + run_datetime="20260715_071700", + end_datetime="20260715_071800", + invocation_end_time="2026-07-15T07:18:00", + ) + read_run = self._run( + mock_logger, + num_reads=10, num_writes=0, + run_datetime="20260715_071900", + invocation_start_time="2026-07-15T07:19:00", # 60 s gap + ) + + checker = CheckpointSubmissionRulesChecker( + [write_run, read_run], logger=mock_logger + ) + issues = checker.check_invocation_structure() + + invalid = [i for i in issues if i.validation == PARAM_VALIDATION.INVALID] + assert invalid == [], ( + f"Gap breach must be a warning, not INVALID; got: " + f"{[i.message for i in invalid]}" + ) + warnings = [i for i in issues if i.severity == "warning"] + assert any( + "exceeding" in i.message and "60.0" in i.actual + for i in warnings + ), f"Expected warning-severity gap issue; got issues={issues!r}" + class TestRulesCheckerInitialization: """Tests for rules checker initialization order. From c45278c4da9345cc2c8aff1ffbba1d23e599e24d Mon Sep 17 00:00:00 2001 From: Curtis Anderson Date: Fri, 24 Jul 2026 07:54:49 -0700 Subject: [PATCH 2/2] =?UTF-8?q?feat(checkpointing):=20special=20build=20?= =?UTF-8?q?=E2=80=94=20=C2=A74.7.1=20gap=20breach=20warns=20instead=20of?= =?UTF-8?q?=20invalidating?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Downgrade the >30s write→read inter-phase gap breach from a hard error to a warning in both enforcement points: - submission checker cache_flush_validation: warn_violation instead of log_violation; the check no longer returns False for a gap breach - reportgen-side check_invocation_structure: warning-severity CLOSED issue instead of PARAM_VALIDATION.INVALID Missing/unparseable timestamps and overlapping phases remain hard errors. One-off build for a single submitter; not intended for merge. --- .../rules/submission_checkers/checkpointing.py | 15 +++++++++++---- .../checks/checkpointing_checks.py | 14 +++++++++----- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/mlpstorage_py/rules/submission_checkers/checkpointing.py b/mlpstorage_py/rules/submission_checkers/checkpointing.py index 602a2712..f24aa320 100755 --- a/mlpstorage_py/rules/submission_checkers/checkpointing.py +++ b/mlpstorage_py/rules/submission_checkers/checkpointing.py @@ -134,8 +134,10 @@ def check_invocation_structure(self) -> List[Issue]: within MAX_INTER_PHASE_GAP_SECONDS by a second invocation with 0 writes / 10 reads (the gap covers the cache flush). - Any other arrangement (e.g. ten 1-write runs, overlapping phases, or - a >30s gap between the write and read phases) is INVALID for CLOSED. + Any other arrangement (e.g. ten 1-write runs or overlapping phases) + is INVALID for CLOSED. Special-build relaxation: a >30s gap between + the write and read phases is reported as a warning-severity issue + rather than INVALID. """ issues = [] @@ -268,16 +270,21 @@ def _wr(run): return issues if gap_seconds > MAX_INTER_PHASE_GAP_SECONDS: + # Special-build relaxation: a gap breach is reported as a + # warning-severity issue rather than INVALID, so it never + # invalidates the submission. issues.append(Issue( - validation=PARAM_VALIDATION.INVALID, + validation=PARAM_VALIDATION.CLOSED, message=( f"Gap between write-phase end and read-phase start is " f"{gap_seconds:.1f}s, exceeding the {MAX_INTER_PHASE_GAP_SECONDS}s " - "maximum required by Rules.md §4.7.1." + "maximum required by Rules.md §4.7.1 (accepted with a " + "warning in this build)." ), parameter="checkpoint.invocation_structure", expected=f"≤ {MAX_INTER_PHASE_GAP_SECONDS}s", actual=f"{gap_seconds:.1f}s", + severity="warning", )) return issues diff --git a/mlpstorage_py/submission_checker/checks/checkpointing_checks.py b/mlpstorage_py/submission_checker/checks/checkpointing_checks.py index a855c48b..43c7b888 100644 --- a/mlpstorage_py/submission_checker/checks/checkpointing_checks.py +++ b/mlpstorage_py/submission_checker/checks/checkpointing_checks.py @@ -641,9 +641,12 @@ def cache_flush_validation(self): record per-node collection timestamps); otherwise ``write.summary.end_time`` (predates both fixes). The summary-end fallback still includes the full post-benchmark collection window - in the gap, so the check emits a normal violation on breach but - the submitter should re-run with a current mlpstorage to get the - honest measurement. + in the gap; the submitter should re-run with a current mlpstorage + to get the honest measurement. + + **Special-build relaxation** — a gap breach (>30s) is reported as a + ``warn_violation`` rather than a hard failure, so it never + invalidates the submission. A negative gap indicates NTP skew between the write and read nodes (the metadata fields are timestamped on their respective invocation @@ -768,7 +771,9 @@ def cache_flush_validation(self): gap_seconds, write_end, read_start, ) continue - self.log_violation( + # Special-build relaxation: a gap breach is reported as a + # warning rather than invalidating the submission. + self.warn_violation( "4.7.1", "checkpointCacheFlushValidation", self.path, "failover-callout gap %.1f seconds exceeds 30-second limit " "(%s=%s, %s=%s)", @@ -776,7 +781,6 @@ def cache_flush_validation(self): write_origin_label, write_end, read_origin_label, read_start, ) - valid = False return valid @rule("4.7.1", "checkpointCacheFlushValidation")