feat(prompts): add util for variable name extraction

[hassiebp]

Important

Introduces TemplateParser for template variable extraction and compilation, replacing existing methods in BasePromptClient, and adds tests for new functionality.

• Behavior:
  • Introduces TemplateParser class in model.py for extracting and compiling template variables.
  • Replaces BasePromptClient._compile_template_string with TemplateParser.compile_template in TextPromptClient and ChatPromptClient.
  • Adds variable_names() method to TextPromptClient and ChatPromptClient to extract variable names from templates.
• Tests:
  • Adds tests in test_prompt.py and test_prompt_compilation.py for TemplateParser methods compile_template and find_variable_names.
  • Tests cover various scenarios including missing variables, unmatched tags, and special characters.

^{This description was created by for 81e9006. It will automatically update as commits are pushed.}

[greptile-apps]

Disclaimer: Experimental PR review

PR Summary

Introduced a new TemplateParser class to centralize template parsing logic and added variable name extraction functionality for both text and chat prompts.

• Added TemplateParser class in langfuse/model.py with find_variable_names() and compile_template() methods for robust template handling
• Added variable_names() method to TextPromptClient returning List[str] of template variables
• Added variable_names() method to ChatPromptClient returning List[ChatMessageVariables] with role-specific variables
• Added comprehensive test suite in tests/test_prompt_compilation.py covering edge cases like whitespace, special characters and JSON content

_{3 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile}

[greptile-apps]

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Refactored variable name extraction in prompt clients by converting variable_names() method to a variables property and simplified chat message variable handling to return a flat list.

• Changed variable_names() to variables property in TextPromptClient and ChatPromptClient for better API design
• Simplified chat message variable extraction to return List[str] instead of role-based grouping
• Fixed mutable default argument issue in TemplateParser.compile_template()
• Added handling for None data parameter in compile_template

_{2 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile}

[hassiebp]

@rhighs - I have adjusted the variables getter on the ChatPromptClient to return a list of variables to be consistent with the TextPromptClient. I do not see the value of splitting the variables per chat message.

Do you have a strong opinion on this? If so, could you explain exactly why you'd benefit from that?

[greptile-apps]

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the recent changes, I'll provide a concise summary focusing only on the latest updates not covered in previous reviews:

Simplified template parsing implementation and improved test coverage for the TemplateParser class.

• Added extensive test cases in test_prompt_compilation.py for unescaped JSON handling and complex template scenarios
• Improved TemplateParser.compile_template() to handle unescaped JSON content without character escaping
• Added test coverage for edge cases like content as variable name and empty string replacements
• Added test cases for variable case sensitivity and JSON template handling

The changes focus on improving robustness and test coverage of the template parsing functionality, particularly around JSON content handling.

_{2 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile}

[greptile-apps]

logic: regex pattern \w+ only matches word chars - may need to support other valid variable name chars

[rhighs]

@rhighs - I have adjusted the variables getter on the ChatPromptClient to return a list of variables to be consistent with the TextPromptClient. I do not see the value of splitting the variables per chat message.

Do you have a strong opinion on this? If so, could you explain exactly why you'd benefit from that?

LGTM, no strong opinions tbh