From d6676f7132a69353b954afd4f2eabd4074bdb069 Mon Sep 17 00:00:00 2001 From: sudacode Date: Fri, 20 Feb 2026 20:22:37 -0800 Subject: [PATCH] fix(renderer): stabilize preserveLineBreaks whitespace and newline rendering --- docs/subagents/INDEX.md | 1 + ...hitespace-newline-20260221T040705Z-aw2j.md | 38 +++++++++++++++++ docs/subagents/collaboration.md | 4 ++ src/renderer/subtitle-render.test.ts | 41 ++++++++++++++++++- src/renderer/subtitle-render.ts | 40 +++++++----------- 5 files changed, 98 insertions(+), 26 deletions(-) create mode 100644 docs/subagents/agents/codex-overlay-whitespace-newline-20260221T040705Z-aw2j.md diff --git a/docs/subagents/INDEX.md b/docs/subagents/INDEX.md index 282ca89..fbb9290 100644 --- a/docs/subagents/INDEX.md +++ b/docs/subagents/INDEX.md @@ -25,3 +25,4 @@ Read first. Keep concise. | `codex-preserve-linebreak-display-20260220T110436Z-r8f1` | `codex-preserve-linebreak-display` | `Fix visible overlay display artifact when subtitleStyle.preserveLineBreaks is disabled` | `completed` | `docs/subagents/agents/codex-preserve-linebreak-display-20260220T110436Z-r8f1.md` | `2026-02-20T11:10:51Z` | | `codex-review-refactor-cleanup-20260220T113818Z-i2ov` | `codex-review-refactor-cleanup` | `Review recent TASK-85 refactor effort and identify remaining cleanup work` | `handoff` | `docs/subagents/agents/codex-review-refactor-cleanup-20260220T113818Z-i2ov.md` | `2026-02-20T11:48:28Z` | | `codex-commit-unstaged-20260220T115057Z-k7q2` | `codex-commit-unstaged` | `Commit all current unstaged repository changes with content-derived conventional message` | `in_progress` | `docs/subagents/agents/codex-commit-unstaged-20260220T115057Z-k7q2.md` | `2026-02-20T11:51:18Z` | +| `codex-overlay-whitespace-newline-20260221T040705Z-aw2j` | `codex-overlay-whitespace-newline` | `Fix visible overlay whitespace/newline token rendering bug with TDD regression coverage` | `completed` | `docs/subagents/agents/codex-overlay-whitespace-newline-20260221T040705Z-aw2j.md` | `2026-02-21T04:18:16Z` | diff --git a/docs/subagents/agents/codex-overlay-whitespace-newline-20260221T040705Z-aw2j.md b/docs/subagents/agents/codex-overlay-whitespace-newline-20260221T040705Z-aw2j.md new file mode 100644 index 0000000..563c88c --- /dev/null +++ b/docs/subagents/agents/codex-overlay-whitespace-newline-20260221T040705Z-aw2j.md @@ -0,0 +1,38 @@ +# Agent Log: codex-overlay-whitespace-newline-20260221T040705Z-aw2j + +- alias: codex-overlay-whitespace-newline +- mission: Fix visible overlay whitespace/newline token rendering bug (blank hoverable tokens) with regression tests. +- status: completed +- started_utc: 2026-02-21T04:07:05Z +- last_update_utc: 2026-02-21T04:14:30Z + +## Intent +- Reproduce bug around spaces/newlines + `preserveLineBreaks` in visible overlay. +- Add failing renderer/tokenization regression tests first. +- Patch render/token mapping to avoid hoverable blank tokens and preserve-mode duplication. + +## Touched Files +- src/renderer/subtitle-render.ts +- src/renderer/subtitle-render.test.ts +- docs/subagents/INDEX.md +- docs/subagents/collaboration.md +- docs/subagents/agents/codex-overlay-whitespace-newline-20260221T040705Z-aw2j.md + +## Decisions +- Ignore whitespace-only token surfaces in source alignment. +- Non-preserve mode: flatten token newlines to spaces, whitespace-only surfaces as text nodes. +- Preserve mode alignment: skip unmatched token surfaces (do not append fallback token) to avoid duplicate tail text. + +## Verification +- RED: new whitespace-separator test failed before fix. +- GREEN: renderer suite pass after first fix. +- RED: new preserve-mode duplicate-tail mismatch test failed before fix. +- GREEN: `bun run build && node dist/renderer/subtitle-render.test.js` pass (11/11). +- GREEN: `node --test dist/renderer/subtitle-render.test.js` pass. + +## Blockers +- none + +## Next Step +- User visual verify with `preserveLineBreaks=true` + known mixed-width numeral lines. +- [2026-02-21T04:18:16Z] follow-up: fixed non-token first-paint behavior to collapse line breaks when `preserveLineBreaks=false` (`normalizeSubtitle(..., collapseLineBreaks=true)` in fallback path); added regression test for normalize collapse. diff --git a/docs/subagents/collaboration.md b/docs/subagents/collaboration.md index 1bf8d87..c7b2aa0 100644 --- a/docs/subagents/collaboration.md +++ b/docs/subagents/collaboration.md @@ -16,3 +16,7 @@ Shared notes. Append-only. - [2026-02-20T11:04:36Z] [codex-preserve-linebreak-display-20260220T110436Z-r8f1|codex-preserve-linebreak-display] overlap note: touching `src/renderer/subtitle-render.ts` + renderer tests to fix preserve-linebreaks disabled display artifact while preserving TASK-91 behavior. - [2026-02-20T11:07:29Z] [codex-preserve-linebreak-display-20260220T110436Z-r8f1|codex-preserve-linebreak-display] completed follow-up for TASK-91: non-preserve mode now flattens token CR/LF to spaces instead of emitting `
` from token surfaces; regression test added. - [2026-02-20T11:10:51Z] [codex-preserve-linebreak-display-20260220T110436Z-r8f1|codex-preserve-linebreak-display] second follow-up: handle overlap token streams by aligning non-preserve rendering to normalized source text and skipping unmatched tail tokens (prevents duplicated second-line phrase). +- [2026-02-21T04:07:05Z] [codex-overlay-whitespace-newline-20260221T040705Z-aw2j|codex-overlay-whitespace-newline] overlap note: touching `src/renderer/subtitle-render.ts` + renderer tests for whitespace/newline display/token hover regression around `preserveLineBreaks`. +- [2026-02-21T04:09:02Z] [codex-overlay-whitespace-newline-20260221T040705Z-aw2j|codex-overlay-whitespace-newline] completed: whitespace-only token surfaces no longer become token segments; non-preserve mode now flattens token newlines to spaces and renders whitespace as text nodes; added regression test in renderer suite. +- [2026-02-21T04:14:30Z] [codex-overlay-whitespace-newline-20260221T040705Z-aw2j|codex-overlay-whitespace-newline] preserve-line-breaks follow-up: when token surface mismatches source (e.g., `1` vs `１`), alignment now skips unmatched token instead of appending both source tail + token; fixes duplicated no-break line artifact. +- [2026-02-21T04:18:16Z] [codex-overlay-whitespace-newline-20260221T040705Z-aw2j|codex-overlay-whitespace-newline] follow-up fix: non-token fallback now honors preserveLineBreaks flag by collapsing line breaks when disabled; prevents visible multi-line -> single-line transition while tokenized payload arrives. diff --git a/src/renderer/subtitle-render.test.ts b/src/renderer/subtitle-render.test.ts index 31f87dc..f257368 100644 --- a/src/renderer/subtitle-render.test.ts +++ b/src/renderer/subtitle-render.test.ts @@ -5,7 +5,7 @@ import path from 'node:path'; import type { MergedToken } from '../types'; import { PartOfSpeech } from '../types.js'; -import { alignTokensToSourceText, computeWordClass } from './subtitle-render.js'; +import { alignTokensToSourceText, computeWordClass, normalizeSubtitle } from './subtitle-render.js'; function createToken(overrides: Partial): MergedToken { return { @@ -216,6 +216,45 @@ test('alignTokensToSourceText preserves newline separators between adjacent toke ); }); +test('alignTokensToSourceText treats whitespace-only token surfaces as plain text separators', () => { + const tokens = [ + createToken({ surface: '常人が使えば' }), + createToken({ surface: ' ' }), + createToken({ surface: 'その圧倒的な力に' }), + createToken({ surface: '\n' }), + createToken({ surface: '体が耐えきれず死に至るが…' }), + ]; + + const segments = alignTokensToSourceText(tokens, '常人が使えば その圧倒的な力に\n体が耐えきれず死に至るが…'); + assert.deepEqual( + segments.map((segment) => (segment.kind === 'text' ? `text:${segment.text}` : 'token')), + ['token', 'text: ', 'token', 'text:\n', 'token'], + ); +}); + +test('alignTokensToSourceText avoids duplicate tail when later token surface does not match source', () => { + const tokens = [ + createToken({ surface: '君たちが潰した拠点に' }), + createToken({ surface: '教団の主力は1人もいない' }), + ]; + + const segments = alignTokensToSourceText( + tokens, + '君たちが潰した拠点に\n教団の主力は１人もいない', + ); + assert.deepEqual( + segments.map((segment) => (segment.kind === 'text' ? `text:${segment.text}` : 'token')), + ['token', 'text:\n教団の主力は１人もいない'], + ); +}); + +test('normalizeSubtitle collapses explicit line breaks when collapseLineBreaks is enabled', () => { + assert.equal( + normalizeSubtitle('常人が使えば\\Nその圧倒的な力に\\n体が耐えきれず死に至るが…', true, true), + '常人が使えば その圧倒的な力に 体が耐えきれず死に至るが…', + ); +}); + test('JLPT CSS rules use underline-only styling in renderer stylesheet', () => { const distCssPath = path.join(process.cwd(), 'dist', 'renderer', 'style.css'); const srcCssPath = path.join(process.cwd(), 'src', 'renderer', 'style.css'); diff --git a/src/renderer/subtitle-render.ts b/src/renderer/subtitle-render.ts index 41abf36..fa0d1a6 100644 --- a/src/renderer/subtitle-render.ts +++ b/src/renderer/subtitle-render.ts @@ -9,7 +9,11 @@ type FrequencyRenderSettings = { bandedColors: [string, string, string, string, string]; }; -function normalizeSubtitle(text: string, trim = true, collapseLineBreaks = false): string { +function isWhitespaceOnly(value: string): boolean { + return value.trim().length === 0; +} + +export function normalizeSubtitle(text: string, trim = true, collapseLineBreaks = false): string { if (!text) return ''; let normalized = text.replace(/\\N/g, '\n').replace(/\\n/g, '\n'); @@ -140,24 +144,13 @@ function renderWithTokens( } for (const token of tokens) { - const surface = token.surface; + const surface = token.surface.replace(/\n/g, ' '); + if (!surface) { + continue; + } - if (surface.includes('\n')) { - const parts = surface.split('\n'); - for (let i = 0; i < parts.length; i += 1) { - const part = parts[i]; - if (part) { - const span = document.createElement('span'); - span.className = computeWordClass(token, resolvedFrequencyRenderSettings); - span.textContent = part; - if (token.reading) span.dataset.reading = token.reading; - if (token.headword) span.dataset.headword = token.headword; - fragment.appendChild(span); - } - if (i < parts.length - 1) { - fragment.appendChild(document.createElement('br')); - } - } + if (isWhitespaceOnly(surface)) { + fragment.appendChild(document.createTextNode(surface)); continue; } @@ -187,17 +180,14 @@ export function alignTokensToSourceText( for (const token of tokens) { const surface = token.surface; - if (!surface) { + if (!surface || isWhitespaceOnly(surface)) { continue; } const foundIndex = sourceText.indexOf(surface, cursor); if (foundIndex < 0) { - if (cursor < sourceText.length) { - segments.push({ kind: 'text', text: sourceText.slice(cursor) }); - } - segments.push({ kind: 'token', token }); - cursor = sourceText.length; + // Token text can diverge from source normalization (e.g., half/full-width forms). + // Skip unmatched token to avoid duplicating visible tail text in preserve-line-break mode. continue; } @@ -318,7 +308,7 @@ export function createSubtitleRenderer(ctx: RendererContext) { return; } - const normalized = normalizeSubtitle(text); + const normalized = normalizeSubtitle(text, true, !ctx.state.preserveSubtitleLineBreaks); if (tokens && tokens.length > 0) { renderWithTokens( ctx.dom.subtitleRoot,