fix: refactor config design #144
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
• 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.