fix(ui): preserve CJK scroll alignment

[benvinegar]

Summary

• Preserve the blank half-cell when horizontal scrolling starts inside a fullwidth CJK/emoji grapheme.
• Add regression coverage for sliceTextByWidth("a日本b", 2, 4) so subsequent content no longer shifts left.
• Document the user-visible fix in the changelog.

Testing

• bun test src/ui/lib/ui-lib.test.ts
• bun run typecheck
• bun run format:check

This ports the still-relevant alignment edge case from #287 without taking the stale broader CJK implementation.

This PR description was generated by Pi using OpenAI GPT-5

[greptile-apps]

Greptile Summary

This PR fixes visual misalignment when horizontal scrolling begins inside a fullwidth CJK character or emoji (e.g. offset=2 landing in the middle of a 2-cell 日). Instead of silently skipping the entire character and shifting subsequent content left, the fix inserts a blank space for the "hidden" portion of the character to preserve cell alignment.

• src/ui/lib/text.ts: Adds a padding step inside sliceTextByWidth that emits spaces for the visible cells of a partially-hidden wide cluster before continuing to the next cluster.
• src/ui/lib/ui-lib.test.ts: Updates the regression expectation for sliceTextByWidth("a日本b", 2, 4) from { text: "本b", width: 3 } to { text: " 本b", width: 4 } to capture the alignment fix.
• CHANGELOG.md: Documents the user-visible fix.

Confidence Score: 4/5

Safe to merge — the change is small, self-contained, and the new behavior is directly verified by the updated regression test.

The padding math correctly caps hiddenCellWidth at maxWidth so usedWidth never overflows, and subsequent full-cluster checks remain unaffected. Test coverage captures the key regression but leaves the Math.min cap path and consecutive-wide-char splits without assertions.

src/ui/lib/text.ts and its companion test — the fix looks correct but extra edge-case assertions in ui-lib.test.ts would give stronger confidence against future regressions.

Important Files Changed

Filename	Overview
src/ui/lib/text.ts	Adds blank-space padding for the exposed cells of a wide cluster that is partially hidden by the scroll offset, preserving column alignment for everything that follows. Logic is sound; the hiddenCellWidth > 0 guard is always satisfied by the surrounding conditions but is harmless.
src/ui/lib/ui-lib.test.ts	Single regression test updated to assert the corrected output shape. Covers the core split-CJK case; complementary edge cases (e.g. offset landing in the second cell of consecutive wide chars, or a wide char that spans the entire viewport) are not tested but the critical regression is captured.
CHANGELOG.md	Changelog entry added accurately describing the user-visible fix.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["sliceTextByWidth(text, offset, width)"] --> B{ASCII only?}
    B -- yes --> C[Simple string slice]
    B -- no --> D[Iterate grapheme clusters]
    D --> E{clusterEnd <= startOffset?}
    E -- yes --> F[skip cluster entirely]
    F --> D
    E -- no --> G{clusterStart < startOffset?\ncluster straddles left edge}
    G -- yes --> H["Compute hiddenCellWidth =\nmin(clusterEnd, offset+width) - offset"]
    H --> I["Pad visibleText with spaces\nusedWidth += hiddenCellWidth"]
    I --> J[continue to next cluster]
    J --> D
    G -- no --> K{usedWidth + clusterWidth > maxWidth?}
    K -- yes --> L[break]
    K -- no --> M[Append cluster to visibleText\nusedWidth += clusterWidth]
    M --> D
    D --> N["return { text: visibleText, width: usedWidth }"]
    L --> N

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
src/ui/lib/ui-lib.test.ts:314-320
**Test coverage thin for adjacent and viewport-spanning wide-char splits**

The new regression only covers the case where `offset` lands in the second cell of the first wide character. Two additional edge cases are worth asserting:

1. **Consecutive wide chars, offset in the second character** — e.g. `sliceTextByWidth("日本b", 3, 3)` where the offset lands inside `本` (cells 2–3). Expected: `{ text: " b", width: 2 }`.
2. **Wide character that spans the entire viewport** — e.g. a 4-cell emoji at position 0, offset=1, width=2. The entire result should be two spaces: `{ text: "  ", width: 2 }`. This exercises the `Math.min(clusterEnd, startOffset + maxWidth)` cap path that is currently unreachable by any test.

Without these, a future change that accidentally removes the `Math.min` cap or breaks multi-cluster accumulation would still pass the test suite.

_{Reviews (1): Last reviewed commit: "fix(ui): preserve CJK scroll alignment" | Re-trigger Greptile}

[greptile-apps]

Test coverage thin for adjacent and viewport-spanning wide-char splits

The new regression only covers the case where offset lands in the second cell of the first wide character. Two additional edge cases are worth asserting:

1. Consecutive wide chars, offset in the second character — e.g. sliceTextByWidth("日本b", 3, 3) where the offset lands inside 本 (cells 2–3). Expected: { text: " b", width: 2 }.
2. Wide character that spans the entire viewport — e.g. a 4-cell emoji at position 0, offset=1, width=2. The entire result should be two spaces: { text: " ", width: 2 }. This exercises the Math.min(clusterEnd, startOffset + maxWidth) cap path that is currently unreachable by any test.

Without these, a future change that accidentally removes the Math.min cap or breaks multi-cluster accumulation would still pass the test suite.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/ui/lib/ui-lib.test.ts
Line: 314-320

Comment:
**Test coverage thin for adjacent and viewport-spanning wide-char splits**

The new regression only covers the case where `offset` lands in the second cell of the first wide character. Two additional edge cases are worth asserting:

1. **Consecutive wide chars, offset in the second character** — e.g. `sliceTextByWidth("日本b", 3, 3)` where the offset lands inside `本` (cells 2–3). Expected: `{ text: " b", width: 2 }`.
2. **Wide character that spans the entire viewport** — e.g. a 4-cell emoji at position 0, offset=1, width=2. The entire result should be two spaces: `{ text: "  ", width: 2 }`. This exercises the `Math.min(clusterEnd, startOffset + maxWidth)` cap path that is currently unreachable by any test.

Without these, a future change that accidentally removes the `Math.min` cap or breaks multi-cluster accumulation would still pass the test suite.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[benvinegar]

Added the adjacent-wide-character case (sliceTextByWidth("日本b", 3, 3)) and a viewport-cap case (sliceTextByWidth("日", 1, 1)). I used a 2-cell wide glyph for the cap path because our terminal width model treats emoji grapheme clusters as width 2, not 4.

_{Responded by Pi using OpenAI GPT-5.}

This comment was generated by Pi using OpenAI GPT-5