From 1a4ea59e39c721126b3294152b7b48c236836a22 Mon Sep 17 00:00:00 2001 From: erklu Date: Mon, 19 Oct 2026 21:50:57 +0000 Subject: [PATCH 1/2] Validate: a marked caption never refuses to open its card Hiding kinds of issue in the panel left captions marked in the text with no card to open: the caret landed in the block and nothing happened. When the caret comes to rest in a caption whose issues are all of hidden kinds, the kind of its first issue is now shown again and that issue opens, with a note on the card and a spoken message. A caption with an issue of a shown kind opens on that and keeps its other kinds hidden. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01LCjpD3ehigLiUGH1oVQQAW --- AGENTS.md | 2 +- tests/test_validation.py | 169 +++++++++++++++++++++++++++++++++++--- utils/validation_panel.py | 73 +++++++++++++++- 3 files changed, 230 insertions(+), 14 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 3e22e04..98bc821 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -35,7 +35,7 @@ A caption is a paragraph on a page, not a filled cell: giving each one a fill of The caption the caret is in marks itself too (`caretId` in the component, `.transcript-cell-editing`), read from the selection in `updateCaretBlock()` on keyup, click, input and `focusBlock()` rather than tracked through edits — the caret moves for reasons nothing in the component hears about. It is deliberately separate from `activeId`, the caption being *played*: the reader is very often editing one caption while listening to another, and `.transcript-cell-editing` is stated after `.transcript-cell-active` so the caret wins the background when both are true. `@blur` now runs `onBlur()`, which flushes exactly as it did and clears the marking with it. - `srt.py` — `SRTEditor` core: state, caption CRUD, undo/redo wiring (delegates to `undo_redo.py`), `parse_srt`/`parse_txt`. -- `srt_render.py` — validation and the keyboard-shortcuts dialog. **Validate opens the docked validation panel under the video, below the settings ("Uncertain words" last)** (`utils/validation_panel.py`, SUNET/scribe-ui#138), not the dialog: one issue at a time from `validation_items()` (issues, not captions — a caption with two problems comes up twice), each with a heading naming the rule, a detail and "Caption N" as a button that puts the caret where the problem is. Previous/Next are relabelled, never rebuilt, so focus stays put; at either end focus hops to the other button before the one that went too far is disabled. Each step calls `TranscriptEditor.review()` (the component's `reviewId`, `.transcript-cell-reviewing`), not `select_caption()`, which toggles. Closing keeps the captions' invalid markings and returns focus to Validate. Checkboxes below the panel (one per kind found, "Overlaps (2)", only when there is more than one kind, or a kind found is hidden -- `filters_wanted()`; hiding a kind and fixing the rest otherwise left it hidden with no checkbox to bring it back) hide or show kinds of issue: every item carries a `rule` (`RULE_LABELS` in `validation_panel.py`), hidden kinds are kept across Validate runs, and the issue on screen stays put if its kind is still shown. `set_shown()` only acts when a checkbox disagrees with what is shown, so it can never toggle twice. "Check again" under the issue (`check_again()`, via `SRTEditor.revalidate_items()` / `run_validation()`, shared with Validate) re-checks every caption -- a timing fix changes its neighbours' overlaps too -- then stays on that caption's first remaining issue, or moves to the next issue after it, and says which ("Caption 17 passes now."). The dialog (`show_validation_report`) remains only for a page without the panel. Every issue is said in words ("Error:"/"Warning:" plus the rule), the icon and colour only repeat it. No longer draws anything itself — see above. `collect_validation_issues()` is the check itself, separate from the dialog that reports it (`show_validation_report`) and tested on its own in `tests/test_validation.py`: one entry per caption carrying everything found in it, in caption order, split into **errors** (no text, ends before it starts, same timing as an earlier caption, overlaps a neighbour) and **warnings** — the subtitle guidelines, so subtitles only: line length against `CHARACTER_LIMIT`, line count against `MAX_SUBTITLE_LINES`, time on screen against `MIN_CAPTION_SECONDS`, and speech no caption covers for longer than `MAX_UNCAPTIONED_SPEECH_SECONDS` (`uncaptioned_speech()`, from `speech_runs()` cut at every caption, so a caption deleted from the middle of one long run still shows; filed under the caption the speech follows with `mark=False`, since that caption is not itself wrong, and carrying `speech` {start, end, until} for the panel's "Add caption", which goes through `TranscriptEditor.create_caption()` like a drag on the strip, sized by `new_caption_span()`). It sets `is_valid` (and `has_error`) as it goes, so the editor's own left rule and the report can never disagree — the caption's highest severity decides: red if any of its issues is an error, amber (`.transcript-cell-warning`, the panel's `--color-warning-border`) for warnings only — the review outline too, whichever of its issues the panel is on. Every caption in the report is a row that jumps to it (`select_caption`), since finding "#47" by scrolling was the one thing the report left the reader to do. +- `srt_render.py` — validation and the keyboard-shortcuts dialog. **Validate opens the docked validation panel under the video, below the settings ("Uncertain words" last)** (`utils/validation_panel.py`, SUNET/scribe-ui#138), not the dialog: one issue at a time from `validation_items()` (issues, not captions — a caption with two problems comes up twice), each with a heading naming the rule, a detail and "Caption N" as a button that puts the caret where the problem is. Previous/Next are relabelled, never rebuilt, so focus stays put; at either end focus hops to the other button before the one that went too far is disabled. Each step calls `TranscriptEditor.review()` (the component's `reviewId`, `.transcript-cell-reviewing`), not `select_caption()`, which toggles. Closing keeps the captions' invalid markings and returns focus to Validate. Checkboxes below the panel (one per kind found, "Overlaps (2)", only when there is more than one kind, or a kind found is hidden -- `filters_wanted()`; hiding a kind and fixing the rest otherwise left it hidden with no checkbox to bring it back) hide or show kinds of issue: every item carries a `rule` (`RULE_LABELS` in `validation_panel.py`), hidden kinds are kept across Validate runs, and the issue on screen stays put if its kind is still shown. **A marked caption never refuses to open its card because of a filter** (`reveal_hidden()`, from `follow_caret()`): the marks in the text are for every issue whatever is filtered, so when the caret comes to rest in a caption whose issues are all of hidden kinds, the kind of its first issue is shown again (its checkbox comes back ticked, `draw_filters()`) and that issue opens, with a line on the card and the same said to a screen reader ("Showing \u201cOverlaps\u201d again \u2014 it was hidden." -- through `pending_speech` and `speak()` when the panel had only just opened, since a live region that appears with its text is often not read). A caption that has an issue of a kind that is shown opens on that and its other kinds stay hidden, which is what the reader chose; speech without a caption marks nothing and never brings a kind back. This is what lets a reader hide everything but one kind, fix those, close the panel and still find every other flagged block later. `set_shown()` only acts when a checkbox disagrees with what is shown, so it can never toggle twice. "Check again" under the issue (`check_again()`, via `SRTEditor.revalidate_items()` / `run_validation()`, shared with Validate) re-checks every caption -- a timing fix changes its neighbours' overlaps too -- then stays on that caption's first remaining issue, or moves to the next issue after it, and says which ("Caption 17 passes now."). The dialog (`show_validation_report`) remains only for a page without the panel. Every issue is said in words ("Error:"/"Warning:" plus the rule), the icon and colour only repeat it. No longer draws anything itself — see above. `collect_validation_issues()` is the check itself, separate from the dialog that reports it (`show_validation_report`) and tested on its own in `tests/test_validation.py`: one entry per caption carrying everything found in it, in caption order, split into **errors** (no text, ends before it starts, same timing as an earlier caption, overlaps a neighbour) and **warnings** — the subtitle guidelines, so subtitles only: line length against `CHARACTER_LIMIT`, line count against `MAX_SUBTITLE_LINES`, time on screen against `MIN_CAPTION_SECONDS`, and speech no caption covers for longer than `MAX_UNCAPTIONED_SPEECH_SECONDS` (`uncaptioned_speech()`, from `speech_runs()` cut at every caption, so a caption deleted from the middle of one long run still shows; filed under the caption the speech follows with `mark=False`, since that caption is not itself wrong, and carrying `speech` {start, end, until} for the panel's "Add caption", which goes through `TranscriptEditor.create_caption()` like a drag on the strip, sized by `new_caption_span()`). It sets `is_valid` (and `has_error`) as it goes, so the editor's own left rule and the report can never disagree — the caption's highest severity decides: red if any of its issues is an error, amber (`.transcript-cell-warning`, the panel's `--color-warning-border`) for warnings only — the review outline too, whichever of its issues the panel is on. Every caption in the report is a row that jumps to it (`select_caption`), since finding "#47" by scrolling was the one thing the report left the reader to do. - `srt_review.py` — word-level review/edit marking shared by both formats (`retag_edits`, `review_runs`, confidence flagging). - `srt_search.py`, `srt_export.py` — search panel and export dialog, shared by both formats. - `transcript_editor.py` / `transcript_editor.js` — the editor itself, described above. diff --git a/tests/test_validation.py b/tests/test_validation.py index 01d3143..870719e 100644 --- a/tests/test_validation.py +++ b/tests/test_validation.py @@ -692,18 +692,6 @@ def test_nothing_happens_before_validate_has_run(self, monkeypatch): assert panel.container.visible is False assert panel.draws == [] - def test_a_hidden_kind_is_not_followed(self, monkeypatch): - from utils.validation_panel import shown_items - - panel = self.panel(monkeypatch, visible=False) - panel.hidden = {"overlap", "length"} - panel.items = shown_items(panel.all_items, panel.hidden) - - panel.follow_caret(3) - - assert panel.container.visible is False - assert panel.draws == [] - def test_an_issue_whose_caption_is_gone_is_not_followed(self, monkeypatch): panel = self.panel(monkeypatch) stale = panel.items[2]["caption"] @@ -762,6 +750,163 @@ def speech_items(subject: SRTEditor) -> list: ] +class TestAMarkedCaptionNeverRefusesToOpen: + """ + A caption marked in the text always opens its card, whatever the filters + say. The caret landing in one whose issues are all of hidden kinds shows + the kind of its first issue again; a caption with an issue of a shown kind + opens on that, and its hidden kinds stay hidden. + """ + + def panel(self, monkeypatch, visible=True): + panel = TestFollowingTheCaret().panel(monkeypatch, visible=visible) + panel.drawn_filters = 0 + panel.draw_filters = lambda: setattr( + panel, "drawn_filters", panel.drawn_filters + 1 + ) + + class Label: + def __init__(self): + self.text = "" + self.shown = False + + def set_text(self, text): + self.text = text + + def set_visibility(self, shown): + self.shown = shown + + panel.note = Label() + panel.spoken_position = Label() + panel.pending_speech = "" + + return panel + + def hide(self, panel, *rules): + from utils.validation_panel import shown_items + + panel.hidden = set(rules) + panel.items = shown_items(panel.all_items, panel.hidden) + panel.position = 0 + + def test_a_caption_with_only_hidden_issues_shows_its_kind_again( + self, monkeypatch + ): + panel = self.panel(monkeypatch) + self.hide(panel, "overlap", "length") + + # Caption 3 is marked for an overlap, which is hidden. + panel.follow_caret(3) + + assert panel.hidden == {"length"} + assert panel.current()["caption"].index == 3 + assert panel.current()["rule"] == "overlap" + assert panel.drawn_filters == 1 + assert panel.draws == [{"scroll": False}] + + def test_only_the_kind_of_its_first_issue_comes_back(self, monkeypatch): + panel = self.panel(monkeypatch) + self.hide(panel, "overlap", "length") + + # Caption 2 has both an overlap and a long line, both hidden. + panel.follow_caret(2) + + assert len(panel.hidden) == 1 + assert panel.current()["caption"].index == 2 + + def test_a_caption_with_a_shown_kind_opens_on_that_and_keeps_the_rest_hidden( + self, monkeypatch + ): + panel = self.panel(monkeypatch) + self.hide(panel, "overlap") + + # Caption 2: overlap (hidden) and length (shown). + panel.follow_caret(2) + + assert panel.hidden == {"overlap"} + assert panel.current()["rule"] == "length" + assert panel.drawn_filters == 0 + + def test_it_is_said_on_the_card_and_to_a_screen_reader(self, monkeypatch): + panel = self.panel(monkeypatch) + self.hide(panel, "overlap", "length") + + panel.follow_caret(3) + + said = "Showing \u201cOverlaps\u201d again \u2014 it was hidden." + + assert panel.note.text == said and panel.note.shown + assert panel.spoken_position.text.startswith(said) + assert "Issue" in panel.spoken_position.text + + def test_nothing_is_said_when_nothing_came_back(self, monkeypatch): + panel = self.panel(monkeypatch) + + panel.follow_caret(3) + + assert panel.note.text == "" + assert panel.spoken_position.text == "" + + def test_a_closed_panel_opens_and_says_it_a_moment_later(self, monkeypatch): + panel = self.panel(monkeypatch, visible=False) + self.hide(panel, "overlap", "length") + + panel.follow_caret(3) + + assert panel.container.visible is True + assert panel.note.shown + assert panel.spoken_position.text == "" + assert panel.timers + + panel.speak() + + assert panel.spoken_position.text.startswith("Showing") + assert panel.pending_speech == "" + + panel.speak() + + assert panel.spoken_position.text.startswith("Issue") + + def test_a_caption_with_no_issue_still_changes_nothing(self, monkeypatch): + panel = self.panel(monkeypatch, visible=False) + self.hide(panel, "overlap", "length") + + panel.follow_caret(1) + + assert panel.container.visible is False + assert panel.hidden == {"overlap", "length"} + assert panel.draws == [] + + def test_a_gone_caption_does_not_bring_a_kind_back(self, monkeypatch): + panel = self.panel(monkeypatch) + self.hide(panel, "overlap", "length") + panel.captions = lambda: [] + + panel.follow_caret(3) + + assert panel.hidden == {"overlap", "length"} + assert panel.draws == [] + + def test_speech_without_a_caption_never_brings_its_kind_back(self, monkeypatch): + panel = self.panel(monkeypatch) + panel.all_items = [ + { + "caption": panel.all_items[0]["caption"], + "error": False, + "rule": "speech", + "speech": {"start": 0.0, "end": 1.0, "until": None}, + } + ] + panel.items = [] + panel.hidden = {"speech"} + index = panel.all_items[0]["caption"].index + + panel.follow_caret(index) + + assert panel.hidden == {"speech"} + assert panel.draws == [] + + class TestSpeechOutsideCaptions: """ SUNET/scribe-ui#142: speech that no caption covers, for longer than diff --git a/utils/validation_panel.py b/utils/validation_panel.py index 9373c69..18c64f3 100644 --- a/utils/validation_panel.py +++ b/utils/validation_panel.py @@ -166,6 +166,9 @@ def __init__( self.all_items: list = [] self.items: list = [] self.hidden: set[str] = set() + # Said by the next speak(): a kind of issue that came back as the + # panel opened (reveal_hidden). + self.pending_speech = "" self.checkboxes: dict = {} self.position = 0 self.checked = 0 @@ -566,6 +569,57 @@ def go_to_caption(self) -> None: self.transcript.editor.seek_video(start) self.transcript.moved_to(start) + def reveal_hidden(self, caption_index: int) -> Optional[dict]: + """ + A marked caption must never refuse to open its card because of a + filter. When the caret comes to rest in one whose issues are all of + kinds that are hidden, the kind of its first issue is shown again, and + that issue is the one to show. (A caption with an issue of a kind + that is shown opens on that, and its hidden kinds stay hidden: that + is what the reader chose.) Speech without a caption marks no caption, + so it never brings a kind back. + """ + + item = next( + ( + i + for i in self.all_items + if i["caption"].index == caption_index + and "speech" not in i + and i["rule"] in self.hidden + and self.exists(i) + ), + None, + ) + + if item is None: + return None + + self.hidden.discard(item["rule"]) + self.items = shown_items(self.all_items, self.hidden) + self.draw_filters() + + return item + + def say_revealed(self, item: dict, spoken: bool = True) -> None: + """ + Say that a kind of issue came back because the caret went to a + caption that has one: on the card, and to a screen reader. + """ + + text = f"Showing \u201c{RULE_LABELS[item['rule']]}\u201d again \u2014 it was hidden." + + self.note.set_text(text) + self.note.set_visibility(True) + + if spoken: + self.spoken_position.set_text( + f"{text} Issue {position_text(self.position, len(self.items))}." + ) + else: + # The panel has only just opened; speak() says it a moment later. + self.pending_speech = text + def follow_caret(self, caption_index: int) -> None: """ The caret has come to rest in a caption: show that caption's issue. @@ -575,7 +629,8 @@ def follow_caret(self, caption_index: int) -> None: what keeps "Caption 17", which puts the caret there itself, from redrawing what it was just pressed on. Closed panel, after a Validate: open it on that issue. A caption with no issue, or a - Validate not yet run, changes nothing. + Validate not yet run, changes nothing. A caption marked only for + kinds that are hidden shows one of them again (reveal_hidden). """ current = self.current() @@ -596,6 +651,11 @@ def follow_caret(self, caption_index: int) -> None: None, ) + revealed = None + + if target is None: + revealed = target = self.reveal_hidden(caption_index) + if target is None: return @@ -608,6 +668,10 @@ def follow_caret(self, caption_index: int) -> None: if was_open: self.draw(scroll=False) + + if revealed is not None: + self.say_revealed(revealed) + return # A live region that appears with its text already in it is often not @@ -615,6 +679,10 @@ def follow_caret(self, caption_index: int) -> None: # first and say the issue a moment later. self.container.set_visibility(True) self.draw(scroll=False, speak=False) + + if revealed is not None: + self.say_revealed(revealed, spoken=False) + with self.container: ui.timer(0.2, self.speak, once=True) @@ -627,7 +695,10 @@ def speak(self) -> None: item = self.current() if item is not None: + said = getattr(self, "pending_speech", "") + self.pending_speech = "" self.spoken_position.set_text( + f"{said + ' ' if said else ''}" f"Issue {position_text(self.position, len(self.items))}." ) From 79250132b0eb1e14efedbb1e43037944a8a2d990 Mon Sep 17 00:00:00 2001 From: erklu Date: Sun, 1 Nov 2026 22:46:56 +0000 Subject: [PATCH 2/2] Validate: say a revealed kind once; speech under a caption is not its own issue The sentence 'Showing X again' was set on the card and in the position's label, both inside the one aria-atomic live region, so it was read twice (and a third time through the deferred path). It is on the card only; the position set by draw() or speak() makes the region read it. follow_caret() took a shown speech item filed under the caption as if it were the caption's own issue, so a caption marked for a hidden kind opened on the speech after it and the kind stayed hidden. The caption's own issues come first (shown, then a hidden one to bring back); the speech under it is the last resort. A speech card the panel is open on stays. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01LCjpD3ehigLiUGH1oVQQAW --- AGENTS.md | 2 +- tests/test_validation.py | 109 +++++++++++++++++++++++++++++++++++--- utils/validation_panel.py | 51 +++++++++++------- 3 files changed, 133 insertions(+), 29 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 98bc821..9021524 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -35,7 +35,7 @@ A caption is a paragraph on a page, not a filled cell: giving each one a fill of The caption the caret is in marks itself too (`caretId` in the component, `.transcript-cell-editing`), read from the selection in `updateCaretBlock()` on keyup, click, input and `focusBlock()` rather than tracked through edits — the caret moves for reasons nothing in the component hears about. It is deliberately separate from `activeId`, the caption being *played*: the reader is very often editing one caption while listening to another, and `.transcript-cell-editing` is stated after `.transcript-cell-active` so the caret wins the background when both are true. `@blur` now runs `onBlur()`, which flushes exactly as it did and clears the marking with it. - `srt.py` — `SRTEditor` core: state, caption CRUD, undo/redo wiring (delegates to `undo_redo.py`), `parse_srt`/`parse_txt`. -- `srt_render.py` — validation and the keyboard-shortcuts dialog. **Validate opens the docked validation panel under the video, below the settings ("Uncertain words" last)** (`utils/validation_panel.py`, SUNET/scribe-ui#138), not the dialog: one issue at a time from `validation_items()` (issues, not captions — a caption with two problems comes up twice), each with a heading naming the rule, a detail and "Caption N" as a button that puts the caret where the problem is. Previous/Next are relabelled, never rebuilt, so focus stays put; at either end focus hops to the other button before the one that went too far is disabled. Each step calls `TranscriptEditor.review()` (the component's `reviewId`, `.transcript-cell-reviewing`), not `select_caption()`, which toggles. Closing keeps the captions' invalid markings and returns focus to Validate. Checkboxes below the panel (one per kind found, "Overlaps (2)", only when there is more than one kind, or a kind found is hidden -- `filters_wanted()`; hiding a kind and fixing the rest otherwise left it hidden with no checkbox to bring it back) hide or show kinds of issue: every item carries a `rule` (`RULE_LABELS` in `validation_panel.py`), hidden kinds are kept across Validate runs, and the issue on screen stays put if its kind is still shown. **A marked caption never refuses to open its card because of a filter** (`reveal_hidden()`, from `follow_caret()`): the marks in the text are for every issue whatever is filtered, so when the caret comes to rest in a caption whose issues are all of hidden kinds, the kind of its first issue is shown again (its checkbox comes back ticked, `draw_filters()`) and that issue opens, with a line on the card and the same said to a screen reader ("Showing \u201cOverlaps\u201d again \u2014 it was hidden." -- through `pending_speech` and `speak()` when the panel had only just opened, since a live region that appears with its text is often not read). A caption that has an issue of a kind that is shown opens on that and its other kinds stay hidden, which is what the reader chose; speech without a caption marks nothing and never brings a kind back. This is what lets a reader hide everything but one kind, fix those, close the panel and still find every other flagged block later. `set_shown()` only acts when a checkbox disagrees with what is shown, so it can never toggle twice. "Check again" under the issue (`check_again()`, via `SRTEditor.revalidate_items()` / `run_validation()`, shared with Validate) re-checks every caption -- a timing fix changes its neighbours' overlaps too -- then stays on that caption's first remaining issue, or moves to the next issue after it, and says which ("Caption 17 passes now."). The dialog (`show_validation_report`) remains only for a page without the panel. Every issue is said in words ("Error:"/"Warning:" plus the rule), the icon and colour only repeat it. No longer draws anything itself — see above. `collect_validation_issues()` is the check itself, separate from the dialog that reports it (`show_validation_report`) and tested on its own in `tests/test_validation.py`: one entry per caption carrying everything found in it, in caption order, split into **errors** (no text, ends before it starts, same timing as an earlier caption, overlaps a neighbour) and **warnings** — the subtitle guidelines, so subtitles only: line length against `CHARACTER_LIMIT`, line count against `MAX_SUBTITLE_LINES`, time on screen against `MIN_CAPTION_SECONDS`, and speech no caption covers for longer than `MAX_UNCAPTIONED_SPEECH_SECONDS` (`uncaptioned_speech()`, from `speech_runs()` cut at every caption, so a caption deleted from the middle of one long run still shows; filed under the caption the speech follows with `mark=False`, since that caption is not itself wrong, and carrying `speech` {start, end, until} for the panel's "Add caption", which goes through `TranscriptEditor.create_caption()` like a drag on the strip, sized by `new_caption_span()`). It sets `is_valid` (and `has_error`) as it goes, so the editor's own left rule and the report can never disagree — the caption's highest severity decides: red if any of its issues is an error, amber (`.transcript-cell-warning`, the panel's `--color-warning-border`) for warnings only — the review outline too, whichever of its issues the panel is on. Every caption in the report is a row that jumps to it (`select_caption`), since finding "#47" by scrolling was the one thing the report left the reader to do. +- `srt_render.py` — validation and the keyboard-shortcuts dialog. **Validate opens the docked validation panel under the video, below the settings ("Uncertain words" last)** (`utils/validation_panel.py`, SUNET/scribe-ui#138), not the dialog: one issue at a time from `validation_items()` (issues, not captions — a caption with two problems comes up twice), each with a heading naming the rule, a detail and "Caption N" as a button that puts the caret where the problem is. Previous/Next are relabelled, never rebuilt, so focus stays put; at either end focus hops to the other button before the one that went too far is disabled. Each step calls `TranscriptEditor.review()` (the component's `reviewId`, `.transcript-cell-reviewing`), not `select_caption()`, which toggles. Closing keeps the captions' invalid markings and returns focus to Validate. Checkboxes below the panel (one per kind found, "Overlaps (2)", only when there is more than one kind, or a kind found is hidden -- `filters_wanted()`; hiding a kind and fixing the rest otherwise left it hidden with no checkbox to bring it back) hide or show kinds of issue: every item carries a `rule` (`RULE_LABELS` in `validation_panel.py`), hidden kinds are kept across Validate runs, and the issue on screen stays put if its kind is still shown. **A marked caption never refuses to open its card because of a filter** (`reveal_hidden()`, from `follow_caret()`): the marks in the text are for every issue whatever is filtered, so when the caret comes to rest in a caption whose issues are all of hidden kinds, the kind of its first issue is shown again (its checkbox comes back ticked, `draw_filters()`) and that issue opens, with a line on the card ("Showing \u201cOverlaps\u201d again \u2014 it was hidden."). The line is on the card only: the card is one `aria-atomic` live region, read whole when the position in it is set (by `draw()`, or by `speak()` a moment later when the panel had only just opened, since a live region that appears with its text is often not read), so the sentence is heard with it -- said in the position's own label as well it was heard twice. A caption that has an issue of a kind that is shown opens on that and its other kinds stay hidden, which is what the reader chose; speech without a caption marks nothing and never brings a kind back. Speech filed under a caption (`mark=False`) is not that caption's own issue: `follow_caret()` looks for the caption's own issues first (shown, then hidden ones to bring back) and only then for the speech under it, so a caption marked for a hidden kind is not opened on the speech that follows it -- except that a speech card the reader is already on stays. This is what lets a reader hide everything but one kind, fix those, close the panel and still find every other flagged block later. `set_shown()` only acts when a checkbox disagrees with what is shown, so it can never toggle twice. "Check again" under the issue (`check_again()`, via `SRTEditor.revalidate_items()` / `run_validation()`, shared with Validate) re-checks every caption -- a timing fix changes its neighbours' overlaps too -- then stays on that caption's first remaining issue, or moves to the next issue after it, and says which ("Caption 17 passes now."). The dialog (`show_validation_report`) remains only for a page without the panel. Every issue is said in words ("Error:"/"Warning:" plus the rule), the icon and colour only repeat it. No longer draws anything itself — see above. `collect_validation_issues()` is the check itself, separate from the dialog that reports it (`show_validation_report`) and tested on its own in `tests/test_validation.py`: one entry per caption carrying everything found in it, in caption order, split into **errors** (no text, ends before it starts, same timing as an earlier caption, overlaps a neighbour) and **warnings** — the subtitle guidelines, so subtitles only: line length against `CHARACTER_LIMIT`, line count against `MAX_SUBTITLE_LINES`, time on screen against `MIN_CAPTION_SECONDS`, and speech no caption covers for longer than `MAX_UNCAPTIONED_SPEECH_SECONDS` (`uncaptioned_speech()`, from `speech_runs()` cut at every caption, so a caption deleted from the middle of one long run still shows; filed under the caption the speech follows with `mark=False`, since that caption is not itself wrong, and carrying `speech` {start, end, until} for the panel's "Add caption", which goes through `TranscriptEditor.create_caption()` like a drag on the strip, sized by `new_caption_span()`). It sets `is_valid` (and `has_error`) as it goes, so the editor's own left rule and the report can never disagree — the caption's highest severity decides: red if any of its issues is an error, amber (`.transcript-cell-warning`, the panel's `--color-warning-border`) for warnings only — the review outline too, whichever of its issues the panel is on. Every caption in the report is a row that jumps to it (`select_caption`), since finding "#47" by scrolling was the one thing the report left the reader to do. - `srt_review.py` — word-level review/edit marking shared by both formats (`retag_edits`, `review_runs`, confidence flagging). - `srt_search.py`, `srt_export.py` — search panel and export dialog, shared by both formats. - `transcript_editor.py` / `transcript_editor.js` — the editor itself, described above. diff --git a/tests/test_validation.py b/tests/test_validation.py index 870719e..e8c4c62 100644 --- a/tests/test_validation.py +++ b/tests/test_validation.py @@ -778,7 +778,19 @@ def set_visibility(self, shown): panel.note = Label() panel.spoken_position = Label() - panel.pending_speech = "" + + def draw(scroll=True, speak=True): + # What the real draw() does to the live region: clears the note, + # sets the position unless a later speak() is to. + panel.draws.append( + {"scroll": scroll} if speak else {"scroll": scroll, "speak": speak} + ) + panel.note.set_visibility(False) + panel.spoken_position.set_text( + f"Issue {panel.position + 1} of {len(panel.items)}." if speak else "" + ) + + panel.draw = draw return panel @@ -836,8 +848,10 @@ def test_it_is_said_on_the_card_and_to_a_screen_reader(self, monkeypatch): said = "Showing \u201cOverlaps\u201d again \u2014 it was hidden." assert panel.note.text == said and panel.note.shown - assert panel.spoken_position.text.startswith(said) - assert "Issue" in panel.spoken_position.text + # The card is one live region read whole: the position beside the + # sentence, not the sentence twice. + assert panel.spoken_position.text.startswith("Issue") + assert "Showing" not in panel.spoken_position.text def test_nothing_is_said_when_nothing_came_back(self, monkeypatch): panel = self.panel(monkeypatch) @@ -845,7 +859,7 @@ def test_nothing_is_said_when_nothing_came_back(self, monkeypatch): panel.follow_caret(3) assert panel.note.text == "" - assert panel.spoken_position.text == "" + assert "Showing" not in panel.spoken_position.text def test_a_closed_panel_opens_and_says_it_a_moment_later(self, monkeypatch): panel = self.panel(monkeypatch, visible=False) @@ -860,12 +874,91 @@ def test_a_closed_panel_opens_and_says_it_a_moment_later(self, monkeypatch): panel.speak() - assert panel.spoken_position.text.startswith("Showing") - assert panel.pending_speech == "" + assert panel.spoken_position.text.startswith("Issue") + assert "Showing" not in panel.spoken_position.text + assert panel.note.shown - panel.speak() + def with_speech_under(self, panel, index): + """ + A shown speech item filed under the caption `index`, as the check + files speech under the caption it follows. + """ - assert panel.spoken_position.text.startswith("Issue") + from utils.validation_panel import shown_items + + caption = next(c for c in panel.captions() if c.index == index) + speech = { + "caption": caption, + "error": False, + "rule": "speech", + "title": "Speech without a caption", + "detail": "", + "speech": {"start": 0.0, "end": 1.0, "until": None}, + } + panel.all_items = panel.all_items + [speech] + panel.items = shown_items(panel.all_items, panel.hidden) + + return speech + + def test_speech_under_a_marked_caption_does_not_stand_in_for_its_issue( + self, monkeypatch + ): + panel = self.panel(monkeypatch, visible=False) + self.hide(panel, "overlap", "length") + speech = self.with_speech_under(panel, 3) + + # Caption 3 is marked for an overlap (hidden); the speech after it is + # shown. The card is for the caption's own issue. + panel.follow_caret(3) + + assert panel.hidden == {"length"} + assert panel.current()["rule"] == "overlap" + assert panel.current() is not speech + assert panel.container.visible is True + assert "Overlaps" in panel.note.text + + def test_speech_under_a_caption_with_a_shown_issue_opens_on_the_issue( + self, monkeypatch + ): + panel = self.panel(monkeypatch) + self.hide(panel, "overlap") + speech = self.with_speech_under(panel, 2) + + # Caption 2: overlap (hidden), length (shown), speech (shown). + panel.follow_caret(2) + + assert panel.hidden == {"overlap"} + assert panel.current()["rule"] == "length" + assert panel.current() is not speech + + def test_a_caption_with_nothing_but_speech_under_it_opens_on_the_speech( + self, monkeypatch + ): + panel = self.panel(monkeypatch) + panel.hidden = set() + panel.items = list(panel.all_items) + # Caption 1 has no issue of its own. + speech = self.with_speech_under(panel, 1) + panel.position = len(panel.items) - 2 + + panel.follow_caret(1) + + assert panel.current() is speech + assert panel.hidden == set() + + def test_a_speech_card_the_reader_is_on_stays(self, monkeypatch): + panel = self.panel(monkeypatch) + self.hide(panel, "overlap", "length") + speech = self.with_speech_under(panel, 3) + panel.position = panel.items.index(speech) + panel.draws.clear() + + # Stepping to the speech puts the caret in the caption it follows. + panel.follow_caret(3) + + assert panel.current() is speech + assert panel.hidden == {"overlap", "length"} + assert panel.draws == [] def test_a_caption_with_no_issue_still_changes_nothing(self, monkeypatch): panel = self.panel(monkeypatch, visible=False) diff --git a/utils/validation_panel.py b/utils/validation_panel.py index 18c64f3..3cadf8f 100644 --- a/utils/validation_panel.py +++ b/utils/validation_panel.py @@ -166,9 +166,6 @@ def __init__( self.all_items: list = [] self.items: list = [] self.hidden: set[str] = set() - # Said by the next speak(): a kind of issue that came back as the - # panel opened (reveal_hidden). - self.pending_speech = "" self.checkboxes: dict = {} self.position = 0 self.checked = 0 @@ -601,10 +598,16 @@ def reveal_hidden(self, caption_index: int) -> Optional[dict]: return item - def say_revealed(self, item: dict, spoken: bool = True) -> None: + def say_revealed(self, item: dict) -> None: """ Say that a kind of issue came back because the caret went to a - caption that has one: on the card, and to a screen reader. + caption that has one. + + On the card only. The card is a live region read whole (aria-atomic), + so the sentence is heard with the position when that is set -- by + draw() on an open panel, by speak() a moment later on one just + opened. Said in the position's own label as well it would be heard + twice. """ text = f"Showing \u201c{RULE_LABELS[item['rule']]}\u201d again \u2014 it was hidden." @@ -612,14 +615,6 @@ def say_revealed(self, item: dict, spoken: bool = True) -> None: self.note.set_text(text) self.note.set_visibility(True) - if spoken: - self.spoken_position.set_text( - f"{text} Issue {position_text(self.position, len(self.items))}." - ) - else: - # The panel has only just opened; speak() says it a moment later. - self.pending_speech = text - def follow_caret(self, caption_index: int) -> None: """ The caret has come to rest in a caption: show that caption's issue. @@ -634,19 +629,29 @@ def follow_caret(self, caption_index: int) -> None: """ current = self.current() + was_open = self.container.visible + # What is on screen stays, so the caret arriving from a button on it + # redraws nothing. A closed panel shows nothing: the position it was + # left on is not the reader's choice, least of all a speech card, + # which is not the caption's own issue (below). if ( current is not None and current["caption"].index == caption_index and self.exists(current) + and (was_open or "speech" not in current) ): target = current else: + # The caption's own issues first: speech filed under a caption is + # not what marks it, so it does not stand in for them. target = next( ( item for item in self.items - if item["caption"].index == caption_index and self.exists(item) + if item["caption"].index == caption_index + and "speech" not in item + and self.exists(item) ), None, ) @@ -657,9 +662,18 @@ def follow_caret(self, caption_index: int) -> None: revealed = target = self.reveal_hidden(caption_index) if target is None: - return + # No issue of its own, shown or hidden: the speech that follows it. + target = next( + ( + item + for item in self.items + if item["caption"].index == caption_index and self.exists(item) + ), + None, + ) - was_open = self.container.visible + if target is None: + return if was_open and target is current: return @@ -681,7 +695,7 @@ def follow_caret(self, caption_index: int) -> None: self.draw(scroll=False, speak=False) if revealed is not None: - self.say_revealed(revealed, spoken=False) + self.say_revealed(revealed) with self.container: ui.timer(0.2, self.speak, once=True) @@ -695,10 +709,7 @@ def speak(self) -> None: item = self.current() if item is not None: - said = getattr(self, "pending_speech", "") - self.pending_speech = "" self.spoken_position.set_text( - f"{said + ' ' if said else ''}" f"Issue {position_text(self.position, len(self.items))}." )