Skip to content

fix(mobile): show complete unwrapped diff lines containing tabs - #14605

Open
jakeleventhal wants to merge 1 commit into
pingdotgg:mainfrom
jakeleventhal:t3code/reopen-pr-12234
Open

jakeleventhal wants to merge 1 commit into
pingdotgg:mainfrom
jakeleventhal:t3code/reopen-pr-12234

Conversation

@jakeleventhal

@jakeleventhal jakeleventhal commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Replacement for #12234. That PR was closed under the verification rule because its screenshots predated the renderer rework and its layout tests never scrolled the view. This is the same fix on current main, with a recording of the current view panning to the ends of the tab-indented lines.

Problem

With word wrap off, the iOS review diff counts each tab as one column and draws the row into that narrower rectangle. Tab-indented lines such as a TSConfig extends value wrap inside the rectangle, and horizontal panning never reaches the real end. Word wrap is off by default in the mobile review diff, and review comment cards always pass wordWrap: false, so both surfaces hit this path. #12590 fixed the wrapped path.

This is a focused repair of that drawing bug. It does not change a product default, add a setting, or introduce a workflow. The iOS renderer is the only code that has the defect this patch addresses. Android keeps its own renderer.

Fix

Unwrapped rows are laid out on one line. ReviewDiffCodeLayout records the widest visual line, including tab stops and non-ASCII glyphs. The view sizes horizontal scrolling from that width, without the previous contentWidth cap, and draws native rows at codeStartX - horizontalOffset, so the glyphs move with the pan. Wrapped rows still wrap to the viewport and do not pan.

Evidence

Word wrap off, viewport 390×250, contentWidth 2800. The rows are the tab-indented lines from the original report:

"\t\t\"extends\": \"@riptech/tsconfig/tsconfig.json\","
"\t\t\"extends\": \"@riptech/tsconfig/tsconfig.test.json\","

The shots are the production T3ReviewDiffView, panned through its handleHorizontalPan handler. Before is main at bd89c13020. After is this branch. Host is Mac Catalyst. This machine has Command Line Tools only, so xcrun simctl is unavailable and the T3 device panel cannot boot an iPhone Simulator. The full T3 Code Dev app was not launched.

On current main, panning does not move the lines. Both extends values stay cut off (tscon and tsconfig/).

Before: word wrap off, panned as far as the view allows, TSConfig filenames still cut off

On this branch the same lines start clipped at the viewport edge, then the pan reaches the closing quote and comma on both filenames.

Start Panned to the end
After, start: both extends lines clipped at the viewport edge After, end: both full TSConfig filenames and closing punctuation

The recording is that same pan. Both lines stay in frame, from the clipped start through the closing punctuation on tsconfig.test.json.

scroll-both-lines.mp4

apps/mobile/modules/t3-review-diff/tests/run-ios.sh passed: 18 cases, including unwrapped tab, CJK, and Arabic rows that stay on one line and whose ink falls inside usedWidth. T3ReviewDiffView.swift typechecks for Mac Catalyst against a stubbed ExpoModulesCore.

Not checked: a booted iPhone Simulator, the full mobile app, and Android.

Model: grok-4.7-build-fast. Harness: Grok in T3 Code.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 1, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at b05f3b9

Macroscope's review found this PR approvable — This is a localized iOS diff-rendering fix that makes existing horizontal scrolling account for tabs and shaped text without changing wrapping defaults or introducing a new workflow. The production changes are small and accompanied by focused layout coverage.

Notes:

  • Diff unchanged. Approvability was decided on eligibility alone.

No code changes detected at 2d2343f. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 1, 2026
@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Important

Review skipped

We couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting @coderabbitai full review.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 8ffd0390-fcf8-475a-aab0-b322f2760ce5


📥 Commits

Reviewing files that changed from the base of the PR and between 5cc99e1 and b05f3b9.


📒 Files selected for processing (3)
  • apps/mobile/modules/t3-review-diff/ios/ReviewDiffCodeLayout.swift
  • apps/mobile/modules/t3-review-diff/ios/T3ReviewDiffView.swift
  • apps/mobile/modules/t3-review-diff/tests/ios/main.swift

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.



📝 Walkthrough

Walkthrough

The iOS diff view now supports unwrapped code layout. It measures code and hunk content widths from rendered text and applies the horizontal scroll offset when drawing native-layout text. The iOS layout tests cover unwrapped tabbed and ASCII text.

Changes

iOS diff code layout

Layer / File(s) Summary
Code layout width metrics
apps/mobile/modules/t3-review-diff/ios/ReviewDiffCodeLayout.swift
ReviewDiffCodeLayout supports unbounded width and records the widest visual line for ASCII and native TextKit layouts.
Diff view sizing and rendering
apps/mobile/modules/t3-review-diff/ios/T3ReviewDiffView.swift, apps/mobile/modules/t3-review-diff/tests/ios/main.swift
ReviewDiffContentView uses a shared width policy to prepare layouts and measure code and hunk text. Native-layout drawing applies the horizontal scroll offset. Tests check unwrapped tabbed text and ASCII width.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix


Merge Risk

Merge Risk: ⚪ Minimal · up to b05f3

The change makes unwrapped iOS diff lines horizontally reachable using measured text widths. No actionable merge-blocking issue is established; normal checks remain appropriate, with full-app and iPhone validation still limited.

Security Architecture Review

Security architecture risk: 🔵 Low · up to b05f3

The fix remains within the iOS code display. Large or rapidly replaced content may require more processing than before, but an exploitable availability impact has not been established.

Retained concerns

  • Low · security · inferred: Preparing every unwrapped row expands input-sized native measurement work. Obsolete preparations continue through their row loop before replacement checks reject publication, so large non-ASCII payloads could delay newer content on the shared payload queue. Effective caller bounds and material availability impact remain unverified; this is not evidence that the removed horizontal-width cap was a security control.

Security review details

Security Blast Radius

  • inferred — The identified concern is processing availability in the iOS application displaying supplied code content. The inspected path ends in text measurement and drawing, not a privileged operation or cross-service mutation; broader service or tenant impact is not established.

Security Findings and Attack Paths

  • inferred — A potential availability path is large non-ASCII row content passed through the existing app bridge into eager TextKit measurement, compounded by repeated replacements. Native ingestion has no observed byte, row-count, or line-length bound. Whether an adversary can deliver a sufficiently costly payload through production callers remains unresolved; no exploit was verified.

Trust Boundaries and Controls

  • observed — The bridge is invoked by the existing application wrapper, not a newly introduced network entrypoint. Generation checks prevent obsolete results from replacing current display state, and measurement reuse is thread-local. These controls protect publication identity and measurement ownership, but do not cancel queued work.

Hardening Proposals

  • proposed — Establish an independent preparation budget for payload size and line complexity, and consider stopping obsolete measurement between rows. Preserve measured horizontal scrolling rather than treating the former drawing-width cap as an input-security limit.



🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the mobile fix for displaying complete unwrapped diff lines that contain tabs.
Description check ✅ Passed The description explains the problem, implementation, affected surfaces, visual evidence, focused tests, type-check result, limitations, and agent details. It uses “Fix” and “Evidence” instead of the …


✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR


  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Comment @coderabbitai help to get the list of available commands.

@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 2, 2026 — with ChatGPT Codex Connector
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 2, 2026 05:32

Dismissing prior approval to re-evaluate b05f3b9

@jakeleventhal
jakeleventhal force-pushed the t3code/reopen-pr-12234 branch 4 times, most recently from 4d258fe to 2d2343f Compare October 8, 2026 20:01
With word wrap off, tab-indented rows were counted as one column per tab and drawn into that narrower rect, so the ends could not be scrolled into view. Measure the rendered width and pan those rows with the horizontal offset.
@jakeleventhal
jakeleventhal force-pushed the t3code/reopen-pr-12234 branch from 2d2343f to c9d3fb7 Compare October 9, 2026 21:31

This branch has not been deployed

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

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants