docs: apply review corrections and add second-pass review reports
Corrections: - README: add 6 missing files to index, fix integration test count (2→6) - coverage-analysis: fix chanora_audio tests (221→333), chanora_core (11→38), integration tests (2→6) - docs-out-of-date: remove false maintainability-review claim - link-coverage-report + docs-link-not-covered: fix line number (12→11) New review reports (5): - function-inventory-review.md (score: 7/10) - coverage-docquality-review.md (coverage: 4/10, docquality: 7/10) - mismatch-outofdate-review.md (mismatch: 8/10, outofdate: 9/10) - link-reports-review.md (link: 8/10, notcovered: 7/10) - external-index-review.md (external: 8-9/10, index: 6/10)
This commit is contained in:
Edison Jwa
@@ -0,0 +1,214 @@
|
||||
# Function Inventory Review
|
||||
|
||||
> Review of `docs/offline-knowledge/function-inventory.md` for accuracy, completeness, and quality.
|
||||
> Reviewed on: 2026-06-13
|
||||
|
||||
---
|
||||
|
||||
## 1. Accuracy Check (10 Random Entries)
|
||||
|
||||
**Result: PASS (10/10 correct)**
|
||||
|
||||
| # | Entry | File:Line | Signature | Purpose | Verdict |
|
||||
|---|-------|-----------|-----------|---------|---------|
|
||||
| 1 | `BlobCache::put` | lib.rs:63 | `pub async fn put(&self, prefix: &str, key: &str, data: &[u8]) -> Result<(), BlobCacheError>` | Store a blob with prefix+key | ✅ |
|
||||
| 2 | `ProtocolClient::connect` | adapter.rs:304 | `pub async fn connect(cfg: ConnectConfig) -> Result<Self, ProtocolError>` | Dial server, wait for initial snapshot | ✅ |
|
||||
| 3 | `BridgeChannel` | api.rs:404 | `pub struct BridgeChannel { ... }` | Channel DTO for Dart | ✅ |
|
||||
| 4 | `IdentityFileStore::load` | lib.rs:390 | `pub fn load(&self) -> Result<Option<String>, StorageError>` | Read persisted identity | ✅ |
|
||||
| 5 | `ServerState::from_snapshot` | lib.rs:83 | `pub fn from_snapshot(snapshot: ServerSnapshot) -> Self` | Build from initial snapshot | ✅ |
|
||||
| 6 | `AudioEngine::start` | engine.rs:634 | `pub fn start(cfg: AudioEngineConfig, ...) -> Result<Self, AudioError>` | Start audio engine | ✅ |
|
||||
| 7 | `ChanoraResolver::resolve` | lib.rs:178 | `pub async fn resolve(&self, args: &Args) -> Result<Resolution>` | Resolve with Args | ✅ |
|
||||
| 8 | `ServerPrefetcher::prefetch` | lib.rs:99 | `pub async fn prefetch(&self, host: String) -> Result<(), ServerPrefetchError>` | Schedule fire-and-forget prefetch | ✅ |
|
||||
| 9 | `Redactor::redact` | lib.rs:150 | `pub fn redact(&self, s: &str) -> String` | Apply redaction policy | ✅ |
|
||||
| 10 | `ChanoraSession::new` | lib.rs:245 | `pub fn new() -> Self` | Create session | ✅ |
|
||||
|
||||
---
|
||||
|
||||
## 2. Completeness Check (3 Random Source Files)
|
||||
|
||||
**Result: PASS with 1 error**
|
||||
|
||||
### Rust: `chanora_state/src/lib.rs`
|
||||
All public items verified present in inventory:
|
||||
- `ServerState`, `Reduction`, `ConnectionState`, `Delta`, `StateEvent`, `StateError`
|
||||
- All `ServerState` methods (`from_snapshot`, `replace_from_snapshot`, `channel`, `client`, `channels`, `clients`, `channel_count`, `client_count`, `own_channel`, `clients_in_channel`)
|
||||
- `reduce`, `reduce_reconnect_snapshot`
|
||||
|
||||
**Verdict: ✅ Complete**
|
||||
|
||||
### Dart Widget: `voice_compact.dart`
|
||||
- `VoiceStatusChip` at line 46 ✅
|
||||
- `VoicePttButton` at line 255 ✅
|
||||
|
||||
**Verdict: ✅ Complete**
|
||||
|
||||
### Dart Service: `snapshot_state_mapper.dart`
|
||||
- Inventory lists: `SnapshotStateMapper` at line 43
|
||||
- Actual file contains:
|
||||
- `OwnClientSnapshotState` class at line 3
|
||||
- `ownClientSnapshotState()` function at line 23
|
||||
- `snapshotChannelName()` function at line 43
|
||||
- `snapshotNeededTalkPower()` function at line 48
|
||||
|
||||
**Verdict: ❌ Error** — The inventory lists a non-existent class name `SnapshotStateMapper`. The actual class is `OwnClientSnapshotState` (line 3), and the file contains 3 public functions not listed individually.
|
||||
|
||||
---
|
||||
|
||||
## 3. Dead Code Analysis (3 Items)
|
||||
|
||||
**Result: PASS (3/3 correct)**
|
||||
|
||||
| Claimed Dead Code | Verification | Verdict |
|
||||
|-------------------|--------------|---------|
|
||||
| `publish_permission_state` — `#[cfg_attr(not(target_os = "android"), allow(dead_code))]` | Confirmed at `api.rs:190-191`: `#[cfg_attr(not(target_os = "android"), allow(dead_code))]` | ✅ |
|
||||
| `run()` in chanora_resolver — CLI entry point | Confirmed at `lib.rs:804`: `pub async fn run(args: Args) -> Result<()>` | ✅ |
|
||||
| Platform-gated items (`AndroidVoiceUnit`, `IosVoiceUnit`) | Confirmed: these are `#[cfg]`-gated | ✅ |
|
||||
|
||||
---
|
||||
|
||||
## 4. Useless Code (3 Items)
|
||||
|
||||
**Result: PASS**
|
||||
|
||||
The inventory claims:
|
||||
- No empty impls found
|
||||
- No commented-out function bodies found
|
||||
- No dead trait implementations found
|
||||
|
||||
Verified by searching for empty `impl` blocks and commented-out function bodies. No issues found.
|
||||
|
||||
**Verdict: ✅ Correct**
|
||||
|
||||
---
|
||||
|
||||
## 5. Formatting Check
|
||||
|
||||
**Result: PASS with minor issues**
|
||||
|
||||
| Check | Status | Notes |
|
||||
|-------|--------|-------|
|
||||
| Table alignment | ✅ | All tables properly formatted |
|
||||
| Broken links | ✅ | No links in document |
|
||||
| Missing entries | ⚠️ | `snapshot_state_mapper.dart` has missing public functions |
|
||||
| Duplicate entries | ✅ | No duplicates found |
|
||||
| Consistent column headers | ✅ | All tables use same format |
|
||||
|
||||
---
|
||||
|
||||
## 6. Stats Verification
|
||||
|
||||
**Result: PASS with 4 errors in enum variant counts**
|
||||
|
||||
| Stat | Claimed | Verified | Status |
|
||||
|------|---------|----------|--------|
|
||||
| Rust Crates | 10 | 10 | ✅ |
|
||||
| Rust pub fn | ~180 | Plausible | ✅ |
|
||||
| Rust pub struct | ~90 | Plausible | ✅ |
|
||||
| Rust pub enum | ~50 | Plausible | ✅ |
|
||||
| Rust pub trait | 6 | Plausible | ✅ |
|
||||
| Rust pub const | ~30 | Plausible | ✅ |
|
||||
| Dart files | 56 | Plausible | ✅ |
|
||||
| Dart public classes | ~80 | Plausible | ✅ |
|
||||
| TODO/FIXME comments | 15 | 15 (verified) | ✅ |
|
||||
| Empty/commented stubs | 0 | 0 (verified) | ✅ |
|
||||
|
||||
### Enum Variant Count Errors
|
||||
|
||||
| Enum | Location | Claimed | Actual | Status |
|
||||
|------|----------|---------|--------|--------|
|
||||
| `CoreError` | chanora_core lib.rs:84 | 7 variants | 12 variants | ❌ |
|
||||
| `ProtocolError` | chanora_protocol lib.rs:62 | 9 variants | 10 variants | ❌ |
|
||||
| `BridgeError` | chanora_bridge lib.rs:55 | 8 variants | 7 variants | ❌ |
|
||||
| `AudioError` | chanora_audio lib.rs:100 | 7 variants | 8 variants | ❌ |
|
||||
|
||||
**Actual variant counts:**
|
||||
|
||||
`CoreError` (12 variants):
|
||||
1. Protocol
|
||||
2. State
|
||||
3. Audio
|
||||
4. Storage
|
||||
5. Cache
|
||||
6. FileTransfer
|
||||
7. Diagnostics
|
||||
8. Invariant
|
||||
9. NotConnected
|
||||
10. AlreadyConnected
|
||||
11. AudioNotStarted
|
||||
12. Ptt
|
||||
|
||||
`ProtocolError` (10 variants):
|
||||
1. Invalid
|
||||
2. DnsFailed
|
||||
3. Connect
|
||||
4. DisconnectedEarly
|
||||
5. Lost
|
||||
6. Identity
|
||||
7. Timeout
|
||||
8. ServerRejected
|
||||
9. Backend
|
||||
10. FileTransfer
|
||||
|
||||
`BridgeError` (7 variants):
|
||||
1. InvalidCommand
|
||||
2. DnsFailed
|
||||
3. Connection
|
||||
4. NotConnected
|
||||
5. AlreadyConnected
|
||||
6. ServerRejected
|
||||
7. Unmapped
|
||||
|
||||
`AudioError` (8 variants):
|
||||
1. NoInputDevice
|
||||
2. NoOutputDevice
|
||||
3. StreamConfig
|
||||
4. Opus
|
||||
5. Backend
|
||||
6. PlatformNotReady
|
||||
7. InvalidAudioProcessingConfig
|
||||
8. UnsupportedAudioProcessingConfig
|
||||
|
||||
---
|
||||
|
||||
## Corrections Needed
|
||||
|
||||
1. **`snapshot_state_mapper.dart` entry** (line ~558):
|
||||
- Change `SnapshotStateMapper` → `OwnClientSnapshotState`
|
||||
- Change line reference from `:43` to `:3`
|
||||
- Add missing public functions:
|
||||
- `ownClientSnapshotState` at line 23
|
||||
- `snapshotChannelName` at line 43
|
||||
- `snapshotNeededTalkPower` at line 48
|
||||
|
||||
2. **`CoreError` variant count** (line ~461):
|
||||
- Change "7 variants" → "12 variants"
|
||||
|
||||
3. **`ProtocolError` variant count** (line ~65):
|
||||
- Change "9 variants" → "10 variants"
|
||||
|
||||
4. **`BridgeError` variant count** (line ~116):
|
||||
- Change "8 variants" → "7 variants"
|
||||
|
||||
5. **`AudioError` variant count** (line ~260):
|
||||
- Change "7 variants" → "8 variants"
|
||||
|
||||
---
|
||||
|
||||
## Overall Quality Score
|
||||
|
||||
**Score: 7/10**
|
||||
|
||||
**Strengths:**
|
||||
- Excellent file:line accuracy (100% on sampled entries)
|
||||
- Good signature documentation
|
||||
- Comprehensive coverage of Rust crates
|
||||
- Proper dead code analysis with correct `#[cfg]` annotations
|
||||
- Clean formatting and consistent structure
|
||||
|
||||
**Weaknesses:**
|
||||
- 4 enum variant count errors (off by 1-5)
|
||||
- 1 incorrect Dart class name in Services table
|
||||
- Missing 3 public functions from `snapshot_state_mapper.dart`
|
||||
- No verification of variant counts against source
|
||||
|
||||
**Recommendation:** Fix the 5 corrections listed above. The document is otherwise high quality and suitable for developer reference.
|
||||
Reference in New Issue
Block a user