Skip to content

fix(conversation): count tokens from length-truncated responses - #38460

Open
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/token-count-length-truncation
Open

liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/token-count-length-truncation

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes token accounting for length-truncated responses. When finish_reason == "length" triggers a continuation retry, the tokens consumed by the truncated API call were never tracked — causing underreported usage in /insights and the session DB.

Related Issue

Fixes #38458

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • agent/conversation_loop.py: Track token usage (prompt, completion, total, cache, reasoning) and estimated cost from the truncated response in the restart_with_length_continuation handler, before boosting the output cap and continuing the loop.
  • tests/run_agent/test_run_agent.py: Add test_length_continuation_counts_truncated_tokens — verifies that both the truncated and continuation response tokens are counted in session totals.

How to Test

  1. Ask the model to produce a response that exceeds max_output_tokens (triggers finish_reason="length")
  2. After the continuation retry completes, check /insights — token counts should include both the truncated and continuation calls
  3. Run pytest tests/run_agent/test_run_agent.py::TestRunConversation::test_length_continuation_counts_truncated_tokens -xvs

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Code Intelligence

  • Analyzed: agent/conversation_loop.py:run_conversation — the restart_with_length_continuation handler (line ~3514)
  • Callers: AIAgent.run_conversation() → called by CLI, gateway, batch_runner, cron
  • Blast radius: LOW — only affects token accounting in the length-continuation path; no behavioral change to the retry/continuation logic itself
  • Related patterns: existing token tracking at line ~1879 (normal path) and line ~1712 (thinking-exhausted path); this fills the gap in the third early-exit path

When finish_reason is 'length' and the conversation loop retries with
a continuation prompt, the tokens consumed by the truncated response
were never tracked. This caused underreported usage in /insights and
the session DB.

Track tokens in the restart_with_length_continuation handler before
boosting the output cap and continuing the loop.

Fixes NousResearch#38458
@alt-glitch alt-glitch added comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint type/bug Something isn't working P3 Low — cosmetic, nice to have labels Jun 3, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for identifying the continuation-accounting gap. The premise remains valid on current main: a truncated text response sets _retry.restart_with_length_continuation at agent/conversation_loop.py:1948, and the outer handler continues at agent/conversation_loop.py:4227-4242 before normal accounting runs.

Problems

  • The patch predates the current accounting pipeline. Current normal-response processing at agent/conversation_loop.py:2059-2247 normalizes usage, updates the context engine, accumulates all session buckets, prices usage, and writes a per-call SessionDB delta. The proposed inline counter update at the old handler site would not preserve that full contract.
  • The added test verifies only in-memory aggregates, not the claimed SessionDB//insights result.

Suggested changes

  • Rework the salvage around the current continuation handler to reuse/extract the canonical accounting path.
  • Add a temp-SessionDB regression covering truncated plus continuation usage and persisted accounting.

Automated hermes-sweeper review.

@@ -3512,6 +3512,35 @@ def _perform_api_call(next_api_kwargs):
continue

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Current main's normal accounting also updates the context engine and writes a per-call SessionDB delta (agent/conversation_loop.py:2059-2247). Please salvage this through that canonical pipeline rather than maintaining a reduced parallel counter/cost update here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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-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]: The token consumed is not counted if the api call fails for "length" reason.

3 participants