Skip to content

Conversation

@drewdrewthis
Copy link
Collaborator

@drewdrewthis drewdrewthis commented Oct 9, 2025

• Refactors agent config handling to centralize model configuration resolution in a dedicated utility.
• Introduces ResolvedModelConfig dataclass and resolve_model_config() to merge explicit params with ScenarioConfig defaults.
• Updates JudgeAgent and UserSimulatorAgent to use resolver, fixing precedence and edge cases (e.g., temperature=0.0).
• Adds comprehensive tests covering no-global-config, string default_model, ModelConfig defaults, falsy value handling, extra param merging, and edge cases.

Tests: WHEN / THEN breakdown

No global config (ScenarioConfig.default_config = None)
• WHEN resolve_model_config(model=“openai/gpt-4”)
▫ THEN returns model=“openai/gpt-4”, other fields None, extra_params={}
• WHEN resolve_model_config() with no model
▫ THEN raises ValueError(“Model must be configured…”)
• WHEN resolve_model_config(model=“openai/gpt-4”, api_base=“https://custom.com”, api_key=“sk-test”, temperature=0.5, max_tokens=1000, timeout=60, headers={“X-Test”: “value”})
▫ THEN explicit values are kept; extra_params={“timeout”: 60, “headers”: {“X-Test”: “value”}}

Global config: default_model as string
• GIVEN ScenarioConfig.default_config = ScenarioConfig(default_model=“openai/gpt-4”)
• WHEN resolve_model_config()
▫ THEN model=“openai/gpt-4”
• WHEN resolve_model_config(model=“anthropic/claude-3”)
▫ THEN model=“anthropic/claude-3” (explicit overrides)
• WHEN resolve_model_config(timeout=30)
▫ THEN extra_params={“timeout”: 30}

Global config: default_model as ModelConfig
• GIVEN ScenarioConfig.default_config = ScenarioConfig(default_model=ModelConfig(model=“openai/gpt-4”, api_base=“https://config.com”, api_key=“sk-config”, temperature=0.7, max_tokens=2000))
• WHEN resolve_model_config()
▫ THEN uses all defaults: model/openai/gpt-4, api_base/config.com, api_key/sk-config, temperature=0.7, max_tokens=2000, extra_params={}
• WHEN resolve_model_config(model=“anthropic/claude-3”, api_base=“https://override.com”, api_key=“sk-override”, temperature=0.1, max_tokens=500)
▫ THEN explicit overrides are applied for all fields
• WHEN resolve_model_config(temperature=0.3)
▫ THEN temperature=0.3 (explicit), other fields from config

Falsy values handling
• GIVEN defaults with temperature=0.7, max_tokens=2000
• WHEN resolve_model_config(temperature=0.0)
▫ THEN temperature=0.0 (explicit 0.0 overrides; not treated as falsy)
• WHEN resolve_model_config(max_tokens=0)
▫ THEN max_tokens=0 (explicit 0 overrides; not treated as falsy)
• WHEN resolve_model_config() with temperature unspecified
▫ THEN temperature uses config default (0.7)
• GIVEN config with api_base=“https://config.com”
• WHEN resolve_model_config(api_base=””)
▫ THEN api_base remains “https://config.com” (empty string treated as falsy for strings via ‎⁠or⁠)

Extra params merging
• GIVEN config extra params: timeout=30, headers={“X-Config”: “config-value”}, max_retries=3 in ModelConfig
• WHEN resolve_model_config()
▫ THEN extra_params includes all config extras
• WHEN resolve_model_config(timeout=60, new_param=“value”)
▫ THEN explicit extra overrides: timeout=60; config extras preserved; new_param added
• GIVEN default_model is string (“openai/gpt-4”)
• WHEN resolve_model_config(custom=“param”)
▫ THEN extra_params == {“custom”: “param”} (no config extras to merge)

Edge cases
• GIVEN ModelConfig default temperature=0.0
• WHEN resolve_model_config(temperature=0.5)
▫ THEN temperature=0.5 (explicit overrides default 0.0)
• GIVEN ModelConfig default temperature=0.0
• WHEN resolve_model_config() without temperature
▫ THEN temperature=0.0 (uses ModelConfig default)
• WHEN resolve_model_config(model=“openai/gpt-4”, param=“value”)
▫ THEN no mutation of input kwargs; resolver succeeds without raising

Assumptions
• ScenarioConfig.default_config is mutated in tests and reset in setup/teardown.
• ModelConfig.model_dump(exclude_none=True) returns provider-specific extras to merge into extra_params.

Base automatically changed from fix/extra-params-bug to main October 9, 2025 17:09
@drewdrewthis drewdrewthis force-pushed the fix/refactor-conig-design branch from fe01647 to 7509d72 Compare October 9, 2025 17:11
@drewdrewthis drewdrewthis self-assigned this Oct 9, 2025
@drewdrewthis drewdrewthis marked this pull request as draft October 9, 2025 17:11
@drewdrewthis drewdrewthis marked this pull request as ready for review October 9, 2025 17:47
@rogeriochaves rogeriochaves force-pushed the main branch 2 times, most recently from 77a92af to 9fdb87c Compare December 16, 2025 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants