Skip to content

Commit 4ccbebf

Browse files
Fix active_model being ignored by LLM providers due to wrong config key (#73)
The factory was setting active_model under 'model_id' in the provider config, but providers expect 'model', so the configured model override had no effect. Rename the key and add tests to cover the correct behavior.
1 parent 402cf51 commit 4ccbebf

5 files changed

Lines changed: 59 additions & 5 deletions

File tree

chuck_data/llm/factory.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,9 @@ def _get_provider_config(provider_name: str) -> dict:
8181
config = provider_configs.get(provider_name, {})
8282
logger.debug(f"Loaded config for {provider_name}")
8383

84-
# Add active_model to config if set (overrides provider-specific model_id)
84+
# Add active_model to config if set (overrides provider-specific model)
8585
if hasattr(chuck_config, "active_model") and chuck_config.active_model:
86-
config["model_id"] = chuck_config.active_model
86+
config["model"] = chuck_config.active_model
8787
logger.debug(
8888
f"Using active_model from config: {chuck_config.active_model}"
8989
)

chuck_data/version.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
__version__ = "0.4.5"
1+
__version__ = "0.4.6"

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ license-files = []
1212

1313
[project]
1414
name = "chuck-data"
15-
version = "0.4.5"
15+
version = "0.4.6"
1616
description = "Command line AI for customer data"
1717
readme = "README.md"
1818
requires-python = ">=3.10"

tests/unit/llm/test_factory.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,60 @@ def test_get_provider_config_returns_empty_dict_on_error(self):
5757
config = LLMProviderFactory._get_provider_config("databricks")
5858
assert config == {}
5959

60+
def test_get_provider_config_active_model_uses_model_key(self):
61+
"""active_model is passed as 'model', not 'model_id', so providers accept it."""
62+
with patch("chuck_data.config.get_config_manager") as mock_config:
63+
mock_config_obj = MagicMock()
64+
mock_config_obj.llm_provider_config = {}
65+
mock_config_obj.active_model = "databricks-claude-3-7-sonnet"
66+
mock_config.return_value.get_config.return_value = mock_config_obj
67+
68+
config = LLMProviderFactory._get_provider_config("databricks")
69+
70+
assert config.get("model") == "databricks-claude-3-7-sonnet"
71+
assert "model_id" not in config
72+
73+
def test_get_provider_config_active_model_overrides_provider_specific_model(self):
74+
"""active_model overrides any model set in provider-specific config."""
75+
with patch("chuck_data.config.get_config_manager") as mock_config:
76+
mock_config_obj = MagicMock()
77+
mock_config_obj.llm_provider_config = {"databricks": {"model": "old-model"}}
78+
mock_config_obj.active_model = "databricks-claude-3-7-sonnet"
79+
mock_config.return_value.get_config.return_value = mock_config_obj
80+
81+
config = LLMProviderFactory._get_provider_config("databricks")
82+
83+
assert config.get("model") == "databricks-claude-3-7-sonnet"
84+
85+
def test_get_provider_config_no_active_model_returns_provider_config(self):
86+
"""Without active_model, provider-specific config is returned unchanged."""
87+
with patch("chuck_data.config.get_config_manager") as mock_config:
88+
mock_config_obj = MagicMock()
89+
mock_config_obj.llm_provider_config = {
90+
"databricks": {"model": "some-default-model"}
91+
}
92+
mock_config_obj.active_model = None
93+
mock_config.return_value.get_config.return_value = mock_config_obj
94+
95+
config = LLMProviderFactory._get_provider_config("databricks")
96+
97+
assert config.get("model") == "some-default-model"
98+
99+
def test_create_databricks_provider_with_active_model(self):
100+
"""Factory creates DatabricksProvider with active_model passed as 'model'."""
101+
with patch("chuck_data.config.get_config_manager") as mock_config:
102+
mock_config_obj = MagicMock()
103+
mock_config_obj.llm_provider_config = {}
104+
mock_config_obj.active_model = "databricks-claude-3-7-sonnet"
105+
mock_config.return_value.get_config.return_value = mock_config_obj
106+
107+
from chuck_data.llm.providers.databricks import DatabricksProvider
108+
109+
provider = LLMProviderFactory.create("databricks")
110+
111+
assert isinstance(provider, DatabricksProvider)
112+
assert provider.default_model == "databricks-claude-3-7-sonnet"
113+
60114
@patch("chuck_data.llm.providers.aws_bedrock.boto3")
61115
def test_create_aws_bedrock_provider(self, mock_boto3):
62116
"""Factory can create AWS Bedrock provider."""

uv.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)