Skip to content

fix chunk overlap timestamp merge - #54

Open
ficapy wants to merge 1 commit into
senstella:masterfrom
ficapy:master
Open

fix chunk overlap timestamp merge#54
ficapy wants to merge 1 commit into
senstella:masterfrom
ficapy:master

Conversation

@ficapy

@ficapy ficapy commented Jun 7, 2026

Copy link
Copy Markdown

Background

This PR fixes an issue in chunked transcription where merging overlapping chunks can produce a token sequence whose text order does not match its timestamp order.

In some overlap cases, tokens from the previous chunk and the current chunk are aligned using either a contiguous match or the LCS fallback. The previous implementation rebuilt the merged token sequence from those matches, but it did not guarantee that the final token start timestamps remained monotonically increasing.

That can break the following invariant in the JSON output:

sentence.text == "".join(token.text for token in sentence.tokens)

The reason is that sentence.text is built from the merged token order, while AlignedSentence.__post_init__() sorts sentence.tokens by token.start.

One observed example was that the sentence text was correct:

local Olama server on this computer.

but joining the exported tokens produced:

local. Olama server on this computer

Test Files

I used three files to reproduce and isolate the issue:

demo.webm is the full reproduction case. With chunk_duration=120 and overlap_duration=15, it is split like this:

chunk 1: demo 0s-120s
chunk 2: demo 105s-225s
overlap: demo 105s-120s

segment1.webm and segment2.webm are the two individual chunks from that split:

segment1.webm: demo 0s-120s
segment2.webm: demo 105s-225s

They are included to show that the problem comes from merging the two overlapping chunk hypotheses, not from a single chunk transcription.

In the overlap region, the first chunk ends with:

local.

while the second chunk continues with:

local Olama server on this computer

The old merge logic could combine these into a sequence whose text order looked correct, but whose token timestamps caused the exported tokens to be sorted as:

local. Olama server on this computer

Changes

This PR keeps the existing overlap matching logic and only changes how the matched tokens are merged afterward:

  • Adds a shared _merge_from_pairs() helper used by both contiguous matching and the LCS fallback
  • Only appends tokens that keep the merged sequence time-ordered
  • For matched anchor tokens, prefers the token from the previous chunk, but falls back to the token from the current chunk if the previous one would move time backward
  • Falls back to the existing cutoff merge strategy when there are too few overlap tokens, no LCS match, or the rebuilt sequence is still not time-ordered

This keeps the merged token sequence consistent in both text order and timestamp order.

Validation

I reproduced the issue with demo.webm using the default chunk overlap settings:

uv run python -m parakeet_mlx.cli demo.webm \
  --output-format json \
  --chunk-duration 120 \
  --overlap-duration 15

I also used segment1.webm and segment2.webm with chunking disabled to inspect the two chunk hypotheses independently:

uv run python -m parakeet_mlx.cli segment1.webm segment2.webm \
  --output-format json \
  --chunk-duration 0

After generating the JSON output from the fixed code, I checked the following invariants with a separate validation script:

  • sentence.text == "".join(token.text for token in sentence.tokens)
  • Token start timestamps do not move backward within each sentence
  • Token start timestamps do not move backward globally

The sentence that previously reproduced the issue is now consistent:

A llama is running locally, so when this Python file sends a request there, it's talking to the local Olama server on this computer.

Joining the exported tokens no longer produces local. Olama.

@senstella

Copy link
Copy Markdown
Owner

Hello! First of all, thank you for the pull request. But it raises a few questions I'd want to ask before merging it.

First, regarding to _merge_from_pairs, it has interesting design choice of dropping tokens that doesn't follow monotonic sequences. But consider this sequence

a = [
    AlignedToken(id=1, text="t1", start=5.00, duration=0.30),
    AlignedToken(id=2, text="t2", start=7.00, duration=0.30),
]
b = [
    AlignedToken(id=1, text="t1", start=4.50, duration=0.30),
    AlignedToken(id=3, text="t3", start=4.95, duration=0.30), # real token (first pass missed it)
    AlignedToken(id=2, text="t2", start=7.00, duration=0.30),
]

where first forward failed to predict the t3 but second pass correctly did. In current algorithm you have implemented, it would drop t3. Or say, more realistically,

a = [
    AlignedToken(id=99, text="t99", start=4.50, duration=0.30),
    AlignedToken(id=1, text="t1", start=5.00, duration=0.30),  # model stretch it to the end
]
b = [
    AlignedToken(id=99, text="t99", start=4.50, duration=0.30),
    AlignedToken(id=1, text="t1", start=4.60, duration=0.30),
    AlignedToken(id=2, text="t2", start=4.95, duration=0.30),  # new stuff from here
    AlignedToken(id=3, text="t3", start=6.00, duration=0.30),
]

It would drop t2 here, which will reintroduce #10 kind of bugs. Perhaps not dropping but force aligning them to the previous token (since order is correct before __post_init__) would be less invasive. Or better, tightening the overlap_duration / 2 in various usages,

                    and abs(overlap_a[k].start - overlap_b[l].start)
                    < overlap_duration / 2

or introducing the better heuristic would be a better fix for this issue.

Second, in this line

+return result if _is_time_ordered(result) else _merge_at_cutoff(a, b)

_merge_at_cutoff will never be called since _is_time_ordered always returns True with the guarantee of _append_time_ordered. Hence _is_time_ordered can be safely removed and we can have one happy path of result.

Third, it's relatively minor nitpicking but it introduces too much helper functions at once. Like _append_aligned_token ever get used once, I think it can be safely inlined to the parent helper functions and it'd improve the readability of the whole codebase at one glance too!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants