Skip to content

fix(android): filter thinking and reasoning blocks from chat message display#87939

Closed
xianshishan wants to merge 1 commit into
openclaw:mainfrom
xianshishan:fix/android-thinking-blocks-filter
Closed

fix(android): filter thinking and reasoning blocks from chat message display#87939
xianshishan wants to merge 1 commit into
openclaw:mainfrom
xianshishan:fix/android-thinking-blocks-filter

Conversation

@xianshishan

@xianshishan xianshishan commented May 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Prevent LLM thinking/reasoning content from appearing as visible chat
message bubbles in the Android app. The fix adds explicit handling for
thinking-related content block types at both the parsing and rendering layers.

Changes

  • ChatController.kt: Add explicit "thinking", "reasoning",
    "redacted_thinking" cases to parseChatMessageContent() that return
    null instead of relying on the generic else branch
  • ChatMessageViews.kt: Tighten ChatMessageBody rendering to only
    handle explicit "text" and "image" types, with else -> continue
    to skip any unknown types
  • ChatScreen.kt: Replace if/else rendering with explicit when
    branches for "text" and "image" types, skipping non-displayable
    types in else

Design rationale

The existing else -> null branch in parseChatMessageContent()
already drops unrecognized content types. This change makes thinking
filtering explicit rather than implicit, preventing future regressions
if content type handling evolves. The rendering-layer changes add
defense-in-depth by ensuring only known displayable types can render.

Fixes #87918

@xianshishan
fix(android): filter thinking and reasoning blocks from chat message …
141145b 
…display

Explicitly drop thinking, reasoning, and redacted_thinking content blocks
in parseChatMessageContent() instead of relying on the generic else branch.
Tighten ChatMessageBody and ChatBubble rendering to only handle explicit
text and image types, skipping everything else.

This prevents LLM thinking/reasoning content from leaking into visible
chat message bubbles on Android.

Fixes #87918
@openclaw-barnacle openclaw-barnacle Bot added app: android App: android size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 29, 2026
@clawsweeper

clawsweeper Bot commented May 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed May 29, 2026, 4:06 AM ET / 08:06 UTC.

Summary
The PR adds explicit Android parsing and rendering skips for thinking/reasoning chat content blocks.

PR surface: Other +3. Total +3 across 3 files.

Reproducibility: yes. from source inspection: a chat.history row with role toolResult and text content is accepted by Android history parsing and can be rendered as an OpenClaw bubble. I did not run a live Android device reproduction in this read-only review.

Review metrics: 1 noteworthy metric.

  • Reported leak paths covered: 0 role-level filters, 0 regression tests added. The linked issue's current triage points to text-bearing internal history rows, while this PR only changes content-block and renderer branches already guarded on main.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until 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 the role-level internal row leak and add focused gateway/Android regression coverage.
  • [P1] Add redacted real Android proof showing thinking/tool details no longer appear as chat bubbles.
  • Keep the linked issue open or remove closing syntax until the real leak is fixed.

Proof guidance:

  • [P1] Needs real behavior proof before merge: No after-fix Android run, screenshot, recording, terminal output, live output, or redacted logs are present; the contributor needs to add real behavior proof and then trigger a fresh review if it does not rerun automatically. 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] Merging this as-is could close the linked security-sensitive issue while the role-level internal row leak remains visible in Android history.
  • [P1] There is no after-fix proof from a real Android setup, so the user-visible leak has not been shown fixed.

Maintainer options:

  1. Fix role-level filtering before merge (recommended)
    Update this PR to hide internal toolResult/tool-call/reasoning rows at the gateway projection and/or Android parser, add focused regression coverage, and include redacted Android proof.
  2. Pause for a maintainer-owned fix
    If maintainers want the gateway projection boundary decided separately, keep the linked issue open and pause or close this PR as an incomplete Android-only cleanup.
  3. Merge only as cleanup without closing the issue
    Maintainers could accept the defensive cleanup, but the closing reference should be removed and the linked leak issue must remain open until the real source path is fixed.

Next step before merge

  • [P1] Needs human/contributor follow-up because the branch must address the role-level leak and provide redacted Android real-behavior proof before merge.

Security
Needs attention: The patch does not close the security-sensitive internal reasoning/tool-output exposure it claims to fix.

Review findings

  • [P1] Fix the row-level leak instead of explicit no-op block cases — apps/android/app/src/main/java/ai/openclaw/app/chat/ChatController.kt:657
Review details

Best possible solution:

Filter internal tool-result, tool-call, and reasoning rows from the chat.history display projection with an Android defensive guard and regression tests, then verify the Android chat view with redacted real behavior proof.

Do we have a high-confidence way to reproduce the issue?

Yes, from source inspection: a chat.history row with role toolResult and text content is accepted by Android history parsing and can be rendered as an OpenClaw bubble. I did not run a live Android device reproduction in this read-only review.

Is this the best way to solve the issue?

No. The PR is not the best fix because it only makes already-filtered content-block handling explicit; the maintainable fix should filter internal rows at the gateway display projection and keep an Android defensive guard with tests.

Full review comments:

  • [P1] Fix the row-level leak instead of explicit no-op block cases — apps/android/app/src/main/java/ai/openclaw/app/chat/ChatController.kt:657
    Adding thinking/reasoning here is behaviorally equivalent to the existing else -> null, and the Android renderers already filter unknown content parts before the changed branches run. The linked issue's current triage points to text-bearing internal rows such as toolResult being accepted and rendered, so this patch can close the issue without fixing the leak.
    Confidence: 0.88

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 8363d6596c17.

Label changes

Label changes:

  • add P1: The PR is attempting to fix a current Android chat leak of internal reasoning/tool details into user-visible bubbles.
  • add merge-risk: 🚨 security-boundary: Merging this PR with its closing reference could mark a sensitive internal-transcript exposure fixed without filtering the source-reproducible internal row path.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: No after-fix Android run, screenshot, recording, terminal output, live output, or redacted logs are present; the contributor needs to add real behavior proof and then trigger a fresh review if it does not rerun automatically. 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.

Label justifications:

  • P1: The PR is attempting to fix a current Android chat leak of internal reasoning/tool details into user-visible bubbles.
  • merge-risk: 🚨 security-boundary: Merging this PR with its closing reference could mark a sensitive internal-transcript exposure fixed without filtering the source-reproducible internal row path.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: No after-fix Android run, screenshot, recording, terminal output, live output, or redacted logs are present; the contributor needs to add real behavior proof and then trigger a fresh review if it does not rerun automatically. 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 +3. Total +3 across 3 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 3 8 5 +3
Total 3 8 5 +3

Security concerns:

  • [medium] Internal transcript leak remains unfiltered — apps/android/app/src/main/java/ai/openclaw/app/chat/ChatController.kt:657
    The PR only handles already-dropped content-block types, while the source-reproducible leak path is role-level internal rows that can carry text content into the Android chat display.
    Confidence: 0.84

What I checked:

Likely related people:

  • steipete: Current shallow blame attributes the checked-out Android and gateway projection paths to Peter Steinberger, and the linked issue triage also routes recent chat-history projection work to this area. (role: recent gateway/chat-history contributor; confidence: medium; commits: 621db8f0b1b4, bc44ce2c8e11, b04dd6d05c3d; files: apps/android/app/src/main/java/ai/openclaw/app/chat/ChatController.kt, apps/android/app/src/main/java/ai/openclaw/app/ui/chat/ChatScreen.kt, src/gateway/chat-display-projection.ts)
  • obviyus: GitHub commit metadata shows Ayaan Zaidi authored recent Android chat identity work touching ChatController, and the linked issue triage identifies adjacent Android chat UI work in the affected files. (role: recent Android chat contributor; confidence: medium; commits: a41be2585fca, 8b0e16a1c827, 34a5c47351d1; files: apps/android/app/src/main/java/ai/openclaw/app/chat/ChatController.kt, apps/android/app/src/main/java/ai/openclaw/app/ui/chat/ChatScreen.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.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. labels May 29, 2026
@xianshishan

xianshishan commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #89453 with proper Real behavior proof section.

@xianshishan xianshishan closed this Jun 2, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 19, 2026
Closed
@clawsweeper clawsweeper Bot mentioned this pull request Jul 6, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

app: android App: android merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P1 High-priority user-facing bug, regression, or broken workflow. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Android: LLM thinking/reasoning blocks rendered as separate chat messages

1 participant

@xianshishan