Skip to content

feat: naive token estimation via tiktoken#2031

Open
lizradway wants to merge 1 commit intostrands-agents:mainfrom
lizradway:token-estimation
Open

feat: naive token estimation via tiktoken#2031
lizradway wants to merge 1 commit intostrands-agents:mainfrom
lizradway:token-estimation

Conversation

@lizradway
Copy link
Copy Markdown
Member

@lizradway lizradway commented Apr 1, 2026

Description

  • Adds _estimate_tokens() method to the Model base class for estimating input token count before sending to the model, enabling proactive context management (e.g., triggering compression at a threshold)
  • Uses tiktoken (cl100k_base encoding) as a universal fallback for all 11 providers — individual providers can override with native counting APIs later
  • Handles all content block types: text, toolUse, toolResult, reasoningContent, guardContent, citationsContent; gracefully skips non-serializable content (e.g., image bytes)
  • Adds tiktoken as a hard dependency

Related Issues

#1294

Documentation PR

This should be internally facing, documentation not required

Type of Change

New feature

Testing

How have you tested the change? Verify that the changes do not break functionality or introduce warnings in consuming repositories: agents-docs, agents-tools, agents-cli

  • I ran hatch run prepare

Checklist

  • I have read the CONTRIBUTING document
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@codecov

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 1, 2026

Review Summary (Round 2)

Assessment: Approve ✅

All previously identified blocking issues have been addressed in this update.

Changes Verified
Previous Issue Resolution
API review label required ✅ Method renamed to private _estimate_tokens() - no API review needed
Documentation PR required ✅ Private method is correctly internal-facing
Encoding caching suggestion ✅ Implemented _get_encoding() with _cached_encoding
Docstring expansion ✅ Now includes limitations and override guidance
Missing guardContent test ✅ Added test_estimate_tokens_guard_content_block
Additional Improvements
  • Added edge case tests for non-serializable tool specs
  • Added test for toolUse with binary data
  • Comprehensive test coverage for all content block types

Note: Codecov shows 92.85% patch coverage with 4 partial lines - these are likely error handling branches (except clauses) which are acceptable.

Good implementation with clean code structure, appropriate error handling, and thorough test coverage. 👍

@lizradway lizradway marked this pull request as ready for review April 1, 2026 15:34
@lizradway lizradway added the area-context Session or context related label Apr 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-context Session or context related size/m

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant