fix: Sanitize function names and trim descriptions for LLM compatibility #41

vblagoje · 2024-07-22T11:01:03Z

Why:

Fixes subtle naming bugs to meet LLM naming requirements for function naming patterns and description length limitations.
Not all operation names from OpenAPI spec and long descriptions are accepted by LLMs.

What:

Implemented function name sanitization to match pattern ^[a-zA-Z0-9_-]+$
Trimmed descriptions to max 1024 characters
Updated schema conversion and operation finding logic
Added and expanded tests

How:

sanitized_name = sanitize_function_name("function-name/with/special_chars")
# Result: "function_name_with_special_chars"

sanitized_desc = sanitize({"description": "Long text..."})
# Result: {"description": "Trimmed to 1024 characters..."}

How did you test it:

Unit tests for sanitization functions
Updated integration tests for edge cases
Verified compatibility with existing schemas

Notes for the reviewer:

Regex pattern alignment with LLM conventions
Consistency of sanitized names across the codebase

coveralls · 2024-07-22T11:05:50Z

Pull Request Test Coverage Report for Build 10042587214

Details

37 of 37 (100.0%) changed or added relevant lines in 4 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.2%) to 90.914%

Totals
Change from base Build 10038469299:	0.2%
Covered Lines:	1751
Relevant Lines:	1926

💛 - Coveralls

shadeMe · 2024-07-22T13:03:32Z

haystack_experimental/components/tools/openapi/types.py

+
+def sanitize(data: Dict[str, Any]) -> Dict[str, Any]:
+    """
+    Sanitizes the given function calling definition by adjusting its properties to LLM requirements.


LLM requirements is rather vague. Is there some documentation that defines what that means, i.e., where are these requirements coming from?

@shadeMe I couldn't find this in documentation but you hit it in the error reports returned by LLM once you use enough openapi specs. For example:

# BadRequestError: Error code: 400 - {'error': {'message': "Invalid 'tools[0].function.name': string does not match pattern. Expected a string that matches the pattern '^[a-zA-Z0-9_-]+$'.", 'type': 'invalid_request_error', 'param': 'tools[0].function.name', 'code': 'invalid_value'}}

Once you apply this pattern to method is starts to work. Same for length of description field

Which LLM? Which provider? Are there providers/models that don't accept underscores? If this code doesn't follow an official spec, that we should not make any assumptions and perform this sanitization (normalization, actually) on a per-LLM provider basis at the least.

Also, this kind of impl. detail of LLM-specific requirements shouldn't leak into code that handles OpenAPI - one should perform this normalization just-in-time, i.e, just before passing it to the generator.

Ok, I'll close this PR and rethink it now

shadeMe · 2024-07-22T13:03:48Z

haystack_experimental/components/tools/openapi/types.py

+    :param data: The function calling definition(s) to sanitize.
+    :return: A sanitized function calling definition.
+    """
+    if isinstance(data, dict):


Why is this check required?

It is required for recursive search of the function calling dict structure and sanitize recursive call might not be called on dict. We need recursion to find all those nested description keys and make sure they are "sanitized"

Then the check should be on line 58.

shadeMe · 2024-07-22T13:04:41Z

haystack_experimental/components/tools/openapi/types.py

+    if isinstance(data, dict):
+        sanitized_data: Dict[str, Any] = {}
+        for key, value in data.items():
+            if key == "name" and isinstance(value, str):


Why is the type check needed here? What are the expected types of the value?

Sometimes a parameter in function calling def can have a name name. And this parameter is not a string but another dict

Can we formally codify this in some manner? I feel like we have far too many Any type hints w.r.t the OpenAPI specific code, and having some kind of schema (or at least better type annotation) would help make this more readable.

I understand the sentiment @shadeMe . We have nicely, as much as needed imho, typed OpenAPISpec itself. Here we are dealing with these tools (function) definitions specified by various LLM providers. They look like this:

Cohere https://docs.cohere.com/docs/tool-use#step-1

Anthropic https://docs.anthropic.com/en/docs/build-with-claude/tool-use#tool-use-examples

OpenAI https://cookbook.openai.com/examples/how_to_call_functions_with_chat_models#basic-concepts

In other words they are just dictionaries. And although it would be nice to bring some order and typing perhaps it is not worth it given how they change and get expanded with new options. Maybe just providing references to these dictionary structures would help a reader quite a lot.

shadeMe · 2024-07-22T13:04:58Z

haystack_experimental/components/tools/openapi/types.py

+        for key, value in data.items():
+            if key == "name" and isinstance(value, str):
+                sanitized_data[key] = sanitize_function_name(value)
+            elif key == "description" and isinstance(value, str):


Same as above.

Yes exactly

shadeMe · 2024-07-22T13:05:42Z

haystack_experimental/components/tools/openapi/types.py

+    return sanitize_function_name(path + "_" + http_method.lower())
+
+
+def sanitize(data: Dict[str, Any]) -> Dict[str, Any]:


Let's go with a more explicit name - sanitize_function_calling_definition?

shadeMe · 2024-07-22T13:06:23Z

haystack_experimental/components/tools/openapi/types.py

+        return data
+
+
+def sanitize_function_name(name: str) -> str:


More explicit naming.

haystack_experimental/components/tools/openapi/types.py

Co-authored-by: Madeesh Kannan <[email protected]>

vblagoje · 2024-07-22T14:40:25Z

I'll rethink how to do this in a better way outside of OpenAPISpec across LLMs so we can reuse it in the future. Closing

Sanatizing FC definitions

f2456ec

vblagoje requested a review from a team as a code owner July 22, 2024 11:01

vblagoje requested review from davidsbatista and removed request for a team July 22, 2024 11:01

Sanitize only one FC definition at a time

722120b

shadeMe requested review from shadeMe and removed request for davidsbatista July 22, 2024 12:58

shadeMe suggested changes Jul 22, 2024

View reviewed changes

vblagoje and others added 3 commits July 22, 2024 15:29

Update haystack_experimental/components/tools/openapi/types.py

f0f2079

Co-authored-by: Madeesh Kannan <[email protected]>

Update haystack_experimental/components/tools/openapi/types.py

664956f

Co-authored-by: Madeesh Kannan <[email protected]>

PR feedback

58c81dc

vblagoje closed this Jul 22, 2024

vblagoje deleted the sanitize_functions branch July 23, 2024 10:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Sanitize function names and trim descriptions for LLM compatibility #41

fix: Sanitize function names and trim descriptions for LLM compatibility #41

vblagoje commented Jul 22, 2024 •

edited

Loading

coveralls commented Jul 22, 2024 •

edited

Loading

shadeMe Jul 22, 2024

vblagoje Jul 22, 2024 •

edited

Loading

shadeMe Jul 22, 2024

vblagoje Jul 22, 2024

shadeMe Jul 22, 2024

vblagoje Jul 22, 2024 •

edited

Loading

shadeMe Jul 22, 2024

shadeMe Jul 22, 2024

vblagoje Jul 22, 2024

shadeMe Jul 22, 2024

vblagoje Jul 23, 2024 •

edited

Loading

shadeMe Jul 22, 2024

vblagoje Jul 22, 2024

shadeMe Jul 22, 2024

shadeMe Jul 22, 2024

vblagoje commented Jul 22, 2024

		return sanitize_function_name(path + "_" + http_method.lower())


		def sanitize(data: Dict[str, Any]) -> Dict[str, Any]:

fix: Sanitize function names and trim descriptions for LLM compatibility #41

fix: Sanitize function names and trim descriptions for LLM compatibility #41

Conversation

vblagoje commented Jul 22, 2024 • edited Loading

Why:

What:

How:

How did you test it:

Notes for the reviewer:

coveralls commented Jul 22, 2024 • edited Loading

Pull Request Test Coverage Report for Build 10042587214

Details

💛 - Coveralls

Choose a reason for hiding this comment

vblagoje Jul 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vblagoje Jul 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vblagoje Jul 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vblagoje commented Jul 22, 2024

vblagoje commented Jul 22, 2024 •

edited

Loading

coveralls commented Jul 22, 2024 •

edited

Loading

vblagoje Jul 22, 2024 •

edited

Loading

vblagoje Jul 22, 2024 •

edited

Loading

vblagoje Jul 23, 2024 •

edited

Loading