feat(intelligence): add Graph Reach (eccentricity / diameter / radius)#2985
feat(intelligence): add Graph Reach (eccentricity / diameter / radius)#2985aashir-athar wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (23)
✅ Files skipped from review due to trivial changes (6)
🚧 Files skipped from review as they are similar to previous changes (17)
📝 WalkthroughWalkthroughAdds a deterministic graph-reach engine, an RPC facade, a data-loading tab and a presentational panel for displaying reach metrics, integrates a new "reach" tab in Intelligence, includes tests, and adds translations for multiple locales. ChangesGraph Reach Centrality Feature
Sequence Diagram(s)sequenceDiagram
participant User
participant GraphReachTab
participant graphReachApi
participant Backend as RPC Backend
participant computeGraphReach
participant GraphReachPanel
User->>GraphReachTab: mount / select namespace
GraphReachTab->>graphReachApi: loadNamespaces()
graphReachApi->>Backend: memoryListNamespaces()
Backend-->>graphReachApi: namespaces[]
GraphReachTab->>graphReachApi: loadReach(namespace?)
graphReachApi->>Backend: memoryGraphQuery(namespace?)
Backend-->>graphReachApi: relations[]
graphReachApi->>computeGraphReach: computeGraphReach(relations)
computeGraphReach-->>graphReachApi: ReachResult
graphReachApi-->>GraphReachTab: ReachResult
GraphReachTab->>GraphReachPanel: result + loading + error + onRetry
GraphReachPanel->>User: render metrics, caption, ranked table
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/components/intelligence/GraphReachTab.tsx (1)
40-47: ⚡ Quick winUse
async/awaitfor the bootstrap path and log swallowed namespace failures.The mount effect still uses a
.then().catch()chain and silently drops selector-load errors. Converting this to an inner async function keeps the file consistent with the repo’s TypeScript style and gives you a place to emit the required namespaced debug log when namespaces fail but reach loading continues.♻️ Proposed refactor
+import debug from 'debug'; import { useCallback, useEffect, useRef, useState } from 'react'; @@ +const log = debug('graph-reach:tab'); + const GraphReachTab = () => { @@ useEffect(() => { - // Namespaces are optional UI sugar; a failure to list them must not block - // the reach view, so swallow that error specifically. - loadNamespaces() - .then(setNamespaces) - .catch(() => setNamespaces([])); - void load(''); + void (async () => { + // Namespaces are optional UI sugar; a failure to list them must not block + // the reach view, so swallow that error specifically. + try { + setNamespaces(await loadNamespaces()); + } catch (err) { + log('loadNamespaces failed: %O', err); + setNamespaces([]); + } + await load(''); + })(); }, [load]);As per coding guidelines, "Always use async/await for promises in TypeScript rather than
.then()chains" and "verbose logging required on new/changed flows".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/intelligence/GraphReachTab.tsx` around lines 40 - 47, Convert the useEffect to call an inner async function that awaits both loadNamespaces() and load(''), replacing the .then().catch() chain; inside that async function await loadNamespaces(), call setNamespaces(result) on success, and in the catch block call setNamespaces([]) but also emit a verbose debug log (e.g., console.debug or the component logger) with the caught error to record the swallowed namespace failure; finally await load('') so reach loading always runs, and invoke the inner async function from useEffect while keeping the dependency on [load].
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/lib/i18n/en.ts`:
- Around line 343-344: Add a new i18n key for the singular entity case (e.g.,
'graphReach.summaryCaptionOneEntity') so the copy can be "1 component · 1
entity", and update the UI panel to choose between
'graphReach.summaryCaptionOne' and the new singular key when rendering the
caption by checking the giantComponentSize value (i.e., when component count is
1 use the '...One' key, and when giantComponentSize === 1 use the new singular
key); ensure you reference the existing keys 'graphReach.summaryCaption' and
'graphReach.summaryCaptionOne' when implementing the branch so the caption logic
uses the appropriate key based on giantComponentSize.
---
Nitpick comments:
In `@app/src/components/intelligence/GraphReachTab.tsx`:
- Around line 40-47: Convert the useEffect to call an inner async function that
awaits both loadNamespaces() and load(''), replacing the .then().catch() chain;
inside that async function await loadNamespaces(), call setNamespaces(result) on
success, and in the catch block call setNamespaces([]) but also emit a verbose
debug log (e.g., console.debug or the component logger) with the caught error to
record the swallowed namespace failure; finally await load('') so reach loading
always runs, and invoke the inner async function from useEffect while keeping
the dependency on [load].
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8b4794e5-2fc8-4b9d-853f-aac4ca6c9b4e
📒 Files selected for processing (23)
app/src/components/intelligence/GraphReachPanel.test.tsxapp/src/components/intelligence/GraphReachPanel.tsxapp/src/components/intelligence/GraphReachTab.test.tsxapp/src/components/intelligence/GraphReachTab.tsxapp/src/lib/i18n/chunks/ar-1.tsapp/src/lib/i18n/chunks/bn-1.tsapp/src/lib/i18n/chunks/de-1.tsapp/src/lib/i18n/chunks/en-1.tsapp/src/lib/i18n/chunks/es-1.tsapp/src/lib/i18n/chunks/fr-1.tsapp/src/lib/i18n/chunks/hi-1.tsapp/src/lib/i18n/chunks/id-1.tsapp/src/lib/i18n/chunks/it-1.tsapp/src/lib/i18n/chunks/ko-1.tsapp/src/lib/i18n/chunks/pt-1.tsapp/src/lib/i18n/chunks/ru-1.tsapp/src/lib/i18n/chunks/zh-CN-1.tsapp/src/lib/i18n/en.tsapp/src/lib/memory/graphReach.test.tsapp/src/lib/memory/graphReach.tsapp/src/pages/Intelligence.tsxapp/src/services/api/graphReachApi.test.tsapp/src/services/api/graphReachApi.ts
…ty caption Address CodeRabbit review on tinyhumansai#2985: - buildAdjacency now drops the self-loop EDGE but registers the endpoint as a node, matching the sibling Graph Core fix. A user whose only recorded fact is "Alice→Alice" now appears as a singleton component (size 1, eccentricity 0) instead of vanishing into the empty state. - Adds graphReach.summaryCaptionOneAndOne ('1 component · 1 entity') and a panel branch so the singleton case never renders as the ungrammatical "1 component · 1 entities". - Regression tests for both the engine (Alice→Alice -> singleton) and the panel (renders the all-singular caption variant) lock the behaviour. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
A new read-only "Reach" tab for the Intelligence view: BFS-based eccentricity analysis of the memory knowledge graph. Where the Connection-Path lens answers "is A linked to B, and how far", this lens answers the summary question: "how far is each entity from everything else it can reach". From that fall out the DIAMETER (the longest shortest-path), RADIUS (the smallest), and CENTER (the entities that reach the whole cluster in the fewest hops) — a node neither degree nor PageRank can surface. Engine (pure, deterministic — no React/RPC/clock/RNG): - per-node eccentricity via BFS (with an array-backed queue), - connected-component labelling, - per-component diameter & radius, - giant component (size DESC, then smallest componentId for stable ties), - per-node isCenter flag (ecc === its component's radius). Vertices are canonically id-sorted before indexing, so output is byte-identical regardless of relation input order. Adds ZERO new core surface: composes the already-shipped memoryGraphQuery / memoryListNamespaces JSON-RPC wrappers. Container/presentational split with a monotonic request-token race guard for load-on-mount; i18n across all 13 locales. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ty caption Address CodeRabbit review on tinyhumansai#2985: - buildAdjacency now drops the self-loop EDGE but registers the endpoint as a node, matching the sibling Graph Core fix. A user whose only recorded fact is "Alice→Alice" now appears as a singleton component (size 1, eccentricity 0) instead of vanishing into the empty state. - Adds graphReach.summaryCaptionOneAndOne ('1 component · 1 entity') and a panel branch so the singleton case never renders as the ungrammatical "1 component · 1 entities". - Regression tests for both the engine (Alice→Alice -> singleton) and the panel (renders the all-singular caption variant) lock the behaviour. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1020361 to
55e0405
Compare
Summary
Adds a new read-only "Reach" tab to the Intelligence view: BFS-based eccentricity analysis of the memory knowledge graph. Where #2946's Connection-Path lens answers "is A linked to B, and how far", this lens answers the summary question: "how far is each entity from everything else it can reach". From that fall out three classic shape metrics:
The center of your knowledge is not the highest-degree node nor the highest-PageRank node — it's the one that minimises the worst-case distance to everything else, the natural place a traversal or summary should start. Neither degree nor PageRank surfaces it.
Design
lib/memory/graphReach.ts): treats the(subject)-[predicate]->(object)triples as an undirected simple graph (direction dropped, parallel edges collapsed, self-loops dropped) and computes:componentId= smallest member index, stable label),isCenter(eccentricity === component's radius— every component has a well-defined center).memoryGraphQuery/memoryListNamespacesJSON-RPC wrappers. Read-only — recomputed live, never persisted.componentCount === 1. i18n across all 13 locales.Test plan
vitest— 25 tests (engine: empty / triangle (uniform center) / path A-B-C-D (diameter 3, radius 2, center {B,C}) / star (hub is sole center) / two-component (giant diameter; small component's local ecc) / giant tie-break by smallest componentId; self-loop drop; parallel-edge & direction collapse; both branches of the malformed-row guard; no case-folding; order-independent output across permutations — plus api facade, panel singular + plural caption variants, center badge, container load/namespace-requery/error)tsc --noEmit— cleaneslint— 0 errorsprettier --check— clean🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Localization
Tests