Skip to content

fix(acp): use reported context fill instead of summed turn tokens - #5692

Open
jiuyueQ wants to merge 1 commit into
OpenHands:mainfrom
jiuyueQ:fix/issue-5687-acp-context-usage
Open

jiuyueQ wants to merge 1 commit into
OpenHands:mainfrom
jiuyueQ:fix/issue-5687-acp-context-usage

Conversation

@jiuyueQ

@jiuyueQ jiuyueQ commented Oct 10, 2026 •

Copy link
Copy Markdown

HUMAN:

我希望通过这个 PR 参与 OpenHands 的改进。ACP 把多次模型调用的 token 总量显示成当前上下文用量,容易让用户误以为窗口已经满了。这次代码修复和测试由 Codex 协助完成,目标是让用量显示与服务器实际报告的上下文占用保持一致。


AGENT:

Why

ACP prompt-response token counts can sum many model calls within a turn. Using that sum as context fill can report 1,301,000 tokens against a 1,000,000-token window even when the server reports only 46,000 tokens currently in context.

Summary

  • Use the turn's UsageUpdate.used for the existing latest and accumulated per_turn_token fields, including zero and updates without prompt-response token counts.
  • Preserve the input-plus-output fallback when no usage update arrives, all accounting token buckets, context-window handling, and cost tracking. No schema or public API changes.
  • Add focused observable telemetry regressions and a temporary .pr/reproduce_acp_context_usage.py that exercises a real ACP subprocess through Conversation.run().

Issue Number

Fixes #5687 (SDK context-fill scope).

How to Test

Validated on Ubuntu 20.04 x86_64, Python 3.13.12, uv 0.11.2, with the original frozen workspace dependencies, including LiteLLM 1.93.0.

uv sync --frozen --dev
uv run pre-commit run --files openhands-sdk/openhands/sdk/agent/acp_agent.py tests/sdk/agent/test_acp_agent.py
LITELLM_LOCAL_MODEL_COST_MAP=True uv run pytest tests/sdk/agent/test_acp_agent.py tests/sdk/llm/test_llm_metrics.py -q --timeout=60
LITELLM_LOCAL_MODEL_COST_MAP=True uv run python .pr/reproduce_acp_context_usage.py --expected-context 46000

All file pre-commit checks passed; 534 tests passed. With only the regression tests applied to base 1cfba2117e92730b0fe1c32520a39aad8611bb8c, five telemetry assertions failed and 12 passed. After this fix, all 17 telemetry cases pass.

The same live SDK workflow and offline JSON-RPC ACP subprocess produced these results before and after the fix:

Metric Base Fixed
Latest per_turn_token 1,301,000 46,000
Accumulated per_turn_token 1,301,000 46,000
Context window 1,000,000 1,000,000
Prompt tokens 1,300,000 1,300,000
Completion tokens 1,000 1,000
Cost (USD) 0.25 0.25

This uses a faithful offline ACP server sending the reported UsageUpdate and PromptResponse payload shapes. It verifies subprocess startup, protocol parsing, session updates, the agent step, and exposed metrics. It does not run Hermes, a paid model, or Agent Canvas. LITELLM_LOCAL_MODEL_COST_MAP=True uses the bundled pricing map during offline validation.

Video/Screenshots

Nonvisual SDK metric fix; reproducible runtime output is summarized above.

Design Doc

Small ACP-only value correction using existing fields; no separate design document.

Type

  • Bug fix
  • Feature
  • Refactor
  • Breaking change
  • Docs / chore

Notes

Implemented and validated with AI assistance (Codex). The HUMAN placeholder is preserved for the human contributo.

Companion SDK documentation: OpenHands/docs #922.

The optional Condense UI change in the issue is outside this SDK fix's accepted scope.

Co-authored-by: openhands <openhands@all-hands.dev>
@github-actions

Copy link
Copy Markdown
Contributor

📁 PR Artifacts Notice

This PR contains a .pr/ directory with temporary PR-specific documents. Because this is a fork PR, the workflow will open or update a cleanup PR against main after merge.

@jiuyueQ
jiuyueQ marked this pull request as ready for review October 10, 2026 01:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: ACP context indicator uses summed turn usage instead of UsageUpdate.used

1 participant