Skip to content

refactor(providers,executor): deepen three shallow modules#5052

Merged
waleedlatif1 merged 5 commits into
stagingfrom
refactor/architecture-deepening
Jun 15, 2026
Merged

refactor(providers,executor): deepen three shallow modules#5052
waleedlatif1 merged 5 commits into
stagingfrom
refactor/architecture-deepening

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • executor: collapse ~20 scattered subflow node-ID parsing/building helpers and their regex patterns behind a single SubflowNodeIdCodec; subflow-utils and execution state delegate to it.
  • providers: extract the near-identical StreamingExecution assembly (timing segments, cost, tokens, onComplete wiring, envelope) copy-pasted across ~16 providers into one createStreamingExecution factory; providers inject only their stream iterable + delta extractors. Gemini left as-is (structurally divergent).
  • providers: dedupe the identical inline tool-schema literals across ~16 providers into shared adaptOpenAIChatToolSchema / adaptAnthropicToolSchema helpers; content building reverts to calling the attachments builders directly (the per-provider content seam was pure pass-through indirection).
  • One commit per refactor. No dead code left behind.

Type of Change

  • Refactor (no functional change)

Testing

  • New unit tests: subflow codec, streaming-execution factory, tool-schema adapters (all pass).
  • Full provider + executor suites pass (987 tests); typecheck clean; check:api-validation passes.
  • Behavior audited line-by-line by independent reviewers against HEAD — byte-identical output, ordering, cost/token/timing, and error modes. Zero behavior change.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Extract the ~20 scattered subflow node-ID parsing/building helpers and
their regex patterns into a single SubflowNodeIdCodec. All patterns now
live in one place; subflow-utils and execution state delegate to it.
Pure refactor — byte-identical output, ordering, and error modes.
The near-identical streaming-response assembly (timing segments, cost,
token counts, onComplete wiring, success/logs/metadata envelope) that was
copy-pasted across ~16 providers now goes through createStreamingExecution.
Each provider injects only its stream iterable and delta extractors. Gemini
is intentionally left as-is (its assembly is structurally divergent).
Pure refactor — identical StreamingExecution shape, cost, tokens, timing,
and callback order per provider.
Replace the identical inline tool-schema literals repeated across ~16
providers with shared adaptOpenAIChatToolSchema / adaptAnthropicToolSchema
helpers. Content building reverts to calling the attachments builders
directly (the per-provider content seam was pure pass-through indirection).
Pure refactor — byte-identical tool-schema and content output.
@vercel

vercel Bot commented Jun 15, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jun 15, 2026 6:29am

Request Review

@cursor

cursor Bot commented Jun 15, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Wide refactor across executor subflow ID resolution and provider streaming/tool wiring; regressions would affect workflow execution scoping and LLM responses, though the PR positions this as behavior-preserving with broad test coverage.

Overview
This PR centralizes three areas that were duplicated across the executor and many LLM providers, without changing intended behavior.

Executor: Introduces SubflowNodeIdCodec as the single place for branch subscripts, loop/parallel sentinels, outer-branch clones, and lookup normalization. subflow-utils and execution state stop defining inline regexes and delegate to the codec; adds focused unit tests.

Providers: Adds createStreamingExecution so simple and post-tool streaming paths share one timing/cost/token/envelope builder instead of large copy-pasted blocks in ~16 providers (Gemini unchanged). Adds adaptOpenAIChatToolSchema and adaptAnthropicToolSchema and replaces repeated inline tool literals in those providers.

Docs: The add-model skill now points contributors at these helpers when extending provider code.

New unit tests cover the codec, streaming factory, and tool adapters.

Reviewed by Cursor Bugbot for commit 7454b84. Configure here.

@greptile-apps

greptile-apps Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR deepens three shallow modules through pure refactoring: subflow node-ID parsing is consolidated behind a single SubflowNodeIdCodec object; the near-identical StreamingExecution assembly block copy-pasted across ~16 providers is extracted into a createStreamingExecution factory; and the duplicated inline tool-schema literals are replaced with shared adaptOpenAIChatToolSchema / adaptAnthropicToolSchema helpers.

  • SubflowNodeIdCodec (executor/utils/subflow-node-id-codec.ts): every regex and string template for branch subscripts, sentinel IDs, outer-branch clone scoping, and loop digests is now owned by a single object; subflow-utils.ts and state.ts delegate to it via thin forwarding calls with no API changes.
  • createStreamingExecution (providers/streaming-execution.ts): centralises timing placeholder construction, finalizeTiming hook wiring, and the success/logs/metadata envelope; each provider injects only its native stream factory and calls finalizeTiming() (or not) to match its original drain-callback behaviour.
  • Tool-schema adapters (providers/tool-schema-adapter.ts): deduplicates the two literal shapes (OpenAI Chat Completions function-wrapped, Anthropic input_schema) with typed helpers; SKILL.md is updated to point future contributors to these helpers.

Confidence Score: 5/5

Safe to merge — purely structural consolidation with no behavioural changes across the executor and 16 providers.

Every regex, string template, and stream-assembly block is moved verbatim into the new shared modules. The finalizeTiming call pattern (called or omitted) is preserved per-provider to match each provider's original drain-callback behaviour. The full test suite (987 tests) passes, typecheck is clean, and three new dedicated unit-test files cover the codec, factory, and adapters.

No files require special attention.

Important Files Changed

Filename Overview
apps/sim/executor/utils/subflow-node-id-codec.ts New module centralising all regex patterns and ID-building/parsing helpers for subflow node IDs; logic is moved verbatim from subflow-utils.ts with no behavioural changes.
apps/sim/executor/utils/subflow-utils.ts All regex declarations and inline implementations replaced with thin forwarding calls to SubflowNodeIdCodec; public API is unchanged.
apps/sim/providers/streaming-execution.ts New factory centralising StreamingExecution assembly; timing finalization (providerTiming endTime/duration + segment) is correctly gated on kind='simple' vs 'accumulated'.
apps/sim/providers/tool-schema-adapter.ts New helpers deduplicating inline tool-schema literals for OpenAI-compatible and Anthropic providers; TypeScript return-type annotations enforce the required literal types.
apps/sim/providers/anthropic/core.ts All three streaming paths (simple, accumulated post-tool, accumulated structured-output) correctly call finalizeTiming(), matching the original drain-callback timing updates.
apps/sim/providers/openai/core.ts Simple path calls finalizeTiming(); accumulated post-tool path intentionally omits it—matching the original code which also had no drain-callback timing update on that path.
apps/sim/providers/groq/index.ts Neither the simple nor the accumulated path calls finalizeTiming()—preserving the original behavior where groq's drain callback never updated providerTiming.
apps/sim/executor/execution/state.ts Two local regex patterns and three inline helpers replaced with SubflowNodeIdCodec calls; the two now-unused BRANCH_SUFFIX_PATTERN / LOOP_SUFFIX_PATTERN constants are correctly removed.
apps/sim/providers/streaming-execution.test.ts New test suite covering simple and accumulated timing paths, finalizeTiming mutation, isStreaming flag, and tool-call passthrough via a synchronous fake-stream helper.
apps/sim/executor/utils/subflow-node-id-codec.test.ts New test suite with round-trip and predicate coverage for every codec operation including nested outer-branch cases and findEffectiveContainerId.

Sequence Diagram

sequenceDiagram
    participant P as Provider (e.g. anthropic/core.ts)
    participant F as createStreamingExecution()
    participant O as NormalizedBlockOutput
    participant S as ReadableStream (native)

    P->>F: "createStreamingExecution({ model, timing, initialTokens, initialCost, createStream })"
    F->>O: construct output object (placeholder timing)
    F->>P: "invoke createStream({ output, finalizeTiming })"
    P->>S: "wire native stream factory (createReadableStreamFrom*)"
    S-->>F: ReadableStream returned
    F-->>P: "return { stream, execution }"

    note over S: stream drains asynchronously
    S->>O: write content / tokens / cost
    S->>F: call finalizeTiming()
    F->>O: "overwrite providerTiming.endTime and duration (+ segment if kind='simple')"
Loading

Reviews (2): Last reviewed commit: "refactor(providers): drop placeholder st..." | Re-trigger Greptile

Comment thread apps/sim/providers/streaming-execution.ts Outdated
@waleedlatif1 waleedlatif1 changed the title refactor(providers,executor): deepen three shallow modules (no behavior change) refactor(providers,executor): deepen three shallow modules Jun 15, 2026
…-schema helpers

When editing provider code, reuse createStreamingExecution and the
tool-schema-adapter helpers instead of hand-rolling.
… factory

Build the output object before creating the stream so the factory returns
a fully-typed StreamingExecution without 'undefined as unknown as
ReadableStream'. Keeps the strict API-validation boundary ratchet green
(double-cast count back to baseline). No behavior change: the same output
reference is mutated on drain.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 7454b84. Configure here.

@waleedlatif1 waleedlatif1 merged commit 06191a7 into staging Jun 15, 2026
15 checks passed
@waleedlatif1 waleedlatif1 deleted the refactor/architecture-deepening branch June 15, 2026 08:03
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.

1 participant