fix: resolve false-positive run failures

docs/configuration.md

@@ -86,9 +86,11 @@ Defaults only apply to the **build** command where Dockerfile selection requires
 
 ## Run phase: log error pattern scan
 
-After a successful container run, madengine may scan the **run log file** for fixed substrings (for example `RuntimeError:`, `OutOfMemoryError`, `Traceback (most recent call last)`). If a match is found, the run can be marked `FAILURE` even when performance metrics exist—intended as a safety net when logs show obvious Python or OOM errors.
+After a successful container run, madengine may scan the **run log file** for fixed substrings (for example `RuntimeError:`, `OutOfMemoryError`, `Traceback (most recent call last)`)—intended as a safety net when logs show obvious Python or OOM errors. If a match is found **and no valid performance metrics were extracted**, the run is marked `FAILURE`.
 
-Some suites (for example layer unit tests) intentionally print benign `RuntimeError:` text while pytest still passes. In those cases you can **disable** the scan or **narrow** what counts as an error.
+If valid performance metrics *were* extracted, a pattern match no longer fails the run: the log scan cannot distinguish madengine/framework diagnostics from a model's own generated stdout, so a generative model whose output happens to contain a banned substring (e.g. an LLM writing `"ValueError:"` in a code sample) no longer produces a false `FAILURE` (see ROCM-27774). The match is still printed (in yellow) for triage visibility.
+
+Some suites (for example layer unit tests) intentionally print benign `RuntimeError:` text while pytest still passes, with no performance metrics to fall back on. In those cases you can **disable** the scan or **narrow** what counts as an error.
 
 Keys can be set in `--additional-context` / `--additional-context-file`, or on the **model** entry in `models.json` (same keys). **Runtime context overrides the model** when both are set.
 

src/madengine/execution/container_runner.py

@@ -45,6 +45,7 @@
     log_text_has_error_pattern,
     make_run_log_file_path,
     resolve_log_error_scan_config,
+    resolve_run_status,
     resolve_run_timeout,
 )
 
@@ -1862,11 +1863,13 @@ def run_container(
                                         "(log_error_pattern_scan).[/dim]"
                                     )
 
-                                # Status logic: Must have performance AND no errors to be considered success
+                                # Status logic: valid performance metrics take priority over a log
+                                # error-pattern match, since the scan cannot tell framework/harness
+                                # diagnostics apart from a model's own generated stdout (ROCM-27774).
                                 # Exception: Worker nodes in multi-node training (MAD_COLLECT_METRICS=false)
                                 # are not expected to report global performance metrics
                                 performance_value = run_results.get("performance")
-                                has_performance = (
+                                has_performance = bool(
                                     performance_value
                                     and performance_value.strip()
                                     and performance_value.strip() != "N/A"
@@ -1875,36 +1878,39 @@ def run_container(
                                 # Check if this is a worker node (not collecting metrics)
                                 is_worker_node = os.environ.get("MAD_COLLECT_METRICS", "true").lower() == "false"
 
-                                if has_errors:
-                                    run_results["status"] = "FAILURE"
-                                    self.rich_console.print(
-                                        f"[red]Status: FAILURE (error patterns detected in logs)[/red]"
-                                    )
-                                elif has_performance:
-                                    run_results["status"] = "SUCCESS"
+                                # Multi-node/SLURM in-job run: the login node aggregates the richest
+                                # per-node multiple_results CSV and writes the authoritative perf/status
+                                # record (slurm.collect_results + _select_best_multiple_results_csv).
+                                # Primus emits throughput only on the last global rank, which may land on a
+                                # different node than the designated collector (MAD_COLLECT_METRICS=true),
+                                # so an empty local perf here is not authoritative and must not fail the job.
+                                skip_perf_collection = bool(
+                                    self.additional_context.get("skip_perf_collection", False)
+                                )
+
+                                status, status_reason = resolve_run_status(
+                                    has_performance=has_performance,
+                                    has_errors=has_errors,
+                                    is_worker_node=is_worker_node,
+                                    skip_perf_collection=skip_perf_collection,
+                                )
+                                run_results["status"] = status
+
+                                if status == "FAILURE":
                                     self.rich_console.print(
-                                        f"[green]Status: SUCCESS (performance metrics found, no errors)[/green]"
+                                        f"[red]Status: FAILURE ({status_reason})[/red]"
                                     )
-                                elif is_worker_node:
-                                    # Worker nodes don't report global performance metrics - this is expected
-                                    run_results["status"] = "SUCCESS"
+                                elif has_errors:
+                                    # SUCCESS despite a log error-pattern match: performance metrics
+                                    # were valid, so the match is likely benign model-generated text.
+                                    # Surfaced in yellow (rather than plain green) for triage visibility.
                                     self.rich_console.print(
-                                        f"[green]Status: SUCCESS (worker node, no errors detected)[/green]"
+                                        f"[yellow]Status: SUCCESS ({status_reason})[/yellow]"
                                     )
-                                elif self.additional_context.get("skip_perf_collection", False):
-                                    # Multi-node/SLURM in-job run: the login node aggregates the richest
-                                    # per-node multiple_results CSV and writes the authoritative perf/status
-                                    # record (slurm.collect_results + _select_best_multiple_results_csv).
-                                    # Primus emits throughput only on the last global rank, which may land on a
-                                    # different node than the designated collector (MAD_COLLECT_METRICS=true),
-                                    # so an empty local perf here is not authoritative and must not fail the job.
-                                    run_results["status"] = "SUCCESS"
+                                else:
                                     self.rich_console.print(
-                                        f"[green]Status: SUCCESS (perf collection deferred to login-node aggregation)[/green]"
+                                        f"[green]Status: SUCCESS ({status_reason})[/green]"
                                     )
-                                else:
-                                    run_results["status"] = "FAILURE"
-                                    self.rich_console.print(f"[red]Status: FAILURE (no performance metrics)[/red]")
 
                             except Exception as e:
                                 self.rich_console.print(f"[yellow]Warning: Error in status determination: {e}[/yellow]")

src/madengine/execution/container_runner_helpers.py

@@ -149,6 +149,57 @@ def log_text_has_error_pattern(
     return False
 
 
+def resolve_run_status(
+    has_performance: bool,
+    has_errors: bool,
+    is_worker_node: bool = False,
+    skip_perf_collection: bool = False,
+) -> typing.Tuple[str, str]:
+    """
+    Decide the final run status ("SUCCESS"/"FAILURE") and a short human-readable reason.
+
+    Priority (see ROCM-27774):
+
+    1. Valid extracted performance metrics are the strongest evidence a run actually
+       completed successfully. A post-hoc log error-pattern match cannot distinguish
+       madengine/framework diagnostics from a model's own generated stdout (e.g. an LLM
+       benchmark whose response text contains ``"ValueError:"``), so it must not override
+       a run that already produced valid performance data. The match is still reported so
+       it remains visible for triage, without failing an otherwise-successful run.
+    2. Otherwise, a matched error pattern fails the run (no performance data to
+       contradict it).
+    3. Otherwise, worker nodes / deferred perf-collection runs are expected to have no
+       local performance and are not failed for that reason.
+    4. Otherwise, no performance metrics and no exemption applies -> FAILURE.
+
+    Args:
+        has_performance: Whether valid performance metrics were extracted from the log.
+        has_errors: Whether a configured error pattern was matched in the log.
+        is_worker_node: Whether this is a non-collecting worker node
+            (``MAD_COLLECT_METRICS=false``) in multi-node training.
+        skip_perf_collection: Whether local perf collection is deferred to a login-node
+            aggregator (e.g. multi-node SLURM runs).
+
+    Returns:
+        (status, reason) tuple, e.g. ``("SUCCESS", "performance metrics found, no errors")``.
+    """
+    if has_performance:
+        if has_errors:
+            return (
+                "SUCCESS",
+                "performance metrics found; error pattern also matched in logs "
+                "(likely model-generated output, not treated as failure)",
+            )
+        return "SUCCESS", "performance metrics found, no errors"
+    if has_errors:
+        return "FAILURE", "error patterns detected in logs"
+    if is_worker_node:
+        return "SUCCESS", "worker node, no errors detected"
+    if skip_perf_collection:
+        return "SUCCESS", "perf collection deferred to login-node aggregation"
+    return "FAILURE", "no performance metrics"
+
+
 def resolve_run_timeout(
     model_info: typing.Dict,
     cli_timeout: int,

tests/unit/test_container_runner_helpers.py

@@ -6,6 +6,7 @@
     DEFAULT_LOG_ERROR_PATTERNS,
     log_text_has_error_pattern,
     resolve_log_error_scan_config,
+    resolve_run_status,
 )
 
 
@@ -130,3 +131,50 @@ def test_user_benign_literal_parentheses(self):
             ["(benign marker)"],
             (),
         )
+
+
+class TestResolveRunStatus:
+    """ROCM-27774: valid performance metrics must win over a log error-pattern match,
+    since the scan cannot tell framework diagnostics apart from a model's own
+    generated stdout (e.g. an LLM benchmark response containing "ValueError:")."""
+
+    def test_performance_and_no_errors_is_success(self):
+        status, reason = resolve_run_status(has_performance=True, has_errors=False)
+        assert status == "SUCCESS"
+        assert "no errors" in reason
+
+    def test_performance_with_errors_is_still_success(self):
+        status, reason = resolve_run_status(has_performance=True, has_errors=True)
+        assert status == "SUCCESS"
+        assert "error pattern also matched" in reason
+
+    def test_errors_without_performance_is_failure(self):
+        status, reason = resolve_run_status(has_performance=False, has_errors=True)
+        assert status == "FAILURE"
+        assert "error patterns detected" in reason
+
+    def test_worker_node_without_performance_is_success(self):
+        status, reason = resolve_run_status(
+            has_performance=False, has_errors=False, is_worker_node=True
+        )
+        assert status == "SUCCESS"
+        assert "worker node" in reason
+
+    def test_worker_node_with_errors_is_still_failure(self):
+        # Worker-node exemption only covers missing performance data, not error matches.
+        status, reason = resolve_run_status(
+            has_performance=False, has_errors=True, is_worker_node=True
+        )
+        assert status == "FAILURE"
+
+    def test_skip_perf_collection_without_performance_is_success(self):
+        status, reason = resolve_run_status(
+            has_performance=False, has_errors=False, skip_perf_collection=True
+        )
+        assert status == "SUCCESS"
+        assert "deferred" in reason
+
+    def test_no_performance_no_errors_no_exemption_is_failure(self):
+        status, reason = resolve_run_status(has_performance=False, has_errors=False)
+        assert status == "FAILURE"
+        assert "no performance metrics" in reason