mirror of
https://github.com/ksyasuda/SubMiner.git
synced 2026-02-27 18:22:41 -08:00
fix(renderer): stabilize preserveLineBreaks whitespace and newline rendering
This commit is contained in:
@@ -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-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-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-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` |
|
||||||
|
|||||||
@@ -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.
|
||||||
@@ -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: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 `<br>` from token surfaces; regression test added.
|
- [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 `<br>` 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-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 `1`), 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.
|
||||||
|
|||||||
@@ -5,7 +5,7 @@ import path from 'node:path';
|
|||||||
|
|
||||||
import type { MergedToken } from '../types';
|
import type { MergedToken } from '../types';
|
||||||
import { PartOfSpeech } from '../types.js';
|
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>): MergedToken {
|
function createToken(overrides: Partial<MergedToken>): MergedToken {
|
||||||
return {
|
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教団の主力は1人もいない',
|
||||||
|
);
|
||||||
|
assert.deepEqual(
|
||||||
|
segments.map((segment) => (segment.kind === 'text' ? `text:${segment.text}` : 'token')),
|
||||||
|
['token', 'text:\n教団の主力は1人もいない'],
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
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', () => {
|
test('JLPT CSS rules use underline-only styling in renderer stylesheet', () => {
|
||||||
const distCssPath = path.join(process.cwd(), 'dist', 'renderer', 'style.css');
|
const distCssPath = path.join(process.cwd(), 'dist', 'renderer', 'style.css');
|
||||||
const srcCssPath = path.join(process.cwd(), 'src', 'renderer', 'style.css');
|
const srcCssPath = path.join(process.cwd(), 'src', 'renderer', 'style.css');
|
||||||
|
|||||||
@@ -9,7 +9,11 @@ type FrequencyRenderSettings = {
|
|||||||
bandedColors: [string, string, string, string, string];
|
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 '';
|
if (!text) return '';
|
||||||
|
|
||||||
let normalized = text.replace(/\\N/g, '\n').replace(/\\n/g, '\n');
|
let normalized = text.replace(/\\N/g, '\n').replace(/\\n/g, '\n');
|
||||||
@@ -140,24 +144,13 @@ function renderWithTokens(
|
|||||||
}
|
}
|
||||||
|
|
||||||
for (const token of tokens) {
|
for (const token of tokens) {
|
||||||
const surface = token.surface;
|
const surface = token.surface.replace(/\n/g, ' ');
|
||||||
|
if (!surface) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
if (surface.includes('\n')) {
|
if (isWhitespaceOnly(surface)) {
|
||||||
const parts = surface.split('\n');
|
fragment.appendChild(document.createTextNode(surface));
|
||||||
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'));
|
|
||||||
}
|
|
||||||
}
|
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -187,17 +180,14 @@ export function alignTokensToSourceText(
|
|||||||
|
|
||||||
for (const token of tokens) {
|
for (const token of tokens) {
|
||||||
const surface = token.surface;
|
const surface = token.surface;
|
||||||
if (!surface) {
|
if (!surface || isWhitespaceOnly(surface)) {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
const foundIndex = sourceText.indexOf(surface, cursor);
|
const foundIndex = sourceText.indexOf(surface, cursor);
|
||||||
if (foundIndex < 0) {
|
if (foundIndex < 0) {
|
||||||
if (cursor < sourceText.length) {
|
// Token text can diverge from source normalization (e.g., half/full-width forms).
|
||||||
segments.push({ kind: 'text', text: sourceText.slice(cursor) });
|
// Skip unmatched token to avoid duplicating visible tail text in preserve-line-break mode.
|
||||||
}
|
|
||||||
segments.push({ kind: 'token', token });
|
|
||||||
cursor = sourceText.length;
|
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -318,7 +308,7 @@ export function createSubtitleRenderer(ctx: RendererContext) {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
const normalized = normalizeSubtitle(text);
|
const normalized = normalizeSubtitle(text, true, !ctx.state.preserveSubtitleLineBreaks);
|
||||||
if (tokens && tokens.length > 0) {
|
if (tokens && tokens.length > 0) {
|
||||||
renderWithTokens(
|
renderWithTokens(
|
||||||
ctx.dom.subtitleRoot,
|
ctx.dom.subtitleRoot,
|
||||||
|
|||||||
Reference in New Issue
Block a user