-
Notifications
You must be signed in to change notification settings - Fork 18
Fixed WA's Inf. Loop Msgs | Validated Claude's msg_hist | Added Typing Indicator #171
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
Fixed WA's Inf. Loop Msgs | Validated Claude's msg_hist | Added Typing Indicator #171
Conversation
Main changes: fix: Modify main_whatsapp to handle incoming messages with background tasks for better performance feat: Implement message splitting functionality in WhatsAppPresenter to handle long messages fix: Added validation check to AnsariClaude to ensure that a "tool_use" block is followed by only 1 "tool_result" block (in self.message_history) Extra details: fix: Added \n\n in AnsariClaude's response to separate between the text before tool's output and the tool's output refactor: Remove unused `stream` option feat: Enhance logging capabilities in ansari_logger to support file logging in DEV_MODE docs: Add example JSON structures for API responses in documentation feat: Update .gitignore to include logs directory fix: Update Python version in Jupyter notebook for API response structure
@@ -95,7 +95,7 @@ def process_input(self, user_input: str): | |||
if m: | |||
yield m | |||
|
|||
def replace_message_history(self, message_history: list[dict], use_tool=True, stream=True): |
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.
Removed stream parameters across different files, as that logic is not needed anymore (and was never implemented to begin with0
logger.debug( | ||
f"_log_message called with message role: {message.get('role')}, " | ||
f"content type: {type(message.get('content'))}, " | ||
f"_log_message called with message role: {role}{tool_name_log}, " |
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.
Logging the tool name as well in _log_message()
src/ansari/agents/ansari_claude.py
Outdated
# Claude API requires text content blocks to be non-empty | ||
message_content = [{"type": "text", "text": "I'm processing your request."}] | ||
# If no content blocks, use a single empty text element | ||
message_content = [{"type": "text", "text": ""}] |
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.
Empty content blocks causes Claude to crash (I think), did you discuss the changes in ansari_claude with Waleed?
- Introduced user-specific presenter creation for better context management. - Improved typing indicator functionality with a dedicated loop. - Updated message handling to ensure proper async flow and task management. - Cleaned up code for readability and maintainability.
Details: due to previous commits (which handled async flow), the previous AnsariClaude adjustments are now redundant (at least based on multiple tests before this commit)
@@ -0,0 +1,66 @@ | |||
(manually inserted log) ------------------------------------------------------------------------------ FastAPI receives a message from a whatsapp user on /whatsapp/v1 endpoint, so the event loop picks the `main_webhook()` to be executed now ------------------------------------------------------------------------------ |
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 file should not be checked in (I don't think)
* Adding text delta: 'START OF LLM RESPONSE (PARAPHRASED FROM TOOL RESULT)' (truncated) | ||
|
||
So, "TOOL RESULT:" (at the top) will be directly concatenated to "START OF ...". | ||
But we want to leave `\n\n` between them, so that's what this function does. |
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 am still a bit fuzzy. What problem is this trying to solve?
Main description mentioned here: 670d928
(Note: Directly after merging this PR, the WHATSAPP_API_VERSION env. var should be set to
V22.0
instead ofV21.0
).