78 lines
10 KiB
Markdown
78 lines
10 KiB
Markdown
# Chanora Maintainability Review — 2026-06-08
|
|
|
|
**Document status:** Working-branch review record
|
|
**Branch:** `simplify-project-review`
|
|
**Scope:** Project-wide simplification, fail-safe, and verification review
|
|
|
|
## 1. Purpose
|
|
|
|
This record captures the current maintainability review so implementation, verification, and release documents do not drift behind the code. It focuses on unnecessary Modules, shallow Interfaces, duplicate Implementations, built-in replacement opportunities, and fail-safe gaps that need explicit evidence before release claims.
|
|
|
|
## 2. Changes Already Applied on the Branch
|
|
|
|
| Area | Files | Maintainability result |
|
|
|---|---|---|
|
|
| Core event DTO locality | `core/chanora_core/src/events.rs`, `core/chanora_core/src/lib.rs` | Public Core event DTOs moved out of the oversized Core integration Module while preserving the public `chanora_core::*` Interface through re-exports. |
|
|
| Core network diagnostics locality | `core/chanora_core/src/network_diagnostics.rs`, `core/chanora_core/src/lib.rs` | Private network diagnostic ring-buffer state and its regression test now live next to the Implementation they protect. |
|
|
| Bounded queues | `core/chanora_core/src/network_diagnostics.rs`, `crates/chanora_diagnostics/src/lib.rs` | Replaced `Vec + remove(0)` queue behaviour with `VecDeque`, reducing custom queue code and avoiding O(n) front removal. |
|
|
| PTT backend errors | `crates/chanora_audio/src/ptt_backends/mod.rs` | Replaced manual `Display` / `Error` Implementation with existing `thiserror::Error`; regression test keeps user-facing strings stable. |
|
|
| Render downmix | `crates/chanora_audio/src/voice_render.rs`, `crates/chanora_audio/src/ios_raw_unit.rs` | Removed duplicate mono-i16 downmix loop by using the interleaved helper with one output channel. |
|
|
| State reducer | `crates/chanora_state/src/lib.rs` | Reused the main snapshot reducer for reconnect snapshots instead of duplicating normalization and delta construction. |
|
|
| Workspace metadata | `crates/chanora_resolver/Cargo.toml`, `Cargo.lock` | Resolver inherits workspace package metadata, improving release metadata Locality. |
|
|
| Flutter voice fail-safes | `apps/chanora_flutter/lib/main.dart`, `apps/chanora_flutter/lib/widgets/voice_compact.dart`, `apps/chanora_flutter/lib/services/ios_audio_session_controller.dart` | Commit `d835394` preserves independent mute owners, releases touch PTT on disposal while held, and catches iOS audio-session `MissingPluginException` / activation failures so they do not become unhandled async errors. |
|
|
| Rust realtime callback hardening | `crates/chanora_audio/src/android_voice_unit.rs`, `crates/chanora_audio/src/ios_raw_unit.rs`, `crates/chanora_audio/src/engine.rs` | Commit `8606eb4` hardens Android/iOS realtime callback paths. Android target compilation and device runtime verification are still blocked locally; full lock-free audio-handler/config/debug-recorder redesign remains follow-up work. |
|
|
|
|
## 3. Remaining Simplification Opportunities
|
|
|
|
| Recommendation | Candidate files | Strength | Notes |
|
|
|---|---|---|---|
|
|
| Continue splitting Core internals by responsibility | `core/chanora_core/src/lib.rs` | Strong | Next slices should be reconnect/session, voice projection, storage helpers, and diagnostics export. Keep public re-exports stable. |
|
|
| Make Bridge depend on Core rather than Audio where possible | `crates/chanora_bridge/Cargo.toml`, `crates/chanora_bridge/src/api.rs`, `core/chanora_core/src/lib.rs` | Worth exploring | The Bridge currently has a direct audio edge. Apply the deletion test before removing it. |
|
|
| Decide whether prefetch deserves a crate-level Seam | `crates/chanora_prefetch/src/lib.rs`, `crates/chanora_resolver/src/lib.rs`, `core/chanora_core/src/lib.rs` | Worth exploring | Prefetch is a small TTL cache and fire-and-forget resolver Adapter. Merge into resolver if it is resolver policy; merge into Core if it is app orchestration policy. |
|
|
| Consolidate protocol/core/bridge event catalogues | `crates/chanora_protocol/src/dto.rs`, `core/chanora_core/src/events.rs`, `crates/chanora_bridge/src/api.rs` | Worth exploring | Protocol-owned deltas and Core-owned lifecycle events are currently mirrored through multiple DTO layers. |
|
|
| Reduce bridge DTO mirror boilerplate | `crates/chanora_bridge/src/api.rs` | Worth exploring | Verify Flutter Rust Bridge support before deleting mirrors. If mirrors remain required, centralize conversion patterns and keep field order aligned with Core DTOs. |
|
|
| Clarify protocol voice packet exception | `crates/chanora_protocol/src/lib.rs`, `crates/chanora_audio/Cargo.toml` | Worth exploring | The protocol crate documents `tsclientlib` isolation but deliberately re-exports voice packet types for audio. Document this as an explicit voice wire Seam or move packet construction fully into protocol. |
|
|
| Remove shallow audio helpers only after public API check | `crates/chanora_audio/src/processor/noop.rs`, `crates/chanora_audio/src/processor/platform.rs`, `crates/chanora_audio/src/frame.rs` | Speculative | These Modules are shallow, but deletion must wait until external/public API expectations are checked. |
|
|
| Redesign remaining audio shared state outside realtime callbacks | `crates/chanora_audio/src/engine.rs`, platform voice units, debug recorder/config paths | Strong follow-up | The focused callback hardening is complete, but a full lock-free `AudioHandler` / config / debug-recorder redesign should be planned separately and verified on device. |
|
|
| Review protocol/core disconnect and control-plane bounds | `core/chanora_core/src/lib.rs`, `crates/chanora_protocol/src/adapter.rs` | Strong follow-up | Unless closed by a later code slice, sustained voice traffic and broken transport should be reviewed for bounded control request and disconnect progress. |
|
|
|
|
## 4. Fail-Safe Gaps That Need Evidence
|
|
|
|
| Gap | Risk | Required evidence before release claim |
|
|
|---|---|---|
|
|
| Android Keystore-backed DEK remains deferred | Android identity/bookmark encryption has weaker fail-safe properties than final target secure-storage design. | Android secure-storage audit or waiver; explicit release-readiness limitation. |
|
|
| Android runtime verification not executed in this review | Rust unit tests pass, but device permission/audio/lifecycle paths are not proven without a connected device or emulator. | `adb devices -l` showing a target plus Android build/install/smoke evidence. |
|
|
| Android target compilation blocked locally | Android build evidence cannot be recorded while the Android NDK compiler `aarch64-linux-android-clang` is unavailable in the local toolchain. | Install/fix the Android NDK toolchain, rerun Android target build, then install/smoke on an authorized target. |
|
|
| iOS device runtime verification not executed in this review | The iOS audio-session error path is hardened, but VoiceProcessingIO/session ordering and runtime audio behavior still need device evidence. | iOS device or simulator build/run plus audio-session smoke evidence before iOS runtime success is claimed. |
|
|
| VAD / VoiceActivity wording drift | VAD assets, tests, and scaffolding exist, but product-enabled `VoiceActivity` remains reserved/disabled per DEC-030. | Release, README, and verification wording must distinguish scaffolding/assets/tests from shipped product behavior. |
|
|
| Protocol voice packet re-export is an intentional exception | Future maintainers may assume complete protocol isolation and accidentally widen the Seam. | Architecture note in SAD/SDD or a decision-register entry. |
|
|
| Bridge DTO mirror drift | Field additions can be missed across Core, Bridge, and Dart generated DTOs. | Bridge generation check plus Flutter analyze/test after bridge DTO changes. |
|
|
| Full live reducer integration remains separate from reducer unit coverage | State reducer tests are strong, but runtime UI still has snapshot/probe paths. | SWE.5 integration run proving live protocol events fold through the intended state path, or explicit P1 deferral. |
|
|
| Event replay tooling remains absent | Replay-based diagnosis and regression reproduction are limited. | Event replay tool implementation or waiver. |
|
|
|
|
## 5. Verification Policy for Future Code Changes
|
|
|
|
| Change type | Required verification |
|
|
|---|---|
|
|
| Rust-only change | `cargo fmt --all`, `cargo check --workspace`, `cargo test --workspace` |
|
|
| Bridge DTO/API change | Rust verification plus bridge generation check, `flutter analyze`, and `flutter test --exclude-tags e2e` in `apps/chanora_flutter` |
|
|
| Android platform/audio/permission change | Rust/Flutter verification plus NDK target compilation, `adb devices -l`, Android build/install, and a device or emulator smoke test |
|
|
| Documentation-only change | Read affected docs and ensure cross-links/document index stay current; code tests are not required unless docs describe a code change just made |
|
|
|
|
## 6. Android ADB Status for This Review
|
|
|
|
`adb` is installed at `/opt/homebrew/bin/adb`, but `adb devices -l` returned no connected devices. Android runtime verification is therefore blocked until a device or emulator is connected and authorized.
|
|
|
|
Android target compilation is also blocked locally until the Android NDK compiler `aarch64-linux-android-clang` is available. No Android runtime success, permission-flow success, or audio-lifecycle success is claimed by this review.
|
|
|
|
## 7. Release and Documentation Alignment Notes
|
|
|
|
- Android minimum runtime baseline is API 28 (Android 9.0) per SysRS-288, SRS-187, DEC-004, and the Gradle `minSdk = 28` configuration. Documents must not revive the older API 24 baseline.
|
|
- Flutter app version/build is `0.3.0+100` in `apps/chanora_flutter/pubspec.yaml`. Rust workspace package version remains `0.2.0-beta.1`. Release documents must distinguish these values instead of treating them as one candidate version.
|
|
- The v0.3.0 changelog entry may mention VAD assets/backends only as implementation scaffolding; product-enabled `VoiceActivity` remains disabled/coming-soon until DEC-030 is superseded and runtime verification exists.
|
|
- README wording must describe the existing Flutter/Rust workspace and app scaffold, not a future scaffold that has not been created.
|
|
|
|
## 8. Git Policy
|
|
|
|
No commit is created automatically. Commit only on explicit user demand, after reviewing `git status`, `git diff`, and recent log output.
|