-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
dspy.Retry module and a conversation feedback adapter #7922
base: main
Are you sure you want to change the base?
Conversation
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.
Some comments following up from an offline chat with @okhat and @chenmoneygithub .
The high-level idea is to merge Retry
with Refine
, and see if we can utilize the existing ChatAdapter instead of the ConversationalFeedbackAdapter. A core change we need in order to perform this change is to modify the base instruction of the signature provided to Refine, and include a new dedicated dspy.History input field (which is passed in dynamically by Refine's forward method).
NOTE: This is already in great shape (works reliably, reduces cost, and runs faster). Great work!
def __init__(self, | ||
feedback_history: List[str] = None, | ||
attempt_outputs: List[str] = None, | ||
**kwargs | ||
): | ||
""" | ||
Initialize the adapter with feedback history and attempt outputs. | ||
|
||
Args: | ||
feedback_history: List of feedback strings from previous attempts | ||
attempt_outputs: List of prediction outputs from previous attempts | ||
**kwargs: Additional arguments to pass to ChatAdapter | ||
""" | ||
super().__init__(**kwargs) | ||
self.feedback_history = feedback_history or [] | ||
self.attempt_outputs = attempt_outputs or [] |
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.
Rather than providing feedback history and attempted outputs via the initializer for this adapter, incorporate them into the signature that the adapter accepts during format
. In this way, you can initialize one adapter, and it will operate smoothly across different inputs (with differing feedback histories and attempted outputs).
# Get the original messages, which include system, demos, and the final user message | ||
original_messages = super().format(signature, demos, inputs) | ||
# If there's no feedback history, return the original messages unchanged | ||
if not self.feedback_history: |
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.
Rather than accessing self here (and below), retrieve feedback from a provided a field that includes conversation history. I recommend also adding an optional parameter for the name of the field that includes the conversation history, and have it default to something like "feedback_history".
new_messages[0]["content"] += ( | ||
"\n\nNote: If previous attempts contained errors as identified by the user, " | ||
"you are expected to analyze and understand these errors, and generate a revised response " | ||
"that avoids repeating the same mistakes. Please ensure that your response addresses all issues raised." | ||
) |
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.
Include this in the core signature's instructions rather than the message-creation stage of the adapter.
for output, feedback in zip(self.attempt_outputs, self.feedback_history): | ||
# Add the previous attempt as an assistant message | ||
attempt_msg = {"role": "assistant", "content": output} | ||
new_messages.append(attempt_msg) | ||
# Add the feedback as a user message | ||
feedback_msg = {"role": "user", "content": feedback} | ||
new_messages.append(feedback_msg) | ||
# Add the final user message to prompt the next attempt | ||
final_user_message = original_messages[-1] | ||
new_messages.append({ | ||
"role": "user", | ||
"content": final_user_message['content'] | ||
}) |
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.
Move this code to retry
as part of forward
(or perhaps a helper function that is called by forward
), and construct history objects dynamically. If there is an existing utility for doing this within the adapters codebase, prefer using that existing functionality.
) | ||
|
||
# Use the Retryd module | ||
result = Retryd_summarizer(text="Some text to process") |
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.
nit: Add a line in which you access the output field from the prediction, and show its content via a print.
from dspy.adapters.conversation_feedback_adapter import ConversationalFeedbackAdapter | ||
|
||
|
||
class Retry(Module): |
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.
Merge this with Refine. Add a boolean parameter which determines whether to use LLM feedback (in addition to the verifiers, if present). Add an optional parameter for the LLM-feedback signature, where the default is the existing OfferFeedback
signature.
self.module = dspy.ChainOfThought(signature) | ||
else: | ||
self.module = dspy.Predict(signature) |
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.
Modify the signature to include a new dspy.History
field before creating the module. This is also the place to modify the signature's instructions. For now, we should add what you wrote above about how to process and improve on "user feedback". Worth adding a TODO to add the docstring descriptions of the verifiers as parts of the instruction (e.g., a length-verifier would include a bullet that specifies "{output_field} should be between {min_length} and {max_length} {words/sentences/paragraphs}"). This TODO is likely worth a PR in and of itself.
if self.verbose: | ||
print( | ||
f"Using conversational feedback with {len(feedback_history)} previous attempts") | ||
adapter = ConversationalFeedbackAdapter(feedback_history=feedback_history, attempt_outputs=attempt_outputs) |
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.
You should construct a list of messages and story them as an input (of type dspy.History) within kwargs. We may be able to get by here with just the existing ChatAdapter (unless we see that there is critical functionality in the new adapter which doesn't existing with ChatAdapter + modified signature with conversation history).
This PR introduces a new Retry module and ConversationalFeedbackAdapter to enhance DSPy's validation capabilities:
1. Added Conversational Feedback Approach:
ConversationalFeedbackAdapter
that extends ChatAdapter to inject feedback history2. Implemented Retry Module with Validator Support:
Retry
module that wraps any DSPy module (Predict or ChainOfThought)3. Enhanced Configurability and Debugging:
4. Comprehensive Documentation and Type Safety: