Repository navigation
Fix Selected Response Bug - #274
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 30 |
| Duplication | 6 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #274 +/- ##
==========================================
+ Coverage 98.73% 98.75% +0.02%
==========================================
Files 12 12
Lines 2123 2154 +31
Branches 201 211 +10
==========================================
+ Hits 2096 2127 +31
Misses 6 6
Partials 21 21 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
This pull request is now ready for your review 💯 |
| INVALID_DATETIME_MESSAGE = "Invalid value. `{parameter_name}` must be a datetime object that includes timezone information." | ||
| INVALID_TEMPLATE_MESSAGE = "Invalid template. It must be an instance of `PromptTemplate` or `PresetPromptTemplate`." | ||
| INVALID_RESPONSE_MESSAGE = "Invalid response. It must be an instance of `Response`." | ||
| INVALID_RESPONSE_IDENTIFIER_MESSAGE = "Invalid response identifier. It must be a non-negative integer index or a valid UUIDv4 response ID." |
There was a problem hiding this comment.
I prefer:
Invalid response identifier. It must be a non-negative integer index or a valid response ID.
UUIDv4 is too technical term for an error message.
| else: | ||
| if len(self.responses) > 0 and isinstance(self.responses[0], Response): | ||
| format_kwargs.update({"response": self.responses[0].to_json()}) |
There was a problem hiding this comment.
I don't understand the relatedness of this change. Is this related to the response id issue or a separate bug fix?
There was a problem hiding this comment.
Previously, we only used an index to select a response. The default selected_response was the first one in the list. Now that we use a message ID for this purpose, the default selected response is None. This will lead to failures of some templates that use the term response. To make Memor backward-compatible and ensure a smoother flow, I added this if-else statement to the render method.
Reference Issues/PRs
#243
What does this implement/fix? Explain your changes
Promptclassremove_responsemethod modifiedPromptclassclear_responsesmethod modifiedPromptclassselect_responsemethod modifiedPromptclassupdate_responsesmethod modifiedAny other comments?