Conversation
ae7056c to
1961479
Compare
a33ea12 to
920013f
Compare
|
Thanks for isolating the constructor-side context-resolution cost. The eager resolution remains present on current main, but the branch predates important initialization and compression changes. Problems
Suggested changes
Automated hermes-sweeper review. |
|
You're right, the lazy property alone still left three eager-read sites alive: validation, session metadata, and the deferred runtime snapshot. I moved the quiet-path resolution to first-turn setup, kept the non-quiet CLI banner as the explicit eager path, and reapplied the current I also widened the change across the remaining eager-read surfaces in |
e3b537e to
e58b18b
Compare
…d on main since PR base TestThresholdTokensCap and TestLazyContextResolution landed on main after the NousResearch#38991 lazy-init base; they construct ContextCompressor under a get_model_context_length patch and read threshold_tokens after the with block. With deferred resolution the probe now fires lazily, so resolve inside the mock (same pattern as the rest of the suite) and give the lazy-resolution mock a real return_value.
…d on main since PR base TestThresholdTokensCap and TestLazyContextResolution landed on main after the #38991 lazy-init base; they construct ContextCompressor under a get_model_context_length patch and read threshold_tokens after the with block. With deferred resolution the probe now fires lazily, so resolve inside the mock (same pattern as the rest of the suite) and give the lazy-resolution mock a real return_value.
|
Merged via #57229 — both your commits were cherry-picked onto current main with authorship preserved ( |
…d on main since PR base TestThresholdTokensCap and TestLazyContextResolution landed on main after the NousResearch#38991 lazy-init base; they construct ContextCompressor under a get_model_context_length patch and read threshold_tokens after the with block. With deferred resolution the probe now fires lazily, so resolve inside the mock (same pattern as the rest of the suite) and give the lazy-resolution mock a real return_value.
…d on main since PR base TestThresholdTokensCap and TestLazyContextResolution landed on main after the NousResearch#38991 lazy-init base; they construct ContextCompressor under a get_model_context_length patch and read threshold_tokens after the with block. With deferred resolution the probe now fires lazily, so resolve inside the mock (same pattern as the rest of the suite) and give the lazy-resolution mock a real return_value.
…d on main since PR base TestThresholdTokensCap and TestLazyContextResolution landed on main after the NousResearch#38991 lazy-init base; they construct ContextCompressor under a get_model_context_length patch and read threshold_tokens after the with block. With deferred resolution the probe now fires lazily, so resolve inside the mock (same pattern as the rest of the suite) and give the lazy-resolution mock a real return_value.
Summary
ContextCompressor.__init__calledget_model_context_length()synchronously during agent construction. Inside the resolver at step 5e (agent/model_metadata.py:1716),_query_ollama_api_show()opens a synchronoushttpx.Clientand POSTs to{base_url}/api/showagainst anybase_url, with a 5-second timeout configured atagent/model_metadata.py:1086. For Ollama endpoints this resolves the GGUF context length; for every other server (OpenAI, Anthropic, CLIProxyAPI, etc.) it returns 404/405 after a full round-trip. cProfile benchmarking by the reporter showed this adds roughly 150 to 160ms per construction on local networks, and in a 10-iteration loop each construction paid the full cost: there is no amortization.For the legacy single-agent CLI case where one agent is instantiated at startup, the penalty is invisible. For architectures that instantiate ephemeral per-turn agents or run dense multi-agent orchestration (the reporter's use case), every construction blocks on this probe, destroying the latency budget. The probe is also a security anti-pattern: burying synchronous network egress inside a constructor creates an unobservable SSRF vector if the agent is constructed in a web context with user-controlled routing, and a hung Ollama server locks the main thread during object creation.
The compression feasibility check was already deferred to first turn in
agent_init.py:1616-1622(lazy via_compression_feasibility_checked), but theContextCompressor.__init__context-length resolution atagent/context_compressor.py:616still ran eagerly. This is the remaining synchronous network I/O in the constructor path.The fix replaces
context_lengthand its derived values (threshold_tokens,tail_token_budget,max_summary_tokens) with lazy-resolved properties.ContextCompressor.__init__stores the resolution parameters but does not callget_model_context_length(). The first access to.context_lengthtriggers resolution and caches the result; subsequent accesses return the cached value. A setter ensures theupdate_model()andswitch_modelpaths that assigncompressor.context_length = Ndirectly still work, bypassing the resolver entirely. For agents withquiet_mode=True(gateway subagents, ephemeral agents), the property is never accessed during construction, so the blocking probe is fully deferred. Theconfig_context_lengthfast path (step 0 ofget_model_context_length) still short-circuits without any network I/O when the user has configured an explicit context length. Related issues: #8499, #12977, #13492.Changes
agent/context_compressor.py: replace eagerget_model_context_length()call in__init__with stored parameters; add_resolve_context_length()private method,context_lengthlazy property with getter/setter, and lazy properties forthreshold_tokens,tail_token_budget,max_summary_tokens(+64 lines, -20 lines)tests/agent/test_context_compressor.py: updatecompressorfixture andTestSummaryTargetRatioto resolvecontext_lengthinside the mock'swithblock; addTestLazyContextResolutionwith 3 tests verifying init deferral, setter bypass, and config fast-path (+65 lines)Validation
quiet_mode=False)get_model_context_lengthcalled inContextCompressor.__init__; httpx.post blocks for ~150msget_model_context_lengthcalled on first.context_lengthaccess (triggered byinit_agentprint at line 1606); same latency for CLI, just via propertyquiet_mode=True).context_lengthaccess (first turn, not construction); construction is ~150ms fasterconfig_context_lengthset in config.yamlupdate_model(model, context_length=N)self.context_length = Nswitch_model/ fallback activationcompressor.context_lengthTest plan
pytest tests/agent/test_context_compressor.py -v --timeout=0— 94 passedpytest tests/run_agent/test_compression_feasibility.py -v --timeout=0— 16 passedNot in scope
Making
get_model_context_lengthitself async is deliberately left out. The function is called from many synchronous paths (CLI, config resolution, model switch, plugin init) and converting it would require async propagation across a large surface. The lazy property approach eliminates the blocking I/O from the constructor path without touching the resolver itself. A follow-up could introduce an async variant for the gateway's event-loop context, but this PR focuses on the constructor latency that the report describes.Upstream
Closes #32221.
Reported by @twocash.