V11 #25
V11 #25
Conversation
…ree main target platforms with industry-standard values.
…training, and UI panels for training, dataset, and config, alongside new data processing and Hugging Face model integration, while removing temporary file lists and old documentation
… framework components
…raction, and a comprehensive training loop manager.
… shift, background noise, RIR, and epoch-based noise/RIR subset management.
… and configuration defaults.
… and configuration defaults.
…eletion script, and disable pre-commit hooks and the secrets baseline.
…ng, various model architectures, and training utilities.
…, add pre-commit hooks, and update repository configuration.
…feature extraction, Hugging Face model integration, and dataset reporting scripts along with related documentation.
…sing utilities, and an evaluation UI panel.
…nd export, alongside comprehensive dataset evaluation with QAT support, and remove legacy development files.
…g platform with comprehensive configuration and dataset extraction.
…p caching, and a dedicated server Dockerfile.
…g audio processing, model training, and checkpointing.
…onents, removing a disabled pre-commit configuration.
…g a new CI workflow, Python audit script, and various new guides, while updating project build configurations and the feature checklist.
…nd core training infrastructure
…and VS Code development settings.
…RNN models with attention, and configure static analysis.
…figuration, data processing, model architectures, training loop, and UI panels.
Comprehensive design for reducing False Acceptance Rate through: - Confusable word mining (TTS + phonetic neighbors) - Margin-based loss (ArcFace/CosFace) - Weighted negative sampling - Multi-condition training - Confidence calibration Expected impact: 40-60% FAR reduction
Comprehensive design for training speedup through curriculum learning: - Progressive audio duration (0.75s -> 1.5s) - Progressive augmentation (Tier 1 -> Tier 3) - Progressive difficulty (easy -> hard negatives) - Unified scheduler coordinating all dimensions Expected impact: 1.6-2.0x speedup on long training runs (50+ epochs)
…gressive training, new models, data handling, evaluation, and CI/CD workflows.
… documentation quality, and Pylint, along with updates to `AGENTS.md` and `CLAUDE.md`.
…, and utility scripts, and remove outdated analysis documents.
…including distributed training, various trainers, evaluation tools, and utility scripts.
…word Detection - Added `CascadeOrchestrator` class to manage the Sentry -> Judge -> Teacher pipeline, including stage transitions and result aggregation. - Introduced `TeacherModel` class for cloud-based knowledge distillation, supporting Wav2Vec2 and Whisper backbones. - Created monitoring package with `DriftDetector` for detecting feature and data drift in audio ML pipelines. - Integrated MLflow model registry in `ModelRegistry` for experiment tracking and model versioning. - Developed unit and integration tests for data versioning, quality validation, drift detection, and MLOps integration.
…es, and configuration for wakeword detection.
…ion with streaming algorithm
- Add TFLiteWakeWordModel class for loading and inferring TFLite and ONNX models. - Create WyomingWakeWordServer class to handle Wyoming protocol events and manage wake word detection. - Introduce WakeWordHandler for processing audio chunks and managing detection state. - Add configuration management with WyomingServerConfig for server setup. - Implement feature extraction methods for audio processing. - Create unit tests for server components, including configuration and wake word detection logic. - Define requirements for dependencies in requirements-wyoming.txt.
…integration and version bump to v4.1
There was a problem hiding this comment.
Pull request overview
This is a comprehensive refactoring and enhancement PR labeled "V11" that modernizes the codebase with improved code quality, new features, and better error handling across multiple components.
Key Changes:
- Refactored logging from f-strings to lazy % formatting throughout the codebase
- Added new cascade architecture modules (Sentry, Judge, Teacher, Orchestrator) for distributed wakeword detection
- Enhanced configuration system with new presets, validation improvements, and support for additional loss functions
- Improved error handling with specific exception types replacing broad
Exceptioncatches - Added new data processing capabilities including audio preprocessing, versioning, and quality validation modules
- Cleaned up helper scripts and removed numerous unused/obsolete utility files
- Enhanced server infrastructure with Prometheus metrics and improved security
Reviewed changes
Copilot reviewed 156 out of 277 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/data/audio_utils.py | Improved exception handling and logging format |
| src/data/init.py | Added imports for new data modules |
| src/config/validator.py | Enhanced distillation validation logic |
| src/config/size_calculator.py | Updated logger import |
| src/config/pydantic_validator.py | Formatting improvements and new loss function support |
| src/config/presets.py | Major overhaul of configuration presets with platform-specific and strategy-based configurations |
| src/config/env_config.py | Type hint and formatting improvements |
| src/config/defaults.py | Added new configuration options and validation |
| src/config/cuda_utils.py | Added CUDA health check functionality |
| src/cascade/*.py | New cascade architecture modules for distributed detection |
| src/audio/*.py | New audio preprocessing module |
| server/*.py | Enhanced server with metrics and security improvements |
| scripts/*.py | Various refactoring and cleanup |
| scripts/helper_scripts/*.py | Removed numerous obsolete helper scripts |
| SECONDS_PER_HOUR = 3600 | ||
|
|
||
|
|
There was a problem hiding this comment.
The constant SECONDS_PER_HOUR is defined but never used in this file. If it's intended for use by other modules, consider moving it to a dedicated constants module. If unused, remove it.
| SECONDS_PER_HOUR = 3600 |
| "Starting QAT verification on device: %s" % "cuda" | ||
| ) # Assuming device is "cuda" from original load_model_for_evaluation |
There was a problem hiding this comment.
The device is hardcoded to 'cuda' in the print statement, but the actual device used by load_model_for_evaluation may differ. The device should be determined dynamically (e.g., check if CUDA is available) and used consistently.
| use_precomputed_features_for_training=False, | ||
| fallback_to_audio=True, # Safety fallback if NPY files exist |
There was a problem hiding this comment.
The comment on lines 865-867 explains why use_precomputed_features_for_training=False is critical for distillation, but this same setting appears in multiple presets without the explanatory comment. Consider adding this documentation to other distillation-enabled presets or extracting it into a shared docstring.
| # Create preprocessor (matching checkpoint training config) | ||
| if AudioPreprocessor is None: | ||
| raise RuntimeError( | ||
| "AudioPreprocessor is not available. " "Ensure src.audio.preprocessing module is installed." |
There was a problem hiding this comment.
The error message suggests 'installing' the module, but src.audio.preprocessing is part of this codebase, not an external package. The message should indicate the module is missing or there's an import error, not suggest installation.
| "AudioPreprocessor is not available. " "Ensure src.audio.preprocessing module is installed." | |
| "AudioPreprocessor is not available. Ensure that src.audio.preprocessing.AudioPreprocessor " | |
| "is present in the codebase and importable in this environment." |
scripts/training_insights.py
Outdated
| try: | ||
| checkpoint = torch.load(checkpoint_path, map_location="cpu", weights_only=True) | ||
| checkpoint = torch.load( | ||
| checkpoint_path, map_location="cpu", weights_only=False |
There was a problem hiding this comment.
Changed from weights_only=True to weights_only=False, which allows arbitrary code execution via pickle. This introduces a security vulnerability. If additional metadata beyond model weights is needed, consider extracting it separately or validating the checkpoint source before loading.
| checkpoint_path, map_location="cpu", weights_only=False | |
| checkpoint_path, map_location="cpu", weights_only=True |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||||||
Summary of ChangesHello @sarpel, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a substantial upgrade to the wakeword training platform, focusing on operational excellence and expanded capabilities. It streamlines the development and deployment process through enhanced Docker configurations, enforces code quality with new Git hooks, and provides an optimized training experience with an A100-focused Colab notebook. A major emphasis has been placed on improving documentation across the board, introducing new guides for MLOps, progressive training, and the innovative cascade architecture. Crucially, the platform now integrates with Home Assistant via the Wyoming Protocol, broadening its applicability for smart home voice control. These changes collectively aim to make the platform more robust, user-friendly, and production-ready. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
PR Code Suggestions ✨Latest suggestions up to 5339db7
Previous suggestions✅ Suggestions up to commit e97761e
|
||||||||||||||||||||||||||||||||||||||||||||||||||
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| return False, "❌ No models available. Export a model first in Panel 5." | ||
|
|
||
| path = Path(model_path) | ||
| if not path.exists(): |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 days ago
In general, the fix is to ensure that any file path originating from user-controllable data is restricted to a safe root directory (or small set of directories) and normalized before being trusted. This usually means: (1) defining one or more allowed base directories, (2) resolving the user-supplied path relative to those bases via Path.resolve() or os.path.realpath, and (3) verifying that the resolved path is within one of those allowed directories using a prefix or ancestor check (e.g., is_relative_to or a safe fallback).
For this code, the best minimal fix is to define the same model directories used in get_available_models as allowed roots and update validate_model_path to:
- Reject the "no models" sentinel as before.
- Resolve the given
model_pathto an absolute, normalizedPath. - Check whether the resolved path lies under either
exportsormodels/exports. - Only then check existence and extension.
This preserves existing functionality (valid model files under those dirs still work) while preventing arbitrary file system paths from being treated as valid models. All changes are confined to validate_model_path in src/ui/panel_wyoming.py; we do not need new imports beyond pathlib.Path, which is already present.
Concretely:
- Replace the body of
validate_model_pathstarting frompath = Path(model_path)through the extension check with logic that:- Builds
Pathobjects for the two allowed roots (Path("exports"),Path("models/exports")) and resolves them. - Resolves
model_path(path = Path(model_path).resolve()). - Uses
Path.is_relative_to(Python 3.9+) where available, or a suffix-based prefix check onpath.resolve()compared to each root’sresolve(). - Returns a clear error message if the path is outside those roots.
- Builds
No other parts of the file need modification.
| @@ -78,7 +78,37 @@ | ||
| if model_path == "No models available - export a model first": | ||
| return False, "❌ No models available. Export a model first in Panel 5." | ||
|
|
||
| path = Path(model_path) | ||
| # Restrict models to known export directories and normalize the path | ||
| allowed_dirs = [Path("exports").resolve(), Path("models/exports").resolve()] | ||
|
|
||
| try: | ||
| path = Path(model_path).resolve() | ||
| except OSError: | ||
| return False, f"❌ Invalid model path: {model_path}" | ||
|
|
||
| # Ensure the resolved path is within one of the allowed directories | ||
| def _is_under_allowed_dir(p: Path) -> bool: | ||
| for base in allowed_dirs: | ||
| try: | ||
| # Python 3.9+: use is_relative_to when available | ||
| is_relative_to = getattr(p, "is_relative_to", None) | ||
| if is_relative_to is not None: | ||
| if p.is_relative_to(base): | ||
| return True | ||
| else: | ||
| # Fallback for older Python: compare common parts | ||
| try: | ||
| p.relative_to(base) | ||
| return True | ||
| except ValueError: | ||
| continue | ||
| except Exception: | ||
| continue | ||
| return False | ||
|
|
||
| if not _is_under_allowed_dir(path): | ||
| return False, "❌ Model path must be inside the exports or models/exports directory." | ||
|
|
||
| if not path.exists(): | ||
| return False, f"❌ Model file not found: {model_path}" | ||
|
|
| try: | ||
| # Start subprocess | ||
| wyoming_state.server_process = subprocess.Popen( | ||
| cmd, |
Check failure
Code scanning / CodeQL
Uncontrolled command line Critical
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 days ago
At a high level, the fix is to ensure that all user-supplied values used in the cmd list are validated and constrained before being passed to subprocess.Popen. This can be done by: (1) using fixed-choice allowlists where possible (e.g., feature_type), (2) validating that path inputs refer only to allowed models, (3) constraining text inputs to a safe character set and length, and (4) validating numeric inputs fall within expected ranges and are of the right type. Since the command is already list-based (no shell), we do not need to change the execution mechanism, just ensure arguments are safe and well-formed.
The single best minimal-impact fix here is to introduce a dedicated validation function (e.g., sanitize_and_validate_server_params) that takes all parameters to start_wyoming_server, checks them, and either returns a cleaned set of values or an error (status + log) before the cmd list is built. This function can: (a) re-use validate_model_path for model_path and additionally ensure it matches one of the models returned by get_available_models(), (b) ensure feature_type is in a small allowlist like {"mel", "mfcc"}, (c) check that host is non-empty and contains only reasonable characters ([0-9a-zA-Z-.:]), (d) clamp or validate numeric settings (port in [1, 65535], thresholds, trigger levels, etc.), and (e) normalize types (e.g., cast floats/ints explicitly). The function can return either (True, cleaned_values, "") or (False, None, "error message"); in start_wyoming_server we call it up-front and return an error status if validation fails, otherwise proceed using the sanitized values to construct cmd.
Concretely, in src/ui/panel_wyoming.py we will:
- Add a new helper function near
validate_model_pathandstart_wyoming_serverthat performs validation of all parameters. - Update
start_wyoming_serverto call this helper right after the existingvalidate_model_pathcall, and then use the validated values (e.g.,safe_model_path,safe_wake_word_name, etc.) to populatecmd. - Optionally, tighten
feature_typeby ensuring it is exactly one of the dropdown’s values and fallback if not.
No other files or UI wiring need to change; the Gradio definitions and start_btn.click binding remain the same. We rely only on Python’s standard library (re, ipaddress optionally), which we can safely import at the top of this file.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Now let me create a comprehensive code review: Pull Request Review: V11 - Wyoming Protocol Integration & Platform EnhancementsThis is a comprehensive PR that adds Wyoming Protocol support for Home Assistant integration and includes numerous quality improvements. Below is my detailed review covering code quality, potential bugs, and suggestions. 🎯 Overall AssessmentStatus: Approve with Minor Concerns This PR delivers significant value by:
However, there are several issues that should be addressed before merging. 🔴 Critical Issues1. Security: Pickle Loading Vulnerability (HIGH PRIORITY)Location: Issue: Using # BEFORE (INSECURE)
checkpoint = torch.load(checkpoint_path, map_location="cpu", weights_only=False)
# AFTER (SECURE)
checkpoint = torch.load(checkpoint_path, map_location="cpu", weights_only=True)Why this matters: This is a known security vulnerability (CWE-502). An attacker could craft a malicious checkpoint file that executes arbitrary code when loaded. Recommendation: Use 2. Bug: Incomplete Hard Negative Weighting (HIGH PRIORITY)Location: Issue: Hard negative weighting logic is skipped for margin-based losses due to early # Current code (lines 916-921) - BUGGY
if loss_name == "triplet_loss":
return cast(torch.Tensor, self.criterion(embeddings, targets)) # ❌ Returns early!
elif loss_name in ["arcface", "cosface"]:
return cast(torch.Tensor, self.criterion(embeddings, targets)) # ❌ Returns early!
# The hard negative weighting code below is never reached for these losses
if is_hard_negative is not None:
hn_weight = getattr(self.config.loss, "hard_negative_weight", 1.5)
weights = torch.ones_like(loss)
weights = weights + (is_hard_negative * (hn_weight - 1.0))
loss = loss * weightsFix: def compute_loss(
self,
outputs: torch.Tensor,
targets: torch.Tensor,
_inputs: Optional[torch.Tensor] = None,
processed_inputs: Optional[torch.Tensor] = None,
is_hard_negative: Optional[torch.Tensor] = None,
) -> torch.Tensor:
if outputs.numel() == 0 or targets.numel() == 0:
return torch.tensor(0.0, device=outputs.device, requires_grad=True)
loss_name = self.config.loss.loss_function.lower()
# Compute base loss (with reduction='none' to get per-sample losses)
if loss_name in ["triplet_loss", "arcface", "cosface", "combined_margin"]:
if processed_inputs is not None and hasattr(self.model, "embed"):
embeddings = cast(Any, self.model).embed(processed_inputs)
else:
embeddings = outputs
if loss_name == "triplet_loss":
loss = cast(torch.Tensor, self.criterion(embeddings, targets))
elif loss_name in ["arcface", "cosface"]:
loss = cast(torch.Tensor, self.criterion(embeddings, targets))
elif loss_name == "combined_margin":
loss = cast(torch.Tensor, self.criterion(outputs, embeddings, targets))
else:
# Standard losses like CrossEntropy, FocalLoss
loss = cast(Any, self.criterion)(outputs, targets)
# Apply hard negative weighting to per-sample loss
if is_hard_negative is not None:
hn_weight = getattr(self.config.loss, "hard_negative_weight", 1.5)
weights = torch.ones_like(loss)
weights = weights + (is_hard_negative * (hn_weight - 1.0))
loss = loss * weights
# Return mean of (potentially weighted) per-sample losses
return loss.mean()3. Bug: False Negative Samples Not Injected (HIGH PRIORITY)Location: Issue: The function returns early for false negatives without calling # BEFORE (BUGGY)
def confirm_all_and_inject_handler(mode: str) -> str:
if mode == "Review False Negatives":
confirm_count = eval_state.fn_collector.confirm_all()
return f"✅ Confirmed {confirm_count} false negatives as hard positives." # ❌ Returns without injection!
confirm_count = eval_state.miner.confirm_all_pending()
inject_count = eval_state.miner.inject_to_dataset()
return f"✅ Bulk Action Complete: Confirmed {confirm_count} and added {inject_count} samples to dataset."
# AFTER (FIXED)
def confirm_all_and_inject_handler(mode: str) -> str:
if mode == "Review False Negatives":
confirm_count = eval_state.fn_collector.confirm_all()
inject_count = eval_state.fn_collector.inject_to_dataset() # ✅ Added!
return f"✅ Confirmed and added {inject_count} hard positives to the dataset."
confirm_count = eval_state.miner.confirm_all_pending()
inject_count = eval_state.miner.inject_to_dataset()
return f"✅ Bulk Action Complete: Confirmed {confirm_count} and added {inject_count} hard negatives to dataset."🟡 Medium Priority Issues4. Potential OOM Error During CalibrationLocation: Issue: Loading entire validation set into memory at once: # Current code - May cause OOM with large validation sets
probs = (
calibration_results["calibrator"]
.get_calibrated_probs(
torch.cat([self.model(b[0].to(self.device)).detach().cpu() for b in self.val_loader])
)[:, 1]
.numpy()
)Fix: Process in batches: # Process validation set in batches
all_outputs = []
all_labels = []
for batch in self.val_loader:
inputs, labels = batch[0].to(self.device), batch[1]
with torch.no_grad():
outputs = self.model(inputs).detach().cpu()
all_outputs.append(outputs)
all_labels.append(labels)
all_outputs_tensor = torch.cat(all_outputs)
all_labels_tensor = torch.cat(all_labels)
probs = (
calibration_results["calibrator"]
.get_calibrated_probs(all_outputs_tensor)[:, 1]
.numpy()
)
labels = all_labels_tensor.numpy()5. Cross-Platform Compatibility IssueLocation: Issue: # BEFORE
def save_wandb_key(key: str) -> None:
try:
key_path = Path(".wandb_key")
key_path.write_text(key.strip(), encoding="utf-8")
key_path.chmod(0o600) # ❌ Fails on Windows
except Exception as e:
logger.warning("Failed to save WandB key", error=str(e))
# AFTER
def save_wandb_key(key: str) -> None:
import os
try:
key_path = Path(".wandb_key")
key_path.write_text(key.strip(), encoding="utf-8")
# Set permissions only on non-Windows systems
if os.name != "nt":
key_path.chmod(0o600)
except Exception as e:
logger.warning("Failed to save WandB key", error=str(e))6. Subprocess Resource Leak RiskLocation: Issue: Subprocess may not be properly cleaned up on exceptions. Recommendation: Use context manager or ensure cleanup in finally block: try:
wyoming_state.server_process = subprocess.Popen(
cmd,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
text=True,
bufsize=1,
creationflags=subprocess.CREATE_NEW_PROCESS_GROUP if sys.platform == "win32" else 0,
)
wyoming_state.is_running = True
# ... rest of logic
except Exception as e:
# Ensure cleanup on failure
if wyoming_state.server_process:
wyoming_state.server_process.kill()
wyoming_state.server_process = None
wyoming_state.is_running = False
logger.exception("Failed to start Wyoming server")
return f"❌ Failed to start: {str(e)}", log + f"\n❌ Error: {e}"7. Missing Queue Mode Support in Mining ButtonLocation: Issue: The "Mine from Test Results" button doesn't respect the # Current code
mine_from_test_btn.click(
fn=mine_hard_negatives_handler, # ❌ Always mines hard negatives
outputs=[injection_status],
).then(fn=get_mining_queue_data, outputs=[mining_queue_df])
# Fixed code
mine_from_test_btn.click(
fn=lambda mode: collect_false_negatives() if mode == "Review False Negatives" else mine_hard_negatives_handler(),
inputs=[queue_mode],
outputs=[injection_status],
).then(
fn=lambda mode: get_fn_queue_data() if mode == "Review False Negatives" else get_mining_queue_data(),
inputs=[queue_mode],
outputs=[mining_queue_df],
)🟢 Code Quality ObservationsStrengths ✅
Minor Improvements8. Documentation Audit FailuresThe CI shows 13 ERROR-level documentation issues:
Fix: Run the documentation maintenance tool and address these issues: python scripts/docs_maintenance.py --fix9. Unused ImportLocation: The PR description mentions "Removed unused imports" but verify that all cleanup was completed. 10. Magic NumbersLocation: self._cooldown_samples = int(config.sample_rate * 0.5) # 500ms cooldownRecommendation: Make configurable: cooldown_duration_ms = getattr(config, 'cooldown_ms', 500)
self._cooldown_samples = int(config.sample_rate * cooldown_duration_ms / 1000)11. Code Duplication in Feature ExtractionLocation: The config loading logic is duplicated from 📋 Checklist for Merge
💡 Suggestions for Future
🎉 SummaryThis is a substantial and well-executed PR that adds significant value to the platform. The Wyoming Protocol integration is clean and well-documented. The code quality improvements throughout the codebase are excellent. The critical issues identified are fixable and should be addressed before merge. Once these are resolved, this will be a great addition to the project! Recommendation: Request changes for the 3 critical bugs, then approve after fixes are applied. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces significant updates across the project, focusing on improved development workflows, documentation, and Dockerization. Key changes include a comprehensive .dockerignore and expanded .gitattributes for better build context and file handling, along with enhanced .gitignore rules for secrets and temporary files. The .pre-commit-config.yaml is updated with new local hooks to prevent committing build artifacts, large model files, and data files, and refines existing hooks. A new .vscode/settings.json is added for consistent IDE configuration. The AGENTS.md and CLAUDE.md files are either new or completely rewritten to provide detailed guidelines for AI coding agents and a mentor protocol for learners, respectively. The Colab_Training_Platform.ipynb is transformed into a full A100-optimized training pipeline. The Dockerfile and docker-compose.yml are extensively updated for multi-stage builds, non-root users, BuildKit caching, health checks, and improved resource management. The README.md is updated to version 4.1, highlighting a new Wyoming Protocol Server for Home Assistant integration and adding a comprehensive documentation section. New configuration files (configs/config_20251231_054207.yaml, configs/config_20251231_084830.yaml) are added, though one contains an invalid num_classes: 1 setting for focal loss, which a reviewer noted would cause a runtime error. Documentation files (GUIDE.md, docs/CODE_QUALITY_REVIEW.md, docs/DEVELOPMENT_ROADMAP.md, docs/DOCS_MAINTENANCE.md, docs/MLOPS_GUIDE.md, docs/PROGRESSIVE_TRAINING_GUIDE.md, docs/SYSTEM_CAPABILITIES.md, docs/TECHNICAL_GUIDE.md) are added or updated, detailing code quality, MLOps, progressive training, and the new Wyoming server. The Makefile gains new targets for documentation auditing. Python scripts (run.py, scripts/benchmark_focal_qat.py, scripts/compare_with_without_distillation.py, scripts/docs_maintenance.py, scripts/extract_hard_negatives.py, scripts/save_hpo_profiles.py, scripts/test_augmentation_coverage.py, scripts/train_with_distillation.py, scripts/training_insights.py, scripts/verify_qat.py) see various improvements, including logging, argument parsing, error handling, and new functionality. The server/Dockerfile and server/app.py are enhanced with Prometheus metrics and robust error handling, while server/inference_engine.py receives security improvements for checkpoint loading. New modules for cascade architecture (src/audio/preprocessing.py, src/cascade/__init__.py, src/cascade/judge.py, src/cascade/orchestrator.py, src/cascade/sentry.py, src/cascade/teacher.py) are introduced, and configuration defaults (src/config/defaults.py, src/config/env_config.py, src/config/cuda_utils.py) are refined. Review comments highlighted critical issues such as incorrect num_classes in a new config file and bare except: pass statements in audio loading, which were noted as needing specific exception handling and logging for robustness. Additionally, the exclusion of the tests directory from black and isort formatting was questioned, suggesting test code should adhere to the same style standards.
I am having trouble creating individual review comments. Click here to see my feedback.
configs/config_20251231_054207.yaml (41)
The num_classes is set to 1, but the loss_function is focal_loss. For binary classification with focal loss or cross-entropy, num_classes must be 2. This configuration will cause a runtime error during training, likely a CUDA assertion error due to mismatched tensor shapes. Since this file is being added to the repository, it should contain a valid configuration.
num_classes: 2Colab_Training_Platform.ipynb (361-362)
Using a bare except: pass silently swallows all exceptions, including KeyboardInterrupt or SystemExit, and can hide bugs. This makes debugging difficult. It's better to catch specific exceptions you expect from audio loading (e.g., soundfile.LibsndfileError, RuntimeError from librosa) and at least log a warning.
except Exception as e:
print(f"Warning: Failed to load noise file {f}: {e}")
Colab_Training_Platform.ipynb (374-375)
Similar to the noise file loading, this bare except: pass can hide important errors. Catching specific exceptions and logging them will make the data loading process more robust and easier to debug.
except Exception as e:
print(f"Warning: Failed to load RIR file {f}: {e}")
pyproject.toml (136-143)
The tests directory is now excluded from formatting by black and linting by isort. Test code should generally adhere to the same quality and style standards as application code to ensure readability and maintainability. Was excluding the test suite intentional? If not, it's recommended to remove tests from the exclude and skip lists.
![sentry[bot]](https://avatars.githubusercontent.com/u/1396951?s=60&v=4){kind=small}
| self.model = TFLiteWakeWordModel( | ||
| model_path=model_path, | ||
| sample_rate=sample_rate, | ||
| n_mels=n_mels, | ||
| n_fft=n_fft, | ||
| hop_length=hop_length, | ||
| feature_type=feature_type, | ||
| ) |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Code Review: PR #25Overall Assessment: Strong Enhancement ✅Substantial PR with excellent security and threading improvements. Few critical bugs need fixing. Key Strengths
Critical Issues to Fix1. Input Validation Missing (panel_wyoming.py:136-165) - HIGH
2. Memory Leak Risk (panel_wyoming.py:176-183) - MEDIUM
3. QAT Optimizer Issue (trainer.py:773-775) - MEDIUM
Code Quality Highlights✅ F-string to %-formatting migration for structured logging Testing GapsMissing unit tests for:
RecommendationsMust Fix Before Merge:
Should Fix (High Priority): Nice to Have: Final Verdict: Approve with Minor FixesScore: 8.5/10
Great work on Wyoming Protocol integration! 🚀 Reviewed following CLAUDE.md Elite Mentor Protocol |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughBu PR, Home Assistant için Wyoming Wake Word protokolünü ve sunucusunu projeye ekler; UI'ye bir Wyoming paneli eklenir, dokümantasyon güncellenir, Docker/compose ve yeni bağımlılıklar eklenir, ayrıca ilgili testler ve yapılandırma modelleri sağlanır. (≤50 kelime) Changes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/training/trainer.py (1)
676-696: Kritik okunabilirlik sorunu: Çok uzun list comprehension.Satır 681'deki tek satır ifade çok karmaşık: model inference, device transfer, detach, CPU transfer, slicing ve numpy dönüşümü hepsi iç içe. Bu tür nested (iç içe) operasyonlar debug (hata ayıklama) sırasında çok zorlaştırıcı.
Özellikle model çıktılarını toplarken bir sorun çıkarsa (örn. out-of-memory, shape mismatch), hatanın nereden kaynaklandığını anlamak çok zor olacak.
🔎 Önerilen refactoring - Adım adım ayırma
if calibration_results is not None: try: + # Collect model predictions from validation set + all_outputs = [] + for batch in self.val_loader: + inputs = batch[0].to(self.device) + outputs = self.model(inputs).detach().cpu() + all_outputs.append(outputs) + + # Get positive class probabilities probs = ( calibration_results["calibrator"] .get_calibrated_probs( - torch.cat([self.model(b[0].to(self.device)).detach().cpu() for b in self.val_loader]) + torch.cat(all_outputs) )[:, 1] .numpy() ) labels = torch.cat([b[1] for b in self.val_loader]).numpy()Bu değişiklik:
- Her adımı görünür kılıyor
- Debug edilebilirliği artırıyor
- Memory kullanımını izlemeyi kolaylaştırıyor
- Kod incelemelerinde mantığı anlaşılır kılıyor
README.md (1)
15-18: Dokümantasyon bağlantıları onarılması gerekiyor - bağlantılar boş sayfalara gidiyor.Şöyle düşün: Bir rehber kitabının "Bölüm 5'i okuyun" diye yazması ama kitapta Bölüm 5 olmaması gibi. Okuyucu kafası karışır. İşte burada da aynı durum var.
README.md'deki bu üç bağlantı var ama dosyalar gerçekten yok:
DOCUMENTATION.md- satır 15'te bağlantı var ama dosya yokCONFIG_PRESETS_GUIDE.md- satır 15'te bağlantı var ama dosya yokTECHNICAL_FEATURES.md- satır 18'de bağlantı var ama dosya yokBulunabilecek dosyalar:
GUIDE.md(kök dizinde)docs/TECHNICAL_GUIDE.md(docs klasöründe)Bu dosyaları eğer var ise doğru isimlerle değiştirebilir veya yeni dosyalar oluşturabilirsin. Ama şu anki durumda insanlar bu bağlantılara tıklayınca hiçbir şey görmüyor.
docs/TECHNICAL_GUIDE.md (1)
682-707: Bölüm numaralandırması tutarsız görünüyor.Merhaba! Dikkatimi çeken bir durum var. Bölüm numaraları şu sırayla gidiyor:
- Bölüm 7 → Bölüm 11 → Bölüm 9 → Bölüm 10 → Bölüm 12...
Bu, okuyucular için kafa karıştırıcı olabilir. "9. Wyoming Protocol Server" bölümü mantıksal olarak "11. System Architecture"dan önce gelmeli gibi görünüyor. Bölüm numaralarını sıralı hale getirmek okuma deneyimini iyileştirecektir.
🧹 Nitpick comments (20)
src/training/trainer.py (1)
426-439: Hard negative ratio güncelleme mantığı sağlam.Kod hem sampler'da hem de dataset'te hard negative oranını güncellemeye çalışıyor, bu esnek bir yaklaşım. İlk satırdaki compound condition (birleşik koşul) teknik olarak doğru ama biraz karmaşık görünüyor.
🔎 İsteğe bağlı okunabilirlik iyileştirmesi
Eğer tercih edersen, compound condition'ı ayırabilirsin:
- if hasattr(self.train_loader, "batch_sampler") and self.train_loader.batch_sampler: + batch_sampler = getattr(self.train_loader, "batch_sampler", None) + if batch_sampler is not None: - sampler = self.train_loader.batch_sampler + sampler = batch_samplerAma mevcut hali de kabul edilebilir, bu sadece bir stil tercihi.
requirements.txt (1)
97-101: Güzel bir ekleme! Bağımlılıklar düzgün organize edilmiş.Düşün ki bir kütüphane kurmak istiyorsun - burada Wyoming protokolü için gerekli paket ekleniyor. Sürüm kısıtlaması
>=1.8.0gayet mantıklı çünkü daha yeni sürümlerden de faydalanabilirsin. Yorum satırındakitflite-runtimealternatifi de kullanıcılara esneklik sağlıyor.Bir öneri: Ana
requirements.txtdosyasında sadecewyoming>=1.8.0var amasrc/wyoming_server/requirements-wyoming.txtdosyasında daha fazla bağımlılık (structlog, pydantic vb.) mevcut. Kullanıcıların Wyoming sunucusunu çalıştırmak için hangi dosyayı kullanması gerektiğini açıklayan bir not eklemek faydalı olabilir.src/ui/panel_wyoming.py (3)
239-245: Genel exception yakalamak yerine spesifik exception'ları hedefle.Ruff burada
BLE001uyarısı veriyor.except Exceptionkullanmak, beklenmedik hataları gizleyebilir. Hangi hataların oluşabileceğini düşünüp onları yakalamalısın.🔎 Daha spesifik exception handling
- except Exception as e: - log += f"❌ Error stopping server: {e}\n" + except OSError as e: + log += f"❌ OS error stopping server: {e}\n" + except ProcessLookupError: + log += "⚠️ Process already terminated\n"
361-561: UI panel yapısı çok iyi organize edilmiş!Gradio paneli mantıklı bir şekilde bölümlere ayrılmış:
- Sol kolon: Model seçimi ve ayarlar
- Sağ kolon: Sunucu kontrolü ve çıktılar
Event handler'ları temiz şekilde bağlanmış. Kullanıcı deneyimi açısından güzel bir iş çıkarılmış.
Küçük bir not: Line 511'de
is_validdeğişkeni kullanılmıyor (Ruff RUF059). Alt çizgi ile prefixle:- is_valid, msg = validate_model_path(model_path) + _is_valid, msg = validate_model_path(model_path)
565-575: Unusedstateparametresi temizlenebilir.Bu fonksiyon
stateparametresini alıyor ama hiç kullanmıyor. RuffARG001uyarısı veriyor. İki seçeneğin var:
- Parametreyi kaldır (eğer gelecekte kullanılmayacaksa)
- Alt çizgi ile prefixle:
_state(diğer panellerle tutarlılık için)İkinci seçenek muhtemelen daha uygun çünkü docstring "for consistency with other panels" diyor.
🔎 Önerilen düzeltme
-def create_wyoming_deployment_panel(state: Optional[gr.State] = None) -> gr.Blocks: +def create_wyoming_deployment_panel(_state: Optional[gr.State] = None) -> gr.Blocks:src/ui/app.py (1)
227-231: Wyoming panel entegrasyonu temiz ve tutarlı!Yeni tab mevcut yapıyla uyumlu şekilde eklenmiş:
- Tab ID'si sıralı (6)
global_statediğer panellerle aynı şekilde aktarılıyor- İsimlendirme ve emoji kullanımı tutarlı
Tek küçük öneri: Documentation tab'ı artık 7 numaralı ama footer'da hala "v1.0" yazıyor (line 237). Version bump yapıldıysa burayı da güncellemek mantıklı olabilir.
src/wyoming_server/Dockerfile (1)
7-14: Güvenlik önerisi: Container içinde root olmayan kullanıcı ekle.Düşün ki bir ev var ve herkes admin yetkisiyle dolaşıyor - bir şey ters giderse tüm eve zarar verebilir. Docker'da da benzer durum var. Container'ı root olarak çalıştırmak, bir güvenlik açığı durumunda saldırganın tam yetkiye sahip olmasına neden olabilir.
Bu best practice olarak önerilir ama kritik bir sorun değil.
🔎 Non-root kullanıcı ekleme
FROM python:3.10-slim # Set environment variables ENV PYTHONDONTWRITEBYTECODE=1 ENV PYTHONUNBUFFERED=1 ENV PYTHONPATH=/app +# Create non-root user +RUN groupadd --gid 1000 wyoming && \ + useradd --uid 1000 --gid wyoming --shell /bin/bash --create-home wyoming + WORKDIR /appVe dosyanın sonuna:
# Create exports directory for model mounting -RUN mkdir -p /app/exports +RUN mkdir -p /app/exports && chown -R wyoming:wyoming /app +# Switch to non-root user +USER wyoming + # Default port EXPOSE 10400src/wyoming_server/requirements-wyoming.txt (2)
1-2: "Minimal" tanımı PyTorch içerdiğinde biraz yanıltıcı olabilir.Dosya başlığında "Minimal Dependencies" ve "Lightweight package list" yazıyor, ama
torch>=2.1.0vetorchaudio>=2.1.0toplamda ~2GB boyutunda. Bu, Docker image'ını oldukça büyütecek.Düşün ki "hafif bavul" deyip içine halter koyuyorsun - teknik olarak hala bavul ama hafif değil!
Eğer sadece inference yapılacaksa ve model ONNX/TFLite formatındaysa, PyTorch olmadan da çalışabilir mi? Eğer çalışabilirse, torch'u opsiyonel yapmak image boyutunu dramatik şekilde azaltır.
🔎 Alternatif yaklaşım
# Wyoming Wake Word Server - Minimal Dependencies -# Lightweight package list for Docker deployment +# Dependencies for Docker deployment +# Note: torch/torchaudio are required for audio feature extraction # Wyoming protocol SDK wyoming>=1.8.0 # Audio processing numpy>=1.26.0 + +# Feature extraction (required for mel spectrogram computation) +# These add ~2GB to the image size torch>=2.1.0 torchaudio>=2.1.0
4-30: Sürüm sabitleme (pinning) olmadan reproducibility riski.Tüm paketler
>=ile tanımlanmış. Bu esneklik sağlar ama altı ay sonra aynı Dockerfile'ı build ettiğinde farklı sürümler gelip beklenmedik sorunlar çıkarabilir.Düşün ki bir yemek tarifi var ve "biraz tuz" yazıyor - herkes farklı miktarda koyar. Aynı şekilde
>=1.8.0yarın2.0.0getirebilir ve breaking change olabilir.Production deployment için en azından major version'ları sabitlemeyi düşün:
🔎 Daha güvenli sürüm kısıtlamaları
-wyoming>=1.8.0 +wyoming>=1.8.0,<2.0.0 -numpy>=1.26.0 +numpy>=1.26.0,<2.0.0 -torch>=2.1.0 +torch>=2.1.0,<2.2.0 -pydantic>=2.5.0 +pydantic>=2.5.0,<3.0.0src/wyoming_server/__init__.py (1)
29-34:__all__listesi alfabetik olarak sıralanabilir.Merhaba! Burada küçük bir düzenleme önerisi var. Statik analiz aracı
__all__listesinin sıralı olmadığını belirtiyor. Bu, kodun okunabilirliğini artıran ve isort kurallarıyla uyumlu bir düzenleme olur.🔎 Önerilen düzenleme
__all__ = [ + "TFLiteWakeWordModel", + "WakeWordHandler", "WyomingServerConfig", - "WakeWordHandler", - "TFLiteWakeWordModel", "WyomingWakeWordServer", ]tests/test_wyoming_server.py (3)
90-91: Regex pattern için raw string kullanılmalı.Merhaba! Burada küçük bir detay var.
match=parametresine verilen pattern'de.karakteri var. Regex'te.özel bir karakter olduğu için, bu string'i raw string (r"...") olarak tanımlamak daha güvenli olur. Şimdilik çalışıyor çünkü.zaten herhangi bir karakterle eşleşir, ama niyetimizi açıkça belirtmek için:🔎 Önerilen düzenleme
- with pytest.raises(ValueError, match="must be .tflite or .onnx"): + with pytest.raises(ValueError, match=r"must be \.tflite or \.onnx"): WyomingServerConfig(model_path=model_path)
231-252: Trigger level testi eksik doğrulama içeriyor.Bu test, trigger_level davranışını göstermeyi amaçlıyor ama ikinci chunk'ın sonucu kontrol edilmiyor. Yorum "cooldown timing'e bağlı olarak tetiklenebilir veya tetiklenmeyebilir" diyor - bu, testin deterministik olmadığını gösterir.
Test'i daha güvenilir hale getirmek için:
- Cooldown'ı devre dışı bırakabilir veya
_samples_since_detectiondeğerini manuel olarak ayarlayabilirsiniz🔎 Önerilen iyileştirme
@pytest.mark.unit def test_trigger_level( self, mock_model: MagicMock, mock_config: WyomingServerConfig ) -> None: """Test trigger level (multiple consecutive detections).""" - mock_config.trigger_level = 2 # Require 2 consecutive detections + # Create handler with trigger_level=2 + handler = WakeWordHandler(model=mock_model, config=mock_config) + handler.trigger_level = 2 # Require 2 consecutive detections mock_model.predict.return_value = (0.9, 1) # High probability - handler = WakeWordHandler(model=mock_model, config=mock_config) handler.start_detection() + # Bypass cooldown for testing + handler._samples_since_detection = handler._cooldown_samples # First chunk - should not trigger yet audio1 = np.random.randint(-32768, 32767, 24000, dtype=np.int16).tobytes() result1 = handler.process_audio_chunk(audio1, sample_rate=16000) assert result1 is None # Need 2 consecutive + # Bypass cooldown again + handler._samples_since_detection = handler._cooldown_samples + # Second chunk - should trigger audio2 = np.random.randint(-32768, 32767, 24000, dtype=np.int16).tobytes() - _ = handler.process_audio_chunk(audio2, sample_rate=16000) - # Note: This may or may not trigger depending on cooldown timing - # The test demonstrates the trigger level concept + result2 = handler.process_audio_chunk(audio2, sample_rate=16000) + assert result2 is not None # Should trigger after 2 consecutive + assert result2["name"] == "hey_test"
345-362: Entegrasyon testleri henüz implement edilmemiş.Placeholder testler şu an için kabul edilebilir, ancak gerçek model dosyası ile tam pipeline testinin eklenmesi önemli olacaktır. Bu testler, Wyoming sunucusunun production ortamında doğru çalıştığından emin olmak için kritik.
Bu testleri implement etmemi ister misiniz? Gerçek bir TFLite modeli ile entegrasyon testi oluşturabilirim veya bu görev için bir issue açabilirim.
docs/TECHNICAL_GUIDE.md (1)
700-733: Kod bloklarına dil tanımlayıcısı eklenebilir.ASCII diyagramları içeren kod blokları için
textveyaplaintextdil tanımlayıcısı eklemek, markdown lint uyarılarını ortadan kaldırır ve bazı editörlerde daha iyi render edilmesini sağlar.🔎 Önerilen düzenleme örneği
-``` +```text Audio Files → AudioProcessor → FeatureExtractor → [CMVN] → [Augmentation] → FeatureCache → Dataset → BalancedSampler → DataLoader → Model → Loss → Optimizer → [EMA] → Checkpoint</details> </blockquote></details> <details> <summary>src/wyoming_server/models.py (2)</summary><blockquote> `258-286`: **Output processing mantığı kapsamlı ama küçük bir sadeleştirme yapılabilir.** Farklı model çıktı formatlarını (binary softmax, sigmoid, multi-class) düzgün şekilde işliyorsunuz. Ancak Line 283'teki kontrol gereksiz - `else` branch'inde zaten `len(output) > 2` olduğundan `len(output) > 1` her zaman true olacak. <details> <summary>🔎 Önerilen sadeleştirme</summary> ```diff else: # Multi-class (use index 1 as wake word) probabilities = self._softmax(output) - probability = float(probabilities[1]) if len(output) > 1 else float(output[0]) + probability = float(probabilities[1]) # Index 1 is wake word class predicted_class = int(np.argmax(probabilities))
340-357: YAML dosyası için context manager doğru kullanılmış, ancak hata yönetimi eklenebilir.
with open(config_path) as f:kullanımı doğru. Ancak YAML dosyasının bozuk olma ihtimaline karşı bir try-except eklemek daha güvenli olabilir.🔎 Önerilen iyileştirme
# Try to load config from associated checkpoint or config file config_path = export_path.with_suffix(".yaml") if config_path.exists(): import yaml - with open(config_path) as f: - loaded_config = yaml.safe_load(f) - if "data" in loaded_config: - data_config = loaded_config["data"] - config.update( - { - "sample_rate": data_config.get("sample_rate", config["sample_rate"]), - ... - } - ) + try: + with open(config_path) as f: + loaded_config = yaml.safe_load(f) + if loaded_config and "data" in loaded_config: + data_config = loaded_config["data"] + config.update( + { + "sample_rate": data_config.get("sample_rate", config["sample_rate"]), + ... + } + ) + except yaml.YAMLError as e: + logger.warning("Failed to load config YAML", path=str(config_path), error=str(e))src/wyoming_server/__main__.py (1)
193-198: Import hatası içinlogger.exceptionkullanmak daha bilgilendirici olur.
logger.erroryerinelogger.exceptionkullanırsanız, stack trace otomatik olarak log'a eklenir. Bu, sorunun kaynağını bulmayı kolaylaştırır.🔎 Önerilen düzenleme
# Import server try: from src.wyoming_server.server import WyomingWakeWordServer except ImportError as e: - logger.error("Failed to import Wyoming server", error=str(e)) - logger.error("Install Wyoming with: pip install wyoming") + logger.exception("Failed to import Wyoming server") + logger.info("Install Wyoming with: pip install wyoming") return 1src/wyoming_server/handler.py (2)
53-55: Buffer maxlen değeri chunk boyutuna bağlı olarak yetersiz kalabilir.
maxlen=100sabit değeri, çok küçük audio chunk'lar geldiğinde yeterli olmayabilir. Örneğin, 80ms chunk'lar (1280 sample @ 16kHz) ile 100 chunk = 8 saniye veri tutar ki bu yeterli. Ancak daha küçük chunk'lar gelirse sorun olabilir.Dinamik bir hesaplama düşünülebilir:
🔎 Alternatif yaklaşım
# Dinamik maxlen hesaplama (chunk boyutuna göre) min_chunks_needed = int(np.ceil(self.samples_needed / self.chunk_samples)) buffer_maxlen = max(100, min_chunks_needed * 2) self._audio_buffer: deque[np.ndarray] = deque(maxlen=buffer_maxlen)
258-267: Resampling fallback'i için kullanıcıyı uyarmak faydalı olabilir.Linear interpolation fallback çalışıyor ama ses kalitesini düşürebilir. Kullanıcının
resampyyüklememesi durumunda bir uyarı log'lamak, potansiyel kalite sorunlarının kaynağını anlamayı kolaylaştırır.🔎 Önerilen düzenleme
try: import resampy return resampy.resample(audio, orig_rate, target_rate) except ImportError: # Fallback to simple linear interpolation + logger.warning( + "resampy not installed, using linear interpolation (may reduce quality)", + orig_rate=orig_rate, + target_rate=target_rate, + ) ratio = target_rate / orig_rate new_length = int(len(audio) * ratio) indices = np.linspace(0, len(audio) - 1, new_length) return np.interp(indices, np.arange(len(audio)), audio)src/wyoming_server/config.py (1)
161-212: Checkpoint'ten config yükleme güvenli ama tip ipuçları eksik.Fonksiyon güvenlik açısından iyi uygulamaları takip ediyor:
- satır:
weights_only=Truearbitrary kod çalıştırılmasını önlüyor ✅- İki config formatını (object ve dict) destekliyor ✅
- Bilinmeyen formatlara karşı hata fırlatıyor ✅
Ancak iyileştirme alanları var:
Tip ipuçları: 184-189 ve 193-198 satırlarındaki config çıkarım değişkenleri tip ipuçlarından yoksun. Bu mypy/pyright hatalarına yol açabilir.
Eksik doğrulama: Fonksiyon checkpoint'teki değerlerin Wyoming sunucusu için mantıklı olduğunu doğrulamıyor. Örneğin, eğer checkpoint
audio_duration=0.96siçeriyorsa (openWakeWord uyumlu), bu Wyoming için ideal olmayabilir.🔎 Tip ipuçları ve doğrulama ekleyen önerilen düzeltme
def load_config_from_checkpoint(checkpoint_path: Path) -> WyomingServerConfig: """ Load Wyoming server configuration from a training checkpoint. Args: checkpoint_path: Path to PyTorch checkpoint file Returns: WyomingServerConfig with settings from the checkpoint """ import torch checkpoint = torch.load(str(checkpoint_path), map_location="cpu", weights_only=True) if "config" not in checkpoint: raise ValueError("Checkpoint does not contain configuration") config_data = checkpoint["config"] # Handle both dict and WakewordConfig objects if hasattr(config_data, "data"): # WakewordConfig object data_config = config_data.data - sample_rate = data_config.sample_rate - audio_duration = data_config.audio_duration - n_mels = data_config.n_mels - n_fft = data_config.n_fft - hop_length = data_config.hop_length - feature_type = data_config.feature_type + sample_rate: int = data_config.sample_rate + audio_duration: float = data_config.audio_duration + n_mels: int = data_config.n_mels + n_fft: int = data_config.n_fft + hop_length: int = data_config.hop_length + feature_type: str = data_config.feature_type elif isinstance(config_data, dict): # Dict format data_config = config_data.get("data", {}) - sample_rate = data_config.get("sample_rate", 16000) - audio_duration = data_config.get("audio_duration", 1.5) - n_mels = data_config.get("n_mels", 64) - n_fft = data_config.get("n_fft", 400) - hop_length = data_config.get("hop_length", 160) - feature_type = data_config.get("feature_type", "mel") + sample_rate: int = data_config.get("sample_rate", 16000) + audio_duration: float = data_config.get("audio_duration", 1.5) + n_mels: int = data_config.get("n_mels", 64) + n_fft: int = data_config.get("n_fft", 400) + hop_length: int = data_config.get("hop_length", 160) + feature_type: str = data_config.get("feature_type", "mel") else: raise ValueError(f"Unknown config format: {type(config_data)}") + # Log warning if config differs from Wyoming defaults + if audio_duration < 1.0: + import logging + logging.warning( + "Checkpoint audio_duration (%.2fs) is shorter than Wyoming recommended minimum (1.0s). " + "This may affect detection accuracy.", + audio_duration + ) + # We need a model path, but this is just config extraction # Return a partial config that can be completed later return WyomingServerConfig( model_path=Path("placeholder.tflite"), # Must be set by caller sample_rate=sample_rate, audio_duration=audio_duration, n_mels=n_mels, n_fft=n_fft, hop_length=hop_length, feature_type=feature_type, )Not: 205. satırdaki placeholder path sorunu ayrı bir critical issue olarak bayraklandı.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
README.mddocs/TECHNICAL_GUIDE.mdrequirements.txtsrc/training/trainer.pysrc/ui/app.pysrc/ui/panel_wyoming.pysrc/wyoming_server/Dockerfilesrc/wyoming_server/__init__.pysrc/wyoming_server/__main__.pysrc/wyoming_server/config.pysrc/wyoming_server/docker-compose.yamlsrc/wyoming_server/handler.pysrc/wyoming_server/models.pysrc/wyoming_server/requirements-wyoming.txtsrc/wyoming_server/server.pytests/test_wyoming_server.py
🧰 Additional context used
📓 Path-based instructions (6)
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.py: Always usesrc.configclasses for configuration management instead of hardcoding hyperparameters (e.g., useconfig.training.learning_raterather than raw values)
Use the project's structured logger by importing fromsrc.config.loggerwithsetup_logger(__name__)instead of using standard logging
Usepathlib.Pathfor all file and directory operations instead of string-based path handling
Enforce strict type hints throughout the codebase, especially for Pydantic models
**/*.py: Use type hints everywhere in Python code with specific type declarations
Prefer functional patterns in Python (map, filter, reduce) for transforming data sequences
Use context managers ('with' statement) in Python to ensure resources are properly cleaned up
Use dataclasses in Python to reduce boilerplate for data container definitions
In Python, always log errors with context before raising exceptions; never use silent exception handlers
Use snake_case for variable and function names in Python
**/*.py: Line length must be 120 characters (enforced by Black and flake8)
Use Black for code formatting (non-negotiable)
Use isort withprofile = "black"for import sorting
Use double quotes for strings (Black default)
Import order must follow: Future → Standard library → Third-party → First-party (src.)
Type hints are required on all function signatures
UseOptional[X]for nullable types in type hints
UseTYPE_CHECKINGguard from typing module for circular imports
Use Google-style docstrings with Args, Returns, and Raises sections
Use structlog for logging instead of print() statements
Always use pathlib.Path for file operations, never os.path
Use src.config.defaults dataclasses for configuration, never hardcoded values or raw dicts
Use custom exceptions from src.exceptions instead of generic exceptions
Never hardcode hyperparameters - always use config. attributes
Files:
src/wyoming_server/__init__.pytests/test_wyoming_server.pysrc/training/trainer.pysrc/wyoming_server/server.pysrc/wyoming_server/models.pysrc/ui/panel_wyoming.pysrc/wyoming_server/config.pysrc/ui/app.pysrc/wyoming_server/__main__.pysrc/wyoming_server/handler.py
**/*.{py,ts,tsx,cpp,h,hpp,ino}
📄 CodeRabbit inference engine (CLAUDE.md)
Use PascalCase for class names in all languages (Python, TypeScript, C++)
Files:
src/wyoming_server/__init__.pytests/test_wyoming_server.pysrc/training/trainer.pysrc/wyoming_server/server.pysrc/wyoming_server/models.pysrc/ui/panel_wyoming.pysrc/wyoming_server/config.pysrc/ui/app.pysrc/wyoming_server/__main__.pysrc/wyoming_server/handler.py
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
GPU/Device handling must check torch.cuda.is_available() and move tensors/models to device
Files:
src/wyoming_server/__init__.pysrc/training/trainer.pysrc/wyoming_server/server.pysrc/wyoming_server/models.pysrc/ui/panel_wyoming.pysrc/wyoming_server/config.pysrc/ui/app.pysrc/wyoming_server/__main__.pysrc/wyoming_server/handler.py
tests/test_*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/test_*.py: Test files must be named test_.py in the tests/ directory
Test classes must be named Test (e.g., class TestConfig:)
Test functions must be named test_* (e.g., def test_example())
Every test must include at least one pytest marker: @pytest.mark.unit, @pytest.mark.integration, @pytest.mark.slow, or @pytest.mark.gpu
Use pytest fixtures from conftest.py: default_config, device, sample_audio, tmp_path
Files:
tests/test_wyoming_server.py
src/training/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Respect
config.optimizer.mixed_precisionsetting by usingtorch.cuda.ampin training loops when enabled
src/training/**/*.py: Use torch.cuda.amp (autocast and GradScaler) for mixed precision training
Use CheckpointManager from src.training.checkpoint_manager for checkpoint operations
Files:
src/training/trainer.py
src/training/trainer.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Implement EMA (Exponential Moving Average) weight updates in training loops when enabled via configuration
Files:
src/training/trainer.py
🧠 Learnings (4)
📚 Learning: 2025-11-28T07:24:59.919Z
Learnt from: CR
Repo: sarpel/wakeword-training-platform PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T07:24:59.919Z
Learning: Applies to src/training/trainer.py : Implement EMA (Exponential Moving Average) weight updates in training loops when enabled via configuration
Applied to files:
src/training/trainer.py
📚 Learning: 2026-01-04T15:45:01.213Z
Learnt from: CR
Repo: sarpel/wakeword-training-platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-04T15:45:01.213Z
Learning: Project framework stack: PyTorch + Gradio for UI and model training
Applied to files:
src/ui/app.py
📚 Learning: 2025-11-28T07:24:59.919Z
Learnt from: CR
Repo: sarpel/wakeword-training-platform PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T07:24:59.919Z
Learning: Applies to src/training/**/*.py : Respect `config.optimizer.mixed_precision` setting by using `torch.cuda.amp` in training loops when enabled
Applied to files:
docs/TECHNICAL_GUIDE.md
📚 Learning: 2026-01-04T15:45:01.213Z
Learnt from: CR
Repo: sarpel/wakeword-training-platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-04T15:45:01.213Z
Learning: Applies to src/training/**/*.py : Use torch.cuda.amp (autocast and GradScaler) for mixed precision training
Applied to files:
docs/TECHNICAL_GUIDE.md
🧬 Code graph analysis (8)
tests/test_wyoming_server.py (3)
src/wyoming_server/config.py (3)
WyomingServerConfig(13-137)uri(119-121)samples_needed(124-126)src/wyoming_server/handler.py (7)
WakeWordHandler(20-289)is_detecting(270-272)start_detection(73-87)stop_detection(89-94)process_audio_chunk(102-187)_bytes_to_audio(189-211)get_status(279-289)src/wyoming_server/models.py (4)
predict(214-234)TFLiteWakeWordModel(19-310)_softmax(289-292)_sigmoid(295-297)
src/training/trainer.py (2)
src/training/qat_utils.py (1)
convert_model_to_quantized(192-205)src/data/augmentation.py (2)
set_epoch(347-364)SpecAugment(595-642)
src/wyoming_server/server.py (4)
src/wyoming_server/config.py (3)
WyomingModelInfo(140-158)WyomingServerConfig(13-137)uri(119-121)src/wyoming_server/handler.py (6)
WakeWordHandler(20-289)start_detection(73-87)reset(96-100)process_audio_chunk(102-187)stop_detection(89-94)get_status(279-289)src/wyoming_server/models.py (2)
TFLiteWakeWordModel(19-310)get_model_info(299-310)src/wyoming_server/__main__.py (1)
run(239-248)
src/ui/panel_wyoming.py (3)
src/wyoming_server/server.py (1)
is_running(337-339)src/wyoming_server/config.py (1)
validate_model_path(100-107)src/data/file_cache.py (1)
get(82-110)
src/wyoming_server/config.py (2)
src/ui/panel_wyoming.py (1)
validate_model_path(68-88)src/data/file_cache.py (1)
get(82-110)
src/ui/app.py (1)
src/ui/panel_wyoming.py (1)
create_wyoming_deployment_panel(565-575)
src/wyoming_server/__main__.py (1)
src/wyoming_server/server.py (3)
WyomingWakeWordServer(158-350)run(296-327)stop(329-334)
src/wyoming_server/handler.py (3)
src/wyoming_server/config.py (3)
WyomingServerConfig(13-137)samples_needed(124-126)chunk_samples(129-131)src/wyoming_server/models.py (2)
TFLiteWakeWordModel(19-310)predict(214-234)src/wyoming_server/server.py (1)
get_status(341-350)
🪛 GitHub Actions: Documentation Quality
README.md
[warning] 15-15: Broken internal link: 📘 Complete Guide
[warning] 18-18: Broken internal link: ⚙️ Presets
[warning] 41-41: Broken internal link: DOCUMENTATION.md
[warning] 93-93: Broken internal link: Architecture Deep Dive
🪛 GitHub Check: CodeQL
src/ui/panel_wyoming.py
[failure] 82-82: Uncontrolled data used in path expression
This path depends on a user-provided value.
[failure] 177-177: Uncontrolled command line
This command line depends on a user-provided value.
This command line depends on a user-provided value.
This command line depends on a user-provided value.
This command line depends on a user-provided value.
This command line depends on a user-provided value.
This command line depends on a user-provided value.
This command line depends on a user-provided value.
This command line depends on a user-provided value.
This command line depends on a user-provided value.
This command line depends on a user-provided value.
This command line depends on a user-provided value.
This command line depends on a user-provided value.
🪛 markdownlint-cli2 (0.18.1)
README.md
145-145: Multiple spaces after hash on atx style heading
(MD019, no-multiple-space-atx)
docs/TECHNICAL_GUIDE.md
701-701: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
717-717: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
737-737: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
746-746: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
756-756: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🪛 Ruff (0.14.10)
src/wyoming_server/__init__.py
29-34: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
tests/test_wyoming_server.py
90-90: Pattern passed to match= contains metacharacters but is neither escaped nor raw
(RUF043)
src/wyoming_server/server.py
172-172: Possible binding to all interfaces
(S104)
203-205: Avoid specifying long messages outside the exception class
(TRY003)
src/wyoming_server/models.py
66-66: Avoid specifying long messages outside the exception class
(TRY003)
88-88: Avoid specifying long messages outside the exception class
(TRY003)
100-103: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
100-103: Avoid specifying long messages outside the exception class
(TRY003)
126-128: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
126-128: Avoid specifying long messages outside the exception class
(TRY003)
src/ui/panel_wyoming.py
176-176: subprocess call: check for execution of untrusted input
(S603)
200-200: Consider moving this statement to an else block
(TRY300)
204-204: Use explicit conversion flag
Replace with conversion flag
(RUF010)
239-239: Do not catch blind exception: Exception
(BLE001)
339-358: Possible SQL injection vector through string-based query construction
(S608)
433-433: Possible binding to all interfaces
(S104)
511-511: Unpacked variable is_valid is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
565-565: Unused function argument: state
(ARG001)
src/wyoming_server/config.py
78-78: Possible binding to all interfaces
(S104)
104-104: Avoid specifying long messages outside the exception class
(TRY003)
106-106: Avoid specifying long messages outside the exception class
(TRY003)
115-115: Avoid specifying long messages outside the exception class
(TRY003)
176-176: Avoid specifying long messages outside the exception class
(TRY003)
200-200: Avoid specifying long messages outside the exception class
(TRY003)
src/ui/app.py
29-29: Unused noqa directive (non-enabled: E402)
Remove unused noqa directive
(RUF100)
src/wyoming_server/__main__.py
100-100: Possible binding to all interfaces
(S104)
196-196: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
197-197: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
src/wyoming_server/handler.py
209-209: Avoid specifying long messages outside the exception class
(TRY003)
🔍 Remote MCP GitHub Copilot
Now I have comprehensive information about the PR. Let me compile the relevant context for the review.
PR Review Context Summary
Pull Request Overview
- PR #25 (v11): Comprehensive enhancement integrating Wyoming Protocol Server for Home Assistant alongside substantial training improvements
- 40 commits, +2,941 additions / -98 deletions across 16 changed files
- Related PR #1 (merged): Prior work on GPU-accelerated augmentation, Optuna HPO, W&B integration, and LR Finder
Key Components & Notable Changes
1. Wyoming Protocol Server (New Integration)
Files: src/wyoming_server/ directory (8 new modules + Docker support)
Critical Implementation Details:
- Configuration (
config.py): Pydantic-basedWyomingServerConfigwith validation for model paths, feature types, threshold bounds (0.0-1.0) - Model Loading (
models.py): Dual TFLite/ONNX support (~370 lines) with fallback resampling for non-16kHz audio; usesresampywith linear interpolation fallback - Stream Handler (
handler.py):WakeWordHandlerimplements:- Configurable trigger level (consecutive detections required)
- 500ms cooldown post-detection
- Audio buffering with automatic mono conversion and resampling
- Proper cleanup with buffer trimming to prevent unbounded growth
- Server (
server.py): Async Wyoming protocol server withWyomingWakeWordEventHandlerfor protocol event processing - CLI (
__main__.py): 252-line entry point with structured logging setup and comprehensive argument parsing - Docker: Multi-stage Dockerfile (Python 3.10-slim) with health checks and resource limits
Port: Fixed to 10400 (Wyoming standard)
Dependencies: New requirements-wyoming.txt with conditional TFLite (tflite-runtime or tensorflow), ONNX runtime, resampy fallback, structlog, pydantic
2. Documentation Updates
- README.md: Version bumped to v4.1 "Wyoming Protocol Release"; added Wyoming quick-start guide with Docker deployment
- TECHNICAL_GUIDE.md: Major restructuring — sections renumbered; 200+ lines added for Wyoming server documentation (Section 9) with architecture diagrams, config tables, troubleshooting
3. UI Integration
- panel_wyoming.py (NEW, 575 lines): Full Gradio-based Wyoming deployment panel featuring:
- Model selection dropdown with auto-refresh
- Server start/stop controls with graceful process management
- Docker Compose and Home Assistant config generators
- Real-time server status display
- Global
WyomingServerStatefor subprocess management
- app.py: Panel count increased from 6 to 7; new "🏠 6. Home Assistant" tab with Wyoming panel; Documentation shifted to tab 7
4. Core Training Enhancements
- trainer.py (-80, +431 net): Multiple stylistic refactors collapsing multi-line statements to single lines; SpecAugment moved to device with
.to(self.device); progressive scheduler integration; QAT preparation for quantized models; improved error handling - evaluator.py (+64, -11): Raw audio evaluation method (
evaluate_audio()) added; QAT model preparation during loading; f-string logging converted to %-style - presets.py (+674, -318): Restructured from 6 to 8 presets (3 hardware targets + 5 training strategies); new
get_homeassistant_preset()for 32 mel bands and TFLite export
5. Data & Feature Extraction
- panel_dataset.py (+197, -208): Multi-augmentation controls (1-10x multiplier); defaults updated: n_mels 64→32 (openWakeWord compatibility), n_fft 400→512, audio_duration 1.5s
- panel_config.py (+387, -93): Added mixup parameters (
use_mixup,mixup_alpha,mixup_prob), attention option (use_attention); updated TCN channels, focal alpha defaults
Key Review Concerns
HIGH PRIORITY:
- Thread Safety in UI: Panel subprocess management (
WyomingServerState) uses class-level mutable state; concurrent access during start/stop requires verification - Model Loading Fallback: ONNX/TFLite fallback chain relies on ImportError catching; ensure graceful degradation messaging
- Audio Resampling: Linear interpolation fallback in handler may introduce artifacts; consider quality loss impact on edge devices
- Async Event Loop: Windows-specific event loop policy set in
__main__.py— test for compatibility
MEDIUM PRIORITY:
5. Configuration Validation: Pydantic model forbids extra fields (extra="forbid"); verify all upstream code provides exactly expected fields
6. Dependency Conflicts: Wyoming SDK (>=1.8.0) may conflict with existing structlog/torch versions; ensure tested compatibility matrix
7. Docker Image Size: Python 3.10-slim + torch creates ~3GB+ image; no multi-stage optimization
LOW PRIORITY:
8. Style consistency: Multi-line refactoring in trainer.py is mechanical but increases diff complexity; confirm no semantic changes
9. Documentation link consistency: Wyoming Server Guide references Section 9, but renumbering may shift numbering
Related PR Context
PR #1 (merged) laid groundwork with GPU-accelerated augmentation, Optuna HPO, W&B integration, and LR Finder. PR #25 builds atop this with production-ready deployment (Wyoming), advanced training (QAT, progressive scheduling), and robust defaults (openWakeWord compatibility).
⏰ 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). (3)
- GitHub Check: Seer Code Review
- GitHub Check: claude-review
- GitHub Check: review
🔇 Additional comments (28)
src/training/trainer.py (8)
183-189: Kod mantığı doğru ve okunabilir.cuDNN hatası için uygulanan workaround (geçici çözüm) mantıklı. Loglama ifadesinin tek satıra indirilmesi de okunabilirliği koruyarak kod stilini iyileştirmiş.
203-214: torch.compile entegrasyonu iyi uygulanmış.
cast(nn.Module, torch.compile(...))kullanımı tip güvenliğini sağlıyor. Exception handling (hata yakalama) çok kapsamlı ve üretim ortamı için uygun. Tek satıra indirme işlemi de bu örnekte okunabilirliği bozmamış.
326-332: Önemli device placement iyileştirmesi!SpecAugment'e
.to(self.device)eklenmesi kritik bir düzeltme. GPU-tabanlı augmentation için, modülün doğru cihaza (GPU/CPU) taşınması gerekiyor. Bu değişiklik performans ve doğruluk açısından önemli.Bu tür device placement'ları kodlama rehberine uygun ve eğitim sırasında hata riskini azaltıyor.
354-372: Progressive training entegrasyonu doğru kurulmuş.
ProgressiveSchedulerkurulumu temiz ve yapılandırılabilir. Üç ayrı progression türü (duration, augmentation, difficulty) bağımsız olarak kontrol edilebiliyor. Bu esnek bir tasarım.Tek satıra indirilen
getattrçağrıları da bu örnekte okunabilirliği koruyarak kod yoğunluğunu azaltmış.
413-420: Augmentation parametreleri doğru güncelleniyor.Ternary operator (koşullu ifade) kullanımı bu durumda uygun:
noise_snrvarsa probability kullan, yoksa 0.0. Kod yoğunluğunu azaltırken mantığı net tutuyor.
491-503: Epoch başlangıç loglama doğru formatta.Her iki logger çağrısı da structlog için doğru
%-styleformatını kullanıyor. Epoch numarası ve dynamic alpha bilgileri net şekilde loglanıyor. Tek satır formatı da burada okunaklı.
708-762: Progressive training uygulaması düzgün entegre edilmiş.Progressive scheduler'ın epoch güncellemesi, augmentation tier değişiklikleri ve SpecAugment toggle mantığı doğru. Özellikle satır 758'de
.to(self.device)kullanımı (satır 331'de olduğu gibi) önemli - yeniden oluşturulan SpecAugment modülünün doğru device'a taşınmasını garanti ediyor.Cast operatörleri de tip güvenliği sağlıyor ve mypy gibi static type checker'ların mutlu olmasını sağlıyor.
636-658: QAT (Quantization-Aware Training) entegrasyonu profesyonel.Quantization (nicemleme) sürecinde modelin CPU'ya taşınması (satır 646) doğru bir uygulama - PyTorch'un quantization API'si CPU'da daha stabil çalışıyor.
convert_model_to_quantizedçağrısı da tip güvenli şekilde cast edilmiş.Hata durumunda bile program çökmüyor, sadece uyarı veriyor (satır 658). Bu production-ready (üretime hazır) bir yaklaşım.
README.md (1)
97-142: Wyoming Protocol bölümü çok iyi hazırlanmış!Bu bölüm bir öğretici gibi düzenlenmiş - önce hızlı başlangıç, sonra Docker deployment, ardından özellikler tablosu ve yapılandırma seçenekleri. Kullanıcılar adım adım takip edebilir.
Özellikle beğendiğim noktalar:
- Kod blokları açık ve kopyalanabilir
- Tablo formatı özellikleri özetliyor
- Hem manuel hem Docker yöntemi sunulmuş
src/wyoming_server/Dockerfile (2)
35-37: Healthcheck iyi düşünülmüş!Socket bağlantısı basit ve etkili bir healthcheck yöntemi. Timing değerleri makul:
- 30s interval: Çok sık değil
- 10s timeout: Yeterli süre
- 5s start_period: Servisin başlaması için zaman
39-43: ENTRYPOINT ve CMD ayrımı doğru kullanılmış.
ENTRYPOINTsabit kısmı tanımlarken,CMDvarsayılan argümanları sağlıyor. Bu sayede kullanıcılardocker runkomutunda sadece argümanları override edebilir:docker run wyoming-wakeword --model /custom/model.tflite --name my_wakewordsrc/wyoming_server/docker-compose.yaml (2)
46-52: Resource limitleri makul seviyede.512MB memory limit çoğu wake word modeli için yeterli olacaktır. TFLite modelleri genellikle küçüktür ve inference işlemi hafif bellek kullanır.
256MB reservation da container'ın minimum ihtiyacını garanti altına alıyor.
Eğer daha büyük ONNX modelleri kullanılacaksa bu değerler artırılabilir ama şimdilik gayet uygun.
54-59: Log rotation konfigürasyonu disk dolmasını önlüyor.10MB max dosya boyutu ve 3 dosya rotasyonu, toplamda maksimum 30MB log alanı kullanılacak demek. Bu, uzun süreli deployment'larda disk alanı sorununu önler.
src/wyoming_server/__init__.py (1)
1-27: Modül dokümantasyonu ve import yapısı güzel görünüyor!Docstring'de hem CLI kullanımı hem de programatik kullanım örnekleri verilmiş. Bu, diğer geliştiricilerin modülü nasıl kullanacağını anlamasını kolaylaştırır. İyi iş! 👍
tests/test_wyoming_server.py (1)
1-16: Test yapısı ve fixture'lar iyi organize edilmiş!Test dosyası coding guidelines'a uygun şekilde yapılandırılmış:
- ✅ Dosya adı
test_*.pyformatında- ✅ Test sınıfları
Test*ile başlıyor- ✅ Test fonksiyonları
test_*ile başlıyor- ✅ Pytest marker'ları kullanılmış (
@pytest.mark.unit,@pytest.mark.integration)docs/TECHNICAL_GUIDE.md (1)
707-896: Wyoming Protocol Server dokümantasyonu kapsamlı ve faydalı!Bu bölüm çok iyi hazırlanmış:
- ✅ Mimari diyagramı açık ve anlaşılır
- ✅ Quick start adımları detaylı
- ✅ Konfigürasyon parametreleri tablo halinde
- ✅ Docker deployment ve Home Assistant yapılandırması dahil
- ✅ Troubleshooting tablosu pratik sorunları kapsıyor
Yeni başlayanlar için bile anlaşılabilir bir rehber olmuş! 👏
src/wyoming_server/models.py (2)
12-16: Proje logger'ı yerine doğrudan structlog kullanılmış.Coding guidelines'a göre,
src.config.loggermodülündensetup_logger(__name__)kullanılması öneriliyor. Ancak Wyoming server modülü bağımsız bir bileşen olarak tasarlandığından, bu kabul edilebilir bir tercih olabilir.Wyoming server modülünün standalone çalışabilmesi için bu tercih yapılmış olabilir. Proje genelinde tutarlılık istiyorsanız değiştirebilirsiniz:
# Alternatif: Proje logger'ını kullanmak için from src.config.logger import setup_logger logger = setup_logger(__name__)
177-212: Feature extraction implementasyonu doğru görünüyor.Ses verisi torch tensor'una dönüştürülüyor, özellik çıkarımı yapılıyor ve model girdisi için uygun şekle getiriliyor. Mel spectrogram ve MFCC arasında seçim yapılabilmesi esneklik sağlıyor. 👍
src/wyoming_server/__main__.py (2)
226-236: Hata yönetimi ve cleanup yapısı doğru implement edilmiş!
try/except/finallyyapısı sayesinde:
KeyboardInterruptdüzgün yakalanıyor- Her durumda
server.stop()çağrılıyor- Exception'lar log'lanıyor ve uygun exit code döndürülüyor
Bu, production-ready bir CLI için gerekli olan robust error handling. 👍
239-252: Windows uyumluluğu için event loop policy doğru ayarlanmış.Windows'ta
asyncio.run()kullanırkenWindowsSelectorEventLoopPolicyayarlamak, bazı async I/O sorunlarını önler. Bu, cross-platform uyumluluğu için önemli bir detay.src/wyoming_server/handler.py (2)
102-187: Audio processing akışı iyi tasarlanmış!Bu method şunları düzgün şekilde yönetiyor:
- ✅ Bytes → float array dönüşümü
- ✅ Sample rate uyumsuzluğu (resampling)
- ✅ Stereo → mono dönüşümü
- ✅ Sliding window buffering
- ✅ Cooldown (çift tetiklemeyi önleme)
- ✅ Trigger level (ardışık detection gerekliliği)
- ✅ Buffer trimming (memory leak önleme)
Streaming audio processing için tüm edge case'ler düşünülmüş. 👏
189-211: PCM dönüşümü 8-bit audio'yu desteklemiyor.16-bit ve 32-bit audio destekleniyor ama 8-bit (sample_width=1) desteklenmiyor. Wyoming protokolünde 8-bit audio nadir olsa da, daha açıklayıcı bir hata mesajı veya support eklenebilir.
Wyoming protokolünde tipik olarak hangi audio formatları kullanılıyor? Eğer sadece 16-bit standard ise, mevcut implementasyon yeterli.
src/wyoming_server/server.py (2)
67-151: Wyoming protokol event handling doğru implement edilmiş.Event handler tüm Wyoming event tiplerini düzgün şekilde işliyor:
Describe→ Server info döndürürDetect→ Detection modunu başlatırAudioStart→ Handler'ı resetlerAudioChunk→ Audio'yu işler ve detection varsa bildirirAudioStop→ Detection'ı durdurur,NotDetectedgönderirBilinmeyen event tipleri warning olarak loglanıyor ama bağlantıyı kesmiyor - bu doğru yaklaşım.
353-411: Helper fonksiyonlar kullanışlı ve doğru implement edilmiş.
create_server_from_config()config nesnesinden server oluşturmayı kolaylaştırıyor.run_server()ise hızlı kurulum için pratik bir convenience function. Her ikisi de API'nin kullanımını basitleştiriyor.src/wyoming_server/config.py (4)
118-131: LGTM! Hesaplanmış özellikler doğru.Üç özellik de mantıksal olarak doğru:
uri: Wyoming protokolü TCP URI formatını düzgün oluşturuyorsamples_needed: Çıkarım için gerekli ses örneği sayısını doğru hesaplıyor (örnek: 16000 Hz × 1.5s = 24000 örnek)chunk_samples: Tamponlama için parça başına örnek sayısını doğru hesaplıyor (örnek: 16000 Hz × 80ms ÷ 1000 = 1280 örnek)Salt okunur özellikler olarak kalmaları uygun.
140-158: LGTM! Model metadata yapısı temiz.
WyomingModelInfomodeli, wake word modellerinin metadata'sını temsil etmek için iyi tasarlanmış. Attribution alanları (author,url) ve opsiyonel teknik detaylar (architecture,file_size_kb) içeriyor. Bu muhtemelen model kataloğu veya kayıt amaçlı kullanılıyor.Varsayılan değerler mantıklı ve yapı Pydantic best practice'lerine uygun.
58-74: Varsayılan değerler eğitim yapılandırmasıyla uyuşuyor - revizyon gerekli değil.Sorun incelemesi tamamlandı: Eğitim yapılandırması varsayılanları (
src/config/defaults.py) n_mels=64 ve n_fft=400 olarak ayarlanmıştır. Wyoming sunucusu yapılandırması (src/wyoming_server/config.py) da tam olarak aynı değerleri kullanmaktadır. Yani "eğitim yapılandırmasıyla eşleşmeli" yorumu doğru şekilde uygulanmıştır.PR özeti içinde bahsedilen n_mels=32, n_fft=512 değerleri belirli ön ayarlar (kenar cihazlar için) içindir; ana eğitim varsayılanları değildir. Özet önceden varsayılan değerleri değiştirmemiştir.
Likely an incorrect or invalid review comment.
133-137:extra="forbid"ayarı açık alan geçişi örüntüleriyle güvenli.Kodda WyomingServerConfig kullanımlarını inceledim ve hiçbir yerde dict unpacking (**kwargs) veya dış kaynak yapılandırma yükleme bulunmuyor. Tüm instantiationlar—testler, server.py ve config.py'da—sadece bilinen alanları açıkça geçiyor.
Bu demek oluyor ki
extra="forbid"aslında koruyucu bir uygulamadır. Birinin yanlışlıkla yanlış bir alan adı yazması durumunda (örneğinthreholdyerinethreshold), Pydantic bunu hemen yakalar ve hata verir. Çoğu insanın düşündüğünün aksine, bu sıkı doğrulama burada problemli değil—aksine iyi tasarım.Checkpoint'lerden veya UI'den gelen ekstra alanlar hakkındaki endişe kodda hayata geçmediği için önemli değil.
Likely an incorrect or invalid review comment.
|
|
||
| --- | ||
|
|
||
| ## What's New in v4.1 |
There was a problem hiding this comment.
Markdown formatı düzeltilmeli - başlıktan sonra fazla boşluk var.
Static analysis aracı burada bir sorun tespit etmiş. ## işaretinden sonra sadece bir boşluk olmalı, iki değil. Bu küçük bir şey ama tutarlılık için düzeltilmeli.
🔎 Önerilen düzeltme
-## What's New in v4.1
+## What's New in v4.1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## What's New in v4.1 | |
| ## What's New in v4.1 |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
145-145: Multiple spaces after hash on atx style heading
(MD019, no-multiple-space-atx)
🤖 Prompt for AI Agents
In README.md around line 145, the Markdown header has two spaces after the "##"
("## What's New in v4.1"); change it to a single space ("## What's New in
v4.1") so the header follows standard Markdown spacing conventions.
| logger.info(f"\n🏆 BEST F1 SCORE: {best_f1_metrics.f1_score:.4f} ⭐ (Epoch {best_f1_epoch + 1})") | ||
| if best_fpr_metrics and best_fpr_epoch + 1 > 1: | ||
| logger.info( | ||
| f"BEST FPR: {best_fpr_metrics.fpr:.4f} (Epoch {best_fpr_epoch + 1})" | ||
| ) | ||
| logger.info(f"BEST FPR: {best_fpr_metrics.fpr:.4f} (Epoch {best_fpr_epoch + 1})") |
There was a problem hiding this comment.
Loglama standardına uymayan f-string kullanımı.
Bu iki satırda f-string kullanılmış, ancak proje standartları ve PR hedefleri structlog için %-style formatını gerektiriyor. Dosyanın geri kalanı (satır 491, 503, 561, 596) doğru formatı kullanıyor, bu satırlar tutarsızlık yaratmış.
F-string'ler structlog'un yapılandırılmış loglama özelliklerinden faydalanamaz ve log toplama sistemleri tarafından parse edilemez.
🔎 Önerilen düzeltme
if best_f1_metrics:
- logger.info(f"\n🏆 BEST F1 SCORE: {best_f1_metrics.f1_score:.4f} ⭐ (Epoch {best_f1_epoch + 1})")
+ logger.info("\n🏆 BEST F1 SCORE: %.4f ⭐ (Epoch %d)", best_f1_metrics.f1_score, best_f1_epoch + 1)
if best_fpr_metrics and best_fpr_epoch + 1 > 1:
- logger.info(f"BEST FPR: {best_fpr_metrics.fpr:.4f} (Epoch {best_fpr_epoch + 1})")
+ logger.info("BEST FPR: %.4f (Epoch %d)", best_fpr_metrics.fpr, best_fpr_epoch + 1)🤖 Prompt for AI Agents
In src/training/trainer.py around lines 620 to 622, two logger calls use
f-strings which violate the project's structlog %-style logging standard;
replace these f-strings with %-style formatted logging calls and pass values as
arguments (e.g., use "BEST F1 SCORE: %.4f (Epoch %d)" and "BEST FPR: %.4f (Epoch
%d)" with corresponding metric and epoch arguments), remove the leading newline
from the message or preserve it consistently with other logs, and ensure both
calls follow the same structured logging pattern used elsewhere in the file.
| from src.ui.panel_evaluation import create_evaluation_panel # noqa: E402 | ||
| from src.ui.panel_export import create_export_panel # noqa: E402 | ||
| from src.ui.panel_training import create_training_panel # noqa: E402 | ||
| from src.ui.panel_wyoming import create_wyoming_deployment_panel # noqa: E402 |
There was a problem hiding this comment.
Gereksiz noqa direktifi kaldırılabilir.
Ruff diyor ki bu # noqa: E402 direktifi gereksiz. Düşün ki bir trafik levhası var ama o yolda zaten trafik yok - levha boşuna duruyor.
Bu satırdaki import zaten sorun çıkarmıyor, o yüzden noqa direktifi kaldırılabilir.
🔎 Önerilen düzeltme
-from src.ui.panel_wyoming import create_wyoming_deployment_panel # noqa: E402
+from src.ui.panel_wyoming import create_wyoming_deployment_panel📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from src.ui.panel_wyoming import create_wyoming_deployment_panel # noqa: E402 | |
| from src.ui.panel_wyoming import create_wyoming_deployment_panel |
🧰 Tools
🪛 Ruff (0.14.10)
29-29: Unused noqa directive (non-enabled: E402)
Remove unused noqa directive
(RUF100)
🤖 Prompt for AI Agents
In src/ui/app.py around line 29, the import line includes an unnecessary "#
noqa: E402" directive; remove the trailing noqa comment so the import reads
simply from src.ui.panel_wyoming import create_wyoming_deployment_panel, then
save the file (no other code changes needed).
| class WyomingServerState: | ||
| """Global state for Wyoming server management.""" | ||
|
|
||
| def __init__(self) -> None: | ||
| self.server_process: Optional[subprocess.Popen] = None | ||
| self.server_thread: Optional[threading.Thread] = None | ||
| self.is_running: bool = False | ||
| self.last_status: str = "Not started" | ||
| self.config: Dict[str, Any] = {} | ||
|
|
||
|
|
||
| # Global state | ||
| wyoming_state = WyomingServerState() |
There was a problem hiding this comment.
Thread safety endişesi: Global state concurrent erişime karşı korumasız.
Düşün ki iki kişi aynı anda bir kapıdan geçmeye çalışıyor - çarpışma olabilir. Burada wyoming_state global değişkeni birden fazla Gradio callback'i tarafından aynı anda değiştirilebilir.
Özellikle is_running ve server_process alanları race condition'a açık. Örneğin: Kullanıcı hızlıca "Start" ve "Stop" butonlarına basarsa beklenmedik davranışlar oluşabilir.
🔎 Thread-safe alternatif
+import threading
+
class WyomingServerState:
"""Global state for Wyoming server management."""
def __init__(self) -> None:
+ self._lock = threading.Lock()
self.server_process: Optional[subprocess.Popen] = None
self.server_thread: Optional[threading.Thread] = None
self.is_running: bool = False
self.last_status: str = "Not started"
self.config: Dict[str, Any] = {}
+
+ def start_server(self, process: subprocess.Popen, config: Dict[str, Any]) -> bool:
+ """Thread-safe server start."""
+ with self._lock:
+ if self.is_running:
+ return False
+ self.server_process = process
+ self.is_running = True
+ self.config = config
+ return True
+
+ def stop_server(self) -> Optional[subprocess.Popen]:
+ """Thread-safe server stop - returns process to terminate."""
+ with self._lock:
+ if not self.is_running:
+ return None
+ process = self.server_process
+ self.server_process = None
+ self.is_running = False
+ self.config = {}
+ return process🤖 Prompt for AI Agents
In src/ui/panel_wyoming.py around lines 26 to 38, the global wyoming_state is
not thread-safe (race conditions on is_running and server_process) and must be
protected for concurrent Gradio callbacks; add a threading.Lock (or
threading.RLock) as an attribute on WyomingServerState, initialize it in
__init__, and change all accesses and mutations of shared fields (is_running,
server_process, server_thread, last_status, config) to acquire the lock (use
with self.lock:) for both readers and writers; ensure external code that touches
wyoming_state also uses the lock when performing multi-step checks/updates
(e.g., check-then-start or check-then-stop) to avoid TOCTOU races.
| def validate_model_path(model_path: str) -> Tuple[bool, str]: | ||
| """ | ||
| Validate that model path exists and is valid. | ||
| Args: | ||
| model_path: Path to model file | ||
| Returns: | ||
| Tuple of (is_valid, message) | ||
| """ | ||
| if model_path == "No models available - export a model first": | ||
| return False, "❌ No models available. Export a model first in Panel 5." | ||
|
|
||
| path = Path(model_path) | ||
| if not path.exists(): | ||
| return False, f"❌ Model file not found: {model_path}" | ||
|
|
||
| if path.suffix.lower() not in (".tflite", ".onnx"): | ||
| return False, f"❌ Invalid model format: {path.suffix}" | ||
|
|
||
| return True, f"✅ Valid model: {path.name}" |
There was a problem hiding this comment.
Kritik: Path traversal güvenlik açığı düzeltilmeli.
Düşün ki bir kasa var ve şifre olarak ../../../etc/passwd yazabiliyorsun - bu durumda sistemdeki herhangi bir dosyaya erişebilirsin. Burada da benzer bir durum var. Kullanıcı model_path olarak ../../sensitive_file gibi bir şey gönderebilir.
CodeQL bu satırı işaretlemiş ve haklı. Model yolunun güvenli bir dizin içinde kalmasını sağlamalısın.
🔎 Önerilen güvenlik düzeltmesi
def validate_model_path(model_path: str) -> Tuple[bool, str]:
"""
Validate that model path exists and is valid.
"""
if model_path == "No models available - export a model first":
return False, "❌ No models available. Export a model first in Panel 5."
path = Path(model_path)
+
+ # Security: Prevent path traversal attacks
+ allowed_dirs = [Path("exports").resolve(), Path("models/exports").resolve()]
+ try:
+ resolved_path = path.resolve()
+ if not any(
+ resolved_path == allowed_dir or allowed_dir in resolved_path.parents
+ for allowed_dir in allowed_dirs
+ ):
+ return False, "❌ Model must be in exports/ or models/exports/ directory"
+ except (OSError, ValueError):
+ return False, "❌ Invalid model path"
+
if not path.exists():
return False, f"❌ Model file not found: {model_path}"
if path.suffix.lower() not in (".tflite", ".onnx"):
return False, f"❌ Invalid model format: {path.suffix}"
return True, f"✅ Valid model: {path.name}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def validate_model_path(model_path: str) -> Tuple[bool, str]: | |
| """ | |
| Validate that model path exists and is valid. | |
| Args: | |
| model_path: Path to model file | |
| Returns: | |
| Tuple of (is_valid, message) | |
| """ | |
| if model_path == "No models available - export a model first": | |
| return False, "❌ No models available. Export a model first in Panel 5." | |
| path = Path(model_path) | |
| if not path.exists(): | |
| return False, f"❌ Model file not found: {model_path}" | |
| if path.suffix.lower() not in (".tflite", ".onnx"): | |
| return False, f"❌ Invalid model format: {path.suffix}" | |
| return True, f"✅ Valid model: {path.name}" | |
| def validate_model_path(model_path: str) -> Tuple[bool, str]: | |
| """ | |
| Validate that model path exists and is valid. | |
| Args: | |
| model_path: Path to model file | |
| Returns: | |
| Tuple of (is_valid, message) | |
| """ | |
| if model_path == "No models available - export a model first": | |
| return False, "❌ No models available. Export a model first in Panel 5." | |
| path = Path(model_path) | |
| # Security: Prevent path traversal attacks | |
| allowed_dirs = [Path("exports").resolve(), Path("models/exports").resolve()] | |
| try: | |
| resolved_path = path.resolve() | |
| if not any( | |
| resolved_path == allowed_dir or allowed_dir in resolved_path.parents | |
| for allowed_dir in allowed_dirs | |
| ): | |
| return False, "❌ Model must be in exports/ or models/exports/ directory" | |
| except (OSError, ValueError): | |
| return False, "❌ Invalid model path" | |
| if not path.exists(): | |
| return False, f"❌ Model file not found: {model_path}" | |
| if path.suffix.lower() not in (".tflite", ".onnx"): | |
| return False, f"❌ Invalid model format: {path.suffix}" | |
| return True, f"✅ Valid model: {path.name}" |
🧰 Tools
🪛 GitHub Check: CodeQL
[failure] 82-82: Uncontrolled data used in path expression
This path depends on a user-provided value.
🤖 Prompt for AI Agents
In src/ui/panel_wyoming.py around lines 68 to 88, the current
validate_model_path allows path traversal (e.g. ../../etc/passwd); change the
function to enforce that model_path resolves inside a configured models
directory: resolve both the models base directory (e.g. MODELS_DIR) and the
provided path with Path.resolve(strict=False), reject if the resolved path is
not a child of the base (use pathlib.Path.commonpath or compare
resolved_path.parts[:len(base.parts)]), also reject symlinks that escape the
base (resolve with strict=True where appropriate), keep existing checks for
existence and suffix (.tflite, .onnx) only after confirming containment, and
return a clear error message when the path is outside the allowed directory.
| try: | ||
| # Try tflite_runtime first (lighter weight) | ||
| import tflite_runtime.interpreter as tflite # type: ignore | ||
| except ImportError: | ||
| try: | ||
| # Fall back to full TensorFlow | ||
| import tensorflow.lite as tflite # type: ignore | ||
| except ImportError: | ||
| raise ImportError( | ||
| "TFLite runtime required. Install with: " | ||
| "pip install tflite-runtime or pip install tensorflow" | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
raise ... from err kullanarak hata zincirini koruyun.
Merhaba! Burada önemli bir best practice var. Bir except bloğu içinde yeni bir exception raise ederken, from err eklemek orijinal hatanın stack trace'ini korur. Bu, debugging sırasında hatanın gerçek kaynağını bulmayı kolaylaştırır.
🔎 Önerilen düzenleme
def _load_tflite_model(self) -> None:
"""Load TFLite model."""
try:
# Try tflite_runtime first (lighter weight)
import tflite_runtime.interpreter as tflite # type: ignore
except ImportError:
try:
# Fall back to full TensorFlow
import tensorflow.lite as tflite # type: ignore
- except ImportError:
+ except ImportError as err:
raise ImportError(
"TFLite runtime required. Install with: "
"pip install tflite-runtime or pip install tensorflow"
- )
+ ) from err🧰 Tools
🪛 Ruff (0.14.10)
100-103: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
100-103: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In src/wyoming_server/models.py around lines 92-103, the current ImportError
raised when both tflite_runtime and tensorflow.lite imports fail loses the
original exception context; update the except handlers to capture the original
exception (e.g., except ImportError as err) and re-raise the new ImportError
using "raise ImportError(...) from err" so the original stack trace and error
message are preserved for debugging.
| def _load_onnx_model(self) -> None: | ||
| """Load ONNX model.""" | ||
| try: | ||
| import onnxruntime as ort # type: ignore | ||
| except ImportError: | ||
| raise ImportError( | ||
| "ONNX Runtime required. Install with: pip install onnxruntime" | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
ONNX import hatası için de from err ekleyin.
Aynı şekilde, ONNX Runtime import hatası için de exception chaining kullanılmalı.
🔎 Önerilen düzenleme
def _load_onnx_model(self) -> None:
"""Load ONNX model."""
try:
import onnxruntime as ort # type: ignore
- except ImportError:
+ except ImportError as err:
raise ImportError(
"ONNX Runtime required. Install with: pip install onnxruntime"
- )
+ ) from err📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _load_onnx_model(self) -> None: | |
| """Load ONNX model.""" | |
| try: | |
| import onnxruntime as ort # type: ignore | |
| except ImportError: | |
| raise ImportError( | |
| "ONNX Runtime required. Install with: pip install onnxruntime" | |
| ) | |
| def _load_onnx_model(self) -> None: | |
| """Load ONNX model.""" | |
| try: | |
| import onnxruntime as ort # type: ignore | |
| except ImportError as err: | |
| raise ImportError( | |
| "ONNX Runtime required. Install with: pip install onnxruntime" | |
| ) from err |
🧰 Tools
🪛 Ruff (0.14.10)
126-128: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
126-128: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In src/wyoming_server/models.py around lines 121 to 128, the ImportError raised
when attempting to import onnxruntime should use exception chaining; catch the
ImportError as a variable (e.g., err) and re-raise the new ImportError with the
same message using "from err" so the original traceback is preserved for
debugging.
| # For TFLite support (choose one): | ||
| # Option A: Lightweight runtime (recommended for deployment) | ||
| # tflite-runtime>=2.14.0 | ||
| # Option B: Full TensorFlow (if tflite-runtime unavailable) | ||
| # tensorflow>=2.14.0 |
There was a problem hiding this comment.
TFLite bağımlılığı yorumda bırakılmış ama .tflite modelleri destekleniyor.
README ve UI'da .tflite model desteği vurgulanıyor ama burada TFLite runtime yorum satırında. Bu durumda TFLite modeli kullanmak isteyen kullanıcı runtime hatası alacak.
İki seçenek var:
tflite-runtimeveyatensorflowvarsayılan olarak ekle- Kullanıcıya hangi durumda hangisini eklemesi gerektiğini net açıkla
🔎 Önerilen düzeltme
# Model inference
# For TFLite support (choose one):
-# Option A: Lightweight runtime (recommended for deployment)
-# tflite-runtime>=2.14.0
-# Option B: Full TensorFlow (if tflite-runtime unavailable)
-# tensorflow>=2.14.0
+# Option A: Lightweight runtime (recommended for most systems)
+tflite-runtime>=2.14.0
+# Option B: Use tensorflow if tflite-runtime fails to install on your platform
+# pip install tensorflow>=2.14.0Veya Dockerfile'da pip install tflite-runtime || pip install tensorflow pattern'ı kullanılabilir.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # For TFLite support (choose one): | |
| # Option A: Lightweight runtime (recommended for deployment) | |
| # tflite-runtime>=2.14.0 | |
| # Option B: Full TensorFlow (if tflite-runtime unavailable) | |
| # tensorflow>=2.14.0 | |
| # For TFLite support (choose one): | |
| # Option A: Lightweight runtime (recommended for most systems) | |
| tflite-runtime>=2.14.0 | |
| # Option B: Use tensorflow if tflite-runtime fails to install on your platform | |
| # pip install tensorflow>=2.14.0 |
🤖 Prompt for AI Agents
In src/wyoming_server/requirements-wyoming.txt around lines 13-17 the TFLite
dependency is commented out which will cause runtime failures when loading
.tflite models; either (A) enable a default by uncommenting and adding
tflite-runtime>=2.14.0 (or tensorflow>=2.14.0 if you prefer full TF) to the
requirements file, or (B) keep them optional but add a clear comment and update
README/UI and Dockerfile instructions to show the exact install fallback (e.g.,
try pip install tflite-runtime and fall back to pip install tensorflow) so users
know which package to install for .tflite support.
| try: | ||
| await self._server.run( | ||
| partial( | ||
| WyomingWakeWordEventHandler, | ||
| wyoming_info=wyoming_info, | ||
| handler=self._create_handler(), | ||
| config=self.config, | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Handler instance tüm client'lar arasında paylaşılıyor - concurrency riski!
Merhaba! Burada önemli bir potansiyel sorun var. _create_handler() server'ın run() metodu içinde bir kez çağrılıyor ve bu tek handler instance'ı tüm bağlanan client'lara veriliyor. Birden fazla client aynı anda bağlanırsa, handler'ın internal state'i (buffer, detection state vb.) karışabilir.
🔎 Önerilen düzeltme
Her client bağlantısı için yeni bir handler oluşturulmalı:
try:
await self._server.run(
partial(
WyomingWakeWordEventHandler,
wyoming_info=wyoming_info,
- handler=self._create_handler(),
+ handler_factory=self._create_handler, # Factory pattern
config=self.config,
)
)Ve WyomingWakeWordEventHandler.__init__ içinde:
def __init__(
self,
wyoming_info: Info,
handler_factory: Callable[[], WakeWordHandler], # Factory instead of instance
config: WyomingServerConfig,
*args,
**kwargs,
) -> None:
super().__init__(*args, **kwargs)
self.wyoming_info = wyoming_info
self.handler = handler_factory() # Create new handler per client
self.config = configCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/wyoming_server/server.py around lines 314 to 322, the code calls
self._create_handler() once and passes the single handler instance to
WyomingWakeWordEventHandler, causing that handler to be shared across all
clients (concurrency/state corruption risk); change to pass a handler factory
(e.g. handler_factory=self._create_handler without calling it) so each
connection constructs its own handler, and update
WyomingWakeWordEventHandler.__init__ to accept a Callable factory
(handler_factory: Callable[[], WakeWordHandler]) and inside __init__ call
handler_factory() to create a fresh handler for that client; also update any
type hints and usages accordingly.
| async def stop(self) -> None: | ||
| """Stop the Wyoming server.""" | ||
| if self._server is not None: | ||
| self._running = False | ||
| # Server will stop on next event | ||
| logger.info("Stopping Wyoming server") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# İlk olarak dosyayı okuyalım ve stop() metodunun tam bağlamını görelim
cat -n src/wyoming_server/server.py | head -350 | tail -50Repository: sarpel/wakeword-training-platform
Length of output: 1971
🏁 Script executed:
# _running flag'inin nerede kullanıldığını bulalım
rg "_running" src/wyoming_server/server.py -A 2 -B 2Repository: sarpel/wakeword-training-platform
Length of output: 1065
🏁 Script executed:
# AsyncServer'ın temel yapısını anlamak için __init__ ve main loop'u görelim
ast-grep --pattern 'async def $_($_) {
$$$
}' src/wyoming_server/server.pyRepository: sarpel/wakeword-training-platform
Length of output: 59
🏁 Script executed:
# AsyncServer'ın import ve kullanımını kontrol et
rg "AsyncServer" src/wyoming_server/server.py -B 3 -A 1Repository: sarpel/wakeword-training-platform
Length of output: 686
🏁 Script executed:
# Tüm dosyadaki async method'ları listele
rg "async def " src/wyoming_server/server.py -nRepository: sarpel/wakeword-training-platform
Length of output: 280
🏁 Script executed:
# Wyoming SDK'dan AsyncServer hakkında bilgi al
grep -r "AsyncServer" --include="*.py" | head -20Repository: sarpel/wakeword-training-platform
Length of output: 337
🏁 Script executed:
# Web'de Wyoming SDK hakkında AsyncServer bilgisi ara
# Ancak önce requirements dosyasını kontrol edelim
find . -name "requirements*.txt" -o -name "pyproject.toml" -o -name "setup.py" | head -5Repository: sarpel/wakeword-training-platform
Length of output: 195
🏁 Script executed:
# AsyncServer'ın kullanıldığı yerleri daha ayrıntılı incele
cat -n src/wyoming_server/server.py | sed -n '314,335p'Repository: sarpel/wakeword-training-platform
Length of output: 959
🏁 Script executed:
# Eğer task management varsa kontrol et
rg "asyncio\.|Task\|gather\|create_task" src/wyoming_server/server.pyRepository: sarpel/wakeword-training-platform
Length of output: 115
🏁 Script executed:
# run() metodu nasıl çağrılıyor? Bir task olarak mı başlatılıyor?
rg "\.run\(\)" src/wyoming_server/server.py -B 5 -A 5Repository: sarpel/wakeword-training-platform
Length of output: 271
🏁 Script executed:
# run_server adlı fonksiyonu kontrol et (380. satırda görüldü)
cat -n src/wyoming_server/server.py | sed -n '380,420p'Repository: sarpel/wakeword-training-platform
Length of output: 1095
🏁 Script executed:
# run() method'u nasıl kullanılıyor, create_task varsa kontrol et
cat -n src/wyoming_server/server.py | sed -n '1,100p'Repository: sarpel/wakeword-training-platform
Length of output: 3682
🏁 Script executed:
# Kodda task yönetimi var mı kontrol et - create_task kullanımı
rg "create_task\|ensure_future\|gather" src/ --include="*.py" | head -10Repository: sarpel/wakeword-training-platform
Length of output: 518
🏁 Script executed:
# run() ve stop() nasıl birlikte çalışıyor görmek için, bu fonksiyondaki tam akış
cat -n src/wyoming_server/server.py | sed -n '296,335p'Repository: sarpel/wakeword-training-platform
Length of output: 1534
🏁 Script executed:
# run() metodunun tamamını görelim
cat -n src/wyoming_server/server.py | sed -n '296,328p'Repository: sarpel/wakeword-training-platform
Length of output: 1244
🏁 Script executed:
# Wyoming SDK'ında AsyncServer.run() method'unun parametre alıp almadığını kontrol et
python3 << 'EOF'
import sys
try:
from wyoming.server import AsyncServer
import inspect
# AsyncServer.run metodu hakkında info al
run_method = getattr(AsyncServer, 'run', None)
if run_method:
sig = inspect.signature(run_method)
print(f"AsyncServer.run signature: {sig}")
print(f"AsyncServer.run docstring:\n{run_method.__doc__}")
except Exception as e:
print(f"Error: {e}")
print("Wyoming SDK may not be installed or AsyncServer info not available")
EOFRepository: sarpel/wakeword-training-platform
Length of output: 176
🏁 Script executed:
# stop() metodu async olmasına rağmen ne işe yarıyor - kullanım örneği ara
rg "await.*stop\(\)" src/ -B 3 -A 3Repository: sarpel/wakeword-training-platform
Length of output: 682
🏁 Script executed:
# run() method'u task olarak mı başlatılıyor, yoksa doğrudan await mı?
cat -n src/wyoming_server/server.py | sed -n '407,411p'Repository: sarpel/wakeword-training-platform
Length of output: 262
stop() method'u server'ı aktif olarak durdurmaz, sadece bir bayrak ayarlar.
stop() sadece _running bayrağını False yapar ama AsyncServer üzerinde asıl durdurma işlemini gerçekleştirmez. Yorumda yazılanı dusundugunde ("Sunucu bir sonraki event'te durur"), bu Wyoming SDK'ının AsyncServer.run() implementasyonunun _running bayrağını kontrol ettiğini varsayıyor. Ancak şu anda, stop() method'u hiçbir garanti vermiyormuş gibi görünüyor.
Wyoming SDK'ında AsyncServer'ın close(), shutdown() gibi bir metodu varsa veya cancel edilmesi gerekiyorsa, stop() method'u bunu çağırmalı.
🤖 Prompt for AI Agents
In src/wyoming_server/server.py around lines 329-334, stop() only flips
self._running without actually stopping the AsyncServer; change it to actively
stop the underlying server: if self._server is not None set self._running =
False then detect and call the appropriate shutdown method on self._server
(prefer close() or shutdown() if present) and await it when it is a coroutine;
if the server needs task cancellation, cancel the server task and await it; wrap
the stop call in a try/except to log errors and ensure the server reference is
cleared after successful shutdown.
…ies, remove claude settings file
|
APIError: |
|
| await self._server.run( | ||
| partial( | ||
| WyomingWakeWordEventHandler, | ||
| wyoming_info=wyoming_info, | ||
| handler=self._create_handler(), | ||
| config=self.config, | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Bug: A single WakeWordHandler instance is shared across all client connections because self._create_handler() is called immediately, causing race conditions and state corruption.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
The Wyoming AsyncServer expects a handler factory that creates a new handler per client. However, the code calls self._create_handler() immediately, creating a single WakeWordHandler instance that is shared across all client connections. This handler contains unprotected mutable state, such as _audio_buffer and _is_detecting. When multiple clients connect simultaneously, they will operate on this same instance, leading to race conditions, mixed audio from different clients in the buffer, incorrect wake word detection logic, and potential state corruption.
💡 Suggested Fix
Wrap the WyomingWakeWordEventHandler instantiation in a lambda or a factory function to ensure self._create_handler() is called once for each new client connection, creating a separate handler instance for each. For example: await self._server.run(lambda *args, **kwargs: WyomingWakeWordEventHandler(..., handler=self._create_handler(), ...)).
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/wyoming_server/server.py#L318-L325
Potential issue: The `Wyoming AsyncServer` expects a handler factory that creates a new
handler per client. However, the code calls `self._create_handler()` immediately,
creating a single `WakeWordHandler` instance that is shared across all client
connections. This handler contains unprotected mutable state, such as `_audio_buffer`
and `_is_detecting`. When multiple clients connect simultaneously, they will operate on
this same instance, leading to race conditions, mixed audio from different clients in
the buffer, incorrect wake word detection logic, and potential state corruption.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 515104
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
tests/test_qat_full_pipeline.py (1)
21-26: Test koşulu ve device seçimi çelişiyor.Test, CUDA mevcut olmadığında atlanıyor (satır 21-24) ama sonra device'ı "cpu" olarak ayarlıyorsun (satır 26). Bu mantıksal bir çelişki:
skipif(not torch.cuda.is_available())→ CUDA yoksa testi atladevice = "cpu"→ CPU kullanEğer test CPU'da çalışabiliyorsa, CUDA kontrolüne gerek yok. Eğer CUDA gerekliyse, device "cuda" olmalı.
Ayrıca,
conftest.py'dakidevice()fixture'ını kullanmak yerine hardcode ediyorsun. Bu, coding guidelines'a aykırı (her zaman fixture'ları kullan).🔎 Önerilen düzeltme
Seçenek 1: Test gerçekten CPU'da çalışabiliyorsa
-@pytest.mark.skipif( - not torch.cuda.is_available(), - reason="QAT pipeline requires CUDA for reliable export", -) -def test_full_pipeline_qat_export(tmp_path): - device = "cpu" # Use CPU for CI reliability +def test_full_pipeline_qat_export(tmp_path, device): + # device fixture from conftest.py automatically selects best availableSeçenek 2: Test gerçekten CUDA gerektiriyorsa
@pytest.mark.skipif( not torch.cuda.is_available(), reason="QAT pipeline requires CUDA for reliable export", ) def test_full_pipeline_qat_export(tmp_path): - device = "cpu" # Use CPU for CI reliability + device = "cuda"src/ui/panel_training.py (2)
335-370: Thread-safety eksikliği:create_lr_plotfonksiyonunda kilit kullanılmıyor.Merhaba! Şöyle düşünelim: Birden fazla kişi aynı anda bir deftere yazıyor ve okuyor. Karışıklık olmaması için sırayla erişmeleri gerekir - işte
training_state_lockbunun için var.
create_loss_plot,create_accuracy_plotvecreate_metrics_plotfonksiyonlarıtraining_state_lockkullanıyor (satır 161, 213, 265), amacreate_lr_plotvecreate_throughput_plotfonksiyonları kullanmıyor. Bu, race condition'a yol açabilir.🔎 Önerilen düzeltme
def create_lr_plot() -> Figure: """Create learning rate curve plot""" fig, ax = plt.subplots(figsize=(10, 5)) - if len(training_state.history.get("learning_rate", [])) > 0: - epochs = training_state.history["epochs"] - lrs = training_state.history["learning_rate"] + with training_state_lock: + if len(training_state.history.get("learning_rate", [])) > 0: + epochs = training_state.history["epochs"][:] + lrs = training_state.history["learning_rate"][:] + else: + epochs = [] + lrs = [] + if epochs: ax.plot( epochs, lrs,
373-407: Thread-safety eksikliği:create_throughput_plotfonksiyonunda da kilit kullanılmıyor.Aynı sorun burada da var. Tutarlılık için bu fonksiyon da
training_state_lockkullanmalı.🔎 Önerilen düzeltme
def create_throughput_plot() -> Figure: """Create throughput (samples/sec) curve plot""" fig, ax = plt.subplots(figsize=(10, 5)) - if len(training_state.history.get("throughput", [])) > 0: - epochs = training_state.history["epochs"] - throughput = training_state.history["throughput"] + with training_state_lock: + if len(training_state.history.get("throughput", [])) > 0: + epochs = training_state.history["epochs"][:] + throughput = training_state.history["throughput"][:] + else: + epochs = [] + throughput = [] + if epochs: ax.plot( epochs, throughput,src/ui/panel_evaluation.py (1)
1277-1284: PotansiyelAttributeError: config None olabilir.Bu lambda içinde
config_state.get("config").data.data_rootçağrısı yapılıyor. Eğerconfig_state.get("config")None döndürürse,.dataerişimiAttributeErrorfırlatır.Ternary operator'deki kontrol (
if config_state.get("config") else "data") sadece son kısım için geçerli, ilk kısımda hata oluşabilir.🔎 Önerilen düzeltme
evaluate_testset_btn.click( - fn=lambda test_split_path, threshold, target_fah, use_advanced_metrics, config_state: evaluate_test_set( - config_state.get("config").data.data_root if config_state.get("config") else "data", + fn=lambda test_split_path, threshold, target_fah, use_advanced_metrics, config_state: evaluate_test_set( + config_state.get("config").data.data_root if config_state and config_state.get("config") else "data", test_split_path, threshold, target_fah, use_advanced_metrics, ),Veya daha güvenli:
fn=lambda test_split_path, threshold, target_fah, use_advanced_metrics, config_state: evaluate_test_set( getattr(getattr(config_state.get("config"), "data", None), "data_root", "data"), ... )
♻️ Duplicate comments (7)
scripts/training_insights.py (1)
61-61:weights_only=Truebu script'in amacını bozuyor - kritik hata!Bu değişiklik, analyzer script'ini tamamen bozar. İşte neden:
Satır 73-100 arasında kod şu checkpoint key'lerini okumaya çalışıyor:
checkpoint["config"](satır 73)checkpoint["history"](satır 81)checkpoint["model_state_dict"](satır 86)checkpoint["epoch"](satır 94)checkpoint["best_val_loss"](satır 99)Ama
weights_only=Truesadece tensor verilerini yükler! Dictionary'ler, list'ler ve diğer Python objeleri yüklenmez. Bu değişiklikle:
checkpoint["config"]bulunamaz → analyzer config analizi yapamazcheckpoint["history"]bulunamaz → training history analizi yapamaz- Analyzer'ın temel fonksiyonları çalışmaz
Geçmiş review comment'inde (past_review_comments) de bu sorunun güvenlik açısı vurgulanmış. Ancak bu script'in amacı checkpoint'leri analiz etmek olduğu için, bu verilere ihtiyaç var.
Çözüm: Bu script güvenilir kaynaklardan (kendi eğittiğimiz modellerden) checkpoint yüklediği için,
weights_only=Falsekullanmalı VE bunu açıkça dokümante etmeli.🔎 Önerilen düzeltme
try: - checkpoint = torch.load(checkpoint_path, map_location="cpu", weights_only=True) + # Note: weights_only=False is required to load config, history, and metadata + # This script should only analyze checkpoints from trusted sources (own training runs) + checkpoint = torch.load(checkpoint_path, map_location="cpu", weights_only=False) except Exception as e:Ek güvenlik önlemi olarak, dosya yolunu validate edebilirsin:
+ # Validate checkpoint is in expected directory + if not checkpoint_path.is_relative_to(Path("models")) and not checkpoint_path.is_relative_to(Path("checkpoints")): + self.insights.append( + TrainingInsight( + category="critical", + title="Unsafe Checkpoint Path", + message=f"Checkpoint path {checkpoint_path} is outside expected directories", + ) + ) + return self.insights + try: - checkpoint = torch.load(checkpoint_path, map_location="cpu", weights_only=True) + checkpoint = torch.load(checkpoint_path, map_location="cpu", weights_only=False)src/wyoming_server/server.py (3)
317-325: Kritik: Handler instance tüm client'lar arasında paylaşılıyor - race condition!Merhaba! Bunu şöyle düşün: Bir sınıfta tek bir defter var ve 30 öğrenci aynı anda o deftere yazmaya çalışıyor. Ne olur? Kaos!
Burada
_create_handler()bir kez çağrılıyor ve bu tek handler instance'ı tüm client bağlantılarına veriliyor. Her client'ın kendi buffer'ı, detection state'i olması gerekir.🔎 Önerilen düzeltme: Factory pattern kullan
try: await self._server.run( partial( WyomingWakeWordEventHandler, wyoming_info=wyoming_info, - handler=self._create_handler(), + handler_factory=self._create_handler, # Factory pattern config=self.config, ) )Ve
WyomingWakeWordEventHandler.__init__içinde:from typing import Callable def __init__( self, wyoming_info: Info, handler_factory: Callable[[], WakeWordHandler], # Factory config: WyomingServerConfig, *args, **kwargs, ) -> None: super().__init__(*args, **kwargs) self.wyoming_info = wyoming_info self.handler = handler_factory() # Her client için yeni handler self.config = config
233-242: Kritik: TFLite model thread-safe değil - concurrent erişimde crash olabilir.TensorFlow Lite Interpreter thread-safe değil. Birden fazla client aynı anda model'e inference yaptığında:
- Yanlış sonuçlar alabilirsin
- Segmentation fault ile crash olabilir
Bu, özellikle yukarıdaki handler paylaşım sorunu ile birleşince daha da ciddi bir problem.
🔎 Çözüm seçenekleri
Seçenek 1: Her client için yeni model instance (yukarıdaki factory pattern ile birlikte)
Seçenek 2: Model çağrılarına lock ekle (models.py içinde):
import threading class TFLiteWakeWordModel: def __init__(self, ...): self._inference_lock = threading.Lock() # ... existing code ... def predict(self, audio: np.ndarray) -> float: with self._inference_lock: # ... existing inference code ...Seçenek 3: ONNX runtime kullan (thread-safe)
332-337:stop()metodu server'ı aktif olarak durdurmaz.
stop()sadece_runningbayrağınıFalseyapar. AmaAsyncServer.run()bloke ediyor olabilir ve bu bayrak değişikliğini hemen görmeyebilir.Wyoming SDK'nın
AsyncServer'ı birclose()veya benzeri metod sunuyorsa, onu çağırmalısın.🔎 Önerilen düzeltme
async def stop(self) -> None: """Stop the Wyoming server.""" if self._server is not None: self._running = False - # Server will stop on next event + # Actively close the server if possible + if hasattr(self._server, 'close'): + await self._server.close() + elif hasattr(self._server, 'shutdown'): + await self._server.shutdown() logger.info("Stopping Wyoming server")src/ui/panel_wyoming.py (3)
26-38: Thread safety eksikliği: Global state korumasız.Merhaba! Düşün ki iki kişi aynı anda bir lambayı açıp kapatmaya çalışıyor - sonuç belirsiz olur. Burada da
wyoming_statebirden fazla Gradio callback'i tarafından aynı anda değiştirilebilir.Özellikle şu senaryo tehlikeli: Kullanıcı hızlıca "Start" ve "Stop" butonlarına basarsa,
is_runningveserver_processtutarsız durumda kalabilir.🔎 Thread-safe çözüm
+import threading + class WyomingServerState: """Global state for Wyoming server management.""" def __init__(self) -> None: + self._lock = threading.Lock() self.server_process: Optional[subprocess.Popen] = None self.server_thread: Optional[threading.Thread] = None self.is_running: bool = False self.last_status: str = "Not started" self.config: Dict[str, Any] = {} + + def safe_start(self, process: subprocess.Popen, config: Dict[str, Any]) -> bool: + """Thread-safe server start.""" + with self._lock: + if self.is_running: + return False + self.server_process = process + self.is_running = True + self.config = config + return True + + def safe_stop(self) -> Optional[subprocess.Popen]: + """Thread-safe server stop - returns process to terminate.""" + with self._lock: + if not self.is_running: + return None + process = self.server_process + self.server_process = None + self.is_running = False + return process
68-88: Kritik güvenlik açığı: Path traversal saldırısına açık.CodeQL bu satırı işaretlemiş ve haklı. Kullanıcı
model_patholarak../../etc/passwdveya..\..\windows\system32\configgibi bir şey gönderebilir.Şöyle düşün: Bir kasa şifresi var, ama şifre olarak
../yazarak kasanın dışına çıkabiliyorsun. Bu ciddi bir güvenlik açığı.🔎 Güvenli path validation
def validate_model_path(model_path: str) -> Tuple[bool, str]: """Validate that model path exists and is valid.""" if model_path == "No models available - export a model first": return False, "❌ No models available. Export a model first in Panel 5." path = Path(model_path) + + # Security: Path traversal koruması + allowed_dirs = [Path("exports").resolve(), Path("models/exports").resolve()] + try: + resolved_path = path.resolve() + # Path'in izin verilen dizinlerden birinde olduğunu doğrula + is_safe = any( + resolved_path == allowed_dir or + any(parent == allowed_dir for parent in resolved_path.parents) + for allowed_dir in allowed_dirs + ) + if not is_safe: + return False, "❌ Model exports/ veya models/exports/ dizininde olmalı" + except (OSError, ValueError) as e: + return False, f"❌ Geçersiz model yolu: {e}" + if not path.exists(): return False, f"❌ Model file not found: {model_path}" if path.suffix.lower() not in (".tflite", ".onnx"): return False, f"❌ Invalid model format: {path.suffix}" return True, f"✅ Valid model: {path.name}"
136-165: Kritik güvenlik açığı: Command injection riski.CodeQL 12 farklı noktada bu riski tespit etmiş. Kullanıcı girdileri (
wake_word_name,host,feature_typevb.) doğrudan subprocess komutuna ekleniyor.Şöyle düşün: Bir formda isim soruyorsun ve kullanıcı
hey_assistant; rm -rf /yazıyor. Bu direkt terminale gidiyor!🔎 Input sanitization önerisi
import re def sanitize_wake_word_name(name: str) -> str: """Sadece alfanumerik ve alt çizgi kabul et.""" sanitized = re.sub(r'[^a-zA-Z0-9_]', '', name) if not sanitized: raise ValueError("Wake word en az bir alfanumerik karakter içermeli") return sanitized[:64] # Uzunluk limiti def sanitize_host(host: str) -> str: """Host formatını doğrula.""" # IP adresleri ve hostname'ler için if not re.match(r'^[a-zA-Z0-9.\-:]+$', host): raise ValueError("Geçersiz host formatı") if len(host) > 253: raise ValueError("Host çok uzun") return host def validate_feature_type(feature_type: str) -> str: """Whitelist ile feature type doğrula.""" allowed = {"mel", "mfcc"} if feature_type not in allowed: raise ValueError(f"Feature type şunlardan biri olmalı: {allowed}") return feature_type def validate_numeric_range(value: float, min_val: float, max_val: float, name: str) -> float: """Numeric değerlerin aralıkta olduğunu doğrula.""" if not (min_val <= value <= max_val): raise ValueError(f"{name} {min_val}-{max_val} aralığında olmalı") return valueSonra
start_wyoming_serverbaşında:try: wake_word_name = sanitize_wake_word_name(wake_word_name) host = sanitize_host(host) feature_type = validate_feature_type(feature_type) threshold = validate_numeric_range(threshold, 0.0, 1.0, "Threshold") # ... diğer validasyonlar except ValueError as e: return f"❌ Validation error: {e}", str(e)
🧹 Nitpick comments (7)
scripts/docs_maintenance.py (1)
1060-1092:print()yerine structured logger kullan.Bu fonksiyon doğru çalışıyor, ama kodlama kılavuzlarımıza göre
print()yerine structured logger kullanmalıyız. Özellikle 1070, 1081, 1083, 1085, 1087, 1089 satırlarında.ELI15 açıklama:
print()sadece console'a yazıyor, ama structured logger hem console'a hem dosyaya yazabilir, log level'ları var (info, warning, error), ve logları daha sonra analiz etmek çok daha kolay. Professional projelerde her zaman logger kullanırız.Ancak şu an için tüm dosya
print()kullanıyor, bu yüzden consistency açısından bu fonksiyonu tek başına değiştirmek garip olabilir. İdeal olan tüm dosyayı refactor etmek.Kodlama kılavuzlarına göre değerlendirme.
docs/CODE_QUALITY_REVIEW.md (1)
28-37: Pipeline uyarısı: Başlık seviyeleri atlanıyor.Merhaba! Şöyle düşün: Bir kitapta Bölüm 1'den (h1) direkt Alt Bölüm 1.1.1'e (h3) atlıyorsun, ama Bölüm 1.1'i (h2) atlıyorsun. Bu, okuyucular ve ekran okuyucular için kafa karıştırıcı olabilir.
Pipeline'da şu uyarılar var:
- "CRITICAL-2: Secret Exposure Risk" → h3 ama h2 yok
- "CRITICAL-3: Mutable Default Argument" → h3 ama h2 yok
Düzeltme önerisi:
### CRITICAL-2yerine## CRITICAL-2kullanabilir veya ara bir## Critical Issues Detailbaşlığı ekleyebilirsin.src/ui/panel_training.py (1)
524-524: Kullanılmayankwargsparametresi.Static analysis bu parametrenin kullanılmadığını belirtiyor. Bu muhtemelen callback interface uyumluluğu için var - eğer öyleyse, bunu açıkça belirtmek iyi olur.
🔎 Önerilen düzeltme
- def on_batch_end(self, batch_idx: int, loss: float, acc: float, **kwargs: Any) -> None: + def on_batch_end(self, batch_idx: int, loss: float, acc: float, **_kwargs: Any) -> None:Veya docstring ekleyerek:
def on_batch_end(self, batch_idx: int, loss: float, acc: float, **kwargs: Any) -> None: """Handle batch end event. Args: kwargs: Additional arguments for callback interface compatibility (unused). """src/ui/panel_evaluation.py (2)
134-134: Gradio pattern:gr.Progress()default argument.Static analysis bunu işaretliyor (B008), ama bu Gradio'nun standart pattern'i. Gradio, progress callbacks için bu yapıyı bekliyor. Bu bir false positive olarak değerlendirilebilir, ama eğer uyarıyı susturmak istersen:
🔎 Alternatif yaklaşım
-def run_background_mining(file_path: str, threshold: float, resume: bool, progress=gr.Progress()) -> str: +def run_background_mining(file_path: str, threshold: float, resume: bool, progress: gr.Progress = None) -> str: # type: ignore[assignment] """Run long-form background mining.""" + if progress is None: + progress = gr.Progress()Ancak bu, Gradio'nun beklediği interface'i bozabilir. Mevcut hali Gradio dokümantasyonuyla uyumlu.
839-839: Kullanılmayan değişken:thresholds.
get_roc_curve_dataüç değer döndürüyor amathresholdshiç kullanılmıyor. Python convention'ına göre_ile prefix'lemeli.🔎 Önerilen düzeltme
- fpr_array, tpr_array, thresholds = get_roc_curve_data(eval_state.evaluator, test_dataset, batch_size=32) + fpr_array, tpr_array, _thresholds = get_roc_curve_data(eval_state.evaluator, test_dataset, batch_size=32)src/ui/panel_wyoming.py (2)
571-581: Kullanılmayanstateparametresi.
create_wyoming_deployment_panelfonksiyonundastateparametresi hiç kullanılmıyor. Bu, diğer panel'lerle tutarlılık için eklenmiş görünüyor, ama ya kullanılmalı ya da kaldırılmalı.🔎 Önerilen düzeltme
-def create_wyoming_deployment_panel(state: Optional[gr.State] = None) -> gr.Blocks: +def create_wyoming_deployment_panel(_state: Optional[gr.State] = None) -> gr.Blocks: """ Create Wyoming deployment panel (alias for create_wyoming_panel). Args: - state: Optional global state (for consistency with other panels) + _state: Optional global state (unused, for interface consistency) Returns: Gradio Blocks panel """ return create_wyoming_panel()
516-518: Kullanılmayan değişken:is_valid.
on_model_selectfonksiyonundais_validhiç kullanılmıyor.🔎 Önerilen düzeltme
def on_model_select(model_path: str) -> str: - is_valid, msg = validate_model_path(model_path) + _is_valid, msg = validate_model_path(model_path) return msg
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
AGENTS.mdCLAUDE.mddocs/CODE_QUALITY_REVIEW.mddocs/DEVELOPMENT_ROADMAP.mddocs/MLOPS_GUIDE.mddocs/PROGRESSIVE_TRAINING_GUIDE.mdscripts/docs_maintenance.pyscripts/training_insights.pysrc/training/trainer.pysrc/ui/panel_evaluation.pysrc/ui/panel_training.pysrc/ui/panel_wyoming.pysrc/wyoming_server/server.pytests/test_qat_full_pipeline.py
💤 Files with no reviewable changes (1)
- docs/DEVELOPMENT_ROADMAP.md
✅ Files skipped from review due to trivial changes (4)
- docs/MLOPS_GUIDE.md
- AGENTS.md
- docs/PROGRESSIVE_TRAINING_GUIDE.md
- CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (1)
- src/training/trainer.py
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.py: Always usesrc.configclasses for configuration management instead of hardcoding hyperparameters (e.g., useconfig.training.learning_raterather than raw values)
Use the project's structured logger by importing fromsrc.config.loggerwithsetup_logger(__name__)instead of using standard logging
Usepathlib.Pathfor all file and directory operations instead of string-based path handling
Enforce strict type hints throughout the codebase, especially for Pydantic models
**/*.py: Use type hints everywhere in Python code with specific type declarations
Prefer functional patterns in Python (map, filter, reduce) for transforming data sequences
Use context managers ('with' statement) in Python to ensure resources are properly cleaned up
Use dataclasses in Python to reduce boilerplate for data container definitions
In Python, always log errors with context before raising exceptions; never use silent exception handlers
Use snake_case for variable and function names in Python
**/*.py: Line length must be 120 characters (enforced by Black and flake8)
Use Black for code formatting (non-negotiable)
Use isort withprofile = "black"for import sorting
Use double quotes for strings (Black default)
Import order must follow: Future → Standard library → Third-party → First-party (src.)
Type hints are required on all function signatures
UseOptional[X]for nullable types in type hints
UseTYPE_CHECKINGguard from typing module for circular imports
Use Google-style docstrings with Args, Returns, and Raises sections
Use structlog for logging instead of print() statements
Always use pathlib.Path for file operations, never os.path
Use src.config.defaults dataclasses for configuration, never hardcoded values or raw dicts
Use custom exceptions from src.exceptions instead of generic exceptions
Never hardcode hyperparameters - always use config. attributes
Files:
tests/test_qat_full_pipeline.pysrc/ui/panel_wyoming.pyscripts/training_insights.pysrc/wyoming_server/server.pysrc/ui/panel_evaluation.pyscripts/docs_maintenance.pysrc/ui/panel_training.py
**/*.{py,ts,tsx,cpp,h,hpp,ino}
📄 CodeRabbit inference engine (CLAUDE.md)
Use PascalCase for class names in all languages (Python, TypeScript, C++)
Files:
tests/test_qat_full_pipeline.pysrc/ui/panel_wyoming.pyscripts/training_insights.pysrc/wyoming_server/server.pysrc/ui/panel_evaluation.pyscripts/docs_maintenance.pysrc/ui/panel_training.py
tests/test_*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/test_*.py: Test files must be named test_.py in the tests/ directory
Test classes must be named Test (e.g., class TestConfig:)
Test functions must be named test_* (e.g., def test_example())
Every test must include at least one pytest marker: @pytest.mark.unit, @pytest.mark.integration, @pytest.mark.slow, or @pytest.mark.gpu
Use pytest fixtures from conftest.py: default_config, device, sample_audio, tmp_path
Files:
tests/test_qat_full_pipeline.py
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
GPU/Device handling must check torch.cuda.is_available() and move tensors/models to device
Files:
src/ui/panel_wyoming.pysrc/wyoming_server/server.pysrc/ui/panel_evaluation.pysrc/ui/panel_training.py
🧠 Learnings (4)
📚 Learning: 2025-11-28T07:24:59.919Z
Learnt from: CR
Repo: sarpel/wakeword-training-platform PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T07:24:59.919Z
Learning: Use `src/export/onnx_exporter.py` for exporting models to deployment artifacts rather than using alternative export methods
Applied to files:
tests/test_qat_full_pipeline.py
📚 Learning: 2026-01-04T15:45:01.213Z
Learnt from: CR
Repo: sarpel/wakeword-training-platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-04T15:45:01.213Z
Learning: Project framework stack: PyTorch + Gradio for UI and model training
Applied to files:
docs/CODE_QUALITY_REVIEW.mdsrc/ui/panel_evaluation.pysrc/ui/panel_training.py
📚 Learning: 2026-01-04T15:45:01.213Z
Learnt from: CR
Repo: sarpel/wakeword-training-platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-04T15:45:01.213Z
Learning: Applies to src/training/**/*.py : Use CheckpointManager from src.training.checkpoint_manager for checkpoint operations
Applied to files:
docs/CODE_QUALITY_REVIEW.md
📚 Learning: 2026-01-04T15:45:01.213Z
Learnt from: CR
Repo: sarpel/wakeword-training-platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-04T15:45:01.213Z
Learning: Training logic should be implemented in src/training/ module
Applied to files:
docs/CODE_QUALITY_REVIEW.md
🧬 Code graph analysis (4)
tests/test_qat_full_pipeline.py (3)
tests/conftest.py (1)
device(24-26)tests/test_focal_loss_advanced.py (1)
device(14-15)tests/test_losses_advanced.py (1)
device(9-10)
src/ui/panel_wyoming.py (5)
src/wyoming_server/server.py (1)
is_running(340-342)src/ui/panel_evaluation.py (1)
get_available_models(166-177)src/wyoming_server/config.py (1)
validate_model_path(100-107)entrypoint.sh (1)
log(27-29)src/data/file_cache.py (1)
get(82-110)
src/ui/panel_evaluation.py (7)
src/evaluation/inference.py (2)
MicrophoneInference(33-333)SimulatedMicrophoneInference(336-390)src/config/cuda_utils.py (2)
get_cuda_validator(293-295)validate(22-58)src/evaluation/judge_client.py (2)
JudgeClient(13-65)verify_audio(22-57)server/app.py (2)
verify_audio(172-224)metrics(165-168)src/evaluation/file_evaluator.py (1)
evaluate_files(69-157)src/evaluation/mining.py (2)
inject_to_dataset(130-149)confirm_all_pending(117-128)src/evaluation/dataset_evaluator.py (1)
evaluate_dataset(20-157)
src/ui/panel_training.py (5)
src/training/wandb_callback.py (2)
on_batch_end(68-75)WandbCallback(20-86)src/training/qat_utils.py (1)
prepare_model_for_qat(94-189)src/data/audio_utils.py (1)
AudioProcessor(218-335)src/data/processor.py (1)
AudioProcessor(20-106)src/data/cmvn.py (1)
compute_cmvn_from_dataset(261-347)
🪛 GitHub Actions: Documentation Quality
docs/CODE_QUALITY_REVIEW.md
[warning] 1-1: Structure: Heading skips levels: h1 to h3 ('CRITICAL-2: Secret Exposure Risk')
[warning] 1-1: Structure: Heading skips levels: h1 to h3 ('CRITICAL-3: Mutable Default Argument')
[warning] 1-1: Structure: Heading skips levels: h1 to h3 ('HIGH - God Class - Trainer')
🪛 GitHub Check: CodeQL
src/ui/panel_wyoming.py
[failure] 82-82: Uncontrolled data used in path expression
This path depends on a user-provided value.
[failure] 177-177: Uncontrolled command line
This command line depends on a user-provided value.
This command line depends on a user-provided value.
This command line depends on a user-provided value.
This command line depends on a user-provided value.
This command line depends on a user-provided value.
This command line depends on a user-provided value.
This command line depends on a user-provided value.
This command line depends on a user-provided value.
This command line depends on a user-provided value.
This command line depends on a user-provided value.
This command line depends on a user-provided value.
This command line depends on a user-provided value.
🪛 Ruff (0.14.10)
src/ui/panel_wyoming.py
176-176: subprocess call: check for execution of untrusted input
(S603)
200-200: Consider moving this statement to an else block
(TRY300)
210-210: Use explicit conversion flag
Replace with conversion flag
(RUF010)
245-245: Do not catch blind exception: Exception
(BLE001)
345-364: Possible SQL injection vector through string-based query construction
(S608)
439-439: Possible binding to all interfaces
(S104)
517-517: Unpacked variable is_valid is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
571-571: Unused function argument: state
(ARG001)
src/wyoming_server/server.py
172-172: Possible binding to all interfaces
(S104)
203-205: Avoid specifying long messages outside the exception class
(TRY003)
src/ui/panel_evaluation.py
134-134: Do not perform function call gr.Progress in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
839-839: Unpacked variable thresholds is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
src/ui/panel_training.py
524-524: Unused method argument: kwargs
(ARG002)
1084-1084: Do not catch blind exception: Exception
(BLE001)
🔍 Remote MCP GitHub Copilot
Concise review-context summary (relevant facts)
-
New Wyoming/Home Assistant integration added under src/wyoming_server/:
- Files added: init.py, main.py, config.py, handler.py, models.py, server.py, Dockerfile, docker-compose.yaml, requirements-wyoming.txt (detailed config, model wrapper, streaming handler, async server, CLI, Docker support).
-
UI integration:
- New Gradio panel src/ui/panel_wyoming.py (≈581 lines) and app.py updated to add a Home Assistant tab and 7th Documentation tab. Panel manages subprocess lifecycle, generates docker-compose and HA config, exposes start/stop and status.
-
Tests:
- New tests/tests_wyoming_server.py covering WyomingServerConfig, WakeWordHandler, and model feature extraction utilities (unit + some integration placeholders).
-
Deployment artifacts:
- src/wyoming_server/Dockerfile (python:3.10-slim, installs libsndfile1/libgomp1, installs requirements-wyoming.txt, HEALTHCHECK on 127.0.0.1:10400). docker-compose.yaml provided with port 10400, mounts exports, and external network homeassistant.
-
Config & validation:
- WyomingServerConfig is a Pydantic model (extra="forbid", validate_assignment=True) with validators for model_path and feature_type; computed properties uri, samples_needed, chunk_samples. load_config_from_checkpoint returns a partial config.
-
Model support & inference:
- TFLiteWakeWordModel supports TFLite (tflite-runtime or tensorflow) and ONNX (onnxruntime), includes feature-extraction (Mel/MFCC) and inference paths; load_model_from_export reads optional YAML export config.
-
Streaming handler & server behavior:
- WakeWordHandler implements buffering, resampling (resampy with linear-interp fallback), mono conversion, sliding window inference, trigger_level (consecutive detections), 500ms cooldown, and buffer trimming. WyomingWakeWordEventHandler maps Wyoming events (Describe/Detect/AudioStart/AudioChunk/AudioStop) to handler actions.
-
CLI & runtime:
- main.py provides a CLI with many args (model, threshold, trigger, port, sample-rate, audio-duration, n-mels, n-fft, hop-length, feature-type), sets Windows event loop policy, and logs guidance if Wyoming SDK missing.
-
Dependencies:
- requirements.txt added optional top-level wyoming>=1.8.0. New src/wyoming_server/requirements-wyoming.txt lists wyoming, numpy, torch, torchaudio, (tflite-runtime OR tensorflow), onnxruntime, resampy, structlog, pydantic, pyyaml.
Potential review focus areas (concrete, prioritized)
- Model-path / Pydantic strictness: config.Config.extra="forbid" may break callers that pass extra fields — verify all call sites set exact keys or allow backward compatibility. [src/wyoming_server/config.py]
- Dependency surface & image size: Dockerfile + torch in requirements-wyoming.txt may produce very large images — consider multi-stage builds, smaller runtime (onnx-only) or move heavy deps to optional layers. [src/wyoming_server/Dockerfile, requirements-wyoming.txt]
- Resampling fallback quality: handler falls back to numpy interp if resampy absent — evaluate detection accuracy impact and document/require resampy in production. [src/wyoming_server/handler.py, requirements-wyoming.txt]
- Process/subprocess management in UI panel: panel_wyoming uses subprocess.Popen and global WyomingServerState — check thread-safety, concurrent start/stop races, and cleanup on app exit. [src/ui/panel_wyoming.py]
- Wyoming SDK optionality: many runtime paths raise ImportError with guidance; confirm CI/Dev env documents how to enable and test Wyoming integration. [src/wyoming_server/server.py, main.py]
- Port and network assumptions: defaults to 10400 and uses external Docker network named homeassistant — ensure docs/README and compose instructions clearly state network preconditions. [README.md, docker-compose.yaml]
- Tests: new unit tests cover config and handler basics; consider adding integration test(s) that run the Docker image or run the CLI entry point in CI with a small mock model to catch runtime ImportError or runtime-flag issues. [tests/test_wyoming_server.py]
Sources used
- Repository PR file list and diffs retrieved via GitHub Copilot pull request API:
⏰ 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). (2)
- GitHub Check: Seer Code Review
- GitHub Check: claude-review
🔇 Additional comments (13)
scripts/docs_maintenance.py (1)
1140-1146: CLI entegrasyonu düzgün yapılmış!"fix" subcommand'ının eklenmesi çok temiz yapılmış:
--dry-runflag'i var (önce test et, sonra uygula mantığı - güzel!)- Argparse entegrasyonu doğru
- Execution path mantıklı
Öğrenciye övgü anı: CLI tasarımında dry-run özelliği eklemek çok iyi bir pratik. Kullanıcı önce
--dry-runile ne değişeceğini görür, eğer sonuçtan memnunsa gerçekten uygular. Bu "ölçümlerle iki kere, kesimle bir kere" mantığının kod versiyonu!Also applies to: 1209-1211
scripts/training_insights.py (3)
112-127: String formatting iyileştirmeleri - kod daha okunabilir.Satır 112-113'te std ve mean hesaplamalarını tek satıra indirgemişsin, satır 121 ve 125'te de string birleştirmeleri parantez içine almışsın. Bu değişiklikler:
- Kodu daha kompakt hale getiriyor
- Black formatter ile uyumlu
- Okunabilirliği koruyor
İyi bir refactor! 👍
161-166: Plateau tespiti için string formatting iyileştirmesi.Message ve recommendation string'lerini daha temiz formatta birleştirmişsin. Kod daha okunabilir ve maintainable.
302-312: Distillation ve QAT mesajlarında formatting iyileştirmeleri.String concatenation'ları parantez içine alarak Black formatter ile uyumlu hale getirmişsin. Özellikle satır 310'daki f-string kullanımı daha temiz görünüyor.
docs/CODE_QUALITY_REVIEW.md (1)
1-393: Dokümantasyon kalitesi iyi görünüyor.Bu Code Quality Review raporu, projedeki teknik borçları ve iyileştirme alanlarını açıkça belgeliyor. Yapılan küçük formatlama düzeltmeleri (satır 5-6, 35, 211, 316) okunabilirliği artırıyor.
src/ui/panel_training.py (2)
1687-1694: Güvenlik: WandB key dosyası için izin ayarı iyi düşünülmüş.Harika bir detay!
.wandb_keydosyasının izinlerini0o600(sadece sahip okuyabilir/yazabilir) olarak ayarlamak, API anahtarının güvenliğini artırır. Windows'ta bu çalışmadığı içinos.name != "nt"kontrolü de doğru.
422-582: Training worker ve callback yapısı düzgün uygulanmış.
training_workerfonksiyonu veLiveUpdateCallbacksınıfı, training durumunu güvenli bir şekilde güncelliyor.safe_appendhelper fonksiyonu ile thread-safe history güncellemesi yapılması çok iyi bir pratik.src/wyoming_server/server.py (2)
172-172:0.0.0.0tüm arayüzlere bağlanıyor - beklenen davranış.Static analysis bu satırı işaretliyor (S104), ama bu bir Wyoming server için beklenen davranış. Server'ın network üzerinden erişilebilir olması gerekiyor. Yorum veya dokümantasyon ile bunun bilinçli bir karar olduğunu belirtmek iyi olur.
# nosec B104yorumu zaten var, bu yeterli.
36-156: Event handler yapısı iyi tasarlanmış.
WyomingWakeWordEventHandlersınıfı Wyoming protocol event'lerini düzgün bir şekilde handle ediyor:
- Describe → server info döndürür
- Detect → detection başlatır
- AudioStart/Chunk/Stop → audio işleme ve detection
Logging de yapılandırılmış (structured) ve client_id ile context sağlıyor.
src/ui/panel_evaluation.py (2)
600-608: FN/FP injection mantığı düzgün uygulanmış.
confirm_all_and_inject_handlerfonksiyonu, mod'a göre doğru collector'ı kullanıyor ve kullanıcıya anlamlı geri bildirim sağlıyor. Hard positive ve hard negative ayrımı net.
559-580: Queue mode switching mantığı temiz.
get_queue_data_by_modeveupdate_queue_modefonksiyonları, UI'deki mod değişikliklerini düzgün handle ediyor. Kullanıcı talimatlarının moda göre güncellenmesi iyi bir UX detayı.src/ui/panel_wyoming.py (2)
345-364: Static analysis false positive: S608 SQL injection.Bu satır YAML f-string oluşturuyor, SQL değil. Static analysis aracı bunu yanlış algılamış. Bu bir güvenlik riski değil.
267-326: Docker compose ve HA config generation iyi tasarlanmış.
generate_docker_composevegenerate_ha_configfonksiyonları, deployment için kullanışlı YAML çıktıları üretiyor. Healthcheck, resource limits ve Home Assistant entegrasyon talimatları düşünülmüş.
| try: | ||
| content = file_path.read_text(encoding="utf-8") | ||
| except UnicodeDecodeError: | ||
| return [] |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Sessiz hata yönetimi: Kodlama kılavuzlarına aykırı.
Kodlama kılavuzlarımıza göre, Python'da "her zaman exception'ları context ile birlikte logla; asla sessiz exception handler kullanma" kuralı var. Burada ise UnicodeDecodeError yakalanıyor ama hiçbir log mesajı yok - sadece boş liste dönüyor.
Şöyle düşün: Bir dosya okunamadı ama kullanıcı bundan haberdar olmadı. Sanki hiç problem yokmuş gibi devam ediyor program. Bu da debugging'i zorlaştırır.
Ayrıca coding guidelines'a göre print() yerine structured logger (from src.config.logger import setup_logger) kullanmalıyız.
🔎 Önerilen düzeltme
Dosyanın başına logger ekle:
from src.config.logger import setup_logger
logger = setup_logger(__name__)Sonra error handling kısmını güncelle:
try:
content = file_path.read_text(encoding="utf-8")
except UnicodeDecodeError:
+ logger.warning(f"Could not decode {file_path} as UTF-8, skipping auto-fix")
return []Kodlama kılavuzlarına göre değerlendirme.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In scripts/docs_maintenance.py around lines 1018 to 1021, the UnicodeDecodeError
is being swallowed and the function returns [] with no logging; add a structured
logger (import setup_logger from src.config.logger and create logger =
setup_logger(__name__) near the top of the file) and update the except block to
log a descriptive error including the exception context (use logger.exception or
logger.error with exc_info=True) mentioning the file_path that failed to decode,
then continue to return [] as before.
| for i, line in enumerate(lines): | ||
| stripped = line.rstrip() | ||
| if stripped != line: | ||
| new_lines.append(stripped) | ||
| # Only report meaningful whitespace changes (ignoring just newline adjustments) | ||
| if stripped != line.replace("\n", ""): | ||
| fixes.append(f"Line {i+1}: Removed trailing whitespace") | ||
| else: |
There was a problem hiding this comment.
Mantık hatası: Gereksiz whitespace temizleme kontrolü hiçbir zaman tetiklenmiyor.
Burada öğretici bir detay var: lines listesi zaten content.split("\n") ile oluşturulmuş, yani her satır içinde artık \n karakteri yok. O yüzden 1035. satırdaki if stripped != line.replace("\n", ""): kontrolü her zaman false döner çünkü line içinde zaten \n bulunmuyor.
Düşün ki bir metni satırlara ayırdın - o satırların içinde satır sonu karakteri kalmaz. Sonra "satır sonu karakterini çıkarınca farklı mı?" diye soruyorsun - ama zaten yok ki!
Sonuç: Trailing whitespace temizleme işlemi yapılıyor (new_lines.append(stripped)), ama kullanıcıya bilgi mesajı hiç verilmiyor. Bu da kullanıcının hangi satırlarda değişiklik yapıldığını görmesini engelliyor.
🔎 Önerilen düzeltme
# Fix trailing whitespace
for i, line in enumerate(lines):
stripped = line.rstrip()
if stripped != line:
new_lines.append(stripped)
- # Only report meaningful whitespace changes (ignoring just newline adjustments)
- if stripped != line.replace("\n", ""):
- fixes.append(f"Line {i+1}: Removed trailing whitespace")
+ fixes.append(f"Line {i+1}: Removed trailing whitespace")
else:
new_lines.append(line)🤖 Prompt for AI Agents
In scripts/docs_maintenance.py around lines 1030-1037, the nested check for
line.replace("\n", "") is always false because lines were produced by
content.split("\n") (no trailing '\n'), so fixes is never appended; fix by
either (A) simpler: remove the inner condition and append the fix whenever
stripped != line (move fixes.append into the first if block) so
trailing-whitespace removals are reported, or (B) if you want to preserve the
original newline-aware logic, read lines with splitlines(True) so each line
keeps its ending and the existing inner check will work; implement one of these
changes and ensure tests/usage still produce correct new_lines and fixes
entries.
| print(f"\nTotal link issues: {len(issues)}") | ||
| print(f"\nTotal link issues: {len(issues)}") |
There was a problem hiding this comment.
Kritik: Tekrarlanan satır - copy-paste hatası.
1205 ve 1206. satırlar tamamen aynı:
print(f"\nTotal link issues: {len(issues)}")Bu tür tekrarlar genellikle copy-paste yaparken fare'nin çift tıklamasından ya da editörde yanlışlıkla iki kere yapıştırmadan kaynaklanır. Bir satırın silinmesi gerekiyor.
Kullanıcı aynı mesajı iki kere görür ki bu da kafa karıştırıcı ve profesyonel görünmüyor.
🔎 Önerilen düzeltme
print(f"\nTotal link issues: {len(issues)}")
- print(f"\nTotal link issues: {len(issues)}")
return 1 if issues else 0🤖 Prompt for AI Agents
In scripts/docs_maintenance.py around lines 1205-1206, there are two identical
print statements printing the total link issues; remove one of the duplicate
lines so only a single print(f"\nTotal link issues: {len(issues)}") remains to
avoid repeating the same message.
| if hasattr(config, "model_dump") | ||
| else vars(config) | ||
| ) | ||
| self.config = config.model_dump() if hasattr(config, "model_dump") else vars(config) |
There was a problem hiding this comment.
Config çıkarma fallback'i bazı config tiplerinde başarısız olabilir.
Kod, config objesinin model_dump() veya vars() ile dictionary'ye dönüştürülebileceğini varsayıyor:
self.config = config.model_dump() if hasattr(config, "model_dump") else vars(config)Ancak bazı edge case'lerde bu çalışmayabilir:
- Eğer config bir
dictise,vars(dict)çalışmaz - Bazı custom sınıflar
__dict__attribute'üne sahip olmayabilir - Frozen dataclass'lar veya
__slots__kullanan sınıflarvars()ile çalışmaz
🔎 Önerilen düzeltme
if "config" in checkpoint:
config = checkpoint["config"]
- if isinstance(config, dict):
- self.config = config
- else:
- self.config = config.model_dump() if hasattr(config, "model_dump") else vars(config)
+ if isinstance(config, dict):
+ self.config = config
+ elif hasattr(config, "model_dump"):
+ self.config = config.model_dump()
+ elif hasattr(config, "__dict__"):
+ self.config = vars(config)
+ else:
+ # Fallback: try to convert to dict using asdict from dataclasses
+ try:
+ from dataclasses import asdict
+ self.config = asdict(config)
+ except (TypeError, AttributeError):
+ self.insights.append(
+ TrainingInsight(
+ category="warning",
+ title="Config Extraction Failed",
+ message=f"Unable to extract config of type {type(config)}",
+ )
+ )🤖 Prompt for AI Agents
In scripts/training_insights.py around line 78, the fallback that converts
config to a dict uses only model_dump() or vars(), which fails for dict inputs,
frozen dataclasses, objects with __slots__, or custom classes without __dict__;
change it to robustly handle these cases by: 1) if config is already a dict, use
it; 2) elif it has model_dump(), call that; 3) elif
dataclasses.is_dataclass(config), use dataclasses.asdict(); 4) elif
hasattr(config, "__dict__"), use vars(config) or config.__dict__; 5) else
attempt a safe fallback by trying to call a to_dict()/toJSON()/serialize method
if present or use a try/except around json.dumps with a custom default to
extract attributes, and raise a clear error if none succeed.
| mock_resolve.return_value = Path( | ||
| "C:/Users/Sarpel/Desktop/project_1/models/checkpoints/test.pt" | ||
| ) | ||
| mock_resolve.return_value = Path("C:/Users/Sarpel/Desktop/project_1/models/checkpoints/test.pt") |
There was a problem hiding this comment.
Hardcode edilmiş mutlak path mock testlerde kullanılmamalı.
Mock'ta gerçek bir kullanıcı path'i (C:/Users/Sarpel/Desktop/...) hardcode ediyorsun. Bu:
- Başka geliştirici makinelerinde çalışmaz
- Test'in amacını gizler (neden özel bir path gerekiyor?)
- Mock'ın gerçekten ne test ettiğini anlaşılmaz kılar
Mock'lar genellikle geçerli path'leri simüle etmek için tmp_path'i veya göreceli path'leri kullanmalı.
🔎 Önerilen düzeltme
- mock_resolve.return_value = Path("C:/Users/Sarpel/Desktop/project_1/models/checkpoints/test.pt")
+ mock_resolve.return_value = checkpoint_pathBu değişiklikle, mock gerçek checkpoint path'ini döndürür ve test daha anlamlı olur.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In tests/test_qat_full_pipeline.py around line 59, the mock return value uses a
hardcoded absolute user-specific path which breaks portability and obscures
intent; replace the hardcoded Path with a dynamic temporary path (use the pytest
tmp_path fixture or construct a relative Path within the test) and have the mock
return that Path (e.g., create the checkpoint file under tmp_path /
"models/checkpoints/test.pt" or return Path("models/checkpoints/test.pt")
relative to the test sandbox) so the test runs on other machines and clearly
simulates a valid checkpoint path.
| mock_torch_load.return_value = torch.load( | ||
| checkpoint_path, map_location=device, weights_only=False | ||
| ) | ||
| mock_torch_load.return_value = torch.load(checkpoint_path, map_location=device, weights_only=True) |
There was a problem hiding this comment.
weights_only=True ile checkpoint yükleme başarısız olabilir.
Mock, gerçek checkpoint'i weights_only=True ile yüklüyor. Ancak checkpoint'ler genellikle sadece model ağırlıklarından fazlasını içerir:
configdictionaryhistorydictionaryoptimizer_state_dictepoch,best_val_lossgibi metadata
weights_only=True, sadece tensor verilerini yükler ve yukarıdaki verileri yükleyemez. Bu, torch.load() çağrısının hata vermesine neden olabilir.
Güvenlik açısından weights_only=True kullanmak doğru bir karar, ama bu durumda gerçek checkpoint yerine sadece model_state_dict içeren bir mock dictionary döndürmelisin.
🔎 Önerilen düzeltme
with patch("torch.load") as mock_torch_load:
- mock_torch_load.return_value = torch.load(checkpoint_path, map_location=device, weights_only=True)
+ # Create a mock checkpoint with necessary keys
+ real_checkpoint = torch.load(checkpoint_path, map_location=device, weights_only=False)
+ mock_checkpoint = {
+ "model_state_dict": real_checkpoint.get("model_state_dict", {}),
+ "config": real_checkpoint.get("config"),
+ }
+ mock_torch_load.return_value = mock_checkpointAlternatif olarak, eğer exporter sadece model_state_dict kullanıyorsa:
with patch("torch.load") as mock_torch_load:
- mock_torch_load.return_value = torch.load(checkpoint_path, map_location=device, weights_only=True)
+ real_checkpoint = torch.load(checkpoint_path, map_location=device, weights_only=False)
+ mock_torch_load.return_value = {"model_state_dict": real_checkpoint["model_state_dict"]}🤖 Prompt for AI Agents
In tests/test_qat_full_pipeline.py around line 61, the mock currently calls
torch.load(..., weights_only=True) which can fail because real checkpoints
contain more than tensors; change the mock to return a dict with a
'model_state_dict' key (value should be the state_dict tensors used by the
exporter) instead of actually loading the real checkpoint with
weights_only=True; ensure the mock returns only the model_state_dict (and any
minimal metadata required by the code) so the test uses a safe, controlled
checkpoint-like object.
🎯 PR Review: v4.1 Wyoming Protocol & Progressive Training Release📊 Review SummaryOverall Assessment: ✅ APPROVE with Minor Recommendations This is an excellent PR that introduces production-ready features with strong attention to detail. The code quality is high, security considerations are well-implemented, and test coverage is comprehensive. Stats:
✅ Strengths1. Security Best Practices ⭐The safe_load_checkpoint() implementation (train_with_distillation.py:49-100) demonstrates defense-in-depth:
2. Thread Safety ⭐Excellent use of thread-safe patterns in
Why this matters: Gradio UI runs in multi-threaded context. Without locks, concurrent reads/writes to 3. Wyoming Protocol Server 🏠Professional-grade Home Assistant integration:
4. Progressive Training Architecture ⚡Well-structured progressive scheduler implementation:
5. Documentation Quality 📚Beginner-friendly rewrite follows CLAUDE.md principles:
6. Test Coverage 🧪Outstanding test suite:
|
PR Type
Enhancement, Documentation
Description
Progressive training support: Added
ProgressiveSchedulerwith dynamic augmentation and audio duration updates throughout training phasesThread-safe training state management: Implemented
training_state_lockto prevent race conditions during concurrent access to training historyQuantization-Aware Training (QAT): Added QAT model preparation, confidence calibration, and optimal threshold finding for model optimization
Enhanced data loading: Replaced manual DataLoader creation with centralized
create_dataloaders()factory function for consistent sampler strategy handlingFalse negative collection: Added dual-mode mining queue supporting both false positive and false negative collection with visualization
Restructured configuration presets: Reorganized from 6 general configs to 8 specialized presets (3 hardware-target + 5 training strategy presets) with comprehensive augmentation and distillation configs
Beginner-friendly documentation: Completely rewrote documentation with ELI5 explanations, practical analogies, and non-technical language for accessibility
Multi-augmentation support: Added augmentation multiplier (1-10x) and extraction augmentation controls for batch feature extraction
Updated default parameters: Changed
n_melsto 32 (openWakeWord compatibility),n_fftto 512,audio_durationto 1.5s, batch size to 64, epochs to 150Enhanced evaluation: Added raw audio window evaluation with confidence scores and threshold-based classification
Secure checkpoint loading: Implemented
safe_load_checkpoint()with path traversal prevention and weights-only validationImproved logging: Converted f-string logging to structured format strings for consistency and performance
Documentation maintenance tool: Added comprehensive audit and quality assurance system with link validation and HTML/JSON reporting
Cascade architecture module: New initialization file exporting main components for 3-stage distributed pipeline
Code quality improvements: Fixed formatting, type hints, and error handling throughout codebase; removed obsolete helper scripts and documentation files
Diagram Walkthrough
File Walkthrough
10 files
panel_training.py
Thread-safe training state, progressive training, and improved dataloadingsrc/ui/panel_training.py
import, removed noqa comments, added gc import for memory management
training_state_lockto prevent race conditions during concurrentaccess
Figuretype hint instead ofplt.Figureand added thread-safe data copying in
create_loss_plot(),create_accuracy_plot(),create_metrics_plot()include_mined_negatives,gradient_accumulation_steps, and progressive training settings(
use_progressive,progressive_duration,progressive_augmentation,progressive_difficulty,progressive_min_duration,progressive_phase1_end,progressive_phase2_end)create_dataloaders()factory function for consistent sampler strategyhandling
gc.collect(),torch.cuda.empty_cache()) attraining start and checkpoint configuration validation with detailed
mismatch warnings
configuration mismatch detection when resuming checkpoints
start_training_wrapper()to properly map all UI inputs tofunction parameters including new progressive training options
permissions (0o600)
accumulation, and mined negatives in the training panel
file
evaluator.py
Add raw audio evaluation and QAT model loading supportsrc/evaluation/evaluator.py
get_roc_curve_datafunction from dataset_evaluatormodule
structlog best practices
evaluate_audio()method to evaluate raw audio windowsdirectly with confidence scores and threshold-based classification
checkpoint loading to fuse layers and match checkpoint structure
QAT preparation and better warning messages
parameter passing instead of string interpolation
presets.py
Restructure presets to hardware targets and training strategiessrc/config/presets.py
specialized ones: 3 hardware-target presets (
get_esp32s3_preset,get_rpi_zero2w_preset,get_x86_64_preset) and 5 training strategypresets for fixing specific problems (hard negatives, recall,
overfitting, noise robustness, F1 balance)
and industry-aligned hyperparameters for each preset
get_homeassistant_presetforopenWakeWord/Wyoming protocol compatibility with 32 Mel bands and
TFLite export
mixup, spec augment, and multi-parameter tuning (time stretch, pitch
shift, noise SNR ranges)
QAT settings tailored to each hardware target
separating hardware targets from refinement strategies
panel_evaluation.py
Add false negative collection and dual-mode mining queuesrc/ui/panel_evaluation.py
FalseNegativeCollectorimport and instance to track missedwakewords alongside false positives
collect_false_negatives,generate_fn_gallery_html,clear_false_negatives, andget_fn_queue_datato handle false negative collection and visualization
queue_moderadio button) allowing users toswitch between reviewing false positives and false negatives
update_queue_modefunction to dynamically update UIinstructions and data based on selected mode
clear_mining_queuefunction and improvedconfirm_all_and_inject_handlerto support both FP and FN modesFeatureExtractorinitialization inrun_benchmark_testandimproved type hints with
castandFigureimportsevaluation functions
panel_dataset.py
Add multi-augmentation support and update default parameterssrc/ui/panel_dataset.py
augmentation_multiplierslider(1-10x) and
enable_extraction_augmentationcheckbox for batch featureextraction
n_melsfrom 64 to 32(openWakeWord compatibility),
n_fftfrom 400 to 512,audio_durationto1.5s
batch_extract_handlerto support augmentation multiplier,augmentation config, background noise directory, and RIR directory
parameters
versions (including augmented variants)
auto_start_handlerfunction and associated auto-start pipelineUI components
dataset processing functions
docs_maintenance.py
Documentation maintenance and quality assurance systemscripts/docs_maintenance.py
lines of code
hierarchy, freshness, TODOs, links, alt text, style)
checking and HTML/JSON report generation
categorization
trainer.py
Enhanced trainer with progressive training and QAT supportsrc/training/trainer.py
Windows-specific, type checking, third-party, local)
Trainerattributes withtype hints
mechanism
ProgressiveScheduleranddynamic augmentation/duration updates
with optimal threshold finding
for better performance
_apply_progressive_config()and helper methods for audioduration, augmentation tier, and hard negative ratio updates
CosFace, and combined margin losses
"every_5_epochs"panel_config.py
UI configuration panel updates with mixup and attention supportsrc/ui/panel_config.py
CalibrationConfig,CMVNConfig,SizeTargetConfig,StreamingConfig)use_attention,use_mixup,mixup_alpha,mixup_prob,alpha_increase_rate(512), batch size (64), epochs (150)
RIR/SpecAugment accordion layout
"64, 64, 64, 64",focal alpha to 0.75)
indentation throughout
train_with_distillation.py
Distillation training script with secure checkpoint loadingscripts/train_with_distillation.py
library, third-party, local)
safe_load_checkpoint()function with security validation(path traversal prevention, weights_only loading, structure
validation)
consistency and performance
generic
Exceptioncheckpoint validation
__init__.py
Cascade architecture module initializationsrc/cascade/init.py
SentryModel,JudgeModel,TeacherModel,CascadeOrchestrator,CascadeResultpipeline for wakeword detection
1 files
panel_docs.py
Simplified documentation to beginner-friendly explanationssrc/ui/panel_docs.py
beginner-friendly guide with ELI5 (Explain Like I'm 5) explanations
Configuration, Training, Evaluation, Deployment, Troubleshooting) to 5
sections (Dataset Handling, Configuration Logic, Training &
Evaluation, Export & Deployment, Troubleshooting)
non-technical explanations using analogies (e.g., "Flashcards per
Quiz" for batch size, "Attention Span" for audio duration)
than mathematical definitions
narrative format
quick reference
accessibility to non-ML users
2 files
lr_finder.py
Code formatting and type safety improvementssrc/training/lr_finder.py
and function signatures
plotmethod: castlrs[min_grad_idx]tofloatfor
suggested_lrvariableoperations
verify_full_optimization_stack.py
Minor formatting fix for test filetests/verify_full_optimization_stack.py
101 files
Summary by CodeRabbit
Sürüm Notları - v4.1 "Wyoming Protocol Release"
Yeni Özellikler
Belgelendirme
Bağımlılıklar
Testler
Chores
✏️ Tip: You can customize this high-level summary in your review settings.