Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
169 changes: 157 additions & 12 deletions tests/test_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down Expand Up @@ -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
Expand Down
73 changes: 72 additions & 1 deletion utils/validation_panel.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Comment on lines +615 to +621

def follow_caret(self, caption_index: int) -> None:
"""
The caret has come to rest in a caption: show that caption's issue.
Expand All @@ -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()
Expand All @@ -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)
Comment on lines +656 to +657

if target is None:
return

Expand All @@ -608,13 +668,21 @@ 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
# read; one that changes after it is on screen is. So open the panel
# 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)

Expand All @@ -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))}."
)

Expand Down
Loading