Draw solved velocity as arrows in pyflow run's declared-field path - #89
Open
AdamClemens wants to merge 1 commit into
Open
Draw solved velocity as arrows in pyflow run's declared-field path#89AdamClemens wants to merge 1 commit into
AdamClemens wants to merge 1 commit into
Conversation
Closes the gap TASK-051's own commit flagged explicitly rather than fixed: playback.py's combined play() path renders both a solved velocity's arrows and its declared fields' panels together, but bootstrap.py's _add_declared_field_transport -- the identical config shape's live pyflow run path -- colour-mapped the panels and never drew the velocity carrying them at all. Reachable from two shipped demos: pyflow run on smoke_transport.yaml or thermal_buoyancy.yaml showed no indication of flow direction or magnitude despite a real, pressure-corrected velocity driving the transport. Checked docs/implementation/golden-demos.md and Stage 6's Completion Criteria first, per this repo's "deliberately scoped out for a stated reason" convention -- both were silent on live-run arrows either way. tests/unit/test_golden_demo_annotations.py's own _draws_arrows did call the omission "correctly exempt", but that comment predated TASK-051 and was stale rather than a live decision, so this closes it rather than reversing something intentional. _add_declared_field_transport now builds/rebuilds field_visualization.build_vector_field_arrows every frame when config.simulation.velocity_solved is set, the same "remove old, build new" shape _add_solved_velocity_rendering already uses, and returns a third arrows_drawn closure joined into bootstrap()'s show_vector_scale via the existing _either_path_drew_arrows. Written test-first (tests/unit/test_bootstrap.py:: test_bootstrap_with_velocity_solved_and_a_declared_field_also_draws_arrows, confirmed red before the fix landed). Blast radius: test_golden_demo_annotations.py's _draws_arrows no longer exempts velocity_solved-with-declared-fields; smoke_transport.yaml, thermal_buoyancy.yaml and multi_field_plume.yaml all gained a field_display.vector_label, since P-019 requires one on every demo that draws arrows and none needed one while this path drew none. src/pyflow/CLAUDE.md and docs/implementation/golden-demos.md record the fix and its history. roadmap.md/status.md's test count moved 1199 -> 1200 for the one new test. Verified live, not only unit-tested: ran pyflow run --backend offscreen against all three affected demos and confirmed each now holds exactly one arrow gfx.Line object, a correct "Velocity (model units): length = 0.3 x magnitude" HUD line, and 100k+ non-background rendered pixels. Also fixed an unrelated, pre-existing false positive found while running make preflight from inside this session's own git worktree checkout (.claude/worktrees/<name>): test_check_claims.py's test_only_tracked_markdown_files_are_read flagged "worktrees" appearing among repo_root's own ancestor path components as if it were the nested- worktree double-read bug it was written to catch. Narrowed to a path relative to repo_root, which is the only shape the original bug had; the test's actual load-bearing assertion (read == tracked) already passed throughout. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
_add_declared_field_transport(pyflow run's live path for a config combiningsimulation.velocity_solvedwith declaredfields) now draws the solved velocity as arrows alongside its declared fields' panels, the same "remove old, build new" shape_add_solved_velocity_renderingalready uses. Closes the gap TASK-051's own commit explicitly flagged rather than fixed (playback.py's combinedplay()path already drew both;pyflow rundrew only panels).docs/implementation/golden-demos.mdand Stage 6's Completion Criteria first — both silent on live-run arrows either way, so this isn't reversing a deliberate scoping decision.test_golden_demo_annotations.py's own "correctly exempt" comment predated TASK-051 and was stale, not a live decision.test_check_claims.py::test_only_tracked_markdown_files_are_read, found while runningmake preflightfrom inside this session's owngit worktreecheckout (.claude/worktrees/<name>) — the test flagged"worktrees"appearing amongrepo_root's own ancestor path components as if it were the nested-worktree double-read bug it exists to catch. Narrowed to a path relative torepo_root.Test plan
tests/unit/test_bootstrap.py::test_bootstrap_with_velocity_solved_and_a_declared_field_also_draws_arrowsfirst, confirmed red, then implemented.test_golden_demo_annotations.py's_draws_arrowsupdated;smoke_transport.yaml/thermal_buoyancy.yaml/multi_field_plume.yamleach gained afield_display.vector_label(P-019);src/pyflow/CLAUDE.md,docs/implementation/golden-demos.md,roadmap.md/status.mdtest count all updated in this change.pyflow run --backend offscreenagainst all three affected demos — each now holds exactly one arrowgfx.Lineobject, a correct"Velocity (model units): length = 0.3 x magnitude"HUD line, and 100k+ non-background rendered pixels.make preflightgreen locally (1182 passed).make check-docs/references/manifest/documents/stages/scenarios/config-template/dates/duplicate-blocks,mypy --strict,ruffall clean.make cion a real runner, both platforms (CI, not yet observed).🤖 Generated with Claude Code