From 7441b3cdfe6892aa76716757e05d832fc8035ff4 Mon Sep 17 00:00:00 2001 From: Sam Valladares Date: Mon, 20 Apr 2026 18:47:01 -0500 Subject: [PATCH] fix(graph): default /api/graph to newest-memory center, add sort param MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit memory_timeline PR #37 exposed the same class of bug in the graph endpoint: the dashboard Graph page (and the /api/graph endpoint it hits) defaulted to centering on the most-connected memory, ran BFS at depth 3, and capped the subgraph at 150 nodes. On a mature corpus this clustered the visualization around a historical hotspot and hid freshly ingested memories that hadn't accumulated edges yet. User-visible symptom: TimeSlider on /graph showing "Feb 21 → Mar 1 2026" when the database actually contains memories through today (Apr 20). **Backend (`crates/vestige-mcp/src/dashboard/handlers.rs`):** - `GraphParams` gains `sort: Option` (accepted: "recent" | "connected", unknown falls back to "recent"). - New internal `GraphSort` enum + case-insensitive `parse()`. - Extracted `default_center_id(storage, sort)` so handler logic and tests share the same branching. Recent path picks `get_all_nodes(1, 0)` (ORDER BY created_at DESC). Connected path picks `get_most_connected_memory`, degrading gracefully to recent if the DB has no edges yet. - Default behaviour flipped from "connected" to "recent" — matches user expectation of "show me my recent stuff". **Dashboard (`apps/dashboard`):** - `api.graph()` accepts `sort?: 'recent' | 'connected'` with JSDoc explaining the rationale. - `/graph/+page.svelte` passes `sort: 'recent'` when no query or center_id is active. Query / center_id paths unchanged — they already carry their own centering intent. **Tests:** 6 new unit tests in `handlers::tests`: - `graph_sort_parse_defaults_to_recent` (None, empty, garbage, "recent", "Recent", "RECENT") - `graph_sort_parse_accepts_connected_case_insensitive` - `default_center_id_recent_returns_newest_node` — ingest 3 nodes, assert newest is picked - `default_center_id_connected_prefers_hub_over_newest` — wire a hub node with 3 spokes, then ingest a newer "lonely" node; assert the hub wins in Connected mode even though it's older - `default_center_id_connected_falls_back_to_recent_when_no_edges` — fresh DB with no connections still returns newest, not 404 - `default_center_id_returns_not_found_on_empty_db` — both modes return 404 cleanly on empty storage Build + test: - cargo test -p vestige-mcp --lib handlers:: → 6/6 pass - cargo test --workspace --lib → 830/830 pass, 0 failed - cargo clippy -p vestige-core -p vestige-mcp --lib -- -D warnings → clean - npm run check → 0 errors, 0 warnings - npm test → 361/361 pass Binary already installed at ~/.local/bin/vestige-mcp (copied from cargo build --release -p vestige-mcp). New Claude Desktop / Code sessions will pick it up automatically when they respawn their MCP subprocess. The dashboard HTTP server on port 3927 needs a manual relaunch from a terminal with the usual pattern: nohup bash -c 'tail -f /dev/null | \ VESTIGE_DASHBOARD_ENABLED=true ~/.local/bin/vestige-mcp' \ > /tmp/vestige-mcp.log 2>&1 & disown --- apps/dashboard/src/lib/stores/api.ts | 16 +- .../src/routes/(app)/graph/+page.svelte | 7 +- crates/vestige-mcp/src/dashboard/handlers.rs | 203 ++++++++++++++++-- 3 files changed, 206 insertions(+), 20 deletions(-) diff --git a/apps/dashboard/src/lib/stores/api.ts b/apps/dashboard/src/lib/stores/api.ts index e0cafec..ff5b22a 100644 --- a/apps/dashboard/src/lib/stores/api.ts +++ b/apps/dashboard/src/lib/stores/api.ts @@ -63,7 +63,21 @@ export const api = { fetcher(`/timeline?days=${days}&limit=${limit}`), // Graph - graph: (params?: { query?: string; center_id?: string; depth?: number; max_nodes?: number }) => { + // + // `sort` controls the default center when no query/center_id is given: + // - "recent" (default) — newest memory; matches user expectation of + // "show me what I just added". Previously the backend defaulted to + // "connected" which clustered on historical hotspots and hid + // fresh memories that hadn't accumulated edges yet. + // - "connected" — densest node; richer initial subgraph for a + // well-aged corpus. Exposed for a future UI toggle. + graph: (params?: { + query?: string; + center_id?: string; + depth?: number; + max_nodes?: number; + sort?: 'recent' | 'connected'; + }) => { const qs = params ? '?' + new URLSearchParams( Object.entries(params) .filter(([, v]) => v !== undefined) diff --git a/apps/dashboard/src/routes/(app)/graph/+page.svelte b/apps/dashboard/src/routes/(app)/graph/+page.svelte index 28842a2..09dd661 100644 --- a/apps/dashboard/src/routes/(app)/graph/+page.svelte +++ b/apps/dashboard/src/routes/(app)/graph/+page.svelte @@ -87,7 +87,12 @@ max_nodes: maxNodes, depth: 3, query: query || undefined, - center_id: centerId || undefined + center_id: centerId || undefined, + // Center on the newest memory by default. Prevents the old + // "most-connected" behaviour from clustering on historical + // hotspots and hiding today's memories behind the 150-node + // cap. Future UI toggle can flip this to 'connected'. + sort: query || centerId ? undefined : 'recent' }); if (graphData) { liveNodeCount = graphData.nodeCount; diff --git a/crates/vestige-mcp/src/dashboard/handlers.rs b/crates/vestige-mcp/src/dashboard/handlers.rs index d1c3187..e29d173 100644 --- a/crates/vestige-mcp/src/dashboard/handlers.rs +++ b/crates/vestige-mcp/src/dashboard/handlers.rs @@ -468,6 +468,30 @@ pub struct GraphParams { pub center_id: Option, pub depth: Option, pub max_nodes: Option, + /// How to choose the default center when neither `query` nor `center_id` + /// is provided. "recent" (default) uses the newest memory — matches + /// what users actually expect ("show me my recent stuff"). "connected" + /// uses the most-connected memory for a richer initial subgraph; used + /// to be the default but ended up clustering on historical hotspots + /// and hiding fresh memories that hadn't accumulated edges yet. + /// Unknown values fall back to "recent". + pub sort: Option, +} + +/// Which memory to center the default subgraph on. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum GraphSort { + Recent, + Connected, +} + +impl GraphSort { + fn parse(raw: Option<&str>) -> Self { + match raw.map(str::to_ascii_lowercase).as_deref() { + Some("connected") => Self::Connected, + _ => Self::Recent, + } + } } /// Get memory graph data (nodes + edges with layout positions) @@ -477,6 +501,7 @@ pub async fn get_graph( ) -> Result, StatusCode> { let depth = params.depth.unwrap_or(2).clamp(1, 3); let max_nodes = params.max_nodes.unwrap_or(50).clamp(1, 200); + let sort = GraphSort::parse(params.sort.as_deref()); // Determine center node let center_id = if let Some(ref id) = params.center_id { @@ -491,24 +516,7 @@ pub async fn get_graph( .map(|n| n.id.clone()) .ok_or(StatusCode::NOT_FOUND)? } else { - // Default: most connected memory (for a rich initial graph) - let most_connected = state - .storage - .get_most_connected_memory() - .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; - if let Some(id) = most_connected { - id - } else { - // Fallback: most recent memory - let recent = state - .storage - .get_all_nodes(1, 0) - .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; - recent - .first() - .map(|n| n.id.clone()) - .ok_or(StatusCode::NOT_FOUND)? - } + default_center_id(&state.storage, sort)? }; // Get subgraph @@ -565,6 +573,46 @@ pub async fn get_graph( }))) } +/// Pick the default subgraph center when neither `query` nor `center_id` +/// was provided. Factored out so both the route handler and unit tests can +/// exercise the same branching (recent vs connected + empty-db fallback) +/// without spinning up a full axum server. +fn default_center_id( + storage: &std::sync::Arc, + sort: GraphSort, +) -> Result { + match sort { + GraphSort::Recent => { + let recent = storage + .get_all_nodes(1, 0) + .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; + recent + .first() + .map(|n| n.id.clone()) + .ok_or(StatusCode::NOT_FOUND) + } + GraphSort::Connected => { + let most_connected = storage + .get_most_connected_memory() + .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; + if let Some(id) = most_connected { + Ok(id) + } else { + // Nothing connected yet (fresh DB, or every node is isolated) — + // fall through to the newest memory so the user still sees + // SOMETHING rather than a 404. + let recent = storage + .get_all_nodes(1, 0) + .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; + recent + .first() + .map(|n| n.id.clone()) + .ok_or(StatusCode::NOT_FOUND) + } + } + } +} + // ============================================================================ // SEARCH (dedicated endpoint) // ============================================================================ @@ -1088,3 +1136,122 @@ pub async fn list_intentions( "filter": status_filter, }))) } + +#[cfg(test)] +mod tests { + use super::*; + use chrono::Utc; + use std::sync::Arc; + use tempfile::tempdir; + use vestige_core::memory::IngestInput; + use vestige_core::{ConnectionRecord, Storage}; + + #[test] + fn graph_sort_parse_defaults_to_recent() { + assert_eq!(GraphSort::parse(None), GraphSort::Recent); + assert_eq!(GraphSort::parse(Some("")), GraphSort::Recent); + assert_eq!(GraphSort::parse(Some("recent")), GraphSort::Recent); + assert_eq!(GraphSort::parse(Some("RECENT")), GraphSort::Recent); + assert_eq!(GraphSort::parse(Some("Recent")), GraphSort::Recent); + assert_eq!(GraphSort::parse(Some("garbage")), GraphSort::Recent); + } + + #[test] + fn graph_sort_parse_accepts_connected_case_insensitive() { + assert_eq!(GraphSort::parse(Some("connected")), GraphSort::Connected); + assert_eq!(GraphSort::parse(Some("CONNECTED")), GraphSort::Connected); + assert_eq!(GraphSort::parse(Some("Connected")), GraphSort::Connected); + } + + fn seed_storage() -> (tempfile::TempDir, Arc) { + let dir = tempdir().unwrap(); + let db_path = dir.path().join("test.db"); + let storage = Arc::new(Storage::new(Some(db_path)).unwrap()); + (dir, storage) + } + + fn ingest(storage: &Storage, content: &str) -> String { + let node = storage + .ingest(IngestInput { + content: content.to_string(), + node_type: "fact".to_string(), + ..Default::default() + }) + .unwrap(); + node.id + } + + #[test] + fn default_center_id_recent_returns_newest_node() { + let (_dir, storage) = seed_storage(); + ingest(&storage, "first"); + ingest(&storage, "second"); + let newest = ingest(&storage, "third"); + + let center = default_center_id(&storage, GraphSort::Recent).unwrap(); + assert_eq!( + center, newest, + "Recent mode should pick the newest ingested memory" + ); + } + + fn link(storage: &Storage, source: &str, target: &str) { + let now = Utc::now(); + storage + .save_connection(&ConnectionRecord { + source_id: source.to_string(), + target_id: target.to_string(), + strength: 0.9, + link_type: "semantic".to_string(), + created_at: now, + last_activated: now, + activation_count: 0, + }) + .unwrap(); + } + + #[test] + fn default_center_id_connected_prefers_hub_over_newest() { + let (_dir, storage) = seed_storage(); + let hub = ingest(&storage, "hub node"); + let spoke_a = ingest(&storage, "spoke A"); + let spoke_b = ingest(&storage, "spoke B"); + let spoke_c = ingest(&storage, "spoke C"); + // Wire the spokes into `hub` so it has the most connections. Leave + // the final `lonely` node unconnected — it's the newest by + // insertion order and would win in Recent mode. + for spoke in [&spoke_a, &spoke_b, &spoke_c] { + link(&storage, &hub, spoke); + } + let _lonely = ingest(&storage, "lonely newcomer"); + + let center = default_center_id(&storage, GraphSort::Connected).unwrap(); + assert_eq!( + center, hub, + "Connected mode should pick the densest node, not the newest" + ); + } + + #[test] + fn default_center_id_connected_falls_back_to_recent_when_no_edges() { + let (_dir, storage) = seed_storage(); + ingest(&storage, "alpha"); + let newest = ingest(&storage, "beta"); + + // No connections exist — Connected mode should degrade to Recent + // rather than returning 404. + let center = default_center_id(&storage, GraphSort::Connected).unwrap(); + assert_eq!(center, newest); + } + + #[test] + fn default_center_id_returns_not_found_on_empty_db() { + let (_dir, storage) = seed_storage(); + + let recent_err = default_center_id(&storage, GraphSort::Recent).unwrap_err(); + assert_eq!(recent_err, StatusCode::NOT_FOUND); + + let connected_err = default_center_id(&storage, GraphSort::Connected).unwrap_err(); + assert_eq!(connected_err, StatusCode::NOT_FOUND); + } +}