-
Notifications
You must be signed in to change notification settings - Fork 0
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.
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
DynamicPrompt class creation #13
base: cloned_master_4bbaa
Are you sure you want to change the base?
DynamicPrompt class creation #13
Conversation
Clone of the PR langchain-ai/langchain#49 |
My review is in progress 📖 - I will have feedback for you in a few minutes! |
WalkthroughThe recent updates to the Langchain library enhance its prompt management capabilities by introducing the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have reviewed your code and found 2 potential issues.
@root_validator() | ||
def template_is_valid(cls, values: Dict) -> Dict: | ||
"""Check that suffix and input variables are consistent.""" | ||
input_variables = values["input_variables"] | ||
suffix = values["suffix"] | ||
template_format = values["template_format"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The template_is_valid
method in the DynamicPrompt
class currently only validates the suffix of the template. For a dynamic prompt, it's important to validate the entire template, including the prefix and examples. Please update this method to construct and validate the full template to ensure that all parts of the dynamic prompt are consistent with the input variables.
Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.
def format(self, **kwargs: Any) -> str: | ||
"""Dynamically format the prompt with the inputs. | ||
|
||
Args: | ||
kwargs: Any arguments to be passed to the prompt template. | ||
|
||
Returns: | ||
A formatted string. | ||
|
||
Example: | ||
|
||
.. code-block:: python | ||
|
||
prompt.format(variable1="foo") | ||
""" | ||
# TODO segment self.examples based on example length & | ||
# input_variables length here | ||
example_str = self.example_separator.join(self.examples) | ||
template = self.prefix + example_str + self.suffix | ||
return _FORMATTER_MAPPING[self.template_format](template, **kwargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The format
method in the DynamicPrompt
class lacks proper error handling. It directly uses the formatted template without catching and handling potential exceptions that could occur during the formatting process. To improve the robustness of the code, consider wrapping the formatting logic inside a try-except block. This will allow you to catch and handle any exceptions, such as missing or invalid input variables, and provide meaningful error messages or raise appropriate exceptions to handle exceptional cases gracefully.
Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.
/review |
PR Reviewer Guide 🔍
|
@coderabbitai review |
Actions performedReview triggered.
|
@coderabbitai review |
Actions performedReview triggered.
|
@coderabbitai review |
Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
langchain/prompt.py (1)
166-184
: Review thetemplate_is_valid
method implementations for consistency.There are two implementations of the
template_is_valid
method inlangchain/prompt.py
. One checks the entire template and input variables, while the other only checks the suffix. Ensure that the validation logic is consistent and that the correct version is being used.
- File:
langchain/prompt.py
- Lines: Look for both implementations and verify their intended use.
Analysis chain
Expand validation to the entire template in
template_is_valid
.The method currently only validates the suffix. Consider expanding the validation to include the prefix and examples to ensure consistency across the entire template, as noted in previous comments.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `template_is_valid` to ensure it is only validating the suffix. # Test: Search for the method definition and usage. Expect: Validation logic should cover entire template. rg --type python -A 5 $'def template_is_valid'Length of output: 78
Script:
#!/bin/bash # Description: Verify the usage of `template_is_valid` to ensure it is only validating the suffix. # Corrected Test: Search for the method definition and usage. Expect: Validation logic should cover entire template. rg --type py -A 5 $'def template_is_valid'Length of output: 920
Tools
Ruff
183-183: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- langchain/init.py (2 hunks)
- langchain/prompt.py (1 hunks)
Additional context used
Ruff
langchain/prompt.py
183-183: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
Additional comments not posted (4)
langchain/__init__.py (1)
Line range hint
20-31
: LGTM! The import and__all__
list update are correct.The addition of
DynamicPrompt
to the import statement and the__all__
list is appropriate and ensures that the class is accessible from the top level of the module.langchain/prompt.py (3)
106-120
: ClassDynamicPrompt
documentation is clear and informative.The docstring provides a clear example of how to use the
DynamicPrompt
class, which is helpful for users.
122-138
: Attributes are well-defined and documented.The attributes of
DynamicPrompt
are clearly defined with appropriate docstrings, enhancing readability and understanding.
186-202
:from_examples
method is correctly implemented.The method correctly initializes a
DynamicPrompt
instance with the provided examples, prefix, suffix, and other parameters.
def format(self, **kwargs: Any) -> str: | ||
"""Dynamically format the prompt with the inputs. | ||
|
||
Args: | ||
kwargs: Any arguments to be passed to the prompt template. | ||
|
||
Returns: | ||
A formatted string. | ||
|
||
Example: | ||
|
||
.. code-block:: python | ||
|
||
prompt.format(variable1="foo") | ||
""" | ||
# TODO segment self.examples based on example length & | ||
# input_variables length here | ||
example_str = self.example_separator.join(self.examples) | ||
template = self.prefix + example_str + self.suffix | ||
return _FORMATTER_MAPPING[self.template_format](template, **kwargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Address TODO and improve error handling in format
.
The format
method has a TODO comment indicating incomplete logic. Additionally, consider wrapping the formatting logic in a try-except block to handle potential errors gracefully, as suggested in previous comments.
Would you like assistance in implementing the error handling or completing the TODO logic?
Checking that this structure looks generally ok -- going to sub in logic where the TODO comment is then add a test.
Description by Korbit AI
Note
This feature is in early access. You can enable or disable it in the Korbit Console.
What change is being made?
Add a new
DynamicPrompt
class to thelangchain
module for creating dynamic prompts for language models.Why are these changes being made?
This change introduces a flexible and reusable way to generate prompts dynamically based on provided examples, prefixes, and suffixes, enhancing the prompt generation capabilities of the library. This approach allows for more complex and varied prompt structures, which can be beneficial for different use cases and improves the overall functionality of the
langchain
module.Summary by CodeRabbit
New Features
DynamicPrompt
class for enhanced prompt management with large language models.DynamicPrompt
accessible at the top level of the package for easier use.Documentation
DynamicPrompt
in the module's__all__
list.