fix(tests): repair 4 pre-existing test failures on main

[seonghobae]

Summary

Repair the pre-existing Responses/spend/test failures that README PR #994 exposed after its runner admission repair made the full suite execute.

• keep orchestrator/free on the integrated single-worker auto-route contract while non-free virtual models retain conduct-path assertions;
• isolate optional model-judge calls from worker-only usage fixtures;
• pin repository-owned Tests/Fuzz/Security jobs to ubuntu-24.04 rather than the starved floating selector;
• preserve provider-reported judge usage at the transport boundary with explicit provider-vs-synthetic provenance and immutable usage snapshots;
• accept both Chat Completions (prompt_tokens / completion_tokens) and Responses (input_tokens / output_tokens) token-counter families without converting authoritative all-zero provider evidence into estimated spend.

Causal repair lineage

The hosted full suite first exposed worker-accounting fixtures whose evidence scope was polluted by the optional fast-mlsirm judge call. Those tests now suppress only that optional judge path while retaining the actual worker provider call and its usage assertions.

A subsequent valid review showed that an all-zero usage mapping was ambiguous: the same value shape can be a real provider measurement or synthetic mock/fast-mlsirm zero-fill. The source repair therefore records served_usage_source at the transport boundary, follows the agent that actually served after failover, snapshots mutable provider usage, treats provider-reported zero as measured, and leaves synthetic/unknown zero unmeasured.

A later exact-head review correctly found that the zero-usage validator only recognized legacy Chat Completions fields. The current source and regression suite now accept complete non-negative Responses counters (input_tokens, output_tokens, total_tokens) as the same authoritative usage family. Positive aggregate fallback uses the same aliases. The focused regression explicitly proves provider-reported all-zero Responses usage survives accounting.

Temporary self-modifying repair workflows used during the source repair have been removed from the branch. Superseded temp-writer review findings are resolved only because those workflow files no longer exist.

Why this owns README #994's blocker

README PR #994 did not create these accounting/test defects; its runner repair exposed them. The causal production/test fixes therefore live here. #994 must converge onto protected main only after this owner repair integrates and then reacquire exact-head README checks/reviews on the new base.

Current authority

Exact current head: 8f9fd87283fc6998fc590d3ab7ea3c6007db3bdb on recorded protected base main@4d143601c2904a28e95d091b261c0a15e9a4f283.

Current review-thread inventory is resolved. Fresh exact-head Tests, Security, Fuzz, SAST Semgrep, and Security Scan runs are queued and therefore non-passing. No predecessor-head workflow result transfers. Merge only if this unchanged head becomes terminal-clean and then-live review/thread/branch governance permits the ordinary protected path.

Scope

Tests, repository-owned CI runner admission, and the accounting source repair required to make their asserted evidence truthful. No security threshold, release, deployment, credential, or governance gate is weakened.

Summary by CodeRabbit

• 개선 사항

  • 모델 판정 결과의 토큰 사용량이 제공자 보고값과 합성값을 구분해 기록됩니다.
  • 제공자가 보고한 토큰 사용량이 0인 경우에도 유효한 측정값으로 보존됩니다.
  • 출처가 불명확하거나 테스트용으로 생성된 사용량은 실제 사용량으로 기록되지 않습니다.
  • 사용량 및 추적 정보가 변경되지 않는 스냅샷으로 보존됩니다.
  • 다양한 채팅 및 응답 형식의 토큰 사용량 처리가 보강되었습니다.

• 품질 개선

  • 자동화된 보안·퍼징·품질 검사 환경이 Ubuntu 24.04로 고정되었습니다.
  • 비동기 임베딩 배치 처리의 완료 확인이 안정화되었습니다.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 74d3e08d-d289-428c-bc94-d445fbc72c6b

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: e0903695-49fd-4bf7-ae9f-6c203d6d3135

📥 Commits

Reviewing files that changed from the base of the PR and between 02b61e4 and 65f5a03.

📒 Files selected for processing (1)

• tests/test_provider_embedding_batch_backend.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

Judge 사용량의 출처와 snapshot 처리를 명확히 하고, provider 및 mock usage의 회계 규칙과 테스트를 갱신했습니다. GitHub Actions 작업의 실행 러너를 ubuntu-24.04로 고정했습니다.

Changes

Judge 사용량 회계

Layer / File(s)	Summary
사용량 캡처 및 출처 분류 contextual_orchestrator/orchestrator.py	_FastMLSIJudgeAdapter가 usage snapshot과 served_usage_source를 기록합니다. Provider와 mock 출처를 구분합니다.
Judge 사용량 증거 판정 contextual_orchestrator/orchestrator.py, tests/test_model_judge.py	Chat 및 Responses token 형식을 검증합니다. Adapter usage를 우선 사용하고, 조건에 맞는 집계 usage를 fallback으로 사용합니다.
사용량 출처 회귀 검증 tests/test_model_judge_usage_provenance_regression.py	Provider의 전부 0인 usage, mock usage, 단일 usage 읽기, usage snapshot 보존을 검증합니다.
Provider 및 spend 회계 테스트 격리 tests/test_provider_usage_capture.py, tests/test_spend_analytics.py, tests/test_batch_optimizer.py, tests/test_provider_embedding_batch_backend.py	선택적 fast-mlsirm judge를 비활성화하고 worker usage 및 tokenizer usage 계약을 독립적으로 검증합니다. 비동기 embedding 작업의 완료 대기를 명시합니다.

CI 실행 환경 고정

Layer / File(s)	Summary
GitHub Actions 러너 고정 .github/workflows/ci.yml, .github/workflows/fuzz.yml, .github/workflows/security.yml	pytest, 벤치마크, 퍼징, CodeQL 및 Python 공급망 작업의 runner를 ubuntu-24.04로 변경합니다.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: 🔵 Low · up to 65f5a

The embedding-batch test synchronization change is approved, but an earlier workflow configuration concern remains unresolved and should be addressed before relying on that automation.

Sequence Diagram(s)

sequenceDiagram
  participant FastMLSIJudgeAdapter
  participant ProviderOrMockTransport
  participant TaskOrchestrator
  FastMLSIJudgeAdapter->>ProviderOrMockTransport: Judge 요청
  ProviderOrMockTransport-->>FastMLSIJudgeAdapter: 응답과 usage 반환
  FastMLSIJudgeAdapter->>FastMLSIJudgeAdapter: usage snapshot과 출처 기록
  FastMLSIJudgeAdapter-->>TaskOrchestrator: judge 결과와 accounting fields 반환
  TaskOrchestrator->>TaskOrchestrator: judge_usage 우선 판정

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	제목은 PR의 주요 목적이었던 기존 테스트 4건의 실패 수정과 직접 관련되며, 간결하고 명확합니다.
Docstring Coverage	✅ Passed	Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 30 functions across 7 files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/orchestrated-responses-stream-and-spend-analytics-20260901

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[devin-ai-integration]

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

[seonghobae]

Pushed fd9aac52: an additional, complementary fix on top of f3ef3dcd/25ec4823.

Devin's finding that test_exact_output_without_prompt_usage_is_explicitly_unavailable depends on whether the optional fast-mlsirm package is installed traced back to a real production bug, not just a test-isolation gap: _judge_adapter_accounting_fields() used a plain truthiness check (if judge_adapter.served_usage:) on the usage dict. fast-mlsirm aggregates a missing trace usage into a non-empty, all-zero mapping ({"prompt_tokens": 0, "completion_tokens": 0, "total_tokens": 0}), which is truthy in Python yet carries no real measurement — so that call site fabricated it as "reported" spend, disagreeing with the sibling call site a few lines above, which was already guarded against the identical shape (Devin review on #961).

Fix: a shared _usage_has_positive_evidence() helper, applied consistently at both call sites, so a completed-but-unmeasured judge call stays honestly attributed as unmeasured rather than counted as reported-zero spend.

Added two regression tests to tests/test_model_judge.py (RED against the reverted fix, GREEN with it restored):

• test_judge_adapter_zero_aggregate_served_usage_is_not_treated_as_reported
• test_judge_adapter_positive_served_usage_is_still_reported

This is independent of and additive to the test-isolation fix already on this branch — both fixes fix different halves of the same underlying usage_source divergence. Full local suite: 3302 passed, 2 skipped (the 1 collection error is numpy missing in this sandbox's venv, unrelated — test_psychometric_routing.py isn't touched by this change). The six directly related test files (test_batch_optimizer.py, test_model_judge.py, test_nim_benchmark.py, test_orchestrator_client_boundaries.py, test_orchestrator_dispatch_boundaries.py, test_spend_analytics.py) all pass on the combined head: 233 passed.

---

Generated by Claude Code

[devin-ai-integration]

Devin Review found 4 new potential issues.

[seonghobae]

CodeRabbit follow-up: verified all three findings; docstrings fixed, two already stale, one flagged for owner decision

Re-checked against the PR's actual current head, since CodeRabbit's review (up to 4351a) was submitted against a mid-repair commit, several commits before this branch's current tip.

Docstring coverage (fixed, pushed in 2f325e8e). CodeRabbit's 52.17% (12/23) is a real number from its own diff-scoped scanner, not this repo's interrogate gate (which already passed 100% throughout — it excludes tests/ and private/semiprivate functions, so the gap was entirely in touched test functions and nested test-double classes that CodeRabbit's scanner does count). Verified the exact function set (12/23 = 52.1739...%, matching CodeRabbit's number precisely once nested defs inside touched test functions are included) and added 11 concise docstrings across the 5 flagged files. Full suite re-verified clean afterward: 3316 passed, 2 skipped, 1 failed (known environment-scoped fast_mlsirm ModuleNotFoundError, not a regression).

"Drops valid Responses usage" (already resolved, no action needed). _usage_has_positive_evidence and _usage_is_reported_token_mapping (contextual_orchestrator/orchestrator.py) already fall back to Responses-style input_tokens/output_tokens aliases (usage.get("prompt_tokens", usage.get("input_tokens")) etc.) — landed in commit 6eafe232 ("accept Responses usage counters in both accounting helpers"), well before the current head. tests/test_model_judge_usage_provenance_regression.py::test_provider_reported_responses_zero_usage_remains_reported exercises exactly this and passes.

"Non-parseable repair workflow" (already resolved, no action needed). .github/workflows/_temp_usage_alias_repair.yml and _temp_usage_counter_family_repair.yml (and their siblings _temp_usage_provenance_repair.yml, _temp_usage_alias_repair_v3.yml) do not exist on the current head — confirmed by listing .github/workflows/ at this branch's tip. They were one-shot RED→fix→GREEN→remove-itself workflows per this repo's convention; their fixes were applied by hand once the workflow runners themselves failed to materialize jobs, then the files were deleted in commits 06e6369e, dfc04d89, bd8f8f87, 2f5f84ec, ebb087b1. Nothing to fix in the current diff.

Adapter reuse lifecycle — documenting for explicit owner attention, not fixing here.

_FastMLSIJudgeAdapter (contextual_orchestrator/orchestrator.py) tracks one provider call's evidence in single-slot mutable instance attributes: served_agent_id, served_model, served_usage, served_usage_source, served_output. Each call to complete() / complete_structured() overwrites these — it does not accumulate them. _model_judge_verification() constructs a fresh adapter per verification call (orchestrator.py:~8160), so there is no cross-request reuse issue.

The latent issue is reuse within a single verification call. fast_mlsirm.llm_judge.ContextualOrchestratorJudge.judge() calls the adapter's complete()/complete_structured() exactly once when category_method resolves to "direct" — the only path this repo's sole call site (orchestrator.py, fast_judge.judge(task=..., answer=..., criteria=(...)), no category_count passed) ever exercises. But judge()'s binary_threshold / cumulative_threshold methods (fast_mlsirm/llm_judge.py, _binary_threshold_judgments) call self._complete(...) once per (criterion × threshold) pair — up to MAX_BINARY_THRESHOLD_CALLS (64) — against the same adapter instance, optionally concurrently via a ThreadPoolExecutor (_binary_threshold_concurrency) whenever the client exposes local_concurrency > 1.

If that path were ever exercised, each complete_structured() call would clobber the adapter's served_usage/served_agent_id/etc. from the prior call, so _judge_adapter_accounting_fields() would only ever see the last provider call's usage — silently losing every earlier call's spend — and under the thread-pool path it would be a genuine data race (concurrent writers mutating the same instance's attributes).

Verified this is dormant, not currently firing: category_count is never passed to judge() anywhere in contextual_orchestrator/ or tests/ today (grep -rn "category_count" contextual_orchestrator/ tests/test_model_judge*.py → no matches), so _binary_threshold_judgments is unreachable from this codebase as it stands. Not blocking this PR's merge on its own.

This needs an explicit owner decision on intended adapter lifecycle before category_count/binary-threshold judging is ever wired up here, e.g.:

• forbid reuse — raise if complete()/complete_structured() is called more than once per adapter instance, forcing one adapter per provider call; or
• make the adapter accumulate a list of served calls instead of overwriting a single slot, with _judge_adapter_accounting_fields() summing/reporting all of them; or
• require any future category_count-based judge path to construct a fresh adapter per boundary call rather than sharing one.

Flagging precisely so it isn't lost, per this session's standing loop directive.

---

🤖 Generated with Claude Code

https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4

---

Generated by Claude Code

[seonghobae]

Rebased onto current main + noema-review root cause

Merge. Branch was behind on the well-known stale base 8839081.... Fetched fresh, re-verified live head (2f325e8e...) and mergeable_state: behind matched, confirmed no concurrent edits (last commit ~10h old), then ran git fetch origin main && git merge origin/main --no-edit. The merge was conflict-free (ort auto-merged cleanly) — this PR's own diff (9 files: orchestrator.py, CI workflow pins, 5 accounting/spend-analytics test files) doesn't touch CHANGELOG.md or docs/product-technical-gap-baseline.md, so neither of those usual conflict points was in play here. The one file both sides touched, .github/workflows/fuzz.yml, merged correctly: this PR's ubuntu-24.04 pin and main's new "Fuzz governed rater observation parser" step both survived. Pushed non-force: 2f325e8e..cef6214f.

Verification on the merged head (python -m pip install --require-hashes -r requirements.lock && python -m pip install --no-deps -e .):

• PR's own claimed scope (test_batch_optimizer.py, test_model_judge.py, test_model_judge_usage_provenance_regression.py, test_provider_usage_capture.py, test_spend_analytics.py): 82/82 passed. The "4 pre-existing test failures" this PR fixes are confirmed fixed on the merged head.
• Full suite (python -m pytest tests -q): 3360 passed, 2 skipped, 2 failed in 844.98s. Both failures are unrelated to this PR and to the merge — confirmed via git diff origin/main -- <file> returning empty for both, i.e. byte-identical to main and untouched by either this PR's diff or the merge resolution:
  • test_admin_contract.py::test_model_group_mutations_refresh_audit_events — NameError: name 'json' is not defined (pre-existing missing import json in that test file on main, unrelated to this PR).
  • test_psychometric_routing.py::test_fast_mlsirm_fit_uses_judge_acceptance_item_for_context_score — ModuleNotFoundError: No module named 'fast_mlsirm' (that dependency is gated python_full_version >= "3.12" in requirements.lock/pyproject.toml; this verification ran on Python 3.11, so it's an environment gap, not a regression).

Neither failure is a regression introduced by this merge.

noema-review root cause (infra-side, matching #993/#976/#984/#972). The failed run (https://github.com/ContextualWisdomLab/contextual-orchestrator/actions/runs/33613811994/job/100194921923) shows the reviewer-model line-citation-drift pattern: the LLM reviewer cited contextual_orchestrator/orchestrator.py:406 (RIGHT side) on its first pass and :382 on its one permitted bounded-repair pass, but neither is an exact changed-side line in the diff — the evidence validator's nearest-line search landed on 409-413 and 385-389 respectively. Both the original review and its repair attempt were rejected for the same reason (... which is not an exact changed-side line), so the job errored with Noema model-output repair remained invalid and no verdict was ever posted. This is a review-infrastructure line-binding issue, not a defect in this PR's diff.

Ready for a fresh noema-review run on the new head cef6214f.

Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4

---

Generated by Claude Code

[seonghobae]

Diagnosis: pre-39a4348e admin-contract staleness, not a defect in this PR's own diff

The "Full unit and contract suite" required check was failing with:

FAILED tests/test_admin_contract.py::test_model_group_mutations_refresh_audit_events - NameError: name 'json' is not defined. Did you forget to import 'json'
1 failed, 3361 passed, 2 skipped in 729.47s

Root cause: this branch's merge-base with main was 212ff437 (the commit that introduced the missing-import json / extraction bug in tests/test_admin_contract.py::test_model_group_mutations_refresh_audit_events). The fix for that shared bug landed on main as 39a4348e (#1035), but this branch predates it, so it inherited the stale failing test. PR #1002's own diff (Responses/spend accounting test fixes, README-related) does not touch test_admin_contract.py — confirmed via git diff 212ff437...cef6214f -- tests/ showing only test_batch_optimizer.py, test_model_judge.py, test_model_judge_usage_provenance_regression.py, test_provider_usage_capture.py, test_spend_analytics.py.

The separately-failing CodeQL analysis check is the known, already-tracked org-wide "default setup enabled" infra issue being fixed centrally in .github (#1750/#1788) — left untouched here.

What was done

1. Verified no concurrent activity on fix/orchestrated-responses-stream-and-spend-analytics-20260901 (head unchanged at cef6214f from updated_at: 2026-09-02T19:28:23Z through the merge and both pre-push re-checks).
2. git fetch origin main && git merge --no-edit in a scratch worktree — clean merge, no conflicts. Diff of the resulting merge commit vs. origin/main is exactly 418 insertions(+), 40 deletions(-) — matching this PR's own stated diff stat precisely, confirming the merge is base-main + this PR's changes and nothing else.
3. Verification (offline sandbox venv seeded from local uv cache to work around this session's egress policy on raw GitHub archive downloads — CI's own hosted runner is unaffected and re-ran everything fresh post-push):
   • tests/test_admin_contract.py alone: 3 passed (previously 1 failed).
   • PR's own affected accounting/spend files (test_batch_optimizer.py, test_model_judge.py, test_model_judge_usage_provenance_regression.py, test_provider_usage_capture.py, test_spend_analytics.py): 82 passed.
   • Test files touched by the merged-in main commits (test_hourly_opencode_loop_contract.py, test_web_search.py, fuzz/test_fuzz_properties.py): 59 passed.
   • Full suite (python -m pytest -q, matching CI's uv run --locked --extra api --extra db --extra queue --group dev python -m pytest -q): 3401 passed, 2 skipped, 0 failed in 772.74s (0:12:52).
4. Pushed the merge commit non-force: cef6214f..02b61e41 (fast-forward). Re-checked remote branch head immediately before pushing — unchanged.

CI's hosted "Full unit and contract suite" will re-run on the new head 02b61e41 and is expected to go green now that 39a4348e is included.

---

_Generated by Claude Code

---

Generated by Claude Code

[seonghobae]

Ported test-only race fix from #1044 (not yet merged)

Diagnosis. This PR's "Full unit and contract suite" check failed on head 02b61e4 with exactly one failure, unrelated to this PR's own diff:

FAILED tests/test_provider_embedding_batch_backend.py::test_unknown_tokenizer_byte_bound_never_becomes_recorded_usage[한글🙂é] - KeyError: 'total_tokens'
1 failed, 3400 passed, 2 skipped

This is the exact race condition already root-caused and fixed in #1044 (fix/embedding-batch-wait-timeout-race), currently open and unmerged: test_unknown_tokenizer_uses_authoritative_provider_usage and test_unknown_tokenizer_byte_bound_never_becomes_recorded_usage call complete_embeddings_batch() on a provider-backed (non-mock) embedding agent without wait_timeout. That backend completes asynchronously in a background ThreadPoolExecutor thread, so the calling thread can read the document before the job finishes, hitting the not-is_complete early-return branch that omits total_tokens entirely. Under CI load this triggers intermittently; the specific failing Unicode parametrize case is incidental, not causal.

Per the standing PR-governance rule (port the same change into the affected PR now rather than wait on the origin PR to merge — it no-ops once the base carries it), I ported #1044's identical diff directly onto this PR's head rather than waiting on #1044.

Change applied (byte-identical to #1044's diff, verified via matching pre/post blob hashes 0eb661f..ec44918): added wait_timeout=1 to both complete_embeddings_batch(...) calls in tests/test_provider_embedding_batch_backend.py, plus the same explanatory comment and the same added assert document["status"] == "completed" in the second test. Test-only, no production code touched.

Verification:

• tests/test_provider_embedding_batch_backend.py (all 20 tests): passing, repeated 10x with zero flakes.
• pytest -k "embedding or token_counting or cost_router": 261 passed, 1 skipped.
• Full suite (pytest tests -q, Python 3.11 sandbox venv against requirements.lock): 1 failed, 3400 passed, 2 skipped in 743.11s. The sole failure — test_psychometric_routing.py::test_fast_mlsirm_fit_uses_judge_acceptance_item_for_context_score (ModuleNotFoundError: No module named 'fast_mlsirm') — is the same pre-existing sandbox-only artifact fix(tests): wait for provider embedding batch completion before assertions #1044's own verification documented (this environment's git proxy can't fetch the native fast-mlsirm wheel), unrelated to this change. The originally reported target failure is gone; the embedding batch tests now pass cleanly.
• interrogate: not applicable — tests/ is excluded from the 100%-docstring gate in pyproject.toml, and no new functions were added.

Freshness: confirmed PR head unchanged (02b61e41b074f6f2f675b0d7f3b7c9e4f02e0168, updated_at static) immediately before pushing.

Pushed non-force as a fast-forward: 02b61e4..65f5a03 → new head 65f5a0397d5e8dc958b00e3a20b2c2999750b8af.

---

🤖 Generated with Claude Code

---

Generated by Claude Code