Skip to content

test(run_agent): align switch_model test with new clear-on-switch semantics - #24776

Closed
briandevans wants to merge 1 commit into
NousResearch:mainfrom
briandevans:fix/test-switch-model-clears-stale-config-ctx
Closed

test(run_agent): align switch_model test with new clear-on-switch semantics#24776
briandevans wants to merge 1 commit into
NousResearch:mainfrom
briandevans:fix/test-switch-model-clears-stale-config-ctx

Conversation

@briandevans

Copy link
Copy Markdown
Contributor

Summary

  • 8ac351407 (#24724, closing fix: clear stale config context_length override on model switch #21509) flipped switch_model's semantics from "preserve _config_context_length across switches" to "clear it so the new model is auto-resolved"
  • The test tests/run_agent/test_switch_model_context.py::test_switch_model_preserves_config_context_length (added by fd3e855d5 under the old interpretation) still asserts the stale value is passed through and is now a baseline CI failure on main
  • Update that test (and the module docstring) to verify the new invariant

The bug

On clean origin/main:

FAILED tests/run_agent/test_switch_model_context.py::test_switch_model_preserves_config_context_length
    AssertionError: assert None == 32768

The test asserts get_model_context_length is called with the old config_context_length=32_768, but after 8ac351407 the override is cleared to None before the runtime field swap. The test name "preserves" no longer describes the intended behavior — it was a direct casualty of the policy flip.

The fix

Rename the test to test_switch_model_clears_stale_config_context_length and assert the new invariant:

  1. get_model_context_length is called with config_context_length=None
  2. agent._config_context_length itself is cleared on the instance so any subsequent resolution stays clean
  3. The compressor is still re-pointed at the new model (unchanged behavior)

The sibling test_switch_model_without_config_context_length keeps passing because the unconditional-None case is identical under both policies — left it untouched.

Test plan

  • Focused regression test: uv run --with pytest --with pytest-xdist --with pytest-asyncio python3 -m pytest tests/run_agent/test_switch_model_context.py -v → 2 passed
  • Adjacent suite: uv run ... python3 -m pytest tests/run_agent/ → 1354 passed, 9 skipped, 0 failed
  • Regression guard: on clean origin/main the renamed test passes; before the rename (i.e. on current main) test_switch_model_preserves_config_context_length fails with assert None == 32768

Related

…antics

Commit 8ac3514 ("fix(agent): clear stale config context_length on model
switch", NousResearch#24724 closing NousResearch#21509) flipped switch_model's behavior: the
per-agent _config_context_length override is now cleared to None before
the runtime field swap so the new model's actual context window is
resolved via the full chain (custom_providers, endpoint probe,
models.dev, etc.) instead of inheriting the previous model's stale
value.

That commit broke tests/run_agent/test_switch_model_context.py::
test_switch_model_preserves_config_context_length, which was added by
fd3e855 under the older "preserve across switches" interpretation and
still asserts the stale value is passed through to
get_model_context_length. The test contradicts the now-intended
invariant and fails on every CI run on origin/main with
`AssertionError: assert None == 32768`.

Update the test (and the module docstring) to verify the new invariant:
get_model_context_length receives `config_context_length=None` after the
switch, and the agent attribute itself is cleared. The sibling
test_switch_model_without_config_context_length keeps passing unchanged
because the unconditional-None case is the same in both worlds.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 13, 2026 03:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates a failing run_agent regression test to match the updated AIAgent.switch_model() semantics where _config_context_length is cleared on model switch so the new model’s context window is auto-resolved.

Changes:

  • Update module docstring to describe the new “clear-on-switch” invariant.
  • Rename the test to reflect the new behavior and adjust assertions to expect config_context_length=None and a cleared agent._config_context_length.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 59 to +62
mock_ctx_len.assert_called_once()
call_kwargs = mock_ctx_len.call_args.kwargs
assert call_kwargs.get("config_context_length") == 32_768
assert call_kwargs.get("config_context_length") is None

@alt-glitch alt-glitch added type/test Test coverage or test infrastructure P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint labels May 13, 2026
@briandevans

Copy link
Copy Markdown
Contributor Author

CI audit — all 20 test failures are pre-existing baselines on clean origin/main (486b692dd, run 25777086942). Zero failures touch this PR's renamed test, which passes (and removes test_switch_model_preserves_config_context_length from main's failure list — origin/main has 21, this PR has 20).

Test Symptom Root cause on main
tests/agent/test_bedrock_adapter.py::TestResolveBedrocRegion (×3) ModuleNotFoundError: No module named 'botocore' [bedrock] removed from [all] in #24515 (lazy-install policy); already addressed in #24601 (commit 11496dd)
tests/agent/test_bedrock_integration.py::TestPackaging::test_bedrock_in_all_extra assert '"hermes-agent[bedrock]"' in pyproject fails Same as above; addressed in #24601
tests/hermes_cli/test_bedrock_model_picker.py::TestBedrockRegionRouting (×2) ModuleNotFoundError: No module named 'botocore' Same as above; addressed in #24601 (commit 5d0059a)
tests/tools/test_transcription.py (×2) ModuleNotFoundError: No module named 'faster_whisper' [voice] removed from [all]; tracked by @CharlieKerfoot's #6375 (out of scope per #24601)
tests/gateway/test_dingtalk.py (×8) assert 500 == 200, mocks not called, assert False [dingtalk] removed from [all]; SDK absent so guarded module short-circuits
tests/gateway/test_feishu_bot_admission.py (×1) KeyError: 'uri' [feishu] removed from [all] — same lazy-install fallout
tests/hermes_cli/test_startup_plugin_gating.py::test_builtin_set_covers_every_registered_subcommand _BUILTIN_SUBCOMMANDS is missing these live subcommands: ['lsp'] Real baseline — lsp subcommand registered but not added to _BUILTIN_SUBCOMMANDS allowlist

This PR is a pure test-only change to tests/run_agent/test_switch_model_context.py. The renamed test (test_switch_model_clears_stale_config_context_length) is the only test in the touched file and passes locally + in CI.

@briandevans

Copy link
Copy Markdown
Contributor Author

Closing to keep the queue clean — happy to reopen if this is still useful.

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

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint 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.

3 participants