From 6c2dcac562ce2bea7f7f6a97d41a64205996353e Mon Sep 17 00:00:00 2001 From: JakimPL Date: Mon, 12 Oct 2026 20:39:56 +0200 Subject: [PATCH 1/8] Fixed: the pitch refinement crashing on a stretch of digital silence --- docs/concepts/reconstruction.md | 4 +- src/sampletones_core/fft/instantaneous.py | 19 +++++--- .../fft/test_instantaneous.py | 48 ++++++++++++++++++- .../reconstructor/refinement/test_refiner.py | 16 +++++++ 4 files changed, 77 insertions(+), 10 deletions(-) diff --git a/docs/concepts/reconstruction.md b/docs/concepts/reconstruction.md index 863e4e858..3e778c12c 100644 --- a/docs/concepts/reconstruction.md +++ b/docs/concepts/reconstruction.md @@ -337,8 +337,8 @@ dividers. A bend counts divider steps from its own note, so each note's frames a frame that reads nothing keeps a bend its own note read, and a new note starts from its own reading. A frame makes no proposal where it rests, where the stem holding it leaves that channel out, where its -channel is not pitched (the noise channel's sixteen periods have no finer grid), or where its reading -falls below the confidence threshold. A conversion that bent no note records both bend dimensions as ones +channel is not pitched (the noise channel's sixteen periods have no finer grid), where the recording is +silent, or where its reading falls below the confidence threshold. A conversion that bent no note records both bend dimensions as ones the channel governs, so it writes the instrument an unrefined run writes. Each stem entry says which recordings are carried, and on which channels, in `bends`: a subset of the diff --git a/src/sampletones_core/fft/instantaneous.py b/src/sampletones_core/fft/instantaneous.py index 9d74c6866..afe9aefc7 100644 --- a/src/sampletones_core/fft/instantaneous.py +++ b/src/sampletones_core/fft/instantaneous.py @@ -41,11 +41,12 @@ class InstantaneousPitch: Reading around a stated reference is what makes the answer usable: the reference names which bins carry the note's harmonics, and each harmonic's estimate is divided back down and weighted - by the energy standing behind it. The harmonics are read in order, each one settled against the - fundamental the ones below it agreed on, which is what keeps an upper harmonic on the right side - of the whole turn its phase states the reading to within. A later harmonic counts where its partial - stands within half a semitone of where that fundamental puts it, the room a note owns, so a - partial another voice sounds beside it stays out of the reading. + by the energy standing behind it. A harmonic is read where its bin carries energy, since a phase + turns at a partial's rate only where a partial sounds. The harmonics are read in order, each one + settled against the fundamental the ones below it agreed on, which is what keeps an upper harmonic + on the right side of the whole turn its phase states the reading to within. A later harmonic counts + where its partial stands within half a semitone of where that fundamental puts it, the room a note + owns, so a partial another voice sounds beside it stays out of the reading. The first harmonic read has the note alone to be settled against. From about 1 kHz up a note's room is wider than the whole turn the reading repeats over, so there a second reading, over a lag @@ -100,7 +101,8 @@ def at(self, frame: int, reference: float) -> Optional[FundamentalReading]: Returns: Optional[FundamentalReading]: The reading, or ``None`` where the frame lies outside - the transform or the reference names no bin the transform covers. + the transform, the reference names no bin the transform covers, or every bin it + names is silent in that frame. """ opening, closing = self._pair(frame) if opening is None or closing is None: @@ -115,6 +117,10 @@ def at(self, frame: int, reference: float) -> Optional[FundamentalReading]: if bin_index is None: continue + energy = float(self._magnitudes[bin_index, opening]) ** 2 + if energy <= 0.0: + continue + expected = running * harmonic if weight <= 0.0: expected = self._first_guide(bin_index, opening, expected) @@ -123,7 +129,6 @@ def at(self, frame: int, reference: float) -> Optional[FundamentalReading]: if weight > 0.0 and not expected / HARMONIC_ROOM <= partial <= expected * HARMONIC_ROOM: continue - energy = float(self._magnitudes[bin_index, opening]) ** 2 weighted += energy * partial / harmonic weight += energy share += float(self._bin_energies[bin_index, opening]) diff --git a/tests/unit/sampletones_core/fft/test_instantaneous.py b/tests/unit/sampletones_core/fft/test_instantaneous.py index 041ca1ddf..55f50dff9 100644 --- a/tests/unit/sampletones_core/fft/test_instantaneous.py +++ b/tests/unit/sampletones_core/fft/test_instantaneous.py @@ -1,5 +1,5 @@ import math -from typing import Final, List, Optional +from typing import Final, List, Optional, Tuple import numpy as np import pytest @@ -37,6 +37,10 @@ BASS_FREQUENCY: Final[float] = 65.41 BESIDE_FIFTH_HARMONIC: Final[float] = 349.23 ON_FOURTH_HARMONIC: Final[float] = 4 * BASS_FREQUENCY +GAP_SECONDS: Final[float] = 1.0 +GAP_MIDDLE_FRAMES: Final[int] = 5 +GATED_CENTS: Final[float] = 25.0 +SILENT_REFERENCES: Final[Tuple[float, ...]] = (LOW_BASS_FREQUENCY, A4_FREQUENCY, 3520.0) def _harmonic(frequency: float, seed: int = 0, seconds: float = SECONDS) -> np.ndarray: @@ -206,6 +210,48 @@ def test_a_voice_on_a_harmonic_agrees_with_the_note(self) -> None: assert abs(_bass_cents_under(ON_FOURTH_HARMONIC)) < CENT_TOLERANCE +@pytest.fixture(scope="module") +def silent_reader() -> InstantaneousPitch: + return InstantaneousPitch(np.zeros(int(SAMPLE_RATE * SECONDS), dtype=np.float32), SAMPLE_RATE, HOP) + + +@pytest.fixture(scope="module") +def gated_reader() -> InstantaneousPitch: + """Two hits of a tone standing ``GATED_CENTS`` off A4, with a stretch of digital silence between them.""" + tone = _harmonic(A4_FREQUENCY * 2 ** (GATED_CENTS / CENTS_PER_OCTAVE)) + gap = np.zeros(int(SAMPLE_RATE * GAP_SECONDS), dtype=np.float32) + return InstantaneousPitch(np.concatenate([tone, gap, tone]), SAMPLE_RATE, HOP) + + +class TestASilentStretch: + """Digital silence gives every bin a coefficient of zero, which carries no phase to read. One-shot + samples and gated loops hold such stretches between their hits, and a note the decoder holds across + one has no fundamental there.""" + + @pytest.mark.parametrize("reference", SILENT_REFERENCES, ids=lambda reference: f"{reference:g} Hz") + def test_a_silent_recording_is_read_as_nothing( + self, + silent_reader: InstantaneousPitch, + reference: float, + ) -> None: + assert all(silent_reader.at(frame, reference) is None for frame in range(silent_reader.columns)) + + def test_the_middle_of_a_silent_gap_is_read_as_nothing(self, gated_reader: InstantaneousPitch) -> None: + middle = int((SECONDS + GAP_SECONDS / 2) * SAMPLE_RATE / HOP) + frames = range(middle - GAP_MIDDLE_FRAMES, middle + GAP_MIDDLE_FRAMES + 1) + + assert all(gated_reader.at(frame, A4_FREQUENCY) is None for frame in frames) + + def test_the_hits_around_a_silent_gap_are_read_where_they_stand(self, gated_reader: InstantaneousPitch) -> None: + tone_frames = int(SECONDS * SAMPLE_RATE / HOP) + hits = [*range(tone_frames), *range(gated_reader.columns - tone_frames, gated_reader.columns)] + read = [gated_reader.at(frame, A4_FREQUENCY) for frame in hits] + readings = [reading for reading in read if reading is not None] + + assert len(readings) == len(hits) + assert abs(_median_cents(readings, A4_FREQUENCY) - GATED_CENTS) < CENT_TOLERANCE + + class TestHowMuchOfAFrameStandsBehindItsReading: def test_a_pitched_frame_reads_with_confidence(self) -> None: readings = _readings(_harmonic(A4_FREQUENCY), A4_FREQUENCY) diff --git a/tests/unit/sampletones_core/reconstructions/reconstructor/refinement/test_refiner.py b/tests/unit/sampletones_core/reconstructions/reconstructor/refinement/test_refiner.py index dc11e3c05..0196336a5 100644 --- a/tests/unit/sampletones_core/reconstructions/reconstructor/refinement/test_refiner.py +++ b/tests/unit/sampletones_core/reconstructions/reconstructor/refinement/test_refiner.py @@ -174,6 +174,22 @@ def test_the_next_note_takes_the_bend_it_reads(self, config: Config, monkeypatch assert bends[SILENT_FRAMES:] == [BEND] * FRAMES +class TestANoteHeldOverSilence: + """The decoder may hold a note through a stretch of digital silence, which states no fundamental.""" + + def test_a_note_over_silence_stays_at_its_note(self, config: Config) -> None: + stems = _stems(StemEntry(id=STEM_A, settings=StemSettings(channels=TONES, bends=list(TONES)))) + refined = PitchRefiner(config=config, channels=get_generators_by_channels(config, TONES), stems=stems) + streams = refined.refine( + {channel_name: _stream(channel_name) for channel_name in TONES}, + {channel_name: [STEM_A] * FRAMES for channel_name in TONES}, + {STEM_A: np.zeros(int(config.library.sample_rate * SECONDS))}, + ) + + assert not any(_bends(streams, ChannelName.PULSE1)) + assert not any(_bends(streams, ChannelName.TRIANGLE)) + + class TestWhatTheRefinementReadsPerStem: """A reading is taken for the recording a bend is read from, and for no other.""" From fdd60ec632b005ce11364f116767ef00e5947d35 Mon Sep 17 00:00:00 2001 From: JakimPL Date: Mon, 12 Oct 2026 20:40:07 +0200 Subject: [PATCH 2/8] Fixed: the file manager stand-in's cases running its shell scripts on Windows --- automation/boundaries/reveals.py | 3 +++ tests/unit/automation/boundaries/test_reveals.py | 9 +++++++++ 2 files changed, 12 insertions(+) diff --git a/automation/boundaries/reveals.py b/automation/boundaries/reveals.py index ca9a3ca55..43f3493ea 100644 --- a/automation/boundaries/reveals.py +++ b/automation/boundaries/reveals.py @@ -21,6 +21,9 @@ class FileManagerStandIn: application's own code runs and reaches them: the first names a file manager they stand for, the file manager writes the path it was given to ``shown_log``, and ``xdg-open`` writes a folder there and a file to ``opened_log``, one line each. + + The programs are shell scripts, which Linux and macOS start by name from their ``#!`` line. + Windows starts a program by name from an ``.exe`` file. """ def __init__( diff --git a/tests/unit/automation/boundaries/test_reveals.py b/tests/unit/automation/boundaries/test_reveals.py index f2cc622c1..1569d53e2 100644 --- a/tests/unit/automation/boundaries/test_reveals.py +++ b/tests/unit/automation/boundaries/test_reveals.py @@ -4,6 +4,12 @@ import pytest from automation.boundaries.reveals import FILE_MANAGER, FileManagerStandIn +from sampletones_shared.utils.system.system import System + +requires_scripts_started_by_name = pytest.mark.skipif( + System.current() == System.WINDOWS, + reason="the stand-in programs are shell scripts, and Windows starts a program by name from an .exe file", +) @pytest.fixture @@ -25,18 +31,21 @@ def test_nothing_is_noted_before_a_program_runs(self, stand_in: FileManagerStand assert stand_in.shown() == () assert stand_in.opened() == () + @requires_scripts_started_by_name def test_xdg_open_on_a_folder_shows_it(self, stand_in: FileManagerStandIn, tmp_path: Path) -> None: subprocess.run(["xdg-open", str(tmp_path)], check=True) assert stand_in.shown() == (tmp_path,) assert stand_in.opened() == () + @requires_scripts_started_by_name def test_xdg_open_on_a_file_opens_it(self, stand_in: FileManagerStandIn, page: Path) -> None: subprocess.run(["xdg-open", str(page)], check=True) assert stand_in.opened() == (page,) assert stand_in.shown() == () + @requires_scripts_started_by_name def test_the_file_manager_shows_what_it_is_given(self, stand_in: FileManagerStandIn, page: Path) -> None: subprocess.run([FILE_MANAGER, str(page)], check=True) From 85b8acaef7eed167e1c8d06118f2e8f9efa3f492 Mon Sep 17 00:00:00 2001 From: JakimPL Date: Mon, 12 Oct 2026 20:40:17 +0200 Subject: [PATCH 3/8] Noted: skipping a recording that fails in a run of one per recording --- docs/development/bugs-and-todos.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/development/bugs-and-todos.md b/docs/development/bugs-and-todos.md index 2025272bd..4e667bb05 100644 --- a/docs/development/bugs-and-todos.md +++ b/docs/development/bugs-and-todos.md @@ -46,6 +46,9 @@ dimension the import starts carrying. * Waveform construction preview: a waveform drawn from the partial reconstruction while the converter reconstructs a single file. * Picking several rows of the converter's list at once, so a group leaves or settles in one gesture. +* An option to skip a recording that fails in a run of one reconstruction per recording, on the Converter + card and on `sampletones convert`. A run ends at its first failure, so one broken file stops a whole + folder. * Selecting parts of a reconstruction's waveform to crop, trim, cut and paste them along with their instructions. * Marking broken files where the browsers list them: a reconstruction that fails to load, one whose From a4cde65f54b5de6bf3a590f40edf54dfda54501a Mon Sep 17 00:00:00 2001 From: JakimPL Date: Mon, 12 Oct 2026 21:38:39 +0200 Subject: [PATCH 4/8] Fixed: the screen stalling after a library generation --- docs/development/application/render-thread.md | 6 + docs/development/architecture.md | 1 + docs/development/progress.md | 15 ++ .../logic/instruction/library.py | 7 +- .../logic/main/converter/logic.py | 13 +- src/sampletones_application/services/base.py | 37 ++++- .../services/conversion/service.py | 4 +- .../utils/callbacks/latest.py | 59 ++++++++ .../utils/callbacks/priority.py | 4 +- .../utils/callbacks/queue.py | 4 +- .../logic/instruction/test_library_logic.py | 52 ++++++- .../logic/main/converter/test_logic.py | 54 ++++++- .../services/conftest.py | 4 +- .../services/test_base.py | 75 +++++++++- .../services/test_conversion.py | 38 +++++ .../utils/callbacks/test_latest.py | 140 ++++++++++++++++++ 16 files changed, 493 insertions(+), 20 deletions(-) create mode 100644 src/sampletones_application/utils/callbacks/latest.py create mode 100644 tests/unit/sampletones_application/utils/callbacks/test_latest.py diff --git a/docs/development/application/render-thread.md b/docs/development/application/render-thread.md index 60aa99b03..b9af65f9b 100644 --- a/docs/development/application/render-thread.md +++ b/docs/development/application/render-thread.md @@ -19,6 +19,12 @@ budget (`scheduling.queue_budget_seconds`), so a large backlog spreads across fr continues. Every background result reaches UI state this way. Applying one to UI state directly from the worker thread is forbidden. +A report of where a run stands crosses through `LatestDelivery` (`utils/callbacks/latest.py`). One delivery +waits in the queue at a time, and the drain that reaches it hands over the newest report filed, so a burst +of reports costs one handling. The delivery keeps the place its first report took, which keeps a result +posted after that report, such as the run's outcome, behind it. [Progress](../progress.md) gives the +principle. + A result carries what the landing installs. The regeneration worker renders every sound the Reconstructions tab draws or plays: each channel's audio, the mix of what the reader hears, the mix of the recordings heard, which the waveform draws as the original, and the envelopes each channel exports for the diff --git a/docs/development/architecture.md b/docs/development/architecture.md index abe568907..0957d55b4 100644 --- a/docs/development/architecture.md +++ b/docs/development/architecture.md @@ -205,6 +205,7 @@ Runs long operations (file conversion, waveform regeneration, export, playback s - Every service inherits `ServiceBase[ResultType]`, which provides `subscribe(handler)`, `unsubscribe(handler)` and `_emit(result)`. - `_emit` posts the result to `CallbackQueue`, which puts every handler on the render thread (principle 6). +- `_emit` delivers every result it is given, which is how an outcome travels. `_report` carries where a run stands, and the newest report waiting supersedes the ones before it, so a subscriber draws a run once per drain however often the run reports. - Result types are a tagged union, so a subscriber matches exhaustively. A long operation reports through `ServiceStarted`, `ServiceProgress`, `ServiceIntermediate`, `ServiceSuccess`, `ServiceError` and `ServiceCanceled`, and a service whose reports carry more, such as the playhead or the request they answer, declares a union of its own in its `result.py`. - A service is one subpackage holding `service.py` and `result.py`, so its implementation and the contract its subscribers type against are reached separately. The generic contracts every service reports through are in `services/result.py`. `ServiceProgress.fraction` is the one reading a bar draws. See [`progress.md`](progress.md). - A service imports nothing from `logic/`, `view_model/`, `ui/`, `coordinators/` or `config/`, which the boundary check holds. diff --git a/docs/development/progress.md b/docs/development/progress.md index 2be200d28..83ad4382a 100644 --- a/docs/development/progress.md +++ b/docs/development/progress.md @@ -53,6 +53,12 @@ Both carriers use it: `StageProgress` in the application services, and `JobRepor `JobReporter` throttles on the worker side, so the line between processes carries only what a bar can be redrawn at. +The render thread takes the newest report. A report states the whole of where a run stands, so a report +still waiting for the render loop is superseded by the next one. A run filing thousands of reports between +two frames, such as a library generation counting its instructions, is drawn once per drain, and the screen +keeps pace with the run. The outcome of the run arrives after the reports filed before it. +[The render thread](application/render-thread.md) describes the crossing. + ### 4. A run reports in items when it has many, and in the part done of one item when it has one An operation counts the items it is measured in: files, samples, jobs. Where it is measured in many items, @@ -85,6 +91,15 @@ queue that reports travel up, and a flag that a withdrawal travels down. Both li context the pool's workers run in. Each end reaches a task as an ordinary value it is built with, so a worker started as a fresh interpreter reconnects on its own. That makes one channel serve every platform. +### 6. A report redraws the run over a setup read once + +A report says where a run stands: the count, the stage, the item under way. The setup the run was started +from stands still while it runs, so a panel reads what it shows of the setup once per change to it, and +every report draws over that reading. The converter reads its rows, and the recordings its button names, +where a gesture changes what it gathered. A report therefore costs the same for one gathered recording as +for thousands. The button names the run the same way before it, during it and after it, whether the run +succeeded, failed or was canceled. + --- ## Mechanics diff --git a/src/sampletones_application/logic/instruction/library.py b/src/sampletones_application/logic/instruction/library.py index e6192536c..c37d2dcfb 100644 --- a/src/sampletones_application/logic/instruction/library.py +++ b/src/sampletones_application/logic/instruction/library.py @@ -8,6 +8,8 @@ from sampletones_application.logic.instruction.library_manager import ( InstructionsLibraryManager, ) +from sampletones_application.utils.callbacks.latest import LatestDelivery +from sampletones_application.utils.callbacks.priority import DEFAULT_PRIORITY from sampletones_application.utils.callbacks.queue import CallbackQueue from sampletones_application.view_model.instruction.library import ( LibraryPanelViewModel, @@ -92,9 +94,10 @@ def __init__( self._msg_window_not_available = language_manager["instructions.library.message.status_window_not_available"] self._msg_load_error = language_manager["instructions.library.message.status_load_error"] + self._generation_reports = LatestDelivery(self._on_generation_progress, priority=DEFAULT_PRIORITY) self._library_manager.set_callbacks( on_generation_start=partial(CallbackQueue.add, self._on_generation_start), - on_generation_progress=partial(CallbackQueue.add, self._on_generation_progress), + on_generation_progress=self._generation_reports.post, on_generation_completed=partial(CallbackQueue.add, self._on_generation_completed), on_generation_error=partial(CallbackQueue.add, self._on_generation_error), on_generation_canceled=partial(CallbackQueue.add, self._on_generation_canceled), @@ -490,7 +493,7 @@ def _on_generation_progress( task_status: TaskStatus, task_progress: TaskProgress, ) -> None: - """Paints a report of the generation under way.""" + """Paints the newest report of the generation under way.""" match task_status: case TaskStatus.COMPLETED: self._emit_view(self._language_manager["instructions.library.message.status_saving"], progress=1.0) diff --git a/src/sampletones_application/logic/main/converter/logic.py b/src/sampletones_application/logic/main/converter/logic.py index e33dab690..e5cf0a932 100644 --- a/src/sampletones_application/logic/main/converter/logic.py +++ b/src/sampletones_application/logic/main/converter/logic.py @@ -102,6 +102,7 @@ def __init__( selected=None, ) self._rows = self._read_rows() + self._converted = self._read_converted() self._run = ConversionRun(conversion_service, messages=self._messages) self._run.on_report = self._on_report @@ -529,6 +530,7 @@ def _rewrite(self, state: ConverterState) -> None: self._remember(state.settings) self._state = self._redirected(state.selecting(state.selected)) self._rows = self._read_rows() + self._converted = self._read_converted() self._run.return_to_idle() self._emit(self._messages.idle, 0.0) @@ -642,10 +644,19 @@ def _read_rows(self) -> Tuple[StemRowViewModel, ...]: """ return stem_rows(self._state.gathering, mixes=self.mixes) + def _read_converted(self) -> Tuple[Path, ...]: + """The recordings that take part in the setup now standing, read once for that setup. + + A run writing one reconstruction apiece derives an entry for every gathered recording, so + the reading is taken where the setup changes. The button names the run from it before, during + and after the run, whatever the run comes to. + """ + return playing_sources(self._state) + def _action_label(self) -> str: """What the button says the run writes, read from the recordings taking part in it.""" return self._messages.action_label( phase=self._run.phase, mixes=self.mixes, - converted=playing_sources(self._state), + converted=self._converted, ) diff --git a/src/sampletones_application/services/base.py b/src/sampletones_application/services/base.py index d543d3046..9dc5c6ff8 100644 --- a/src/sampletones_application/services/base.py +++ b/src/sampletones_application/services/base.py @@ -1,11 +1,21 @@ from abc import ABC +from dataclasses import dataclass from typing import Callable, Generic, List, TypeVar +from sampletones_application.utils.callbacks.latest import LatestDelivery from sampletones_application.utils.callbacks.queue import CallbackQueue T = TypeVar("T") +@dataclass(frozen=True) +class _Subscription(Generic[T]): + """One subscriber, and the delivery its reports wait in.""" + + handler: Callable[[T], None] + reports: LatestDelivery[[T]] + + class ServiceBase(ABC, Generic[T]): """ The boundary between synchronous application logic and asynchronous computation. @@ -17,22 +27,37 @@ class ServiceBase(ABC, Generic[T]): ordinary synchronous values. - *Exhaustive result types* — result types form a tagged union; subscribers handle each case with ``match``, making incomplete handling a type error. + + A result travels one of two ways. ``_emit`` delivers every result it is given, which is how an + outcome travels. ``_report`` carries where a run stands, and a report still waiting for the + render thread is superseded by the next, so a subscriber draws the newest one once per drain. """ def __init__(self, priority: int = 0) -> None: self._priority = priority - self._listeners: List[Callable[[T], None]] = [] + self._subscriptions: List[_Subscription[T]] = [] def subscribe(self, handler: Callable[[T], None]) -> None: - self._listeners.append(handler) + self._subscriptions.append( + _Subscription( + handler=handler, + reports=LatestDelivery(handler, priority=self._priority), + ) + ) def unsubscribe(self, handler: Callable[[T], None]) -> None: - if handler in self._listeners: - self._listeners.remove(handler) + for subscription in self._subscriptions: + if subscription.handler == handler: + self._subscriptions.remove(subscription) + return def _emit(self, result: T) -> None: - for listener in self._listeners: - CallbackQueue.add(listener, result, priority=self._priority) + for subscription in self._subscriptions: + CallbackQueue.add(subscription.handler, result, priority=self._priority) + + def _report(self, result: T) -> None: + for subscription in self._subscriptions: + subscription.reports.post(result) @property def class_name(self) -> str: diff --git a/src/sampletones_application/services/conversion/service.py b/src/sampletones_application/services/conversion/service.py index 811467a9f..983843e42 100644 --- a/src/sampletones_application/services/conversion/service.py +++ b/src/sampletones_application/services/conversion/service.py @@ -90,7 +90,7 @@ def _on_progress( ) -> None: match task_status: case TaskStatus.RUNNING | TaskStatus.CANCELING: - self._emit(self._reading(task_progress)) + self._report(self._reading(task_progress)) case _: pass @@ -170,4 +170,4 @@ def forward_library_progress( _status: TaskStatus, progress: TaskProgress, ) -> None: - self._emit(ServiceIntermediate(data=progress)) + self._report(ServiceIntermediate(data=progress)) diff --git a/src/sampletones_application/utils/callbacks/latest.py b/src/sampletones_application/utils/callbacks/latest.py new file mode 100644 index 000000000..865ed899b --- /dev/null +++ b/src/sampletones_application/utils/callbacks/latest.py @@ -0,0 +1,59 @@ +import threading +from functools import partial +from typing import Callable, Generic, Optional, ParamSpec + +from sampletones_application.utils.callbacks.queue import CallbackQueue +from sampletones_shared.types.callback import VoidCallback + +ReportParameters = ParamSpec("ReportParameters") + + +class LatestDelivery(Generic[ReportParameters]): + """Carries a run's reports to the render thread, where the newest one stands for those before it. + + A run reports where it stands as often as it moves, from whichever thread moves it, and each + report states the whole of that reading. One delivery therefore waits in the queue at a time, + and the drain that reaches it hands ``handler`` the newest report filed. A run filing thousands + of reports between two frames costs the render thread one handling per drain. + + The delivery takes its place in the queue with the first report it carries, so whatever is + posted after that report, such as the run's outcome, still runs after it. + """ + + def __init__( + self, + handler: Callable[ReportParameters, None], + *, + priority: int, + ) -> None: + """Holds the handler the newest report reaches. + + Args: + handler: What takes a report on the render thread. + priority: The priority the delivery waits in the queue at, which the run's other + results share so they keep their order. + """ + self._handler = handler + self._priority = priority + self._lock = threading.Lock() + self._newest: Optional[VoidCallback] = None + + def post( + self, + *args: ReportParameters.args, + **kwargs: ReportParameters.kwargs, + ) -> None: + """Files a report from any thread, which supersedes a report still waiting for the render thread.""" + with self._lock: + waiting = self._newest is not None + self._newest = partial(self._handler, *args, **kwargs) + + if not waiting: + CallbackQueue.add(self._deliver, priority=self._priority) + + def _deliver(self) -> None: + with self._lock: + newest, self._newest = self._newest, None + + if newest is not None: + newest() diff --git a/src/sampletones_application/utils/callbacks/priority.py b/src/sampletones_application/utils/callbacks/priority.py index a72cb8899..2cef62aa2 100644 --- a/src/sampletones_application/utils/callbacks/priority.py +++ b/src/sampletones_application/utils/callbacks/priority.py @@ -1,7 +1,9 @@ from __future__ import annotations from dataclasses import dataclass -from typing import Tuple +from typing import Final, Tuple + +DEFAULT_PRIORITY: Final[int] = 0 @dataclass(frozen=True) diff --git a/src/sampletones_application/utils/callbacks/queue.py b/src/sampletones_application/utils/callbacks/queue.py index 3f4c7f322..7c47220b8 100644 --- a/src/sampletones_application/utils/callbacks/queue.py +++ b/src/sampletones_application/utils/callbacks/queue.py @@ -6,7 +6,7 @@ from typing import Any, ClassVar, List from sampletones_application.utils.callbacks.failures import UnhandledFailures -from sampletones_application.utils.callbacks.priority import CallbackPriority +from sampletones_application.utils.callbacks.priority import DEFAULT_PRIORITY, CallbackPriority from sampletones_application.utils.callbacks.task import CallbackTask from sampletones_shared.exceptions import CallbackQueueStop from sampletones_shared.logger import logger @@ -60,7 +60,7 @@ def add( cls, callback: Callback, *args: Any, - priority: int = 0, + priority: int = DEFAULT_PRIORITY, delay: int = 0, **kwargs: Any, ) -> None: diff --git a/tests/unit/sampletones_application/logic/instruction/test_library_logic.py b/tests/unit/sampletones_application/logic/instruction/test_library_logic.py index 7a9b98056..2fe8d2c23 100644 --- a/tests/unit/sampletones_application/logic/instruction/test_library_logic.py +++ b/tests/unit/sampletones_application/logic/instruction/test_library_logic.py @@ -22,7 +22,7 @@ from sampletones_core.constants.enums import GeneratorName, SpectrumMethod from sampletones_core.fft import Window from sampletones_core.library import InstructionLibraryData, InstructionLibraryKey, LibraryState -from sampletones_core.parallelization import TaskStatus +from sampletones_core.parallelization import TaskProgress, TaskStatus from sampletones_shared.application import SAMPLETONES_LIBRARY_DATA_VERSION from sampletones_shared.exceptions import ( DeserializationError, @@ -568,6 +568,56 @@ def test_every_ending_paints_the_idle_controls(self, catalog: Catalog, ending: A ) +def _report(catalog: Catalog, status: TaskStatus, completed: int) -> None: + """Files the report a creator's monitor sends once ``completed`` instructions are finished.""" + catalog.creator.callbacks["on_progress"]( + status, + TaskProgress(total=GENERATED_INSTRUCTIONS, completed=completed), + ) + + +class TestTheReportsOfAGeneration(BaseTestSuite): + """A creator reports every instruction it finishes. The render loop paints the newest report waiting, + once, and the generation's close follows it.""" + + def test_reports_filed_before_a_drain_wait_as_one(self, catalog: Catalog) -> None: + catalog.start_generation() + + for completed in range(1, GENERATED_INSTRUCTIONS + 1): + _report(catalog, TaskStatus.RUNNING, completed) + + assert catalog.queue.held == 1 + + def test_a_report_after_a_drain_waits_again(self, catalog: Catalog) -> None: + catalog.start_generation() + _report(catalog, TaskStatus.COMPLETED, GENERATED_INSTRUCTIONS) + catalog.queue.drain() + + _report(catalog, TaskStatus.COMPLETED, GENERATED_INSTRUCTIONS) + + assert catalog.queue.held == 1 + + def test_the_newest_report_is_the_one_painted(self, catalog: Catalog) -> None: + catalog.start_generation() + painted = len(catalog.views) + + _report(catalog, TaskStatus.RUNNING, 1) + _report(catalog, TaskStatus.COMPLETED, GENERATED_INSTRUCTIONS) + catalog.queue.drain() + + assert [view.status_text for view in catalog.views[painted:]] == ["saving"] + + def test_the_close_paints_after_the_report_before_it(self, catalog: Catalog) -> None: + catalog.start_generation() + painted = len(catalog.views) + + _report(catalog, TaskStatus.COMPLETED, GENERATED_INSTRUCTIONS) + catalog.write_library() + catalog.queue.drain() + + assert (catalog.views[painted].status_text, catalog.views[-1].is_generating) == ("saving", False) + + class TestTheCatalogFollowingTheConfiguration: """A configuration change naming another library directory roots the catalog there, and any other change repaints the status over the libraries already loaded.""" diff --git a/tests/unit/sampletones_application/logic/main/converter/test_logic.py b/tests/unit/sampletones_application/logic/main/converter/test_logic.py index 85682a682..eadd75ae6 100644 --- a/tests/unit/sampletones_application/logic/main/converter/test_logic.py +++ b/tests/unit/sampletones_application/logic/main/converter/test_logic.py @@ -1,5 +1,5 @@ from pathlib import Path -from typing import Any, Callable, FrozenSet, List, Optional, Tuple +from typing import Any, Callable, Final, FrozenSet, List, Optional, Tuple from unittest.mock import MagicMock, patch import pytest @@ -12,8 +12,15 @@ from sampletones_application.logic.instruction.readiness import LibraryReadiness from sampletones_application.logic.main.converter.logic import ConverterLogic from sampletones_application.logic.main.converter.run import ConversionSuccess +from sampletones_application.logic.main.converter.setup import playing_sources from sampletones_application.services.conversion.result import ConversionItem, ConversionResult -from sampletones_application.services.result import ServiceError, ServiceProgress, ServiceSuccess +from sampletones_application.services.result import ( + ServiceCanceled, + ServiceError, + ServiceIntermediate, + ServiceProgress, + ServiceSuccess, +) from sampletones_application.view_model.main.converter import ( ACTIVE_PHASES, ConversionPhase, @@ -24,6 +31,7 @@ from sampletones_core.constants.algorithm import UNIT_DRIVE from sampletones_core.constants.enums import ChannelName, HierarchyMode from sampletones_core.library import LibraryState +from sampletones_core.parallelization import TaskProgress from sampletones_core.reconstructions.converter import ConversionJob, GroupConversion from sampletones_core.reconstructions.converter.paths import get_audio_files from tests.suite.base import BaseTestSuite @@ -31,6 +39,8 @@ from tests.unit.sampletones_application.logic.main.converter.texts import TEXTS SCHEDULING: str = "sampletones_application.logic.main.converter.logic.CallbackQueue.add" +PLAYING_SOURCES: Final[str] = "sampletones_application.logic.main.converter.logic.playing_sources" +LIBRARY_REPORTS: Final[int] = 100 LOUD_DRIVE: float = 2.0 @@ -475,6 +485,46 @@ def test_several_rows_name_none_of_them(self, converter_logic: ConverterLogic) - assert _view(converter_logic).input_path is None +class TestTheRecordingsTheButtonNames(BaseTestSuite): + """The button names the run from the recordings read where the setup changed. Every report the run + files and every ending it comes to reuse that reading.""" + + def test_a_gesture_reads_the_recordings_again(self, converter_logic: ConverterLogic) -> None: + with patch(PLAYING_SOURCES, wraps=playing_sources) as reading: + _listed(converter_logic, "kick", "snare") + + assert reading.called + assert _view(converter_logic).action_label == "Convert 2 recordings" + + @pytest.mark.parametrize( + "ending", + [ + ServiceSuccess(value=(Path("/reconstructions/kick.stn"), Path("/reconstructions/snare.stn"))), + ServiceError(exception=RuntimeError("a recording failed")), + ServiceCanceled(), + ], + ids=["succeeded", "failed", "canceled"], + ) + def test_reports_and_the_ending_reuse_the_reading( + self, + converter_logic: ConverterLogic, + service: MagicMock, + ending: ConversionResult, + ) -> None: + _listed(converter_logic, "kick", "snare") + named = _view(converter_logic).action_label + + with patch(SCHEDULING), patch(PLAYING_SOURCES, wraps=playing_sources) as reading: + converter_logic.start_conversion(confirmed=True) + for completed in range(1, LIBRARY_REPORTS + 1): + _reports(service, ServiceIntermediate(data=TaskProgress(total=LIBRARY_REPORTS, completed=completed))) + _reports(service, ending) + converter_logic.close() + + assert reading.call_count == 0 + assert _view(converter_logic).action_label == named + + class TestWhatACompletedConversionLeaves(BaseTestSuite): """A completed conversion tells its listener what it wrote, so the follow-up offer can target the single reconstruction or the folder holding a batch.""" diff --git a/tests/unit/sampletones_application/services/conftest.py b/tests/unit/sampletones_application/services/conftest.py index f7394efd3..98f18de6f 100644 --- a/tests/unit/sampletones_application/services/conftest.py +++ b/tests/unit/sampletones_application/services/conftest.py @@ -1,3 +1,3 @@ -from tests.suite.application import synchronous_executor, synchronous_queue +from tests.suite.application import live_queue, synchronous_executor, synchronous_queue -__all__ = ["synchronous_executor", "synchronous_queue"] +__all__ = ["live_queue", "synchronous_executor", "synchronous_queue"] diff --git a/tests/unit/sampletones_application/services/test_base.py b/tests/unit/sampletones_application/services/test_base.py index 7cb2ffd39..56214ab35 100644 --- a/tests/unit/sampletones_application/services/test_base.py +++ b/tests/unit/sampletones_application/services/test_base.py @@ -1,14 +1,23 @@ -from typing import List +from typing import Final, List from unittest.mock import MagicMock, call, patch +import pytest + from sampletones_application.services.base import ServiceBase from sampletones_application.utils.callbacks.queue import CallbackQueue +from tests.suite.application import draw_frame +from tests.suite.base import BaseTestSuite + +OUTCOME: Final[int] = 9 class _ConcreteService(ServiceBase[int]): def emit(self, value: int) -> None: self._emit(value) + def report(self, value: int) -> None: + self._report(value) + class TestServiceBaseSubscription: def test_subscriber_receives_emitted_result(self) -> None: @@ -126,3 +135,67 @@ def test_zero_listeners_no_queue_calls(self) -> None: service._emit(0) mock_add.assert_not_called() + + +@pytest.mark.usefixtures("live_queue") +class TestServiceBaseReports(BaseTestSuite): + """A report waiting for the render loop is superseded by the next, while every emitted result arrives.""" + + def test_a_subscriber_takes_the_newest_report_once_per_drain(self) -> None: + service = _ConcreteService() + results: List[int] = [] + service.subscribe(results.append) + + service.report(1) + service.report(2) + service.report(3) + draw_frame() + + assert results == [3] + + def test_every_emitted_result_arrives(self) -> None: + service = _ConcreteService() + results: List[int] = [] + service.subscribe(results.append) + + service.emit(1) + service.emit(2) + service.emit(3) + draw_frame() + + assert results == [1, 2, 3] + + def test_an_outcome_emitted_after_reports_arrives_after_the_newest(self) -> None: + service = _ConcreteService() + results: List[int] = [] + service.subscribe(results.append) + + service.report(1) + service.report(2) + service.emit(OUTCOME) + draw_frame() + + assert results == [2, OUTCOME] + + def test_each_registration_takes_the_newest_report(self) -> None: + service = _ConcreteService() + results: List[int] = [] + service.subscribe(results.append) + service.subscribe(results.append) + + service.report(1) + service.report(2) + draw_frame() + + assert results == [2, 2] + + def test_an_unsubscribed_handler_takes_no_report(self) -> None: + service = _ConcreteService() + results: List[int] = [] + service.subscribe(results.append) + service.unsubscribe(results.append) + + service.report(1) + draw_frame() + + assert results == [] diff --git a/tests/unit/sampletones_application/services/test_conversion.py b/tests/unit/sampletones_application/services/test_conversion.py index 61cc96d3e..d9f1a388b 100644 --- a/tests/unit/sampletones_application/services/test_conversion.py +++ b/tests/unit/sampletones_application/services/test_conversion.py @@ -22,6 +22,7 @@ from sampletones_core.reconstructions.reconstructor.stems.configs.config import StemsConfig from sampletones_core.reconstructions.reconstructor.stems.configs.settings import StemSettings from sampletones_core.reconstructions.stage import ReconstructionStage +from tests.suite.application import draw_frame from tests.suite.base import BaseTestSuite MockConverterClass: TypeAlias = Tuple[MagicMock, MagicMock, Dict[str, Callable[..., Any]]] @@ -45,6 +46,7 @@ EARLY: Final[float] = 0.2 HALFWAY: Final[float] = 0.5 NEARLY_DONE: Final[float] = 0.9 +LIBRARY_INSTRUCTIONS: Final[int] = 100 def _under_way(*fractions: float) -> Tuple[TaskStep, ...]: @@ -498,3 +500,39 @@ def test_cancel_when_not_running_does_not_call_converter_cancel( converter.is_running.return_value = False conversion_service.cancel() converter.cancel.assert_not_called() + + +@pytest.mark.usefixtures("live_queue") +class TestReportsWaitingForTheRenderLoop(BaseTestSuite): + """A run's reports reach the subscriber as the newest one waiting, and the run's outcome after it.""" + + def test_library_reports_filed_before_a_drain_arrive_as_the_newest(self, service: Service) -> None: + conversion_service, _, _, results = service + for completed in range(1, LIBRARY_INSTRUCTIONS + 1): + conversion_service.forward_library_progress( + TaskStatus.RUNNING, + TaskProgress(total=LIBRARY_INSTRUCTIONS, completed=completed), + ) + + draw_frame() + + assert [result.data.completed for result in results] == [LIBRARY_INSTRUCTIONS] + + def test_reports_filed_across_drains_arrive_once_per_drain(self, service: Service) -> None: + _, _, callbacks, results = service + for completed in range(SEVERAL_RECONSTRUCTIONS): + callbacks["on_progress"](TaskStatus.RUNNING, _account(SEVERAL_RECONSTRUCTIONS, completed)) + draw_frame() + + assert [result.completed for result in results] == list(range(SEVERAL_RECONSTRUCTIONS)) + + def test_the_outcome_arrives_after_the_reports_before_it(self, service: Service) -> None: + _, _, callbacks, results = service + for completed in range(SEVERAL_RECONSTRUCTIONS): + callbacks["on_progress"](TaskStatus.RUNNING, _account(SEVERAL_RECONSTRUCTIONS, completed)) + callbacks["on_completed"]((Path(OUTPUT),)) + + draw_frame() + + assert [type(result) for result in results] == [ServiceProgress, ServiceSuccess] + assert results[0].completed == SEVERAL_RECONSTRUCTIONS - 1 diff --git a/tests/unit/sampletones_application/utils/callbacks/test_latest.py b/tests/unit/sampletones_application/utils/callbacks/test_latest.py new file mode 100644 index 000000000..1c25cf031 --- /dev/null +++ b/tests/unit/sampletones_application/utils/callbacks/test_latest.py @@ -0,0 +1,140 @@ +import threading +from typing import Final, List, Union + +import pytest + +from sampletones_application.utils.callbacks.latest import LatestDelivery +from sampletones_application.utils.callbacks.priority import DEFAULT_PRIORITY +from sampletones_application.utils.callbacks.queue import CallbackQueue +from tests.suite.application import draw_frame +from tests.suite.base import BaseTestSuite + +REPORTS: Final[int] = 1_000 +WORKERS: Final[int] = 4 +OUTCOME: Final[str] = "outcome" + +Handled = List[Union[int, str]] + + +class Handler: + """What a delivery hands over, and the threads it was handed over on.""" + + def __init__(self) -> None: + self.handled: Handled = [] + self.threads: List[threading.Thread] = [] + + def __call__(self, report: int) -> None: + self.handled.append(report) + self.threads.append(threading.current_thread()) + + +@pytest.fixture +def handler() -> Handler: + return Handler() + + +@pytest.fixture +def delivery(handler: Handler) -> LatestDelivery[[int]]: + return LatestDelivery(handler, priority=DEFAULT_PRIORITY) + + +@pytest.mark.usefixtures("live_queue") +class TestTheNewestReportStandsForTheRest(BaseTestSuite): + """Reports filed between two drains reach the handler as the newest of them, once.""" + + def test_nothing_is_handled_before_the_drain(self, delivery: LatestDelivery[[int]], handler: Handler) -> None: + delivery.post(1) + + assert handler.handled == [] + + def test_a_drain_hands_over_the_newest_report_alone( + self, + delivery: LatestDelivery[[int]], + handler: Handler, + ) -> None: + for report in range(REPORTS): + delivery.post(report) + + draw_frame() + + assert handler.handled == [REPORTS - 1] + + def test_a_report_after_a_drain_waits_for_the_next_one( + self, + delivery: LatestDelivery[[int]], + handler: Handler, + ) -> None: + delivery.post(1) + draw_frame() + delivery.post(2) + delivery.post(3) + + draw_frame() + + assert handler.handled == [1, 3] + + def test_a_drain_with_no_report_hands_nothing_over( + self, + delivery: LatestDelivery[[int]], + handler: Handler, + ) -> None: + delivery.post(1) + draw_frame() + + draw_frame() + + assert handler.handled == [1] + + +@pytest.mark.usefixtures("live_queue") +class TestTheReportKeepsItsPlaceInTheQueue(BaseTestSuite): + """The delivery waits where its first report put it, so the run's other results keep their order.""" + + def test_what_is_posted_after_a_report_runs_after_the_newest_one( + self, + delivery: LatestDelivery[[int]], + handler: Handler, + ) -> None: + delivery.post(1) + CallbackQueue.add(handler.handled.append, OUTCOME, priority=DEFAULT_PRIORITY) + delivery.post(2) + + draw_frame() + + assert handler.handled == [2, OUTCOME] + + def test_what_is_posted_before_a_report_runs_before_it( + self, + delivery: LatestDelivery[[int]], + handler: Handler, + ) -> None: + CallbackQueue.add(handler.handled.append, OUTCOME, priority=DEFAULT_PRIORITY) + delivery.post(1) + + draw_frame() + + assert handler.handled == [OUTCOME, 1] + + +@pytest.mark.usefixtures("live_queue") +class TestReportsFromOtherThreads(BaseTestSuite): + """Workers file reports from threads of their own, and the thread draining the queue hands one over.""" + + def test_reports_from_several_workers_are_handled_once_on_the_draining_thread( + self, + delivery: LatestDelivery[[int]], + handler: Handler, + ) -> None: + workers = [ + threading.Thread(target=lambda: [delivery.post(report) for report in range(REPORTS)]) + for _ in range(WORKERS) + ] + for worker in workers: + worker.start() + for worker in workers: + worker.join() + + draw_frame() + + assert handler.handled == [REPORTS - 1] + assert handler.threads == [threading.current_thread()] From c620e2a90bf480d7e09394a352a2007d0f31c307 Mon Sep 17 00:00:00 2001 From: JakimPL Date: Tue, 13 Oct 2026 00:00:44 +0200 Subject: [PATCH 5/8] Skipped: a recording that fails in a batch, which goes on and names it at the end --- docs/development/progress.md | 10 +++ docs/guide/command-line.md | 3 +- .../logic/main/converter/run.py | 17 +++-- .../services/conversion/result.py | 5 +- .../services/conversion/service.py | 9 ++- .../headless/conversion/runners.py | 11 ++- .../headless/conversion/skipped.py | 25 +++++++ .../library/creator/creator.py | 5 +- .../parallelization/outcome.py | 17 ++++- .../parallelization/processor.py | 71 ++++++++++++++++--- .../reconstructions/converter/__init__.py | 3 + .../reconstructions/converter/conversion.py | 7 +- .../reconstructions/converter/converter.py | 26 ++++++- .../reconstructions/converter/outcome.py | 21 ++++++ .../parallelization/test_failed_tasks.py | 49 +++++++++++++ tests/suite/parallelization.py | 67 ++++++++++++++++- .../coordinators/tabs/test_main.py | 4 +- .../logic/main/converter/test_logic.py | 21 +++--- .../logic/main/converter/test_run.py | 28 ++++++-- .../headless/conversion/test_skipped.py | 49 +++++++++++++ .../converter/test_conversion.py | 12 ++-- .../converter/test_converter.py | 60 +++++++++++++++- 22 files changed, 456 insertions(+), 64 deletions(-) create mode 100644 src/sampletones_core/headless/conversion/skipped.py create mode 100644 src/sampletones_core/reconstructions/converter/outcome.py create mode 100644 tests/integration/sampletones_core/parallelization/test_failed_tasks.py create mode 100644 tests/unit/sampletones_core/headless/conversion/test_skipped.py diff --git a/docs/development/progress.md b/docs/development/progress.md index 83ad4382a..435042e17 100644 --- a/docs/development/progress.md +++ b/docs/development/progress.md @@ -116,6 +116,16 @@ arrived may be read after it. The run records the tasks it has counted (`TaskSte reports go. That keeps a task's own progress and the run's completed count from describing the same work twice, and it keeps the reading inside the run it describes. +### A batch goes on past a failing item + +A task fails by raising or by losing its worker. A run whose answers make sense only together ends with +the first failure: a library missing one instruction is no library. So does a run of one task, whose +failure is the whole of its outcome. A run of several tasks that stand on their own +(`skips_failed_tasks`), such as a folder converted one recording at a time, records the failure against +its task, logs it with its traceback, and goes on. One unreadable or crashing recording among thousands +then costs that recording alone. The completed outcome keeps the answers in task order and names every +task the run went on without, so whoever reports the end of the run names them too. + ### The pool's and the channel's lifetime The run's monitor thread (`TaskProcessor`) is the one owner of its pool. However the run ends, the monitor diff --git a/docs/guide/command-line.md b/docs/guide/command-line.md index 13449d239..107e3ed4f 100644 --- a/docs/guide/command-line.md +++ b/docs/guide/command-line.md @@ -50,7 +50,8 @@ A `convert` command takes either `--channels` or `--stems`. - **Convert one file**: `sampletones convert input.wav -o output.stn` - **Convert a folder**: `sampletones convert path/to/folder`. Every recording in the folder - becomes its own reconstruction, saved in your reconstructions folder. + becomes its own reconstruction, saved in your reconstructions folder. A recording that fails is + skipped, and the end of the run lists the skipped recordings with their errors. - **Mix several recordings into one reconstruction**: `sampletones convert bass.wav lead.wav --stems stems.json`. The stems file lists the recordings in the same order. [Reconstructions](../formats/reconstructions.md) shows it. diff --git a/src/sampletones_application/logic/main/converter/run.py b/src/sampletones_application/logic/main/converter/run.py index 48e7da89d..33218c5f1 100644 --- a/src/sampletones_application/logic/main/converter/run.py +++ b/src/sampletones_application/logic/main/converter/run.py @@ -17,7 +17,7 @@ from sampletones_core.configs import Config from sampletones_core.library import InstructionLibraryKey, LibraryState from sampletones_core.parallelization import TaskProgress -from sampletones_core.reconstructions.converter import ConversionPlan +from sampletones_core.reconstructions.converter import ConversionOutcome, ConversionPlan from sampletones_shared.types.callback import VoidCallback from sampletones_shared.utils.callbacks import CallbackMixin @@ -26,12 +26,14 @@ @dataclass(frozen=True) class ConversionSuccess: - """The outcome a completed conversion hands to its listener: the reconstructions it wrote. + """The outcome a completed conversion hands to its listener: the reconstructions it wrote, and + the recordings it skipped because they failed. One written reconstruction is one the reader can open straight away; several are a batch, which the reader reaches as a folder.""" written: Tuple[Path, ...] + skipped: Tuple[Path, ...] @property def is_single(self) -> bool: @@ -194,8 +196,8 @@ def _on_service_result(self, result: ConversionResult) -> None: self._handle_progress_result(progress) case ServiceIntermediate(data=progress): self._handle_library_progress(progress) - case ServiceSuccess(value=written): - self._settle_as_complete(written) + case ServiceSuccess(value=outcome): + self._settle_as_complete(outcome) case ServiceError(exception=exception): self._settle_as_failed(exception) case ServiceCanceled(): @@ -229,15 +231,16 @@ def _handle_library_progress(self, progress: TaskProgress) -> None: total = max(progress.total, 1) self._report(self._messages.preparing_library(request.library_state), progress.completed / total) - def _settle_as_complete(self, written: Tuple[Path, ...]) -> None: + def _settle_as_complete(self, outcome: ConversionOutcome) -> None: """Settles a finished run, telling its listener what was written before reporting. The reconstruction a run wrote is what a reader is then looking at, so the listener takes it up first and the report that follows names it. """ - self._written = written + self._written = outcome.written self._phase = ConversionPhase.COMPLETED - self.call(self.on_success, ConversionSuccess(written=written)) + skipped = tuple(source for skipped_job in outcome.skipped for source in skipped_job.job.sources) + self.call(self.on_success, ConversionSuccess(written=outcome.written, skipped=skipped)) self._report(self._messages.completed, 1.0) def _settle_as_failed(self, exception: Exception) -> None: diff --git a/src/sampletones_application/services/conversion/result.py b/src/sampletones_application/services/conversion/result.py index f53dd89a9..62ac721af 100644 --- a/src/sampletones_application/services/conversion/result.py +++ b/src/sampletones_application/services/conversion/result.py @@ -1,6 +1,6 @@ from dataclasses import dataclass from pathlib import Path -from typing import Optional, Tuple, Union +from typing import Optional, Union from sampletones_application.services.result import ( ServiceCanceled, @@ -11,6 +11,7 @@ ServiceSuccess, ) from sampletones_core.parallelization import TaskProgress +from sampletones_core.reconstructions.converter import ConversionOutcome from sampletones_core.reconstructions.stage import ReconstructionStage @@ -46,7 +47,7 @@ class ConversionItem: ServiceStarted, ServiceProgress[ConversionItem], ServiceIntermediate[TaskProgress], - ServiceSuccess[Tuple[Path, ...]], + ServiceSuccess[ConversionOutcome], ServiceError, ServiceCanceled, ] diff --git a/src/sampletones_application/services/conversion/service.py b/src/sampletones_application/services/conversion/service.py index 983843e42..6fcebd45d 100644 --- a/src/sampletones_application/services/conversion/service.py +++ b/src/sampletones_application/services/conversion/service.py @@ -1,5 +1,4 @@ -from pathlib import Path -from typing import Optional, Tuple +from typing import Optional from sampletones_application.services.base import ServiceBase from sampletones_application.services.conversion.result import ( @@ -19,7 +18,7 @@ from sampletones_core.configs import Config from sampletones_core.parallelization import ETAEstimator, TaskProgress, TaskStatus from sampletones_core.parallelization.task import TaskStep -from sampletones_core.reconstructions.converter import ConversionPlan, ReconstructionConverter +from sampletones_core.reconstructions.converter import ConversionOutcome, ConversionPlan, ReconstructionConverter from sampletones_core.reconstructions.stage import ReconstructionStage from sampletones_shared.logger import logger @@ -156,8 +155,8 @@ def _step(task_progress: TaskProgress) -> Optional[ReconstructionStep]: total=step.total, ) - def _on_completed(self, written: Tuple[Path, ...]) -> None: - self._emit(ServiceSuccess(value=written)) + def _on_completed(self, outcome: ConversionOutcome) -> None: + self._emit(ServiceSuccess(value=outcome)) def _on_error(self, exception: Exception) -> None: self._emit(ServiceError(exception=exception)) diff --git a/src/sampletones_core/headless/conversion/runners.py b/src/sampletones_core/headless/conversion/runners.py index 77454b670..bcf3b370f 100644 --- a/src/sampletones_core/headless/conversion/runners.py +++ b/src/sampletones_core/headless/conversion/runners.py @@ -5,11 +5,13 @@ from sampletones_core.configs import Config from sampletones_core.headless.conversion.request import ConversionRequest +from sampletones_core.headless.conversion.skipped import skipped_lines from sampletones_core.headless.library import ensure_library from sampletones_core.parallelization import TaskProgress, TaskStatus from sampletones_core.reconstructions import Reconstructor from sampletones_core.reconstructions.converter import ( ConversionJob, + ConversionOutcome, DirectoryConversion, ReconstructionConverter, reconstruct_job, @@ -83,7 +85,8 @@ def reconstruct_directory( """Reconstructs every recording under the directory, each alone under the setup's stem. The library the configuration names is prepared first. The results mirror the directory's - tree inside the configuration's reconstructions directory. + tree inside the configuration's reconstructions directory. A recording that fails is skipped, + and the run names every skipped recording, with its error, once it ends. Args: directory: The directory of recordings. @@ -97,9 +100,11 @@ def on_start() -> None: progress_bar.disable = False logger.info(f"Starting reconstruction for directory {directory}") - def on_completed(written: Tuple[Path, ...]) -> None: - logger.info(f"Reconstructed {len(written)} files from {directory}") + def on_completed(outcome: ConversionOutcome) -> None: progress_bar.close() + logger.info(f"Reconstructed {len(outcome.written)} files from {directory}") + for line in skipped_lines(outcome): + logger.warning(line) def on_progress( task_status: TaskStatus, diff --git a/src/sampletones_core/headless/conversion/skipped.py b/src/sampletones_core/headless/conversion/skipped.py new file mode 100644 index 000000000..bed6c5711 --- /dev/null +++ b/src/sampletones_core/headless/conversion/skipped.py @@ -0,0 +1,25 @@ +from typing import Final, Tuple + +from sampletones_core.reconstructions.converter import ConversionOutcome, SkippedJob + +ONE_RECORDING: Final[int] = 1 + + +def skipped_lines(outcome: ConversionOutcome) -> Tuple[str, ...]: + """What a finished run says about the recordings it went on without: a count, then one line apiece. + + A run that skipped nothing says nothing. + """ + if not outcome.skipped: + return () + + count = len(outcome.skipped) + noun = "recording" if count == ONE_RECORDING else "recordings" + return (f"Skipped {count} {noun} that failed:", *(describe_skipped(skipped) for skipped in outcome.skipped)) + + +def describe_skipped(skipped: SkippedJob) -> str: + """One skipped job: the recordings it read and the error that ended it.""" + sources = ", ".join(str(source) for source in skipped.job.sources) + exception = skipped.exception + return f" {sources}: {type(exception).__name__}: {exception}" diff --git a/src/sampletones_core/library/creator/creator.py b/src/sampletones_core/library/creator/creator.py index 9b3ad718a..53abcb84a 100644 --- a/src/sampletones_core/library/creator/creator.py +++ b/src/sampletones_core/library/creator/creator.py @@ -6,6 +6,7 @@ from sampletones_core.generators import GeneratorUnion, get_generators_map from sampletones_core.instructions import InstructionUnion from sampletones_core.parallelization import TaskProcessor +from sampletones_core.parallelization.outcome import RunCompleted from sampletones_shared.logger import LoggerProtocol from sampletones_shared.logger import logger as default_logger @@ -60,9 +61,9 @@ def _get_task_function( def _process_results( self, - results: List[Any], + completed: RunCompleted[Any], ) -> Tuple[InstructionLibraryKey, InstructionLibraryData]: - data: Dict[InstructionUnion, InstructionLibraryFragment[Any]] = dict(results) + data: Dict[InstructionUnion, InstructionLibraryFragment[Any]] = dict(completed.results) library_data = InstructionLibraryData.create(self.config, data) key = InstructionLibraryKey.create(self.config.library, self.window) diff --git a/src/sampletones_core/parallelization/outcome.py b/src/sampletones_core/parallelization/outcome.py index b85b833a0..690a82865 100644 --- a/src/sampletones_core/parallelization/outcome.py +++ b/src/sampletones_core/parallelization/outcome.py @@ -1,14 +1,27 @@ from dataclasses import dataclass -from typing import Generic, List, TypeVar, Union +from typing import Generic, List, Tuple, TypeVar, Union T = TypeVar("T") +@dataclass(frozen=True) +class TaskFailure: + """A task that ended in an error while its run went on, named by its place among the tasks.""" + + index: int + exception: Exception + + @dataclass(frozen=True) class RunCompleted(Generic[T]): - """A run whose every task answered, with the answers in the order the tasks were given.""" + """A run whose every task was answered. + + ``results`` holds the answers in the order the tasks were given. A run that outlives a failing + task holds that task in ``failures`` and leaves its answer out of ``results``. + """ results: List[T] + failures: Tuple[TaskFailure, ...] @dataclass(frozen=True) diff --git a/src/sampletones_core/parallelization/processor.py b/src/sampletones_core/parallelization/processor.py index 5081c6513..5c1bfebc5 100644 --- a/src/sampletones_core/parallelization/processor.py +++ b/src/sampletones_core/parallelization/processor.py @@ -2,7 +2,7 @@ import threading from abc import ABC, abstractmethod from concurrent.futures._base import CancelledError -from typing import Any, Callable, Final, Generic, List, Optional, TypeVar +from typing import Any, Callable, Final, Generic, Iterator, List, Optional, TypeVar from pebble import ProcessMapFuture, ProcessPool @@ -10,7 +10,13 @@ from sampletones_core.parallelization.channel.process import ProcessProgressChannel from sampletones_core.parallelization.channel.protocol import ProgressChannel, StepReporter from sampletones_core.parallelization.channel.pump import ProgressPump -from sampletones_core.parallelization.outcome import RunCanceled, RunCompleted, RunFailed, RunOutcome +from sampletones_core.parallelization.outcome import ( + RunCanceled, + RunCompleted, + RunFailed, + RunOutcome, + TaskFailure, +) from sampletones_core.parallelization.steps import TaskSteps from sampletones_core.parallelization.task import ( TaskProgress, @@ -33,6 +39,10 @@ class TaskProcessor(ABC, CallbackMixin, Generic[T]): The monitor thread builds the tasks, creates the pool, gathers the answers and, however the run ends, ends the pool and the progress channel before it announces the outcome. Whoever reacts to ``on_completed``, ``on_canceled`` or ``on_error`` therefore meets a run with no worker left. + + A task that raises, or whose worker dies, ends the run with that error. A run that + ``skips_failed_tasks`` records the failure against the task instead, logs it, and goes on, so + its completed outcome names every task it went on without. """ def __init__( @@ -111,6 +121,11 @@ def shutdown(self) -> None: def is_running(self) -> bool: return self.running + @property + def skips_failed_tasks(self) -> bool: + """Whether a failing task leaves the rest of the run going, which a run decides from its tasks.""" + return False + @abstractmethod def _create_tasks(self) -> List[Any]: ... @@ -118,7 +133,11 @@ def _create_tasks(self) -> List[Any]: ... def _get_task_function(self) -> Callback: ... @abstractmethod - def _process_results(self, results: List[T]) -> Any: ... + def _process_results(self, completed: RunCompleted[T]) -> Any: ... + + def _task_name(self, index: int) -> str: + """How the log names the task at ``index``.""" + return f"Task {index}" def _task_reporter(self, index: int) -> StepReporter: """The line the task at ``index`` reports its own progress on. @@ -192,7 +211,7 @@ def _gather(self) -> RunOutcome[T]: """Builds the tasks and runs them on a pool that has ended by the time this returns.""" try: tasks = self._create_tasks() - except Exception as exception: + except Exception as exception: # pylint: disable=broad-exception-caught return RunFailed(exception) if self.canceling: @@ -226,27 +245,57 @@ def _drain( self.call(self.on_start) results: List[T] = [] + failures: List[TaskFailure] = [] self.running = True self.status = TaskStatus.RUNNING self._notify_progress() try: iterator = self.future.result() while not self.canceling: - results.append(next(iterator)) + self._take_answer(iterator, results, failures) self._steps.complete(self.completed_tasks) self.completed_tasks += 1 self._notify_progress() except StopIteration: - return RunCompleted(results) + return RunCompleted(results, tuple(failures)) except (CancelledError, OperationCanceled): return RunCanceled() - except Exception as exception: + except Exception as exception: # pylint: disable=broad-exception-caught return RunFailed(exception) finally: self.running = False return RunCanceled() + def _take_answer( + self, + iterator: Iterator[T], + results: List[T], + failures: List[TaskFailure], + ) -> None: + """Takes the next task's answer, or the error it ended in where the run goes on without it. + + Raises: + StopIteration: Once every task has answered. + CancelledError: If the run was withdrawn. + OperationCanceled: If a task gave up on hearing the run was withdrawn. + Exception: The error a task ended in, where the run ends with it. + """ + try: + results.append(next(iterator)) + except (StopIteration, CancelledError, OperationCanceled): + raise + except Exception as exception: # pylint: disable=broad-exception-caught + if not self.skips_failed_tasks: + raise + + failure = TaskFailure(index=self.completed_tasks, exception=exception) + self.logger.error_with_traceback( + exception, + f"{self._task_name(failure.index)} failed, and the run goes on without it: {exception}", + ) + failures.append(failure) + def _end_pool( self, pool: ProcessPool, @@ -264,8 +313,8 @@ def _end_pool( def _announce(self, outcome: RunOutcome[T]) -> None: match outcome: - case RunCompleted(results=results): - self._finalize_completion(results) + case RunCompleted() as completed: + self._finalize_completion(completed) case RunCanceled(): self._finalize_cancellation() case RunFailed(exception=exception): @@ -293,13 +342,13 @@ def _finalize_cancellation(self) -> None: self._notify_progress() self.call(self.on_canceled) - def _finalize_completion(self, results: List[T]) -> None: + def _finalize_completion(self, completed: RunCompleted[T]) -> None: self.logger.info("Conversion completed successfully") self.status = TaskStatus.COMPLETED self._notify_progress() - processed_result = self._process_results(results) + processed_result = self._process_results(completed) self.call(self.on_completed, processed_result) def _stop_with_error(self, exception: Exception) -> None: diff --git a/src/sampletones_core/reconstructions/converter/__init__.py b/src/sampletones_core/reconstructions/converter/__init__.py index 345cd560a..f32a02de9 100644 --- a/src/sampletones_core/reconstructions/converter/__init__.py +++ b/src/sampletones_core/reconstructions/converter/__init__.py @@ -1,6 +1,7 @@ from .conversion import reconstruct_job from .converter import ReconstructionConverter from .job import ConversionJob +from .outcome import ConversionOutcome, SkippedJob from .plan import ( BatchConversion, BatchEntry, @@ -13,9 +14,11 @@ "BatchConversion", "BatchEntry", "ConversionJob", + "ConversionOutcome", "ConversionPlan", "DirectoryConversion", "GroupConversion", "ReconstructionConverter", + "SkippedJob", "reconstruct_job", ] diff --git a/src/sampletones_core/reconstructions/converter/conversion.py b/src/sampletones_core/reconstructions/converter/conversion.py index 0306ee774..c649b548c 100644 --- a/src/sampletones_core/reconstructions/converter/conversion.py +++ b/src/sampletones_core/reconstructions/converter/conversion.py @@ -2,7 +2,6 @@ from pathlib import Path from typing import Tuple -from sampletones_shared.exceptions import UnsupportedAudioFormatError from sampletones_shared.logger import logger from ..progress import ReconstructionReporter @@ -14,8 +13,8 @@ def reconstruct_job(arguments: Tuple[Reconstructor, ConversionJob, Reconstructio """Builds one job's reconstruction and writes it where the job says. Runs in a pool worker, so the job travels with the reconstructor that builds it and the - reporter it tells its progress to. A source in a format the loader has no reader for is - reported and left, which keeps one such file from ending a batch. + reporter it tells its progress to. An error the job meets ends the job, and the run decides + whether the jobs around it go on. Returns: The file the job named, whether or not a reconstruction reached it. @@ -35,8 +34,6 @@ def reconstruct_job(arguments: Tuple[Reconstructor, ConversionJob, Reconstructio except KeyboardInterrupt: logger.info("Reconstruction interrupted by user.") raise - except UnsupportedAudioFormatError: - logger.warning(f"Skipping job due to unsupported audio format: {job.sources}") finally: gc.collect() diff --git a/src/sampletones_core/reconstructions/converter/converter.py b/src/sampletones_core/reconstructions/converter/converter.py index dc4bc630d..f4e808223 100644 --- a/src/sampletones_core/reconstructions/converter/converter.py +++ b/src/sampletones_core/reconstructions/converter/converter.py @@ -4,6 +4,8 @@ from sampletones_core.configs import Config from sampletones_core.constants.enums import ChannelName from sampletones_core.parallelization import TaskProcessor +from sampletones_core.parallelization.outcome import RunCompleted +from sampletones_core.parallelization.task import SINGLE_TASK from sampletones_shared.logger import LoggerProtocol from sampletones_shared.logger import logger as default_logger @@ -11,6 +13,7 @@ from ..reconstructor.reconstructor import Reconstructor from .conversion import reconstruct_job from .job import ConversionJob +from .outcome import ConversionOutcome, SkippedJob from .plan.protocol import ConversionPlan from .progress import JobReporter @@ -21,6 +24,9 @@ class ReconstructionConverter(TaskProcessor[Path]): The plan is resolved once the run starts, on the monitor thread, so a plan that scans a directory does its reading there. Every job is then built by one worker, and the run reports the reconstructions it wrote. + + A run of several jobs, such as a folder converted one recording at a time, goes on past a job + that fails and reports it among the jobs it skipped. A run of one job ends with that job's error. """ def __init__( @@ -41,6 +47,11 @@ def start(self) -> None: super().start() + @property + def skips_failed_tasks(self) -> bool: + """A run holding several jobs goes on past a job that fails.""" + return len(self.jobs) > SINGLE_TASK + def _create_tasks(self) -> List[Any]: """One task per job, sharing the reconstructor the whole run's channels are built for.""" self.jobs = self.plan.jobs(self.config) @@ -56,9 +67,18 @@ def _get_task_function( ) -> Callable[[Tuple[Reconstructor, ConversionJob, ReconstructionReporter]], Path]: return reconstruct_job - def _process_results(self, results: List[Path]) -> Tuple[Path, ...]: - """The reconstructions the run wrote, in job order.""" - return tuple(output_path for output_path in results if output_path.exists()) + def _process_results(self, completed: RunCompleted[Path]) -> ConversionOutcome: + """The reconstructions the run wrote, in job order, and the jobs it went on without.""" + return ConversionOutcome( + written=tuple(output_path for output_path in completed.results if output_path.exists()), + skipped=tuple( + SkippedJob(job=self.jobs[failure.index], exception=failure.exception) for failure in completed.failures + ), + ) + + def _task_name(self, index: int) -> str: + """The recordings the job at ``index`` reads, which is how a reader finds it in the log.""" + return ", ".join(str(source) for source in self.jobs[index].sources) def _notify_progress(self) -> None: job = self.job_under_way(self.completed_tasks) diff --git a/src/sampletones_core/reconstructions/converter/outcome.py b/src/sampletones_core/reconstructions/converter/outcome.py new file mode 100644 index 000000000..66c47b45f --- /dev/null +++ b/src/sampletones_core/reconstructions/converter/outcome.py @@ -0,0 +1,21 @@ +from dataclasses import dataclass +from pathlib import Path +from typing import Tuple + +from .job import ConversionJob + + +@dataclass(frozen=True) +class SkippedJob: + """A job a run went on without, and the error that ended it.""" + + job: ConversionJob + exception: Exception + + +@dataclass(frozen=True) +class ConversionOutcome: + """What a finished run wrote, in job order, and the jobs it went on without.""" + + written: Tuple[Path, ...] + skipped: Tuple[SkippedJob, ...] diff --git a/tests/integration/sampletones_core/parallelization/test_failed_tasks.py b/tests/integration/sampletones_core/parallelization/test_failed_tasks.py new file mode 100644 index 000000000..b22a96209 --- /dev/null +++ b/tests/integration/sampletones_core/parallelization/test_failed_tasks.py @@ -0,0 +1,49 @@ +from typing import Final, List, Tuple + +import pytest +from pebble import ProcessExpired + +from sampletones_core.parallelization.outcome import RunCompleted +from tests.suite.base import BaseTestSuite +from tests.suite.parallelization import FatedProcessor, TaskFate + +TWO_WORKERS: Final[int] = 2 +FATES: Final[Tuple[TaskFate, ...]] = ( + TaskFate.ANSWERS, + TaskFate.RAISES, + TaskFate.ANSWERS, + TaskFate.DIES, + TaskFate.ANSWERS, +) + + +class TestARunGoingOnPastFailingTasks(BaseTestSuite): + """A run that skips failing tasks answers every other task and names each one that failed, whether the + task raised or its worker died. A run that does not skip them ends with the first error.""" + + def test_the_answers_and_the_failures_are_both_named(self) -> None: + completed: List[RunCompleted[int]] = [] + run = FatedProcessor(FATES, skips=True, max_workers=TWO_WORKERS) + run.set_callbacks(on_completed=completed.append) + + run.start() + run.wait() + + (outcome,) = completed + assert outcome.results == [0, 2, 4] + assert [failure.index for failure in outcome.failures] == [1, 3] + assert isinstance(outcome.failures[0].exception, ValueError) + assert isinstance(outcome.failures[1].exception, ProcessExpired) + + def test_a_run_holding_to_its_tasks_ends_with_the_first_error(self) -> None: + completed: List[RunCompleted[int]] = [] + errors: List[Exception] = [] + run = FatedProcessor(FATES, skips=False, max_workers=TWO_WORKERS) + run.set_callbacks(on_completed=completed.append, on_error=errors.append) + + run.start() + with pytest.raises(ValueError): + run.wait() + + assert completed == [] + assert [type(error) for error in errors] == [ValueError] diff --git a/tests/suite/parallelization.py b/tests/suite/parallelization.py index a7b11b48a..95ab3ba46 100644 --- a/tests/suite/parallelization.py +++ b/tests/suite/parallelization.py @@ -1,9 +1,12 @@ +import os import threading from dataclasses import dataclass +from enum import StrEnum from pathlib import Path from typing import Any, Callable, Final, FrozenSet, List, Optional, Tuple from sampletones_core.parallelization.channel.protocol import StepReporter +from sampletones_core.parallelization.outcome import RunCompleted from sampletones_core.parallelization.processor import TaskProcessor from sampletones_core.parallelization.task import TaskProgress, TaskStatus, TaskStep from sampletones_shared.exceptions import OperationCanceled @@ -15,6 +18,7 @@ NOTHING_COMPLETED: Final[int] = 0 WHOLE_TASK: Final[float] = 1.0 ONE_STEP: Final[int] = 1 +DIED_EXIT_CODE: Final[int] = 3 LIVE_STATUSES: Final[FrozenSet[TaskStatus]] = frozenset({TaskStatus.RUNNING, TaskStatus.COMPLETED}) @@ -81,8 +85,8 @@ def _create_tasks(self) -> List[Any]: def _get_task_function(self) -> Callable[[CountingTask], int]: return count_task - def _process_results(self, results: List[int]) -> Tuple[int, ...]: - return tuple(results) + def _process_results(self, completed: RunCompleted[int]) -> Tuple[int, ...]: + return tuple(completed.results) def failing_task(task: CountingTask) -> int: @@ -111,6 +115,65 @@ def _get_task_function(self) -> Callable[[CountingTask], int]: return failing_task +class TaskFate(StrEnum): + """How a task of a run ends: with an answer, with an error, or with its worker gone.""" + + ANSWERS = "answers" + RAISES = "raises" + DIES = "dies" + + +@dataclass(frozen=True) +class FatedTask: + """One task of a run whose ending the test decides.""" + + index: int + fate: TaskFate + + +def fated_task(task: FatedTask) -> int: + """Answers which task it is, raises, or ends its worker, as the task's fate says. + + Raises: + ValueError: Where the task's fate is to raise. + """ + match task.fate: + case TaskFate.RAISES: + raise ValueError(f"task {task.index} raised") + case TaskFate.DIES: + os._exit(DIED_EXIT_CODE) + + return task.index + + +class FatedProcessor(TaskProcessor[int]): + """A run whose tasks end the way the test lays out, and which goes on past a failing one where asked.""" + + def __init__( + self, + fates: Tuple[TaskFate, ...], + *, + skips: bool, + max_workers: int, + ) -> None: + super().__init__(max_workers=max_workers) + self._fates = fates + self._skips = skips + + @property + def skips_failed_tasks(self) -> bool: + return self._skips + + def _create_tasks(self) -> List[Any]: + return [FatedTask(index=index, fate=fate) for index, fate in enumerate(self._fates)] + + def _get_task_function(self) -> Callable[[FatedTask], int]: + return fated_task + + def _process_results(self, completed: RunCompleted[int]) -> RunCompleted[int]: + return completed + + class ProgressRecorder: """Keeps every reading a run offers, and waits for the one a test is after. diff --git a/tests/unit/sampletones_application/coordinators/tabs/test_main.py b/tests/unit/sampletones_application/coordinators/tabs/test_main.py index 92b24000a..1b47b8f83 100644 --- a/tests/unit/sampletones_application/coordinators/tabs/test_main.py +++ b/tests/unit/sampletones_application/coordinators/tabs/test_main.py @@ -255,7 +255,7 @@ def test_file_success_refreshes_and_offers_to_load(self) -> None: coordinator = _success_coordinator() output_path = Path("/reconstructions/kick.rcn") - coordinator._on_conversion_success(ConversionSuccess(written=(output_path,))) + coordinator._on_conversion_success(ConversionSuccess(written=(output_path,), skipped=())) coordinator._hooks.on_refresh_trees.assert_called_once_with() coordinator._dialogs.show_confirmation.assert_called_once() @@ -272,7 +272,7 @@ def test_a_batch_offers_to_open_the_folder(self) -> None: coordinator = _success_coordinator() written = (Path("/reconstructions/kick.rcn"), Path("/reconstructions/snare.rcn")) - coordinator._on_conversion_success(ConversionSuccess(written=written)) + coordinator._on_conversion_success(ConversionSuccess(written=written, skipped=())) _, kwargs = coordinator._dialogs.show_confirmation.call_args assert kwargs["ok_label"] == OPEN_BUTTON_KEY diff --git a/tests/unit/sampletones_application/logic/main/converter/test_logic.py b/tests/unit/sampletones_application/logic/main/converter/test_logic.py index eadd75ae6..e44e7a047 100644 --- a/tests/unit/sampletones_application/logic/main/converter/test_logic.py +++ b/tests/unit/sampletones_application/logic/main/converter/test_logic.py @@ -32,7 +32,7 @@ from sampletones_core.constants.enums import ChannelName, HierarchyMode from sampletones_core.library import LibraryState from sampletones_core.parallelization import TaskProgress -from sampletones_core.reconstructions.converter import ConversionJob, GroupConversion +from sampletones_core.reconstructions.converter import ConversionJob, ConversionOutcome, GroupConversion from sampletones_core.reconstructions.converter.paths import get_audio_files from tests.suite.base import BaseTestSuite from tests.suite.language import FakeLanguageManager @@ -144,6 +144,11 @@ def _card_channels(converter_logic: ConverterLogic) -> FrozenSet[ChannelName]: ) +def _written(*paths: Path) -> ConversionOutcome: + """The outcome of a run that wrote ``paths`` and skipped nothing.""" + return ConversionOutcome(written=paths, skipped=()) + + def _item_of(job: ConversionJob) -> ConversionItem: """The item the conversion service reports for ``job`` while the run works on it.""" return ConversionItem(source=job.sources[0], output_path=job.output_path) @@ -499,7 +504,7 @@ def test_a_gesture_reads_the_recordings_again(self, converter_logic: ConverterLo @pytest.mark.parametrize( "ending", [ - ServiceSuccess(value=(Path("/reconstructions/kick.stn"), Path("/reconstructions/snare.stn"))), + ServiceSuccess(value=_written(Path("/reconstructions/kick.stn"), Path("/reconstructions/snare.stn"))), ServiceError(exception=RuntimeError("a recording failed")), ServiceCanceled(), ], @@ -538,10 +543,10 @@ def test_success_carries_the_reconstructions_that_were_written( converter_logic.on_success = on_success written = (Path("/reconstructions/kick.stn"),) - _reports(service, ServiceSuccess(value=written)) + _reports(service, ServiceSuccess(value=_written(*written))) assert _phase(converter_logic) == ConversionPhase.COMPLETED - on_success.assert_called_once_with(ConversionSuccess(written=written)) + on_success.assert_called_once_with(ConversionSuccess(written=written, skipped=())) def test_one_written_reconstruction_becomes_the_displayed_output( self, @@ -550,7 +555,7 @@ def test_one_written_reconstruction_becomes_the_displayed_output( ) -> None: written = (Path("/reconstructions/kick.stn"),) - _reports(service, ServiceSuccess(value=written)) + _reports(service, ServiceSuccess(value=_written(*written))) assert _view(converter_logic).output_path == written[0] @@ -562,13 +567,13 @@ def test_a_batch_loads_the_folder_and_one_file_loads_itself( converter_logic.on_load_file = MagicMock() converter_logic.on_load_directory = MagicMock() - _reports(service, ServiceSuccess(value=(Path("/reconstructions/kick.stn"),))) + _reports(service, ServiceSuccess(value=_written(Path("/reconstructions/kick.stn")))) converter_logic.handle_load_request() converter_logic.on_load_file.assert_called_once_with(Path("/reconstructions/kick.stn")) _reports( service, - ServiceSuccess(value=(Path("/reconstructions/kick.stn"), Path("/reconstructions/snare.stn"))), + ServiceSuccess(value=_written(Path("/reconstructions/kick.stn"), Path("/reconstructions/snare.stn"))), ) converter_logic.handle_load_request() converter_logic.on_load_directory.assert_called_once_with() @@ -647,7 +652,7 @@ def test_a_settled_conversion_holds_nothing( converter_logic: ConverterLogic, service: MagicMock, ) -> None: - _reports(service, ServiceSuccess(value=(Path("/reconstructions/kick.stn"),))) + _reports(service, ServiceSuccess(value=_written(Path("/reconstructions/kick.stn")))) assert converter_logic.is_active is False diff --git a/tests/unit/sampletones_application/logic/main/converter/test_run.py b/tests/unit/sampletones_application/logic/main/converter/test_run.py index 69d2146c9..fc5782c2f 100644 --- a/tests/unit/sampletones_application/logic/main/converter/test_run.py +++ b/tests/unit/sampletones_application/logic/main/converter/test_run.py @@ -21,12 +21,17 @@ ) from sampletones_application.view_model.main.converter import ConversionPhase from sampletones_core.configs import Config +from sampletones_core.constants.enums import DEFAULT_CHANNELS from sampletones_core.library import LibraryState from sampletones_core.parallelization import TaskProgress +from sampletones_core.reconstructions.converter import ConversionJob, ConversionOutcome, SkippedJob +from sampletones_core.reconstructions.reconstructor.stems.configs.config import StemsConfig +from sampletones_core.reconstructions.reconstructor.stems.configs.settings import StemSettings from tests.suite.base import BaseTestSuite from tests.unit.sampletones_application.logic.main.converter.texts import messages WRITTEN: Tuple[Path, ...] = (Path("/reconstructions/kick.stn"),) +SKIPPED_SOURCE: Path = Path("/samples/snare.wav") def _request(library_state: LibraryState) -> ConversionRequest: @@ -105,7 +110,7 @@ def test_canceling_holds_resources_until_the_service_answers(self, driver: Drive @pytest.mark.parametrize( ("result", "phase"), [ - (ServiceSuccess(value=WRITTEN), ConversionPhase.COMPLETED), + (ServiceSuccess(value=ConversionOutcome(written=WRITTEN, skipped=())), ConversionPhase.COMPLETED), (ServiceError(exception=RuntimeError("boom")), ConversionPhase.FAILED), (ServiceCanceled(), ConversionPhase.CANCELED), ], @@ -217,11 +222,26 @@ class TestWhatACompletedRunHandsOver(BaseTestSuite): def test_success_carries_the_reconstructions_that_were_written(self, driver: Driver) -> None: driver.begin() - driver.reports_from_service(ServiceSuccess(value=WRITTEN)) + driver.reports_from_service(ServiceSuccess(value=ConversionOutcome(written=WRITTEN, skipped=()))) - driver.run.on_success.assert_called_once_with(ConversionSuccess(written=WRITTEN)) + driver.run.on_success.assert_called_once_with(ConversionSuccess(written=WRITTEN, skipped=())) assert driver.run.written == WRITTEN + def test_success_names_the_recordings_the_run_went_on_without(self, driver: Driver) -> None: + skipped = SkippedJob( + job=ConversionJob( + sources=(SKIPPED_SOURCE,), + stems=StemsConfig.single_entry(StemSettings.covering(list(DEFAULT_CHANNELS))), + output_path=SKIPPED_SOURCE.with_suffix(".stn"), + ), + exception=ZeroDivisionError("float division by zero"), + ) + driver.begin() + + driver.reports_from_service(ServiceSuccess(value=ConversionOutcome(written=WRITTEN, skipped=(skipped,)))) + + driver.run.on_success.assert_called_once_with(ConversionSuccess(written=WRITTEN, skipped=(SKIPPED_SOURCE,))) + def test_a_failure_hands_over_the_exception(self, driver: Driver) -> None: error = RuntimeError("boom") driver.begin() @@ -247,7 +267,7 @@ def test_a_request_given_up_before_the_service_took_it_cancels_all_the_same(self def test_closing_lets_the_written_reconstructions_go(self, driver: Driver) -> None: driver.begin() - driver.reports_from_service(ServiceSuccess(value=WRITTEN)) + driver.reports_from_service(ServiceSuccess(value=ConversionOutcome(written=WRITTEN, skipped=()))) driver.run.close() diff --git a/tests/unit/sampletones_core/headless/conversion/test_skipped.py b/tests/unit/sampletones_core/headless/conversion/test_skipped.py new file mode 100644 index 000000000..c3ee89735 --- /dev/null +++ b/tests/unit/sampletones_core/headless/conversion/test_skipped.py @@ -0,0 +1,49 @@ +from pathlib import Path +from typing import Final + +from pebble import ProcessExpired + +from sampletones_core.constants.enums import DEFAULT_CHANNELS +from sampletones_core.headless.conversion.request import classic_setup +from sampletones_core.headless.conversion.skipped import skipped_lines +from sampletones_core.reconstructions.converter import ConversionJob, ConversionOutcome, SkippedJob +from tests.suite.base import BaseTestSuite + +KICK: Final[Path] = Path("/samples/kick.wav") +SNARE: Final[Path] = Path("/samples/snare.wav") + + +def _skipped(source: Path, exception: Exception) -> SkippedJob: + job = ConversionJob( + sources=(source,), + stems=classic_setup(DEFAULT_CHANNELS), + output_path=source.with_suffix(".stn"), + ) + return SkippedJob(job=job, exception=exception) + + +class TestWhatARunSaysAboutTheRecordingsItSkipped(BaseTestSuite): + """A run that went on without some recordings counts them, then names each with its error.""" + + def test_a_run_that_skipped_nothing_says_nothing(self) -> None: + assert skipped_lines(ConversionOutcome(written=(KICK.with_suffix(".stn"),), skipped=())) == () + + def test_each_skipped_recording_is_named_with_its_error(self) -> None: + outcome = ConversionOutcome( + written=(), + skipped=( + _skipped(KICK, ZeroDivisionError("float division by zero")), + _skipped(SNARE, ProcessExpired("Abnormal termination")), + ), + ) + + assert skipped_lines(outcome) == ( + "Skipped 2 recordings that failed:", + f" {KICK}: ZeroDivisionError: float division by zero", + f" {SNARE}: ProcessExpired: Abnormal termination", + ) + + def test_one_skipped_recording_is_counted_as_one(self) -> None: + outcome = ConversionOutcome(written=(), skipped=(_skipped(KICK, ValueError("unreadable")),)) + + assert skipped_lines(outcome)[0] == "Skipped 1 recording that failed:" diff --git a/tests/unit/sampletones_core/reconstructions/converter/test_conversion.py b/tests/unit/sampletones_core/reconstructions/converter/test_conversion.py index a2246b2f2..f12fc3f09 100644 --- a/tests/unit/sampletones_core/reconstructions/converter/test_conversion.py +++ b/tests/unit/sampletones_core/reconstructions/converter/test_conversion.py @@ -14,7 +14,6 @@ from sampletones_core.reconstructions.reconstructor.reconstructor import Reconstructor from sampletones_core.reconstructions.reconstructor.stems.configs.config import StemsConfig from sampletones_core.reconstructions.reconstructor.stems.configs.settings import StemSettings -from sampletones_shared.exceptions import UnsupportedAudioFormatError from sampletones_shared.utils.progress import silent_reporter CHANNELS: Final[List[ChannelName]] = list(DEFAULT_CHANNELS) @@ -86,15 +85,20 @@ def test_always_returns_output_path( job = _job(tmp_path, output_path) assert reconstruct_job((mock_reconstructor, job, silent_reporter)) == output_path - def test_unsupported_audio_format_error_is_swallowed( + def test_an_error_the_job_meets_reaches_the_run( self, mock_reconstructor: MagicMock, tmp_path: Path, ) -> None: - mock_reconstructor.reconstruct.side_effect = UnsupportedAudioFormatError("bad format") + """The run decides whether the jobs around a failing one go on, so the job hands its error up.""" + mock_reconstructor.reconstruct.side_effect = ZeroDivisionError("float division by zero") output_path = tmp_path / "song.stn" job = _job(tmp_path, output_path) - assert reconstruct_job((mock_reconstructor, job, silent_reporter)) == output_path + + with pytest.raises(ZeroDivisionError): + reconstruct_job((mock_reconstructor, job, silent_reporter)) + + assert not output_path.exists() def test_keyboard_interrupt_is_reraised( self, diff --git a/tests/unit/sampletones_core/reconstructions/converter/test_converter.py b/tests/unit/sampletones_core/reconstructions/converter/test_converter.py index 2932bf149..16013248b 100644 --- a/tests/unit/sampletones_core/reconstructions/converter/test_converter.py +++ b/tests/unit/sampletones_core/reconstructions/converter/test_converter.py @@ -7,10 +7,13 @@ from sampletones_core.constants.enums import ( DEFAULT_CHANNELS, ) +from sampletones_core.parallelization.outcome import RunCompleted, TaskFailure from sampletones_core.reconstructions.converter import ( + ConversionOutcome, DirectoryConversion, GroupConversion, ReconstructionConverter, + SkippedJob, reconstruct_job, ) from sampletones_core.reconstructions.reconstructor.stems.configs.config import StemsConfig @@ -34,6 +37,16 @@ def _group(path: Path, stems: StemsConfig) -> GroupConversion: return GroupConversion(sources=(path,), stems=stems) +def _directory_of(directory: Path, stems: StemsConfig, *names: str) -> ReconstructionConverter: + """A converter over a folder holding ``names``, with its jobs read from the folder.""" + for name in names: + (directory / name).touch() + converter = ReconstructionConverter(Config(), DirectoryConversion(directory=directory, stems=stems)) + with patch(_RECONSTRUCTOR_PATCH): + converter._create_tasks() + return converter + + class TestReconstructionConverterInit: def test_stores_config_as_copy(self, config: Config, stems: StemsConfig, tmp_path: Path) -> None: converter = ReconstructionConverter(config, _group(tmp_path / "song.wav", stems)) @@ -162,7 +175,10 @@ def test_reports_the_reconstructions_that_were_written( written = tmp_path / "a.stn" written.touch() converter = ReconstructionConverter(config, _group(tmp_path / "song.wav", stems)) - assert converter._process_results([written]) == (written,) + assert converter._process_results(RunCompleted([written], ())) == ConversionOutcome( + written=(written,), + skipped=(), + ) def test_leaves_out_a_job_that_wrote_nothing( self, @@ -170,12 +186,50 @@ def test_leaves_out_a_job_that_wrote_nothing( stems: StemsConfig, tmp_path: Path, ) -> None: - """A source the loader has no reader for names its output and writes none, so the run + """A reconstruction that came to nothing names its output and writes none, so the run reports only what a reader can open.""" written = tmp_path / "a.stn" written.touch() converter = ReconstructionConverter(config, _group(tmp_path / "song.wav", stems)) - assert converter._process_results([written, tmp_path / "missing.stn"]) == (written,) + assert converter._process_results(RunCompleted([written, tmp_path / "missing.stn"], ())).written == (written,) + + def test_names_each_job_the_run_went_on_without(self, stems: StemsConfig, tmp_path: Path) -> None: + converter = _directory_of(tmp_path, stems, "a.wav", "b.wav", "c.wav") + error = ZeroDivisionError("float division by zero") + written = converter.jobs[0].output_path + written.parent.mkdir(parents=True, exist_ok=True) + written.touch() + + outcome = converter._process_results(RunCompleted([written], (TaskFailure(index=1, exception=error),))) + + assert outcome == ConversionOutcome( + written=(written,), + skipped=(SkippedJob(job=converter.jobs[1], exception=error),), + ) + + +class TestWhichRunsGoOnPastAFailingJob: + """A run of several jobs goes on past a job that fails, and a run of one ends with it.""" + + def test_a_folder_of_several_recordings_goes_on(self, stems: StemsConfig, tmp_path: Path) -> None: + assert _directory_of(tmp_path, stems, "a.wav", "b.wav").skips_failed_tasks + + def test_a_folder_of_one_recording_ends_with_it(self, stems: StemsConfig, tmp_path: Path) -> None: + assert not _directory_of(tmp_path, stems, "a.wav").skips_failed_tasks + + def test_a_mix_ends_with_its_job(self, config: Config, stems: StemsConfig, tmp_path: Path) -> None: + audio_file = tmp_path / "song.wav" + audio_file.touch() + converter = ReconstructionConverter(config, _group(audio_file, stems)) + with patch(_RECONSTRUCTOR_PATCH): + converter._create_tasks() + + assert not converter.skips_failed_tasks + + def test_the_log_names_a_job_by_its_recordings(self, stems: StemsConfig, tmp_path: Path) -> None: + converter = _directory_of(tmp_path, stems, "a.wav", "b.wav") + + assert converter._task_name(1) == str(converter.jobs[1].sources[0]) class TestReconstructionConverterNotifyProgress: From 604fcbb5e87153bd5498e877da4c5742cb0f7070 Mon Sep 17 00:00:00 2001 From: JakimPL Date: Tue, 13 Oct 2026 01:10:12 +0200 Subject: [PATCH 6/8] Named: the recordings a batch skipped in the dialog at its end --- docs/development/bugs-and-todos.md | 3 - docs/guide/converting.md | 3 + .../categories/conversion.py | 66 ++++++++++ .../coordinators/tabs/main.py | 31 ++++- src/sampletones_config/lang/en.yaml | 5 + tests/screens/main/run/test_skipping.py | 123 ++++++++++++++++++ tests/suite/screens/seeds/recordings.py | 13 ++ .../categories/test_conversion.py | 60 +++++++++ .../coordinators/tabs/test_main.py | 38 ++++++ 9 files changed, 338 insertions(+), 4 deletions(-) create mode 100644 src/sampletones_application/categories/conversion.py create mode 100644 tests/screens/main/run/test_skipping.py create mode 100644 tests/unit/sampletones_application/categories/test_conversion.py diff --git a/docs/development/bugs-and-todos.md b/docs/development/bugs-and-todos.md index 4e667bb05..2025272bd 100644 --- a/docs/development/bugs-and-todos.md +++ b/docs/development/bugs-and-todos.md @@ -46,9 +46,6 @@ dimension the import starts carrying. * Waveform construction preview: a waveform drawn from the partial reconstruction while the converter reconstructs a single file. * Picking several rows of the converter's list at once, so a group leaves or settles in one gesture. -* An option to skip a recording that fails in a run of one reconstruction per recording, on the Converter - card and on `sampletones convert`. A run ends at its first failure, so one broken file stops a whole - folder. * Selecting parts of a reconstruction's waveform to crop, trim, cut and paste them along with their instructions. * Marking broken files where the browsers list them: a reconstruction that fails to load, one whose diff --git a/docs/guide/converting.md b/docs/guide/converting.md index 219844014..e20fdcf5d 100644 --- a/docs/guide/converting.md +++ b/docs/guide/converting.md @@ -113,6 +113,9 @@ button reads **Open**. It takes you to the **Reconstruction** tab, where you pic If the open reconstruction has unsaved changes, **Load** asks what to do with them. If the conversion replaced that same file, click **Cancel** and use **Save reconstruction as...** to keep your changes. +If a recording can't be converted, the conversion skips it and goes on with the rest. The dialog at +the end lists the recordings it skipped. + The first conversion with new settings takes longer, because it builds the [instruction library](../concepts/instruction-library.md) for them. diff --git a/src/sampletones_application/categories/conversion.py b/src/sampletones_application/categories/conversion.py new file mode 100644 index 000000000..e6d65a7e7 --- /dev/null +++ b/src/sampletones_application/categories/conversion.py @@ -0,0 +1,66 @@ +from dataclasses import dataclass +from pathlib import Path +from typing import Final, Optional, Self, Tuple + +from sampletones_application.categories.manager import LanguageManager +from sampletones_application.categories.skipped import ROW_BULLET + +MAX_REPORTED_RECORDINGS: Final[int] = 8 +ONE_RECORDING: Final[int] = 1 + + +@dataclass(frozen=True) +class SkippedRecordingMessages: + """The words a batch reports the recordings it went on without in. + + A batch converts each recording on its own, so a recording that fails is skipped and the rest are + converted. The report counts the skipped recordings, names each by its file name, and closes on how + many more it leaves out. The log keeps the error each one ended in. + + Attributes: + one: The heading over a single skipped recording. + several: The heading counting several skipped recordings. + more: The line naming the recordings past the ones listed. + """ + + one: str + several: str + more: str + + @classmethod + def build(cls, language_manager: LanguageManager) -> Self: + """Resolves every word the report prints. + + Args: + language_manager: The catalog the words are read from. + + Returns: + Self: The bundle the end of a conversion reads. + """ + return cls( + one=language_manager["main.converter.message.skipped_recording_heading"], + several=language_manager["main.converter.template.skipped_recordings_heading"], + more=language_manager["main.converter.template.skipped_recordings_more"], + ) + + def notice(self, skipped: Tuple[Path, ...]) -> Optional[str]: + """Phrases the recordings a batch skipped, under a heading that counts them. + + Args: + skipped: The recordings the batch went on without, in the order it reached them. + + Returns: + Optional[str]: The paragraph the end of a conversion appends, and ``None`` where every + recording was converted. + """ + if not skipped: + return None + + listed = skipped[:MAX_REPORTED_RECORDINGS] + heading = self.one if len(skipped) == ONE_RECORDING else self.several.format(count=len(skipped)) + lines = [heading, *(f"{ROW_BULLET}{recording.name}" for recording in listed)] + remaining = len(skipped) - len(listed) + if remaining > 0: + lines.append(self.more.format(count=remaining)) + + return "\n".join(lines) diff --git a/src/sampletones_application/coordinators/tabs/main.py b/src/sampletones_application/coordinators/tabs/main.py index 2e88e60d5..385336b2a 100644 --- a/src/sampletones_application/coordinators/tabs/main.py +++ b/src/sampletones_application/coordinators/tabs/main.py @@ -3,7 +3,9 @@ import dearpygui.dearpygui as dpg +from sampletones_application.categories.conversion import SkippedRecordingMessages from sampletones_application.categories.manager import LanguageManager +from sampletones_application.categories.skipped import PARAGRAPH_BREAK from sampletones_application.config.managers.config import ConfigManager from sampletones_application.config.managers.session import SessionManager from sampletones_application.constants.output import OutputKind @@ -131,6 +133,7 @@ def __init__( self._side_panel_count: int self._config_height = layout.config_height self._ttl_progress = language_manager["main.converter.title.progress_dialog"] + self._skipped_recording_messages = SkippedRecordingMessages.build(language_manager) self._repaint_priority = layout.scheduling.priorities.gui_action self._build_explorer( @@ -519,7 +522,16 @@ def _confirm_overwriting_target(self, targets: Tuple[Path, ...]) -> None: ) def _on_conversion_success(self, success: ConversionSuccess) -> None: + """Offers what the run wrote, with the recordings it skipped named beneath the offer. + + A run that skipped every recording it held has nothing to offer, so it reports the recordings + it skipped and the converter returns to idle. + """ self._hooks.on_refresh_trees() + if success.skipped and not success.written: + self._report_nothing_written(success) + return + if success.is_single: message = self._language_manager["main.converter.message.load_file_prompt"] ok_label = self._language_manager["main.converter.label.load_button"] @@ -531,7 +543,7 @@ def _on_conversion_success(self, success: ConversionSuccess) -> None: self._dialogs.show_confirmation( TAG_MAIN_CONVERTER_DIALOG_LOAD, - message, + self._with_skipped(message, success), self._language_manager["main.converter.title.load_dialog"], self._converter_logic.handle_load_request, ok_label=ok_label, @@ -540,6 +552,23 @@ def _on_conversion_success(self, success: ConversionSuccess) -> None: on_cancel=self._converter_logic.close, ) + def _report_nothing_written(self, success: ConversionSuccess) -> None: + self._converter_logic.close() + self._dialogs.show_info( + self._converter_panel.tag, + self._with_skipped(self._language_manager["main.converter.message.nothing_written"], success), + self._language_manager["main.converter.title.nothing_written_dialog"], + modal=True, + ) + + def _with_skipped(self, message: str, success: ConversionSuccess) -> str: + """``message`` followed by the paragraph naming the recordings the run skipped, where it skipped any.""" + notice = self._skipped_recording_messages.notice(success.skipped) + if notice is None: + return message + + return PARAGRAPH_BREAK.join((message, notice)) + def _request_output(self, output: OutputKind) -> None: """Answers the output switch, asking which recordings to mix where the list overflows one. diff --git a/src/sampletones_config/lang/en.yaml b/src/sampletones_config/lang/en.yaml index 27a6f7690..c02135a69 100644 --- a/src/sampletones_config/lang/en.yaml +++ b/src/sampletones_config/lang/en.yaml @@ -428,6 +428,11 @@ main.converter.title.load_dialog: "Reconstruction complete" main.converter.title.cancel_dialog: "Cancel conversion?" main.converter.message.load_file_prompt: "The reconstruction is ready. Load it now?" main.converter.message.load_directory_prompt: "The reconstructions are ready. Open the Reconstruction tab?" +main.converter.message.skipped_recording_heading: "This recording couldn't be converted, so it was skipped:" +main.converter.template.skipped_recordings_heading: "{count} recordings couldn't be converted, so they were skipped:" +main.converter.template.skipped_recordings_more: "and {count} more" +main.converter.title.nothing_written_dialog: "Nothing converted" +main.converter.message.nothing_written: "No reconstruction was written." main.converter.message.cancel_prompt: "Stop the current reconstruction?" main.converter.template.progress_template: "Progress: {}/{} files" main.converter.template.stage_template: " — {stage} {completed}/{total}" diff --git a/tests/screens/main/run/test_skipping.py b/tests/screens/main/run/test_skipping.py new file mode 100644 index 000000000..a0838da11 --- /dev/null +++ b/tests/screens/main/run/test_skipping.py @@ -0,0 +1,123 @@ +import operator +from pathlib import Path +from typing import Final, List + +import pytest + +from automation.screen import Screen +from automation.steps.main import gather, home_path +from automation.worlds.home import World +from tests.screens.main.run.conftest import recording +from tests.screens.main.run.constants import RUN_TIMEOUT_SECONDS +from tests.screens.main.run.steps import written +from tests.suite.screens.seeds.recordings import UnreadableRecording +from tests.suite.screens.worlds.recordings import converting_world + +TAKES: Final[str] = "Takes" +BROKEN_TAKES: Final[str] = "Broken" +FIRST: Final[str] = "first.wav" +SECOND: Final[str] = "second.wav" +BROKEN: Final[str] = "broken.wav" +DAMAGED: Final[str] = "damaged.wav" +SKIPPED_ONE: Final[str] = "main.converter.message.skipped_recording_heading" +SKIPPED_SEVERAL: Final[str] = "main.converter.template.skipped_recordings_heading" +NOTHING_WRITTEN: Final[str] = "main.converter.message.nothing_written" + + +class TestABatchGoesOnPastABrokenRecording: + """A folder holding a recording no reader takes converts the rest, and the end names the one it skipped. + + The folder is gathered and converted. The end of the run offers what it wrote and lists the broken + recording under its heading. The reconstructions of the readable recordings are written, and the + broken one has none. + """ + + @pytest.fixture + def world(self) -> World: + folder = home_path(TAKES) + return converting_world( + ( + recording(folder / FIRST), + UnreadableRecording(destination=folder / BROKEN), + recording(folder / SECOND), + ) + ) + + def test_the_rest_is_written_and_the_end_names_the_broken_one(self, screen: Screen) -> None: + converter = screen.main.converter + destinations: List[Path] = [] + + def convert(screen: Screen) -> None: + gather(screen, home_path(TAKES)) + destinations.append(Path(converter.destination())) + + converter.press_action() + + screen.claim_error(BROKEN) + screen.bridge.expect( + converter.end_prompt.is_shown, + bool, + description="the run's end", + timeout=RUN_TIMEOUT_SECONDS, + ) + + def the_end_names_it(screen: Screen) -> None: + words = converter.end_prompt.words() + converter.end_prompt.cancel() + screen.expect(converter.end_prompt.is_shown, operator.not_, description="the end closed") + + assert screen.words(SKIPPED_ONE) in words + assert BROKEN in words + assert FIRST not in words + assert sorted(path.stem for path in written(destinations[0])) == [Path(FIRST).stem, Path(SECOND).stem] + + screen.scenario(convert, the_end_names_it).run() + + +class TestABatchOfBrokenRecordingsWritesNothing: + """A folder whose every recording is broken writes nothing, and its notice says so and names them all. + + The notice offers nothing to load, and the button names the run again once it is dismissed. + """ + + @pytest.fixture + def world(self) -> World: + folder = home_path(BROKEN_TAKES) + return converting_world( + ( + UnreadableRecording(destination=folder / BROKEN), + UnreadableRecording(destination=folder / DAMAGED), + ) + ) + + def test_the_notice_names_every_broken_recording(self, screen: Screen) -> None: + converter = screen.main.converter + notice = converter.notice + destinations: List[Path] = [] + named: List[str] = [] + + def convert(screen: Screen) -> None: + gather(screen, home_path(BROKEN_TAKES)) + destinations.append(Path(converter.destination())) + named.append(converter.action()) + + converter.press_action() + + screen.claim_error(BROKEN) + screen.claim_error(DAMAGED) + screen.bridge.expect(notice.is_shown, bool, description="the notice", timeout=RUN_TIMEOUT_SECONDS) + + def the_notice_names_them(screen: Screen) -> None: + words = notice.words() + notice.dismiss() + screen.expect(notice.is_shown, operator.not_, description="the notice dismissed") + + assert words.startswith(screen.words(NOTHING_WRITTEN)) + assert screen.words(SKIPPED_SEVERAL).format(count=2) in words + assert BROKEN in words + assert DAMAGED in words + assert not converter.end_prompt.is_shown() + assert converter.action() == named[0] + assert written(destinations[0]) == [] + + screen.scenario(convert, the_notice_names_them).run() diff --git a/tests/suite/screens/seeds/recordings.py b/tests/suite/screens/seeds/recordings.py index a3deb9d33..f50c46a1a 100644 --- a/tests/suite/screens/seeds/recordings.py +++ b/tests/suite/screens/seeds/recordings.py @@ -13,6 +13,7 @@ STORED_RECORDING_SECONDS: Final[float] = 0.5 STORED_RECORDING_FREQUENCY: Final[float] = 220.0 AUDIO_PATH_FIELD: Final[str] = "audio_filepath" +UNREADABLE_BYTES: Final[bytes] = b"these bytes hold no sound" @dataclass(frozen=True) @@ -31,6 +32,18 @@ def write(self) -> None: soundfile.write(self.destination, tone, RECORDING_SAMPLE_RATE) +@dataclass(frozen=True) +class UnreadableRecording: + """A file in the home named like a recording, whose bytes no audio reader takes.""" + + destination: Path + + def write(self) -> None: + """Writes bytes that hold no sound under the recording's name.""" + self.destination.parent.mkdir(parents=True, exist_ok=True) + self.destination.write_bytes(UNREADABLE_BYTES) + + def stored_recording() -> Recording: """The recording the archived reconstruction names, laid where its relative path leads from the home. diff --git a/tests/unit/sampletones_application/categories/test_conversion.py b/tests/unit/sampletones_application/categories/test_conversion.py new file mode 100644 index 000000000..349af2e66 --- /dev/null +++ b/tests/unit/sampletones_application/categories/test_conversion.py @@ -0,0 +1,60 @@ +from pathlib import Path +from typing import Final, Tuple + +import pytest + +from sampletones_application.categories.conversion import MAX_REPORTED_RECORDINGS, SkippedRecordingMessages +from sampletones_application.categories.skipped import ROW_BULLET +from tests.suite.base import BaseTestSuite +from tests.suite.language import FakeLanguageManager + +ONE: Final[str] = "one skipped:" +SEVERAL: Final[str] = "{count} skipped:" +MORE: Final[str] = "and {count} more" +FOLDER: Final[Path] = Path("/samples/drums") + + +def _recordings(count: int) -> Tuple[Path, ...]: + return tuple(FOLDER / f"hit {index:03d}.wav" for index in range(count)) + + +@pytest.fixture(name="messages") +def messages_fixture() -> SkippedRecordingMessages: + return SkippedRecordingMessages.build( + FakeLanguageManager( + { + "main.converter.message.skipped_recording_heading": ONE, + "main.converter.template.skipped_recordings_heading": SEVERAL, + "main.converter.template.skipped_recordings_more": MORE, + } + ) + ) + + +class TestTheRecordingsABatchSkipped(BaseTestSuite): + """The end of a batch counts the recordings it skipped, names each by file, and cuts a long list.""" + + def test_a_batch_that_skipped_nothing_says_nothing(self, messages: SkippedRecordingMessages) -> None: + assert messages.notice(()) is None + + def test_one_skipped_recording_stands_under_its_own_heading(self, messages: SkippedRecordingMessages) -> None: + assert messages.notice(_recordings(1)) == f"{ONE}\n{ROW_BULLET}hit 000.wav" + + def test_several_are_counted_and_named_by_file(self, messages: SkippedRecordingMessages) -> None: + assert messages.notice(_recordings(2)) == f"2 skipped:\n{ROW_BULLET}hit 000.wav\n{ROW_BULLET}hit 001.wav" + + def test_a_long_list_is_cut_and_counts_the_rest(self, messages: SkippedRecordingMessages) -> None: + extra = 3 + notice = messages.notice(_recordings(MAX_REPORTED_RECORDINGS + extra)) + + assert notice is not None + lines = notice.split("\n") + assert lines[0] == f"{MAX_REPORTED_RECORDINGS + extra} skipped:" + assert len(lines) == MAX_REPORTED_RECORDINGS + 2 + assert lines[-1] == f"and {extra} more" + + def test_a_list_that_fits_names_every_recording(self, messages: SkippedRecordingMessages) -> None: + notice = messages.notice(_recordings(MAX_REPORTED_RECORDINGS)) + + assert notice is not None + assert len(notice.split("\n")) == MAX_REPORTED_RECORDINGS + 1 diff --git a/tests/unit/sampletones_application/coordinators/tabs/test_main.py b/tests/unit/sampletones_application/coordinators/tabs/test_main.py index 1b47b8f83..f9abeb213 100644 --- a/tests/unit/sampletones_application/coordinators/tabs/test_main.py +++ b/tests/unit/sampletones_application/coordinators/tabs/test_main.py @@ -5,6 +5,8 @@ import pytest +from sampletones_application.categories.conversion import SkippedRecordingMessages +from sampletones_application.categories.skipped import ROW_BULLET from sampletones_application.constants.conversion import MAX_STEM_SOURCES from sampletones_application.constants.output import OutputKind from sampletones_application.coordinators.tabs.hooks import MainTabHooks @@ -41,6 +43,10 @@ LOAD_BUTTON_KEY: Final[str] = "main.converter.label.load_button" OPEN_BUTTON_KEY: Final[str] = "main.converter.label.open_button" CLOSE_BUTTON_KEY: Final[str] = "main.converter.label.close_button" +SKIPPED_HEADING_KEY: Final[str] = "main.converter.message.skipped_recording_heading" +NOTHING_WRITTEN_KEY: Final[str] = "main.converter.message.nothing_written" +NOTHING_WRITTEN_TITLE_KEY: Final[str] = "main.converter.title.nothing_written_dialog" +SKIPPED_RECORDING: Final[Path] = Path("/samples/broken.wav") STOP_BUTTON_KEY: Final[str] = "main.converter.label.stop_button" CONTINUE_BUTTON_KEY: Final[str] = "main.converter.label.continue_button" NOTHING_BELOW_KEY: Final[str] = "main.converter.message.scan_nothing_below" @@ -243,7 +249,9 @@ def _success_coordinator() -> MainTabCoordinator: coordinator._dialogs = dialogs_on_the_line() coordinator._hooks = _hooks(operation_active=False) coordinator._converter_logic = MagicMock() + coordinator._converter_panel = MagicMock() coordinator._language_manager = FakeLanguageManager() + coordinator._skipped_recording_messages = SkippedRecordingMessages.build(FakeLanguageManager()) return coordinator @@ -278,6 +286,36 @@ def test_a_batch_offers_to_open_the_folder(self) -> None: assert kwargs["ok_label"] == OPEN_BUTTON_KEY assert kwargs["path"] is None + def test_the_offer_names_the_recordings_the_run_skipped_beneath_it(self) -> None: + coordinator = _success_coordinator() + written = (Path("/reconstructions/kick.rcn"),) + + coordinator._on_conversion_success(ConversionSuccess(written=written, skipped=(SKIPPED_RECORDING,))) + + args, _ = coordinator._dialogs.show_confirmation.call_args + assert args[1] == f"{LOAD_FILE_MESSAGE_KEY}\n\n{SKIPPED_HEADING_KEY}\n{ROW_BULLET}{SKIPPED_RECORDING.name}" + + def test_a_run_that_wrote_nothing_reports_what_it_skipped_and_offers_nothing(self) -> None: + coordinator = _success_coordinator() + + coordinator._on_conversion_success(ConversionSuccess(written=(), skipped=(SKIPPED_RECORDING,))) + + coordinator._dialogs.show_confirmation.assert_not_called() + coordinator._converter_logic.close.assert_called_once_with() + args, kwargs = coordinator._dialogs.show_info.call_args + assert args[1] == f"{NOTHING_WRITTEN_KEY}\n\n{SKIPPED_HEADING_KEY}\n{ROW_BULLET}{SKIPPED_RECORDING.name}" + assert args[2] == NOTHING_WRITTEN_TITLE_KEY + assert kwargs["modal"] is True + + def test_a_run_that_wrote_nothing_and_skipped_nothing_still_offers_the_folder(self) -> None: + coordinator = _success_coordinator() + + coordinator._on_conversion_success(ConversionSuccess(written=(), skipped=())) + + coordinator._dialogs.show_info.assert_not_called() + _, kwargs = coordinator._dialogs.show_confirmation.call_args + assert kwargs["ok_label"] == OPEN_BUTTON_KEY + class TestCancelConfirmation: """Canceling is destructive, so the panel's cancel intent asks for confirmation before the From 863615d92d43a5947f08689fae0fa64bb3c5d641 Mon Sep 17 00:00:00 2001 From: JakimPL Date: Tue, 13 Oct 2026 01:10:47 +0200 Subject: [PATCH 7/8] Kept: the converter's skipping cases writing under their own directory --- .../converter/test_converter.py | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/tests/unit/sampletones_core/reconstructions/converter/test_converter.py b/tests/unit/sampletones_core/reconstructions/converter/test_converter.py index 16013248b..f9da4f28f 100644 --- a/tests/unit/sampletones_core/reconstructions/converter/test_converter.py +++ b/tests/unit/sampletones_core/reconstructions/converter/test_converter.py @@ -1,4 +1,5 @@ from pathlib import Path +from typing import Final from unittest.mock import patch import pytest @@ -21,6 +22,8 @@ from sampletones_shared.exceptions import NoFilesToProcessError _RECONSTRUCTOR_PATCH = "sampletones_core.reconstructions.converter.converter.Reconstructor" +RECORDINGS: Final[str] = "recordings" +RECONSTRUCTIONS: Final[str] = "reconstructions" @pytest.fixture(scope="module") @@ -38,10 +41,21 @@ def _group(path: Path, stems: StemsConfig) -> GroupConversion: def _directory_of(directory: Path, stems: StemsConfig, *names: str) -> ReconstructionConverter: - """A converter over a folder holding ``names``, with its jobs read from the folder.""" + """A converter over a folder of ``names`` under ``directory``, writing its reconstructions beside it. + + The jobs are read from the folder, and their outputs stand under ``directory`` too, so a case writing + one keeps it in its own directory. + """ + recordings = directory / RECORDINGS + recordings.mkdir() for name in names: - (directory / name).touch() - converter = ReconstructionConverter(Config(), DirectoryConversion(directory=directory, stems=stems)) + (recordings / name).touch() + config = Config() + general = config.general.model_copy(update={"reconstructions_directory": str(directory / RECONSTRUCTIONS)}) + converter = ReconstructionConverter( + config.model_copy(update={"general": general}), + DirectoryConversion(directory=recordings, stems=stems), + ) with patch(_RECONSTRUCTOR_PATCH): converter._create_tasks() return converter From 551e07a964211e5ea1ca3b715b0271aed13721c8 Mon Sep 17 00:00:00 2001 From: JakimPL Date: Tue, 13 Oct 2026 09:21:15 +0200 Subject: [PATCH 8/8] Settled: held worker reports round after round until the workers have nothing more to report --- .../integration/history/test_interleavings.py | 6 +++++- tests/suite/application.py | 18 +++++++++++++++--- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/tests/integration/history/test_interleavings.py b/tests/integration/history/test_interleavings.py index 3763520b0..9c74f9232 100644 --- a/tests/integration/history/test_interleavings.py +++ b/tests/integration/history/test_interleavings.py @@ -29,10 +29,14 @@ @contextmanager def held_reports(session: HistorySession) -> Iterator[HeldQueue]: - """Holds what the workers report until the case drains it, each gesture meanwhile landing what it does at once.""" + """Holds what the workers report until the case drains it, each gesture meanwhile landing what it does at once. + + What the drain hands the workers lands before the case goes on, so the next gesture meets no work on its way. + """ queue = HeldQueue() with patch.object(CallbackQueue, "add", queue.add), session.audit.settling(immediately): yield queue + queue.settle() def _edit_lead( diff --git a/tests/suite/application.py b/tests/suite/application.py index f9d152628..6f8e7e067 100644 --- a/tests/suite/application.py +++ b/tests/suite/application.py @@ -91,18 +91,30 @@ def drain(self) -> None: callback, args = self._held.pop(0) callback(*args) + def settle(self) -> None: + """Waits for the workers and runs what they reported, round after round, until a round brings nothing. + + Running a report can hand a worker more to do, such as the render a landing asks for, and that + worker reports once the drain has passed. The rounds go on until the workers have ended with + nothing left to report. + """ + SingleThreadExecutor.join_all() + while self._held: + self.drain() + SingleThreadExecutor.join_all() + def settled(gesture: VoidCallback) -> None: """Runs ``gesture``, waits for the workers it started, and runs what they reported. A worker reports through the queue the render loop drains, so the reports are held while the - workers run and drained once they have ended, in the order they arrived. + workers run and drained once they have ended, in the order they arrived, until the workers have + nothing more to report. """ queue = HeldQueue() with patch.object(CallbackQueue, "add", queue.add): gesture() - SingleThreadExecutor.join_all() - queue.drain() + queue.settle() @pytest.fixture