- 
                Notifications
    You must be signed in to change notification settings 
- Fork 1.2k
chore: refactor create_and_execute_turn and resume_turn #1399
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
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.
@ehhuang  I wonder when we should run the test with the generated mocks? It seems like server implementation changes will require re-running --record_response.
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.
ideally not? unless the key or response structure changed? did the requests going into the inference layer change?
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 don't expect that this change requires rerecord. Might be a bug if so, either in the mock util or code. Which key was missing?
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.
FAILED tests/integration/agents/test_agents.py::test_rag_agent_with_attachments[txt=meta-llama/Llama-3.3-70B-Instruct-toolgroup0] - KeyError: 'No cached response found for key: (\'meta-llama/Llama-3.3-70B-Instruct\', [SystemMessage(role=\'system\', content=\'You are ...
FAILED tests/integration/agents/test_agents.py::test_rag_agent_with_attachments[txt=meta-llama/Llama-3.3-70B-Instruct-builtin::rag/knowledge_search] - KeyError: 'No cached response found for key: (\'meta-llama/Llama-3.3-70B-Instruct\', [SystemMessage(role=\'system\', content=\'You are ...
Oh I think there's some key missing, going to commit this new generated json.
| if event.payload.event_type == AgentTurnResponseEventType.step_complete.value: | ||
| steps.append(event.payload.step_details) | ||
|  | ||
| async for chunk in self._run_turn(request, turn_id): | 
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.
love it
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.
beauty
| if event.payload.event_type == AgentTurnResponseEventType.step_complete.value: | ||
| steps.append(event.payload.step_details) | ||
|  | ||
| async for chunk in self._run_turn(request, turn_id): | 
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.
beauty
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 don't expect that this change requires rerecord. Might be a bug if so, either in the mock util or code. Which key was missing?
What does this PR do?
create_turnandresume_turn#1212Test Plan