From 7925f10c6d4e3455e5511acfa130306533eac312 Mon Sep 17 00:00:00 2001 From: smb Date: Fri, 4 Sep 2026 23:54:13 +0200 Subject: [PATCH] fix: count every per-frame stage in the overall render progress The overall budget was len(timestamps) * 2 while a render runs three per-frame stages. Segmentation filled the first half, encode the second, and post-process reported nothing at all: it published its own stage progress but left the overall figure untouched. The dialog derives both the frame rate and the remaining time from that figure, so both froze with it. The reporter measured 23:47 remaining, unchanged across 72 seconds, while about 18 seconds of work were left. Two things the transform stage changed since the issue was filed make a single multiplier wrong in any case. The rebuild path reaches the same code without ever setting an overall context, so its counter was empty through post-process and then started a fresh scale at encode; that is now the common path, because rebuilding is what a transform edit does. And post-process stages only the kept frames, so after a trim the three stages have different lengths. The budget is therefore computed from the work the job will do: timestamps plus twice the kept count for a render, twice the kept count for a rebuild, with each stage occupying its own share. Auto-fit still reports the encode stage as indeterminate, since the number of attempts is unknown. Deliberately not weighting the stages by their measured rates. The defect is a counter that stops and an estimate that stops being recomputed; honest counting fixes both, and an imprecise estimate is a smaller, separate question best answered with numbers from the fixed build. Closes #23 Co-Authored-By: Claude Opus 5 (1M context) --- scripts/guardrails-baseline.json | 2 +- src/matteloop/jobs/render.py | 43 ++++++++++++++++++----- src/matteloop/jobs/transform_stage.py | 7 ++-- tests/jobs/test_rebuild.py | 34 ++++++++++++++++++ tests/jobs/test_render.py | 50 +++++++++++++++++++++++++-- tests/jobs/test_transform_stage.py | 14 +++++++- 6 files changed, 134 insertions(+), 16 deletions(-) diff --git a/scripts/guardrails-baseline.json b/scripts/guardrails-baseline.json index eda301b..0523f23 100644 --- a/scripts/guardrails-baseline.json +++ b/scripts/guardrails-baseline.json @@ -24,7 +24,7 @@ "long_functions": 1 }, "src/matteloop/jobs/render.py": { - "lines": 2938, + "lines": 2871, "long_functions": 11 }, "src/matteloop/jobs/segmentation_host.py": { diff --git a/src/matteloop/jobs/render.py b/src/matteloop/jobs/render.py index a039fc3..6688aa7 100644 --- a/src/matteloop/jobs/render.py +++ b/src/matteloop/jobs/render.py @@ -211,6 +211,18 @@ def publish( ) -> Path: ... +def _overall_budget( + timestamp_count: int, kept_count: int, *, rebuilt: bool +) -> tuple[int, int]: + post_process_and_encode = 2 * kept_count + total = ( + post_process_and_encode + if rebuilt + else timestamp_count + post_process_and_encode + ) + return (0 if rebuilt else timestamp_count, total) + + @dataclass(frozen=True, slots=True) class PreparedSegmentation: port: SegmentationPort @@ -1046,7 +1058,12 @@ def render(self, request: RenderRequest, context: JobContext) -> RenderArtifact: request.sampling.end, request.sampling.fps, ) - delays = webp_delays(len(timestamps), request.sampling.fps) + timestamp_count = len(timestamps) + kept_count = len(request.transform.kept_range(timestamp_count)) + overall_start, overall_total = _overall_budget( + timestamp_count, kept_count, rebuilt=False + ) + delays = webp_delays(timestamp_count, request.sampling.fps) source_sha = self._source.complete_sha256(request.source, context) context.checkpoint("source-hash") inputs = cut_cache_key_inputs( @@ -1071,8 +1088,8 @@ def render(self, request: RenderRequest, context: JobContext) -> RenderArtifact: for index, timestamp in enumerate(timestamps): context.set_frame_context( index + 1, - len(timestamps), - overall=(index, len(timestamps) * 2), + timestamp_count, + overall=(index, overall_total), ) cut, actual = _produce_cut_frame( self._source, @@ -1095,9 +1112,12 @@ def render(self, request: RenderRequest, context: JobContext) -> RenderArtifact: gc.collect(0) context.checkpoint("cut-stage") context.progress( - "render-cut", index + 1, total=len(timestamps), + "render-cut", + index + 1, + total=timestamp_count, detail=f"Cut frame {index + 1} of {len(timestamps)}", - overall_completed=index + 1, overall_total=len(timestamps) * 2, + overall_completed=index + 1, + overall_total=overall_total, ) union_metadata = ( None @@ -1138,6 +1158,7 @@ def render(self, request: RenderRequest, context: JobContext) -> RenderArtifact: union, notes, tuple(scratch_owners), + overall=(overall_start, overall_total), rebuilt=False, ) scratch_owners.clear() @@ -1208,7 +1229,12 @@ def rebuild( "choose-matching-cuts", context.job_id, ) - delays = webp_delays(len(timestamps), request.sampling.fps) + timestamp_count = len(timestamps) + kept_count = len(request.transform.kept_range(timestamp_count)) + overall_start, overall_total = _overall_budget( + timestamp_count, kept_count, rebuilt=True + ) + delays = webp_delays(timestamp_count, request.sampling.fps) snapshot_width = durable_manifest.width snapshot_height = durable_manifest.height worst_case = request.framing.dimensions_after_padding_and_stretch( @@ -1258,6 +1284,7 @@ def rebuild( union, notes, tuple(scratch_owners), + overall=(overall_start, overall_total), rebuilt=True, ) scratch_owners.clear() @@ -1340,6 +1367,7 @@ def _encode_snapshot( notes: list[str], scratch_owners: tuple[Path, ...], *, + overall: tuple[int, int], rebuilt: bool, ) -> RenderArtifact: plan = framing_plan((manifest.width, manifest.height), union, request.framing) @@ -1354,6 +1382,7 @@ def _encode_snapshot( scratch / "framed-inputs", tracker, context, + overall=overall, ) candidate = self._output_publisher.candidate_path( request.output.path, context.job_id, scratch @@ -2840,5 +2869,3 @@ def _is_sha256(value: object) -> bool: and len(value) == 64 and all(character in "0123456789abcdefABCDEF" for character in value) ) - - diff --git a/src/matteloop/jobs/transform_stage.py b/src/matteloop/jobs/transform_stage.py index f050196..d51a155 100644 --- a/src/matteloop/jobs/transform_stage.py +++ b/src/matteloop/jobs/transform_stage.py @@ -59,6 +59,7 @@ def stage_encoder_frames( framed_directory: Path, tracker: RgbaOwnershipTracker, context: JobContext, + overall: tuple[int, int], ) -> tuple[tuple[Path, ...], tuple[int, ...]]: """Frame, crop, and resize the kept cut frames and persist them. @@ -90,9 +91,7 @@ def stage_encoder_frames( kept_count = len(kept) framed_paths: list[Path] = [] for position, index in enumerate(kept): - context.set_frame_context( - position + 1, kept_count, overall=context.overall_progress - ) + context.set_frame_context(position + 1, kept_count, overall=overall) context.checkpoint("framing") framed_paths.append( _stage_frame( @@ -104,6 +103,8 @@ def stage_encoder_frames( position + 1, total=kept_count, detail=f"Frame {position + 1} of {kept_count}", + overall_completed=overall[0] + position + 1, + overall_total=overall[1], ) return tuple(framed_paths), transform.select_kept(delays) diff --git a/tests/jobs/test_rebuild.py b/tests/jobs/test_rebuild.py index 137da4c..c7e332f 100644 --- a/tests/jobs/test_rebuild.py +++ b/tests/jobs/test_rebuild.py @@ -23,6 +23,7 @@ from matteloop.core.state import JobKind from matteloop.core.timebase import webp_delays from matteloop.core.webp import validate_webp +from matteloop.jobs.context import CancellationState, JobContext, ProgressEvent from matteloop.jobs.render import ( AtomicOutputPublisher, FilesystemWorkspacePort, @@ -493,6 +494,39 @@ def test_trim_keeps_exactly_the_selected_frames_and_their_delays(tmp_path) -> No assert info.delays_ms == expected +def test_rebuild_reports_adjacent_framing_and_encode_overall_stages(tmp_path) -> None: + workspace = FilesystemWorkspacePort() + seed_request = request(tmp_path) + original = render_service(workspace=workspace).render( + seed_request, job(tmp_path, "seed-progress", JobKind.RENDER) + ) + events: list[ProgressEvent] = [] + + _rebuild_service(workspace, FakeEncoder()).rebuild( + replace( + seed_request, + rebuild=True, + output=replace(seed_request.output, filename="progress.webp"), + ), + original.cut_workspace, + JobContext( + "rebuild-progress", + JobKind.REBUILD, + tmp_path / "rebuild-work", + events.append, + CancellationState(), + ), + ) + + framing = [event for event in events if event.stage == "Framing"] + encode = [event for event in events if event.stage == "Encode"] + assert [event.overall_completed for event in framing] == [1, 2] + assert [event.overall_completed for event in encode] == [2, 4] + assert all(event.overall_total == 4 for event in framing + encode) + assert framing[-1].overall_completed == encode[0].overall_completed + assert encode[-1].overall_completed == encode[-1].overall_total + + @pytest.mark.parametrize( ("transform", "fragment"), [ diff --git a/tests/jobs/test_render.py b/tests/jobs/test_render.py index 5f76ab9..bb4fc0f 100644 --- a/tests/jobs/test_render.py +++ b/tests/jobs/test_render.py @@ -24,6 +24,7 @@ FramingSpec, OutputSpec, SamplingSpec, + TransformSpec, ) from matteloop.core.state import JobKind from matteloop.core.webp import EncodeSummary, encode_lossless_webp, validate_webp @@ -1278,10 +1279,16 @@ def test_render_reports_counted_stages_and_global_frame_units(tmp_path) -> None: (1, 1), (2, 2), ] - assert all(event.total == 2 and event.overall_total == 4 for event in cuts) + assert all(event.total == 2 and event.overall_total == 6 for event in cuts) assert (encode[0].completed, encode[0].total) == (0, 2) - assert (encode[0].overall_completed, encode[0].overall_total) == (2, 4) - assert (encode[-1].completed, encode[-1].overall_completed) == (2, 4) + assert (encode[0].overall_completed, encode[0].overall_total) == (4, 6) + assert (encode[-1].completed, encode[-1].overall_completed) == (2, 6) + counted = [event for event in events if event.overall_completed is not None] + overall_values = [event.overall_completed for event in counted] + assert overall_values == sorted(overall_values) + framing = [event for event in events if event.stage == "Framing"] + assert [event.overall_completed for event in framing] == [3, 4] + assert counted[-1].overall_completed == counted[-1].overall_total == 6 assert {event.stage for event in events} >= { "Cut promotion", "Framing", @@ -1289,6 +1296,35 @@ def test_render_reports_counted_stages_and_global_frame_units(tmp_path) -> None: } +def test_render_uses_kept_frames_for_the_post_process_and_encode_budget( + tmp_path, +) -> None: + events: list[ProgressEvent] = [] + render_request = replace( + request(tmp_path), transform=TransformSpec(first_frame=1, last_frame=1) + ) + + render_service().render( + render_request, + JobContext( + "trimmed-progress", + JobKind.RENDER, + tmp_path / "job-work", + events.append, + CancellationState(), + ), + ) + + framing = [event for event in events if event.stage == "Framing"] + encode = [event for event in events if event.stage == "Encode"] + assert [(event.completed, event.overall_completed) for event in framing] == [(1, 3)] + assert [(event.completed, event.overall_completed) for event in encode] == [ + (0, 3), + (1, 4), + ] + assert all(event.overall_total == 4 for event in framing + encode) + + def test_render_succeeds_when_a_static_passage_repeats_sampled_pixels( tmp_path, ) -> None: @@ -1362,6 +1398,14 @@ def decode(self, *args, **kwargs): for event in auto_fit_events ) assert all(event.overall_indeterminate for event in auto_fit_events) + cuts = [event for event in events if event.stage == "render-cut"] + framing = [event for event in events if event.stage == "Framing"] + assert all( + event.overall_completed is not None + and event.overall_total == 6 + and not event.overall_indeterminate + for event in cuts + framing + ) @pytest.mark.parametrize( diff --git a/tests/jobs/test_transform_stage.py b/tests/jobs/test_transform_stage.py index 3986336..53925e5 100644 --- a/tests/jobs/test_transform_stage.py +++ b/tests/jobs/test_transform_stage.py @@ -14,6 +14,7 @@ from matteloop.core.rgba import RgbaOwnershipTracker from matteloop.core.specs import CropSpec, FramingSpec, TransformSpec from matteloop.core.state import JobKind +from matteloop.jobs.context import CancellationState, JobContext from matteloop.jobs.transform_stage import framing_plan, stage_encoder_frames from tests.jobs.render_support import job @@ -62,7 +63,14 @@ def read_cut(index: int, tracker: RgbaOwnershipTracker) -> Image.Image: plan = FramingPlan((128, 128), global_bounds=None, padding=0, stretch_x=Fraction(1)) delays = (100, 100, 100) tracker = RgbaOwnershipTracker((128, 128)) - context = job(tmp_path, "job-1", JobKind.RENDER) + events = [] + context = JobContext( + "job-1", + JobKind.RENDER, + tmp_path / "job-work", + events.append, + CancellationState(), + ) paths, returned_delays = stage_encoder_frames( read_cut, @@ -73,6 +81,7 @@ def read_cut(index: int, tracker: RgbaOwnershipTracker) -> Image.Image: tmp_path / "framed-inputs", tracker, context, + (0, 6), ) assert paths == tuple( @@ -85,6 +94,8 @@ def read_cut(index: int, tracker: RgbaOwnershipTracker) -> Image.Image: assert saved.size == (128, 128) assert saved.tobytes() == frame.tobytes() assert returned_delays == delays + assert [event.overall_completed for event in events] == [1, 2, 3] + assert all(event.overall_total == 6 for event in events) def test_stage_encoder_frames_clamps_a_crop_stored_against_a_larger_frame( @@ -119,6 +130,7 @@ def read_cut(index: int, tracker: RgbaOwnershipTracker) -> Image.Image: tmp_path / "framed-inputs", tracker, context, + (0, 2), ) with Image.open(paths[0]) as saved: