feat: Implement concurrency guards for GGUF engines, optimize context windows, and add spoken-word system prompts to prevent self-triggering VAD feedback. - #11
Conversation
… windows, and add spoken-word system prompts to prevent self-triggering VAD feedback.
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughLocal GGUF engines now serialize generation with an atomic lock to prevent concurrent runs. System prompts moved to a prompt store with character-specific TTS voice selection. Llama/Qwen default context lengths were increased. TTS playback readiness is now gated on actual audio buffer consumption. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant GGUFEngine
participant Lock as generationLock
participant TTS as TTS System
participant AudioBuffer
participant Delegate
participant VAD
Client->>GGUFEngine: Request generation
GGUFEngine->>Lock: Atomically check/set _isGenerating
alt already generating
GGUFEngine->>Delegate: localLLM(didFinishGeneration: "")
GGUFEngine-->>Client: Return early (dropped)
else proceed
GGUFEngine->>GGUFEngine: Generate tokens
GGUFEngine->>TTS: Send text for TTS
TTS->>TTS: Select voice, sanitize text
TTS->>AudioBuffer: Schedule PCM (increment pendingTTSBuffers)
AudioBuffer->>AudioBuffer: On play complete decrement pendingTTSBuffers
alt pendingTTSBuffers == 0 && generation finished
GGUFEngine->>VAD: Set status = .ready
end
GGUFEngine->>Lock: Reset _isGenerating
end
sequenceDiagram
participant User
participant VoiceHandler
participant RecordingLock
participant Transcriber
participant VAD
User->>VoiceHandler: Voice input starts
VoiceHandler->>VoiceHandler: Check state.status == .ready
alt ready
VoiceHandler->>RecordingLock: set isRecordingVoice = true
User-->>VoiceHandler: Audio captured
else not ready
VoiceHandler-->>User: Ignore input
end
User->>VoiceHandler: Voice input ends
VoiceHandler->>RecordingLock: capture wasTrulyRecording, clear buffer, stop recording
alt wasTrulyRecording
VoiceHandler->>Transcriber: Send audio for transcription
Transcriber-->>VAD: Transcript result
else no audio
VoiceHandler-->>VAD: Skip transcription
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 48 minutes and 12 seconds.Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@NeuraLink/AI/GGUF/GGUFQwenEngine.swift`:
- Around line 43-46: The current LlamaBridge initialization uses contextLength:
2048 which can cause OOM on 4GB devices; change the default to a lower safe
value (e.g., 512 or 1024) in the LlamaBridge(...) call (reference: LlamaBridge,
contextLength) and add a short comment documenting the tradeoff and device RAM
requirement, or add a runtime check that inspects available memory and
caps/adjusts contextLength before calling LlamaBridge(modelPath: url.path,
contextLength: ...).
In `@NeuraLink/AI/LocalLLMManager`+TTS.swift:
- Around line 32-43: The switch case for "sonya" in LocalLLMManager+TTS.swift
returns a prompt that names the persona "Dedicatus" which likely is a copy-paste
identity mismatch; either change the prompt string inside the case "sonya" from
"You are Dedicatus" to "You are Sonya" (or the correct persona name) or, if this
is intentional, add a clarifying comment above the case explaining that Sonya
intentionally uses the alternate persona "Dedicatus" so reviewers and future
maintainers understand the mismatch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a36e7724-2fa9-45df-9278-3f74dbd1451f
📒 Files selected for processing (9)
NeuraLink/AI/CharacterPersona.swiftNeuraLink/AI/GGUF/GGUFLlamaEngine+Generate.swiftNeuraLink/AI/GGUF/GGUFLlamaEngine.swiftNeuraLink/AI/GGUF/GGUFQwenEngine+Generate.swiftNeuraLink/AI/GGUF/GGUFQwenEngine.swiftNeuraLink/AI/GGUF/LlamaBridge.swiftNeuraLink/AI/LocalLLMManager+Delegates.swiftNeuraLink/AI/LocalLLMManager+TTS.swiftNeuraLink/AI/LocalLLMManager.swift
💤 Files with no reviewable changes (1)
- NeuraLink/AI/CharacterPersona.swift
…al session restart functionality
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
NeuraLink/AI/LocalLLMManager.swift (1)
460-465:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove the full-response log.
Printing every assistant response verbatim leaks conversation content into device logs. If you need this for debugging, gate it behind
#if DEBUGor redact it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@NeuraLink/AI/LocalLLMManager.swift` around lines 460 - 465, In localLLM(didFinishGeneration fullText: String) remove or stop the unconditional print of the full assistant response to avoid leaking conversation content; either delete the print("[LocalLLM] Full response: \(fullText)") call or wrap it behind a conditional compilation flag like `#if` DEBUG (or redact the text) so fullText is only logged in debug builds or when explicitly allowed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/npu.md`:
- Line 71: Update the broken relative markdown link that points to the TTS
extension: replace the current target "./NeuraLink/AI/LocalLLMManager+TTS.swift"
with a one-level-up relative path "../NeuraLink/AI/LocalLLMManager+TTS.swift" so
the reference to LocalLLMManager+TTS.swift resolves correctly from the docs
folder.
In `@NeuraLink/AI/LocalLLMManager.swift`:
- Around line 252-269: Introduce a session token to invalidate in-flight TTS
callbacks: add a property like ttsSessionToken (UUID) and assign a fresh UUID
inside restart() (alongside resetting pendingTTSBuffers/ttsGenerationDone); when
starting any TTS generation capture the current ttsSessionToken and have all
async paths (the .dataConsumed callbacks, the async increment path, and
didFinishGeneration) check that the captured token still equals the manager's
ttsSessionToken before mutating pendingTTSBuffers, ttsGenerationDone, or other
session state; additionally ensure those mutations run on the same serialized
execution context (MainActor or a dedicated serial queue) to avoid races.
In `@NeuraLink/UI/AI/PersonaSettingsView.swift`:
- Around line 108-130: The display name edits are not persisted in local LLM
mode; update the Save Changes and Reset to Default branches so local mode also
persists or restores the persona name (e.g., call a new or existing
LocalLLMPromptStore methods like saveName(for: modelID, name: persona.name) and
effectiveName(for: modelID) when isLocalLLMMode) OR make the name field
read-only when isLocalLLMMode by disabling the TextField bound to persona.name;
modify the Save Changes block (currently calling
LocalLLMPromptStore.shared.savePrompt(...)) and the Reset to Default block
(currently calling LocalLLMPromptStore.shared.resetPrompt(...) and reading
effectivePrompt) to handle persona.name consistently.
---
Outside diff comments:
In `@NeuraLink/AI/LocalLLMManager.swift`:
- Around line 460-465: In localLLM(didFinishGeneration fullText: String) remove
or stop the unconditional print of the full assistant response to avoid leaking
conversation content; either delete the print("[LocalLLM] Full response:
\(fullText)") call or wrap it behind a conditional compilation flag like `#if`
DEBUG (or redact the text) so fullText is only logged in debug builds or when
explicitly allowed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dffb1b00-eef2-451e-a0d4-dca441faab32
📒 Files selected for processing (8)
NeuraLink/AI/LocalLLMManager+TTS.swiftNeuraLink/AI/LocalLLMManager.swiftNeuraLink/AI/LocalLLMPromptStore.swiftNeuraLink/UI/AI/AISettingsView.swiftNeuraLink/UI/AI/PersonaSettingsView.swiftREADME.mddocs/npu.mddocs/npu_migration.md
💤 Files with no reviewable changes (1)
- docs/npu_migration.md
🚧 Files skipped from review as they are similar to previous changes (1)
- NeuraLink/AI/LocalLLMManager+TTS.swift
feat: Implement concurrency guards for GGUF engines, optimize context windows, and add spoken-word system prompts to prevent self-triggering VAD feedback.
Summary by CodeRabbit
Bug Fixes
New Features
Performance
Documentation