feat(agent): per-model / per-provider compression threshold overrides - #19110
feat(agent): per-model / per-provider compression threshold overrides#19110deep-name wants to merge 2 commits into
Conversation
Adds two empty-by-default override maps to the compression schema and a pure-function resolver that picks the active threshold: model_thresholds[model] > provider_thresholds[provider] > threshold > 0.50 Resolver lives at agent/context_compressor.py module level so it can be reused by callers; the ContextCompressor itself stays pure. Wire-up at the existing instantiation site (run_agent.py) calls the resolver once at startup. The startup banner now reads threshold_percent from the compressor instead of the global config so override values are reflected. Out-of-range or non-numeric override values log a one-time warning per (scope, identifier) and fall through to the next precedence level. The dedup set is module-level and intentionally not reset on config reload — long-lived gateway processes prioritize log-noise control over reload feedback. Provider keys must be exact PROVIDER_REGISTRY ids (anthropic, gemini, kimi-coding, xai, openrouter, ...). Aliases like "google" or "moonshot" are scoped to auxiliary-model routing and silently fall through; the resolver docstring and the cli-config.yaml.example block call this out. Scope: this PR adds the schema, resolver, and startup wire-up. It does NOT touch ContextCompressor.update_model() — model-switch re-resolution pairs with the threshold_percent= parameter introduced by NousResearch#18638. Once that lands, callers compose the two: update_model(threshold_percent=resolve_compression_threshold( model, provider, config)) No migration version bump: additive empty-dict defaults + .get(...) or {} in the resolver mean existing user configs behave identically without touching the config file. Tests: 25 unit tests cover precedence, validation (range + non-numeric + dedup), composition with ContextCompressor, and source-presence checks on the run_agent.py wire-up and schema additions. Fixes NousResearch#18733
Adds ContextCompressor.re_resolve_threshold(), invoked at the three runtime
model-switch sites (switch_model, _try_activate_fallback,
_restore_primary_runtime), so per-model and per-provider compression
overrides take effect the moment the user switches models.
ContextEngine.re_resolve_threshold() is a no-op on the base class so plugin
context engines are safe to invoke generically without hasattr checks.
The compression_config kwarg accepts either a Mapping (snapshot) or a
zero-arg callable returning a Mapping (live). The callable form lets a
future /reload-config feature plug in without API changes; for now the
wire-up passes a snapshot. A callable that raises or returns a non-Mapping
emits a deduplicated WARNING and the model switch continues — a broken
config source must never break the user's session.
The resolver now also emits a one-time WARNING hint when an alias key like
"google" or "moonshot" appears in provider_thresholds: the lookup still
falls through silently (since self.provider is the canonical PROVIDER_REGISTRY
id), but the operator gets a pointer at the canonical id ("gemini",
"kimi-coding") so they can fix their config.
Tests: +17 new, total 42. Covers re-resolution after model switch, the
ordering invariant between update_model/re_resolve_threshold, callable
getter live semantics + tolerated-but-loud failure paths, alias hint
emission + dedup. Existing validation tests now pin log levels at WARNING
so a future change can't silently demote them to DEBUG.
Composition contract with PR NousResearch#18638 (issue NousResearch#18617): NousResearch#18638 adds
threshold_percent= preservation on update_model; this PR adds policy-driven
re-resolution. Policy wins over preservation because that's the user's
stated intent. The two diffs add lines to the same callsite blocks but
don't overlap line-for-line, so neither blocks the other on merge order.
Refs NousResearch#18733
|
Thanks for pushing this forward — the provider alias hints and threshold validation are genuinely useful bits of UX polish. I just reviewed #38139 which takes a narrower approach (per-model overrides only, ~195 lines, clean resolver module). I think that narrower PR is the better path to merge first:
The two features worth cherry-picking from here into a follow-up:
Would you be open to contributing those as a follow-up on top of #38139 once it lands? |
|
Thanks for the thorough precedence and validation work. The feature request remains valid: current main exposes only a global user threshold plus built-in model-specific policy ( Problems
Suggested changes
Automated hermes-sweeper review. |
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Four PRs address or reference the request. #60781 and #63020 developed the per-model approach that ultimately landed through #69339, while #19110 overlaps on per-model thresholds but additionally proposes exact-match per-provider overrides, validation, alias hints, and runtime re-resolution.
Related pull requests
- #19110
related— (+807/-3) — author action: rebase onto main, or split out the part that can merge. Consistent with the automated keep_open review, the per-provider resolver, threshold validation, and provider-alias diagnostics remain salvageable, but the per-model portion overlaps #69339 and the runtime integration must be adapted to the current compressor helpers and model-transition paths. - #60781 [closed]
related— (+341/-4) — superseded by #63020. This first per-model implementation introduced longest-substring matching and model-switch resolution, but omitted gateway cache invalidation and correct interaction with the small-context floor; the author closed it in favor of the rebased v2. - #63020 [closed]
related— (+323/-11) — superseded by merged #69339. It added per-model longest-substring resolution, gateway cache invalidation, and small-context-floor integration; #69339 cherry-picked it with preserved authorship and addressed the blocking contributor review gaps around plugin initialization ordering, DEFAULT_CONFIG, tests, and documentation. - #69339 [merged]
related— (+571/-12) — merged reference implementation. It implements per-model threshold overrides on main, including model-switch re-resolution, the raise-only small-context floor, gateway cache invalidation, plugin initialization ordering, public configuration, regression tests, and documentation.
Duplicates
#60781 was superseded by #63020, and #63020 was incorporated into #69339. #19110 substantially duplicates #69339 for per-model overrides but is not a full duplicate because its per-provider overrides, validation, and alias diagnostics are additional scope.
Suggested consolidation
Author action: rebase #19110 onto main, or split out the part that can merge. Treat the per-model implementation as already supplied by #69339 at commit f944e84, as documented by the contributor on #63020, and retain only the distinct per-provider resolution, validation, and alias-diagnostic work after adapting it to the current compressor invariants and runtime transition paths; #60781 and #63020 require no further action because their implementation chain culminated in #69339.
Cross-PR triage: Reviewed 4 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 123 kB of PR diffs, 20 kB of issue/PR text, 8 kB of discussion (10 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

What does this PR do?
Adds two empty-by-default override maps to the
compressionconfig schema and a pure-function resolver that picks the active compression threshold per the precedence:User-visible effect: a Claude user running 200K-context conversations can keep
threshold: 0.50as a sensible default for most providers, then lift it to0.65just foranthropic(or just for one specific model) without touching the rest of their setup.The override fires on initial agent startup and whenever the runtime switches models — model switching, fallback activation, and primary-runtime restoration all re-pick the threshold so a per-model rule applies the moment that model becomes active.
Schema additions are additive and default to empty dicts, so existing configs behave identically. No migration version bump required.
Related Issue
#18733 — "Per-model or per-provider compression threshold overrides". Closes the feature request via Option B of the issue body (per-model with per-provider as the fall-back), without removing or reshaping the existing global
compression.threshold.Type of Change
Changes Made
hermes_cli/config.py:633— extendsDEFAULT_CONFIG["compression"]withprovider_thresholds: {}andmodel_thresholds: {}.agent/context_compressor.py:_THRESHOLD_MIN,_THRESHOLD_MAX,_THRESHOLD_DEFAULT._LOGGED_INVALID_OVERRIDE. One warning per scope+identifier; intentionally not reset on reload (long-lived gateway processes prioritize log-noise control over reload feedback)._PROVIDER_ALIAS_HINTSmap. When a user writes an alias key (e.g.provider_thresholds: {google: 0.50}) the resolver still falls through silently (sinceself.provideris the canonical id), but emits a one-time hint line pointing at the canonical id (gemini) so the operator notices._validate_threshold(...)private helper — coerces to float, range-checks, logs once on failure atWARNINGlevel.resolve_compression_threshold(model, provider, config) -> floatpublic resolver.ContextCompressor.__init__— new optionalcompression_configkwarg accepting either aMapping(snapshot — fast path; matches existing CLI_CONFIG semantics) or a zero-arg callable returning aMapping(live path — usable behind a future/reload-confighook without API changes).ContextCompressor.re_resolve_threshold()— re-runs the resolver against the current(self.model, self.provider)pair and recomputesthreshold_tokens. No-op when_compression_configisNone. Loud-failure path: when a callable getter raises an exception or returns a non-Mapping, a deduplicatedWARNINGis logged so a persistent misconfiguration is visible without restart.agent/context_engine.py—ContextEngine.re_resolve_threshold()no-op so plugin engines are safe to invoke generically (nohasattr()checks at call sites).run_agent.py:ContextCompressorinstantiation site, calls the resolver and passes the result asthreshold_percent. Also passescompression_config=_compression_cfgso the snapshot is available for runtime re-resolution.switch_model,_try_activate_fallback,_restore_primary_runtime), invokesself.context_compressor.re_resolve_threshold(). Each call site has an inline ordering-invariant comment so a future refactor that reorders the two calls is forced to read the rationale.📊 Context limit:startup banner now readsthreshold_percentfrom the compressor instance instead of the global config so override values are reflected in the printed line.cli-config.yaml.example— appends a documented, commented-out example block in the existingcompression:section, including the canonical-vs-alias note.tests/agent/test_compression_threshold_resolution.py(new) — 42 tests across eight classes:TestResolutionPrecedence(11): default, global-only, provider-beats-global, model-beats-provider, model-beats-global-when-no-provider, fall-throughs (unknown model / unknown provider / both empty / empty dicts), aliased-key-falls-through.TestValidation(8): non-numeric, below-min, above-max, at-min-boundary, at-max-boundary, dedup-per-key, dedup-distinct-keys, invalid-global-falls-to-built-in (with explicitWARNING-level assertions to prevent silent log-level demotion).TestAliasHint(4): alias key emits canonical-id hint atWARNING, hint deduplicated, canonical key doesn't trigger hint, alias-warning suppressed when canonical match wins.TestModelSwitchReResolution(7): provider override picked up after switch, model override picked up after switch, fall-back-to-global when no override matches, no-op when nocompression_config, MINIMUM_CONTEXT_LENGTH floor respected, idempotent when threshold unchanged, ordering-invariant doc test that assertsre_resolve_threshold()reads current state on every call.TestCallableConfigGetter(4): getter invoked per-call (proves live semantics), getter returning non-Mapping warns + is no-op, getter raising warns + is tolerated, getter-failure warning is deduplicated by exception type.TestBaseEngineNoOp(1): plugin engines that don't overridere_resolve_thresholdget a safe no-op from the base class.TestWireUpComposition(2): resolved value flows intoContextCompressor.threshold_percentandthreshold_tokensvia direct construction.TestWireUpSourcePresence(5): resolver imported, called withmodel=self.model/provider=self.providernext to instantiation, schema keys present, yaml example references canonical-id source,re_resolve_threshold()invoked at ≥3 model-switch sites inrun_agent.py.Coordination with PR #18638 (issue #18617)
PR #18638 is in active review for #18617. It adds a
threshold_percent: float | None = Noneparameter onContextCompressor.update_model()and forwards the existing value at three runtime callsites so a model switch doesn't reset the threshold to whatever the legacy default would set.This PR deliberately does not modify
update_model()'s signature. Override re-picking lives in a separatere_resolve_threshold()method invoked afterupdate_model()returns:The two diffs add lines to the same callsite blocks but don't overlap line-for-line, so neither blocks the other on merge order.
Composition contract (when both PRs land):
update_model(threshold_percent=cc.threshold_percent)re_resolve_threshold()Policy wins over preservation — that's the user's stated intent. When no override matches the new pair, the resolver falls through to the global threshold, which is what
__init__would have picked anyway, so #18638's preservation is a no-op in realistic configs. The two are corrective only when an override explicitly disagrees with the preserved value, in which case the user's configured override is correct.Callers that want to bypass policy should call
update_model(threshold_percent=...)without a follow-upre_resolve_threshold().Failure-mode posture
[0.10, 0.95]): logged once atWARNING, fall through to next precedence level. Tests pin the log level so a future change can't silently demote toDEBUG.google,moonshot,claude, ...): logged once atWARNINGwith a canonical-id hint, fall through. Tests assert the level.WARNINGlogged once per exception type, threshold stays at current value, model switch continues.WARNINGlogged once per type, threshold stays at current value, model switch continues.compression_configisNone: silent no-op; this is the not-configured case.The "log loudly, never crash the model switch" posture is intentional: a misconfigured threshold should never break the user's session. All warnings hit the standard
agent.context_compressorlogger so they appear in default logging configurations.Out of scope (intentional)
compression:block.google→geminibefore lookup). The lookup stays exact-string; aliases fall through with a one-time hint. Auto-normalization would couple the resolver to_PROVIDER_ALIASESand create a drift surface.threshold_percentthey're initialized with; they receive a default no-opre_resolve_threshold()from the base class.update_modelcallsites (aroundrun_agent.py:12219/:12492) — not user-visible model switches; not followed byre_resolve_threshold().How to Test
I ran
pytest tests/agent/ -qlocally on this branch — 2397 passed, 0 failed, no new warnings traceable to this diff.Checklist
Code
pytest tests/agent/ -qand the changes don't introduce new failurestests/agent/test_compression_threshold_resolution.py— 42 tests)Documentation & Housekeeping
cli-config.yaml.exampleupdated with a commented-out example block in the existingcompression:sectionRidden with Loa.