Skip to content

fix(blackbox): two-tier char->token divisor (fixed=4.0, non-fixed=3.5) - #43

Merged
Kyzcreig merged 8 commits into
mainfrom
fix/two-tier-char-divisor
Jun 15, 2026
Merged

Kyzcreig merged 8 commits into
mainfrom
fix/two-tier-char-divisor

Conversation

@Kyzcreig

Copy link
Copy Markdown
Collaborator

What

The /context composition estimate divided all buckets by 3.5, over-counting the fixed prefix (system prompt + tool schemas + skills index) by ~17-24%.

Why / evidence

Measured the real fixed-context chars/token with live turns.db data:

  • o200k (gpt-5.x): 4.09 chars/tok
  • claude-opus: 4.35 chars/tok (n=60 each, near-zero-history full-tool turns)

The non-fixed tail (history/tool-results/tool-args) genuinely packs denser (~3.5) — confirmed by the existing under-count note on high-context turns. So a single divisor can't be right for both; split it.

Change

  • COMPOSITION_CHARS_PER_TOKEN_FIXED (default 4.0, env HERMES_COMPOSITION_CHARS_PER_TOKEN_FIXED, 2.0-8.0 clamp) + _ceil_chars_to_tokens_fixed sibling helper.
  • compose_request_breakdown: sys/skills/tool_schema → /4.0; history/tool_result/tool_arg stay /3.5.
  • estimate_request_tokens_rough: system+tools → /4.0; messages stay /3.5.
  • chat_completion_helpers.py deliberately untouched (mixed-payload layer; can't bucket).
  • Display-only; never billing or provider prompt_tokens.

Acceptance (single asymmetric band)

measured/est(/4.0) ∈ [0.90, 1.05] — claude-opus 1.014, gpt-5.x 0.978. Fixed estimate drops ~12-17% and now tracks measured occupancy.

Tests (28 in file, 89 in composition slice, all green)

Per-bucket /4.0-vs-/3.5 assertions; all-three-non-fixed-stay-3.5 negative test (catches mis-bucketing that fixed+nonfixed==total would miss); collapse guard (equal chars → different tiers); identity+skills==sys invariant; env-override + out-of-range fallback.

Spec: 2026-06-14_two-tier-char-divisor-SPEC.md (v2.1, 2-pass PRD review).

Kyzcreig added 8 commits June 14, 2026 18:57
- _DirectRestMem0Client: POST /memories, POST /search, GET /memories, X-API-Key
- MEM0_HOST set -> direct-REST; unset -> byte-identical cloud MemoryClient (back-compat)
- B3/B4: client scopes user_id AND agent_id even when caller omits them (shared-store safety)
- test hygiene: setattr on real mem0 module, not sys.modules swap (no cross-test pollution)
The mem0 SDK builds httpx.Client with no connection limits or keepalive
expiry, so server-half-closed idle keepalive sockets accumulate as
CLOSE_WAIT fds in the long-lived gateway process and are never reaped.
On a gateway with the launchd-default 256-fd soft limit this starved the
process (49 CLOSE_WAIT to api.mem0.ai observed), so curl/ssh subprocess
spawns and new sockets began failing with EMFILE — surfacing as e.g.
/claude_usage reporting all proxies 'no response' at once.

Pass a bounded httpx.Client (max 10 conns / 5 keepalive / 30s expiry);
the SDK fills in base_url + auth headers on the client we hand it.
hermes_state creates a second FTS5 index using the 'trigram' tokenizer with
INSERT/UPDATE/DELETE triggers on messages. The existing guard only handles
FTS5 being entirely absent ('no such module: fts5') — not the narrower case
where FTS5 works but the trigram tokenizer specifically is missing (e.g. an
anaconda-3.7 python's bundled SQLite). Under such an interpreter, every message
write fired the trigram trigger and aborted with 'no such tokenizer: trigram'.

This bit the safe-restart watcher's append_handoff when the watcher ran under a
bare 'python3' that resolved to anaconda (the PATH-poison trap): the transcript
append silently failed (auto-resume's resume-flag path masked it).

Add _sqlite_supports_trigram() probe + _is_trigram_unavailable_error(). When the
tokenizer is absent: skip creating the trigram table/triggers AND drop any
pre-existing trigram triggers so core message persistence continues; the main
FTS index and search are untouched (CJK/substring search degrades to LIKE).
A later open under a trigram-capable runtime recreates and backfills. Warns once.

Verified: under a forced no-trigram runtime, a message write succeeds, trigram
triggers are dropped, main FTS still indexes the row. 641 state tests pass.
- bound the cloud-fallback MemoryClient httpx pool (limits + keepalive_expiry=30s)
  so idle keepalive sockets don't rot into CLOSE_WAIT and leak fds in a long-lived
  gateway. Self-hosted path uses _DirectRestMem0Client (stdlib urllib, per-call
  close) which is leak-proof by construction; graceful degrade if httpx absent.
- update fallback test to assert the bounded client + limits are passed.
- add real soak test: 2000 add/search round-trips through the real direct-REST
  client against a live loopback server, assert open-fd count plateaus (the actual
  regression test; leak-sim proves the assertion trips on a real leak).
# Conflicts:
#	plugins/memory/mem0/__init__.py
…r fix)

The live cutover surfaced CERTIFICATE_VERIFY_FAILED: the self-hosted endpoint
mem0.ace is served by Caddy with a leaf signed by the private 'Ace Local Root CA',
which fleet hosts don't carry in their system trust store, so the urllib direct-REST
client (unlike curl -k) couldn't verify it.

Add optional ca_bundle (MEM0_CA_BUNDLE / mem0.json ca_bundle): when set AND host is
https, build an ssl context from the bundle so TLS verification works without
mutating every host's system trust store. Unset/http -> None (urllib system default,
unchanged). Test stubs updated for the new context= kwarg; regression test mints a
real self-signed CA PEM and asserts context-built / None across https+bundle,
https-only, http+bundle.
…recall fix)

Live cutover surfaced 0 search results despite 630 aegis memories present: the
direct-REST _scope() injected BOTH user_id AND agent_id on reads, but historical
cloud memories were stored agent-scoped WITHOUT a user_id (628/630 aegis rows have
no user_id), so the AND-filter dropped them (agent-only=20 hits vs agent+user=2).

Reads (search/get_all) now scope to user_id ONLY (scope_agent=False), matching the
provider's _read_filters intent (cross-session recall) and the cloud-era behavior.
Writes (add) still inject both for attribution + B4 anti-global safety. An explicit
agent_id in a read filter is still honored. Tests updated to assert the corrected
read/write asymmetry.
The /context composition estimate divided ALL buckets by 3.5, which
over-counts the FIXED prefix (system prompt + tool schemas + skills index)
by ~17-24%. Measured the real fixed-context ratio with live turns.db data:
o200k/gpt-5.x = 4.09 chars/tok, claude-opus = 4.35 (n=60 each). The
non-fixed tail genuinely packs denser (~3.5), so split the divisor:

- New COMPOSITION_CHARS_PER_TOKEN_FIXED (default 4.0, env-tunable via
  HERMES_COMPOSITION_CHARS_PER_TOKEN_FIXED, same 2.0-8.0 clamp).
- New _ceil_chars_to_tokens_fixed sibling helper.
- compose_request_breakdown: sys/skills/tool_schema -> /4.0; history/
  tool_result/tool_arg stay /3.5. estimate_request_tokens_rough: system+
  tools -> /4.0, messages stay /3.5. chat_completion_helpers untouched
  (mixed-payload layer, can't bucket).

Acceptance: measured/est(/4.0) in [0.90,1.05] on both families (claude-opus
1.014, gpt-5.x 0.978). Fixed estimate drops ~12-17% and now tracks measured.

Tests: per-bucket /4.0-vs-/3.5 assertions, all-three-non-fixed-stay-3.5
negative test, collapse guard (equal chars -> different tiers), identity+
skills==sys invariant, env-override + fallback. 28 pass in the file, 89 in
the composition slice.
@github-actions

Copy link
Copy Markdown

🔎 Lint report: fix/two-tier-char-divisor vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 9932 on HEAD, 9924 on base (🆕 +8)

🆕 New issues (6):

Rule Count
unresolved-import 4
deprecated 1
invalid-assignment 1
First entries
tests/plugins/memory/test_mem0_selfhost.py:186: [unresolved-import] unresolved-import: Cannot resolve imported module `httpx`
tests/plugins/memory/test_mem0_selfhost.py:12: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
plugins/memory/mem0/__init__.py:433: [unresolved-import] unresolved-import: Cannot resolve imported module `httpx`
tests/plugins/memory/test_mem0_selfhost.py:173: [unresolved-import] unresolved-import: Cannot resolve imported module `mem0`
tests/plugins/memory/test_mem0_selfhost.py:464: [deprecated] deprecated: The function `utcnow` is deprecated: Use timezone-aware objects to represent datetimes in UTC; e.g. by calling .now(datetime.timezone.utc)
tests/plugins/memory/test_mem0_selfhost.py:300: [invalid-assignment] invalid-assignment: Object of type `def _fake_request(method, path, *, body=None, params=None) -> Unknown` is not assignable to attribute `_request` of type `def _request(self, method: str, path: str, *, body: dict[Unknown, Unknown] | None = None, params: dict[Unknown, Unknown] | None = None) -> Any`

✅ Fixed issues: none

Unchanged: 5128 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@greptile-apps

greptile-apps Bot commented Jun 15, 2026

Copy link
Copy Markdown

Greptile Summary

This PR introduces a two-tier char→token divisor for the /context composition estimate: fixed content (system prompt, tool schemas, skills index) now uses 4.0 chars/token instead of 3.5, correcting a ~17–24% over-count measured against live turns.db data on two model families. It also adds a self-hosted Mem0 OSS backend via a stdlib-only REST client and fixes a SQLite trigram-tokenizer availability probe to prevent message-write aborts on environments that ship FTS5 without the trigram extension.

  • agent/model_metadata.py: adds COMPOSITION_CHARS_PER_TOKEN_FIXED (default 4.0, env-tunable, 2–8 clamp) and _ceil_chars_to_tokens_fixed; wires sys/skills/tool-schema buckets to the new divisor; non-fixed buckets unchanged at 3.5.
  • plugins/memory/mem0/__init__.py: adds _DirectRestMem0Client (per-call urllib, CA-bundle SSL) activated by MEM0_HOST; also bounds the cloud SDK's httpx pool to prevent fd leaks.
  • hermes_state.py: probes trigram-tokenizer availability before installing triggers; drops existing trigram triggers and warns when absent, keeping core message persistence intact."

Confidence Score: 4/5

The divisor change is display-only and well-tested; the self-hosted mem0 path and trigram guard are additive. The one thing to fix before merging is the raw os.environ cleanup pattern in the out-of-range test.

The core model_metadata.py and hermes_state.py changes are clean and fully covered. The mem0 plugin adds a real new code path with good test coverage. The test for the fixed-divisor out-of-range fallback uses raw os.environ without try/finally, unlike the identical adjacent test which uses monkeypatch — a failure mid-loop would leave COMPOSITION_CHARS_PER_TOKEN_FIXED at an unexpected value for subsequent tests.

tests/test_request_composition.py (raw os.environ cleanup) and plugins/memory/mem0/init.py (api_key required flag in get_config_schema)

Important Files Changed

Filename Overview
agent/model_metadata.py Adds COMPOSITION_CHARS_PER_TOKEN_FIXED (default 4.0) constant, _ceil_chars_to_tokens_fixed helper, and wires sys/tool-schema buckets to the new divisor in both compose_request_breakdown and estimate_request_tokens_rough; non-fixed buckets remain on 3.5.
hermes_state.py Adds _FTS_TRIGRAM_TRIGGERS (proper subset of _FTS_TRIGGERS), a runtime trigram-availability probe, and a _drop_trigram_triggers fallback so environments with FTS5 but no trigram tokenizer no longer abort message writes.
plugins/memory/mem0/init.py Adds _DirectRestMem0Client (stdlib urllib) for self-hosted Mem0 OSS, wires MEM0_HOST/MEM0_ADMIN_API_KEY/MEM0_CA_BUNDLE env vars, and bounds the cloud SDK's httpx pool; api_key is still marked required=True in get_config_schema even though is_available() correctly bypasses it for self-hosted mode.
tests/test_request_composition.py Adds 7 new tests covering the two-tier divisor; test_fixed_divisor_out_of_range_falls_back_to_4 uses raw os.environ without try/finally, diverging from the monkeypatch pattern used by the adjacent equivalent test.
tests/plugins/memory/test_mem0_selfhost.py New 489-line test file covering direct-REST routing, scoping rules, HTTP error surfacing, config-file override precedence, fd-leak soak test, and CA-bundle SSL context wiring.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[compose_request_breakdown / estimate_request_tokens_rough] --> B{Content type?}
    B -->|system_prompt| C[_ceil_chars_to_tokens_fixed
divisor = COMPOSITION_CHARS_PER_TOKEN_FIXED
default 4.0]
    B -->|tool_schemas / skills_index| C
    B -->|history / tool_results / tool_args| D[_ceil_chars_to_tokens
divisor = COMPOSITION_CHARS_PER_TOKEN
default 3.5]
    C --> E[Fixed token bucket
sys_tokens, skills_tokens
tool_schema_tokens]
    D --> F[Non-fixed token bucket
history_tokens, tool_result_tokens
tool_arg_tokens, framing_tokens]
    E --> G[total_tokens = fixed_tokens + nonfixed_tokens]
    F --> G
    H[HERMES_COMPOSITION_CHARS_PER_TOKEN_FIXED env] -.->|2.0-8.0 clamp| C
    I[HERMES_COMPOSITION_CHARS_PER_TOKEN env] -.->|2.0-8.0 clamp| D
Loading

Reviews (1): Last reviewed commit: "fix(blackbox): two-tier char->token divi..." | Re-trigger Greptile

Comment on lines +338 to +347
def test_fixed_divisor_out_of_range_falls_back_to_4():
import importlib
import agent.model_metadata as mm
import os
for bad in ("0", "999", "-3", "notanumber", ""):
os.environ["HERMES_COMPOSITION_CHARS_PER_TOKEN_FIXED"] = bad
importlib.reload(mm)
assert mm.COMPOSITION_CHARS_PER_TOKEN_FIXED == 4.0, f"{bad!r} should fall back to 4.0"
os.environ.pop("HERMES_COMPOSITION_CHARS_PER_TOKEN_FIXED", None)
importlib.reload(mm)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 This test uses os.environ and importlib.reload without a try/finally, unlike the directly adjacent test_chars_per_token_divisor_out_of_range_falls_back_to_default which uses monkeypatch throughout. If any assertion fails mid-loop (e.g. "999" is incorrectly accepted), HERMES_COMPOSITION_CHARS_PER_TOKEN_FIXED stays set and mm.COMPOSITION_CHARS_PER_TOKEN_FIXED is left at the wrong value, silently corrupting token-count assertions in all subsequent composition tests.

Suggested change
def test_fixed_divisor_out_of_range_falls_back_to_4():
import importlib
import agent.model_metadata as mm
import os
for bad in ("0", "999", "-3", "notanumber", ""):
os.environ["HERMES_COMPOSITION_CHARS_PER_TOKEN_FIXED"] = bad
importlib.reload(mm)
assert mm.COMPOSITION_CHARS_PER_TOKEN_FIXED == 4.0, f"{bad!r} should fall back to 4.0"
os.environ.pop("HERMES_COMPOSITION_CHARS_PER_TOKEN_FIXED", None)
importlib.reload(mm)
def test_fixed_divisor_out_of_range_falls_back_to_4(monkeypatch):
import importlib
import agent.model_metadata as mm
for bad in ("0", "999", "-3", "notanumber", ""):
monkeypatch.setenv("HERMES_COMPOSITION_CHARS_PER_TOKEN_FIXED", bad)
importlib.reload(mm)
assert mm.COMPOSITION_CHARS_PER_TOKEN_FIXED == 4.0, f"{bad!r} should fall back to 4.0"
monkeypatch.delenv("HERMES_COMPOSITION_CHARS_PER_TOKEN_FIXED", raising=False)
importlib.reload(mm)

@@ -238,6 +398,9 @@ def save_config(self, values, hermes_home):
def get_config_schema(self):
return [
{"key": "api_key", "description": "Mem0 Platform API key", "secret": True, "required": True, "env_var": "MEM0_API_KEY", "url": "https://app.mem0.ai"},

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 api_key is marked "required": True even though is_available() correctly bypasses it when host is set. Any config UI or validation layer that reads get_config_schema() and enforces required: True fields will block self-hosted mode setup unless the user also supplies a cloud API key they don't have.

Suggested change
{"key": "api_key", "description": "Mem0 Platform API key", "secret": True, "required": True, "env_var": "MEM0_API_KEY", "url": "https://app.mem0.ai"},
{"key": "api_key", "description": "Mem0 Platform API key (required for cloud mode; omit when MEM0_HOST is set)", "secret": True, "required": False, "env_var": "MEM0_API_KEY", "url": "https://app.mem0.ai"},

@Kyzcreig
Kyzcreig merged commit becb4ef into main Jun 15, 2026
24 of 25 checks passed
Kyzcreig added a commit that referenced this pull request Jun 15, 2026
… gate) (#45)

The post-merge acceptance gate (PR #43 §5) FAILED at 4.0 on live Aegis
turns: real claude fixed content packs denser than the pre-flight sample
(Apollo DB) showed. Pooled re-measurement n=60 across both turns.db's =
real fixed ratio 4.38 chars/tok (median 4.45). At /4.0 only 40% of turns
land in the [0.90,1.05] band (mean 0.917, over-counts); /4.2 -> mean 0.963
(estimate ~4% high = safe early-warning side), 75% in-band. gpt-5.x (4.09)
at /4.2 ~ 0.974, also in-band.

Shipped 4.2 as the code DEFAULT (not an env override) so both gateways
stay converged. Tests made divisor-agnostic (reference the live constant)
+ default/fallback asserts 4.2. 28 pass.

Co-authored-by: Kyzcreig <9063726+Kyzcreig@users.noreply.github.com>
Kyzcreig added a commit that referenced this pull request Jun 20, 2026
…olation (#64)

Two follow-up fixes from Greptile review of merged PR #43.

- plugins/memory/mem0/__init__.py: api_key was marked required:True even
  though is_available() bypasses it when host is set (self-hosted gates on
  admin_api_key). The required flag forced any setup/validation UI to demand
  a cloud key for a self-hosted server that never uses one. Mark it optional
  and document the conditional requirement; is_available() remains the real
  gate.
- tests/test_request_composition.py: test_fixed_divisor_default_and_out_of_range_fallback
  mutated os.environ directly without try/finally, so a mid-loop assertion
  failure would leak HERMES_COMPOSITION_CHARS_PER_TOKEN_FIXED into later tests.
  Switch to monkeypatch + try/finally, matching the adjacent override tests.

Adds behavior-contract tests: api_key not required in schema, self-hosted
is_available without an api_key, and cloud mode still requires one.

Co-authored-by: Kyzcreig <9063726+Kyzcreig@users.noreply.github.com>
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