From 223106bd50bdcbb433f6f55482816d03b5d0bbd0 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 21:41:16 +0900 Subject: [PATCH 1/2] ci(noema): keep sanitized sidecar evidence on successful runs The noema-sidecar-evidence upload ran only under failure(), so every successful Noema run discarded the sanitized sidecar stderr that already carries the DEBUG http_request/discovery_complete lines from pin 767e67fb. Match Strix's always() gate; files, pin, name and retention are unchanged. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LeLbwEgYXzCzpaFMTHDcBS --- .github/workflows/noema-review.yml | 4 ++-- CHANGELOG.md | 4 ++++ .../test_noema_orchestrator_workflow_contract.py | 15 +++++++++------ 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/.github/workflows/noema-review.yml b/.github/workflows/noema-review.yml index 9be705a50c..ff648754c8 100644 --- a/.github/workflows/noema-review.yml +++ b/.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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 34281625cb..0b7ddd0ea8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +### Noema review keeps sanitized sidecar evidence on successful runs too + +- `noema-review.yml` uploaded the `noema-sidecar-evidence` artifact only under `if: failure()`, so every successful run threw away the sanitized sidecar stderr. The vendored pin `767e67fb` and `contextual_orchestrator_review_launcher.py` already emit the DEBUG `http_request` and `discovery_complete` lines that stderr contains, so only failed runs kept any post-Gateway latency sample, and the Gateway-performance comparison had no successful baseline. The step now runs under `if: always() && env.PR_NUMBER != ''`, the same condition Strix uses for `strix-reports`. The files, artifact name, pinned `actions/upload-artifact`, `if-no-files-found: ignore` and 5-day retention are unchanged, and the file is still the sanitizer's bounded allowlist output, so exposure does not change. No pin bump or `--log-level` change is needed. Independent of #2325, which only touches `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. diff --git a/tests/test_noema_orchestrator_workflow_contract.py b/tests/test_noema_orchestrator_workflow_contract.py index 490286aacc..68710559dc 100644 --- a/tests/test_noema_orchestrator_workflow_contract.py +++ b/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) From a403d73b034e3eeaf7b89dd4487ad32f1fa4761c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 23:24:12 +0900 Subject: [PATCH 2/2] ci(sidecar): keep discovery timing in the sanitized sidecar stream The sanitizer dropped model_discovery.py's discovery_result (DEBUG, per-provider elapsed_ms) and discovery_complete (INFO) lines, so the Noema sidecar artifact could not split pre-healthz startup into discovery and preflight. Allowlist both with the timestamp, keeping only the provider name and bounded numbers, and correct the CHANGELOG claim that discovery_complete already reached the sanitized file. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LeLbwEgYXzCzpaFMTHDcBS --- CHANGELOG.md | 6 ++-- ..._contextual_orchestrator_sidecar_stream.py | 5 +++ ...l_orchestrator_review_runtime_preflight.py | 36 +++++++++++++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b7ddd0ea8..d60cfc91bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ -### Noema review keeps sanitized sidecar evidence on successful runs too +### 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. The vendored pin `767e67fb` and `contextual_orchestrator_review_launcher.py` already emit the DEBUG `http_request` and `discovery_complete` lines that stderr contains, so only failed runs kept any post-Gateway latency sample, and the Gateway-performance comparison had no successful baseline. The step now runs under `if: always() && env.PR_NUMBER != ''`, the same condition Strix uses for `strix-reports`. The files, artifact name, pinned `actions/upload-artifact`, `if-no-files-found: ignore` and 5-day retention are unchanged, and the file is still the sanitizer's bounded allowlist output, so exposure does not change. No pin bump or `--log-level` change is needed. Independent of #2325, which only touches `contextual_orchestrator_review_sidecar.sh` and its contract test. +- `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 diff --git a/scripts/ci/sanitize_contextual_orchestrator_sidecar_stream.py b/scripts/ci/sanitize_contextual_orchestrator_sidecar_stream.py index 57f33aad2f..61a684940a 100644 --- a/scripts/ci/sanitize_contextual_orchestrator_sidecar_stream.py +++ b/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 diff --git a/tests/test_contextual_orchestrator_review_runtime_preflight.py b/tests/test_contextual_orchestrator_review_runtime_preflight.py index 0b3e38cb6c..d2678b2f8e 100644 --- a/tests/test_contextual_orchestrator_review_runtime_preflight.py +++ b/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