Skip to content

fix(agent): make cost_status sticky by priority (#67764) - #67804

Open
JonthanaHanh wants to merge 1 commit into
NousResearch:mainfrom
JonthanaHanh:fix/cost-status-sticky-priority
Open

fix(agent): make cost_status sticky by priority (#67764)#67804
JonthanaHanh wants to merge 1 commit into
NousResearch:mainfrom
JonthanaHanh:fix/cost-status-sticky-priority

Conversation

@JonthanaHanh

Copy link
Copy Markdown
Contributor

Problem

cost_status was "most-recent-call-wins" everywhere -- a single provider hiccup could downgrade a session's cost accuracy from "actual" to "estimated" even though 99% of calls were authoritative.

Four layers all unconditionally overwrote the stored value:

  1. agent/conversation_loop.py:2321 -- agent.session_cost_status = cost_result.status
  2. agent/codex_runtime.py:150 -- same unconditional assignment
  3. hermes_state.py:2887,2908,3087 -- COALESCE(?, cost_status) takes incoming when non-null
  4. agent/insights.py:583 -- d["cost_status"] = status overwrites per-model dict

Fix

Add sticky_cost_status() helper in agent/usage_pricing.py with priority order:

actual (4) > included (3) > estimated (2) > unknown (1)

Apply in all three Python call sites. The higher-priority status always wins. Equal priority keeps the incoming value (latest wins).

The SQL COALESCE(?, cost_status) layer is not changed -- it correctly stores whatever Python passes, and the Python side now computes the sticky value.

Files Changed

  • agent/usage_pricing.py -- add _COST_STATUS_PRIORITY map and sticky_cost_status() helper
  • agent/conversation_loop.py -- use sticky_cost_status() instead of direct assignment
  • agent/codex_runtime.py -- same
  • agent/insights.py -- use sticky_cost_status() when aggregating per-model status

Fixes #67764

cost_status and cost_source were most-recent-call-wins everywhere:
a single provider hiccup downgrading 'actual' to 'estimated' would
clobber the session-level status even though 99% of calls were
authoritative.

Add sticky_cost_status() helper with priority order:
  actual > included > estimated > unknown

Apply in conversation_loop.py, codex_runtime.py, and insights.py.

Fixes NousResearch#67764
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint area/billing Account usage, credit usage, billing (cross-cutting) area/usage-cost Token accounting, usage reporting, billing, cost tracking needs-decision Awaiting maintainer decision before any implementation sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 20, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #67790 and #67770. #67790 implements the same sticky-status mechanism across the persistent SQL and runtime layers, while this PR is a narrower Python-only subset; maintainer consolidation is needed.

@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 isolating the runtime and /insights overwrite sites. The current change is incomplete for the reported persisted-accounting guarantee.

Problems

  • The new runtime assignment does not affect the values passed to persistence: current main still calls update_token_counts(..., cost_status=cost_result.status) in agent/conversation_loop.py:2956 and agent/codex_runtime.py:166. SessionDB then applies latest-non-null semantics at hermes_state.py:4669, hermes_state.py:4690, and hermes_state.py:4869.
  • The PR changes no tests. The priority rule needs behavioral coverage across the helper, both SessionDB update modes, the per-model UPSERT, runtime accounting, and /insights.

Suggested changes

  • Enforce the priority rule in the SessionDB aggregate and per-model accumulation paths, not only in the two in-memory fields.
  • Add regression tests for promotion and non-downgrade sequences such as estimated → actual → estimated.

Automated hermes-sweeper review.

Comment thread agent/codex_runtime.py
if cost_result.amount_usd is not None:
agent.session_estimated_cost_usd += float(cost_result.amount_usd)
agent.session_cost_status = cost_result.status
agent.session_cost_status = sticky_cost_status(

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 makes the in-memory field sticky, but the later update_token_counts() call still receives raw cost_result.status (current main agent/codex_runtime.py:166), and SessionDB overwrites stored status with that non-null value (hermes_state.py:4690, 4869). Please enforce the same rule at persistence so restarted sessions and /insights retain the guarantee.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 30, 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) area/usage-cost Token accounting, usage reporting, billing, cost tracking comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit 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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: cost_status overwritten on every API call across SQL, in-memory, and /insights aggregations

3 participants