Skip to content

Refactor with libs#40

Open
jackbridger wants to merge 2 commits intomainfrom
refactor-with-libs
Open

Refactor with libs#40
jackbridger wants to merge 2 commits intomainfrom
refactor-with-libs

Conversation

@jackbridger
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ 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".

Comment thread src/index.ts Outdated
Comment on lines +1120 to +1124
ws.addEventListener('close', () => {
log('WebSocket connection closed');
this.ws = null;
this._performDisconnectCleanup().catch((error) => {
console.error('Error during disconnect cleanup:', error);
logError('Error during disconnect cleanup %O', error);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Don’t nullify websocket when using ReconnectingWebSocket

The new connect() path now instantiates a ReconnectingWebSocket, which automatically reopens the same instance after any transient network drop. However the close handler at these lines still treats the socket like a one-shot WebSocket: it unconditionally sets this.ws = null and calls _performDisconnectCleanup(). When the library reconnects, _wsSend (lines 887‑893) continues to see this.ws as null, so no further audio/data frames are ever transmitted, and the cleanup has already torn down the recorder/VAD so there is nothing left to send anyway. The end result is that the client permanently stops streaming after the first automatic reconnect, defeating the purpose of switching to ReconnectingWebSocket. The close handler should either avoid clearing/cleaning up for reconnectable sockets or reinitialize state and reassign this.ws when the socket reopens.

Useful? React with 👍 / 👎.

…d enhance turn tracking

This commit consolidates all post-v2.8.0 changes into a single refactor that
modernises the audio client implementation, improves device handling, and
adds stronger turn and reconnect logic.

Key improvements:

### Audio & Encoding
- Swap manual base64 conversion for  for faster, safer binary encoding.
- Replace array-based VAD pre-buffer with  for predictable FIFO behaviour.
- Standardise audio pipeline to make amplitude monitoring and streaming more reliable.

### Device Management
- Introduce throttling for device list updates and amplitude events.
- Add robust device switching flow using  to serialize recorder restarts.
- Improve default-device detection and fallback logic.
- Ensure VAD and recorder reinitialize correctly on device changes.

### WebSocket & Turn Handling
- Integrate  to stabilise long-running sessions.
- Add proper reconnect logic (preserve conversation ID, clean resets).
- Track  directly from  events and unify handling.
- Add fallback turn_id logic for audio deltas to prevent orphaned audio buffers.

### Validation & Config
- Add Zod schemas to validate server messages and VAD config.
- Introduce  for safe, deep option merging with defaults.

### Misc
- Adopt debug logging via  package.
- Add  and clean up config organisation.
- General code cleanup, consistency improvements, and safety checks.

Overall this refactor significantly improves reliability, reduces edge-case
failures, and modernises internal structure to better support future features.
@jackbridger jackbridger force-pushed the main branch 2 times, most recently from 6a98e16 to 5571970 Compare January 5, 2026 15:59
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