Skip to content

Extract recursive tool loop into TextGenerationLoop and convert OpenAI#652

Draft
vinitkadam03 wants to merge 3 commits into
laravel:0.xfrom
vinitkadam03:split/foundation-openai
Draft

Extract recursive tool loop into TextGenerationLoop and convert OpenAI#652
vinitkadam03 wants to merge 3 commits into
laravel:0.xfrom
vinitkadam03:split/foundation-openai

Conversation

@vinitkadam03
Copy link
Copy Markdown

@vinitkadam03 vinitkadam03 commented May 23, 2026

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 TextGenerationLoop as 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 ParsesTextResponses trait contained a processResponse method that recursively called itself after executing tools, building the final TextResponse inline. The same pattern was duplicated across providers.

After this PR, the gateway looks like this:

// Gateway implements SingleTurnTextGateway
public function generateSingleTurn(...): SingleTurnResponse
{
    $body = $this->buildSingleTurnBody(...);
    $response = $this->client->post('responses', $body);
    return $this->parseTextResponse($response->json(), $provider, filled($schema));
}

// generateText delegates to the loop
public function generateText(...): TextResponse
{
    return $this->buildTextGenerationLoop()->generate(...);
}

TextGenerationLoop owns tool execution, max steps, step accumulation, and final response assembly. SingleTurnTextGateway is an opt-in sibling contract to the existing TextGateway, so unconverted providers keep working while we migrate one at a time.

StreamEnd now carries an optional responseId so the loop can chain OpenAI's previous_response_id across 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.

@vinitkadam03 vinitkadam03 marked this pull request as draft May 23, 2026 11:29
@vinitkadam03 vinitkadam03 force-pushed the split/foundation-openai branch from 8629890 to bc9e0c2 Compare May 23, 2026 11:29
@vinitkadam03 vinitkadam03 changed the title Extract tool loop into StepLoop; OpenAI as reference single-turn gateway Extract tool loop into StepLoop; Migrate to single turn responses one by one, starting with OpenAI as reference single-turn gateway May 23, 2026
@vinitkadam03 vinitkadam03 changed the title Extract tool loop into StepLoop; Migrate to single turn responses one by one, starting with OpenAI as reference single-turn gateway Extract tool loop into StepLoop; Migrate to single turn gateways one by one, starting with OpenAI as reference single-turn gateway May 23, 2026
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.
@vinitkadam03 vinitkadam03 force-pushed the split/foundation-openai branch from bc9e0c2 to 06bf13c Compare May 23, 2026 19:11
@vinitkadam03
Copy link
Copy Markdown
Author

vinitkadam03 commented May 23, 2026

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.

@pushpak1300 pushpak1300 self-requested a review May 23, 2026 20:14
@pushpak1300
Copy link
Copy Markdown
Member

pushpak1300 commented May 26, 2026

  • Renamed StepLoop to TextGenerationLoop so the name is clearer.
  • generateText() / streamText() still exist and now delegate through the loop.
  • Fixed the streaming behavior so intermediate provider-turn StreamEnd events are not exposed before the tool loop finishes.
  • Fixed max-step behavior so tools are not executed on the final allowed step when their results cannot be sent back to the provider.
  • Added focused tests for both regressions.

@pushpak1300 pushpak1300 changed the title Extract tool loop into StepLoop; Migrate to single turn gateways one by one, starting with OpenAI as reference single-turn gateway Extract recursive tool loop into TextGenerationLoop and convert OpenAI May 26, 2026
@pushpak1300 pushpak1300 marked this pull request as ready for review May 27, 2026 14:54
@pushpak1300 pushpak1300 marked this pull request as draft May 27, 2026 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants