-
Notifications
You must be signed in to change notification settings - Fork 310
Introduce a transforming interceptor to EventHandler #1424
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: develop
Are you sure you want to change the base?
Conversation
edbc6ed to
50e06a5
Compare
| logger.debug { "Executing LLM call (event id: $eventId, prompt: $prompt, tools: [${tools.joinToString { it.name }}])" } | ||
| context.pipeline.onLLMCallStarting(eventId, context.executionInfo, context.runId, prompt, model, tools, context) | ||
| logger.debug { "Transforming prompt (event id: $eventId, prompt: $prompt, tools: [${tools.joinToString { it.name }}])" } | ||
| val transformedPrompt = context.pipeline.onLLMPromptTransforming( |
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.
TODO: other methods here like "executeStreaming" or "moderate" should also call "onLLMPromptTransforming".
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.
@serge-p7v, could you please clarify the general question here. With this update, we have two events with same set of parameters one after another. It looks a bit redundant for me, if I do not miss anything, as you can transform prompt inside the onLLMCallStarting handler and prompt is a variable in the AIAgentLLMWriteSession that can be updated. Would that work as well? Why do we need a separate interceptor here?
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.
Currently, it is not possible to change the prompt inside of onLLMCallStarting (the prompt is immutable); for the intercepting approach we need to modify onLLMCallStarting. There might be two issues here:
- Prompt transformations through AIAgentLLMWriteSession would change the conversation state. In the PR they are applied to a current call to an LLM (they are kind of "transient").
onLLMCallStartingwould become a possibly mutating handler and it would be harder to reason about what it can do in a chain. In the PRonLLMPromptTransformingdoes only prompt transformation, so by chaining multipleonLLMCallStartingwith multipleonLLMPromptTransformingwe can be sure where the transformation is.
About the same set of parameters: good idea, thank you, most likely the transformer needs only the prompt and the context!
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.
@sdubov has suggested a great idea with extracting memory-specific handlers into a different feature. Converting the PR to draft.
…Handler feature that can modify the prompt before sending it to an LLM
50e06a5 to
ae57fe5
Compare
…sformingContext because it's likely not needed for prompt transformations
Add onLLMPromptTransforming handler that can transform Prompt before passing it to onLLMCallStarting.
Motivation and Context
For some use cases like RAG or chat memory it might be useful to intercept every request to an LLM in order to augment it somehow (with relevant documents from a knowledge base for RAG or with previous messages for chat memory).
Breaking Changes
None.
Type of the changes
Checklist
developas the base branchAdditional steps for pull requests adding a new feature