Fix dashboard counts to update on load

[ncurrie3]

Description

This PR updates the Dashboard page (/) to make the "Active Agents" and "Groves" counts dynamic.
Previously, these values were hardcoded as -- and did not update on load.

Changes:

• Subscribed to dashboard scope in stateManager.
• Added initial data fetch in connectedCallback via a new loadData method.
• Added event listeners for agents-updated and groves-updated to keep counts fresh.

Tests

• Manual verification: Confirmed that counts are populated immediately on load and reflect the actual number of running agents and groves.
• No automated tests were added as this is a UI behavior enhancement relying on existing state management.

---

Fixes #154

It's a good idea to open an issue first for discussion.

• Tests pass (Manual verification)
• Appropriate changes to documentation are included in the PR

[gemini-code-assist]

Code Review

This pull request introduces the ability to override the Hub API endpoint via a new HubEndpoint option in the backend synchronization logic, including support for environment variable fallbacks in hub-connected contexts. On the frontend, the home page is updated to dynamically fetch and display the count of active agents and groves by integrating with the stateManager. Feedback was provided regarding the stateManager's seeding methods, which are currently additive and do not dispatch update events, potentially leading to stale data or desynchronized UI components.

[gemini-code-assist]

The seedAgents and seedGroves methods in StateManager are additive (they use Map.set without clearing existing entries). Since loadData performs a full fetch of all agents and groves for the dashboard, calling these methods without clearing the existing state in the manager will result in stale data persisting if items were deleted on the server but no SSE 'deleted' event was received. Additionally, seedAgents and seedGroves do not dispatch update events; consequently, other components listening to the state manager will not be synchronized with the data fetched here. Consider updating StateManager to support a 'reset' or 'sync' operation that replaces the map and notifies listeners, or ensure the scope is fully reset before seeding.

[ncurrie3]

/gemini review pls :)

[gemini-code-assist]

Code Review

This pull request updates the StateManager to allow clearing data during seeding and introduces event notifications for state changes. The ScionPageHome component is refactored to fetch and display real-time counts for active agents and groves by subscribing to these state updates. Feedback suggests addressing a potential race condition in the asynchronous data loading process and updating the seedAgents documentation to reflect the newly added notification logic.

[gemini-code-assist]

This data loading logic has two issues:

1. Race Condition: loadData is asynchronous and may complete after the user has navigated away from the dashboard. Calling stateManager.seedAgents at that point would populate the global state with dashboard data while the app is in a different scope (e.g., a specific Grove), leading to inconsistent UI state.
2. Redundant Updates: Manually setting this.agents and this.groves here is unnecessary because seedAgents and seedGroves now trigger events that this component already listens to via onAgentsUpdated and onGrovesUpdated.

It is recommended to check this.isConnected before seeding and let the state manager's events handle the local property updates.

      if (!this.isConnected) return;

      if (agentsResp.ok) {
        const data = await agentsResp.json();
        const agents = Array.isArray(data) ? data : data.agents || [];
        stateManager.seedAgents(agents);
      }

      if (grovesResp.ok) {
        const data = await grovesResp.json();
        const groves = Array.isArray(data) ? data : data.groves || [];
        stateManager.seedGroves(groves);
      }

[gemini-code-assist]

This call to notify contradicts the method's docstring (lines 389-390), which explicitly states that notifications are not triggered. Please update the docstring to reflect that seedAgents now notifies listeners of the update to ensure consistency and avoid confusion for future maintainers.

[ptone]

PR #155 Review: Fix dashboard counts to update on load

Executive Summary

This PR makes the dashboard's "Active Agents" and "Groves" stat cards dynamic by fetching data via the REST API and subscribing to SSE state updates. The change is low-to-medium risk — the dashboard component itself is well-structured, but the modifications to seedAgents/seedGroves/seedBrokers in StateManager introduce an unintended behavioral change for all existing callers across the codebase.

---

Critical Issues

1. seed*() now emits notifications — breaks contract with existing callers

File: web/src/client/state.ts

The original seedAgents, seedGroves, and seedBrokers methods were explicitly documented as not triggering notifications:

"Does not trigger notifications — the calling component already holds the data from its own fetch."

The PR adds this.notify(...) calls unconditionally to all three methods. There are 10+ existing call sites across agents.ts, groves.ts, grove-detail.ts, agent-detail.ts, broker-detail.ts, and brokers.ts that rely on the no-notification contract. These callers already assign the data to their own @state() properties (triggering a Lit re-render) and then call seed*() to populate the state manager for SSE delta merging.

With this change, every seed*() call will now:

1. Fire a notification event
2. Cause all listeners to call stateManager.getAgents() / stateManager.getGroves() (which creates a new array from the map)
3. Trigger unnecessary Lit re-renders across any mounted component listening to those events

Impact: Spurious re-renders on every page that seeds data. While not a crash bug, it degrades performance and violates the principle of least surprise.

Suggested Fix: Don't change the existing seed*() signatures. Instead, create a separate method or have loadData() in home.ts manually call notify after seeding, or add an opt-in parameter:

// Option A: Add opt-in notify parameter (keep default as false for backward compat)
seedAgents(agents: Agent[], { clear = false, notify = false } = {}): void {
  if (clear) this.state.agents.clear();
  for (const agent of agents) {
    this.state.agents.set(agent.id, agent);
  }
  if (notify) this.notify('agents-updated');
}

// Option B: Just notify from the caller
// In home.ts loadData():
stateManager.seedAgents(agents);
// Manually trigger update since home page needs it
this.agents = stateManager.getAgents();

Option B is simpler and matches the pattern used by other pages — they set this.agents directly from fetch results and call seed*() only for SSE baseline. The home page can do the same without changing the shared StateManager.

2. The clear parameter adds risk with no current user

The clear parameter is added to all three seed methods but is never actually used (clear = false default, and no caller passes true). Dead code in a shared state manager is a footgun — a future caller passing clear = true would wipe the entire agents map even if SSE deltas have already merged newer data, causing a brief flash of stale-only data.

Suggested Fix: Remove the clear parameter entirely from this PR. If needed later, it should be introduced with clear semantics and documentation.

---

Observations

3. loadData() doesn't use clear — potential stale entries

File: web/src/components/pages/home.ts, loadData() method

When the dashboard fetches agents from /api/v1/agents, it calls stateManager.seedAgents(agents) without clearing. If an agent was deleted between page loads but an SSE event hasn't arrived yet, the stale entry remains in the map. This is a pre-existing issue with the seed pattern, but it's worth noting that the dashboard will show an inflated count in that edge case.

For the dashboard, this is acceptable since counts are eventually consistent via SSE.

4. this.isConnected guard is good but incomplete

File: web/src/components/pages/home.ts, line in loadData()

if (!this.isConnected) return;

This guard appears after Promise.all resolves, which is correct for preventing updates on unmounted components. However, the guard only checks once — between processing agentsResp and grovesResp, the component could theoretically disconnect. In practice this is negligible since both responses are already resolved, but for completeness, a second check before seedGroves would be defensive.

5. .filter() in template re-runs every render

File: web/src/components/pages/home.ts

<span>${this.agents.filter(a => isAgentRunning(a)).length}</span>

This allocates a new filtered array on every render cycle. For a dashboard with potentially hundreds of agents, consider caching this as a getter or computed property:

private get activeAgentCount(): number {
  return this.agents.filter(isAgentRunning).length;
}

This is a minor readability improvement — performance impact is negligible for typical agent counts.

6. Response shape handling is defensive and good

const agents = Array.isArray(data) ? data : data.agents || [];

This gracefully handles both Agent[] and { agents: Agent[] } response shapes, which is resilient to API evolution.

---

Positive Feedback

• Clean lifecycle management: connectedCallback/disconnectedCallback properly add and remove event listeners with pre-bound references, avoiding memory leaks.
• isConnected guard: Checking component connection status after async operations is a best practice that prevents stale updates.
• Scope subscription: Calling stateManager.setScope({ type: 'dashboard' }) ensures the SSE client subscribes to the correct event stream, consistent with the pattern used by other pages.
• Error handling: The try/catch in loadData() with console.error is appropriate for a non-critical data fetch — the UI degrades to showing 0 rather than crashing.

---

Final Verdict

Needs Rework — specifically on the StateManager.seed*() changes.

The home.ts component changes are well-implemented and ready to merge. However, the state.ts changes (adding unconditional notify() calls and the unused clear parameter) alter the contract of a shared service used by 6+ other components. The safest path is to revert the state.ts changes and instead have home.ts directly assign this.agents and this.groves from its own fetch results (the same pattern used by agents.ts, groves.ts, grove-detail.ts, etc.), then call the unmodified seed*() for SSE baseline.