Sign up
View stargazersSign up

fix(mobile): Android diff word highlights sit on the changed characters#33568

open4
main
GitHub fork: fix/android-diff-highlight-measured-width
opened3d agobyOonly21mil
3 files changed
View on GitHubTwo-way sync enabledPR last imported just now

Problem

On Android, the red and green word highlights in Review changes can miss the text they mark: the native diff canvas places each box at character count × the width of "M", but draws the code with measured run widths. The two drift apart with CJK or emoji before the change (word wrap off, any device), and on devices where monospace resolves to a proportional font, as in the report, even on plain ASCII lines.

Fixes https://git.cafe/pingdotgg/t3code/issues/27146

Change

drawWordDiffRanges now measures the highlight's start and width with the same runs, fonts and line starts that drawCode uses to draw the text, through a small codeWidth helper that measureRowWidth now uses too. It only runs for visible added or deleted rows that have word ranges. Wrapped non-ASCII rows and iOS are unchanged.

Scope

Bug fix: highlights mark the wrong text. A focused fix of an obvious Android bug, with a diagnosis in https://git.cafe/pingdotgg/t3code/issues/27146 that points at this exact code: one drawing function, its one caller, and a regression test.

Verification

To see it on Linux: apps/mobile/modules/t3-review-diff/tests/run-android.sh --console=plain --warning-mode=none --no-daemon --rerun --tests ReviewDiffGeometryTest.unwrappedWordDiffHighlightsCoverTheTextDrawCodeDraws fails with main's placement lines (weight=medium prefix=if left expected:<64.0> but was:<100.0>, the box 36 px to the right of the text) and passes on this branch.

Re-checked against main f419698aea on 2026-11-12: the test fails on that main without the change and passes with it, and the branch merges cleanly.

Re-checked on 2026-11-13 against main 5f85768394, which d8296d3ccd merged: the test fails with main's placement lines and passes on d8296d3ccd, and that main still shows the bug on the device below.

On the device, 2026-11-13: an Android 16 arm64 emulator with software rendering, each revision's own dev client, a "Test machine" server with a sample repo, Review changes on NOTES.md with CJK, emoji and plain changed lines, word wrap off, light theme.

Main 5f85768394: on the two CJK lines each highlight sits one column left of the changed characters; the emoji and plain lines are right

Main `5f85768394`: on the two CJK lines each highlight sits one column left of the changed characters; the emoji and plain lines are right

d8296d3ccd: every highlight sits on the changed characters

`d8296d3ccd`: every highlight sits on the changed characters

Main 5f85768394, rows zoom: a crop of the main capture above at 2x

Main `5f85768394`, rows zoom: a crop of the main capture above at 2x

d8296d3ccd, rows zoom: a crop of the d8296d3ccd capture above at 2x

`d8296d3ccd`, rows zoom: a crop of the `d8296d3ccd` capture above at 2x

The report's trigger (a proportional monospace, on main ddc4f57602 and the earlier head 5b6dd09eab, whose three changed files are byte-identical on d8296d3ccd), the merged tree (698cc0b4c3, not pushed), earlier CJK-line captures and iOS (unchanged, main and merged tree) are in the details.

Not checked:

  • iOS on this branch's own older base. iOS code is unchanged, and the merged tree's iOS build is main's.
The report's-trigger pair, merged-tree, earlier CJK and iOS captures, full test runs, an earlier host-GPU pair and the full earlier text

Problem (full text)

On Android, the red and green word highlights in Review changes can miss the text they mark. The native diff canvas places each box at character count × the width of "M", but it draws the code with measured run widths. The two drift apart whenever a character is not "M"-wide:

  • CJK or emoji before the change, with word wrap off, on any device.
  • Devices where the monospace family resolves to a proportional font, as in the report. There, even plain ASCII lines drift.

Fixes https://git.cafe/pingdotgg/t3code/issues/27146

Change (full text)

drawWordDiffRanges now measures the highlight's start and width with the same runs, fonts and line starts that drawCode uses to draw the text. A small codeWidth helper does that measuring, and measureRowWidth now uses it too, so the run loop lives in one place. The caller passes the row's tokens and style, and computes the baseline before the highlight call because measuring reconfigures the shared paint per run.

There is no fixed-pitch shortcut. asciiFixedPitch is only refreshed while word wrap is off, so it can be stale for wrapped rows. The cost matches what drawCode already does: it measures every drawn segment each frame, and this only runs for visible added or deleted rows that have word ranges.

Wrapped non-ASCII rows already use native text spans and are unchanged. iOS was not changed.

Scope and approval (full text)

A focused fix of an obvious Android bug, with a diagnosis in https://git.cafe/pingdotgg/t3code/issues/27146 that points at this exact code. It changes one drawing function, its one caller, and a regression test. There is no behavior change beyond highlight placement.

Verification (full text)

Run order: main c58f472a35, this branch 5b6dd09eab, then this branch merged locally with that main (9685fbbe1b, not pushed).

The Android captures below show this placement error where it appears on the emulator I used: on lines with CJK characters, where main draws the boxes about one character to the left of the changed text. The report in #27146 is the same error with a different trigger, a phone whose monospace font is proportional, where the boxes on plain ASCII lines sit to the right of the changed text and are too wide, more so further along the line.

The regression test ReviewDiffGeometryTest.unwrappedWordDiffHighlightsCoverTheTextDrawCodeDraws reproduces it: it checks each box against where drawCode draws the changed text, and its medium-weight if case is plain ASCII in a monospace that is not fixed-width under Robolectric. With main's placement lines put back, it fails with the box 36 px to the right of the text:

text
1$ apps/mobile/modules/t3-review-diff/tests/run-android.sh --console=plain --warning-mode=none --no-daemon --rerun --tests ReviewDiffGeometryTest.unwrappedWordDiffHighlightsCoverTheTextDrawCodeDraws
2To honour the JVM settings for this build a single-use Daemon process will be forked. For more on this, please refer to https://docs.gradle.org/9.4.1/userguide/gradle_daemon.html#sec:disabling_the_daemon in the Gradle documentation.
3Daemon will be stopped at the end of the build
4> Task :extractAndroidTestJars UP-TO-DATE
5> Task :instrumentTokenFixture UP-TO-DATE
6> Task :compileRegression UP-TO-DATE
7
8> Task :test
9
10ReviewDiffGeometryTest > unwrappedWordDiffHighlightsCoverTheTextDrawCodeDraws FAILED
11 java.lang.AssertionError: weight=medium prefix=if left expected:<64.0> but was:<100.0>
12 at org.junit.Assert.fail(Assert.java:89)
13 at org.junit.Assert.failNotEquals(Assert.java:835)
14 at org.junit.Assert.assertEquals(Assert.java:577)
15 at expo.modules.t3reviewdiff.ReviewDiffGeometryTest.unwrappedWordDiffHighlightsCoverTheTextDrawCodeDraws(ReviewDiffGeometryTest.kt:228)
16
171 test completed, 1 failed
18
19> Task :test FAILED
20
21FAILURE: Build failed with an exception.
22
23* What went wrong:
24Execution failed for task ':test'.
25> There were failing tests. See the report at: file:///mnt/t3code/apps/mobile/modules/t3-review-diff/tests/android/build/reports/tests/test/index.html
26
27* Try:
28> Run with --scan to get full insights from a Build Scan (powered by Develocity).
29
30BUILD FAILED in 9s
314 actionable tasks: 1 executed, 3 up-to-date
32exit 1

This branch, where it passes:

text
1$ apps/mobile/modules/t3-review-diff/tests/run-android.sh --console=plain --warning-mode=none --no-daemon --rerun --tests ReviewDiffGeometryTest.unwrappedWordDiffHighlightsCoverTheTextDrawCodeDraws
2To honour the JVM settings for this build a single-use Daemon process will be forked. For more on this, please refer to https://docs.gradle.org/9.4.1/userguide/gradle_daemon.html#sec:disabling_the_daemon in the Gradle documentation.
3Daemon will be stopped at the end of the build
4> Task :extractAndroidTestJars UP-TO-DATE
5> Task :instrumentTokenFixture UP-TO-DATE
6> Task :compileRegression UP-TO-DATE
7
8> Task :test
9
10ReviewDiffGeometryTest > unwrappedWordDiffHighlightsCoverTheTextDrawCodeDraws PASSED
11
12BUILD SUCCESSFUL in 9s
134 actionable tasks: 1 executed, 3 up-to-date
14Consider enabling configuration cache to speed up this build: https://docs.gradle.org/9.4.1/userguide/configuration_cache_enabling.html
15exit 0

Device, the report's trigger. To reproduce the report, I changed the system font mapping on the emulator, not the app: in the emulator's own font configuration (fonts.xml and font_fallback.xml), the system monospace family points at the proportional Droid Sans, and cmd font dump lists DroidSans.ttf for monospace at each capture. The app was not patched. Android 16 arm64 emulator with host GPU rendering, each revision's own dev client and Metro from its checkout, taps through T3's Device panel, a "Test machine" server with a seeded repo whose changed lines are plain ASCII version numbers, word wrap off. Run order: main ddc4f57602, this branch 5b6dd09eab, then this branch merged locally with that main (698cc0b4c3, not pushed).

Main ddc4f57602, proportional monospace: every word highlight sits in the empty space right of the line's text, not on the changed numbers

Main `ddc4f57602`, proportional `monospace`: every word highlight sits in the empty space right of the line's text, not on the changed numbers

Earlier head 5b6dd09eab, proportional monospace: every word highlight covers the changed numbers

Earlier head `5b6dd09eab`, proportional `monospace`: every word highlight covers the changed numbers

Merged 698cc0b4c3, proportional monospace: every word highlight covers the changed numbers, as on this branch

Merged `698cc0b4c3`, proportional `monospace`: every word highlight covers the changed numbers, as on this branch

Device, CJK lines. Android 16 emulator on Linux with software rendering (SwiftShader), a T3 Code Dev build and Metro from each revision's checkout, taps through T3's Device panel, a "Test machine" server with a seeded repo, word wrap off.

Main, CJK lines: the word highlights sit about one character left of the changed version numbers

Main, CJK lines: the word highlights sit about one character left of the changed version numbers

This branch, CJK lines: the word highlights cover the changed version numbers, as on the emoji and plain lines

This branch, CJK lines: the word highlights cover the changed version numbers, as on the emoji and plain lines

Merged tree, CJK lines: the word highlights cover the changed version numbers, as on the emoji and plain lines

Merged tree, CJK lines: the word highlights cover the changed version numbers, as on the emoji and plain lines

iOS was not changed. iOS 26.5 simulator, taps through the Device panel, main and the merged tree:

iOS on main: word highlights cover the changed version numbers

iOS on main: word highlights cover the changed version numbers

iOS on the merged tree: the same screen as main

iOS on the merged tree: the same screen as main

Full module test runs and an earlier host-GPU pair

All 23 module tests, with main's placement, on this branch and on the merged tree:

text
1$ GRADLE=gradle-9.4.1 apps/mobile/modules/t3-review-diff/tests/run-android.sh --console=plain --no-daemon
2# checkout: 5b6dd09eab with base-variant.patch (main c58f472a35 placement inside drawWordDiffRanges, head signature)
3To honour the JVM settings for this build a single-use Daemon process will be forked. For more on this, please refer to https://docs.gradle.org/9.4.1/userguide/gradle_daemon.html#sec:disabling_the_daemon in the Gradle documentation.
4Daemon will be stopped at the end of the build
5> Task :extractAndroidTestJars
6> Task :instrumentTokenFixture
7> Task :compileRegression
8
9> Task :test
10
11ReviewDiffCodeLayoutTest > asciiSegmentsCoverTheWholeLineAndFit PASSED
12
13ReviewDiffCodeLayoutTest > cachedContentWidthMatchesFreshMeasurementAfterUpdatesAndRemoval PASSED
14
15ReviewDiffCodeLayoutTest > unicodeAndTabsFitWithoutSplittingClusters PASSED
16
17ReviewDiffCodeLayoutTest > obsoletePreparationStopsBeforeWalkingTheRemainingRows PASSED
18
19ReviewDiffCodeLayoutTest > highlightsUseNativeTextRangesAndSurviveSyntaxArrival PASSED
20
21ReviewDiffCodeLayoutTest > changingCommentHeightReusesCodeButWidthAndContentInvalidateIt PASSED
22
23ReviewDiffGeometryTest > unwrappedColorAndUnderlinePatchesDoNotReadOffscreenRows PASSED
24
25ReviewDiffGeometryTest > stickyHeadersUseTheHeaderIndexAndKeepPushOffAndHitTesting PASSED
26
27ReviewDiffGeometryTest > asciiWidthCacheDoesNotRetainPerRowMeasurementObjects PASSED
28
29ReviewDiffGeometryTest > headerPathLimitsKeepFirstMatchAndNeverReadCodeRows PASSED
30
31ReviewDiffGeometryTest > unwrappedWordDiffHighlightsCoverTheTextDrawCodeDraws FAILED
32 java.lang.AssertionError: weight=medium prefix=if left expected:<64.0> but was:<100.0>
33 at org.junit.Assert.fail(Assert.java:89)
34 at org.junit.Assert.failNotEquals(Assert.java:835)
35 at org.junit.Assert.assertEquals(Assert.java:577)
36 at expo.modules.t3reviewdiff.ReviewDiffGeometryTest.unwrappedWordDiffHighlightsCoverTheTextDrawCodeDraws(ReviewDiffGeometryTest.kt:228)
37
38ReviewDiffGeometryTest > asciiWidthMatchesPaintForEverySyntaxFontAndWeight PASSED
39
40ReviewDiffGeometryTest > asciiSyntaxArrivalDoesNotRebuildWrappedOffsetsOrRetainPerRowLayouts PASSED
41
42ReviewDiffGeometryTest > lazyAsciiHeightAndLineStartsMatchTheExistingLayoutAtEveryBoundary PASSED
43
44ReviewDiffGeometryTest > geometryChangesKeepTheVisibleRowAndClampWithinShortenedRows PASSED
45
46ReviewDiffTokenDecodeTest > queuedFullReplacementSkipsOnlyObsoleteWorkAndPreservesPatchOrdering PASSED
47
48ReviewDiffTokenDecodeTest > enteredFullSuccessAndErrorFinishButOnlyTheReplacementPublishes PASSED
49
50ReviewDiffTokenDecodeTest > cleanupAfterEntryKeepsTheExistingCurrentFullPublication PASSED
51
52ReviewDiffTokenDecodeTest > cleanupDiscardsQueuedFullWorkOnTheProductionExecutor PASSED
53
54ReviewDiffViewportTest > collapsedFilesKeepViewportIndicesInTheFullPayload PASSED
55
56ReviewDiffViewportTest > resetDiscardsOldRowsAndPendingDecodeBeforePublishingTheNewDestination PASSED
57
58ReviewDiffViewportTest > decodedRowsWaitForTheReplacementFilePropBeforePublishingAViewport PASSED
59
60ReviewDiffViewportTest > immediateNavigationStopsAnActiveFling PASSED
61
6223 tests completed, 1 failed
63
64> Task :test FAILED
65
66[Incubating] Problems report is available at: file:///mnt/t3code/apps/mobile/modules/t3-review-diff/tests/android/build/reports/problems/problems-report.html
67
68FAILURE: Build failed with an exception.
69
70* What went wrong:
71Execution failed for task ':test'.
72> There were failing tests. See the report at: file:///mnt/t3code/apps/mobile/modules/t3-review-diff/tests/android/build/reports/tests/test/index.html
73
74* Try:
75> Run with --scan to get full insights from a Build Scan (powered by Develocity).
76
77Deprecated Gradle features were used in this build, making it incompatible with Gradle 10.
78
79You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.
80
81For more on this, please refer to https://docs.gradle.org/9.4.1/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.
82
83BUILD FAILED in 22s
844 actionable tasks: 4 executed
85exit 1
text
1$ GRADLE=gradle-9.4.1 apps/mobile/modules/t3-review-diff/tests/run-android.sh --console=plain --no-daemon
2# checkout: 5b6dd09eab
3To honour the JVM settings for this build a single-use Daemon process will be forked. For more on this, please refer to https://docs.gradle.org/9.4.1/userguide/gradle_daemon.html#sec:disabling_the_daemon in the Gradle documentation.
4Daemon will be stopped at the end of the build
5> Task :extractAndroidTestJars
6> Task :instrumentTokenFixture
7> Task :compileRegression
8
9> Task :test
10
11ReviewDiffCodeLayoutTest > asciiSegmentsCoverTheWholeLineAndFit PASSED
12
13ReviewDiffCodeLayoutTest > cachedContentWidthMatchesFreshMeasurementAfterUpdatesAndRemoval PASSED
14
15ReviewDiffCodeLayoutTest > unicodeAndTabsFitWithoutSplittingClusters PASSED
16
17ReviewDiffCodeLayoutTest > obsoletePreparationStopsBeforeWalkingTheRemainingRows PASSED
18
19ReviewDiffCodeLayoutTest > highlightsUseNativeTextRangesAndSurviveSyntaxArrival PASSED
20
21ReviewDiffCodeLayoutTest > changingCommentHeightReusesCodeButWidthAndContentInvalidateIt PASSED
22
23ReviewDiffGeometryTest > unwrappedColorAndUnderlinePatchesDoNotReadOffscreenRows PASSED
24
25ReviewDiffGeometryTest > stickyHeadersUseTheHeaderIndexAndKeepPushOffAndHitTesting PASSED
26
27ReviewDiffGeometryTest > asciiWidthCacheDoesNotRetainPerRowMeasurementObjects PASSED
28
29ReviewDiffGeometryTest > headerPathLimitsKeepFirstMatchAndNeverReadCodeRows PASSED
30
31ReviewDiffGeometryTest > unwrappedWordDiffHighlightsCoverTheTextDrawCodeDraws PASSED
32
33ReviewDiffGeometryTest > asciiWidthMatchesPaintForEverySyntaxFontAndWeight PASSED
34
35ReviewDiffGeometryTest > asciiSyntaxArrivalDoesNotRebuildWrappedOffsetsOrRetainPerRowLayouts PASSED
36
37ReviewDiffGeometryTest > lazyAsciiHeightAndLineStartsMatchTheExistingLayoutAtEveryBoundary PASSED
38
39ReviewDiffGeometryTest > geometryChangesKeepTheVisibleRowAndClampWithinShortenedRows PASSED
40
41ReviewDiffTokenDecodeTest > queuedFullReplacementSkipsOnlyObsoleteWorkAndPreservesPatchOrdering PASSED
42
43ReviewDiffTokenDecodeTest > enteredFullSuccessAndErrorFinishButOnlyTheReplacementPublishes PASSED
44
45ReviewDiffTokenDecodeTest > cleanupAfterEntryKeepsTheExistingCurrentFullPublication PASSED
46
47ReviewDiffTokenDecodeTest > cleanupDiscardsQueuedFullWorkOnTheProductionExecutor PASSED
48
49ReviewDiffViewportTest > collapsedFilesKeepViewportIndicesInTheFullPayload PASSED
50
51ReviewDiffViewportTest > resetDiscardsOldRowsAndPendingDecodeBeforePublishingTheNewDestination PASSED
52
53ReviewDiffViewportTest > decodedRowsWaitForTheReplacementFilePropBeforePublishingAViewport PASSED
54
55ReviewDiffViewportTest > immediateNavigationStopsAnActiveFling PASSED
56
57[Incubating] Problems report is available at: file:///mnt/t3code/apps/mobile/modules/t3-review-diff/tests/android/build/reports/problems/problems-report.html
58
59Deprecated Gradle features were used in this build, making it incompatible with Gradle 10.
60
61You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.
62
63For more on this, please refer to https://docs.gradle.org/9.4.1/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.
64
65BUILD SUCCESSFUL in 23s
664 actionable tasks: 4 executed
67Consider enabling configuration cache to speed up this build: https://docs.gradle.org/9.4.1/userguide/configuration_cache_enabling.html
68exit 0
text
1$ GRADLE=gradle-9.4.1 apps/mobile/modules/t3-review-diff/tests/run-android.sh --console=plain --no-daemon
2# checkout: 9685fbbe1b
3To honour the JVM settings for this build a single-use Daemon process will be forked. For more on this, please refer to https://docs.gradle.org/9.4.1/userguide/gradle_daemon.html#sec:disabling_the_daemon in the Gradle documentation.
4Daemon will be stopped at the end of the build
5> Task :extractAndroidTestJars
6> Task :instrumentTokenFixture
7> Task :compileRegression
8
9> Task :test
10
11ReviewDiffCodeLayoutTest > asciiSegmentsCoverTheWholeLineAndFit PASSED
12
13ReviewDiffCodeLayoutTest > cachedContentWidthMatchesFreshMeasurementAfterUpdatesAndRemoval PASSED
14
15ReviewDiffCodeLayoutTest > unicodeAndTabsFitWithoutSplittingClusters PASSED
16
17ReviewDiffCodeLayoutTest > obsoletePreparationStopsBeforeWalkingTheRemainingRows PASSED
18
19ReviewDiffCodeLayoutTest > highlightsUseNativeTextRangesAndSurviveSyntaxArrival PASSED
20
21ReviewDiffCodeLayoutTest > changingCommentHeightReusesCodeButWidthAndContentInvalidateIt PASSED
22
23ReviewDiffGeometryTest > unwrappedColorAndUnderlinePatchesDoNotReadOffscreenRows PASSED
24
25ReviewDiffGeometryTest > stickyHeadersUseTheHeaderIndexAndKeepPushOffAndHitTesting PASSED
26
27ReviewDiffGeometryTest > asciiWidthCacheDoesNotRetainPerRowMeasurementObjects PASSED
28
29ReviewDiffGeometryTest > headerPathLimitsKeepFirstMatchAndNeverReadCodeRows PASSED
30
31ReviewDiffGeometryTest > unwrappedWordDiffHighlightsCoverTheTextDrawCodeDraws PASSED
32
33ReviewDiffGeometryTest > asciiWidthMatchesPaintForEverySyntaxFontAndWeight PASSED
34
35ReviewDiffGeometryTest > asciiSyntaxArrivalDoesNotRebuildWrappedOffsetsOrRetainPerRowLayouts PASSED
36
37ReviewDiffGeometryTest > lazyAsciiHeightAndLineStartsMatchTheExistingLayoutAtEveryBoundary PASSED
38
39ReviewDiffGeometryTest > geometryChangesKeepTheVisibleRowAndClampWithinShortenedRows PASSED
40
41ReviewDiffTokenDecodeTest > queuedFullReplacementSkipsOnlyObsoleteWorkAndPreservesPatchOrdering PASSED
42
43ReviewDiffTokenDecodeTest > enteredFullSuccessAndErrorFinishButOnlyTheReplacementPublishes PASSED
44
45ReviewDiffTokenDecodeTest > cleanupAfterEntryKeepsTheExistingCurrentFullPublication PASSED
46
47ReviewDiffTokenDecodeTest > cleanupDiscardsQueuedFullWorkOnTheProductionExecutor PASSED
48
49ReviewDiffViewportTest > collapsedFilesKeepViewportIndicesInTheFullPayload PASSED
50
51ReviewDiffViewportTest > resetDiscardsOldRowsAndPendingDecodeBeforePublishingTheNewDestination PASSED
52
53ReviewDiffViewportTest > decodedRowsWaitForTheReplacementFilePropBeforePublishingAViewport PASSED
54
55ReviewDiffViewportTest > immediateNavigationStopsAnActiveFling PASSED
56
57[Incubating] Problems report is available at: file:///mnt/t3code/apps/mobile/modules/t3-review-diff/tests/android/build/reports/problems/problems-report.html
58
59Deprecated Gradle features were used in this build, making it incompatible with Gradle 10.
60
61You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.
62
63For more on this, please refer to https://docs.gradle.org/9.4.1/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.
64
65BUILD SUCCESSFUL in 22s
664 actionable tasks: 4 executed
67Consider enabling configuration cache to speed up this build: https://docs.gradle.org/9.4.1/userguide/configuration_cache_enabling.html
68exit 0

The regression test alone on the merged tree:

text
1$ apps/mobile/modules/t3-review-diff/tests/run-android.sh --console=plain --warning-mode=none --no-daemon --rerun --tests ReviewDiffGeometryTest.unwrappedWordDiffHighlightsCoverTheTextDrawCodeDraws
2To honour the JVM settings for this build a single-use Daemon process will be forked. For more on this, please refer to https://docs.gradle.org/9.4.1/userguide/gradle_daemon.html#sec:disabling_the_daemon in the Gradle documentation.
3Daemon will be stopped at the end of the build
4> Task :extractAndroidTestJars UP-TO-DATE
5> Task :instrumentTokenFixture UP-TO-DATE
6> Task :compileRegression UP-TO-DATE
7
8> Task :test
9
10ReviewDiffGeometryTest > unwrappedWordDiffHighlightsCoverTheTextDrawCodeDraws PASSED
11
12BUILD SUCCESSFUL in 8s
134 actionable tasks: 1 executed, 3 up-to-date
14Consider enabling configuration cache to speed up this build: https://docs.gradle.org/9.4.1/userguide/configuration_cache_enabling.html
15exit 0

Earlier pair, same fixture, on an Android 16 arm64 emulator rendering on the host GPU (OpenGL ES Translator (Apple M4), OpenGL ES 3.0 (4.1 Metal)): main e85e0094dd before, this branch 5b6dd09eab after. Main has not changed these files since.

Before, CJK lines: the word highlights sit left of the changed version numbers

After, CJK lines: the word highlights cover the changed version numbers, as on the emoji and plain lines

After, CJK lines: the word highlights cover the changed version numbers, as on the emoji and plain lines

Not checked:

  • iOS on this branch's own older base. iOS code is unchanged, and the merged tree's iOS build is main's.

Generated with Claude Opus 5.5 in T3 Code Nightly

Fixes pingdotgg/t3code#27146

Checks

21 successful · 10 skipped
All checks passed
15d50b7

Added 1 commit

  • fix(mobile): Android word-diff highlights sit on the changed text5b6dd09
CcoderabbitaiBot3d ago
Synced from GitHub

Explain Change →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

External image Run configuration

External image Commits

Reviewing files that changed from the base of the PR and between d8296d3ccdfeab5b74202a3ea4a73c629bf710d6 and 15d50b79c42b399a9fe08a471ec7efbf60b456e6.

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
Walkthrough

Android word-diff highlight bounds now use measured text widths across token runs and the active style. The row drawing path passes the same tokens and style to code and highlight drawing. A geometry test checks alignment for different fonts and prefix types.

Changes: Android word-diff highlight alignment

Layer / File(s)Summary
Measure highlight bounds using code styling
apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/ReviewDiffCanvasDrawing.kt		Code-row width measurement now measures requested character intervals across token runs with their font styles. Word-diff bounds use measured widths for the text before and within each range on each visual line.
Pass row styling and test alignment
apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffView.kt, apps/mobile/modules/t3-review-diff/tests/android/src/test/kotlin/expo/modules/t3reviewdiff/ReviewDiffGeometryTest.kt		drawLineRow passes the row’s tokens and style to code and word-diff drawing. The geometry test checks highlight bounds against drawn text for two font styles and ASCII, CJK, and emoji prefixes.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: juliusmarminge, jakeleventhal

Merge Risk: External image Minimal · previous review 43642

Android word-diff highlights now follow the measured text widths used to draw the code, which fixes the reported misplacement. Any residual difference from kerning or ligatures is expected to be sub-pixel. No actionable merge-blocking risk remains.

Pre-merge checks: Passed 4

Check nameStatusExplanation
Title checkPassedThe title "fix(mobile): Android diff word highlights sit on the changed characters" matches the main change. The diff makes Android word-diff highlight boxes measure with the same runs, fonts, and lin…
Linked Issues checkPassedIssue #27146 requires Android word-diff highlights to sit over the changed characters when the monospace family is proportional. The reviewed diff shows drawWordDiffRanges no longer uses `textPaint.…
Out of Scope Changes checkPassedThe reviewed diff touches two source files and one regression test. ReviewDiffCanvasDrawing.kt changes width measurement for word-diff highlights and measureRowWidth, which both serve the #27146 p…
Description checkPassedThe description is thorough and covers the required template sections. Problem: it explains that Android word-highlight boxes are placed using character count times the width of "M", while code is dra…
Full details: Title check

Explanation

The title "fix(mobile): Android diff word highlights sit on the changed characters" matches the main change. The diff makes Android word-diff highlight boxes measure with the same runs, fonts, and line starts that drawCode uses, so they cover the changed text. It follows the conventional commit format and names the platform and the affected feature.

Full details: Linked Issues check

Explanation

Issue #27146 requires Android word-diff highlights to sit over the changed characters when the monospace family is proportional. The reviewed diff shows drawWordDiffRanges no longer uses textPaint.measureText("M") times character count. It now calls the new codeWidth helper for the prefix before the range and for the range itself. codeWidth walks the same token runs as drawCode, configures the same code paint per run, and measures sub-spans with measureText(run.content, start, end). T3ReviewDiffView.drawLineRow now passes the same tokens and style to both highlight and code drawing, so highlight placement uses the same inputs as the drawn text. measureRowWidth also uses codeWidth. The change is limited to visible added or deleted rows with word ranges, and wrapped rows return early through the existing nativeLayout check. The regression test ReviewDiffGeometryTest.unwrappedWordDiffHighlightsCoverTheTextDrawCodeDraws covers the reported case. The author reports it fails on main and passes on this branch. I did not run the tests. This assessment relies on the diff and the reported results.

Full details: Out of Scope Changes check

Explanation

The reviewed diff touches two source files and one regression test. ReviewDiffCanvasDrawing.kt changes width measurement for word-diff highlights and measureRowWidth, which both serve the #27146 placement bug. T3ReviewDiffView.kt only reorders the row token lookup and passes the existing tokens and style to drawWordDiffRanges, which the fix requires. The test file adds the regression check for the same behavior. No unrelated behavior change appears in the reviewed hunks. The codeWidth refactor shares code between highlight placement and row width measurement, so it is part of the same fix.

Full details: Description check

Explanation

The description is thorough and covers the required template sections. Problem: it explains that Android word-highlight boxes are placed using character count times the width of "M", while code is drawn with measured run widths, so they drift with CJK or emoji text and with a proportional monospace font. Change: drawWordDiffRanges now measures with the same runs, fonts, and line starts as drawCode through a shared codeWidth helper, and the caller passes tokens and style. Scope: it is a bug fix linked to issue #27146, with a note on why no separate issue is needed beyond that link. Verification: it gives the focused Gradle test command, the failing and passing output, full module test runs, and before/after device screenshots for Android and iOS. It also lists what was not checked, namely iOS on the branch's older base. The template asks for the model and harness used; the description says it was generated with Claude Opus 5.5 in T3 Code Nightly, so that is covered. The description is long and includes many details in a collapsible section, but the required information is present.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
CcoderabbitaiBotreviewed3d ago
Synced from GitHub
External image Additional comments: 1

apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/ReviewDiffCanvasDrawing.kt: 1

407-408: External image Trivial · Performance & Scalability · Poor tradeoff

External image Unverified finding Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

Cache highlight measurements to avoid repeated paint reconfiguration per range.

Each word-diff range now calls codeWidth twice per visual line. Each call runs configureCodePaint and measureText over the token runs, from column 0 up to to. This work runs on every onDraw for every visible add/delete row that has ranges. Rows with many ranges and long token lists cost O(ranges × runs) per draw.

The cost is bounded to visible rows, so it is acceptable for now. If profiling shows draw regressions, compute prefix offsets once per row. Then derive each range's left and width from the offsets.

As per coding guidelines: "Make sure all changes are considerate of performance impact."

Source: Coding guidelines

External image Review info

External image Run configuration

External image Commits

Reviewing files that changed from the base of the PR and between ad9e87111fd9b2f10ec320b4de0a6ad3a092278a and 5b6dd09eab2a6b7b62bf7289b1cee9cbab9f9559.

External image Files selected for processing: 3

  • apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/ReviewDiffCanvasDrawing.kt
  • apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffView.kt
  • apps/mobile/modules/t3-review-diff/tests/android/src/test/kotlin/expo/modules/t3reviewdiff/ReviewDiffGeometryTest.kt

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

Added 1 commit

  • Merge remote-tracking branch 'origin/main' into fix/android-diff-highlight-measured-width436420c
CcoderabbitaiBotreviewed22h ago
Synced from GitHub
External image Additional comments: 1

apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/ReviewDiffCanvasDrawing.kt-354-354: 1

354-354: External image Minor · Functional Correctness · Quick win

External image Unverified finding Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

Preserve shaping context inside each drawn run.

When a highlight boundary falls inside a kerned pair or ligature, this call measures the substring independently. However, drawCode draws the complete token slice at Line 456. The substring can have different advances, so the highlight can still miss the changed characters. Android 15 provides contextual run advances for measuring offsets without breaking shaping. (raw.githubusercontent.com)

Calculate both boundary advances within the same token-and-visual-line slice that drawCode draws. Subtract those advances for the highlight width. Extend the regression test with a range inside one token; the current cases highlight complete tokens only.

External image Review info

External image Run configuration

External image Commits

Reviewing files that changed from the base of the PR and between e0d0fb33dd438dbe40737683ddd8224ea581ec08 and 436420c7ed8960a75ad7ecebc7a48d2047f87402.

External image Files selected for processing: 3

  • apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/ReviewDiffCanvasDrawing.kt
  • apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffView.kt
  • apps/mobile/modules/t3-review-diff/tests/android/src/test/kotlin/expo/modules/t3reviewdiff/ReviewDiffGeometryTest.kt

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

JJulius Marminge15h ago

The automated pipeline stopped on this pull request and will not merge it:

the proof could not be captured: the agent ran past its 120 minutes

It stays open for a person to finish or close.

Added 1 commit

  • Merge origin/main into fix/android-diff-highlight-measured-widthd8296d3
Oonly21mil4h ago
Synced from GitHub

Note

🤖 Opus 5.5 replying on behalf of Victor.

I ran it on an Android 16 emulator with each revision's own dev client (two cold native builds, each under two minutes): main 5f85768394, which d8296d3ccd merged, then d8296d3ccd, in Review changes on a sample NOTES.md with word wrap off. On main the highlights on the two CJK lines sit one column left, starting one column before the changed characters and ending one short, while the emoji and plain lines are right; on d8296d3ccd every highlight sits on the changed characters. The regression test on d8296d3ccd fails with main's placement lines put back, then passes as is:

text
1ReviewDiffGeometryTest > unwrappedWordDiffHighlightsCoverTheTextDrawCodeDraws FAILED
2 java.lang.AssertionError: weight=medium prefix=if left expected:<64.0> but was:<100.0>
3ReviewDiffGeometryTest > unwrappedWordDiffHighlightsCoverTheTextDrawCodeDraws PASSED

Main 5f85768394: the CJK lines' highlights sit one column left of the changed characters

Main `5f85768394`: the CJK lines' highlights sit one column left of the changed characters

d8296d3ccd: every highlight sits on the changed characters

`d8296d3ccd`: every highlight sits on the changed characters

Main 5f85768394, rows zoom: a crop of the main capture above at 2x

Main `5f85768394`, rows zoom: a crop of the main capture above at 2x

d8296d3ccd, rows zoom: a crop of the d8296d3ccd capture above at 2x

`d8296d3ccd`, rows zoom: a crop of the `d8296d3ccd` capture above at 2x

Added 1 commit

  • Merge origin/main into fix/android-diff-highlight-measured-width15d50b7