Extract recursive tool loop into TextGenerationLoop and convert OpenAI#652
Draft
vinitkadam03 wants to merge 3 commits into
Draft
Extract recursive tool loop into TextGenerationLoop and convert OpenAI#652vinitkadam03 wants to merge 3 commits into
vinitkadam03 wants to merge 3 commits into
Conversation
8629890 to
bc9e0c2
Compare
Lifts the multi-step tool loop out of gateway parsing/streaming traits and into a dedicated orchestrator (StepLoop). Gateways implementing the new SingleTurnTextGateway contract execute one provider turn at a time and return a SingleTurnResponse; the loop handles tool dispatch and conversation accumulation. TextGateway and the Manager dispatch path are untouched — SingleTurnTextGateway is a sibling contract, opted into per gateway, so subsequent gateways can migrate one PR at a time. OpenAI and (transitively, via shared traits) Azure OpenAI are converted as the reference implementation; every other gateway remains on the existing multi-step path. Refs laravel#347, alternative path to laravel#349.
bc9e0c2 to
06bf13c
Compare
Author
|
Hey @pushpak1300, following up on #349. Per your suggestion about cherry-picking rather than merging everything in one go, I took a stab at a smaller foundation-only path: this PR ships StepLoop + the new contract with just OpenAI (and Azure, since they share traits) converted. Every other gateway is untouched. Kept it in draft for now; no rush, just trying to help. PR2 for the Anthropic migration is ready to stack on this once the approach is confirmed. Honest open questions called out in the description (StreamEnd carrier, SingleTurnTextGateway as public contract, StepContext bag pattern). Happy to iterate on any of them. |
Member
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Edited by @pushpak1300
Currently every gateway reimplements the same recursive multi-step tool loop. If you want to add a new provider, you have to copy orchestration logic (tool dispatch, max-step enforcement, step aggregation, response assembly) in addition to the actual request and response mapping.
This PR introduces
TextGenerationLoopas a centralized orchestrator and converts OpenAI and Azure OpenAI to single-turn gateways. Gateways now only build one request, parse one response, and yield one stream turn. The loop handles everything else.Before, OpenAI's
ParsesTextResponsestrait contained aprocessResponsemethod that recursively called itself after executing tools, building the finalTextResponseinline. The same pattern was duplicated across providers.After this PR, the gateway looks like this:
TextGenerationLoopowns tool execution, max steps, step accumulation, and final response assembly.SingleTurnTextGatewayis an opt-in sibling contract to the existingTextGateway, so unconverted providers keep working while we migrate one at a time.StreamEndnow carries an optionalresponseIdso the loop can chain OpenAI'sprevious_response_idacross turns. It works, but it couples a generic event to a provider-specific detail. WDYT about moving this to a side channel instead?The
buildTextGenerationLoop()helper is duplicated between OpenAI and Azure. It should probably become a shared trait once one more gateway is converted.