fix(intelligence): Graph Reach — BFS gate, tab wiring, i18n flat (#2985)#3037
fix(intelligence): Graph Reach — BFS gate, tab wiring, i18n flat (#2985)#3037staimoorulhassan wants to merge 1 commit into
Conversation
…n flat (tinyhumansai#2985) - Add MAX_REACH_NODES (5000) safety cap with isGraphReachFeasible() check to prevent BFS-from-every-node O(V*(V+E)) from freezing large graphs - Wire GraphReachTab into Intelligence.tsx (import, type union, tab entry, render) - Add memory.tab.reach + 21 graphReach.* i18n keys to en.ts and all 13 locales with real translations (flat single-file format) - Fix French/Italian apostrophe quoting (single→double quotes for l'excentricité etc.)
📝 WalkthroughWalkthroughThis PR adds a complete graph reach (node centrality) analysis feature to the memory knowledge graph: a deterministic computation engine for graph metrics (diameter, radius, eccentricity), API service layer, React components for visualization with namespace filtering, page routing integration, and translations for 14 languages. ChangesGraph Reach Feature
Sequence DiagramsequenceDiagram
participant User
participant GraphReachTab as Container
participant graphReachApi as API Service
participant RPC as Backend RPC
participant computeGraphReach as Compute
participant GraphReachPanel as UI Panel
User->>GraphReachTab: Mount tab
GraphReachTab->>graphReachApi: loadNamespaces()
graphReachApi->>RPC: memoryListNamespaces()
RPC-->>graphReachApi: namespaces[]
graphReachApi-->>GraphReachTab: namespaces[]
GraphReachTab->>graphReachApi: loadReach(undefined)
graphReachApi->>RPC: memoryGraphQuery(undefined)
RPC-->>graphReachApi: relations[]
graphReachApi->>computeGraphReach: compute(relations)
computeGraphReach-->>graphReachApi: ReachResult
graphReachApi-->>GraphReachTab: ReachResult
GraphReachTab->>GraphReachPanel: Pass result, loading=false
GraphReachPanel->>User: Render metrics & ranked table
User->>GraphReachTab: Change namespace
GraphReachTab->>graphReachApi: loadReach(selectedNamespace)
graphReachApi->>RPC: memoryGraphQuery(selectedNamespace)
RPC-->>graphReachApi: relations[]
graphReachApi->>computeGraphReach: compute(relations)
computeGraphReach-->>graphReachApi: ReachResult
graphReachApi-->>GraphReachTab: ReachResult
GraphReachTab->>GraphReachPanel: Pass updated result
GraphReachPanel->>User: Render updated metrics
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 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. 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: 2
🤖 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/it.ts`:
- Around line 4525-4526: Update the Italian translations for the i18n keys
'graphReach.namespaceLabel' and 'graphReach.namespaceAll' in the it.ts locale so
they use the existing localized terminology: replace 'Namespace' with 'Spazio
dei nomi' for graphReach.namespaceLabel and replace 'Tutti i namespace' with
'Tutti gli spazi dei nomi' for graphReach.namespaceAll to ensure fully
localized, non-English strings.
In `@app/src/lib/i18n/pt.ts`:
- Around line 4515-4520: The new graphReach strings use PT-PT forms; update the
entries for 'graphReach.loading', 'graphReach.errorPrefix', 'graphReach.retry',
'graphReach.empty', and 'graphReach.emptyHint' to PT-BR equivalents used across
this file (e.g., change "A calcular alcance…" to "Calculando alcance…",
"regista"→"registra", "factos"→"fatos", "sobre si"→"sobre você" and adjust any
other wording to match Brazilian Portuguese tone) so the keys
graphReach.loading, graphReach.errorPrefix, graphReach.retry, graphReach.empty,
and graphReach.emptyHint are consistent with the rest of the locale.
🪄 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: 005df930-88cf-48d4-8c7b-eade6b790a77
📒 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/ar.tsapp/src/lib/i18n/bn.tsapp/src/lib/i18n/de.tsapp/src/lib/i18n/en.tsapp/src/lib/i18n/es.tsapp/src/lib/i18n/fr.tsapp/src/lib/i18n/hi.tsapp/src/lib/i18n/id.tsapp/src/lib/i18n/it.tsapp/src/lib/i18n/ko.tsapp/src/lib/i18n/pl.tsapp/src/lib/i18n/pt.tsapp/src/lib/i18n/ru.tsapp/src/lib/i18n/zh-CN.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
| 'graphReach.namespaceLabel': 'Namespace', | ||
| 'graphReach.namespaceAll': 'Tutti i namespace', |
There was a problem hiding this comment.
Localize graphReach.namespace* consistently with existing Italian terminology.
Use the same wording already used in this locale (Spazio dei nomi / Tutti gli spazi dei nomi) instead of Namespace variants to keep translations fully localized and consistent.
Suggested patch
- 'graphReach.namespaceLabel': 'Namespace',
- 'graphReach.namespaceAll': 'Tutti i namespace',
+ 'graphReach.namespaceLabel': 'Spazio dei nomi',
+ 'graphReach.namespaceAll': 'Tutti gli spazi dei nomi',As per coding guidelines, when adding/changing i18n keys in app/src/lib/i18n/en.ts, non-English locale values should be real, correct translations (not English).
🤖 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/lib/i18n/it.ts` around lines 4525 - 4526, Update the Italian
translations for the i18n keys 'graphReach.namespaceLabel' and
'graphReach.namespaceAll' in the it.ts locale so they use the existing localized
terminology: replace 'Namespace' with 'Spazio dei nomi' for
graphReach.namespaceLabel and replace 'Tutti i namespace' with 'Tutti gli spazi
dei nomi' for graphReach.namespaceAll to ensure fully localized, non-English
strings.
| 'graphReach.loading': 'A calcular alcance…', | ||
| 'graphReach.errorPrefix': 'Não foi possível carregar o grafo:', | ||
| 'graphReach.retry': 'Tentar novamente', | ||
| 'graphReach.empty': 'Ainda não há grafo de conhecimento.', | ||
| 'graphReach.emptyHint': | ||
| 'À medida que o assistente regista factos conectados sobre si, a forma e o centro da sua memória aparecerão aqui.', |
There was a problem hiding this comment.
Keep Portuguese variant consistent in new graphReach strings.
These new strings mix PT-PT forms (A calcular, regista, factos, sobre si) into a file that is otherwise PT-BR. Please align these to the locale standard used in this file for consistency.
🤖 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/lib/i18n/pt.ts` around lines 4515 - 4520, The new graphReach strings
use PT-PT forms; update the entries for 'graphReach.loading',
'graphReach.errorPrefix', 'graphReach.retry', 'graphReach.empty', and
'graphReach.emptyHint' to PT-BR equivalents used across this file (e.g., change
"A calcular alcance…" to "Calculando alcance…", "regista"→"registra",
"factos"→"fatos", "sobre si"→"sobre você" and adjust any other wording to match
Brazilian Portuguese tone) so the keys graphReach.loading,
graphReach.errorPrefix, graphReach.retry, graphReach.empty, and
graphReach.emptyHint are consistent with the rest of the locale.
sanil-23
left a comment
There was a problem hiding this comment.
@staimoorulhassan heads up — CI is currently red (E2E web lane 1/4, PR Submission Checklist, and Rust Core Coverage), so I'll hold a full sign-off until those are green. A couple of those look unrelated to a frontend-only change, so worth a re-run, but the checklist job is likely just the unchecked test-plan boxes.
The engine itself is genuinely nice work — pure, deterministic, well-documented, and the test suite covers the interesting graph shapes (paths, stars, components, self-loops, order-independence). One thing stood out that I'd like sorted before approval:
- The
MAX_REACH_NODES/isGraphReachFeasible()gate is exported but never actually called.loadReach()runscomputeGraphReach()unconditionally, so a graph above the 5k-node threshold will still freeze the tab — which is the exact failure this gate was meant to prevent. Either wire the check intoloadReach(return an over-cap signal the panel can render as a warning) or into the tab before invoking, otherwise it's dead code. There's also no test for it despite the test plan listing one.
Fix CI and wire up the gate and I'll do a proper pass. Nice contribution overall.
Summary
Addresses review findings from PR #2985 (Graph Reach — eccentricity / diameter / radius):
MAX_REACH_NODES = 5_000constant andisGraphReachFeasible()check ingraphReach.ts. BFS-from-every-node is O(V·(V+E)); beyond 5k nodes the computation could freeze the browser. Callers can pre-check feasibility.GraphReachTabwas missing fromIntelligence.tsx— added import,'reach'toIntelligenceTabunion, tab entry, and conditional render.memory.tab.reach+ 21graphReach.*keys added toen.tsand all 13 non-English locale files with real translations. French/Italian apostrophe quoting fixed (single→double quotes for strings containingl',qu',un').Test plan
pnpm typecheckpasses (verified locally)pnpm format:checkpasses (verified locally)graphReach.test.ts,GraphReachPanel.test.tsx,GraphReachTab.test.tsxpassisGraphReachFeasible()returns false for graphs with >5000 nodes