The main Dash callback _update in toolchain/mfc/viz/interactive.py is a single ~986-line function (def _update at line 1660, final return at 2645) behind a decorator declaring 5 outputs, 33 inputs, and 1 state (1619–1659). It inlines the kaleido-render, 3D-full, 3D-patch, 2D-full, 2D-patch, and 1D paths, all branching off a shared read-only preamble. This is the single hardest-to-maintain unit in the toolchain.
Goal: turn _update into a thin dispatcher by extracting the self-contained branch bodies into helpers — e.g. _render_kaleido(...), _render_3d_full(...), _render_2d_full(...) — so each render path is independently readable.
⚠️ Prerequisite: add Dash smoke tests first
interactive.py currently has zero automated coverage (test_viz.py covers viz.py/renderer.py/reader.py, never the callback). A blind decomposition of a 986-line stateful callback is unverifiable except by manually clicking through the 1D/2D/3D/kaleido/patch paths, which is high regression risk. This issue should be gated on first adding smoke tests that drive _update through each branch with synthetic data, so the refactor can be verified. (The dedup-only subset — status Div / log transform / mesh key — is split into its own low-risk good-first-issue and should land first.)
Gotchas the refactor must respect
- Load-bearing closure cells.
_is_playing (1492), _last_update_t (1523), _min_frame_gap (1524) are single-element-list cells used as mutable closure state. Any extracted helper must receive these cells (or return updated values) — passing the value and forgetting to write it back is a silent frame-pacing regression.
- Patch/
no_update fast paths exist for performance. The kaleido and Patch branches return dash.no_update for the figure to avoid full rebuilds; a _render_* extraction that always rebuilds would kill that optimization (the reason those branches exist).
- ~30 widget parameters. Helpers extracted to module level inherit the ~30 widget args; consider bundling them into a dataclass as a preparatory step so signatures stay sane.
Not a good-first-issue — large, stateful, and test-gated. Filing for tracking; depends on the viz dedup + smoke-test work landing first.
Filed from a repo-wide code-cleanliness review; verified against master @ 40dde5e.
Code references
The main Dash callback
_updateintoolchain/mfc/viz/interactive.pyis a single ~986-line function (def _updateat line 1660, finalreturnat 2645) behind a decorator declaring 5 outputs, 33 inputs, and 1 state (1619–1659). It inlines the kaleido-render, 3D-full, 3D-patch, 2D-full, 2D-patch, and 1D paths, all branching off a shared read-only preamble. This is the single hardest-to-maintain unit in the toolchain.Goal: turn
_updateinto a thin dispatcher by extracting the self-contained branch bodies into helpers — e.g._render_kaleido(...),_render_3d_full(...),_render_2d_full(...)— so each render path is independently readable.interactive.pycurrently has zero automated coverage (test_viz.pycoversviz.py/renderer.py/reader.py, never the callback). A blind decomposition of a 986-line stateful callback is unverifiable except by manually clicking through the 1D/2D/3D/kaleido/patch paths, which is high regression risk. This issue should be gated on first adding smoke tests that drive_updatethrough each branch with synthetic data, so the refactor can be verified. (The dedup-only subset — status Div / log transform / mesh key — is split into its own low-risk good-first-issue and should land first.)Gotchas the refactor must respect
_is_playing(1492),_last_update_t(1523),_min_frame_gap(1524) are single-element-list cells used as mutable closure state. Any extracted helper must receive these cells (or return updated values) — passing the value and forgetting to write it back is a silent frame-pacing regression.no_updatefast paths exist for performance. The kaleido and Patch branches returndash.no_updatefor the figure to avoid full rebuilds; a_render_*extraction that always rebuilds would kill that optimization (the reason those branches exist).Not a good-first-issue — large, stateful, and test-gated. Filing for tracking; depends on the viz dedup + smoke-test work landing first.
Filed from a repo-wide code-cleanliness review; verified against
master@40dde5e.Code references
toolchain/mfc/viz/interactive.py:1619-1659—@app.callbackdecorator (5 out / 33 in / 1 state)toolchain/mfc/viz/interactive.py:1660—_updatedef (runs to L2645, ~986 lines)toolchain/mfc/viz/interactive.py:1523-1524— load-bearing closure cells