Skip to content

fix(test): re-add explicit feasibility check call in lazy-check integration test#32001

Open
talwayh1 wants to merge 2 commits into
NousResearch:mainfrom
talwayh1:ci-fix/test-compression-feasibility-lazy-check
Open

fix(test): re-add explicit feasibility check call in lazy-check integration test#32001
talwayh1 wants to merge 2 commits into
NousResearch:mainfrom
talwayh1:ci-fix/test-compression-feasibility-lazy-check

Conversation

@talwayh1

Copy link
Copy Markdown

What

The test test_init_feasibility_check_uses_aux_context_override_from_config was broken by commit ffde18286 which removed the explicit agent._check_compression_model_feasibility() call needed to trigger the aux-model context probe. Since #28957 (defer feasibility check to first compression attempt), the check is lazy and won't fire during AIAgent.__init__ — the test must drive it manually.

Root Cause

The lazy deferral in #28957 moved check_compression_model_feasibility out of __init__ and into compress_context(). The test was partially updated (changed assert_called_once_withassert_any_call, added #12977 comment) but the call to _check_compression_model_feasibility() was accidentally deleted while the assertion was moved outside the with block.

Fix

  • Move the _aux_compression_context_length_config assertion back inside the with block
  • Re-add agent._check_compression_model_feasibility() call inside the with block so the mock captures the aux-model context probe
  • Update docstring to clarify the lazy-check contract

Related

Tranquil-Flow and others added 2 commits May 25, 2026 11:09
…easibility check (NousResearch#12977)

Two-part fix in agent/conversation_compression.check_compression_model_feasibility
(after the main-init refactor moved the function out of run_agent):

1. When the aux compression model matches the main model (same name +
   base_url, which happens when no separate compression model is
   configured), fall back to self._config_context_length for the aux
   model's get_model_context_length call. Without this fallback the
   aux probe re-detects context for the same endpoint and ignores
   custom_providers.models.context_length.

2. Re-derive the main model's compression threshold from
   get_model_context_length instead of reading the potentially-stale
   threshold_tokens from the compressor — important when custom_providers
   context_length is loaded after the compressor was originally
   constructed.

(Fix NousResearch#1 from the original PR — propagating _config_context_length after
custom_providers resolution in __init__ — is already integrated in
agent/agent_init.py line 1227, after the custom_providers branch.)

Fixes NousResearch#12977
…ntext_length calls

PR NousResearch#12977 added a second get_model_context_length call to re-derive
the main model's compression threshold inside check_compression_model_feasibility().
The existing tests used assert_called_once_with which broke when the
function was called twice (once for aux model, once for main model).

Changes:
- Changed assert_called_once_with to assert_any_call in 3 tests
- Used side_effect=[aux_ctx, main_ctx] instead of return_value in 4 tests
- Added threshold_percent to _make_agent mock compressor (root cause:
  MagicMock.int() returns 1, making threshold=max(1, 64k)=64k)
@alt-glitch alt-glitch added type/test Test coverage or test infrastructure P3 Low — cosmetic, nice to have comp/agent Core agent loop, run_agent.py, prompt builder labels May 25, 2026
@hclsys

hclsys commented May 25, 2026

Copy link
Copy Markdown

The test repair is correct — since #28957 deferred the feasibility check out of __init__ into lazy first-compression, the test must drive _check_compression_model_feasibility() explicitly, and re-adding that call is right.

Two notes on the rest of the diff:

  1. Scope/title: this is more than a test fix — agent/conversation_compression.py gets a real behavior change for [Bug]: custom_providers.models.context_length not propagated to auxiliary compression feasibility check #12977 (aux-model context falls back to the main model's resolved context_length when aux==main, and the threshold is re-derived instead of read from the possibly-stale context_compressor.threshold_tokens). The body does mention [Bug]: custom_providers.models.context_length not propagated to auxiliary compression feasibility check #12977 (good), but a fix(test): title undersells a compression-correctness fix — a maintainer triaging by title might merge the production change unreviewed. Worth retitling to fix(compression): (with the test repair noted) or splitting.

  2. The threshold re-derivation calls get_model_context_length(agent.model, ...) again. I checked the cost: it short-circuits on config_context_length (model_metadata.py:~35) and the feasibility check is gated by _compression_feasibility_checked so it runs once per session, not per turn — so the extra resolution is bounded, good. One small thing: the except Exception: main_ctx_resolved = agent.context_compressor.context_length swallows any error during re-resolution and silently falls back to the (potentially stale) compressor value — which reintroduces the exact staleness [Bug]: custom_providers.models.context_length not propagated to auxiliary compression feasibility check #12977 is fixing, just on the error path. A debug-log in that except (or narrowing it) would make a genuine resolution failure visible instead of silently degrading. The aux==main fallback logic itself looks correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent loop, run_agent.py, prompt builder P3 Low — cosmetic, nice to have type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants