Skip to content

Fix timestamp editor reading short fractions as milliseconds - #1660

Merged
raivisdejus merged 2 commits into
chidiwilliams:mainfrom
Dev-next-gen:fix/timestamp-editor-fraction
Oct 17, 2026
Merged

raivisdejus merged 2 commits into
chidiwilliams:mainfrom
Dev-next-gen:fix/timestamp-editor-fraction

Conversation

@Dev-next-gen

Copy link
Copy Markdown
Contributor

In the transcription viewer, editing a segment's start or end cell and typing 00:00:05.5 stores 5005 ms, not 5500 ms. On focus out the cell is redrawn as 00:00:05.005. Typing 00:00:05.1234 stores 6234 ms, so the extra digit pushes the segment more than a second later.

parse_timestamp reads everything after the dot as a whole number of milliseconds (int("5"), int("1234")), whatever its length. The editor itself writes the fraction as three digits (to_timestamp), so 05.5 and 05.005 end up as the same value even though the cell shows 05.500 for half a second.

The fix reads the fraction as a decimal: it is padded or cut to three digits before converting, so .5 is 500 ms, .45 is 450 ms and .1234 is 123 ms. Three-digit input such as 01:23:45.678 gives the same result as before.

I added parser tests for short and long fractions and a TimeStampLineEdit test that types 00:00:05.5 and fires a focus out. Before the change the three new tests fail (5005 != 5500, 6234 != 5123, and the widget's get_milliseconds() returns 5005). After it they pass, along with the other tests in transcription_segments_editor_widget_test.py (44 passed).

Found by a defect-hunting pipeline I build and run (Dev-next-gen), using Claude Code with Anthropic's Claude Opus 5.5.

Dev-next-gen and others added 2 commits October 14, 2026 22:58
Typing 00:00:05.5 in a segment start or end cell stored 5005 ms
instead of 5500 ms, and 00:00:05.1234 stored 6234 ms. The digits after
the dot were parsed as a whole number of milliseconds regardless of
their count.
@raivisdejus
raivisdejus enabled auto-merge (squash) October 17, 2026 08:32
@codecov

codecov Bot commented Oct 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.20%. Comparing base (053b388) to head (8df1b6c).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1660      +/-   ##
==========================================
- Coverage   83.88%   83.20%   -0.69%     
==========================================
  Files         115      115              
  Lines       13834    13835       +1     
==========================================
- Hits        11605    11511      -94     
- Misses       2229     2324      +95     
Flag Coverage Δ
Linux 83.20% <100.00%> (+0.04%) ⬆️
macOS ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Dev-next-gen

Copy link
Copy Markdown
Contributor Author

The red test (macos-latest) stops on tests/widgets/speaker_identification_widget_test.py::TestSpeakerIdentificationWidget::test_identification_worker_run (assert 0 == 1). The worker logs Can't load tokenizer for 'kredor/punctuate-all' right after fetching that model from the Hugging Face cache during the test (job log). This PR only touches the timestamp editor and its tests. Because pytest.ini has -x, the run ended there at 978 passed, before it reached transcription_segments_editor_widget_test.py. On Ubuntu (1368 passed) and Windows (1316 passed), the four timestamp tests show PASSED.

I couldn't find a main run where this test fails: the last two test (macos-latest) runs on main and #1659 passed. So I read it as a download hiccup, not as something this change can cause. Re-running that one job would settle it, if you have a moment.

Found by a defect-hunting pipeline I build and run (Dev-next-gen), using Claude Code with Anthropic's Claude Opus 5.5.

@raivisdejus
raivisdejus merged commit 6186d09 into chidiwilliams:main Oct 17, 2026
13 of 16 checks passed
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