fix(tokenizer): never start a vocab load that can only be fetched - #34
Conversation
0.8.1 moved `import tiktoken` off the daemon thread and added a 1 s atexit join. That budget covers a cache hit (~140 ms) and nothing else: on a miss, `get_encoding` does an unbounded `requests.get` plus a BPE parse, so the thread is still mid-flight when the join expires and gets killed anyway — out of the dynamic linker, into `malloc`. Reproduced by repeating the two serve protocol tests with the cache guaranteed empty: 1 of 90 `test_serve_subprocess_e2e` runs died with `double free or corruption (fasttop)` / `-6` after a fully correct protocol stream, against 0 of 60 warm and 0 of 260 blocked-egress. So the earlier fix covered only the warm half — the half CI sees, since warming the cache is the standard mitigation. So when the cache can serve nothing, start no thread: mark the encoding unavailable, warn once with the fix, and stay on chars/4. Token counts go approximate on hosts that never warmed the cache; AGENT_CORE_TIKTOKEN_FETCH=1 opts back into the fetch and its window.
…ing the harness suite
|
Verification follow-up — two things the original description did not cover. 1. AgentCore's own venv has no
The crash's precondition is what is gone: in the cold arm no 2. A consumer break, found by running the ApodexHarness suite against this branch with That also makes the point that this is invisible on a warm host — the same reason 0.8.1 looked complete. |
… hash
Two things the gate left unsaid.
**Which estimator the fallback is.** The two warnings, the module docstring
and the changelog all called it "chars/4". It is not:
`context_budget.estimate_tokens` counts each CJK character as a token and
divides only the non-CJK remainder by 4 — that replacement of the old flat
`len//4` is the whole subject of `tokens.py`'s docstring. The wording also
pointed the wrong way: "chars/4" reads as "CJK collapses to a quarter", and
a reader who knows the old 4x CJK under-count would conclude a Chinese
workload loses its context guard outright. Measured against cl100k_base:
pure Chinese 0.84x of the real count
mixed Chinese/Latin 0.88x
English prose 1.14x
JSON tool arguments 0.72x
It does err low on CJK, and lower on argument-heavy JSON, by 12-28% rather
than 4x — the guard's 1.5x buffer is what covers that. The changelog now
carries the numbers, because "approximate" alone leaves a host unable to
judge its own hard gateway limit.
**What proving a local load costs.** The gate SHA-256s the cached vocab
(~1.7 MB for cl100k_base, ~2 ms measured) on the calling thread, once per
encoding per process. This module's whole subject is what may and may not
run there, and 0.8.1 documented the 25 ms import for exactly that reason, so
the hash belongs in the same paragraph — including why tiktoken's own
constructor cannot do the verifying: calling it is the fetch being avoided.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`_CACHE_SPECS` pins tiktoken's cache key and expected content hash, and nothing could falsify either. Every existing test monkeypatches the table and writes a payload whose hash it just computed, so a typo in either constant — or a tiktoken release that re-publishes a vocab — would leave the gate permanently shut with the whole suite green. The failure is silent by construction: exact token counts quietly become heuristic ones, and the one WARNING blames the host's cache for a directory that is correctly warmed. The truth is readable without touching the network: `tiktoken_ext.openai_public.cl100k_base`'s source carries the blobpath and the `expected_hash` its loader validates against, and the cache key is `sha1(blobpath)` per `read_file_cached`. Reading the source rather than calling the constructor matters — calling it is the fetch this module exists to avoid. The test loops `_CACHE_SPECS`, so a future encoding is covered by adding it there. Negative-controlled: corrupting either pinned constant by one character turns it red. CI gets tiktoken for this one test through `--with`, deliberately not in the shared environment. Installing it suite-wide turned `test_agent_loop_engine.py::test_synthesised_zero_usage_does_not_reset_the_token_estimate` red, because that test's arithmetic is calibrated on the heuristic: its 4000-char filler is ~1000 heuristic tokens and far fewer real ones. That hazard now has a changelog paragraph of its own, since any host asserting on token totals inherits it the moment a cache goes warm. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
0.8.2 narrowed the Tier 1 card from 3 source URLs to 1 and told consumers that a host needing several independent sources per claim "should now raise `_MINI_CARD_MAX_URLS` deliberately". That knob did not exist: the constant is module-private and read at import, so the only ways to get the old width back were monkeypatching a private name or forking the compactor. `KeepLastNToolResultsCompactor(max_card_urls=...)` threads through to `_elided_tool_card(..., max_urls=...)`. The default is `_MINI_CARD_MAX_URLS`, so nothing changes for a caller that does not pass it — the measured 99.2% first-URL coverage stays the default, and the shim in MiroHarness needs no edit (a new keyword argument on a public class is backward compatible). Negative values raise at construction, alongside the existing `keep_tool_result` guard; 0 is legal and drops the card's source line while keeping the call and any argument URL. Tests: 4 cases in tests/test_keep_last_n_compactor.py — the default is still 1, a raised value actually retains that many URLs while staying inside the body budget, 0 drops the source line, and -1 is rejected.
What
0.8.1 moved
import tiktokento the caller thread and added a 1 satexitjoin. That covers a cache hit, but not a cache miss:
get_encoding()can performan unbounded network fetch plus BPE parsing, leaving its daemon thread alive when
the interpreter exits. Repeated cold-cache protocol tests reproduced one
post-response
double free or corruption (fasttop)/-6failure in 90 runs.This PR starts no tokenizer thread unless the requested
cl100k_baseartifact isalready cached and passes tiktoken's expected SHA-256. Missing, disabled,
unrelated, corrupt, oversized, FIFO, and other non-regular cache
entries fail closed to the callers' CJK-aware heuristics. Operators can warm the
cache during build/setup, or explicitly restore runtime fetching with
AGENT_CORE_TIKTOKEN_FETCH=1and accept the original exit-time risk.Safety details
cl100k_base, the only encodingrequested by AgentCore.
accepted only when regular, and read at most 4 MiB before hashing. A FIFO or a
concurrently growing file cannot turn validation into an unbounded read.
return
Noneinstead of repeating the filesystem work.exercises tiktoken's actual cache lookup with network reads stubbed out. CI and
release jobs run it in an isolated environment using the tokenizer version in
uv.lock.Consumer impact (MINOR)
A host with no valid warm cache now remains on approximate token counts instead
of fetching once and eventually becoming exact. The fallback is not a hard upper
bound: measured heuristic/real ratios were 0.84x for Chinese, 0.88x mixed
Chinese/Latin, 1.14x English prose, 0.72x JSON tool arguments, and roughly
0.10-0.125x for emoji-heavy samples. The context guard's 1.5x buffer does not
cover every input. Hosts sizing against a hard provider limit should warm the
cache.
Images that bake
TIKTOKEN_CACHE_DIRremain on exact counts. If caching isdisabled with
TIKTOKEN_CACHE_DIR="", set it to a writable directory beforerunning:
python -c "import tiktoken; tiktoken.get_encoding('cl100k_base')"Tests that intentionally assert heuristic token arithmetic should pin that
estimator; this PR does so for the context-overflow regression test that was
previously sensitive to the host's cache state.
Verification
1505 passed, 1 skipped1506 passedgit diff --check: cleanVersion remains
0.9.0: this deliberately changes observable cold-cachebehavior and therefore crosses the repository's
0.MINOR.PATCHcompatibilityboundary.