Repository navigation
feat: ship standalone in-process providers and YOLOX local mode - #3
Conversation
|
Warning Review limit reached
Next review available in: 36 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThis PR removes the standalone provider broker (FastAPI app, systemd service, deploy docs, caregiver auth guard, readiness script, and related tests) and replaces it with an in-process fixed-policy OpenAI provider plus a new offline local ONNX/YOLOX detection and Sherpa-ONNX TTS mode. Configuration, API routes, frontend UI, runtime cancellation/stop handling, packaging, and documentation are all updated to reflect the 0.2.0 standalone release with provider selection (openai/local) instead of broker credentials. ChangesStandalone provider redesign
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant UI
participant Main as main.py
participant Provider as ProviderClient
participant OpenAI as OpenAI API
participant Local as LocalProvider
UI->>Main: POST /api/game/start
Main->>Main: check load_config().configured
Main->>Provider: select_target(frames)
alt provider == openai
Provider->>OpenAI: POST chat/completions
OpenAI-->>Provider: JSON target
else provider == local
Provider->>Local: select_target(frames)
Local-->>Provider: Target
end
Provider-->>Main: validated Target
Main-->>UI: game state
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (3)
reachy_mini_i_spy/static/index.html (1)
69-69: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: stale wording in helper text.
"provider URL" no longer applies (the broker/URL was removed), and the hardcoded "$0.10/month in credits" figure will likely drift out of date. Consider trimming to just the fixed-policy statement.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@reachy_mini_i_spy/static/index.html` at line 69, Update the helper text paragraph near the fixed policy statement to remove the stale provider URL reference and the time-sensitive Hugging Face credit amount, retaining only the accurate statement about fixed models, prompts, and safety bounds.reachy_mini_i_spy/local_assets.py (1)
98-108: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor: hardcoded
download_bytescan drift from the registry.
local_assets_status()reports a fixed34_483_424regardless of the actualVOICE_ASSETScontents. If a voice asset is added/changed, this estimate silently goes stale.♻️ Suggested fix
- "download_bytes": 34_483_424, + "download_bytes": sum(0 if _voice_ready(a) else _ARCHIVE_SIZE_ESTIMATE for a in VOICE_ASSETS.values()),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@reachy_mini_i_spy/local_assets.py` around lines 98 - 108, Update local_assets_status() so download_bytes is calculated from the current VOICE_ASSETS registry rather than a hardcoded constant, summing each registered voice asset’s expected download size using the existing asset metadata.reachy_mini_i_spy/local_provider.py (1)
271-315: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winLocal target's
minimum_confidencegate is effectively a no-op.
_detect()(line 191) already discards any detection withscore < self.SCORE_THRESHOLD, so every candidate passed intoselect_target()already satisfiesscore >= SCORE_THRESHOLD. Passingminimum_confidence=self.SCORE_THRESHOLD(line 313) tovalidate_targettherefore can never reject on confidence — the check is vacuously true. This also means the local provider's effective confidence floor (0.30) is much lower than the cloud provider's fixed 0.78 floor; worth confirming that's the deliberate design rather than a leftover placeholder.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@reachy_mini_i_spy/local_provider.py` around lines 271 - 315, Review the confidence-threshold flow between _detect and select_target: validate_target’s minimum_confidence check is redundant because _detect already filters detections below SCORE_THRESHOLD. Remove the vacuous validation argument or otherwise enforce the intended local confidence floor explicitly, and confirm the local threshold is deliberately distinct from the cloud provider’s 0.78 floor. Keep target validation and selection behavior unchanged apart from correcting the effective confidence policy.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/ARCHITECTURE.md`:
- Around line 68-75: Update the local-mode description near the “local path”
statement to clarify that it replaces both cloud vision and cloud speech
synthesis backends with ONNX object detection and sherpa-onnx, while leaving the
safety state machine unchanged. Preserve the listed safety, determinism,
language-table, stop/generation, and licensing details.
In `@reachy_mini_i_spy/config.py`:
- Around line 29-38: The public_dict method currently performs
local_assets_status verification on every status/settings request, including
OpenAI mode. Update public_dict to avoid computing local_assets when provider is
not "local", or cache detector verification using the detector file’s size and
modification time; preserve the existing local_assets payload behavior for
local-provider configurations.
In `@reachy_mini_i_spy/local_provider.py`:
- Around line 15-16: Add the sherpa-onnx core/runtime dependency at version
1.13.4 to the relevant CI installation step before jobs import
local_provider.py. Ensure the CI platform installs the package that provides
libonnxruntime.so, while preserving the existing onnxruntime and sherpa_onnx
imports.
- Around line 396-401: Update the callback passed in the TTS engine’s generate
call to return a non-zero value while synthesis should continue and return zero
when self._cancelled is set. Preserve the existing cancellation behavior by
inverting the _cancelled.is_set() result.
In `@reachy_mini_i_spy/main.py`:
- Around line 97-118: Restrict the `settings_app` server binding to localhost
instead of `0.0.0.0`, updating the `custom_app_url` configuration used when
creating the app. Preserve the existing settings routes and ensure the app
remains reachable from the device itself while no longer accepting
network-interface connections.
In `@README.md`:
- Line 59: Update the README paragraph describing Hugging Face hosted inference
to remove the mutable “$0.10/month” credit figure, or replace it with a dated,
authoritative reference. Apply the same correction to the matching inline note
while preserving the explanation that free users require an HF token and hosted
inference is not used as fallback.
In `@SECURITY.md`:
- Line 5: Update the “Supported version” section in SECURITY.md to explicitly
identify 0.2.0 as the supported release and retain the applicable acceptance
profile; if 0.2.0 is not yet accepted, clearly label the policy as pre-release
instead of using an anonymous in-development release description.
In `@tests/test_public_positioning.py`:
- Line 39: Replace the stale "provider broker" assertion in the public
positioning test with an assertion matching the new provider-selection or
local-mode label rendered in index.html, while preserving the surrounding
page-content checks.
---
Nitpick comments:
In `@reachy_mini_i_spy/local_assets.py`:
- Around line 98-108: Update local_assets_status() so download_bytes is
calculated from the current VOICE_ASSETS registry rather than a hardcoded
constant, summing each registered voice asset’s expected download size using the
existing asset metadata.
In `@reachy_mini_i_spy/local_provider.py`:
- Around line 271-315: Review the confidence-threshold flow between _detect and
select_target: validate_target’s minimum_confidence check is redundant because
_detect already filters detections below SCORE_THRESHOLD. Remove the vacuous
validation argument or otherwise enforce the intended local confidence floor
explicitly, and confirm the local threshold is deliberately distinct from the
cloud provider’s 0.78 floor. Keep target validation and selection behavior
unchanged apart from correcting the effective confidence policy.
In `@reachy_mini_i_spy/static/index.html`:
- Line 69: Update the helper text paragraph near the fixed policy statement to
remove the stale provider URL reference and the time-sensitive Hugging Face
credit amount, retaining only the accurate statement about fixed models,
prompts, and safety bounds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fd27127d-4616-464c-a1ad-66f7d86a3e83
📒 Files selected for processing (39)
CHANGELOG.mdMANIFEST.inREADME.mdSECURITY.mddeploy/README.mddeploy/hermes-ispy-broker.servicedocs/ADDING_A_LANGUAGE.mddocs/ARCHITECTURE.mddocs/RELEASE_PROVENANCE.mddocs/SAFETY_CONTRACT.mdhermes_broker/__init__.pyhermes_broker/app.pyindex.htmlpyproject.tomlreachy_mini_i_spy/auth.pyreachy_mini_i_spy/config.pyreachy_mini_i_spy/game.pyreachy_mini_i_spy/local_assets.pyreachy_mini_i_spy/local_provider.pyreachy_mini_i_spy/main.pyreachy_mini_i_spy/models/README.mdreachy_mini_i_spy/models/YOLOX_LICENSE.txtreachy_mini_i_spy/models/__init__.pyreachy_mini_i_spy/models/yolox_nano.onnxreachy_mini_i_spy/provider.pyreachy_mini_i_spy/runtime.pyreachy_mini_i_spy/static/index.htmlreachy_mini_i_spy/static/main.jsscripts/check_artifacts.pyscripts/verify_broker_readiness.pytests/test_auth.pytests/test_broker.pytests/test_config.pytests/test_gate_adversarial.pytests/test_local_assets.pytests/test_local_provider.pytests/test_provider.pytests/test_public_positioning.pytests/test_runtime.py
💤 Files with no reviewable changes (9)
- hermes_broker/init.py
- scripts/verify_broker_readiness.py
- deploy/README.md
- tests/test_auth.py
- reachy_mini_i_spy/auth.py
- MANIFEST.in
- deploy/hermes-ispy-broker.service
- hermes_broker/app.py
- tests/test_broker.py
|
Review remediation complete on
Verification: 117 local tests, GitHub CI green, artifact checks green, official Pollen clean install/entry-point/uninstall validator green, exact Wireless artifact acceptance green. |
Summary
Verification
reachy-mini-app-assistant checkpassed on the CM4, including clean install, entry-point registration and uninstallbd30ae8a4d4cb5fa1327d66bf323421d277fb16c045a6b99a503eead0c94aa7aphysically accepted on Reachy Mini WirelessHardware scope
Summary by CodeRabbit
New Features
Safety
Documentation