fix: guard against cached context_length=0 poisoning resolution chain

[OmarB97]

Why

The context window display in Hermes can show a broken ? (est) or ctx -- value when the persistent context-length cache contains a 0 entry. This happens because get_model_context_length() has a guard if cached is not None — but 0 is not None evaluates to True, so the function returns 0 instead of falling through to the hardcoded defaults (e.g. 1M for deepseek-v4-pro).

The zero propagates to ContextCompressor.context_length, which then causes every downstream display code path to fail:

• CLI status bar: ctx -- (unknown denominator)
• TUI status bar: falls back to showing total tokens without context percentage
• Gateway /usage: shows pct=0% with total=0

While save_context_length() is never intentionally called with 0, a corrupted cache file (manual edit, YAML parse glitch, or a future buggy probe) can introduce a 0 entry and permanently poison the resolution chain.

What changed

Two-layer defense in agent/model_metadata.py:

1. get_model_context_length(): After retrieving a cached value, check cached <= 0 before checking the provider-specific invalidation chain. Non-positive values are logged as a warning, invalidated from the cache, and the function falls through to normal resolution (which ends at the hardcoded 256K fallback).

2. save_context_length(): Added a guard that refuses to persist values <= 0, logging a warning. This prevents new poison entries from ever being written.

How to review

• agent/model_metadata.py — the two changed functions (~40 lines of new comments/code, ~34 lines re-indented within an else: block)
• The re-indented block is the existing Codex/Kimi/Nous cache invalidation chain — no logic changes there, just indented under the new else: branch

Evidence

Manual testing:

# Test 1: save rejects 0
save_context_length('test-model', 'https://api.test.com', 0)
→ "Refusing to cache non-positive context length ..."
→ get_cached_context_length('test-model', 'https://api.test.com') → None ✓

# Test 2: cached 0 handling
Manually wrote 0 to cache YAML
get_model_context_length('test-model', base_url='https://api.test-zero.com')
→ "Dropping non-positive cache entry ..."
→ Returns 256,000 (DEFAULT_FALLBACK_CONTEXT) ✓
→ Cache entry auto-invalidated ✓

# Test 3: normal resolution still works
get_model_context_length('deepseek-v4-pro', base_url='https://api.deepseek.com', provider='deepseek')
→ 1,000,000 ✓

Test suite:

• tests/agent/test_model_metadata.py — 100 passed, 0 failed ✓
• tests/hermes_cli/test_custom_provider_context_length.py — all passing ✓
• 8 pre-existing failures in test_compression_feasibility.py (aux compression model tests, unrelated to this change)

Verification

• Manual: delete/reset context_length_cache.yaml and verify status bar shows correct context window after first API call
• Manual: manually inject context_length: 0 in cache YAML, restart, verify warning logged and context resolves correctly

Risks / gaps

• Low risk: the guard is purely defensive — the existing code already handles the cache-miss path (falls through to hardcoded defaults)
• The else: block re-indentation is the only structural change; the provider-specific invalidation logic (Codex, Kimi, Nous) is unchanged
• If a future probe genuinely returns 0 for a real model, this guard would prevent caching but the model would still get DEFAULT_FALLBACK_CONTEXT (256K) — better than showing unknown

---

/collaborators @NousResearch/hermes-agent-maintainers

[OmarB97]

Refreshed this branch onto current upstream main as a single refresh commit (transplant — the branch predated the history replacement, so there was no merge base to rebase across). The change is identical to the original diff except for one conflict resolution in agent/model_metadata.py: save_context_length() now computes its key via the newer _context_cache_key() helper (main's slash-normalization) instead of the old raw f-string, with the non-positive guard unchanged in front of it; the cached <= 0 guard slots ahead of the invalidation chain, whose newer Kimi/MiniMax/Grok branches keep their elif semantics. Original head was d62ed5eb92f057d8c707ba937b44f168f2df0677 for provenance. Checks are re-running on the refreshed head.

[OmarB97]

Provenance note for today's force-push: the branch was rebuilt as a single refresh commit (7d6d3a9b0e) on current upstream/main — the previous head (d62ed5eb) had lost its merge base to the repository's history replacement, so this reapplies the non-positive context-length guard (one file, agent/model_metadata.py) onto today's mainline with no stale-history payload. Content is unchanged vs. the originally-reviewed diff; the branch is conflict-free again and checks are re-running.

[teknium1]

Thanks for refreshing this focused cache-boundary fix. The numeric cached-zero premise is confirmed on current main: agent/model_metadata.py:2110-2111 accepts any non-None cache hit and :2173-2178 returns it directly.

Problems

• The cache guard does not cover the sibling live-metadata path. _resolve_endpoint_context_length() accepts 0 as an integer at agent/model_metadata.py:1060-1062, and the custom-endpoint branch returns it at :2213-2215.
• The new cached <= 0 comparison assumes YAML cache values are numeric. _load_context_cache() returns raw YAML data (:1078-1080), so a quoted value can raise TypeError at the new guard instead of being invalidated.
• The diff adds no regression tests; tests/agent/test_model_metadata.py:478-521 already demonstrates the isolated on-disk cache pattern needed here.

Suggested changes

• Validate positive integer context lengths at the endpoint-metadata boundary and reject malformed cache values at the cache-hit boundary.
• Add tests for rejected writes, numeric cached-zero invalidation/fallback, and a zero returned by endpoint metadata.

Automated hermes-sweeper review.

[teknium1]

_resolve_endpoint_context_length() currently returns any integer, including 0 (agent/model_metadata.py:1060-1062), and the custom-endpoint branch returns it at :2213-2215. Please reject non-positive endpoint metadata too; otherwise a live bad response still reaches the compressor even when cache writes are refused.

[teknium1]

Merged via PR #85507 (merge commit d3a8be4) — your commit was cherry-picked onto current main with your authorship preserved in git log. We added regression tests on top (the never-persisted and dropped-and-re-resolved cases, sabotage-verified). Thanks for catching the 0 is not None short-circuit — a nasty poison class.