1144-Fix VTT target import overwriting a cell with its last overlapping sub-cue

[sampjvv]

Summary

Closes #1144

Importing a subtitle file as a target let the last cue overlapping a cell replace that cell's real match — text and timestamps — while the import toast reported child cells that were never written. On the reported case (the_chosen_test 204 + TheChosen_204_bod_retimed.vtt) this destroys 102 of 408 cells.

Both reproductions in the issue were confirmed live before and after the fix, using minimal two-cue files:

Case	Before	After
Source cue nested in the previous cue	Cell A held TRANSLATION B @ 12→14; cell B empty	Cell A TRANSLATION A @ 10→18; cell B TRANSLATION B @ 12→14
Two target cues over one source cell	Cell held only SUB-CUE B @ 12→14	Cell holds TRANSLATION A SUB-CUE B @ 10→18

Two independent defects

1. The aligner's tie-break sent nested cues to the wrong cell. subtitlesCellAligner scored candidates with if (overlap > maxOverlap), so an exact tie always fell to the earlier cell. When a source cell's range is nested inside the previous cell's, a cue timed like the nested cell overlaps both by exactly the same amount — it was handed to the enclosing cell as an additional overlap, and the nested cell was left empty forever.

Ties are now broken by how closely the two ranges coincide (overlap divided by the span they jointly cover). A cue matching the nested cell exactly scores 1.0 against it and ~0.14 against the enclosing one. Only exact ties change: ordinary one-to-one alignments and straddling cues are untouched, which the new tests pin down.

2. The write path could not express "an extra cue on this cell", so it overwrote instead. handleWriteTranslation keyed every aligned entry by the parent cell's id:

const targetId = alignedCell.notebookCell?.metadata?.id ?? alignedCell.importedContent.id;

For an isAdditionalOverlap entry notebookCell is the parent, so the fallback after ?? was dead code and each extra cue replaced the primary match in updatesMap — last one wins. And since the rebuild loop walks existingNotebook.cells, an aligned entry could never have produced an extra cell in the first place; only the paratext collection could.

Additional cues are now folded into the text of the cell they overlap, in temporal order, with the cell keeping its best match's timestamps (extra cues contribute text only — widening the range would push the cell into overlapping its neighbour). What was folded in is recorded in metadata.data.mergedOverlaps, so the merge is auditable and nothing is silently lost.

Merging rather than creating child cells was a deliberate call. The ${parentId}:${suffix} id the old aligner built is a legacy convention: migration_cellIdsToUuid only attaches parentId when an id has more than two colon segments, so a uuid:suffix id would later be hashed into a fresh UUID with the parent link silently dropped. Real child cells would also trip the modal overlapping-cue warning on every subtitle export, and be dropped by reimportMerge on a source re-import.

Also in this change

• The write path's in-memory core moves to a pure translationWriteMerge module, leaving file I/O and the toast in the provider. This mirrors reimportMerge.ts in the same directory ("pure (no vscode imports) so the merge is unit-testable") and is what makes the write path testable at all — it had no tests before.
• Import stats now match what was written. insertedCount was counting unmatched empty cells as translations (the aligner echoes them back with their own empty content), and childCellCount reported cells that were never created. It is replaced by mergedCueCount, and the notification says how many overlapping cues were merged in.
• The alignment preview no longer overstates the result. Extra cues were listed as ordinary rows and counted in "Matched Cells" even though they never became cells; they now carry a "Merged into cell above" badge and are excluded from that count.
• Deleted compiletimeTests.ts. Nothing imported it, its one compile-time assertion was defanged by an as any (and its type resolved to 'FAILED' regardless of runtime behaviour), and its async checks were fired and forgotten. Replaced by real tests. testData.ts and referenceData.ts are kept.

Repair for projects already damaged

Re-importing a corrected file cannot fix an affected project, because the write path skips any cell that already has text. This adds a manual, idempotent command — "Codex: Repair Subtitle Cells Overwritten by a Sub-Cue" — following the two-pass dry-run → confirm → apply shape of migration_repairVerseRangeDuplication.

The imported VTT is not retained anywhere, so the correct text is genuinely unrecoverable. The repair therefore restores what it can and stages the rest: it puts the cell's timings back from the .source, moves the stranded sub-cue text into metadata.data.repairedFromValue rather than discarding it, empties the cell, and appends MIGRATION edits so the change survives the CRDT sync merge. The confirmation dialog states plainly that a corrected re-import is required afterwards.

Detection is precise rather than heuristic at the file level: lastTranslationImport.stats.childCellCount was only ever written by the buggy importer, and it never wrote a child cell (the fixed one writes mergedCueCount). Within such a file, a cell is flagged when it has text and its range is a strict, materially shorter sub-range of its same-id .source cell. A false positive is self-healing — the cell is cleared and refilled by the re-import the command tells you to run — and the original text is preserved either way.

Testing

• webviews/.../subtitles/aligner.test.ts — new. Two cues over one cell, a cue nested inside the previous cue, the reported 204 timings, plus guards that ordinary and straddling alignments do not move. Three of these fail against the unfixed aligner, verified by reverting it.
• src/test/suite/unit/translationWriteMerge.test.ts — new. An extra cue never replaces the primary; no extra cell is created; text joins in temporal order regardless of the order entries arrive in; already-translated cells and milestones are still skipped; stats equal what is in the returned notebook.
• src/test/suite/unit/subtitleTargetImport.integration.test.ts — new. The real aligner feeding the real write path over both reported cue layouts. The two halves of the fix are independent, so this is what proves the reported symptom is actually gone.
• src/test/suite/unit/subtitleOverlapRepair.test.ts — new. Fingerprint detection, damaged-vs-retimed discrimination, idempotency, and that untouched cells stay untouched.
• Full extension-host suite 1550 passing, webview suite 745 passing, lint and tsc --noEmit clean.
• The repair was additionally run against the actual damaged .codex files captured from the two live reproductions: exact detection, timings restored, stranded text preserved, no-op on a second run.

🤖 Generated with Claude Code

---

Round 2: a corrupted export heals itself on re-import

A file exported from a project damaged by the old importer carries the damage in the file itself: the enclosing cell's cue is stamped with its nested cell's exact timing (and often its text), and no cue covers the true span. Re-importing such a file sent both identical-timed cues to the nested cell and left the enclosing cell empty — clients who kept working in a damaged project deliver their fixes through exactly these exports, so the import now repairs the file's damage silently. Confirmed on a real client export (episode 204, Tibetan): 8 of 408 cues carried a displaced nested timing; all 8 are detected and restored to their true cells. One nuance to expect when testing with such a file: the old bug destroyed text as well as timing, so the export can hold the nested cue's text for both cues. The import then places that identical text in both cells — correct cell, correct timing, and the file's own (damaged) text made visible rather than hidden behind an empty cell. It is deliberately imported as-is: the moment the client corrects the line and re-uploads, the update path below replaces it.

Four changes:

• Cue identifiers now round-trip. The subtitle parser keeps the cue identifier line as data.originalCueId, and the aligner routes a cue whose identifier is one of the notebook's own cell ids straight to that cell (exact-id, confidence 1). Re-importing a project's own export therefore matches exactly, with no timestamp guesswork. Plain numbered files never collide with a cell UUID and are unaffected.
• A repair pre-pass in the aligner (cueRepair.ts, pure) detects the corruption fingerprint — two cues with identical timestamps where one cell matches that range exactly and a materially longer enclosing cell has no cue of its own — and re-times the displaced cue to the enclosing cell's true range before alignment. Identical (start, end) pairs on two cues never occur in legitimate subtitle data (the source side of the 204 pair has zero; the damaged export has exactly the 8), so the detection is precise; anything ambiguous is left untouched. Because the write path stamps the primary cue's timing onto the cell, the same pass restores damaged cell timestamps when the file is re-imported into the damaged project itself.
• Aligners now receive the .source cells. fetchTargetFile only ever sent target cells; every aligner got [] for sourceCells. The source ranges are the authoritative reference the repair needs, since target cells carry the damage in a same-project re-import.
• A re-import now updates machine-written translations. The write path used to skip any cell that already had text, so a corrected re-upload could never deliver a fix. Differing imported text now replaces a machine-written value, recording the previous and new value (and any timing change) as initial-import edit-history entries — nothing is discarded, and the change survives the sync merge instead of being reverted. Text a person typed in the editor, or a validated value, is never replaced; empty incoming text never clobbers real text. The import toast reports these as "N updated".

Testing (round 2)

• webviews/.../subtitles/cueRepair.test.ts — new. The real 204 timings, the legitimate-nested-translation case, the non-starving-parent and no-matching-cell guards, the ambiguity bail-out, the materially-shorter threshold, float-noise tolerance, and multiple damaged pairs in one file.
• webviews/.../subtitles/aligner.test.ts — extended. Id-routing regardless of timing, id outranking a timestamp match, numbered-file identifiers ignored, fresh-project repair, same-project repair (id routing plus source timings), and that a lone nested-timed cue or a parent with its own cue is left alone.
• src/test/suite/unit/translationWriteMerge.test.ts — extended. Overwrite on differing machine-written text; previous and new value recorded in the edit history; timing changes recorded as edits; identical text, human-typed text, validated values, and empty incoming text all left alone.
• End-to-end against the real client files: the actual English source and the corrupted Tibetan export run through the real parser and aligner — all 8 displaced cues restored, the enclosing cell receives its cue re-timed to the true span, the nested cell keeps exactly its own cue, zero mis-merges, zero orphaned paratext.
• Subtitles vitest suites 34 passing; targeted extension-host suites (translationWriteMerge, subtitleTargetImport, vttRoundtrip) 24 passing; tsc -b and lint clean. (The full webview run currently shows 14 failures in ParallelView/SearchTab — a pre-existing localStorage-in-jsdom environment issue, reproduced identically on an untouched checkout.)

---

Round 3: making a mistaken re-import survivable

Round 2 let an import replace machine-written translations so a client's corrected export could land. That is right for the right file and a quiet catastrophe for the wrong one: another episode's subtitles align by timestamp just as neatly, because every project starts at zero. A mis-clicked file would rewrite an entire translation and report it as a success. This round is a review pass over that new power, plus the three smaller gaps the same review turned up.

• A bulk overwrite now asks first. When not one cue named a cell by id — so the file is nobody's export of this project — and it would still replace at least ten translations amounting to more than half the cells that hold work, the user gets a modal naming the file and the counts. Declining writes nothing at all and returns the wizard to target selection. Every other import stays silent, including a legitimate first delivery into an empty project, which only inserts. Confirmed against the real 204 project: re-uploading its own corrected export shows no dialog, while importing the English source over the Tibetan translation offers to replace 391 of 399 cells and stops.

• Edit histories written before edit maps existed are recognized again. valueIsHumanAuthored only understood the current shape (editMap: ["value"]), but projects that predate it store the text as cellValue with no editMap and are only normalized when a sync merge or the edit-history migration happens to touch them (see resolvers.ts and migrationUtils.ts). Until then a person's typed translation looked machine-written, and round 2 would have overwritten it. Both shapes now count, for the protection check and for the back-fill that preserves the previous value.

• A timing-only correction gets through. Identical text used to mean "skip the cell entirely", timing included, so a client who fixed a line purely by retiming it saw nothing happen — and a cell displaced onto a nested range could never recover, because its text was already right. A cell that keeps its text now takes a corrected range, but only from a cue matched by cell id, so a file whose clock is offset cannot shift a whole notebook. Reported as "N retimed".

• Locked and validated cells are left alone. The editor blocks even its own timestamp updates on a locked cell, so an import writing the .codex directly must not be the way around that; no locked cell is touched now. A validated cell is not retimed either: the editor decides what to show as validated from the last entry in a cell's history whatever kind of edit it is, so appending a timing edit would quietly clear someone's sign-off. An automatic correction is not worth that, so it stands down. A withdrawn validation does not block it.

• The damage marker survives later imports. lastTranslationImport.stats.childCellCount is the only thing the repair command uses to recognize a damaged project, and every import overwrote that record — so importing anything at all, even a fully-skipped file, made a damaged project look clean forever. It is now carried forward. A healed project is unaffected: the per-cell scan finds nothing and the command reports nothing.

• A cell keeps its own type. The write path stamped text on everything it touched. Timed paratext cells are exported as cues like any other, so a round trip silently promoted them into ordinary translation cells.

Testing (round 3)

• src/test/suite/unit/translationWriteMerge.test.ts — extended by roughly thirty cases: the guard's thresholds from both sides; legacy cellValue edits protecting a cell and satisfying the back-fill; the retime's conditions one at a time (id-matched only, non-empty text only, not over a hand-placed range, not on a validated cell, sub-millisecond changes ignored, only the field that moved written); locked cells never written to, filled, or retimed; the marker carried forward and not invented; paratext surviving both an overwrite and an insert.
• Targeted extension-host suites 78 passing; subtitles vitest 39 passing; tsc -b and lint clean.
• End-to-end against the real 204 files again, at both ends of the workflow. Re-importing the damaged export into the project it came from restores all eight displaced ranges — six through the retime path with their corrections recorded in edit history, two textless ones through the empty-cell path — and changes nothing else in the other 400 cells, in text or in time. Importing that same export into a fresh project puts all 408 cues in their own cells: no paratext, no merges, and the only empty cells are the nine cues the file carries no text for.

One thing that surfaced while building that harness and is worth knowing for anyone testing this area: WebVTTParser returns cues sorted by start time, not in file order, which reorders precisely the cues this bug displaces. Any test that pairs a parsed target cue with a source cue by index will be wrong about which cell is which.

---

Round 4: two numbers stop lying

Both surfaced after round 3 went out, and both are about the import telling the truth rather than changing what it does.

A legitimate re-upload tripped the round-3 bulk-overwrite warning. A large, well-populated project, a corrected file with only a few real changes — and the modal claimed hundreds of translations would be replaced. The cause was that "did this text change" was byte equality, and a subtitle file that has been through a third-party editor comes back with the same words written differently: composed characters decomposed (Unicode NFD vs NFC — Tibetan is full of such pairs), zero-width spaces inserted as line-wrap hints, lines re-broken, ordinary spaces swapped for non-breaking ones. Every line then compares unequal, so a handful of edits counts as hundreds of "updates" — which trips the modal, and, worse, rewrites every one of those cells' edit histories and timings on each such re-upload.

Text is now compared in a normalized form: NFC, invisible break hints stripped (zero-width space, soft hyphen, BOM), whitespace runs collapsed, trimmed. A cell whose incoming text normalizes equal is skipped exactly as a byte-identical one always was — nothing written, nothing counted. Characters that change what is rendered (ZWNJ, ZWJ, the bidi marks) are deliberately not stripped: a difference there is a real difference. The known trade, made deliberately: a whitespace-only client edit (a re-broken line, a French non-breaking space before punctuation) is treated as no change. Our own exporter re-flows line breaks on the way out (ensureDialogueLineBreaks), so a stored newline was never a faithful record of intent to begin with.

Measured on the real 204 project: a simulated hostile-editor round trip (NFD everywhere, zero-width spaces between characters, NBSP for space, cue ids stripped) produced 399 phantom updates under byte equality — modal fires — and produces 0 under the normalized comparison — nothing written, no modal. When the modal does fire, the provider now logs up to three sample replacements, so the next report of a false positive is diagnosable from the dev console instead of a guessing game.

The "800% confidence" badge. The subtitle aligner reported its confidence as the raw seconds of overlap, which the preview multiplied by 100 — an 8-second overlap displayed as "800% confidence", and since the badge turns green at 0.8, every match longer than 0.8s has painted green for as long as the badge has existed. The number now reported is how completely the cue's time range and the cell's coincide (intersection over union — the same quantity the round-1 tie-break already trusts), and the preview labels it timing fit rather than confidence: a confidence measure is a promise about error rates attached to a claim, and matching clocks supports no claim about whether the words correspond. Only an id match — the file naming the cell outright — still claims confidence, at 1.0, which is the one place that promise is defensible. Two adjacent fixes ride along: unmatched target cells that the aligner echoes back untouched used to report confidence 1.0 and count as "Matched Cells" (untouched cells looked like the import's surest work); they are now flagged as pass-throughs, carry no score, and are excluded from the count and the average.

Testing (round 4)

• src/test/suite/unit/translationWriteMerge.test.ts — a rewrite-suite driven by a fixture chosen so every rewrite is genuinely non-identity (an earlier draft used Tibetan alone, which has no canonical decomposition — six of its eight tests passed with the feature deleted; a guard test now asserts each rewrite changes bytes and survives trimming). Mutation-verified: reverting the normalized comparison fails 8 tests; deleting just the NFC step fails 3; the invisible-character strip 2; the whitespace collapse 2.
• webviews/.../subtitles/aligner.test.ts — timestamp matches report IoU (exact range → 1.0, 8s-in-10s → 0.8, brief-cue-in-long-cell → low), folded cues report their own fit, pass-throughs carry no score, id matches still report 1.
• End-to-end on the real files: the round-3 heal is unchanged (damaged fixture: 8 wrong ranges → 0, same toast), the wrong-file import still triggers the confirmation, and all timestamp scores now lie in [0, 1].
• Targeted extension suites 102 passing; webview NewSourceUploader suites 208 passing; tsc -b and lint clean.

[LeviXIII]

/build

[github-actions]

Pre-release: 0.32.0-pr1146-94db4bc https://github.com/genesis-ai-dev/codex-editor/releases/tag/0.32.0-pr1146-94db4bc

[LeviXIII]

/build

[github-actions]

Pre-release: 0.32.0-pr1146-5207813 https://github.com/genesis-ai-dev/codex-editor/releases/tag/0.32.0-pr1146-5207813

[LeviXIII]

/build

[github-actions]

Build failed: https://github.com/genesis-ai-dev/codex-editor/actions/runs/33006618071

[github-actions]

Build failed: https://github.com/genesis-ai-dev/codex-editor/actions/runs/33006618071

[LeviXIII]

/build

[github-actions]

Build failed: https://github.com/genesis-ai-dev/codex-editor/actions/runs/33015359711

[LeviXIII]

/build

[github-actions]

Pre-release: 0.32.0-pr1146-6912f28 https://github.com/genesis-ai-dev/codex-editor/releases/tag/0.32.0-pr1146-6912f28