Skip to content

fix: warn when falling back to the GPT-4 tokenizer - #341

Draft
kskszq wants to merge 1 commit into
ServiceNow:mainfrom
kskszq:fix/tokenizer-fallback-warning
Draft

kskszq wants to merge 1 commit into
ServiceNow:mainfrom
kskszq:fix/tokenizer-fallback-warning

Conversation

@kskszq

@kskszq kskszq commented Oct 13, 2026

Copy link
Copy Markdown

Problem

When both tiktoken lookup and HuggingFace tokenizer loading fail, get_tokenizer substitutes the GPT-4 tokenizer. The substitution is currently logged at INFO, so experiments using WARNING-level output can miss that their token counts are estimates for a different model.

Change

Promote the actual GPT-4 fallback message to WARNING, retaining the requested model and load error. Successful tokenizer resolution and fallback/caching behavior remain compatible. Add offline tests covering loading failure, missing optional transformers, cached fallback, and successful tiktoken/HuggingFace resolution.

Addresses the warning-level suggestion (1) in #340. This is a partial contribution: model-name normalization, run metadata and an opt-in strict mode are outside this PR, so the issue should remain open.

Validation

  • New regression tests against unmodified 8488c564: 3 failed, 2 passed, specifically because fallback warnings are absent.
  • After the change: 5 passed (tests/llm/test_tokenizer_fallback.py).
  • Black 24.10.0 check on the changed source and test files: passed.
  • git diff --check: passed.
  • Published file blob hashes match the locally tested files; the upstream diff contains only these two files.

Local validation used Python 3.13.7 and pytest 9.1.1, rather than the project's Python 3.11/3.12 and locked dependencies. A temporary bootstrap imported the real production module using existing dependencies, disabled unrelated pytest plugins, and supplied an import-only empty anthropic module because that SDK is unavailable locally; no Anthropic functionality was exercised. Tokenizer loaders are mocked by the committed tests to avoid downloads. This bootstrap is not part of the PR.

Draft pending CI: the full locked environment, browser benchmark suite and repository-wide Black/darglint checks were not run locally because shell network access is restricted.

Prepared with AI assistance.

This branch has not been deployed

No deployments
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.

1 participant