From c414c8297a7bbefd32006fe2121234eedc50b987 Mon Sep 17 00:00:00 2001 From: Beckett Frey Date: Mon, 24 Aug 2026 12:58:17 -0500 Subject: [PATCH] feat: restrict Train Aligners to datasets with manual alignments Implements the counter-proposal from #166 instead of gating the sidebar step. Train Aligners stays reachable and browsable; the constraint is enforced where the user is already looking, at the point of selection. Both dropdowns now filter: - Dataset dropdown lists only datasets carrying at least one hand or corrected alignment. - Alignment dropdown lists only that dataset's manual alignments. An inline note above the dataset dropdown states the rule and the consequence, and is repeated as the dropdown's tooltip. Inline rather than tooltip-only on purpose: the message explains an absence, and a user whose dataset is missing has no reason to hover a control to find out why -- when the list is empty the dropdown is disabled anyway. The predicate lives in storage/alignments.py as is_manual_alignment, with list_manual_alignments and has_manual_alignments over it. It is defined as "not automatic" rather than an allow-list of {hand, corrected}, because create_corrected_alignment takes an arbitrary alignment_type string -- the Correct Alignments page exposes it as free text defaulting to "corrected", so values like "corrected-v2" reach storage and an allow-list would silently hide real hand-corrected work. The two empty states stay distinct. "No datasets registered" and "No datasets with manual alignments" are different problems: telling a user who already has datasets that none are registered would send them off to register a duplicate. Also folds the dataset-population logic in build_ui and reload_datasets into one method. Those two copies had already drifted -- build_ui showed Name/Date/Description while reload_datasets showed Name/Description/ID, so the columns changed under the user on tab switch. Both now use the Name/Date/Description form the other stackers use. Verified end to end against real storage, not just the mocks: a dataset whose only alignment is automatic is excluded from the dropdown, and one with a hand alignment is listed with that alignment selectable. --- .../gui/pages/pipeline/training_stacker.py | 82 ++++++---- src/voxkit/storage/alignments.py | 39 +++++ .../test_training_stacker_manual_filter.py | 152 ++++++++++++++++++ tests/storage/test_alignments.py | 81 ++++++++++ 4 files changed, 320 insertions(+), 34 deletions(-) create mode 100644 tests/gui/test_training_stacker_manual_filter.py diff --git a/src/voxkit/gui/pages/pipeline/training_stacker.py b/src/voxkit/gui/pages/pipeline/training_stacker.py index 26b3062..33c8903 100644 --- a/src/voxkit/gui/pages/pipeline/training_stacker.py +++ b/src/voxkit/gui/pages/pipeline/training_stacker.py @@ -31,6 +31,15 @@ logger = logging.getLogger(__name__) +# Shown inline above the dataset dropdown and repeated as its tooltip. Inline +# rather than tooltip-only because it explains an *absence* -- a user whose +# dataset is missing from the list has no reason to hover over a control to find +# out why, and when the list is empty the dropdown is disabled anyway. +MANUAL_ALIGNMENT_NOTICE = ( + "Training aligners is only meaningful on datasets with manual alignments. " + "Only datasets with manual alignments will appear in the list below." +) + class TrainingStacker(BaseStacker): """Model training pipeline page. @@ -83,8 +92,9 @@ def on_dataset_selected(self): """Handle dataset selection change and load corresponding alignments""" selected_dataset_id = self.train_dataset_dropdown.current_id() - # Load alignments for the selected dataset - alignments_meta = alignments.list_alignments(selected_dataset_id) + # Manual alignments only: a model trained against machine-generated + # alignments just relearns whatever errors those alignments contain. + alignments_meta = alignments.list_manual_alignments(selected_dataset_id) if alignments_meta: data = [] @@ -110,9 +120,9 @@ def on_dataset_selected(self): else: self.train_alignment_dropdown.set_data( - [{"id": None, "data": ("No alignments registered", "", "")}], + [{"id": None, "data": ("No manual alignments for this dataset", "", "")}], ["Method", "Model", "Date", "Status"], - placeholder="No alignments registered", + placeholder="No manual alignments for this dataset", ) self.train_alignment_dropdown.setEnabled(False) @@ -273,25 +283,41 @@ def reload_models(self): if self.model_panel: self.model_panel.reload_models() - def reload_datasets(self): - """Reload datasets in the dropdown""" - datasets_meta = datasets.list_datasets_metadata() - if datasets_meta: - data = [] - for d in datasets_meta: - data.append({"id": d["id"], "data": (d["name"], d["description"], d["id"])}) + def _populate_dataset_dropdown(self): + """Fill the dataset dropdown with datasets that can actually be trained on. + + Only datasets carrying at least one hand or corrected alignment are + listed -- see ``MANUAL_ALIGNMENT_NOTICE``, which tells the user this is + happening. The two empty states are distinct on purpose: "nothing + registered at all" and "nothing registered that qualifies" call for + different next steps from the user. + """ + all_datasets = datasets.list_datasets_metadata() + trainable = [d for d in all_datasets if alignments.has_manual_alignments(d["id"])] + if trainable: + data = [ + {"id": d["id"], "data": (d["name"], d["registration_date"], d["description"])} + for d in trainable + ] self.train_dataset_dropdown.set_data( - data, ["Name", "Description", "ID"], placeholder="Click to select a dataset" + data, ["Name", "Date", "Description"], placeholder="Click to select a dataset" ) self.train_dataset_dropdown.setEnabled(True) else: + empty_message = ( + "No datasets with manual alignments" if all_datasets else "No datasets registered" + ) self.train_dataset_dropdown.set_data( - [{"id": None, "data": ("No datasets registered", "", "")}], - ["Name", "Description", "ID"], - placeholder="No datasets registered", + [{"id": None, "data": (empty_message, "", "")}], + ["Name", "Date", "Description"], + placeholder=empty_message, ) self.train_dataset_dropdown.setEnabled(False) + def reload_datasets(self): + """Reload datasets in the dropdown""" + self._populate_dataset_dropdown() + self.train_alignment_dropdown.set_data( [{"id": None, "data": ("Select a dataset first", "", "")}], ["Method", "Model", "Date", "Status"], @@ -315,28 +341,16 @@ def build_ui(self): dataset_label.setStyleSheet(Labels.SECTION_LABEL) self.content_layout.addWidget(dataset_label) + manual_alignment_note = QLabel(MANUAL_ALIGNMENT_NOTICE) + manual_alignment_note.setStyleSheet(Labels.INFO) + manual_alignment_note.setWordWrap(True) + self.content_layout.addWidget(manual_alignment_note) + self.train_dataset_dropdown = MultiColumnComboBox() self.train_dataset_dropdown.setStyleSheet(Containers.COMBOBOX_STANDARD) + self.train_dataset_dropdown.setToolTip(MANUAL_ALIGNMENT_NOTICE) - # Populate with registered datasets - datasets_meta = datasets.list_datasets_metadata() - if datasets_meta: - data = [] - for d in datasets_meta: - data.append( - {"id": d["id"], "data": (d["name"], d["registration_date"], d["description"])} - ) - self.train_dataset_dropdown.set_data( - data, ["Name", "Date", "Description"], placeholder="Click to select a dataset" - ) - self.train_dataset_dropdown.setEnabled(True) - else: - self.train_dataset_dropdown.set_data( - [{"id": None, "data": ("No datasets registered", "", "")}], - ["Name", "Date", "Description"], - placeholder="No datasets registered", - ) - self.train_dataset_dropdown.setEnabled(False) + self._populate_dataset_dropdown() # Connect to selection handler self.train_dataset_dropdown.currentIndexChanged.connect(self.on_dataset_selected) diff --git a/src/voxkit/storage/alignments.py b/src/voxkit/storage/alignments.py index 4e835de..86ae9de 100644 --- a/src/voxkit/storage/alignments.py +++ b/src/voxkit/storage/alignments.py @@ -22,8 +22,11 @@ another alignment's boundaries, without ever mutating the source - **get_alignment_metadata**: Retrieve metadata for a specific alignment - **get_alignment_type**: Return an alignment's provenance (automatic/hand/corrected) +- **is_manual_alignment**: True if a human produced or corrected the alignment - **update_alignment**: Update the status or details of an existing alignment - **list_alignments**: List all alignments for a given dataset +- **list_manual_alignments**: List only hand/corrected alignments for a dataset +- **has_manual_alignments**: True if a dataset has at least one manual alignment - **delete_alignment**: Remove an alignment from storage Notes @@ -118,6 +121,21 @@ def get_alignment_type(meta: AlignmentMetadata) -> AlignmentType: return "automatic" +def is_manual_alignment(meta: AlignmentMetadata) -> bool: + """Return True if a human produced or corrected this alignment. + + Defined as "not machine-generated" rather than as a membership test against + {"hand", "corrected"} on purpose. ``create_corrected_alignment`` accepts an + arbitrary ``alignment_type`` string -- the Correct Alignments page exposes it + as a free-text field defaulting to "corrected", so real stored values include + things like "corrected-v2". Those are still hand-corrected work, and an + inclusion test would silently drop them. "automatic" is the one value written + by a machine path, and the one value ``get_alignment_type`` falls back to, so + excluding it is both narrower and more durable. + """ + return get_alignment_type(meta) != "automatic" + + def _get_alignments_root(dataset_id: str) -> Path | None: """Get the root directory for storing alignments for a given dataset. @@ -577,6 +595,27 @@ def list_alignments(dataset_id: str) -> List[AlignmentMetadata]: return alignments_found +def list_manual_alignments(dataset_id: str) -> List[AlignmentMetadata]: + """List only the alignments a human produced or corrected. + + Training against machine-generated alignments teaches a model whatever errors + those alignments already contain, so callers that feed training data want this + rather than ``list_alignments``. + + Args: + dataset_id: Identifier of the dataset to list alignments for + + Returns: + List of AlignmentMetadata dictionaries (empty list if none qualify) + """ + return [a for a in list_alignments(dataset_id) if is_manual_alignment(a)] + + +def has_manual_alignments(dataset_id: str) -> bool: + """Return True if a dataset has at least one hand or corrected alignment.""" + return any(is_manual_alignment(a) for a in list_alignments(dataset_id)) + + def delete_alignment(dataset_id: str, alignment_id: str) -> Tuple[bool, str]: """Delete an alignment given its dataset ID and alignment ID. diff --git a/tests/gui/test_training_stacker_manual_filter.py b/tests/gui/test_training_stacker_manual_filter.py new file mode 100644 index 0000000..79c940c --- /dev/null +++ b/tests/gui/test_training_stacker_manual_filter.py @@ -0,0 +1,152 @@ +"""Tests for manual-alignment filtering in TrainingStacker. + +Training against machine-generated alignments teaches a model whatever errors +those alignments already contain, so both dropdowns on the Train Aligners page +list only hand/corrected data. These guard the filter and, just as importantly, +the empty states -- "nothing registered" and "nothing that qualifies" are +different problems and must not collapse into the same message. +""" + +import pytest + +from voxkit.gui.pages.pipeline import training_stacker +from voxkit.gui.pages.pipeline.training_stacker import ( + MANUAL_ALIGNMENT_NOTICE, + TrainingStacker, +) + + +class _Dropdown: + """Records what the stacker put in a dropdown, without needing a real widget.""" + + def __init__(self, current_id=None): + self.rows = None + self.headers = None + self.placeholder = None + self.enabled = None + self._current_id = current_id + + def set_data(self, rows, headers=None, placeholder=None): + self.rows = rows + self.headers = headers + self.placeholder = placeholder + + def setEnabled(self, value): # noqa: N802 - Qt naming + self.enabled = value + + def current_id(self): + return self._current_id + + def ids(self): + return [row["id"] for row in self.rows] + + +def _dataset(dataset_id, name="Dataset"): + return { + "id": dataset_id, + "name": name, + "registration_date": "2026-01-01", + "description": "", + } + + +def _alignment(alignment_id, alignment_type, engine_id="MFAENGINE"): + return { + "id": alignment_id, + "engine_id": engine_id, + "alignment_type": alignment_type, + "model_metadata": {"name": "english_us_arpa"}, + "alignment_date": "2026-01-02", + "status": "completed", + } + + +@pytest.fixture +def stacker(): + """A TrainingStacker with only the attributes these handlers touch.""" + instance = TrainingStacker.__new__(TrainingStacker) + instance.train_dataset_dropdown = _Dropdown(current_id="ds1") + instance.train_alignment_dropdown = _Dropdown() + return instance + + +class TestDatasetDropdownFilter: + def test_lists_only_datasets_with_manual_alignments(self, monkeypatch, stacker): + monkeypatch.setattr( + training_stacker.datasets, + "list_datasets_metadata", + lambda: [_dataset("ds1"), _dataset("ds2"), _dataset("ds3")], + ) + monkeypatch.setattr( + training_stacker.alignments, + "has_manual_alignments", + lambda dataset_id: dataset_id in {"ds1", "ds3"}, + ) + + stacker._populate_dataset_dropdown() + + assert stacker.train_dataset_dropdown.ids() == ["ds1", "ds3"] + assert stacker.train_dataset_dropdown.enabled is True + + def test_registered_but_unqualified_datasets_get_their_own_message(self, monkeypatch, stacker): + """The user has datasets; they just cannot be trained on yet. Telling them + "No datasets registered" here would send them off to register a duplicate.""" + monkeypatch.setattr( + training_stacker.datasets, + "list_datasets_metadata", + lambda: [_dataset("ds1"), _dataset("ds2")], + ) + monkeypatch.setattr(training_stacker.alignments, "has_manual_alignments", lambda _: False) + + stacker._populate_dataset_dropdown() + + assert stacker.train_dataset_dropdown.placeholder == "No datasets with manual alignments" + assert stacker.train_dataset_dropdown.enabled is False + + def test_no_datasets_at_all_keeps_the_original_message(self, monkeypatch, stacker): + monkeypatch.setattr(training_stacker.datasets, "list_datasets_metadata", lambda: []) + monkeypatch.setattr(training_stacker.alignments, "has_manual_alignments", lambda _: False) + + stacker._populate_dataset_dropdown() + + assert stacker.train_dataset_dropdown.placeholder == "No datasets registered" + assert stacker.train_dataset_dropdown.enabled is False + + +class TestAlignmentDropdownFilter: + def test_populates_from_manual_alignments_only(self, monkeypatch, stacker): + """The stacker must call list_manual_alignments, not filter list_alignments + itself -- the predicate lives in storage so it stays testable and shared.""" + captured = {} + + def _list_manual(dataset_id): + captured["dataset_id"] = dataset_id + return [_alignment("al2", "hand"), _alignment("al4", "corrected-v2")] + + monkeypatch.setattr(training_stacker.alignments, "list_manual_alignments", _list_manual) + + stacker.on_dataset_selected() + + assert captured["dataset_id"] == "ds1" + assert stacker.train_alignment_dropdown.ids() == ["al2", "al4"] + assert stacker.train_alignment_dropdown.enabled is True + + def test_empty_state_names_the_actual_requirement(self, monkeypatch, stacker): + """A dataset can have plenty of automatic alignments and still land here, so + "No alignments registered" would be actively wrong.""" + monkeypatch.setattr(training_stacker.alignments, "list_manual_alignments", lambda _: []) + + stacker.on_dataset_selected() + + assert ( + stacker.train_alignment_dropdown.placeholder == "No manual alignments for this dataset" + ) + assert stacker.train_alignment_dropdown.enabled is False + + +class TestNoticeCopy: + def test_notice_explains_both_the_why_and_the_filtering(self): + """Users see an unexpectedly short dataset list; the notice is the only + thing on screen that accounts for it.""" + assert "only meaningful on datasets with manual alignments" in MANUAL_ALIGNMENT_NOTICE + assert "will appear in the list below" in MANUAL_ALIGNMENT_NOTICE diff --git a/tests/storage/test_alignments.py b/tests/storage/test_alignments.py index c9a40dc..e382287 100644 --- a/tests/storage/test_alignments.py +++ b/tests/storage/test_alignments.py @@ -848,3 +848,84 @@ def test_defaults_to_automatic_for_legacy_data(self): meta = {"engine_id": "MFAENGINE"} assert get_alignment_type(meta) == "automatic" + + +class TestIsManualAlignment: + """Training data must come from a human, so this predicate gates the Train + Aligners dropdowns. It excludes "automatic" rather than allow-listing + {hand, corrected}, because alignment_type is a free-text field.""" + + def test_hand_and_corrected_are_manual(self): + from voxkit.storage.alignments import is_manual_alignment + + for t in ("hand", "corrected"): + assert is_manual_alignment({"engine_id": "MFAENGINE", "alignment_type": t}) is True + + def test_automatic_is_not_manual(self): + from voxkit.storage.alignments import is_manual_alignment + + assert ( + is_manual_alignment({"engine_id": "MFAENGINE", "alignment_type": "automatic"}) is False + ) + + def test_custom_correction_label_is_still_manual(self): + """create_corrected_alignment accepts any string -- the Correct Alignments + page exposes it as free text, so values like this reach storage. An + allow-list of {hand, corrected} would silently hide real corrected work.""" + from voxkit.storage.alignments import is_manual_alignment + + assert is_manual_alignment({"engine_id": "MFA (Nina)", "alignment_type": "corrected-v2"}) + + def test_legacy_alignments_without_the_field_are_classified_by_sentinel(self): + from voxkit.storage.alignments import HAND_ALIGNMENT_SENTINEL, is_manual_alignment + + assert is_manual_alignment({"engine_id": HAND_ALIGNMENT_SENTINEL}) is True + assert is_manual_alignment({"engine_id": "MFAENGINE"}) is False + + +class TestListManualAlignments: + """Filtering is layered over list_alignments, whose directory scan is covered + by TestListAlignments -- these patch it out to test the predicate alone.""" + + ALIGNMENTS = [ + {"id": "a1", "engine_id": "MFAENGINE", "alignment_type": "automatic"}, + {"id": "a2", "engine_id": "hand", "alignment_type": "hand"}, + {"id": "a3", "engine_id": "MFAENGINE", "alignment_type": "automatic"}, + {"id": "a4", "engine_id": "MFA (Nina)", "alignment_type": "corrected-v2"}, + ] + + def test_drops_automatic_and_preserves_order(self, monkeypatch): + from voxkit.storage import alignments as alignments_module + + monkeypatch.setattr(alignments_module, "list_alignments", lambda _: self.ALIGNMENTS) + + result = alignments_module.list_manual_alignments("ds1") + + assert [a["id"] for a in result] == ["a2", "a4"] + + def test_empty_when_every_alignment_is_automatic(self, monkeypatch): + from voxkit.storage import alignments as alignments_module + + monkeypatch.setattr( + alignments_module, + "list_alignments", + lambda _: [{"id": "a1", "engine_id": "MFAENGINE", "alignment_type": "automatic"}], + ) + + assert alignments_module.list_manual_alignments("ds1") == [] + + def test_has_manual_alignments_reflects_the_filter(self, monkeypatch): + from voxkit.storage import alignments as alignments_module + + monkeypatch.setattr(alignments_module, "list_alignments", lambda _: self.ALIGNMENTS) + assert alignments_module.has_manual_alignments("ds1") is True + + monkeypatch.setattr(alignments_module, "list_alignments", lambda _: self.ALIGNMENTS[:1]) + assert alignments_module.has_manual_alignments("ds1") is False + + def test_has_manual_alignments_false_for_dataset_with_no_alignments(self, monkeypatch): + from voxkit.storage import alignments as alignments_module + + monkeypatch.setattr(alignments_module, "list_alignments", lambda _: []) + + assert alignments_module.has_manual_alignments("ds1") is False