genx/transformers: separate StreamKit and provider packages - #370
Conversation
Move AST, Doubao realtime and Duplex, and DashScope realtime provider runtimes and tests into typed packages.\n\nCut model loader, peer GenX, E2E callers, and docs over to the package constructors while preserving per-call sessions and stream lifecycle behavior.\n\nFixes #362
|
@codex review |
Remove checksums that are no longer reachable after moving realtime provider dependencies out of the root transformers package.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2dd78c7db5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Document the package-specific Config and New entry points required by issue 362 instead of promising deleted flat constructors.
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The agentkit name mixed provider-neutral Stream lifecycle with agent/tool ownership, while the remaining ASR and TTS adapters still lived in the flat transformers package. - Move buffering, response lifecycle, interruption, and shared TTS processing into internal/streamkit - Give Doubao ASR/TTS and MiniMax TTS package-owned typed constructors with per-call state - Cut repository consumers over without compatibility wrappers and move audio normalization to codecconv - Add race coverage, direct provider E2E coverage, and final bilingual package documentation
Remove stale provider and audio transitive checksums after the transformer package split changed the example module dependency graph.
There was a problem hiding this comment.
💡 Codex Review
When callers construct Doubao Realtime Duplex sessions that need provider tools, this new public Config has no way to pass []doubaospeech.RealtimeDuplexFunctionTool into the transformer, even though realtimeConfig still sends t.tools to Session.Tools and the pre-split API exposed WithDoubaoRealtimeDuplexTools. Those sessions now always start with an empty tool list, so function-call/toolkit flows cannot be advertised to the provider; add a Tools field and wire it through withTools in New.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Expose provider-native function declarations through the typed Realtime Duplex Config, wire them into each provider session, and deep-copy nested schema data at construction time.
Ignore delayed terminal events from an already interrupted response when collecting the next Realtime Duplex round, while preserving errors for the active round.
|
@codex review |
Ignore only delayed errors from prior Stream IDs so normal assistant output remains observable while the next realtime round is collected.
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
gizclaw/pkgs/genx/transformers/dashscoperealtime/implementation.go
Lines 459 to 462 in e51d289
When DashScope emits both input transcription and model text for the same turn (the default ASR/text+audio configuration), this reuses the input streamID for assistant text. The user transcript path has already emitted a text/plain EOS for that same route, and the deleted ResponseStream wrapper used to rewrite model chunks onto a fresh response ID; without that, route-aware consumers can merge the transcript with the assistant response or see text data after EOS. Generate/use a response-local StreamID for model output before pushing these chunks.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Keep input transcription on the caller's input route while allocating a fresh StreamID for each bound model response. This prevents model text and audio from following a text/plain EOS emitted for the user transcript.
|
Fixed the current-head DashScope StreamID finding in
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47b06d0f6a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Keep MiniMax synthesis on a per-chunk WebSocket stream and independently bind DashScope transcription and response events to queued turns. This preserves first-audio latency while preventing ASR completion from consuming the next turn's StreamID.
|
Addressed both current-head P2 findings in
@codex review |
MiniMax transformer tests exercise the public WebSocket transport directly, so keep coder/websocket in the direct requirement block required by go mod tidy.
|
CI tidy failure fixed in |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Moved the MP3 stream filter to the final public
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The previous TTSAudioNormalizer ownership mixed Transformer stream processing with codec conversion, while an MP3-specific replacement would expose a current implementation detail as the public contract. - Add a MIME-dispatched audiostream.Normalizer that preserves codec and MIME - Pass through formats without special handling and keep MP3 ID3v2 filtering internal - Update TTS providers, OpenAI speech consumers, tests, and guides to use the generic API
a546d45 to
f8be694
Compare
|
Corrected the audio-stream public contract in
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8be694386
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
When AST emits both source subtitles and translated subtitles for one segment, assigning translation.streamID to the same id as source puts the user transcript and model translation on the same StreamID/text/plain channel; after the source side emits its text EOS, translation text can arrive on an already-ended channel. The previous doubaoast.New path wrapped the legacy adapter in agentkit.NewResponseStream, which rewrote RoleModel chunks onto a response-local ID, but this raw implementation no longer does that, so route-aware consumers can reject or mis-record the translated text. Use a fresh response StreamID for translation/model text, or restore equivalent response isolation.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
transformerspackage into package-owned typed constructorsagentkitpackage with provider-neutralinternal/streamkitlifecycle primitives and delete the unused response-wrapper layerTransformsession isolation and concurrent useaudio/codecconv, cut all repository consumers over directly, and retain no compatibility wrappersValidation
go test -race ./pkgs/genx/transformers/internal/streamkit/... ./pkgs/genx/transformers/doubaoasr/... ./pkgs/genx/transformers/doubaotts/... ./pkgs/genx/transformers/doubaoast/... ./pkgs/genx/transformers/doubaorealtime/... ./pkgs/genx/transformers/doubaorealtimeduplex/... ./pkgs/genx/transformers/dashscoperealtime/... ./pkgs/genx/transformers/minimaxtts/...go test -run '^$' -tags gizclaw_genx_e2e ./tests/genx-e2e/...go test -count=1 -v -tags gizclaw_genx_e2e ./tests/genx-e2e/transformer -run '^(TestDoubaoSAUCASR|TestDoubaoSeedV2TTS|TestMiniMaxTTS)$'(all three live provider cases passed without skip)go test -count=1 -v -tags gizclaw_genx_e2e tests/genx-e2e/transformer/doubao_realtime_duplex_test.go -run '^TestDoubaoRealtimeDuplexConversation$'(two live rounds passed with transcript, assistant text/audio, interruption, and session restart)go test ./...go vet ./pkgs/audio/codecconv/... ./pkgs/genx/modelloader/... ./pkgs/genx/transformers/... ./pkgs/gizclaw/services/ai/openaiapi/... ./pkgs/gizclaw/services/ai/peergenx/...npm --prefix guides ci && npm --prefix guides run build:sitego mod tidy -diffandGOFLAGS=-mod=readonly go test -count=1 ./...git diff --checkbash tests/gizclaw-e2e/run_tests.shran for 1,693 seconds: all live AST, Doubao Realtime, Flowcraft, push-to-talk, realtime, history, auto-split, and interrupt phases passed; the overall command remains nonzero only for pre-existing chat unit testTestVerifyAssistantAudioASRIgnoresFailedTailAfterExpectedTextIsCovered, reproduced unchanged onmainFixes #362
Closes #404