fix(android): parse talk directive aliases case-insensitively - #99592
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed July 3, 2026, 3:37 PM ET / 19:37 UTC. Summary PR surface: Other +17. Total +17 across 2 files. Reproducibility: yes. Source inspection shows current main treats mixed-case directive aliases as known but exact-looks up typed values, and the PR head shows the remaining ambiguous-duplicate path in the new helper. Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Keep the Android parser-boundary fix, but first make folded-alias lookup reject ambiguous non-exact duplicates, add focused regression coverage, and either update the Swift parser too or narrow the PR body to Android-only proof. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection shows current main treats mixed-case directive aliases as known but exact-looks up typed values, and the PR head shows the remaining ambiguous-duplicate path in the new helper. Is this the best way to solve the issue? No. The parser is the right layer, but the current helper should preserve exact precedence while rejecting multiple folded non-exact matches; the shared OpenClawKit claim also needs either code or PR-body alignment. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 7cc3cb0d28c8. Label changesLabel justifications:
Evidence reviewedPR surface: Other +17. Total +17 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
ca03b37 to
9563a8e
Compare
9563a8e to
4210df9
Compare
|
Land-ready maintainer proof for exact head
Known live-proof boundary: the isolated Gateway has no speech provider, so it could not synthesize a live assistant reply containing a directive. Typed parser and collision behavior is directly covered on Android and Swift. No before/after screenshot is attached because this parser change has no UI delta. |
|
Merged via squash.
|
What Problem This Solves
Talk directive known-key detection already treated aliases case-insensitively, but typed value lookup remained exact-only. Mixed-case JSON such as
{"Voice":"...","NoSpeakerBoost":true}was therefore reported as known while silently failing to apply the directive. Android and shared Apple implementations had the same mismatch.Why This Change Was Made
User Impact
Generated or dictated Talk directive JSON can vary key casing without losing voice, language, speed, latency, or boolean overrides. Ambiguous duplicate casing fails closed consistently across native clients.
Evidence
Exact head:
4210df90305cf0a2808f39b0d27bf3b29c5bee52ANDROID_HOME=$HOME/Library/Android/sdk ./gradlew :app:testPlayDebugUnitTest --tests ai.openclaw.app.voice.TalkDirectiveParserTestANDROID_HOME=$HOME/Library/Android/sdk ./gradlew :app:assemblePlayDebugswift test --package-path apps/shared/OpenClawKit --filter TalkDirectiveTeststalk.ptt.cancelroundtrip returnedstatus: idle.Known live-proof boundary: the isolated Gateway has no configured speech provider, so it could not synthesize a live assistant reply containing the directive. The complete typed parser behavior and collision contract are exercised on both native implementations; the real Gateway/node smoke verifies the candidate Talk runtime remains connected and callable.
No screenshot is attached because the parser change has no UI delta.