Skip to content

Fix subtitle parsing, autosave and settings reset; add tests and CI - #1

Merged
angushushu merged 5 commits into
mainfrom
fix/reliability-fixes
Sep 27, 2026
Merged

angushushu merged 5 commits into
mainfrom
fix/reliability-fixes

Conversation

@angushushu

@angushushu angushushu commented Sep 27, 2026 •

Copy link
Copy Markdown
Member

Summary

Bug fixes

  • The SRT parser dropped subtitle text.
    • A text line starting with a digit (e.g. "I owe you / 20 dollars.") cut off the rest of its cue.
    • Timestamps using . for milliseconds dropped the whole cue.
    • Cues are now split on blank lines. The index line is optional, cue settings after the timing are ignored, and the BOM is stripped. On a real 299-cue file the output is identical to before.
  • Autosave overwrote the project.
    • With a .vat open, autosave rewrote it every 15 s, so answering No to "Save before exit?" didn't discard anything.
    • The autosave file was never deleted, so every launch offered to restore it, and restoring made the hidden file the working project.
    • Autosave now writes .<name>.autosave.vat, which records the project it belongs to. Save and discard delete it. Restore is offered only when the autosave is newer than the project, and continues on the original project.
  • Failed or cancelled saves. A failed or cancelled save now keeps the window open. Before, the window closed anyway. Autosave errors go to the status bar instead of a modal dialog every 15 s.
  • Cancel at the save prompt orphaned the window when opening another project or starting a new one.
  • "Reset to Default" didn't reset hotkeys. A shallow copy shared the defaults' hotkeys dict, so rebinding a key rewrote the default. A settings file with only some hotkeys also lost the defaults for the rest.
  • Character names with commas became two characters in the exported CSV and in CIGA. Such names are now rejected.
  • A copied project folder could restore into the original project. Its hidden autosave still names the original, so an autosave is now offered only when it belongs to the project being opened (found by Copilot review).

Cleanup

  • Deleted extract.py, a stale one-off script that would overwrite annotator_window.py if run, and the unused ManageCharactersDialog.
  • Stopped tracking settings.json and characters.txt, which the app rewrites as you use it. characters.example.txt documents the format.
  • Logos are bundled in assets/, so the app and build work outside a CIGA checkout.
  • README: the clone step is cd ciga-annotator, and the launch command is python run.py (python src/main.py fails with No module named 'src').
  • pyinstaller moves to requirements-dev.txt, together with pytest.

Tests and CI

  • The core modules have unit tests.
  • An offscreen integration test drives the real window through autosave, save, discard, cancel, failed Save As and restore.
  • CI runs the tests on Windows and Linux.

Test plan

  • python -m pytest: 50 passed
  • Each commit passes the suite on its own
  • Reintroducing the old autosave target fails 4 window tests
  • CI green on this PR
  • Manual: annotate a few lines, wait 15 s, kill the process, relaunch, and restore
  • Manual: python build.py and launch the exe

🤖 Generated with Claude Code

angushushu and others added 4 commits September 27, 2026 15:49
The cue regex ended each cue at the first newline followed by a digit, so a
text line starting with a number ("20 dollars.") was cut off with the rest
of its cue, and timestamps with '.' milliseconds dropped the whole cue.
Cues are now split on blank lines; the index line is optional, cue settings
after the timing are ignored, and the BOM is stripped. Output is identical
on well-formed files. Removes a debug print.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
AppSettings shallow-copied DEFAULT_SETTINGS, sharing the nested hotkeys
dict, so rebinding a key also rewrote the default and Reset restored the
user's own binding. A settings file with only some hotkeys also replaced
the whole dict, dropping the defaults for the rest. Removes an unused Qt
import so the core modules no longer need Qt.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With a project open, autosave overwrote the .vat every 15 seconds, so
answering No to "Save before exit?" did not discard anything. The
autosave file was also never removed, so every launch offered to restore
it, and restoring made the hidden file the working project.

Autosave now writes .<name>.autosave.vat next to the project (or the
subtitles before the first save), recording which project it belongs to.
Save and discard remove it; a restore is offered only when it is newer
than the project and continues on the original project. Save reports
failure, and a failed or cancelled save keeps the window open. Autosave
failures show in the status bar instead of a modal dialog every 15 s.

Also in the window:
- reject commas in character names; commas separate characters in the
  exported CSV and in CIGA's input, so such a name became two characters
- cancelling the save prompt no longer orphans the window when opening
  another project or starting a new one

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- delete extract.py, a one-off migration script that would overwrite
  annotator_window.py if run, and the unused ManageCharactersDialog
- stop tracking settings.json and characters.txt, which the app rewrites
  as it is used; characters.example.txt documents the format
- bundle the logos in assets/ so the app and build work outside a CIGA
  checkout
- README: the clone step is 'cd ciga-annotator' and the launch command is
  'python run.py' ('python src/main.py' fails with 'No module named src')
- move pyinstaller to requirements-dev.txt with pytest
- CI runs the tests on Windows and Linux

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 27, 2026 19:50

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Autosave metadata must be validated, and loaded character names must reject commas before export.

Review effort: Lite
Findings: 1 High severity

Open (1)
What changed in this PR

This PR fixes subtitle parsing, autosave/recovery, settings reset, character validation, packaging, and documentation while adding tests and CI.

Changes:

  • Improved SRT parsing and atomic autosave recovery.
  • Corrected save/cancel flows and settings handling.
  • Added validation, integration tests, cross-platform CI, and cleanup.
File Summary
tests/​test_project.py Project and autosave tests
tests/​test_parsers.py SRT parsing tests
tests/​test_characters.py Character validation tests
tests/​test_app_settings.py Settings persistence tests
tests/​test_annotator_window.py UI save and recovery tests
src/​ui/​char_dialog.py Removed obsolete dialog
src/​ui/​annotator_window.py Updated save, recovery, and validation behavior
src/​main.py Fixed resources and window transitions
src/​core/​project.py Added atomic project and autosave utilities
src/​core/​parsers.py Improved SRT parsing
src/​core/​characters.py Added character-name validation
src/​core/​app_settings.py Fixed settings isolation and hotkey merging
settings.json Removed tracked user state
requirements.txt Runtime dependencies
requirements-dev.txt Development dependencies
README.md Updated usage instructions
pytest.ini Configured pytest
extract.py Removed stale script
characters.example.txt Documented character format
build.py Updated bundled asset paths
.gitignore Ignored generated state
.github/​workflows/​tests.yml Added Windows/Linux CI

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/ui/annotator_window.py
Copying or moving a project folder carries its hidden autosave along, and
that file still names the original project. Restoring it in the copy set
the working project to the original's path, so Save could overwrite a
different project. An autosave is now trusted only when its recorded owner's
autosave location is that very file; otherwise, or if it is malformed, no
restore is offered, and opening it directly continues as an unsaved session.

Reported by Copilot review on #1.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@angushushu
angushushu merged commit 19c1eb1 into main Sep 27, 2026
4 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