Skip to content

fix(android): parse talk directive aliases case-insensitively - #99592

Merged
steipete merged 1 commit into
openclaw:mainfrom
ly85206559:codex/android-talk-directive-case-keys
Jul 3, 2026
Merged

steipete merged 1 commit into
openclaw:mainfrom
ly85206559:codex/android-talk-directive-case-keys

Conversation

@ly85206559

@ly85206559 ly85206559 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

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

  • Apply case-insensitive alias lookup to every typed directive accessor on Android and in shared OpenClawKit.
  • Preserve deterministic alias order and exact-key precedence.
  • Reject multiple non-exact keys that fold to the same alias instead of depending on Android/Swift map iteration order.
  • Cover strings, booleans, integers, doubles, exact duplicates, and ambiguous duplicates on both platforms.

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: 4210df90305cf0a2808f39b0d27bf3b29c5bee52

  • ANDROID_HOME=$HOME/Library/Android/sdk ./gradlew :app:testPlayDebugUnitTest --tests ai.openclaw.app.voice.TalkDirectiveParserTest
  • ANDROID_HOME=$HOME/Library/Android/sdk ./gradlew :app:assemblePlayDebug
  • swift test --package-path apps/shared/OpenClawKit --filter TalkDirectiveTests
  • Fresh Codex autoreview after fixing its deterministic-collision finding: no actionable findings.
  • Final Android candidate installed on an Android 16 emulator connected to an isolated real Gateway; a real Gateway-to-node talk.ptt.cancel roundtrip returned status: 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.

@openclaw-barnacle openclaw-barnacle Bot added app: android App: android size: XS triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. and removed triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jul 3, 2026
@clawsweeper

clawsweeper Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 3, 2026, 3:37 PM ET / 19:37 UTC.

Summary
The PR changes Android Talk directive value lookup to fall back to case-insensitive JSON key matching and adds a mixed-case Android parser unit test.

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: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦐 gold shrimp
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Fix ambiguous folded-alias handling and add Android regression coverage for the conflict case.
  • [P1] Add after-fix real behavior proof that a mixed-case directive is parsed and applied in Android Talk mode, with private endpoints, tokens, phone numbers, and other sensitive details redacted.
  • [P1] Align the PR body with the actual diff by either adding the shared OpenClawKit fix/proof or narrowing the claims to Android.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR body includes an emulator/Gateway smoke, but it does not show the changed mixed-case directive parsing behavior or an observed improved Talk request after the fix. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] The supplied real setup proof only shows an Android emulator/Gateway roundtrip for talk.ptt.cancel returning idle; it does not show a mixed-case Talk directive being parsed or applied after the fix.
  • [P1] The PR body still claims shared OpenClawKit and duplicate-collision coverage, while the live diff only changes Android and the Swift parser remains exact-key lookup on current main.

Maintainer options:

  1. Decide the mitigation before merge
    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.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] A narrow code repair is available, but external contributor real behavior proof is still required before merge, so this should stay in human review rather than an automated repair lane.

Security
Cleared: The diff only changes Android parser logic and a unit test; it does not touch dependencies, workflows, lockfiles, secrets, permissions, or code download/execution paths.

Review findings

  • [P2] Reject ambiguous folded directive keys — apps/android/app/src/main/java/ai/openclaw/app/voice/TalkDirectiveParser.kt:208
Review details

Best 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:

  • [P2] Reject ambiguous folded directive keys — apps/android/app/src/main/java/ai/openclaw/app/voice/TalkDirectiveParser.kt:208
    The new fallback returns the first case-insensitive JSON entry when no exact key exists. With input like {"Voice":"a","VOICE":"b"}, the parser silently applies one conflicting voice while unknown-key reporting treats both keys as known; detect multiple non-exact folded matches and fail closed, with a regression test.
    Confidence: 0.91

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 7cc3cb0d28c8.

Label changes

Label justifications:

  • P2: This is a focused Android Talk parser correctness fix with limited blast radius but a real user-visible voice/directive impact.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body includes an emulator/Gateway smoke, but it does not show the changed mixed-case directive parsing behavior or an observed improved Talk request after the fix. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Other +17. Total +17 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 0 0 0 0
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 2 22 5 +17
Total 2 22 5 +17

What I checked:

Likely related people:

  • obviyus: Authored and merged the central Android Talk gateway-synthesis PR that established the playback path this parser feeds. (role: feature owner / adjacent area owner; confidence: high; commits: 61965e500f93, e3afaca1a61d, 98d593956493; files: apps/android/app/src/main/java/ai/openclaw/app/voice/TalkModeManager.kt, apps/android/app/src/main/java/ai/openclaw/app/voice/TalkSpeakClient.kt)
  • Solvely-Colin: Authored the recent merged Android Voice/Talk readiness and catalog-alias work adjacent to this Talk mode surface. (role: recent adjacent contributor; confidence: medium; commits: 7cfc66ad0746, 161b585d5a4b; files: apps/android/app/src/main/java/ai/openclaw/app/voice/TalkModeManager.kt)
  • steipete: Merged the adjacent Android Voice readiness PR and is currently assigned on this PR, making him a likely routing candidate for maintainer review. (role: recent merger / assigned reviewer; confidence: medium; commits: 7cfc66ad0746; files: apps/android/app/src/main/java/ai/openclaw/app/voice/TalkModeManager.kt)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@ly85206559
ly85206559 force-pushed the codex/android-talk-directive-case-keys branch from ca03b37 to 9563a8e Compare July 3, 2026 15:59
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. labels Jul 3, 2026
@steipete steipete self-assigned this Jul 3, 2026
@steipete
steipete force-pushed the codex/android-talk-directive-case-keys branch from 9563a8e to 4210df9 Compare July 3, 2026 19:33
@steipete
steipete marked this pull request as ready for review July 3, 2026 19:33
@steipete

steipete commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Land-ready maintainer proof for exact head 4210df90305cf0a2808f39b0d27bf3b29c5bee52:

  • Expanded the original Android fix into the shared native contract: Android and OpenClawKit now apply case-insensitive lookup to every typed Talk directive alias, preserve exact-key precedence, and deterministically reject ambiguous non-exact collisions.
  • Android parser tests, Android Play APK assembly, and focused OpenClawKit Swift tests passed; coverage includes strings, booleans, integers, doubles, exact duplicates, and ambiguous duplicates on both platforms.
  • Fresh autoreview found and drove the Swift collision-order correction; the final review has no actionable findings.
  • Exact-head CI is green: https://github.com/openclaw/openclaw/actions/runs/28680038378
  • The final Android candidate was installed on Android 16 against an isolated real Gateway. A real Gateway-to-node talk.ptt.cancel roundtrip returned status: idle, confirming the candidate Talk runtime remained connected and callable.

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.

@steipete
steipete merged commit b623ff7 into openclaw:main Jul 3, 2026
76 of 78 checks passed
@steipete

steipete commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Merged via squash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app: android App: android P2 Normal backlog priority with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants