feat(intelligence): add Graph Core (k-core decomposition)#2980
Open
aashir-athar wants to merge 2 commits into
Open
feat(intelligence): add Graph Core (k-core decomposition)#2980aashir-athar wants to merge 2 commits into
aashir-athar wants to merge 2 commits into
Conversation
Contributor
|
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 (4)
🚧 Files skipped from review as they are similar to previous changes (13)
📝 WalkthroughWalkthroughAdds a deterministic k-core decomposition engine, a small API wrapper, a container tab and presentational panel with RTL tests, page integration, and translation keys across 14 locales. ChangesGraph Core Analysis Feature
Sequence Diagram(s)sequenceDiagram
participant User
participant GraphCoreTab
participant graphCoreApi
participant computeGraphCore
participant MemoryRPC
User->>GraphCoreTab: open Core tab / select namespace
GraphCoreTab->>graphCoreApi: loadNamespaces() (on mount)
GraphCoreTab->>graphCoreApi: loadCore(namespace?)
graphCoreApi->>MemoryRPC: memoryGraphQuery(namespace?)
MemoryRPC-->>graphCoreApi: GraphRelation[]
graphCoreApi->>computeGraphCore: computeGraphCore(relations)
computeGraphCore-->>graphCoreApi: CoreResult
graphCoreApi-->>GraphCoreTab: CoreResult
GraphCoreTab-->>User: render via GraphCorePanel(result)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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. 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 |
coderabbitai
Bot
added
feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
app/src/components/intelligence/GraphCorePanel.test.tsx (1)
34-34: ⚡ Quick winAvoid hardcoding i18n strings in test assertions.
Tests assert against English strings like
'No knowledge graph yet.','Entities','Connections','Shell decomposition', etc. This tightly couples tests to the current i18n implementation. If translation keys or English text change, tests break even though component behavior is correct.♻️ Use test IDs or semantic queries instead
Option 1 (preferred): Add
data-testidattributes to key elements in the component and query by test ID:// In GraphCorePanel.tsx metric tiles section - <div className="text-[10px] uppercase tracking-wider text-stone-400 dark:text-neutral-500"> + <div data-testid={`metric-${tile.label}`} className="text-[10px] uppercase tracking-wider text-stone-400 dark:text-neutral-500">Then in tests:
-expect(screen.getByText('Entities')).toBeInTheDocument(); +expect(screen.getByTestId('metric-graphCore.metricEntities')).toBeInTheDocument();Option 2: Query by ARIA roles and accessible names (already partially done for headings):
-expect(screen.getByText('Shell decomposition')).toBeInTheDocument(); +expect(screen.getByRole('heading', { name: /shell decomposition/i })).toBeInTheDocument();Also applies to: 40-40, 47-56
🤖 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/GraphCorePanel.test.tsx` at line 34, The tests in GraphCorePanel.test.tsx hardcode i18n strings (e.g. the assertion using screen.getByText('No knowledge graph yet.') and other checks for 'Entities', 'Connections', 'Shell decomposition'), making them brittle; update the tests to avoid textual assertions by replacing screen.getByText usages with semantic queries or test IDs (e.g. use getByRole/getByLabelText/getByTestId) and, if necessary, add data-testid attributes to the GraphCorePanel component for the specific elements being asserted (entities list header, connections header, empty-state node) so tests assert on stable identifiers like getByTestId('graph-empty-state') or getByRole('heading', { name: /Entities/i }) instead of English strings. Ensure all occurrences referenced in this diff (the getByText at startLine 34 and other checks around lines 40 and 47–56) are updated to use these semantic queries or test IDs.
🤖 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/components/intelligence/GraphCorePanel.test.tsx`:
- Around line 1-6: Tests render GraphCorePanel directly but the component uses
useT() and needs I18nContext; update the tests to wrap renders with an I18n
provider. Import and use the project test helper (e.g. renderWithProviders or
renderWithI18n from app/src/test/test-utils.tsx) or create a small wrapper that
supplies I18nProvider, then replace bare render(...) calls with
renderWithProviders(...) (or renderWithI18n(...)) when mounting GraphCorePanel
so useT() has the required context.
In `@app/src/components/intelligence/GraphCoreTab.test.tsx`:
- Around line 41-45: Tests render GraphCoreTab without providing I18n context
causing useT() to throw; update the three GraphCoreTab tests that call
render(<GraphCoreTab />) (the case that asserts mockLoadCore and the two other
cases) to wrap the component in the existing test i18n provider helper from
app/src/test/ (use the shared test helper that mounts components with
I18nContext) so GraphCoreTab can call useT() during render, and keep assertions
on mockLoadCore and the screen text unchanged.
In `@app/src/components/intelligence/GraphCoreTab.tsx`:
- Around line 40-47: Refactor the useEffect in GraphCoreTab.tsx to an
async/await flow: create and immediately invoke an async function inside
useEffect that awaits loadNamespaces() and calls setNamespaces(...) in the
success path, and catches errors to call setNamespaces([]); also await load('')
and handle its errors. Add diagnostic debug logs around these steps (e.g.,
before/after awaiting loadNamespaces(), on namespace success with the returned
value, on namespace failure including the caught error, before/after calling
load(''), and on load error) using the project logger used elsewhere so failures
are observable. Ensure you keep the behavior that namespace failures do not
block the core view (still setNamespaces([]) on error) and preserve the
dependency array ([load]).
In `@app/src/lib/memory/graphCore.ts`:
- Around line 71-76: The loop that strips self-loops in computeGraphCore
currently returns before registering nodes, so change the relation-handling loop
to always ensure both endpoints are registered via neighbours(subject) and
neighbours(object) (or the method that initializes node entries) before skipping
edge addition when subject === object; keep the continue for not adding
self-loop edges but do not skip node registration. Update logic in the block
that uses isRelation and neighbours to only skip adding edges for self-loops,
not node creation, and add a regression test for computeGraphCore([A→A])
asserting nodeCount === 1 and that the node has degree: 0 and coreness: 0.
---
Nitpick comments:
In `@app/src/components/intelligence/GraphCorePanel.test.tsx`:
- Line 34: The tests in GraphCorePanel.test.tsx hardcode i18n strings (e.g. the
assertion using screen.getByText('No knowledge graph yet.') and other checks for
'Entities', 'Connections', 'Shell decomposition'), making them brittle; update
the tests to avoid textual assertions by replacing screen.getByText usages with
semantic queries or test IDs (e.g. use getByRole/getByLabelText/getByTestId)
and, if necessary, add data-testid attributes to the GraphCorePanel component
for the specific elements being asserted (entities list header, connections
header, empty-state node) so tests assert on stable identifiers like
getByTestId('graph-empty-state') or getByRole('heading', { name: /Entities/i })
instead of English strings. Ensure all occurrences referenced in this diff (the
getByText at startLine 34 and other checks around lines 40 and 47–56) are
updated to use these semantic queries or test IDs.
🪄 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: ebcae14e-36a6-4009-bbba-4da74d8eea71
📒 Files selected for processing (23)
app/src/components/intelligence/GraphCorePanel.test.tsxapp/src/components/intelligence/GraphCorePanel.tsxapp/src/components/intelligence/GraphCoreTab.test.tsxapp/src/components/intelligence/GraphCoreTab.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/graphCore.test.tsapp/src/lib/memory/graphCore.tsapp/src/pages/Intelligence.tsxapp/src/services/api/graphCoreApi.test.tsapp/src/services/api/graphCoreApi.ts
Comment on lines
+1
to
+6
| import { fireEvent, render, screen } from '@testing-library/react'; | ||
| import { describe, expect, it, vi } from 'vitest'; | ||
|
|
||
| import { computeGraphCore } from '../../lib/memory/graphCore'; | ||
| import type { GraphRelation } from '../../utils/tauriCommands/memory'; | ||
| import GraphCorePanel from './GraphCorePanel'; |
Contributor
There was a problem hiding this comment.
Missing I18n context provider for component tests.
GraphCorePanel calls useT() (line 21 of the component), which requires an I18nContext provider, but these tests use bare render() from @testing-library/react without wrapping the component in any provider. The tests will fail at runtime when useT() attempts to access the missing context.
🔧 Wrap renders in I18n provider
Check if app/src/test/test-utils.tsx exports a renderWithProviders helper that includes I18nProvider. If so, import and use it:
-import { fireEvent, render, screen } from '`@testing-library/react`';
+import { fireEvent, screen } from '`@testing-library/react`';
import { describe, expect, it, vi } from 'vitest';
+import { render } from '../../test/test-utils';
import { computeGraphCore } from '../../lib/memory/graphCore';If renderWithProviders does not include I18nProvider, you can create a local wrapper:
+import { I18nProvider } from '../../lib/i18n/I18nContext';
+
+const renderWithI18n = (ui: React.ReactElement) => {
+ return render(<I18nProvider>{ui}</I18nProvider>);
+};Then replace all render(...) calls with renderWithI18n(...).
🤖 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/GraphCorePanel.test.tsx` around lines 1 - 6,
Tests render GraphCorePanel directly but the component uses useT() and needs
I18nContext; update the tests to wrap renders with an I18n provider. Import and
use the project test helper (e.g. renderWithProviders or renderWithI18n from
app/src/test/test-utils.tsx) or create a small wrapper that supplies
I18nProvider, then replace bare render(...) calls with renderWithProviders(...)
(or renderWithI18n(...)) when mounting GraphCorePanel so useT() has the required
context.
Comment on lines
+41
to
+45
| it('loads core (all namespaces) on mount and renders the result', async () => { | ||
| render(<GraphCoreTab />); | ||
| expect(mockLoadCore).toHaveBeenCalledWith(undefined); | ||
| await waitFor(() => expect(screen.getByText('Deepest-core entities')).toBeInTheDocument()); | ||
| }); |
Contributor
There was a problem hiding this comment.
Wrap component in I18nContext provider to fix missing context error.
GraphCoreTab uses useT() (line 14 of the component), but the test renders the component without providing the required I18n context. This will cause a runtime error when the component tries to access the context.
🔧 Proposed fix
+import { renderWithProviders } from '../../test/test-utils';
+
describe('<GraphCoreTab />', () => {
// ... existing setup ...
it('loads core (all namespaces) on mount and renders the result', async () => {
- render(<GraphCoreTab />);
+ renderWithProviders(<GraphCoreTab />);
expect(mockLoadCore).toHaveBeenCalledWith(undefined);
await waitFor(() => expect(screen.getByText('Deepest-core entities')).toBeInTheDocument());
});Apply the same change to the other two test cases (lines 49 and 58).
As per coding guidelines, use existing test helpers from app/src/test/.
🤖 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/GraphCoreTab.test.tsx` around lines 41 - 45,
Tests render GraphCoreTab without providing I18n context causing useT() to
throw; update the three GraphCoreTab tests that call render(<GraphCoreTab />)
(the case that asserts mockLoadCore and the two other cases) to wrap the
component in the existing test i18n provider helper from app/src/test/ (use the
shared test helper that mounts components with I18nContext) so GraphCoreTab can
call useT() during render, and keep assertions on mockLoadCore and the screen
text unchanged.
Comment on lines
+40
to
+47
| useEffect(() => { | ||
| // Namespaces are optional UI sugar; a failure to list them must not block | ||
| // the core view, so swallow that error specifically. | ||
| loadNamespaces() | ||
| .then(setNamespaces) | ||
| .catch(() => setNamespaces([])); | ||
| void load(''); | ||
| }, [load]); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Refactor namespace loading to use async/await instead of .then() chains.
The .then()/.catch() pattern on lines 44-45 violates the guideline requiring async/await for all promises in TypeScript. Refactor to use async/await with try/catch.
Additionally, verbose debug logging is required for new/changed flows per coding guidelines, but this new async load flow lacks diagnostic logs for namespace fetching.
♻️ Proposed refactor
+import debug from 'debug';
+
+const log = debug('openhuman:intelligence:graph-core');
+
const GraphCoreTab = () => {
// ... existing state ...
useEffect(() => {
- // Namespaces are optional UI sugar; a failure to list them must not block
- // the core view, so swallow that error specifically.
- loadNamespaces()
- .then(setNamespaces)
- .catch(() => setNamespaces([]));
+ const fetchNamespaces = async () => {
+ try {
+ const ns = await loadNamespaces();
+ log('loaded %d namespaces', ns.length);
+ setNamespaces(ns);
+ } catch (err) {
+ log('namespace load failed (non-blocking): %O', err);
+ setNamespaces([]);
+ }
+ };
+ void fetchNamespaces();
void load('');
}, [load]);As per coding guidelines, use async/await for promises and add namespaced debug logs for new 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/GraphCoreTab.tsx` around lines 40 - 47,
Refactor the useEffect in GraphCoreTab.tsx to an async/await flow: create and
immediately invoke an async function inside useEffect that awaits
loadNamespaces() and calls setNamespaces(...) in the success path, and catches
errors to call setNamespaces([]); also await load('') and handle its errors. Add
diagnostic debug logs around these steps (e.g., before/after awaiting
loadNamespaces(), on namespace success with the returned value, on namespace
failure including the caught error, before/after calling load(''), and on load
error) using the project logger used elsewhere so failures are observable.
Ensure you keep the behavior that namespace failures do not block the core view
(still setNamespaces([]) on error) and preserve the dependency array ([load]).
aashir-athar
added a commit
to aashir-athar/openhuman
that referenced
this pull request
May 29, 2026
Address CodeRabbit review on tinyhumansai#2980: previously `computeGraphCore([A→A])` returned an empty graph (the entire relation was dropped, taking the endpoint with it), so a user whose only recorded fact was "Alice→Alice" would vanish into the empty state. Now the self-loop EDGE is still dropped (a self-loop is not a neighbour and cannot deepen a core), but the endpoint is REGISTERED as a node with degree 0 and coreness 0 — so Alice appears in the graph. Adds a regression test for the self-loop-only case, updates the buildAdjacency comment, and refines the module header to spell out the new node-vs-edge distinction. The "missing I18nContext provider" criticals were verified as false positives: I18nContext at lib/i18n/I18nContext.tsx:66 ships a fully functional English default that useT() picks up without a provider, so the existing test pattern (matching the merged graphCentrality tests) works correctly — every test passes. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
A new read-only "Core" tab for the Intelligence view: k-core decomposition of the memory knowledge graph. It separates the load-bearing CORE of the user's knowledge — entities mutually reinforced by many interlinked facts — from the PERIPHERY of leaves and bridges. A high-degree hub of one-off facts still has coreness 1; depth, not degree, marks the core — something neither a frequency count nor PageRank can express. Engine (pure, deterministic — no React/RPC/clock/RNG): - coreness per node via the Batagelj-Zaversnik O(V+E) bucket algorithm, - degeneracy (max coreness) and the shell decomposition, - kCoreSize(k) for the nested k-core sizes. Coreness is a graph invariant and the peel is seeded from an id-sorted vertex list, 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>
Address CodeRabbit review on tinyhumansai#2980: previously `computeGraphCore([A→A])` returned an empty graph (the entire relation was dropped, taking the endpoint with it), so a user whose only recorded fact was "Alice→Alice" would vanish into the empty state. Now the self-loop EDGE is still dropped (a self-loop is not a neighbour and cannot deepen a core), but the endpoint is REGISTERED as a node with degree 0 and coreness 0 — so Alice appears in the graph. Adds a regression test for the self-loop-only case, updates the buildAdjacency comment, and refines the module header to spell out the new node-vs-edge distinction. The "missing I18nContext provider" criticals were verified as false positives: I18nContext at lib/i18n/I18nContext.tsx:66 ships a fully functional English default that useT() picks up without a provider, so the existing test pattern (matching the merged graphCentrality tests) works correctly — every test passes. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
aashir-athar
force-pushed
the
feat/graph-core
branch
from
c597bc9 to
e7af154
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a new read-only "Core" tab to the Intelligence view: k-core decomposition of the memory knowledge graph. Where Centrality asks "which entities are important" (PageRank), this lens asks "how deep in the densely-connected core does each entity sit" — separating the load-bearing core of the user's knowledge (entities mutually reinforced by many interlinked facts) from the periphery of leaves and bridges.
The key insight: a high-degree hub of one-off facts still has coreness 1, while a modest entity embedded in a dense cluster has high coreness. Depth, not degree, marks the core — and neither a frequency count nor PageRank captures it.
Design
lib/memory/graphCore.ts): treats the(subject)-[predicate]->(object)triples as an undirected simple graph (direction dropped, parallel edges collapsed, self-loops dropped) and computes:kCoreSize(k)for the nested k-core sizes.memoryGraphQuery/memoryListNamespacesJSON-RPC wrappers. Read-only — recomputed live, never persisted.Test plan
vitest— 28 tests (engine: empty / triangle (uniform 2-core) / path (1-core) / star-tree (hub high-degree but coreness 1) / K4 (uniform 3-core) / triangle-with-pendant (core=2, pendant=1) with hand-derived coreness; shell decomposition;kCoreSizenested cores; self-loop drop; parallel-edge & direction collapse; malformed-row drop; no case-folding; order-independent output — plus api facade, panel states + shell histogram + core badge, container load/namespace-requery/error)tsc --noEmit— cleaneslint— 0 errorsprettier --check— clean🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Localization
Tests