Skip to content

fix: parse model.parameters.tools as list #160

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jsonbailey wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: parse model.parameters.tools as list #160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 28 additions & 27 deletions packages/sdk/server-ai/src/ldai/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,44 +53,45 @@
_DISABLED_JUDGE_DEFAULT = AIJudgeConfigDefault.disabled()


def _parse_tools(tools_data: Optional[Dict[str, Any]]) -> Optional[Dict[str, LDTool]]:
"""Parse the root-level tools map from a flag variation dict."""
if not isinstance(tools_data, dict):
if tools_data is not None:
log.warning('Skipping tools: expected a dict, got %s', type(tools_data).__name__)
return None
result: Dict[str, LDTool] = {}
for tool_name, tool_dict in tools_data.items():
if not isinstance(tool_dict, dict):
log.warning('Skipping tool "%s": expected a dict, got %s', tool_name, type(tool_dict).__name__)
continue
result[tool_name] = LDTool(
name=tool_dict.get('name', tool_name),
description=tool_dict.get('description'),
type=tool_dict.get('type'),
parameters=tool_dict.get('parameters'),
custom_parameters=tool_dict.get('customParameters'),
)
return result or None


def _resolve_tools(variation: Dict[str, Any]) -> Optional[Dict[str, LDTool]]:
if 'tools' in variation:
return _parse_tools(variation['tools'])
tools_data = variation['tools']
if not isinstance(tools_data, dict):
return None
tools: Dict[str, LDTool] = {}
for tool_name, tool_dict in tools_data.items():
if isinstance(tool_dict, dict):
Copy link
Copy Markdown

@cursor cursor Bot May 1, 2026

Choose a reason for hiding this comment

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

Silent data dropping in inlined parsing logic lacks logging

Low Severity

The removed _parse_tools helper previously logged warnings when discarding malformed tool entries (e.g., non-dict values). That same parsing logic is now inlined into _resolve_tools at lines 63 and 85, where entries failing isinstance checks are silently skipped without any warning or debug log. The rule requires log messages when flag variation data is silently dropped because it doesn't match the expected type. Although _resolve_tools is exempted for navigation type-checks (e.g. if not isinstance(model, dict): return None), the entry-level iteration and filtering on lines 63 and 85 is data-validation/parsing work — the same logic that _parse_tools performed with warnings.

Additional Locations (1)
Fix in Cursor Fix in Web

Triggered by learned rule: Flag silent data dropping in parsing helpers — warn/log on malformed input

Reviewed by Cursor Bugbot for commit 05e9c78. Configure here.

tools[tool_name] = LDTool(
name=str(tool_dict.get('name', tool_name)),
description=tool_dict.get('description'),
type=tool_dict.get('type'),
parameters=tool_dict.get('parameters'),
custom_parameters=tool_dict.get('customParameters'),
)
return tools or None

model = variation.get('model')
if not isinstance(model, dict):
return None
parameters = model.get('parameters')
if not isinstance(parameters, dict):
return None
tools_data = parameters.get('tools')
if not isinstance(tools_data, dict):
if tools_data is not None:
log.warning('Skipping model.parameters.tools: expected a dict, got %s', type(tools_data).__name__)
tools_list = parameters.get('tools')
if not isinstance(tools_list, list):
return None

return _parse_tools(tools_data)
tools = {}
for item in tools_list:
if isinstance(item, dict) and item.get('name'):
tool_name = str(item['name'])
tools[tool_name] = LDTool(
name=tool_name,
description=item.get('description'),
type=item.get('type'),
parameters=item.get('parameters'),
custom_parameters=item.get('customParameters'),
)
return tools or None


class LDAIClient:
Expand Down
62 changes: 46 additions & 16 deletions packages/sdk/server-ai/tests/test_tools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,14 @@ def td() -> TestData:
'name': 'gpt-5',
'parameters': {
'temperature': 0.5,
'tools': {
'param-tool': {
'tools': [
{
'name': 'param-tool',
'type': 'function',
'description': 'A tool from model params',
'parameters': {'type': 'object'},
}
},
],
},
},
'messages': [{'role': 'user', 'content': 'Hello'}],
Expand All @@ -92,12 +92,12 @@ def td() -> TestData:
'model': {
'name': 'gpt-5',
'parameters': {
'tools': {
'model-param-tool': {
'tools': [
{
'name': 'model-param-tool',
'type': 'function',
}
},
],
},
},
'messages': [{'role': 'user', 'content': 'Hello'}],
Expand All @@ -114,7 +114,7 @@ def td() -> TestData:
)

td.update(
td.flag('completion-model-params-tools-as-list')
td.flag('completion-model-params-tools-list-format')
.variations(
{
'model': {
Expand All @@ -133,18 +133,17 @@ def td() -> TestData:
)

td.update(
td.flag('completion-model-params-tools-missing-name')
td.flag('completion-model-params-tools-as-dict')
.variations(
{
'model': {
'name': 'gpt-5',
'parameters': {
'tools': {
'valid-tool': {
'name': 'valid-tool',
'dict-tool': {
'name': 'dict-tool',
'type': 'function',
},
'bad-entry': 'not-a-dict',
},
},
},
Expand All @@ -155,6 +154,29 @@ def td() -> TestData:
.variation_for_all(0)
)

td.update(
td.flag('completion-model-params-tools-bad-entries')
.variations(
{
'model': {
'name': 'gpt-5',
'parameters': {
'tools': [
{
'name': 'valid-tool',
'type': 'function',
},
'not-a-dict',
],
},
},
'messages': [{'role': 'user', 'content': 'Hello'}],
'_ldMeta': {'enabled': True, 'variationKey': 'v1', 'version': 1},
},
)
.variation_for_all(0)
)

return td


Expand Down Expand Up @@ -249,15 +271,23 @@ def test_completion_config_root_tools_take_priority_over_model_params(client, co
assert 'model-param-tool' not in result.tools


def test_completion_config_model_params_tools_as_list_returns_none(client, context):
result = client.completion_config('completion-model-params-tools-as-list', context, AICompletionConfigDefault())
def test_completion_config_model_params_tools_list_format_is_parsed(client, context):
result = client.completion_config('completion-model-params-tools-list-format', context, AICompletionConfigDefault())

assert result.tools is not None
assert 'list-tool' in result.tools
assert result.tools['list-tool'].type == 'function'


def test_completion_config_model_params_tools_dict_format_returns_none(client, context):
result = client.completion_config('completion-model-params-tools-as-dict', context, AICompletionConfigDefault())

assert result.tools is None


def test_completion_config_model_params_tools_skips_bad_entries_silently(client, context):
result = client.completion_config('completion-model-params-tools-missing-name', context, AICompletionConfigDefault())
def test_completion_config_model_params_tools_skips_bad_entries(client, context):
result = client.completion_config('completion-model-params-tools-bad-entries', context, AICompletionConfigDefault())

assert result.tools is not None
assert 'valid-tool' in result.tools
assert 'bad-entry' not in result.tools
assert len(result.tools) == 1
Loading