refactor: eliminate repetitive Provider match dispatch with with_adapter! macro#3979
refactor: eliminate repetitive Provider match dispatch with with_adapter! macro#3979devin-ai-integration[bot] wants to merge 1 commit intomainfrom
with_adapter! macro#3979Conversation
…ter macro Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
✅ Deploy Preview for hyprnote-storybook canceled.
|
✅ Deploy Preview for hyprnote canceled.
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| move |raw: &str| { | ||
| let responses: Vec<owhisper_interface::stream::StreamResponse> = match provider { | ||
| Provider::Deepgram => DeepgramAdapter.parse_response(raw), | ||
| Provider::AssemblyAI => AssemblyAIAdapter.parse_response(raw), | ||
| Provider::Soniox => SonioxAdapter.parse_response(raw), | ||
| Provider::Fireworks => FireworksAdapter.parse_response(raw), | ||
| Provider::OpenAI => OpenAIAdapter.parse_response(raw), | ||
| Provider::Gladia => GladiaAdapter.parse_response(raw), | ||
| Provider::ElevenLabs => ElevenLabsAdapter.parse_response(raw), | ||
| Provider::DashScope => DashScopeAdapter.parse_response(raw), | ||
| Provider::Mistral => mistral_adapter.parse_response(raw), | ||
| }; | ||
| let responses: Vec<owhisper_interface::stream::StreamResponse> = | ||
| with_adapter!(provider, |a| a.parse_response(raw)); |
There was a problem hiding this comment.
🔴 MistralAdapter's stateful word_counter is reset on every response_transformer invocation
The build_response_transformer closure now creates a fresh MistralAdapter::default() on every invocation via the with_adapter! macro, instead of reusing a single instance across all calls.
Root Cause and Impact
MistralAdapter has a word_counter: Arc<AtomicU64> field that tracks word indices across streaming transcription responses. In build_delta_response (crates/owhisper-client/src/adapter/mistral/live.rs:241), each word gets a monotonically increasing index via self.word_counter.fetch_add(1, Ordering::Relaxed), and this index is used to compute the word's start and end timestamps.
The old code created the adapter once outside the closure and captured it:
let mistral_adapter = MistralAdapter::default();
move |raw: &str| {
// ...
Provider::Mistral => mistral_adapter.parse_response(raw),
}The new code creates a fresh adapter on every closure call:
move |raw: &str| {
with_adapter!(provider, |a| a.parse_response(raw));
// For Mistral, this expands to:
// let a = MistralAdapter::default(); // word_counter = 0 every time!
// a.parse_response(raw)
}This means for Mistral streaming sessions, every TranscriptionTextDelta response will have words starting at index 0, producing duplicate/overlapping word timestamps (e.g., every delta's first word gets start=0.0, end=1.0). The SessionCreated reset at live.rs:111 also becomes meaningless since the counter is always 0 anyway.
Impact: Mistral streaming transcription word timestamps will be incorrect — all words across different delta events will have overlapping start/end values instead of monotonically increasing ones.
Prompt for agents
In build_response_transformer (hyprnote.rs lines 111-127), the MistralAdapter must be created once and reused across all closure invocations to preserve its word_counter state. The with_adapter! macro cannot be used here for the Mistral case because it creates a new adapter instance each time.
Fix by restoring the pre-created MistralAdapter before the closure, and either:
1. Revert build_response_transformer to use a manual match (the old code), or
2. Handle Mistral as a special case outside the macro, e.g.:
let mistral_adapter = MistralAdapter::default();
move |raw: &str| {
let responses: Vec<...> = match provider {
Provider::Mistral => mistral_adapter.parse_response(raw),
_ => with_adapter!(provider, |a| a.parse_response(raw)),
};
// ... rest of function
}
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Closing: the macro adds cognitive overhead for low value — the 3 functions are private, used once each, and the match arms are already concise. Also introduces a subtle semantic change with MistralAdapter::default() being called per-invocation in build_response_transformer instead of once. |
1 participant
Summary
Introduces a
with_adapter!macro inhyprnote.rsthat centralizes theProvider→ adapter dispatch into a single match block. The three functions that previously each had their own 9-arm match (build_upstream_url_with_adapter,build_initial_message_with_adapter,build_response_transformer) now each delegate to the macro in a single line.No functional changes intended. Test helpers module and function signatures are unchanged.
Review & Testing Checklist for Human
MistralAdapter::default()per-call is safe: The oldbuild_response_transformerpre-createdMistralAdapter::default()once outside the closure and captured it. The new code creates it on every closure invocation via the macro. Check thatMistralAdapter'sDefaultimpl is trivial/stateless so this is semantically equivalent.Provider(Deepgram, AssemblyAI, Soniox, Fireworks, OpenAI, Gladia, ElevenLabs, DashScope, Mistral) — a missing arm would be a compile error, but worth a glance.cargo test -p transcribe-proxyto confirm tests pass (passed locally).Notes