2nd: feat(validate): warnings-only submission-flaw checks — 3.3.7 node capability, 4.7.4 simultaneous R/W

mlpstorage_py/submission_checker/checks/checkpointing_checks.py

@@ -1006,17 +1006,43 @@ def simultaneous_rw_support(self):
         ``SCHEMA_ERROR_RULE_MAP`` tagged with 4.7.4 — those catch
         declaration-side defects that surface before any benchmark runs).
 
-        The rule body preserves the ``@rule`` binding for coverage
-        discovery and emits an INFO line so tooling that greps by rule
-        ID surfaces the rule as "visited and satisfied".
+        Warnings-only enforcement (never fails — submission-window
+        doctrine): a completed checkpointing run has already demonstrated
+        simultaneous R/W on the shared namespace via the CAP-02 probe, so a
+        system description that declares either capability as ``false``
+        CONTRADICTS the run it accompanies. The validator surfaces that
+        inconsistency to reviewers via ``warn_violation`` and still returns
+        valid. Missing capability fields are owned by SystemYamlSchemaCheck
+        (D-A3) and are silent-skipped here to avoid double-emit.
         """
         valid = True
         if self.mode != "checkpointing":
             return valid
         sim_write = self._get_capability("simultaneous_write")
         sim_read = self._get_capability("simultaneous_read")
         if sim_write is None or sim_read is None:
+            # SystemYamlSchemaCheck owns the missing-field violation (D-A3).
             return valid
+        # An explicitly-declared ``false`` contradicts the completed run.
+        denied = [
+            name
+            for name, value in (
+                ("simultaneous_write", sim_write),
+                ("simultaneous_read", sim_read),
+            )
+            if value is False
+        ]
+        if denied:
+            self.warn_violation(
+                "4.7.4", "checkpointSimultaneousRwSupport", self.path,
+                "system YAML declares %s = false, but a completed "
+                "checkpointing run requires simultaneous read/write on a "
+                "shared namespace (Rules.md 4.7.4); the system description "
+                "is likely incorrect.",
+                " and ".join(denied),
+            )
+            return valid
+        # Both capabilities declared-supported — satisfied by construction.
         self.log.info(
             "[4.7.4 checkpointSimultaneousRwSupport] %s: "
             "satisfied by construction — CAP-02 shared-FS probe "

mlpstorage_py/submission_checker/checks/training_checks.py

@@ -692,34 +692,81 @@ def identical_accelerators_per_node(self):
 
         return valid
 
+    # Above this per-host max/min capability ratio, the physical nodes are
+    # "widely enough different" to warrant reviewer attention per Rules.md
+    # 3.3.7. Advisory only — a heterogeneous cluster is not itself illegal.
+    _NODE_CAPABILITY_DIVERGENCE_RATIO = 1.5
+
     @rule("3.3.7", "trainingNodeCapabilityConsistency")
     def node_capability_consistency_check(self):
-        """Rules.md 3.3.7 — node capability consistency (advisory).
-
-        Satisfied by construction at runtime: the cluster collector
-        captures per-host system information twice per run — once at
-        run start (``Benchmark._collect_cluster_start`` in
-        ``benchmarks/base.py:646``) and once at run end
-        (``_collect_cluster_end`` at ``:674``) — and stores both
-        snapshots in ``metadata['cluster_snapshots']`` via
-        ``rules.models.ClusterSnapshots.as_dict()``. Any component drift
-        during the run (CPU count, memory, network ports, kernel, etc.)
-        is therefore captured in the artifact tree; the cluster collector
-        also emits a warning on significant drift so operators can spot
-        stability issues before submission.
-
-        The rule body preserves the ``@rule`` binding for coverage
-        discovery and emits an INFO line so tooling that greps by rule
-        ID surfaces the rule as "visited and satisfied".
+        """Rules.md 3.3.7 — node capability consistency (warnings-only).
+
+        Rules.md 3.3.7 asks the validator to **warn (not fail)** when the
+        physical nodes in a distributed submission differ widely in
+        capability. Each run's ``metadata['cluster_information']`` (the
+        ``ClusterInformation.as_dict`` snapshot) carries per-host memory and
+        CPU-core vectors plus any collector-recorded
+        ``host_consistency_issues``. This check assesses the first run that
+        reports a usable multi-host cluster and emits a
+        ``warn_violation`` when the per-host max/min ratio for memory or CPU
+        cores exceeds ``_NODE_CAPABILITY_DIVERGENCE_RATIO``, or when the
+        collector already flagged inconsistencies. It always returns valid —
+        node heterogeneity is advisory, never a hard failure (submission-
+        window doctrine).
+
+        No cluster_information, or a single-host cluster, is silently
+        passed — there is nothing to compare.
         """
-        self.log.info(
-            "[3.3.7 trainingNodeCapabilityConsistency] %s: "
-            "satisfied by construction — cluster collector captures "
-            "start/end cluster snapshots (Benchmark._collect_cluster_start / "
-            "_collect_cluster_end); component drift is surfaced at runtime",
-            self.path,
-        )
-        return True
+        valid = True
+        if self.mode != "training":
+            return valid
+
+        for _summary, metadata, ts in self.submissions_logs.run_files:
+            if metadata is None:
+                continue
+            cluster = metadata.get("cluster_information")
+            if not isinstance(cluster, dict):
+                continue
+            hosts = cluster.get("hosts")
+            if not isinstance(hosts, list) or len(hosts) < 2:
+                continue
+
+            # Per-host capability vectors. Missing/partial fields are simply
+            # excluded from the comparison for that metric.
+            mem_totals = [
+                (h.get("memory") or {}).get("total")
+                for h in hosts if isinstance(h, dict)
+            ]
+            core_counts = [
+                (h.get("cpu") or {}).get("num_cores")
+                for h in hosts if isinstance(h, dict)
+            ]
+            for label, values in (("memory", mem_totals), ("CPU cores", core_counts)):
+                nums = [v for v in values if isinstance(v, (int, float)) and v > 0]
+                if len(nums) < 2:
+                    continue
+                lo, hi = min(nums), max(nums)
+                if hi / lo > self._NODE_CAPABILITY_DIVERGENCE_RATIO:
+                    self.warn_violation(
+                        "3.3.7", "trainingNodeCapabilityConsistency", self.path,
+                        "run %s: hosts differ widely in %s — min=%s, max=%s "
+                        "(%.2fx); confirm the physical nodes are intended to "
+                        "be heterogeneous (Rules.md 3.3.7).",
+                        ts, label, lo, hi, hi / lo,
+                    )
+
+            issues = cluster.get("host_consistency_issues")
+            if issues:
+                self.warn_violation(
+                    "3.3.7", "trainingNodeCapabilityConsistency", self.path,
+                    "run %s: cluster collector flagged host inconsistencies: %s",
+                    ts, "; ".join(str(i) for i in issues),
+                )
+
+            # Node capability is a property of the physical cluster, identical
+            # across the measured runs — one representative run suffices.
+            return valid
+        return valid
 
     @rule("3.6.1", "trainingClosedSubmissionChecksum")
     def closed_submission_checksum(self):

mlpstorage_py/tests/conftest.py

@@ -459,6 +459,7 @@ def build_submission(tmp_path, **overrides) -> Path:
     run_logfile_df_block = overrides.pop("run_logfile_df_block", None)
     chkpt_logfile_df_block = overrides.pop("chkpt_logfile_df_block", None)
     run_metadata_hosts = overrides.pop("run_metadata_hosts", None)
+    run_metadata_cluster_information = overrides.pop("run_metadata_cluster_information", None)
     chkpt_summary_timestamps = overrides.pop("chkpt_summary_timestamps", False)
     chkpt_split_mode = overrides.pop("chkpt_split_mode", False)
     chkpt_cache_flush_gap_seconds = overrides.pop("chkpt_cache_flush_gap_seconds", 25)
@@ -706,6 +707,11 @@ def build_submission(tmp_path, **overrides) -> Path:
                     run_meta["args"]["data_dir"] = run_data_dir
                 if run_results_dir is not None:
                     run_meta["args"]["results_dir"] = run_results_dir
+                # 3.3.7: inject a cluster_information block (ClusterInformation
+                # .as_dict shape) so node-capability-consistency tests can
+                # exercise divergent / uniform per-host capability vectors.
+                if run_metadata_cluster_information is not None:
+                    run_meta["cluster_information"] = run_metadata_cluster_information
                 (ts_dir / "metadata.json").write_text(
                     json.dumps(run_meta), encoding="utf-8"
                 )

mlpstorage_py/tests/test_checkpointing_check_phase2.py

@@ -595,18 +595,27 @@ class TestChkpt05_SimultaneousRwSupport:
     """
 
     def test_both_true_no_emission(self, tmp_path, mock_logger):
-        """Default capabilities (sim_write=True, sim_read=True) → True, no errors, info emitted."""
+        """Default capabilities (sim_write=True, sim_read=True) → True, no errors/warnings, info emitted."""
         from mlpstorage_py.tests.conftest import build_submission
         root = build_submission(tmp_path)
         check = _run_checkpointing_check(root, mock_logger)
         result = check.simultaneous_rw_support()
         assert result is True
         assert mock_logger.errors == []
-        # Default: emits info with the "satisfied by construction" marker
+        # Both capabilities declared-supported → satisfied by construction:
+        # info emitted, and NO false-positive warning.
+        assert mock_logger.warnings == []
         assert len(mock_logger.infos) >= 1
 
-    def test_sim_write_false_emits_info(self, tmp_path, mock_logger):
-        """sim_write=False → returns True, emits info with [4.7.4 ...] prefix."""
+    def test_sim_write_false_emits_warning(self, tmp_path, mock_logger):
+        """sim_write=False → warnings-only: returns True, emits a [4.7.4 ...] WARNING.
+
+        A completed checkpointing run has already demonstrated simultaneous
+        R/W via the CAP-02 runtime probe, so a system description that
+        declares simultaneous_write=false contradicts the run it accompanies
+        (Rules.md 4.7.4). Surface that inconsistency to reviewers as a
+        warning — never a hard failure.
+        """
         from mlpstorage_py.tests.conftest import build_submission
         root = build_submission(
             tmp_path,
@@ -618,14 +627,17 @@ def test_sim_write_false_emits_info(self, tmp_path, mock_logger):
         assert mock_logger.errors == []
         assert any(
             m.startswith("[4.7.4 checkpointSimultaneousRwSupport]")
-            for m in mock_logger.infos
-        ), f"Expected info starting with [4.7.4 ...]; got {mock_logger.infos}"
+            and "simultaneous_write" in m
+            for m in mock_logger.warnings
+        ), f"Expected a warning starting with [4.7.4 ...]; got {mock_logger.warnings}"
 
     def test_missing_field_silently_passes(self, tmp_path, mock_logger):
-        """Missing simultaneous_write field → silent-pass per D-A3.
+        """Missing simultaneous_write field → silent-pass, NO warning, per D-A3.
 
-        SystemYamlSchemaCheck handles the missing-field violation.
-        CHKPT-05 silent-skips when _get_capability returns None.
+        SystemYamlSchemaCheck owns the missing-field violation; 4.7.4
+        silent-skips when _get_capability returns None to avoid double-emit.
+        The warnings-only inconsistency check fires only for an explicitly
+        declared ``false``, not for an absent field.
         """
         from mlpstorage_py.tests.conftest import build_submission
         root = build_submission(
@@ -637,6 +649,7 @@ def test_missing_field_silently_passes(self, tmp_path, mock_logger):
         result = check.simultaneous_rw_support()
         assert result is True
         assert mock_logger.errors == []
+        assert mock_logger.warnings == []
 
 
 # ---------------------------------------------------------------------------
@@ -880,13 +893,13 @@ class TestChkpt05SatisfiedByConstruction:
     check trips these assertions.
     """
 
-    def test_sim_write_false_emits_info_with_satisfied_marker(self, tmp_path, mock_logger):
-        """sim_write=False → log.info with 'satisfied by construction' marker; no errors.
+    def test_sim_write_false_emits_warning_not_error(self, tmp_path, mock_logger):
+        """sim_write=False → a [4.7.4 ...] WARNING (not an error, not an info-satisfied line).
 
-        The rule doesn't inspect the capability values to decide pass/fail —
-        the runtime gate (CAP-02) has already enforced the invariant before
-        the validator runs. The capability values are still logged for
-        auditability.
+        The rule never fails the submission (warnings-only): the runtime
+        gate (CAP-02) already enforced the invariant. But a declared
+        simultaneous_write=false contradicts that, so the validator surfaces
+        it to reviewers as a warning rather than mislabelling it "satisfied".
         """
         from mlpstorage_py.tests.conftest import build_submission
         root = build_submission(
@@ -897,23 +910,23 @@ def test_sim_write_false_emits_info_with_satisfied_marker(self, tmp_path, mock_l
         result = check.simultaneous_rw_support()
         assert result is True
         assert mock_logger.errors == [], \
-            f"CHKPT-05 must not emit errors (satisfied by CAP-02 at runtime); got {mock_logger.errors}"
-        matching_infos = [
-            m for m in mock_logger.infos
+            f"4.7.4 must not emit errors (warnings-only); got {mock_logger.errors}"
+        matching_warnings = [
+            m for m in mock_logger.warnings
             if "[4.7.4 checkpointSimultaneousRwSupport]" in m
-            and "satisfied by construction" in m
         ]
-        assert len(matching_infos) >= 1, (
-            f"Expected at least one info message with [4.7.4 ...] and "
-            f"'satisfied by construction' marker; got infos: {mock_logger.infos}"
+        assert len(matching_warnings) >= 1, (
+            f"Expected at least one warning with [4.7.4 ...]; "
+            f"got warnings: {mock_logger.warnings}"
         )
 
     def test_returns_true_regardless_of_capability_values(self, tmp_path, mock_logger):
-        """CHKPT-05 returns True regardless of declared capability values.
+        """4.7.4 returns True regardless of declared capability values (warnings-only).
 
         The runtime gate (CAP-02 shared-FS probe) is the authoritative
-        enforcement point; declared simultaneous_write / simultaneous_read
-        values are audit-only from the validator's perspective.
+        enforcement point, so the validator never fails. When both
+        simultaneous_write and simultaneous_read are declared false, it emits
+        a warning naming both but still returns True.
         """
         from mlpstorage_py.tests.conftest import build_submission
         root = build_submission(
@@ -923,4 +936,10 @@ def test_returns_true_regardless_of_capability_values(self, tmp_path, mock_logge
         check = _run_checkpointing_check(root, mock_logger)
         result = check.simultaneous_rw_support()
         assert result is True, \
-            "CHKPT-05 must return True (satisfied by CAP-02) even when both sim_* are False"
+            "4.7.4 must return True (warnings-only) even when both sim_* are False"
+        assert mock_logger.errors == []
+        assert any(
+            "[4.7.4 checkpointSimultaneousRwSupport]" in m
+            and "simultaneous_write" in m and "simultaneous_read" in m
+            for m in mock_logger.warnings
+        ), f"Expected a warning naming both fields; got {mock_logger.warnings}"