Skip to content

fix(ios): preserve single newlines as visible line breaks in chat#98222

Closed
mansourMP wants to merge 1 commit into
openclaw:mainfrom
mansourMP:fix/ios-chat-line-break-rendering
Closed

fix(ios): preserve single newlines as visible line breaks in chat#98222
mansourMP wants to merge 1 commit into
openclaw:mainfrom
mansourMP:fix/ios-chat-line-break-rendering

Conversation

@mansourMP

@mansourMP mansourMP commented Jun 30, 2026

Copy link
Copy Markdown

What Problem This Solves

Fixes #98028 — Assistant responses in the iOS app render as a single blob of text because line breaks are silently collapsed. For example, a multi-line assistant reply like:

Line one
Line two
Line three

…would display as "Line one Line two Line three", making formatted responses, code explanations, and lists unreadable.

Why This Change Was Made

The root cause is in ChatMarkdownPreprocessor.normalize(): single \n characters passed through unchanged, then SwiftUI's AttributedString(markdown:) followed CommonMark rules where a lone \n is a soft break rendered as a space. Only \n\n (paragraph break) or \n (hard break with trailing spaces) produce visible line breaks.

The fix adds a single regex replacement that converts isolated \n to \n\n using lookahead/lookbehind ((?<!\n)\n(?!\n)), so existing \n\n paragraph breaks are preserved untouched.

User Impact

  • Before: Multi-line assistant text renders as one unbroken paragraph
  • After: Each line break in the assistant response renders as a visible paragraph break
  • Existing \n\n paragraph breaks remain unchanged (no quadruple-spacing)
  • Code fences and lists get minor extra spacing (cosmetic tradeoff — still fully functional)

Evidence

  • ✅ 3 new tests added covering: single-newline conversion, existing paragraph-break preservation, and mixed content
  • ✅ 3 existing tests updated to match corrected behavior
  • ✅ All ChatMarkdownPreprocessor tests pass

🤖 Generated with Claude Code
Co-Authored-By: Claude [email protected]

@mansourMP @claude
fix(ios): preserve single newlines as visible line breaks in chat (op…
8ccbd5d 
…enclaw#98028)

Convert single \n to \n\n in ChatMarkdownPreprocessor.normalize() so
assistant responses render with visible line breaks rather than
collapsing into a single blob of text.

Previously, single newlines passed through to AttributedString(markdown:)
which follows CommonMark rules where lone \n is a soft break (space).
The fix uses a lookahead/lookbehind regex to match \n not adjacent to
another \n, preserving existing \n\n paragraph breaks.

Co-Authored-By: Claude <[email protected]>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@clawsweeper

clawsweeper Bot commented Jun 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 30, 2026, 1:03 PM ET / 17:03 UTC.

Summary
The PR changes the shared Swift chat Markdown preprocessor to turn isolated newlines into paragraph breaks and updates preprocessor tests around line breaks, code fences, and metadata stripping.

PR surface: Other +56. Total +56 across 2 files.

Reproducibility: no. high-confidence live iOS reproduction was run for the original bug. The PR regression is source-visible: its shared normalize change rewrites isolated newlines and its own test now expects blank lines inside a fenced code block.

Review metrics: 1 noteworthy metric.

  • Open candidate fixes: 3 open PRs including this PR. Multiple branches target the same iOS assistant newline bug, so maintainers should pick one newline-rendering strategy before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #98028
Summary: This PR is one of multiple candidate fixes for the canonical iOS assistant line-break rendering bug.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

  • Move newline preservation out of normalize into a renderer-only transform that leaves cleaned Markdown unchanged.
  • [P1] Add focused Swift tests for fenced/indented code and user-message preprocessing behavior.
  • [P1] Attach redacted iOS simulator or device proof showing multiline assistant replies after the fix.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body lists tests only; add redacted iOS simulator/device screenshot, recording, terminal output, or logs showing after-fix chat behavior, then update the PR body or ask a maintainer for @clawsweeper re-review.

Risk before merge

Maintainer options:

  1. Move the transform to rendering (recommended)
    Keep preprocess(...).cleaned stable, add display-only hard breaks outside code/fence-sensitive Markdown, and refresh tests plus iOS proof before merge.
  2. Use a sibling candidate
    If maintainers prefer the more targeted approach in fix(ios): preserve assistant line breaks in chat #98054, pause this branch and continue with that canonical fix after its remaining finding is addressed.
  3. Accept global Markdown mutation
    Maintainers could intentionally accept the extra blank rows in code and other line-sensitive Markdown, but that would make the fix broader than the reported assistant prose bug.

Next step before merge

  • [P1] Contributor or maintainer follow-up is needed because the branch has a blocking rendering-regression finding and lacks required real iOS proof; automation cannot supply the contributor's after-fix proof.

Security
Cleared: The diff changes Swift chat rendering code and Swift tests only; it does not touch dependencies, workflows, secrets, package resolution, install scripts, or other supply-chain surfaces.

Review findings

  • [P1] Keep rendering breaks out of normalize — apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatMarkdownPreprocessor.swift:226-229
Review details

Best possible solution:

Land a single renderer-level newline fix that preserves cleaned Markdown and code-block content, includes focused Swift coverage, and has redacted iOS simulator or device proof.

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

No high-confidence live iOS reproduction was run for the original bug. The PR regression is source-visible: its shared normalize change rewrites isolated newlines and its own test now expects blank lines inside a fenced code block.

Is this the best way to solve the issue?

No, not as submitted. The best fix is a renderer-level hard-break transform that leaves cleaned Markdown unchanged and skips code/fence-sensitive content, followed by visible iOS proof.

Full review comments:

  • [P1] Keep rendering breaks out of normalize — apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatMarkdownPreprocessor.swift:226-229
    The new replacement runs inside normalize, which is the shared cleaned-text path used by ChatMarkdownRenderer and ChatViewModel.stripInboundMetadata; replacing every isolated \n with \n\n rewrites Markdown source before parsing. The updated code-fence test now expects blank lines inside a fenced code block, so existing assistant/user code blocks can render with inserted blank rows instead of only preserving prose line breaks; move this to a renderer-only hard-break transform that leaves cleaned Markdown unchanged and skips fenced/indented code.
    Confidence: 0.92

Overall correctness: patch is incorrect
Overall confidence: 0.91

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P2: The PR targets a shipped iOS chat readability bug with limited blast radius and no data-loss, security, or availability impact.
  • add merge-risk: 🚨 compatibility: The diff changes the shared Markdown cleanup path in a way that can alter existing user and assistant message rendering, not only the reported assistant prose case.
  • 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: The PR body lists tests only; add redacted iOS simulator/device screenshot, recording, terminal output, or logs showing after-fix chat behavior, then update the PR body or ask a maintainer for @clawsweeper re-review.

Label justifications:

  • P2: The PR targets a shipped iOS chat readability bug with limited blast radius and no data-loss, security, or availability impact.
  • merge-risk: 🚨 compatibility: The diff changes the shared Markdown cleanup path in a way that can alter existing user and assistant message rendering, not only the reported assistant prose case.
  • 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: The PR body lists tests only; add redacted iOS simulator/device screenshot, recording, terminal output, or logs showing after-fix chat behavior, then update the PR body or ask a maintainer for @clawsweeper re-review.
Evidence reviewed

PR surface:

Other +56. Total +56 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 58 2 +56
Total 2 58 2 +56

What I checked:

Likely related people:

  • dwc1997: git blame and PR history show the current ChatMarkdownRenderer, ChatMarkdownPreprocessor, and ChatMessageViews files were added through the PR authored by this account. (role: introduced current renderer/preprocessor files; confidence: high; commits: f92ec2d4e88b; files: apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatMarkdownRenderer.swift, apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatMarkdownPreprocessor.swift, apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatMessageViews.swift)
  • vincentkoc: GitHub PR metadata for test(shared): add unit tests for surrogate-safe UTF-16 string slicing helpers #97805 shows this account merged the PR that introduced the current shared chat renderer surface. (role: merger of current renderer PR; confidence: medium; commits: f92ec2d4e88b; files: apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatMarkdownRenderer.swift, apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatMarkdownPreprocessor.swift)
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. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jun 30, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jul 1, 2026
Merged
@joshavant

joshavant commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Thanks @mansourMP for working on this.

The underlying issue has been fixed by #98304, merged as commit 47845e1. This PR targeted the right symptom, but it changed the shared Markdown normalization path by turning isolated newlines into paragraph breaks globally.

The landed fix keeps normalized Markdown stable and applies the newline preservation only in the chat display renderer, with coverage for code fences, tables, block Markdown, existing hard breaks, and plain prose. Closing this as superseded by #98304.

@joshavant joshavant closed this Jul 1, 2026
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

merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: iOS app: Assistant response line breaks not rendered

2 participants

@mansourMP @joshavant