refactor: address review feedback on tool custom parameters

- Extract _get_tool_custom_parameter() helper to deduplicate across 4 classes
- Extract _serialize_tool_custom_parameters() helper for to_dict serialization
- Add ToolCustomParametersMap type alias for legibility
- Only store tools that actually have customParameters (skip bare tool entries)
- Add docstring on to_dict explaining completion-config serialization-path fallback
- Add comment about dict insertion order in serialization helper

Co-Authored-By: traci@launchdarkly.com <traci@launchdarkly.com>

Author: devin-ai-integration[bot]

packages/sdk/server-ai/src/ldai/__init__.py

@@ -28,6 +28,7 @@
     LDMessage,
     ModelConfig,
     ProviderConfig,
+    ToolCustomParametersMap,
 )
 from ldai.providers import (
     AgentGraphResult,
@@ -68,6 +69,7 @@
     'LDMessage',
     'ModelConfig',
     'ProviderConfig',
+    'ToolCustomParametersMap',
     'log',
     # Deprecated exports
     'AIConfig',

packages/sdk/server-ai/src/ldai/client.py

@@ -25,6 +25,7 @@
     LDMessage,
     ModelConfig,
     ProviderConfig,
+    ToolCustomParametersMap,
 )
 from ldai.providers import ToolRegistry
 from ldai.providers.runner_factory import RunnerFactory
@@ -756,7 +757,7 @@ def __evaluate(
     ) -> Tuple[
         Optional[ModelConfig], Optional[ProviderConfig], Optional[List[LDMessage]],
         Optional[str], LDAIConfigTracker, bool, Optional[Any], Dict[str, Any],
-        Optional[Dict[str, Dict[str, Any]]]
+        Optional[ToolCustomParametersMap]
     ]:
         """
         Internal method to evaluate a configuration and extract components.
@@ -836,13 +837,22 @@ def __evaluate(
 
         tool_custom_parameters = None
         model_raw = variation.get('model')
-        params_raw = model_raw.get('parameters') if isinstance(model_raw, dict) else None
-        tool_defs_raw = params_raw.get('tools') if isinstance(params_raw, dict) else None
+        params_raw = (
+            model_raw.get('parameters')
+            if isinstance(model_raw, dict) else None
+        )
+        tool_defs_raw = (
+            params_raw.get('tools')
+            if isinstance(params_raw, dict) else None
+        )
         if isinstance(tool_defs_raw, list):
-            parsed: Dict[str, Dict[str, Any]] = {}
+            parsed: ToolCustomParametersMap = {}
             for t in tool_defs_raw:
-                if isinstance(t, dict) and t.get('name'):
-                    parsed[t['name']] = t.get('customParameters') or {}
+                if not isinstance(t, dict) or not t.get('name'):
+                    continue
+                cp = t.get('customParameters')
+                if isinstance(cp, dict) and cp:
+                    parsed[t['name']] = cp
             if parsed:
                 tool_custom_parameters = parsed
 

packages/sdk/server-ai/src/ldai/models.py

@@ -4,6 +4,58 @@
 
 from ldai.tracker import LDAIConfigTracker
 
+# Type alias for tool custom parameters: maps tool name -> custom params dict
+ToolCustomParametersMap = Dict[str, Dict[str, Any]]
+
+
+def _get_tool_custom_parameter(
+    tool_custom_parameters: Optional['ToolCustomParametersMap'],
+    tool_name: str,
+    key: str,
+) -> Any:
+    """Retrieve a custom parameter for a specific tool.
+
+    :param tool_custom_parameters: The tool custom parameters map.
+    :param tool_name: The name of the tool.
+    :param key: The custom parameter key to look up.
+    :return: The parameter value, or None if not found.
+    """
+    if tool_custom_parameters is None:
+        return None
+    tool_params = tool_custom_parameters.get(tool_name)
+    if tool_params is None:
+        return None
+    return tool_params.get(key)
+
+
+def _serialize_tool_custom_parameters(
+    result: Dict[str, Any],
+    tool_custom_parameters: Optional['ToolCustomParametersMap'],
+) -> None:
+    """Serialize tool_custom_parameters into the result dict.
+
+    Injects tools into ``result['model']['parameters']['tools']``.
+    Iteration order follows dict insertion order (Python 3.7+); if the
+    original flag payload order matters, the caller must ensure it is
+    preserved at parse time.
+
+    :param result: The mutable dict being built by to_dict().
+    :param tool_custom_parameters: The tool custom parameters map.
+    """
+    if tool_custom_parameters is None:
+        return
+    model = result.get('model') or {}
+    params = model.get('parameters') or {}
+    tools_list = []
+    for name, custom_params in tool_custom_parameters.items():
+        tool_entry: Dict[str, Any] = {'name': name}
+        if custom_params:
+            tool_entry['customParameters'] = custom_params
+        tools_list.append(tool_entry)
+    params['tools'] = tools_list
+    model['parameters'] = params
+    result['model'] = model
+
 
 @dataclass
 class LDMessage:
@@ -208,43 +260,33 @@ class AICompletionConfigDefault(AIConfigDefault):
     """
     messages: Optional[List[LDMessage]] = None
     judge_configuration: Optional[JudgeConfiguration] = None
-    tool_custom_parameters: Optional[Dict[str, Dict[str, Any]]] = None
+    tool_custom_parameters: Optional[ToolCustomParametersMap] = None
 
-    def get_tool_custom_parameter(self, tool_name: str, key: str) -> Any:
-        """
-        Retrieve a custom parameter for a specific tool.
-
-        :param tool_name: The name of the tool.
-        :param key: The custom parameter key to look up.
-        :return: The parameter value, or None if not found.
-        """
-        if self.tool_custom_parameters is None:
-            return None
-        tool_params = self.tool_custom_parameters.get(tool_name)
-        if tool_params is None:
-            return None
-        return tool_params.get(key)
+    def get_tool_custom_parameter(
+        self, tool_name: str, key: str,
+    ) -> Any:
+        """Retrieve a custom parameter for a specific tool."""
+        return _get_tool_custom_parameter(
+            self.tool_custom_parameters, tool_name, key,
+        )
 
     def to_dict(self) -> dict:
-        """
-        Render the given default values as an AICompletionConfigDefault-compatible dictionary object.
+        """Render the given default values as an AICompletionConfigDefault-compatible dictionary object.
+
+        Note: tool_custom_parameters are serialized into
+        ``model.parameters.tools`` so that they are carried inside the
+        variation dict used as the evaluation fallback.  This is how
+        completion-config defaults reach ``_completion_config`` — there
+        is no separate ``or default.tool_custom_parameters`` fallback
+        like agent configs have.
         """
         result = self._base_to_dict()
         result['messages'] = [message.to_dict() for message in self.messages] if self.messages else None
         if self.judge_configuration is not None:
             result['judgeConfiguration'] = self.judge_configuration.to_dict()
-        if self.tool_custom_parameters is not None:
-            model = result.get('model') or {}
-            params = model.get('parameters') or {}
-            tools_list = []
-            for name, custom_params in self.tool_custom_parameters.items():
-                tool_entry: Dict[str, Any] = {'name': name}
-                if custom_params:
-                    tool_entry['customParameters'] = custom_params
-                tools_list.append(tool_entry)
-            params['tools'] = tools_list
-            model['parameters'] = params
-            result['model'] = model
+        _serialize_tool_custom_parameters(
+            result, self.tool_custom_parameters,
+        )
         return result
 
 
@@ -255,22 +297,15 @@ class AICompletionConfig(AIConfig):
     """
     messages: Optional[List[LDMessage]] = None
     judge_configuration: Optional[JudgeConfiguration] = None
-    tool_custom_parameters: Optional[Dict[str, Dict[str, Any]]] = None
+    tool_custom_parameters: Optional[ToolCustomParametersMap] = None
 
-    def get_tool_custom_parameter(self, tool_name: str, key: str) -> Any:
-        """
-        Retrieve a custom parameter for a specific tool.
-
-        :param tool_name: The name of the tool.
-        :param key: The custom parameter key to look up.
-        :return: The parameter value, or None if not found.
-        """
-        if self.tool_custom_parameters is None:
-            return None
-        tool_params = self.tool_custom_parameters.get(tool_name)
-        if tool_params is None:
-            return None
-        return tool_params.get(key)
+    def get_tool_custom_parameter(
+        self, tool_name: str, key: str,
+    ) -> Any:
+        """Retrieve a custom parameter for a specific tool."""
+        return _get_tool_custom_parameter(
+            self.tool_custom_parameters, tool_name, key,
+        )
 
     def to_dict(self) -> dict:
         """
@@ -294,22 +329,15 @@ class AIAgentConfigDefault(AIConfigDefault):
     """
     instructions: Optional[str] = None
     judge_configuration: Optional[JudgeConfiguration] = None
-    tool_custom_parameters: Optional[Dict[str, Dict[str, Any]]] = None
+    tool_custom_parameters: Optional[ToolCustomParametersMap] = None
 
-    def get_tool_custom_parameter(self, tool_name: str, key: str) -> Any:
-        """
-        Retrieve a custom parameter for a specific tool.
-
-        :param tool_name: The name of the tool.
-        :param key: The custom parameter key to look up.
-        :return: The parameter value, or None if not found.
-        """
-        if self.tool_custom_parameters is None:
-            return None
-        tool_params = self.tool_custom_parameters.get(tool_name)
-        if tool_params is None:
-            return None
-        return tool_params.get(key)
+    def get_tool_custom_parameter(
+        self, tool_name: str, key: str,
+    ) -> Any:
+        """Retrieve a custom parameter for a specific tool."""
+        return _get_tool_custom_parameter(
+            self.tool_custom_parameters, tool_name, key,
+        )
 
     def to_dict(self) -> Dict[str, Any]:
         """
@@ -320,18 +348,9 @@ def to_dict(self) -> Dict[str, Any]:
             result['instructions'] = self.instructions
         if self.judge_configuration is not None:
             result['judgeConfiguration'] = self.judge_configuration.to_dict()
-        if self.tool_custom_parameters is not None:
-            model = result.get('model') or {}
-            params = model.get('parameters') or {}
-            tools_list = []
-            for name, custom_params in self.tool_custom_parameters.items():
-                tool_entry: Dict[str, Any] = {'name': name}
-                if custom_params:
-                    tool_entry['customParameters'] = custom_params
-                tools_list.append(tool_entry)
-            params['tools'] = tools_list
-            model['parameters'] = params
-            result['model'] = model
+        _serialize_tool_custom_parameters(
+            result, self.tool_custom_parameters,
+        )
         return result
 
 
@@ -342,22 +361,15 @@ class AIAgentConfig(AIConfig):
     """
     instructions: Optional[str] = None
     judge_configuration: Optional[JudgeConfiguration] = None
-    tool_custom_parameters: Optional[Dict[str, Dict[str, Any]]] = None
-
-    def get_tool_custom_parameter(self, tool_name: str, key: str) -> Any:
-        """
-        Retrieve a custom parameter for a specific tool.
-
-        :param tool_name: The name of the tool.
-        :param key: The custom parameter key to look up.
-        :return: The parameter value, or None if not found.
-        """
-        if self.tool_custom_parameters is None:
-            return None
-        tool_params = self.tool_custom_parameters.get(tool_name)
-        if tool_params is None:
-            return None
-        return tool_params.get(key)
+    tool_custom_parameters: Optional[ToolCustomParametersMap] = None
+
+    def get_tool_custom_parameter(
+        self, tool_name: str, key: str,
+    ) -> Any:
+        """Retrieve a custom parameter for a specific tool."""
+        return _get_tool_custom_parameter(
+            self.tool_custom_parameters, tool_name, key,
+        )
 
     def to_dict(self) -> Dict[str, Any]:
         """

packages/sdk/server-ai/tests/test_agents.py

@@ -397,13 +397,15 @@ def test_agent_config_has_tools(ldai_client: LDAIClient):
 
     assert agent.enabled is True
     assert agent.tool_custom_parameters is not None
-    assert len(agent.tool_custom_parameters) == 3
+    # Only tools with non-empty customParameters are included
+    assert len(agent.tool_custom_parameters) == 2
 
     assert agent.get_tool_custom_parameter('get-order', 'includeHistory') is True
     assert agent.get_tool_custom_parameter('get-order', 'maxItems') == 5
     assert agent.get_tool_custom_parameter('search-products', 'category') == 'electronics'
+    # 'send-email' has no customParameters, so it is not in the map
+    assert 'send-email' not in agent.tool_custom_parameters
     assert agent.get_tool_custom_parameter('send-email', 'anything') is None
-    assert 'send-email' in agent.tool_custom_parameters
 
 
 def test_agent_config_tools_fallback_to_default(ldai_client: LDAIClient):

packages/sdk/server-ai/tests/test_model_config.py

@@ -454,13 +454,15 @@ def test_completion_config_has_tools(ldai_client: LDAIClient):
     config = ldai_client.completion_config('config-with-tools', context, default)
 
     assert config.tool_custom_parameters is not None
-    assert len(config.tool_custom_parameters) == 3
+    # Only tools with non-empty customParameters are included
+    assert len(config.tool_custom_parameters) == 2
 
     assert config.get_tool_custom_parameter('web_search', 'maxResults') == 10
     assert config.get_tool_custom_parameter('web_search', 'region') == 'us'
     assert config.get_tool_custom_parameter('get_weather', 'units') == 'celsius'
+    # 'calculator' has no customParameters, so it is not in the map
+    assert 'calculator' not in config.tool_custom_parameters
     assert config.get_tool_custom_parameter('calculator', 'anything') is None
-    assert 'calculator' in config.tool_custom_parameters
 
 
 def test_completion_config_no_tools(ldai_client: LDAIClient):