-
Notifications
You must be signed in to change notification settings - Fork 24
Fix PredictModel
to return correct output fields
#55
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
base: main
Are you sure you want to change the base?
Conversation
PredictModel
to return correct output fields
cognify/frontends/dspy/connector.py
Outdated
warnings.warn( | ||
"Original module contained reasoning. This will be stripped. Add reasoning as a cog instead", | ||
f"DSPy module {name} contained reasoning. This may lead to undefined behavior.", |
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.
this needs more explanation to the user. say something like Cognify performs its own reasoning prompt optimization automatically, which may not work well together with DSPy's; consider removing DSPy's reasoning prompts.
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.
Changed
UserWarning, | ||
) | ||
|
||
if isinstance(dspy_predictor, dspy.Retrieve): | ||
warnings.warn( | ||
"Original module is a `Retrieve`. This will be ignored", UserWarning | ||
"Original module is a `dspy.Retrieve`. This will be ignored", UserWarning |
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.
what does this retrive mean? why ignore?
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.
dspy.Retrieve
is their retriever. They still consider retrieval to be a "module", and since we don't optimize retrieval, I just ignore it. This means whenever it gets called in the actual workflow, we will call the original retrieve module.
Modify
PredictModel
(in DSPy connector) to directly use the DSPy output parsing instead of enforcing ourStructuredModel
Also, remove deletion of reasoning field from signature to maintain compatibility with user's original workflow.