Skip to content

fix(telemetry): persist first accounted fallback route - #62302

Closed
mijanx wants to merge 2 commits into
NousResearch:mainfrom
mijanx:fix/first-accounted-fallback-route
Closed

fix(telemetry): persist first accounted fallback route#62302
mijanx wants to merge 2 commits into
NousResearch:mainfrom
mijanx:fix/first-accounted-fallback-route

Conversation

@mijanx

@mijanx mijanx commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Problem

A session row is created with the requested primary model before the first API call. If that primary fails before producing any accounted usage and automatic fallback succeeds, update_token_counts() currently keeps the requested model via COALESCE(model, ?), while other route fields may describe the fallback.

That can persist an incoherent pair such as a GPT model with custom:zai, even though every accounted API call used the fallback route.

Fix

Treat the first accounted usage event as authoritative only while the existing session has api_call_count == 0:

  • replace the pre-created requested model and billing_provider together;
  • carry over fallback billing_base_url and billing_mode when supplied;
  • preserve the existing first-accounted route after any API call has already been recorded.

This keeps the legacy aggregate row internally coherent without pretending it can represent mixed-provider usage.

Scope and related work

This is intentionally narrower than the other open fallback/usage PRs:

This PR only fixes the zero-accounted-call case. It does not attempt to attribute mid-session mixed usage in the single legacy row.

Tests

Added regression coverage proving that:

  1. the first successful fallback replaces the unaccounted requested route with one coherent fallback route;
  2. a later fallback does not rewrite a route after primary usage has already been accounted.

Validation on current main:

python -m pytest tests/test_hermes_state.py -q
341 passed in 6.29s

git diff --check passes for both changed files.

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint telemetry Touches outbound telemetry, usage attribution, or analytics — needs opt-in gating before merge area/billing Account usage, credit usage, billing (cross-cutting) sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state P3 Low — cosmetic, nice to have labels Jul 10, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the focused reproduction and narrow scope. I found one persistence issue that needs resolution.

Problems

  • hermes_state.py:2524 overwrites only sessions.model. TUI resume chooses that value before model_config.model (tui_gateway/server.py:2245), but chooses model_config.provider before billing_provider (tui_gateway/server.py:2251-2257). A session whose primary config remains in model_config can therefore resume with the fallback model paired with the primary provider.
  • Automatic fallback is explicitly turn-scoped: restore_primary_runtime() restores the primary model/provider on the next turn (agent/agent_runtime_helpers.py:1138-1169). Persisting the transient fallback as the resume model conflicts with that lifecycle.

Suggested changes

  • Separate first-call accounting attribution from the durable runtime selection, or reconcile model, model_config, and resume precedence as one atomic contract.
  • Add a regression covering a primary-configured TUI session, first-call fallback persistence, and _stored_session_runtime_overrides().

Automated hermes-sweeper review.

Comment thread hermes_state.py
if first_accounted_route:
conn.execute(
"""UPDATE sessions
SET model = ?, billing_provider = ?,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes the value TUI resume treats as authoritative for the model (tui_gateway/server.py:2245) without updating model_config, whose provider takes precedence over billing_provider (tui_gateway/server.py:2251-2257). A primary-configured session can resume with this fallback model paired with the primary provider; please preserve a coherent resume contract or separate accounting attribution from runtime selection.

@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #62610 after incorporating your first-accounted fallback-route fix into the per-model usage work, with your authorship preserved in git history. The landing tightens the trigger to genuinely accounted usage, clears stale route fields atomically, and covers zero-usage Codex turns. Thanks for identifying the legacy summary-row gap and supplying the focused regression shape.

#62610

@teknium1 teknium1 closed this Jul 11, 2026
@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/billing Account usage, credit usage, billing (cross-cutting) comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state telemetry Touches outbound telemetry, usage attribution, or analytics — needs opt-in gating before merge type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants