mirror of
https://github.com/samvallad33/vestige.git
synced 2026-04-25 00:36:22 +02:00
fix(graph): default /api/graph to newest-memory center, add sort param
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<String>` (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
This commit is contained in:
Sam Valladares
parent
8fe8bb2f39
commit
7441b3cdfe
3 changed files with 206 additions and 20 deletions
|
|
@ -63,7 +63,21 @@ export const api = {
|
|||
fetcher<TimelineResponse>(`/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)
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -468,6 +468,30 @@ pub struct GraphParams {
|
|||
pub center_id: Option<String>,
|
||||
pub depth: Option<u32>,
|
||||
pub max_nodes: Option<usize>,
|
||||
/// 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<String>,
|
||||
}
|
||||
|
||||
/// 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<Json<Value>, 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<vestige_core::Storage>,
|
||||
sort: GraphSort,
|
||||
) -> Result<String, StatusCode> {
|
||||
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<Storage>) {
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue