Skip to content

refactor(voice-engine): extract Gemini Live event loop into native_se…#54

Merged
jjleng merged 1 commit intomainfrom
refactor-gemini-live
Apr 16, 2026
Merged

refactor(voice-engine): extract Gemini Live event loop into native_se…#54
jjleng merged 1 commit intomainfrom
refactor-gemini-live

Conversation

@jjleng
Copy link
Copy Markdown
Contributor

@jjleng jjleng commented Apr 16, 2026

…ssion module

@jjleng
Copy link
Copy Markdown
Contributor Author

jjleng commented Apr 16, 2026

@claude

@claude
Copy link
Copy Markdown

claude bot commented Apr 16, 2026

Claude finished @jjleng's task in 1m 4s —— View job


Code Review

  • Read CLAUDE.md for repo guidelines
  • Read changed files and diff
  • Analyze refactoring and provide review feedback

The refactoring is clean and well-scoped — session.rs is now a proper thin coordinator as its doc comment says, and native_session.rs is a self-contained module with good doc comments (including the ASCII audio-path diagram). No regressions spotted. A few items worth addressing:

Minor Issues

Inconsistent tracing usage (native_session.rs:63)

The file imports error from tracing at the top:

use tracing::{error, info, warn};

But line 63 uses the fully qualified form instead of the imported one:

tracing::error!("[native] No Gemini API key ...");

Lines 102 and 481 correctly use the bare error!. This should be made consistent. Fix this →


Inconsistent use of crate:: inline paths vs top-level imports (native_session.rs)

Types from agent_kit, voice_trace, etc. are all imported at the top, but several internal types use inline crate:: paths inside the function body:

  • native_session.rs:79crate::utils::PlaybackTracker
  • native_session.rs:114crate::utils::SAMPLE_RATE
  • native_session.rs:118soxr::SoxrStreamResampler (used inline, not imported)
  • native_session.rs:123crate::reactor::proc::vad::VadStage
  • native_session.rs:124crate::audio_ml::vad::VadConfig
  • native_session.rs:132crate::utils::AudioRingBuffer
  • native_session.rs:185–186crate::audio_ml::vad::VAD_THRESHOLD_PLAYBACK_RAW / VAD_THRESHOLD_IDLE

These would read more clearly as top-level imports matching the rest of the file's style. Fix this →


#[allow(clippy::too_many_arguments)] suppression (native_session.rs:41)

The function currently takes 13 arguments. This attribute suppresses the lint rather than addressing it. Since this is a refactor, it's a good opportunity to bundle the arguments into a struct (e.g., NativeSessionArgs or reusing an extended NativeMultimodalConfig). That said, this is pre-existing behavior, so it's fine to defer if this is out of scope for this PR.


Nit

native_session.rs:88–96 — The greeting injection happens inside run_native_multimodal after SessionConfig::finalize() has already prepended any language instruction to system_prompt. The resulting prompt order is [language] + [original prompt] + [greeting], which seems intentional, but a brief comment noting that finalize() runs before this would make the ordering explicit to future readers.


Overall: clean extraction, good module doc with audio-path diagram. The two style issues (tracing macro + inline crate paths) are the only things worth fixing before merge.

@jjleng jjleng force-pushed the refactor-gemini-live branch from d493ab5 to 7cda5a5 Compare April 16, 2026 20:45
@jjleng jjleng merged commit b5424c9 into main Apr 16, 2026
1 check passed
@jjleng jjleng deleted the refactor-gemini-live branch April 16, 2026 20:46
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