-
Notifications
You must be signed in to change notification settings - Fork 3
Fix outcome detection #97
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
Conversation
Remove dependency.
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.
Pull Request Overview
This PR fixes outcome detection functionality after changes to the converter handling and removes a dependency on session names. The changes update method calls to use the new converter API and adjust function signatures to support improved outcome detection.
- Updates converter method call from
structuretoloadsfor message parsing - Adds instance parameter to outcome detection functions for better duplicate filtering
- Removes session name dependency from callback initialization
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| agentune_simulate/tests/simulate/rag/test_indexing_retrieval.py | Adds required instance parameter to test function call |
| agentune_simulate/streamlit/pages/Agentune_Simulate_Runner.py | Removes session_name parameter from callback initialization |
| agentune_simulate/agentune/simulate/rag/indexing_and_retrieval.py | Adds instance parameter and duplicate filtering logic |
| agentune_simulate/agentune/simulate/outcome_detection/rag/rag.py | Updates function call to pass instance parameter |
| agentune_simulate/agentune/simulate/outcome_detection/rag/prompt.py | Updates converter method call from structure to loads |
| conversation=query_conversation, | ||
| k=k | ||
| k=k, | ||
| instance=OutcomeDetectionTest(query_conversation, intent=Intent(role=ParticipantRole.CUSTOMER, description="No description")) # No specific intent for this test, |
Copilot
AI
Jul 23, 2025
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 comment has a trailing comma and inconsistent spacing. It should end with a period instead of a comma.
| instance=OutcomeDetectionTest(query_conversation, intent=Intent(role=ParticipantRole.CUSTOMER, description="No description")) # No specific intent for this test, | |
| instance=OutcomeDetectionTest(query_conversation, intent=Intent(role=ParticipantRole.CUSTOMER, description="No description")) # No specific intent for this test. |
| # If there is an exact match between the current conversation and one of the examples, remove it | ||
| retrieved_docs = [ | ||
| (doc, score) for doc, score in retrieved_docs | ||
| if doc.metadata.get('conversation_hash') != hash(instance.conversation.messages) |
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 already pass conversation here, there is no need to pass OutcomeDetectionTest instance, also, it creates a circular dependency on outcome detection.
remove it, and just use the conversation to calculate the hash
|
|
||
| # Get logging callback tracer | ||
| callbacks = get_llm_callbacks(config['session_name']) | ||
| callbacks = get_llm_callbacks() |
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.
No need to remove this
| conversation=query_conversation, | ||
| k=k | ||
| k=k, | ||
| instance=OutcomeDetectionTest(query_conversation, intent=Intent(role=ParticipantRole.CUSTOMER, description="No description")) # No specific intent for this test, |
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.
no need for changes, other than check then maybe check that all conversations are different from the original conversation
| vector_store: The vector store to search for similar conversations | ||
| conversation: Current conversation to find examples for | ||
| k: Number of similar conversations to retrieve | ||
| instance: Optional instance to exclude exact matches (if provided) |
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.
not needed
Fix outcome detection after converter change.
Remove dependency.
What does this PR do?
Adjust the outcome detection to new converter handling.
Changes
Related Issues
Fixes #<issue_number>