fix(cost_router): record unavailable race-loser usage instead of dropping it

[seonghobae]

Bug

This repo's CLAUDE.md/AGENTS.md mandate: "Equivalent model-group endpoints may race only through the normalized, explicit endpoint-equivalence contract. Preserve modality validation, bounded concurrency, deadline, cancellation/drain provenance, and honest duplicate-cost evidence."

CostRoutingCoordinator._record_race_endpoint_usage() (contextual_orchestrator/cost_router.py) is the sink the orchestrator calls for every completed non-winning endpoint in a multi-endpoint model-group race (wired via orchestrator._race_usage_sink, and invoked only for race participants whose HTTP call to the provider actually completed and returned a response body — _race_attempt_collector.emit() in orchestrator.py returns early when error is not None or value is None). A losing race branch is, by definition, a duplicate paid call.

Previously, when self._provider_usage(usage) returned None (usage payload missing or malformed — a real provider quirk, not hypothetical: usage not a dict, or prompt_tokens/completion_tokens not non-negative ints), the function returned immediately with no ledger row created at all. That spend became permanently invisible to cost_report()/rollup()/total() — no row, no "unavailable" label, nothing a cost audit could catch.

This was inconsistent with every other completion path in the same class:

• complete()'s ordinary sync/provider-proxy paths always call _record_completion even when usage can't be parsed, falling back to heuristic estimation (measurement_status="estimated").
• record_stream_usage() always creates a row, even when counts is falsy (prompt_tokens=counts[0] if counts else 0, measurement_status="measured" if counts else "unavailable") — this is exactly the "we know a call happened but can't measure it" case.

_record_race_endpoint_usage was the only one of these sinks that could return without ever calling self.ledger.record_usage (directly or via _record_completion).

Fix

Mirrors record_stream_usage's pattern: when _provider_usage() returns None for a race-loser call, write a measurement_status="unavailable" ledger row with prompt_tokens=0/completion_tokens=0 instead of returning silently. Deliberately does not route this case through _record_completion with None counts — that path falls back to heuristic estimation from messages/answer, which are empty in this context, and would silently produce a fake near-zero "estimated" cost instead of an honest "unavailable" one. Calls self.ledger.record_usage(...) directly instead, same as record_stream_usage.

Also resolves agent before branching on parsed counts (needed by both branches either way) and reuses the resulting (provider, model) tuple for both the new "unavailable" write and the existing "measured" path — avoids computing it twice.

agent = next(
    (item for item in self.orchestrator.candidates if item.id == endpoint_id),
    None,
)
if agent is None:  # pragma: no cover - endpoint came from the current pool
    return
counts = self._provider_usage(usage)
provider_model = self._agent_provider_model(agent, context["model_name"])
if counts is None:
    provider, model = provider_model
    record = self.ledger.record_usage(
        provider=provider, model=model,
        prompt_tokens=0, completion_tokens=0,
        request_channel="sync", route_mode=context["route_mode"],
        workflow_run_id=context["workflow_run_id"], attribution=context["attribution"],
        measurement_status="unavailable",
    )
    context["records"].append(record)
    return
record = self._record_completion(..., provider_model=provider_model, prompt_tokens=counts[0], completion_tokens=counts[1])
context["records"].append(record)

Scope is intentionally minimal — only this one function changed.

Test

Added test_race_loser_with_unparseable_usage_is_recorded_as_unavailable in tests/test_cost_router.py, alongside the existing race-usage tests. It sets usage to None for a race-loser call and asserts the ledger now contains exactly one row with measurement_status == "unavailable", prompt_tokens == 0, completion_tokens == 0, and correct provider/model/workflow_run_id — previously this scenario produced an empty ledger.

Evidence

• python -m pytest tests/test_cost_router.py -q → 26 passed (including the new regression test).
• python -m pytest tests -q (full suite) → 2832 passed, 1 skipped, 1 pre-existing failure (test_fast_mlsirm_fit_uses_judge_acceptance_item_for_context_score, ModuleNotFoundError: No module named 'fast_mlsirm' — unrelated to this change; also called out as a pre-existing environment gap in fix(privacy): enforce OpenRouter ZDR at request time #953's own test plan).
• interrogate -v → 100.0% (RESULT: PASSED (minimum: 100.0%, actual: 100.0%)).
• Isolated coverage of cost_router.py from test_cost_router.py alone confirms the new "unavailable" branch (the added lines) is fully exercised — none of its line numbers appear in coverage report's Missing column.

No production routing/defaults changed; this only affects what gets written to the cost ledger when a race-loser's usage payload is unparseable.

---

Generated by Claude Code

Summary by CodeRabbit

• 버그 수정

  • 레이스 엔드포인트의 사용량 정보를 확인할 수 없는 경우에도 비용 기록이 누락되지 않습니다.
  • 해당 기록은 사용량 상태를 ‘확인 불가’로 표시하고 토큰을 0으로 저장합니다.

• 테스트

  • 사용량 정보가 올바르지 않은 레이스 패배 요청이 정상적으로 기록되는지 검증하는 테스트를 추가했습니다.

[coderabbitai]

Warning

Review limit reached

Next included review available in 57 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: d073ffba-6f72-4bfe-9bc8-e8f30da3394c

📥 Commits

Reviewing files that changed from the base of the PR and between b7f1946 and d90c1cd.

📒 Files selected for processing (5)

• CHANGELOG.md
• contextual_orchestrator/cost_ledger.py
• contextual_orchestrator/cost_router.py
• tests/test_cost_ledger.py
• tests/test_cost_router.py

📝 Walkthrough

Walkthrough

레이스 엔드포인트의 사용량 payload를 파싱할 수 없을 때 비용 원장에 unavailable 상태의 영 토큰 행을 기록합니다. 제공자와 모델 식별자를 한 번 계산하여 재사용합니다. 해당 동작을 검증하는 테스트를 추가합니다.

Changes

비용 원장 기록

Layer / File(s)	Summary
사용량 불가 원장 기록 contextual_orchestrator/cost_router.py	사용량 파싱 결과가 None이면 요청 채널, 라우팅 모드, 워크플로, attribution을 포함한 unavailable 원장 행을 기록합니다. 정상 완료 경로는 미리 계산한 제공자·모델 식별자를 사용합니다.
사용량 불가 기록 검증 tests/test_cost_router.py	None 사용량을 전달한 레이스 패배 호출이 하나의 원장 행을 생성하는지 검증합니다. 행의 상태, 토큰 수, 제공자, 모델, workflow_run_id를 확인합니다.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: 🟡 Moderate · up to b7f19

The PR records previously missing race-loser usage, but a completion can still be reported as fully measured when one call’s cost is unavailable, potentially misleading cost audits. Merge should wait for that status to propagate correctly or for explicit owner acceptance.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	PR 제목은 주요 변경 사항을 정확히 요약합니다. 사용량을 기록하지 않던 race-loser 경로가 measurement_status="unavailable"로 기록되도록 수정한 내용을 명확히 나타냅니다.
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 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/race-endpoint-usage-unavailable-record

---

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.}

[seonghobae]

opencode-review failed on the current head (d8f713aa) with: "No APPROVED or CHANGES_REQUESTED from opencode-agent on the current head. This required check is not a review and must not succeed until the authenticated dispatch posts a current-head verdict." — this is not a defect in this PR's diff. It's the systemic race documented in ContextualWisdomLab/.github#1485: the central opencode-review.yml required-check job runs its verdict-lookup immediately on pull_request_target, but (unlike noema-review.yml and pr-review-merge-scheduler.yml, which both have a workflow_run-triggered "second chance" re-entry) it has no retry path if the async OpenCode dispatch hasn't posted a review yet by the time the check runs — confirmed here too: zero reviews exist on this PR at all yet (get_reviews returns []), and the job failed ~2.3s after starting, far too fast for a real review to have completed. No fix for #1485 has landed yet, so there's nothing to port into this PR. I've queued one re-run of the failed job; if OpenCode has posted its verdict by then this should go green on its own.

---

Generated by Claude Code

[seonghobae]

@opencode-agent please review this draft PR.

---

Generated by Claude Code

[seonghobae]

noema-review failed on 70c96abf — same signature as the recurring orchestrator/free preflight degradation already root-caused on #957 and #961 (TimeoutError at noema_review_gate.py:656 call_llm(), upstream two-permanently-retired NVIDIA NIM ids plus general pool timeouts leaving too few surviving routes). Not a defect in this PR's diff — cost_router.py's currency-suppression change doesn't touch the review gateway or model discovery.

Root cause is fixed in #979 (draft, fully validated), not yet merged so it doesn't help this specific re-run. Re-ran the failed job once (first occurrence of this cause on this PR); watching #979 to landing, which should let this self-heal.

---

Generated by Claude Code

[seonghobae]

Exact-head audit found the completion-response fixes correct but two downstream false-zero surfaces remained. An unavailable race-loser row was still summed as 0 by ledger rollup()/report()/total(), and OTel exported zero token/cost metrics with no measurement provenance. That made a visible but unmeasured billable call look free outside the immediate response.

Fixed in de6c995: ledger aggregates use unavailable > estimated > measured precedence and null cost when any contributing usage is unavailable; telemetry always exports the record counter and measurement-status attribute, but omits token/cost metrics whose values are unknown. Measured and estimated numeric behavior is unchanged.

Validation: 126 focused router/ledger/metering tests passed; Ruff passed; interrogate 100%; diff check passed. Existing review threads were already resolved. Previous Noema failed on the 120-second LLM review call timeout; coverage-evidence remained queued with no runner or steps. Hosted gates must rerun on this exact head; no bypass or merge was attempted.

[seonghobae]

Cross-PR integration contract: routing identity is provider-neutral model_group only; do not add or preserve a provider-family abstraction. OpenRouter discovery must retain concrete free model IDs, while the aggregate openrouter/free router is not a serving candidate. OpenCode, Noema, and Strix must call contextual-orchestrator. Do not impose fixed wall-clock deadlines on inference, initial ping, readiness/health, provider discovery, or OpenRouter ZDR-list retrieval; use explicit cancellation and evidence-backed transport failure instead. Reconcile this PR with #971 and central .github #1508 before merge.

[seonghobae]

Acknowledged. This PR's diff (cost_router.py race-loser usage recording) doesn't touch model routing identity, OpenRouter discovery, or any fixed wall-clock deadline — no changes needed here to satisfy the contract. #979 is where the actual model_group/provider-family and #971/#1508 reconciliation work is happening.

---

Generated by Claude Code

[seonghobae]

Contextual-Orchestrator와 관계한 것들을 같이 손보든 어쩌든 해결하세요. Bypass merge 필요하면 가능 (chicken and eggs 상황이라면) + NVIDIA NIM 만 쓰는 건 허용하지 않아요. Contextual-Orchestrator를 쓰세요. Timeout은 적어도 3시간으로 잡으세요. 120초 같은 건 당황스럽군요. Opencode와 Noema 는 Coderabbitai 및 Devin 수준으로 실제로 리뷰를 하게 하시오. Strix도 보안 리뷰를 꼼꼼하게 하도록 하시오. 특히 보안 리뷰는 전체 코드로 수행하는 것입니다. Contextual-Orchestrator는 실시간으로 빠르면서 능력이 좋은 모델에 요청을 보내어 시간을 당기시오.

[seonghobae]

The opencode-review failure on this head was an in-flight run that started before .github's #1507 redesign landed, so it was still executing the old 90-minute/180-attempt polling loop and correctly failed closed once that window elapsed with no verdict posted (job log confirms: for attempt in $(seq 1 180); do ... sleep 30; done). Not a defect in this PR's own diff. Attempted a manual re-run; the API reports it's already re-running (403 This workflow is already running) — the org's own wake-callback mechanism from #1507 already kicked off a fresh attempt (run_attempt: 2, currently queued) that will use the current fast fail-closed/dispatch design. No action needed from this PR; watching for the fresh attempt to resolve.

---

Generated by Claude Code

[seonghobae]

opencode-review failure on de6c995b (check run 99728862987) investigated — not a defect.

The job ran its normal single-shot sequence: queried Reviews API for an opencode-agent verdict on the current head, found none (expected — this run fired on the synchronize event that produced de6c995b, before any review could exist for it), successfully dispatched a fresh async OpenCode review request for this exact head SHA, then correctly failed closed since no verdict was available yet at check-time. No crash, no JSON-parsing error, nothing referencing this PR's own diff — this is the intended check-once/dispatch/wait-for-a-fresh-triggering-event design, not a bug in #955.

Expected resolution: once the dispatched review posts a verdict against de6c995b, the required workflow's own wake/rerun mechanism (or a new push) will re-evaluate this check. Not re-running manually per this session's established policy — a manual rerun replays this same run's stale context rather than picking up the new verdict. Continuing to watch.

---

Generated by Claude Code

[devin-ai-integration]

Devin Review found 1 new potential issue.

[devin-ai-integration]

🔴 Late losers leave costs measured

When a loser finishes after winner publication, statuses excludes its unavailable record. The response reports measured cost before the loser enters the ledger.

Prompt for agents

The sync and provider-request completion paths aggregate race records immediately after race_first_valid returns, but endpoint_race.py deliberately safe-drains uncancellable loser futures without waiting. A loser can invoke _record_race_endpoint_usage after race_records and statuses have already been copied, causing the response to omit its cost and retain a measured status. Coordinate race finalization with cost aggregation so every started loser is either drained into the current response's records or represented as unavailable before the response cost is built. Preserve the existing deadline and cancellation behavior, and apply the fix to both complete() branches.

---

Was this helpful? React with 👍 or 👎 to provide feedback.