Restructure the session pass into desired-state, differ, and applier - #1230
Merged
Conversation
Reconciler migration phase 2 (#1227). The per-session reconcile pass had grown six interleaved concerns gated by hand-maintained memo records and a predicate that mirrored every gate by hand; #1216/#1219 were composition bugs between them. The pass now reads shared snapshots, decides via the pure desired_cells() policy function (new dashboard/cell_plan.py), and applies the difference against what each widget records it was built from. Rebuild means "current build inputs differ from the widget's", not "a counter moved past a remembered value": the inputs are the layer snapshot (identity pins state, version, plotter, error) plus has_plot, which covers the one upgrade a version counter cannot express — a STOPPED layer's retained data computed by the reveal's first-viewer activation. A failed build leaves the applied record unchanged, so retry is automatic instead of choreographed through deliberately-stale records. Deleted: _cell_signatures and _cell_grid (folded into the widget record), SessionLayer.last_seen_version, the #1220 stale-records device, and the rebuild half of _has_pending_work — the predicate is now a generic input-stamp comparison plus the one inherent wall-clock stall term. _last_layer_version survives, but purely as the wake stamp; its snapshot-early/record-late semantics now applies uniformly to all stamps. The hidden-grid policy (#1216/#1220) is one line in desired_cells() — materialize when visible here or watched anywhere — unit-tested with plain data and no UI-framework imports (guarded). Policy-vs-mechanism review rule added to .claude/rules/dashboard-widgets.md; glossary updated. Tests coupled to the deleted records were adapted mechanically (SessionLayer constructor, _build_cell grid_id parameter, dropped asserts on the two deleted dicts); the phase-0 characterization suite is otherwise unchanged and green, as are the browser smoke tests.
SimonHeybrock
added a commit
that referenced
this pull request
Aug 10, 2026
Phases 0 and 2 are implemented (#1229, #1230); phase 2's criteria are met with one nuance recorded (_last_layer_version survives as a pure wake stamp) plus one addition to the sketch (per-layer has_plot in the build inputs). Phase 3 is declined: flush/freshness are not the odd ones out after the restructuring, so the small imperative tail stays.
The term anchors the reconciler's inputs and the orchestrator's version contract but was used without a definition.
The one regression it found: disabling a grid orphan-swept its layers' SessionLayers while the kept widgets stayed bound to their pipes, and with no build input changing, a re-enabled grid would render its pre-disable frame forever. Disabled grids' layers now release their token but keep their SessionLayer; a regression test pins data flow across disable/re-enable (verified to fail on the previous commit). Gate correctness: the topology stamp and the tab-composition memo are now recorded only after the reconcile succeeded, so an exception retries instead of silently dropping the change — the one input that still violated the pass's record-on-completion rule. The cell sweep checks topology membership via the orchestrator rather than the pass's local grids view, so a reentrant tick observing _grid_widgets mid-rebuild cannot dispose live cells. The stamps are a NamedTuple, removing the positional pairing between _input_stamps and the recorded fields; the two deliberately unstamped inputs (has_viewers, has_plot) are documented with their 5 s bound. Simplifications from the review: deferral is expressed as absence from the plans (CellPlan.materialize and the dead `plot_grid is None` branch are gone — the applier now indexes grid widgets directly, failing fast); has_plot reuses LayerSnapshot.has_displayable_plot instead of restating it; the viewed set is read once per pass via the new PlotDataService.viewed_layers instead of lock-per-layer has_viewers. The purity guard now runs in a subprocess and checks loaded modules, which exposed that cell_plan was only nominally pure: the topology dataclasses lived in plot_orchestrator, which transitively loads the full UI stack. They move to a new leaf module plot_topology.py (giving the glossary's "topology" term a code home); plot_orchestrator imports them unchanged.
2 tasks
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.
Phase 2 of the reconciler migration (#1227) — the go/no-go spike from the proposal (docs in #1228). Stacked on #1229, whose characterization tests are the equivalence oracle.
What changed. The per-session pass is restructured into read → decide → act. A new pure module
dashboard/cell_plan.pycomputes the target widget tree (desired_cells): per cell, whether this session should hold a built widget (materialize — visible here, or watched by any session) and the build inputs the widget must be built from (layer snapshot +has_plot). Each built widget records its inputs; the differ rebuilds exactly when the current inputs no longer compare equal. The #1216 policy is now literally one expression indesired_cells, and the deferred state (hidden, unwatched) absorbs any number of version bumps with zero builds structurally rather than via the #1220 stale-records device. The concurrency model (ADR 0005/0007), token gating, flush batching, and freshness semantics are untouched.Why
has_plotis a separate input: a STOPPED layer's retained data is computed by the reveal's first-viewer activation without a version bump — the one upgrade a counter cannot express; snapshot identity alone would miss it.Acceptance criteria (from the migration plan):
_cell_signatures,_cell_grid,SessionLayer.last_seen_version, and the deliberately-stale-records device are gone;_cell_gridfolded into the widget record. One nuance to be honest about:_last_layer_versionsurvives — but purely as the wake stamp, no longer feeding any rebuild decision, and its snapshot-early/record-late rule now applies uniformly to all stamps instead of being a per-field 12-line choreography._tab_compositionstays (tabs remain imperative, as the plan allowed)._has_pending_workis a generic input-stamp comparison (_input_stamps()vs the recorded stamps) plus the one inherent wall-clock stall term._cell_griddict. Older tests coupled to the deleted mechanism were adapted mechanically (constructor kwarg,_build_cellgained agrid_idparameter, one stale-version test reshaped to assert the retry observably). Browser smoke tests: 16/16 green.tests/dashboard/cell_plan_test.py— 15 plain-data tests, no Panel/HoloViews/Bokeh anywhere (a purity guard test enforces the import rule).A policy-vs-mechanism review rule is added to
.claude/rules/dashboard-widgets.md, and the glossary gains the new terms.Test plan
tests/dashboard: 2133 passed, 4 pre-existing xfails.pytest -m browser: 16 passed.