diff --git a/context/knowledge/gotchas/hera-view.md b/context/knowledge/gotchas/hera-view.md index 6afc95d8..288906d9 100644 --- a/context/knowledge/gotchas/hera-view.md +++ b/context/knowledge/gotchas/hera-view.md @@ -54,6 +54,7 @@ M6a scaffolds the native Hera view: a `HeraPage` (rail | coordinator pane | agen - **The embedded graph renders the PLAN of the selected orchestrator, projected by `heraPlanNodesWithBridge(root, rail.Model().bridgeIndex())` — NOT a provider seam, NOT a tree, NOT `depends_on`.** It is a pure in-memory read over the rail's `Model` (`rebuildPlan` calls it on every `applySelection` when `detailsMode`); no DB call at Draw time (`BuildModel` already attached `OrchView.Blocks` via one bulk `ListHeraBlocks` read). Coordinator roles are NEVER plan nodes (they author the plan); only worker-kind roles project. A planned (never-bound) role → `Node{Planned:true, State:StatePlanned}`; a live/finished role colours from its bound task's status/result. **MUST use the WithBridge form, not the single-arg `heraPlanNodes`** — the bare form leaves `Node.Drillable` false (it can't see sibling orchestrators); the bridge index stamps `Drillable` on a worker whose bound task coordinates a child orchestrator (D6). `rebuildPlan` first pops the drill stack back to root so a stale child frame from the previous selection never lingers. - **planview↔hera import direction is one-way: `hera` imports `planview`, NEVER the reverse.** The projection (`heraPlanNodes*`) and the drill-in resolution (`HeraPage.drillIntoChild` → `bridgeIndex()` → child `OrchView`) live in `hera` because they need the rail Model; `planview` is a generic widget that knows nothing about hera. The seam is the `OnDrillIn(id)` callback: planview fires it with the node id, the page resolves the child + reprojects + `PushOrch`. This is why the App wires `OnEnter` (jump-to-agent-view, App's concern) but the PAGE wires `OnDrillIn` in `NewHeraPage` (it needs hera internals planview can't reach). - **The widget is exposed via `HeraPage.Plan()`; the App wires only `OnEnter`** (jump to a leaf node's agent view) + `OnBranchChange`→`forceRedraw` (log-only, never Sync). Drill-in (`OnDrillIn`) is page-owned (above). Retitled `" Plan "` via `SetTitle`, full-rect coverage via the widget's own `DrawBorderedPanel`/header strip. `TestDetailsPlan_NoSyncOnDraw` pins no-Sync; `TestDetailsPlan_DrawStacksBothPanels` pins both `" Details "` and `" Plan "` render at once (and asserts no stray `" DAG "`/`"Orchestration Tree"`). Node colour comes from `RoleView.TaskStatus`/`TaskResult` (a `{"failed":true}` result → red `✕`, winning over the workflow status). +- **A status step (`s`/`S`) / `ready_to_close` clear re-projects the plan in lock-step with the rail: `doRefresh`→`applySelection`→`rebuildPlan` runs the SAME refresh, but `planview.UpdateData` short-circuits on an unchanged `projectionSig` — so the sig MUST fold the node's resolved status ICON (glyph + Animated), not just `State.Glyph()` (BUG-012).** The plan node's `State` is task-derived (`TaskStatus`/`TaskResult`), but its ICON also carries the rail-parity glyph (`ready_to_close` ✓, hera role-status mark). A role-status step / `ready_to_close` clear changes the ICON while the task-derived State is unchanged (e.g. still `working`), so without the icon in the sig `UpdateData` no-ops and the DAG node renders a STALE ✓ while the rail already moved. The projected `Icon.Glyph` is a stable frame-0 placeholder (spinner frames re-resolve at Draw), so folding it never spams a reproject. Pinned by `planview.TestUpdateData_IconChangeReinstalls` + page-level `TestRefresh_StatusStepReprojectsPlanNode`. ## Rail parity — nesting, coordinator fold, spinner, PR cell, cascade delete diff --git a/internal/tui/hera/plan_test.go b/internal/tui/hera/plan_test.go index eaa94399..725e2c72 100644 --- a/internal/tui/hera/plan_test.go +++ b/internal/tui/hera/plan_test.go @@ -2,6 +2,7 @@ package hera import ( "fmt" + "strings" "testing" "github.com/drn/argus/internal/db" @@ -463,6 +464,61 @@ func TestRefresh_DifferentCoordinatorResetsPlanCursor(t *testing.T) { testutil.Equal(t, pl.CursorPos().Stage, 0) } +// planStatusLine returns the plan widget's "Status:" header line for the node +// under the cursor — the line that renders the node's RESOLVED status glyph +// (1:1 with the rail). Empty when no node is selected. +func planStatusLine(pl *planview.Widget) string { + for _, ln := range pl.HeaderLines() { + if strings.HasPrefix(ln, "Status:") { + return ln + } + } + return "" +} + +// TestRefresh_StatusStepReprojectsPlanNode is the BUG-012 page-level regression: +// with a coordinator selected (details mode), clearing a worker's ready_to_close +// mark — the StepStatus-out-of-`done` effect — must re-project the plan so the +// worker's DAG node updates in lock-step with the rail, NOT keep rendering the +// stale review ✓. The worker's task-derived State (working) is UNCHANGED by the +// step (only the icon flips), so the plan widget's UpdateData short-circuit on an +// unchanged projection signature would leave the node stale — unless the node +// ICON is folded into that signature (the fix). Asserts through the plan header's +// resolved status glyph, which is what the operator actually sees. +func TestRefresh_StatusStepReprojectsPlanNode(t *testing.T) { + d := memDB(t) + orch := seedOrch(t, d, "orch") + seedBoundRole(t, d, orch, "coord", db.HeraKindCoordinator, "t-coord") + seedBoundRole(t, d, orch, "wkr", db.HeraKindWorker, "t-wkr") + // The worker is live + in_progress (working) AND marked ready_to_close, so its + // plan node shows the review ✓ (ready_to_close wins the glyph precedence over + // the working spinner — same as the rail row). + testutil.NoError(t, d.SetMeta("t-wkr", db.HeraMetaNamespace, db.HeraMetaKeyReadyToClose, "true")) + + p := NewHeraPage(d) + p.SetSessionResolver(resolverFor(map[string]*fakeSession{"t-coord": {id: "t-coord", alive: true}})) + p.Refresh() + + testutil.Equal(t, selectOrchByName(p, "orch"), true) + testutil.Equal(t, p.detailsMode, true) + + pl := p.Plan() + // Degenerate single-worker plan: the cursor sits on the worker node, whose + // header Status line renders its resolved glyph. + testutil.Equal(t, pl.CurrentNodeID(), "t-wkr") + testutil.Equal(t, strings.ContainsRune(planStatusLine(pl), theme.IconReview), true) + + // Step the worker OUT of `done`: clear ready_to_close (exactly what + // Ops.StepStatus does), then run the SAME refresh the s/S key triggers. The + // node's State (working) is unchanged — only the icon flips — so the plan node + // must STILL update to the working glyph, never stay pinned to the review ✓. + testutil.NoError(t, d.ClearHeraReadyToClose("t-wkr")) + p.Refresh() + + testutil.Equal(t, pl.CurrentNodeID(), "t-wkr") + testutil.Equal(t, strings.ContainsRune(planStatusLine(pl), theme.IconReview), false) +} + // --- Cancelled planned node rendering (make-hera-plan-living B3) --- // TestRoleViewCancelled_SetFromCancelledAt: a role whose CancelledAt is set diff --git a/internal/tui/planview/planview.go b/internal/tui/planview/planview.go index 0b9a11be..569da5bf 100644 --- a/internal/tui/planview/planview.go +++ b/internal/tui/planview/planview.go @@ -452,10 +452,10 @@ func (w *Widget) installLayout(nodes []Node, edges []Edge) { } // projectionSig hashes a snapshot's structure: every node's ID + State + -// Drillable, and every edge's endpoints. Two snapshots with the same signature -// render identical cells, so UpdateData can no-op (preserving cursor/fan-out) -// when the signature is unchanged. Order-sensitive on purpose — the projection -// is deterministic, so a stable order means a stable signature. +// Drillable + status icon, and every edge's endpoints. Two snapshots with the +// same signature render identical cells, so UpdateData can no-op (preserving +// cursor/fan-out) when the signature is unchanged. Order-sensitive on purpose — +// the projection is deterministic, so a stable order means a stable signature. func projectionSig(nodes []Node, edges []Edge) uint64 { var h uint64 = 1469598103934665603 // FNV-1a offset basis mix := func(s string) { @@ -481,6 +481,24 @@ func projectionSig(nodes []Node, edges []Edge) uint64 { } else { mixU(0) } + // Fold the resolved status icon (the rail-parity glyph carrying the + // ready_to_close ✓ + hera role-status mark) so a status step / ready_to_close + // clear that changes the node's GLYPH without changing its task-derived State + // still flips the sig — otherwise UpdateData no-ops on the unchanged State and + // the DAG node renders a stale ✓ while the rail already updated (BUG-012). The + // projected Icon.Glyph is a stable frame-0 placeholder (spinner frames are + // re-resolved at Draw), so this never spams a reproject. A nil icon (planned / + // failed nodes fall back to State.Glyph) folds a distinct sentinel. + if n.Icon != nil { + mix(string(n.Icon.Glyph)) + if n.Icon.Animated { + mixU(2) + } else { + mixU(3) + } + } else { + mixU(0xEE) // nil-icon sentinel + } } mixU(0xFF) // node/edge boundary for _, e := range edges { diff --git a/internal/tui/planview/planview_test.go b/internal/tui/planview/planview_test.go index f311087b..6b26c431 100644 --- a/internal/tui/planview/planview_test.go +++ b/internal/tui/planview/planview_test.go @@ -942,6 +942,32 @@ func TestUpdateData_CollapsedGroupCursorReanchors(t *testing.T) { testutil.Equal(t, w.CursorPos().Stage, 1) } +// TestUpdateData_IconChangeReinstalls: a hera role-status step / ready_to_close +// clear changes a node's rendered ICON (the rail-parity glyph) WITHOUT changing +// its task-derived State. UpdateData must still re-install the layout so the DAG +// node updates in lock-step with the rail (BUG-012). The icon is folded into the +// projection signature; without it UpdateData would no-op on the unchanged State +// and the node would render a stale ✓ while the rail already moved to working. +func TestUpdateData_IconChangeReinstalls(t *testing.T) { + mk := func(ic *NodeIcon) []Node { + n := liveNode("w0", StateWorking) // task still in_progress (working) in BOTH frames + n.Icon = ic + return []Node{n} + } + reviewIcon := &NodeIcon{Glyph: '✓', Style: tcell.StyleDefault, Animated: false} + workingIcon := &NodeIcon{Glyph: '⠋', Style: tcell.StyleDefault, Animated: true} + + w := New() + w.SetData(mk(reviewIcon), nil) + testutil.Equal(t, w.nodes["w0"].Icon.Glyph, '✓') + + // Same node ID and same State (StateInProgress) — only the icon flipped from + // the ready_to_close ✓ to the working spinner. The stored node must update. + w.UpdateData(mk(workingIcon), nil) + testutil.Equal(t, w.nodes["w0"].Icon.Glyph, '⠋') + testutil.Equal(t, w.nodes["w0"].Icon.Animated, true) +} + // --- Selection highlight + centering (SimulationScreen Draw tests) --- // drawToSim renders w into a fresh SimulationScreen sized (cols, rows) and