feat(#564): collapse completed milestone groups in dependency graph

.claude/agent-memory/cookbook.md

@@ -68,6 +68,7 @@ Fully documented in `.claude/rules/` or `.claude/skills/`. One-line refs only.
 - **Fixed-width columns for right-aligned badge/button groups** -- wrap PR badge, state badge, and action button in a fixed-width container (e.g., `style="width:340px"`) with internal fixed-width divs. Without this, variable text widths cause ragged vertical alignment across rows. [confirmed: 1]
 - **Multi-action handoffs expand into a sub-row, not cramped inline** -- single-action handoffs (e.g., "Integrate PR") render on the main row. Multi-action handoffs (e.g., spec review with 4 buttons) expand into a second line below the title with summary text + buttons. [confirmed: 1]
 - **Deduplicate overlapping polls via `window._sova*` shared caches** -- when multiple JS modules poll the same endpoint (sidebar + page-specific), have the primary poller store its response in `window._sovaActiveAgents = {data, ts}` (or `window._sovaSystemMetrics`). Secondary consumers read from the cache with a staleness check (`Date.now() - cached.ts > 5000`). Server-side: add a `_metrics_cache` tuple with `time.monotonic()` + threading lock + 3s TTL on blocking functions like `get_system_metrics()`. PR #595. [confirmed: 1]
+- **Save-poll race guard must cover ALL mutation paths, not just the primary save** -- when adding `_queueSaveInFlight` + `_queueLastSaved` grace period to prevent polling from overwriting just-saved state, the guard was initially only applied to `saveQueue()`. The review caught that `addToQueue()`, `removeFromQueue()`, and `clearQueue()` use separate API endpoints but write to the same underlying state, so they need the same `_queueLastSaved = Date.now()` timestamp. General rule: when guarding a race condition between polling and mutation, enumerate ALL functions that mutate the shared state. File: `sova/dashboard/templates/supervisor.html`. PR #656. [confirmed: 1]
 - **Clickable card pattern: cursor-pointer + stopPropagation on nested buttons** -- add `cursor-pointer hover:opacity-80` on clickable element with `onclick`. All nested interactive elements must call `event.stopPropagation()`. File: `sova/dashboard/templates/agents.html`. [confirmed: 1]
 - **Extract shared CAS patterns to avoid SonarCloud duplication gate** -- when multiple functions share validation + atomic CAS update logic (e.g., `resume_from_approval` and `reject_spec`), extract a shared helper parameterized by target status. Without this, the duplicated block triggers SonarCloud's 3% new code duplication threshold. PR #334. [confirmed: 1]
 - **DB-level tests needed for functions mocked in router tests** -- router tests that mock service functions (e.g., `reject_spec`, `_find_awaiting_approval_run`) provide integration coverage for the router but leave the service function at 0% coverage. Add separate DB-level tests that seed TaskRun records and call the service directly. PR #334: coverage went from 35.8% to 95.6%. [confirmed: 1]
@@ -160,6 +161,8 @@ Fully documented in `.claude/rules/` or `.claude/skills/`. One-line refs only.
 - **Log diagnostic context before silent break/continue exits** -- when a `break` exits a loop on failure, log `stdout[:200]` and `stderr[:200]` before breaking. The final error message should include the reason, not just "could not be completed". PR #425 review. [confirmed: 1]
 - **New step `validate_output` must implement all four gate checks from architecture.md** -- new steps frequently implement two of the four required gate checks (unstaged diff, staged diff) but skip the other two (commits ahead of base via `git log {base}..HEAD --oneline`, untracked files via `git status --porcelain` lines starting with `??`). The architecture.md rule exists but gets missed when writing new steps. Checklist before shipping a new step: (1) `git diff --stat HEAD`, (2) `git diff --cached --stat`, (3) `git log {base}..HEAD --oneline`, (4) `git status --porcelain | grep "^??"`. PR #498 SOVA review finding. [confirmed: 1]
 - **`git stash pop` from a worktree subdir runs in the calling shell's CWD** -- `cd .claude/worktrees/X && git stash pop` in a Bash tool call does not persist the CWD change. The pop applies to the main repo index, not the worktree's. Use `git -C /absolute/path/to/worktree stash pop` instead, or apply changes directly to worktree files via Python scripts with absolute paths. [confirmed: 1]
+- **Benchmark `log.sh` must run from the worktree CWD, not the main repo** -- `log.sh` uses `git branch --show-current` to resolve the issue number from `feat/issue-NNN`. Running from the main repo (on `main` branch) logs to the wrong issue file or falls back to whatever branch main is on. When running `/review-full` or `/pr` commands for a worktree branch, `cd` to the worktree first or use `bash /path/to/log.sh` from the worktree CWD. PR #656 session: `review_start`/`review_complete` logged to issue-650 instead of issue-632; PR #660 session: spec/review events logged to issue-650 instead of issue-657. [confirmed: 2]
+- **Stale worktree directory + git checkout = main repo corruption** -- when a worktree is cleaned up (pruned, removed by background task) but the physical directory persists, `git` commands from inside that directory search upward and operate on the main repo. Running `git checkout <feature-branch>` from a stale worktree path silently switches the main repo to the feature branch. This causes: (1) main repo is no longer on `main`, (2) new worktree creation for the same branch fails, (3) files from the feature branch appear in the main repo. Diagnosis: `git worktree list` shows only one entry (main checkout on the wrong branch). Fix: `git checkout main` from the main repo directory. Prevention: after `git worktree remove`, verify the directory is deleted; if running commands in a worktree subdir, check `git worktree list` first. PR #660. [confirmed: 1]
 
 ## GitHub API
 
@@ -261,6 +264,7 @@ Fully documented in `.claude/rules/` or `.claude/skills/`. One-line refs only.
 - **Polling loop caches must distinguish API failure from empty result** -- when an inner fetch function returns `[]` on both success (no items) and failure (API error), caching `[]` as a successful sync hides failures for the entire TTL. Return `None` on failure, `[]` on genuine empty result. Only set `_sync_cache[key]` on non-None returns. PR #636. [confirmed: 1]
 - **Single-flight lock for API-backed caches prevents concurrent cache misses** -- two concurrent callers can both pass the TTL check before either completes the fetch. Use `_sync_locks.setdefault(key, asyncio.Lock())` with a recheck inside the lock. Preserves `force=True` behavior by skipping the lock recheck on force. PR #636. [confirmed: 1]
 - **Cache keys must include all dimensions that distinguish entries** -- keying a per-PR cache by `pr_number` alone collides across repos in multi-project mode. Use `(repo, pr_number)` tuples. Same applies to any cache shared by a multi-project supervisor. PR #636. [confirmed: 1]
+- **`_get_project_agents(slug)` must resolve project_dir from registry for non-default slugs** -- in supervisor/daemon context, `get_project_dir()` (contextvars) returns None because there's no HTTP request. Without a registry lookup fallback, all non-default projects get SOVA's own `_default_project_dir`, causing TaskRuns, validate_output, and GitHub API calls to target the wrong project. Fix: after `get_project_dir()` returns None and `slug != _DEFAULT_SLUG`, call `get_project_path(slug)` from `sova.config.registry`. General rule: any service function that resolves project context via contextvars must have a non-HTTP fallback for daemon/background callers. File: `sova/dashboard/services/agent_pool.py`. PR #660. [confirmed: 1]
 - **Invalidate API-backed caches on all state-changing paths** -- graph cache must be invalidated on agent spawn (`execute_decision`) AND epic close (`_try_close_epic`), not just one. Audit all code paths that call state-transition APIs and verify each invalidates the relevant cache. PR #636. [confirmed: 1]
 
 ## Refactoring / Code Quality
@@ -394,6 +398,7 @@ Fully documented in `.claude/rules/` or `.claude/skills/`. One-line refs only.
 - **Daemon subprocess `gh` calls must pass `--repo` explicitly** -- the daemon's CWD may not be the project directory (multi-project mode, worktree context). Always pass `--repo {config.github_repo}` to `gh pr list`, `gh api`, etc. in daemon/supervisor subprocess calls. File: `sova/supervisor/planner.py:_get_open_prs()`. PR #625. [confirmed: 1]
 - **Identity-keyed in-memory state for multi-account isolation** -- global singletons (rate limit trackers, quota caches) conflate state across GitHub identities in multi-project setups. Replace `_tracker: T | None` with `_trackers: dict[str, T]`; key by `github_user` from config/adapter. All callers (adapter, dashboard services, progression engine) must pass the identity. PR #590 CodeRabbit. [confirmed: 1]
 - **Gate checks that block API calls must run before any API call in the same function** -- `evaluate_all()` called `build_dependency_graph()` (which calls `list_tasks()`) before checking the rate limit gate, wasting an API call during cooldown. Move the gate check to the top of any function that makes API calls as a side effect. Same applies to `auto_close_epics()` when building a fallback graph. PR #590 CodeRabbit. [confirmed: 1]
+- **Execution-time liveness re-check for fallback paths without built-in guards** -- `evaluate_all()` checks `_check_already_running()` during evaluation, but `execute_decisions()` runs later. When the execution method delegates to `rollback_issue_state()` (which has its own concurrent-run guard), the primary path is safe. But fallback paths that call `adapter.transition_state()` directly need their own re-check, otherwise a dashboard-started agent between evaluation and execution gets its issue label overwritten. PR #655 CodeRabbit + SOVA review. [confirmed: 1]
 
 ## Auto-Retry / Failure Classification
 

sova/dashboard/templates/supervisor.html

@@ -42,6 +42,8 @@ <h3 class="text-sm font-medium text-gray-300">Dependency Graph <span class="text
         <span class="flex items-center gap-1"><span class="inline-block w-1 h-3 rounded" style="background:#6c7086"></span> Low</span>
       </div>
       <div class="flex items-center gap-1">
+        <button type="button" id="hide-done-btn" onclick="toggleHideDone()" title="Toggle visibility of completed milestone groups"
+                class="text-xs text-gray-400 hover:text-gray-200 px-2 h-6 flex items-center justify-center rounded border border-gray-700/60 hover:border-gray-600 transition-colors">Hide Done</button>
         <button type="button" onclick="zoomIn()" title="Zoom in" class="text-xs text-gray-400 hover:text-gray-200 w-6 h-6 flex items-center justify-center rounded border border-gray-700/60 hover:border-gray-600 transition-colors">+</button>
         <button type="button" onclick="zoomOut()" title="Zoom out" class="text-xs text-gray-400 hover:text-gray-200 w-6 h-6 flex items-center justify-center rounded border border-gray-700/60 hover:border-gray-600 transition-colors">&minus;</button>
         <button type="button" onclick="zoomFit()" title="Fit to view" class="text-xs text-gray-400 hover:text-gray-200 px-2 h-6 flex items-center justify-center rounded border border-gray-700/60 hover:border-gray-600 transition-colors">Fit</button>
@@ -200,6 +202,17 @@ <h3 class="text-sm font-medium text-gray-300">Activity Stream</h3>
 let _selectedNodeId = null;
 let _fitTransform = null;
 let _savedTransform = null;  // user's current pan/zoom, preserved across re-renders
+let _lastGraphData = null;
+
+function _lsGet(key) {
+  try { return localStorage.getItem(key); } catch { return null; }
+}
+
+function _lsSet(key, val) {
+  try { localStorage.setItem(key, val); } catch { /* noop */ }
+}
+
+let _hideDone = _lsGet('sova-graph-hide-done') === '1';
 
 function _computeGraphKey(data) {
   // agent_elapsed_seconds is intentionally excluded: it changes every poll while an agent runs
@@ -251,6 +264,21 @@ <h3 class="text-sm font-medium text-gray-300">Activity Stream</h3>
 function zoomOut() { if (_svgSel && _zoomBehavior) _svgSel.transition().duration(300).call(_zoomBehavior.scaleBy, 0.74); }
 function zoomFit() { if (_svgSel && _zoomBehavior && _fitTransform) _svgSel.transition().duration(350).call(_zoomBehavior.transform, _fitTransform); }
 
+function toggleHideDone() {
+  _hideDone = !_hideDone;
+  _lsSet('sova-graph-hide-done', _hideDone ? '1' : '0');
+  _updateHideDoneBtn();
+  if (_lastGraphData) {
+    if (_svgSel) _savedTransform = d3.zoomTransform(_svgSel.node());
+    renderGraph(_lastGraphData);
+  }
+}
+
+function _updateHideDoneBtn() {
+  const btn = document.getElementById('hide-done-btn');
+  if (btn) btn.textContent = _hideDone ? 'Show Done' : 'Hide Done';
+}
+
 // -- Toggle / enable / disable --
 let _toggleBusy = false;
 
@@ -497,6 +525,7 @@ <h3 class="text-base font-medium text-gray-300 mb-2">Setup Required</h3>
     if (_svgSel) _savedTransform = d3.zoomTransform(_svgSel.node());
 
     errorEl.classList.add('hidden');
+    _lastGraphData = data;
     renderGraph(data);
   } catch (e) {
     loading.classList.add('hidden');
@@ -702,22 +731,42 @@ <h3 class="text-base font-medium text-gray-300 mb-2">Setup Required</h3>
     }
   });
 
+  // Determine completed groups and visibility
+  const TOMBSTONE_H = 28;
+
+  function isGroupCompleted(m) {
+    const nodes = msMap.get(m);
+    return !nodes.length || nodes.every(n => n.state === 'done' || n.state === 'closed');
+  }
+
+  const visibleMs = _hideDone
+    ? sortedMs.filter(m => !isGroupCompleted(m))
+    : [...sortedMs];
+
+  const collapsedGroups = new Set();
+  visibleMs.forEach(m => {
+    if (isGroupCompleted(m) && _lsGet(`sova-graph-expanded:${m}`) !== '1') {
+      collapsedGroups.add(m);
+    }
+  });
+
   // Arrange groups in a 2-column grid (even-index groups = left column, odd = right)
-  const col0MaxW = Math.max(...sortedMs.filter((_, i) => i % 2 === 0).map(m => groupLayouts.get(m).boxW), NW + GP * 2);
+  const col0MaxW = Math.max(...visibleMs.filter((_, i) => i % 2 === 0).map(m => groupLayouts.get(m).boxW), NW + GP * 2);
   const col0X = MARGIN;
   const col1X = col0X + col0MaxW + GGX;
 
   const groupPos = new Map();
   let curY0 = MARGIN, curY1 = MARGIN;
-  sortedMs.forEach((m, i) => {
-    const { boxH } = groupLayouts.get(m);
-    if (i % 2 === 0) { groupPos.set(m, { x: col0X, y: curY0 }); curY0 += boxH + GGY; }
-    else             { groupPos.set(m, { x: col1X, y: curY1 }); curY1 += boxH + GGY; }
+  visibleMs.forEach((m, i) => {
+    const h = collapsedGroups.has(m) ? TOMBSTONE_H : groupLayouts.get(m).boxH;
+    if (i % 2 === 0) { groupPos.set(m, { x: col0X, y: curY0 }); curY0 += h + GGY; }
+    else             { groupPos.set(m, { x: col1X, y: curY1 }); curY1 += h + GGY; }
   });
 
-  // Build global nodePos from group origin + local position
+  // Build global nodePos from group origin + local position (skip collapsed groups)
   const nodePos = {};
-  sortedMs.forEach(m => {
+  visibleMs.forEach(m => {
+    if (collapsedGroups.has(m)) return;
     const { x: gx, y: gy } = groupPos.get(m);
     groupLayouts.get(m).nodeLPos.forEach((lp, id) => {
       const nx = gx + lp.lx, ny = gy + lp.ly;
@@ -727,6 +776,11 @@ <h3 class="text-base font-medium text-gray-300 mb-2">Setup Required</h3>
 
   const allX = Object.values(nodePos).map(p => p.x + NW);
   const allY = Object.values(nodePos).map(p => p.y + NH);
+  collapsedGroups.forEach(m => {
+    const { x, y } = groupPos.get(m);
+    allX.push(x + groupLayouts.get(m).boxW);
+    allY.push(y + TOMBSTONE_H);
+  });
   const contentW = (allX.length ? Math.max(...allX) + GP : 600) + MARGIN;
   const contentH = (allY.length ? Math.max(...allY) + GP : 400) + MARGIN;
 
@@ -824,11 +878,50 @@ <h3 class="text-base font-medium text-gray-300 mb-2">Setup Required</h3>
   }
 
   // --- draw group boxes (behind edges and nodes) ---
-  sortedMs.forEach(m => {
+  visibleMs.forEach(m => {
     const { x, y } = groupPos.get(m);
     const { boxW, boxH, nodeLPos, subgroupMeta } = groupLayouts.get(m);
     const { bg, border, head } = groupPal.get(m);
     const memberIds = JSON.stringify([...nodeLPos.keys()]);
+    const completed = isGroupCompleted(m);
+
+    if (collapsedGroups.has(m)) {
+      const grp = mainG.append('g')
+        .attr('class', 'group-box group-tombstone')
+        .attr('data-members', memberIds)
+        .style('cursor', 'pointer');
+
+      grp.append('rect')
+        .attr('x', x).attr('y', y).attr('width', boxW).attr('height', TOMBSTONE_H)
+        .attr('rx', 6)
+        .attr('fill', bg).attr('fill-opacity', 0.6)
+        .attr('stroke', border).attr('stroke-width', 1).attr('stroke-opacity', 0.4);
+
+      const label = m.replace(/^Phase \d+:\s*/i, '');
+      const tDisplay = label.length > 40 ? label.slice(0, 39) + '…' : label;
+      grp.append('text')
+        .attr('x', x + 10).attr('y', y + 18)
+        .attr('fill', head).attr('font-size', '9px').attr('font-weight', '600')
+        .attr('font-family', 'system-ui, sans-serif').attr('letter-spacing', '0.04em')
+        .attr('opacity', 0.7)
+        .text(tDisplay.toUpperCase());
+
+      const nodeCount = msMap.get(m).length;
+      grp.append('text')
+        .attr('x', x + boxW - 10).attr('y', y + 18)
+        .attr('text-anchor', 'end')
+        .attr('fill', '#a6e3a1').attr('font-size', '9px').attr('font-weight', '500')
+        .attr('font-family', 'system-ui, sans-serif')
+        .attr('opacity', 0.6)
+        .text(`${nodeCount} done`);
+
+      grp.on('click', () => {
+        _lsSet(`sova-graph-expanded:${m}`, '1');
+        if (_svgSel) _savedTransform = d3.zoomTransform(_svgSel.node());
+        renderGraph(_lastGraphData);
+      });
+      return;
+    }
 
     const grp = mainG.append('g').attr('class', 'group-box').attr('data-members', memberIds);
 
@@ -854,6 +947,23 @@ <h3 class="text-base font-medium text-gray-300 mb-2">Setup Required</h3>
       .attr('font-family', 'system-ui, sans-serif').attr('letter-spacing', '0.04em')
       .text(display.toUpperCase());
 
+    if (completed) {
+      grp.append('text')
+        .attr('x', x + boxW - GP).attr('y', y + GH - 8)
+        .attr('text-anchor', 'end')
+        .attr('fill', head).attr('font-size', '8.5px').attr('font-weight', '500')
+        .attr('font-family', 'system-ui, sans-serif')
+        .attr('opacity', 0.5)
+        .style('cursor', 'pointer')
+        .text('collapse')
+        .on('click', (event) => {
+          event.stopPropagation();
+          _lsSet(`sova-graph-expanded:${m}`, '0');
+          if (_svgSel) _savedTransform = d3.zoomTransform(_svgSel.node());
+          renderGraph(_lastGraphData);
+        });
+    }
+
     // Subgroup dividers and labels (for large groups only)
     subgroupMeta.forEach(sg => {
       // Divider line above this subgroup (except first)
@@ -1620,6 +1730,7 @@ <h3 class="text-base font-medium text-gray-300 mb-2">Setup Required</h3>
 }
 
 // -- Init --
+_updateHideDoneBtn();
 loadStatus(); loadQuota(); loadCIBudget(); loadGraph(); loadDecisions(); loadPlan(); loadQueue(); loadSupervisorPersona();
 
 // Poll every 30s as fallback; loadGraph re-renders only when data changes

sova/supervisor/watchdog.py

@@ -5,8 +5,8 @@
 (pipeline not adopted, no output, step timeout, zombie process), and
 takes corrective action (warn via feed event, kill via stop_agent).
 
-Killed agents flow through the existing _wait_and_finalize -> _schedule_retry()
-path for auto-retry. The watchdog never independently retries.
+Killed agents flow through the existing _wait_and_finalize path.
+The watchdog never independently retries.
 """
 
 from __future__ import annotations

tests/test_scheduler.py

@@ -421,12 +421,15 @@ def test_server_start_help(self) -> None:
         assert "host" in result.output.lower() or "port" in result.output.lower()
 
     def test_server_status_shows_not_running(self) -> None:
+        from unittest.mock import patch
+
         from typer.testing import CliRunner
 
         from sova.cli.app import app
 
         runner = CliRunner()
-        result = runner.invoke(app, ["server", "status"])
+        with patch("sova.scheduler.server.read_pid_file", return_value=None):
+            result = runner.invoke(app, ["server", "status"])
         assert result.exit_code == 0
         assert "not running" in result.output.lower() or "stopped" in result.output.lower()