feat: Added voice feature for end-user fix #325 #405
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis PR introduces a voice input system for Cortex, enabling users to control the CLI via voice commands. It adds a new Changes
Sequence DiagramsequenceDiagram
participant User
participant CLI as CortexCLI
participant VIH as VoiceInputHandler
participant Audio as Audio System
participant Model as Whisper Model
participant Callback
User->>CLI: --mic flag or voice command
CLI->>VIH: Initialize + _ensure_dependencies()
VIH->>Audio: _check_microphone()
Audio-->>VIH: Device ready
rect rgba(100, 200, 150, 0.2)
Note over User,VIH: Voice Input Mode
User->>Audio: Speak (after hotkey)
Audio->>VIH: _start_recording() + audio chunks
User->>Audio: Release hotkey (stop)
VIH->>VIH: _stop_recording_stream()
end
rect rgba(100, 150, 200, 0.2)
Note over VIH,Model: Transcription
VIH->>VIH: _load_model() [lazy]
VIH->>Model: transcribe(audio_data)
Model-->>VIH: Segment text
VIH->>VIH: Concatenate segments
end
rect rgba(200, 150, 100, 0.2)
Note over VIH,Callback: Result Processing
VIH->>Callback: Invoke with transcribed text
Callback->>CLI: Return command/question
CLI->>User: Execute or ask further
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
CLA Verification PassedAll contributors have signed the CLA.
|
There was a problem hiding this comment.
Pull request overview
This PR adds voice input capability to Cortex Linux, enabling users to speak commands instead of typing them. The implementation uses local speech-to-text processing with faster-whisper for privacy and low latency. However, there are several critical issues that need to be addressed before merging.
Key Changes
- New voice input module with support for continuous and single-shot voice commands
- Optional
[voice]dependency group in pyproject.toml - Windows compatibility improvements in branding module (ASCII fallback for console output)
- Comprehensive documentation for voice features
- 20 unit tests for voice functionality
Reviewed changes
Copilot reviewed 12 out of 42 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_voice.py | Unit tests for voice input handler with mocked dependencies |
| requirements.txt | Issue: Voice dependencies incorrectly added as required instead of optional |
| pyproject.toml | Proper optional voice dependency group configuration |
| myenv/* | Critical: Entire virtual environment directory committed (should be excluded) |
| docs/VOICE_INPUT.md | Comprehensive user documentation for voice features |
| cortex/branding.py | Windows compatibility with ASCII fallback for special characters |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| handler = VoiceInputHandler() | ||
| assert handler.model_name == "tiny.en" | ||
| assert handler.sample_rate == 16000 | ||
| assert handler.hotkey == "f9" |
There was a problem hiding this comment.
The hotkey default is tested as "f9" in multiple tests (lines 45, 243), but the documentation claims the customizable hotkey is set via CORTEX_VOICE_HOTKEY environment variable which is never tested. The relationship between the default value and the environment variable override should be tested to ensure the configuration mechanism works correctly.
| # Import fresh to get mocked module | ||
| import importlib | ||
|
|
||
| import cortex.voice |
There was a problem hiding this comment.
Module 'cortex.voice' is imported with both 'import' and 'import from'.
| with patch("cortex.voice.cx_print") as mock_print: | ||
| import importlib | ||
|
|
||
| import cortex.voice |
There was a problem hiding this comment.
Module 'cortex.voice' is imported with both 'import' and 'import from'.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
requirements.txt (1)
9-25: Duplicate PyYAML dependencies with inconsistent versions.There are three occurrences of PyYAML in this file:
- Line 9:
PyYAML>=6.0.0- Line 21:
pyyaml>=6.0.0- Line 25:
PyYAML==6.0.3This creates confusion and potential version conflicts. Keep only one entry with a consistent version constraint.
🔎 Proposed fix
# Configuration PyYAML>=6.0.0 # Environment variable loading from .env files python-dotenv>=1.0.0 # Encryption for environment variable secrets cryptography>=42.0.0 # Terminal UI rich>=13.0.0 -# Configuration -pyyaml>=6.0.0 - # Type hints for older Python versions typing-extensions>=4.0.0 -PyYAML==6.0.3
♻️ Duplicate comments (4)
myenv/Scripts/deactivate.bat (1)
1-22: Part of virtual environment that should not be committed.This file is part of the
myenv/virtual environment directory that should be removed from version control. See the comment onmyenv/pyvenv.cfgfor details.myenv/share/man/man1/isympy.1 (1)
1-188: Third-party package artifact that should not be committed.This is a SymPy man page installed in the virtual environment. It's part of the
myenv/directory that should be removed from version control. See the comment onmyenv/pyvenv.cfgfor details.myenv/Scripts/activate.bat (1)
1-34: Part of virtual environment with hardcoded developer paths.This file contains a hardcoded path (
C:\Users\sahil\...) on line 11 and is part of themyenv/virtual environment that should be removed. See the comment onmyenv/pyvenv.cfgfor details.myenv/Scripts/Activate.ps1 (1)
1-528: Part of virtual environment that should not be committed.This PowerShell activation script is part of the
myenv/virtual environment directory that should be removed from version control. See the comment onmyenv/pyvenv.cfgfor details.
🧹 Nitpick comments (4)
docs/VOICE_INPUT.md (1)
66-83: Add language identifier to fenced code blocks for markdown lint compliance.Static analysis flagged missing language specifiers. For terminal output and diagrams, use
textorconsoleas the language identifier.🔎 Suggested fix
Line 66:
-``` +```text $ cortex voiceLine 146:
-``` +```text ┌──────────────┐ ┌──────────────┐cortex/voice.py (3)
296-307: Recording indicator bypasses branding utilities.The recording indicator at line 302 uses raw string formatting
" CX | "instead of thecx_printfunction fromcortex/branding.py. This creates inconsistency in terminal output, especially on Windows wherecx_printuses ASCII-only icons.🔎 Suggested approach
Consider using
console.printwith Rich markup for consistency, or document why direct stdout is necessary (e.g., for\rcarriage return updates).If carriage return updates are required, you could use Rich's
Livecontext orStatusfor animated updates instead of raw stdout manipulation.
425-428: Busy-wait loop could be replaced with event-based waiting.The infinite loop with
time.sleep(0.1)is a busy-wait pattern. While functional, it wastes CPU cycles polling. Consider using anEvent.wait()pattern that blocks until signaled.🔎 Alternative approach
+ self._exit_event = threading.Event() + try: # Keep the main thread alive - while True: - time.sleep(0.1) + self._exit_event.wait() except KeyboardInterrupt: cx_print("\nVoice mode exited.", "info")Then set
self._exit_event.set()in thestop()method.
482-487: Bare except clause silently swallows all exceptions.The bare
exceptat line 485 catches and ignores all exceptions, includingKeyboardInterruptandSystemExit. This could mask programming errors during debugging.🔎 Suggested fix
if self._hotkey_listener: try: self._hotkey_listener.stop() - except Exception: - pass + except OSError: + # Listener may already be stopped + pass self._hotkey_listener = NoneOr log the exception at debug level:
except Exception as e: logging.debug("Error stopping hotkey listener: %s", e)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (29)
myenv/Scripts/coloredlogs.exeis excluded by!**/*.exemyenv/Scripts/cortex.exeis excluded by!**/*.exemyenv/Scripts/ct2-fairseq-converter.exeis excluded by!**/*.exemyenv/Scripts/ct2-marian-converter.exeis excluded by!**/*.exemyenv/Scripts/ct2-openai-gpt2-converter.exeis excluded by!**/*.exemyenv/Scripts/ct2-opennmt-py-converter.exeis excluded by!**/*.exemyenv/Scripts/ct2-opennmt-tf-converter.exeis excluded by!**/*.exemyenv/Scripts/ct2-opus-mt-converter.exeis excluded by!**/*.exemyenv/Scripts/ct2-transformers-converter.exeis excluded by!**/*.exemyenv/Scripts/distro.exeis excluded by!**/*.exemyenv/Scripts/dotenv.exeis excluded by!**/*.exemyenv/Scripts/f2py.exeis excluded by!**/*.exemyenv/Scripts/hf.exeis excluded by!**/*.exemyenv/Scripts/httpx.exeis excluded by!**/*.exemyenv/Scripts/humanfriendly.exeis excluded by!**/*.exemyenv/Scripts/isympy.exeis excluded by!**/*.exemyenv/Scripts/markdown-it.exeis excluded by!**/*.exemyenv/Scripts/numpy-config.exeis excluded by!**/*.exemyenv/Scripts/onnxruntime_test.exeis excluded by!**/*.exemyenv/Scripts/openai.exeis excluded by!**/*.exemyenv/Scripts/pip.exeis excluded by!**/*.exemyenv/Scripts/pip3.12.exeis excluded by!**/*.exemyenv/Scripts/pip3.exeis excluded by!**/*.exemyenv/Scripts/pyav.exeis excluded by!**/*.exemyenv/Scripts/pygmentize.exeis excluded by!**/*.exemyenv/Scripts/python.exeis excluded by!**/*.exemyenv/Scripts/pythonw.exeis excluded by!**/*.exemyenv/Scripts/tiny-agents.exeis excluded by!**/*.exemyenv/Scripts/tqdm.exeis excluded by!**/*.exe
📒 Files selected for processing (13)
cortex/branding.pycortex/cli.pycortex/voice.pydocs/VOICE_INPUT.mdmyenv/Scripts/Activate.ps1myenv/Scripts/activatemyenv/Scripts/activate.batmyenv/Scripts/deactivate.batmyenv/pyvenv.cfgmyenv/share/man/man1/isympy.1pyproject.tomlrequirements.txttests/test_voice.py
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide
Type hints required in Python code
Docstrings required for all public APIs
Files:
tests/test_voice.pycortex/voice.pycortex/branding.pycortex/cli.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Maintain >80% test coverage for pull requests
Files:
tests/test_voice.py
{setup.py,setup.cfg,pyproject.toml,**/__init__.py}
📄 CodeRabbit inference engine (AGENTS.md)
Use Python 3.10 or higher as the minimum supported version
Files:
pyproject.toml
🧬 Code graph analysis (3)
tests/test_voice.py (1)
cortex/voice.py (9)
VoiceInputHandler(39-495)_ensure_dependencies(87-125)_check_microphone(153-175)transcribe(226-270)stop(477-495)VoiceInputError(21-24)MicrophoneNotFoundError(27-30)ModelNotFoundError(33-36)get_voice_handler(498-517)
cortex/voice.py (1)
cortex/branding.py (1)
cx_print(52-82)
cortex/cli.py (1)
cortex/voice.py (4)
VoiceInputError(21-24)VoiceInputHandler(39-495)start_voice_mode(399-432)record_single(434-475)
🪛 GitHub Actions: CI
cortex/cli.py
[error] 580-580: Ruff check failed: W293 Blank line contains whitespace. Command: 'ruff check . --output-format=github'.
🪛 GitHub Check: lint
cortex/cli.py
[failure] 591-591: Ruff (W293)
cortex/cli.py:591:1: W293 Blank line contains whitespace
[failure] 588-588: Ruff (W293)
cortex/cli.py:588:1: W293 Blank line contains whitespace
[failure] 580-580: Ruff (W293)
cortex/cli.py:580:1: W293 Blank line contains whitespace
🪛 GitHub Check: Lint
cortex/cli.py
[failure] 591-591: Ruff (W293)
cortex/cli.py:591:1: W293 Blank line contains whitespace
[failure] 588-588: Ruff (W293)
cortex/cli.py:588:1: W293 Blank line contains whitespace
[failure] 580-580: Ruff (W293)
cortex/cli.py:580:1: W293 Blank line contains whitespace
🪛 LanguageTool
docs/VOICE_INPUT.md
[grammar] ~141-~141: Ensure spelling is correct
Context: ...apture** - Records via sounddevice at 16kHz mono 3. Speech-to-Text - Transcribe...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~141-~141: Ensure spelling is correct
Context: ... Records via sounddevice at 16kHz mono 3. Speech-to-Text - Transcribes using `fa...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
docs/VOICE_INPUT.md
66-66: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
146-146: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Agent
- GitHub Check: test (3.11)
- GitHub Check: test (3.12)
- GitHub Check: test (3.10)
🔇 Additional comments (14)
cortex/branding.py (3)
8-14: Good Windows compatibility improvements for Rich console.The platform-aware Console initialization with
force_terminal=Trueandlegacy_windowsbased on platform detection is a solid approach for cross-platform terminal rendering.
63-79: Well-structured platform-specific icon fallbacks.The ASCII fallback icons for Windows (
|,+,!,x,*) are appropriate replacements for the Unicode characters that may not render correctly in Windows terminals. The conditional structure is clean and maintainable.
92-102: Consistent platform-specific separators.The separator adjustments in
cx_stepandcx_headeralign with the icon changes above, ensuring consistent visual appearance across platforms.pyproject.toml (1)
72-79: Voice optional dependencies are correctly structured.The optional dependency group is properly defined with appropriate version constraints compatible with Python 3.10+. Including it in the
allextra is appropriate for comprehensive installation.docs/VOICE_INPUT.md (1)
1-46: Well-structured documentation with comprehensive coverage.The documentation covers all essential aspects: installation, usage modes (continuous and single), configuration options, troubleshooting, privacy considerations, and API reference. The privacy-first approach (local processing, no audio uploads) is well highlighted.
tests/test_voice.py (1)
10-11: Good test organization with clear class-based grouping.The test suite is well-organized into logical test classes (
TestVoiceInputHandler,TestVoiceInputExceptions,TestGetVoiceHandler,TestRecordingState), each with focused test methods and appropriate fixtures. The mocking strategy for optional dependencies is sound.Also applies to: 199-200, 224-225, 268-269
cortex/cli.py (3)
540-552: Good implementation with proper dependency handling.The voice method correctly handles the optional dependency import with a helpful error message guiding users to install the voice extras. The API key and provider checks follow the established pattern in the codebase.
560-577: Consider case-insensitive verb matching and edge cases.The install verb detection uses
startswith()after lowercasing, which is good. However, the verb removal at lines 574-577 operates on the originaltextbut useslen(verb)from the lowercase version, which works correctly since length is preserved. The logic is sound.One edge case: if the user says "Install", the slicing
text[len(verb):]correctly preserves the original casing of the software name.
1633-1638: Help table correctly advertises new voice capabilities.The Rich help table now includes voice command entries, making the feature discoverable to users.
cortex/voice.py (5)
21-36: Well-designed exception hierarchy.The exception classes are properly structured with a base
VoiceInputErrorand specific subclasses for different error conditions. Docstrings are present as required by coding guidelines. Thepassstatements are appropriate for simple exception classes.
51-85: Good initialization with sensible defaults and type hints.The
__init__method has proper type hints, comprehensive docstrings, and sensible defaults. The environment variable fallback formodel_nameprovides flexibility. Internal state management with explicit initialization is clean.
226-270: Transcription method is well-implemented with proper validation.The
transcribemethod correctly:
- Handles lazy model loading (line 238-239)
- Returns early for empty audio (line 241-242)
- Validates dtype before processing (line 245-246)
- Uses appropriate VAD parameters for voice commands
- Strips and joins segment texts properly
498-517: Factory function provides clean API.The
get_voice_handlerfactory function has proper type hints and docstring. It provides a simpler interface for common use cases while still allowing customization.
177-202: Audio recording implementation is robust.The
_start_recordingmethod properly:
- Clears the buffer before starting
- Uses a callback-based approach for non-blocking capture
- Sets up appropriate stream parameters (16kHz, mono, float32)
- Handles errors by resetting state and raising a specific exception
|
@Sahilbhatane remove all those virtual environment files. You should never push those. |
yeah ik, .gitignore didn't recognized them ig. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
cortex/voice.py (1)
486-490: Improve exception handling in hotkey listener cleanup.The bare
except Exception: passsilently swallows all errors. Following the pattern used at line 498 for the audio stream, add debug logging to track potential issues during cleanup.🔎 Proposed fix
if self._hotkey_listener: try: self._hotkey_listener.stop() - except Exception: - pass + except Exception as e: + logging.debug("Error stopping hotkey listener: %s", e) self._hotkey_listener = Nonecortex/cli.py (3)
554-558: Unusedprovidervariable.The
providervariable is retrieved at line 558 but never used in thevoicemethod. Theprocess_voice_commandcallback already retrieves the API key and usesself.ask()andself.install(), which internally get the provider themselves.🔎 Consider removing unused variable
api_key = self._get_api_key() if not api_key: return 1 - - provider = self._get_provider()
579-600: Fix trailing whitespace to resolve pipeline failures.Based on past review comments, lines 580, 588, and 591 contain trailing whitespace on blank lines, causing CI/linter failures. These must be fixed for the PR to pass.
🔎 Remove trailing whitespace
Ensure lines 580, 588, and 591 are completely empty (no spaces or tabs):
cx_print(f"Installing: {software}", "info") - + # Ask user for confirmation console.print() console.print("[bold cyan]Choose an action:[/bold cyan]") console.print(" [1] Dry run (preview commands)") console.print(" [2] Execute (run commands)") console.print(" [3] Cancel") console.print() - + try: choice = input("Enter choice [1/2/3]: ").strip() - + if choice == "1":
2000-2015: Add error handling for VoiceInputError and its subclasses.The
install --micflow catchesImportErrorbut doesn't handleVoiceInputError,MicrophoneNotFoundError, orModelNotFoundError. If voice recording fails (e.g., no microphone detected, model loading fails), an unhandled exception will propagate.🔎 Proposed fix
if getattr(args, "mic", False): try: - from cortex.voice import VoiceInputHandler + from cortex.voice import VoiceInputError, VoiceInputHandler handler = VoiceInputHandler() cx_print("Press F9 to speak what you want to install...", "info") software = handler.record_single() if not software: cx_print("No speech detected.", "warning") return 1 cx_print(f"Installing: {software}", "info") except ImportError: cli._print_error("Voice dependencies not installed.") cx_print("Install with: pip install cortex-linux[voice]", "info") return 1 + except VoiceInputError as e: + cli._print_error(str(e)) + return 1
🧹 Nitpick comments (1)
.gitignore (1)
14-20: Consolidate duplicate entries for maintainability.The
.gitignorefile contains numerous redundant entries. For example:
env/,venv/,ENV/appear at lines 14–15, 143–145.mypy_cache/,.pytest_cache/,.coverage,htmlcov/appear at lines 70–71 and 146–149This duplication reduces readability and makes future maintenance harder.
🔎 Suggested cleanup: Remove duplicates and consolidate sections
# ============================== # Logs & Misc # ============================== *.log logs/ *.tmp *.bak *.swp - .env - .venv - env/ - venv/ - ENV/ - .mypy_cache/ - .pytest_cache/ - .coverage - htmlcov/ *.out *~ *.swoThen, verify that all non-duplicate entries are already covered in their respective sections above (Virtual Environments, mypy/Pyre/pytype, etc.).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (23)
myenv/Scripts/bandit-baseline.exeis excluded by!**/*.exemyenv/Scripts/bandit-config-generator.exeis excluded by!**/*.exemyenv/Scripts/bandit.exeis excluded by!**/*.exemyenv/Scripts/black.exeis excluded by!**/*.exemyenv/Scripts/blackd.exeis excluded by!**/*.exemyenv/Scripts/coverage-3.12.exeis excluded by!**/*.exemyenv/Scripts/coverage.exeis excluded by!**/*.exemyenv/Scripts/coverage3.exeis excluded by!**/*.exemyenv/Scripts/dmypy.exeis excluded by!**/*.exemyenv/Scripts/mypy.exeis excluded by!**/*.exemyenv/Scripts/mypyc.exeis excluded by!**/*.exemyenv/Scripts/nltk.exeis excluded by!**/*.exemyenv/Scripts/normalizer.exeis excluded by!**/*.exemyenv/Scripts/pip.exeis excluded by!**/*.exemyenv/Scripts/pip3.12.exeis excluded by!**/*.exemyenv/Scripts/pip3.exeis excluded by!**/*.exemyenv/Scripts/py.test.exeis excluded by!**/*.exemyenv/Scripts/pytest.exeis excluded by!**/*.exemyenv/Scripts/ruff.exeis excluded by!**/*.exemyenv/Scripts/safety.exeis excluded by!**/*.exemyenv/Scripts/stubgen.exeis excluded by!**/*.exemyenv/Scripts/stubtest.exeis excluded by!**/*.exemyenv/Scripts/typer.exeis excluded by!**/*.exe
📒 Files selected for processing (6)
.gitignorecortex/cli.pycortex/voice.pymyenv/share/man/man1/bandit.1tests/test_ollama_integration.pytests/test_voice.py
✅ Files skipped from review due to trivial changes (1)
- myenv/share/man/man1/bandit.1
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_voice.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide
Type hints required in Python code
Docstrings required for all public APIs
Files:
tests/test_ollama_integration.pycortex/voice.pycortex/cli.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Maintain >80% test coverage for pull requests
Files:
tests/test_ollama_integration.py
🧬 Code graph analysis (3)
tests/test_ollama_integration.py (1)
scripts/setup_ollama.py (1)
check_ollama_installed(75-77)
cortex/voice.py (1)
cortex/branding.py (1)
cx_print(52-82)
cortex/cli.py (1)
cortex/voice.py (4)
VoiceInputError(21-24)VoiceInputHandler(39-499)start_voice_mode(401-436)record_single(438-479)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test (Python 3.12)
- GitHub Check: Test (Python 3.10)
- GitHub Check: Test (Python 3.11)
- GitHub Check: test (3.12)
- GitHub Check: test (3.10)
- GitHub Check: test (3.11)
🔇 Additional comments (10)
tests/test_ollama_integration.py (1)
15-15: LGTM: Clean refactor toshutil.which.Replacing the subprocess-based Ollama detection with
shutil.whichis a cleaner, more direct approach that aligns with the pattern used inscripts/setup_ollama.py(lines 74-76).Also applies to: 29-30, 37-37
cortex/voice.py (5)
1-36: LGTM: Clean module structure and exception hierarchy.Module docstring clearly describes the feature, and the exception hierarchy provides appropriate granularity for error handling.
39-86: LGTM: Well-structured initialization with environment-driven configuration.Good use of lazy loading for the model and environment variable overrides. The type hints and docstring meet coding guidelines.
87-270: LGTM: Robust dependency checks and transcription pipeline.The dependency validation provides helpful user guidance, and the transcription implementation uses appropriate parameters (beam_size=5, VAD filtering, no_speech_threshold=0.6).
272-436: LGTM: Thread-safe hotkey handling with good UX.The implementation properly uses threading primitives (locks, events) and provides visual feedback during recording. KeyboardInterrupt handling ensures clean exit.
502-521: LGTM: Clean factory function with proper type hints.The factory function follows Python conventions and provides a clean entry point for creating voice handlers.
cortex/cli.py (4)
1633-1633: LGTM: Help documentation clearly advertises voice features.The help table updates appropriately document the new voice input capabilities for users.
Also applies to: 1638-1638
1722-1739: LGTM: Well-structured argument parsing for voice features.The
--micand--singleflags are appropriately configured, and making the question argument optional enables the voice input flow.
1743-1755: LGTM: Consistent --mic flag integration.The install command's
--micflag follows the same pattern as the ask command, maintaining consistency across the CLI.
1989-1998: LGTM: Clean command routing for voice features.The routing logic appropriately delegates to the voice method for both explicit voice commands and
--micflags, with helpful error messages.
- Remove unused 'provider' variable in cli.py - Add logging to except block in voice.py stop() - Remove unused 'threading' import in test_voice.py - Improve test_ensure_dependencies_missing test - Fix test_transcribe_loads_model_if_needed to test lazy loading - Add VoiceInputError handling to install --mic path - Remove optional voice deps from requirements.txt (use pyproject.toml)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
cortex/voice.py (2)
185-189: Consider using logging for audio status messages.Line 187 prints audio status directly to stderr. For consistency with the rest of the module (which uses
cx_printandlogging.debug), consider usinglogging.debug()instead.🔎 Proposed refactor
def audio_callback(indata, frames, time_info, status): if status: - print(f"Audio status: {status}", file=sys.stderr) + logging.debug("Audio status: %s", status) if self._is_recording: self._audio_buffer.append(indata.copy())
238-250: Redundant model check after lazy loading.Lines 248-250 check if
self._model is Noneafter calling_load_model()on line 239. However,_load_model()either setsself._modelor raisesModelNotFoundError, so the model can never beNoneat line 249. This check is dead code.🔎 Proposed refactor
if self._model is None: self._load_model() if len(audio_data) == 0: return "" # faster-whisper expects float32 audio normalized to [-1, 1] if audio_data.dtype != np.float32: audio_data = audio_data.astype(np.float32) - # Model should be loaded at this point - if self._model is None: - raise ModelNotFoundError("Model must be loaded before transcription") - segments, info = self._model.transcribe(docs/VOICE_INPUT.md (1)
66-66: Consider adding language identifiers to code blocks.The fenced code blocks at lines 66 and 146 are missing language identifiers. While not critical, adding them improves syntax highlighting and documentation quality.
🔎 Suggested additions
For line 66 (example session output):
-``` +```console $ cortex voiceFor line 146 (ASCII diagram):
-``` +```text ┌──────────────┐ ┌──────────────┐ ┌──────────────┐Also applies to: 146-157
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
cortex/cli.pycortex/voice.pydocs/VOICE_INPUT.mdrequirements.txttests/test_voice.py
🚧 Files skipped from review as they are similar to previous changes (2)
- requirements.txt
- tests/test_voice.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide
Type hints required in Python code
Docstrings required for all public APIs
Files:
cortex/cli.pycortex/voice.py
🧬 Code graph analysis (2)
cortex/cli.py (3)
cortex/voice.py (4)
VoiceInputError(21-24)VoiceInputHandler(39-499)start_voice_mode(401-436)record_single(438-479)cortex/first_run_wizard.py (1)
_print_error(746-748)cortex/branding.py (1)
cx_print(52-82)
cortex/voice.py (1)
cortex/branding.py (1)
cx_print(52-82)
🪛 LanguageTool
docs/VOICE_INPUT.md
[grammar] ~141-~141: Ensure spelling is correct
Context: ...apture** - Records via sounddevice at 16kHz mono 3. Speech-to-Text - Transcribe...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~141-~141: Ensure spelling is correct
Context: ... Records via sounddevice at 16kHz mono 3. Speech-to-Text - Transcribes using `fa...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
docs/VOICE_INPUT.md
66-66: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
146-146: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (10)
cortex/voice.py (3)
339-383: LGTM! Thread-safe hotkey handling.The hotkey setup properly uses a lock for thread safety and correctly sets the recording flag before starting threads. The logic handles both press-to-start and press-to-stop flows correctly.
385-436: LGTM! Proper error handling and resource cleanup.The recording worker and continuous voice mode implementation correctly handle errors and ensure resources are cleaned up via the
finallyblock.
481-499: LGTM! Exception handling improved.The cleanup logic properly handles exceptions during shutdown and logs them for debugging. This addresses the past review comment about silent exception handling.
docs/VOICE_INPUT.md (2)
49-49: LGTM! Model size documentation is now consistent.The documentation correctly states that the default
base.enmodel is ~150MB, which is consistent with the table at line 117. This addresses the past review comment about the documentation inconsistency.
1-261: LGTM! Comprehensive and well-structured documentation.The documentation provides excellent coverage of the voice input feature, including installation, usage, configuration, troubleshooting, and API reference. The structure is clear and user-friendly.
cortex/cli.py (5)
540-625: LGTM! Well-designed voice input integration.The
voice()method properly handles both continuous and single-shot modes, includes user confirmation for installations, and has comprehensive error handling for missing dependencies and voice input errors.
1989-1996: LGTM! Clean integration of voice input with ask command.The
--micflag integration properly routes to the voice handler and provides clear error messages when neither a question nor the mic flag is provided.
1998-2027: LGTM! VoiceInputError handling properly implemented.The install command's
--micintegration now correctly imports and catchesVoiceInputError(lines 2001, 2014-2016), addressing the past review comment about missing error handling. The implementation provides clear error messages and proper fallback behavior.
1728-1753: LGTM! Parser configuration is correct.The new voice command and
--micflag integrations are properly configured with clear help text and appropriate argument handling. Makingsoftwareoptional (line 1741) correctly supports the--micworkflow.
1987-1988: LGTM! Voice command routing is correct.The routing logic correctly maps the
--singleflag to thecontinuousparameter by negating it, so that by default (no flag) continuous mode is active, and with--singleit switches to single-shot mode.
Sahilbhatane
changed the title
|
3 participants
Related Issue
Closes #325
Summary
Adds voice input capability to Cortex Linux, enabling users to speak commands instead of typing them. This implementation uses local speech-to-text processing with Whisper for privacy and low latency.
Features Added
cortex voicefor continuous voice inputcortex voice --singlefor one-shot recordingcortex install --micandcortex ask --micCORTEX_VOICE_HOTKEY)● Recording...) during speech captureTechnical Details
faster-whisper(optimized Whisper) for accurate, local STTbase.en(~150MB, good accuracy/speed balance)pip install cortex-linux[voice]Files Changed
cortex/voice.py- New voice input handler modulecortex/cli.py- Added voice commands and --mic flagscortex/branding.py- Windows ASCII fallback for console outputpyproject.toml- Added[voice]optional dependenciesdocs/VOICE_INPUT.md- User documentationtests/test_voice.py- Unit tests (20 tests)tests/test_ollama_integration.py- Fixed Windows compatibilityChecklist
pytest tests/)ruff check,black --check)bandit -r cortex/)docs/VOICE_INPUT.md)additionally for Wayland based Ubuntu user (if hotkey doesn't work) -
Summary by CodeRabbit
New Features
--micflag to ask and install commands for voice-based inputDocumentation
Dependencies
✏️ Tip: You can customize this high-level summary in your review settings.