ci(noema): keep sanitized sidecar evidence, including discovery timing, on successful runs

.github/workflows/noema-review.yml

@@ -841,8 +841,8 @@ jobs:
             }' | gh api -X POST "repos/${TARGET_REPOSITORY}/dispatches" --input -
           echo "::notice::Scheduled Noema transport continuation re-dispatch for ${TARGET_REPOSITORY}#${PR_NUMBER} at ${EXPECTED_HEAD_SHA} (attempt ${NEXT_ATTEMPT})."
 
-      - name: Upload contextual-orchestrator sidecar evidence on failure
-        if: failure() && env.PR_NUMBER != ''
+      - name: Upload contextual-orchestrator sidecar evidence
+        if: always() && env.PR_NUMBER != ''
         uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
         with:
           name: noema-sidecar-evidence

CHANGELOG.md

@@ -1,3 +1,9 @@
+### Noema review keeps sanitized sidecar evidence on successful runs, including discovery timing
+
+- `noema-review.yml` uploaded the `noema-sidecar-evidence` artifact only under `if: failure()`, so every successful run threw away the sanitized sidecar stderr. At the vendored pin `767e67fb`, `contextual_orchestrator_review_launcher.py` already logs at DEBUG, and the sanitizer keeps `http_request` (status, `latency_ms`, `request_id`) and the `provider_attempt*` route events, which carry the same `request_id`. Only failed runs therefore kept any post-Gateway latency sample. The step now runs under `if: always() && env.PR_NUMBER != ''`, the same condition Strix uses for `strix-reports`. Files, artifact name, pinned `actions/upload-artifact`, `if-no-files-found: ignore` and 5-day retention are unchanged.
+- The sanitizer dropped `model_discovery.py`'s `discovery_result account=… model_count=… elapsed_ms=…` (DEBUG) and `discovery_complete providers=… models=… errors=…` (INFO), so the artifact could not split pre-healthz startup into discovery and preflight. Both are now allowlisted with the timestamp, and only the provider name and bounded numbers are kept. Any extra field, an uppercase account or a non-numeric count is still dropped. Contract test: `test_sidecar_stream_sanitizer_admits_discovery_timing_events`. It fails on the previous sanitizer and renders the real templates through `logging.Formatter`.
+- No CO pin bump or `--log-level` change is needed. contextual-orchestrator#1218 changes only the standalone `review_gateway` CLI. There is no file overlap with #2325, which touches only `contextual_orchestrator_review_sidecar.sh` and its contract test.
+
 ### Noema transport capacity schedules a bounded continuation re-dispatch
 
 - After gateway failover, HTTP 429/5xx no longer end only as a permanent required-check failure with `caller attempts=1`. ADR-0031 classifies that class as `provider_capacity_unavailable`, keeps the single gateway request per job, surfaces `provider_attempt_count` from the orchestrator error envelope, and authorizes at most two same-head `repository_dispatch` retries after a capped `Retry-After` or deterministic 60–180 s jitter. Review is never skipped. Refs #2165.

scripts/ci/sanitize_contextual_orchestrator_sidecar_stream.py

@@ -74,6 +74,11 @@
         rf"^circuit_opened agent_id={_AGENT_ID} failures={_NUMBER} threshold=\d+ reset_seconds={_NUMBER}$",
         rf"^circuit_reset agent_id={_AGENT_ID}$",
         rf"^circuit_cleared agent_id={_AGENT_ID}$",
+        # contextual_orchestrator/model_discovery.py at the pin: per-provider
+        # discovery duration (DEBUG) and the discovery total (INFO). Only the
+        # provider name and bounded counts/durations are kept.
+        rf"^discovery_result account=[a-z][a-z0-9_]{{0,63}} model_count=\d+ elapsed_ms={_NUMBER}$",
+        r"^discovery_complete providers=\d+ models=\d+ errors=\d+$",
     )
 )
 # Python traceback anatomy. The orchestrator's generic request handler

tests/test_contextual_orchestrator_review_runtime_preflight.py

@@ -2793,3 +2793,39 @@ def test_preflight_lazy_fill_keeps_deferral_for_probed_transient_routes() -> Non
     assert (report["ready_count"], report["deferred_count"], report["rejected_count"]) == (target, 1, 0)
     assert served[-1].priority == -namespace["REVIEW_PREFLIGHT_DEFERRED_PRIORITY_PENALTY"]
     assert report["routes"][0]["status"] == "deferred"
+
+
+def test_sidecar_stream_sanitizer_admits_discovery_timing_events() -> None:
+    """Discovery duration survives sanitization so pre-healthz startup is attributable.
+
+    ``contextual_orchestrator/model_discovery.py`` at the vendored pin logs
+    ``discovery_result account=%s model_count=%d elapsed_ms=%.1f`` (DEBUG) per
+    provider and ``discovery_complete providers=%d models=%d errors=%d`` (INFO).
+    Both used to fold into ``omitted_unstructured_lines``, so the Noema sidecar
+    artifact could not split its startup into discovery and preflight. Render
+    the real templates with the sidecar format; only the timestamp, provider
+    name and bounded numbers may come back.
+    """
+    import logging
+
+    sanitize_line = _load_sanitizer()["sanitize_line"]
+    formatter = logging.Formatter("%(asctime)s %(levelname)s %(name)s %(message)s")
+    records = (
+        (logging.DEBUG, "discovery_result account=%s model_count=%d elapsed_ms=%.1f", ("nvidia_nim", 12, 812.456)),
+        (logging.INFO, "discovery_complete providers=%d models=%d errors=%d", (5, 40, 1)),
+    )
+    for level, template, args in records:
+        record = logging.LogRecord(
+            "contextual_orchestrator.model_discovery", level, __file__, 0, template, args, None
+        )
+        rendered = formatter.format(record)
+        date, time, _level, _name, message = rendered.split(" ", 4)
+        assert sanitize_line(rendered) == f"{date} {time} {message}"
+    assert sanitize_line("discovery_complete providers=5 models=40 errors=1") == (
+        "discovery_complete providers=5 models=40 errors=1"
+    )
+    # Anything outside the bounded fields stays out.
+    assert sanitize_line("discovery_result account=Nvidia model_count=1 elapsed_ms=1.0") is None
+    assert sanitize_line("discovery_result account=nvidia_nim model_count=1 elapsed_ms=1.0 key=sk-x") is None
+    assert sanitize_line("discovery_complete providers=5 models=40 errors=1 detail=boom") is None
+    assert sanitize_line("discovery_complete providers=five models=40 errors=1") is None

tests/test_noema_orchestrator_workflow_contract.py

@@ -493,19 +493,22 @@ def test_noema_review_job_has_no_job_level_timeout() -> None:
     ), "the two-hour-per-model allowance this bound relies on must still be documented"
 
 
-def test_noema_review_uploads_sidecar_evidence_on_failure() -> None:
-    """A failed verdict phase ships the sanitized sidecar stderr and preflight report.
+def test_noema_review_uploads_sidecar_evidence_on_every_outcome() -> None:
+    """Every verdict phase ships the sanitized sidecar stderr and preflight report.
 
     Before this step a failed Noema run left ``artifacts=0`` (run 33981136873:
     3122 s, then HTTP 502, no per-route trace in the job log). The stderr file
     is the sidecar sanitizer's bounded allowlist output -- the same file Strix
-    already publishes in ``strix-reports`` -- so shipping it on failure adds
-    diagnosis without adding exposure (#1935 follow-up).
+    already publishes in ``strix-reports`` under ``always()`` -- so shipping it
+    adds diagnosis without adding exposure (#1935 follow-up). A ``failure()``
+    gate discarded the successful runs' http_request/discovery_complete lines,
+    which are the only post-Gateway latency samples, so the gate matches Strix.
     """
     workflow = workflow_text("noema-review.yml")
-    name = "Upload contextual-orchestrator sidecar evidence on failure"
+    name = "Upload contextual-orchestrator sidecar evidence"
     step = workflow_step(workflow, name)
-    assert "if: failure() && env.PR_NUMBER != ''" in step
+    assert "if: always() && env.PR_NUMBER != ''" in step
+    assert "failure()" not in step
     strix_pin = re.search(
         r"actions/upload-artifact@([0-9a-f]{40})", workflow_text("strix.yml")
     ).group(1)