fix(agent): sticky cost_status priority ladder (#67764) - #67790
Open
DavidMetcalfe wants to merge 2 commits into
Open
fix(agent): sticky cost_status priority ladder (#67764)#67790DavidMetcalfe wants to merge 2 commits into
DavidMetcalfe wants to merge 2 commits into
Conversation
Collaborator
Replaces the previous "most-recent-call-wins" semantics across all four layers of cost_status aggregation: - SQL COALESCE in hermes_state.py at three sites (sessions aggregate absolute + incremental paths, session_model_usage ON CONFLICT) - In-memory attribute writes in agent/conversation_loop.py:2321 and agent/codex_runtime.py:150 - agent/insights.py:583 per-model dict aggregation - tools/delegate_tool.py:2862 subagent-fold guard (now via the same helper for codebase consistency) Sticky-max rule: actual (rank 3) > included (rank 2) > estimated (rank 1) > unknown (rank 0). Once any call has reported actual, the accumulated row stays actual forever — even if subsequent calls report estimated or unknown. SQL injection safety via allow-list sanitization of the interpolated new value. The new sticky_cost_status helper lives in agent/usage_pricing.py alongside the CostStatus literal. cost_source is deliberately unchanged at all four sites (per the brief's Layer 4: no clean priority ladder). Closes NousResearch#67764 Tests: tests/test_cost_status_priority_ladder.py (30 tests covering helper, SQL, dispatch shape, dispatch behavior, end-to-end brief scenarios). 174+ existing adjacent tests pass with no regressions.
DavidMetcalfe
force-pushed
the
fix/cost-status-priority-ladder
branch
from
July 20, 2026 01:53
4e004cd to
0c14524
Compare
11 tasks
1 task
1 task
When cost_status=None (token-only calls with no new cost information), the SQL CASE ladder was coercing None to 'estimated' and overwriting the accumulated status — diverging from both the Python sticky_cost_status() helper and the original COALESCE(?, cost_status) semantics. Fix: skip the sticky ladder when cost_status is None, falling back to COALESCE(NULL, cost_status) which preserves the existing row value. Also sanitize the INSERT binding in _record_model_usage() to match the ON CONFLICT DO UPDATE path — unrecognised non-None values now fall back to 'estimated' on initial insert instead of raw literal injection.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #67764.
Problem
cost_status is supposed to communicate the accuracy confidence of an
accumulated dollar value across a session's per-call rows. Today it's
overwritten by every new API call (latest-call-wins), so a session with
100 actual-confidence calls followed by 1 estimated call downgrades the
whole accumulated row to "estimated" — even though the user-visible
fact "the cost is whatever OpenRouter said" hasn't changed.
Three layers exhibit the bug: SQL UPDATE on sessions (lines 2887, 2908),
SQL UPSERT on session_model_usage (line 3087), and the in-memory
agent.session_cost_status writes at agent/conversation_loop.py:2321,
agent/codex_runtime.py:150. The /insights aggregator (agent/insights.py:583)
had the same shape in Python. tools/delegate_tool.py:2862 had a bespoke
"sticky-ish" guard which is unified onto the same helper.
Fix
Sticky-max rule across all layers: actual (3) > included (2) > estimated
(1) > unknown (0). Once any call has reported actual, the accumulated
row stays actual forever.
returns the higher-rank value (with sentinel -1 for missing/invalid
current, so any valid new wins; equal ranks return the new value; bad
values fall through to "estimated").
The interpolated new value is matched against a Literal allow-list
before embedding (SQL-injection safety). Verified by
test_sessions_status_with_unknown_literal_input.
agent/codex_runtime.py now call sticky_cost_status(...)
instead of direct =. agent/insights.py:583 does the same for the
per-model dict loop.
helper for codebase consistency (and so the rule evolves in one place).
Layer 4: no clean priority ladder for cost_source).
Tests
covering: rank mapping, sticky behavior on each pair of statuses,
return-type Literal guarantee, SQL UPSERT accumulation, sessions
aggregate SQL aggregation, SQL injection sanitization, in-memory
dispatch in conversation_loop / codex_runtime / insights / delegate_tool,
end-to-end brief reproduction scenarios.
tests/agent/test_insights.py, tests/tools/test_delegate*.py: no regressions.
Out of scope
a separate deferred issue. Once Phase 2 lands, the priority ladder
here will gain a real "actual" promotion path.
PR fix(agent): rehydrate session cost counters on agent construction (#67762) #67770 (rehydration). Rehydration stays correct; cost_status
reaches the agent via the same in-memory attribute paths and is
now also sticky across those writes.