Repository navigation
feat(intelligence): add Relationship Types - #2951
aashir-athar wants to merge 4 commits into
Conversation
|
Caution Review failedFailed to post review comments 📝 WalkthroughWalkthroughAdds relationship-type profiling: a deterministic analytics engine, API facade, presentational panel, tab container with namespace selection and retry, tests for each piece, and i18n entries across 16 locales. ChangesRelationship Types Analysis
Sequence DiagramsequenceDiagram
participant UI as RelationshipTypesTab
participant API as relationshipTypesApi
participant RPC as memoryGraphQuery
participant Compute as computeRelationshipTypes
UI->>API: loadRelationshipTypes(namespace?)
API->>RPC: memoryGraphQuery(namespace?)
RPC-->>API: GraphRelation[] (relations)
API->>Compute: computeRelationshipTypes(relations)
Compute-->>API: RelationshipReport
API-->>UI: RelationshipReport
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. 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}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/components/intelligence/RelationshipTypesTab.tsx (1)
22-48: 💤 Low valueStale-response guard and async flow look correct. The monotonic
latestRequestIdtoken properly discards out-of-order responses, and loading/error transitions are gated on the latest request.One minor guideline note: lines 38-41 use
.then()/.catch()forloadNamespaces()rather thanasync/await.♻️ Optional: align with async/await convention
useEffect(() => { - loadNamespaces() - .then(setNamespaces) - .catch(() => setNamespaces([])); + const initNamespaces = async (): Promise<void> => { + try { + setNamespaces(await loadNamespaces()); + } catch { + setNamespaces([]); + } + }; + void initNamespaces(); void load(''); }, [load]);As per coding guidelines: "Always use async/await for promises in JavaScript/TypeScript code".
🤖 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/RelationshipTypesTab.tsx` around lines 22 - 48, The useEffect currently calls loadNamespaces() using .then/.catch; change it to an async/await flow to match project conventions: create an async function inside useEffect (or mark the effect callback async via an inner async IIFE), await loadNamespaces(), call setNamespaces with the result, and catch errors to setNamespaces([]); keep the existing void load('') call and preserve the stale-response guard in load (latestRequestId, setLoading/setError). Target symbols: useEffect block, loadNamespaces(), setNamespaces, load, latestRequestId, and handleNamespace.
🤖 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.
Nitpick comments:
In `@app/src/components/intelligence/RelationshipTypesTab.tsx`:
- Around line 22-48: The useEffect currently calls loadNamespaces() using
.then/.catch; change it to an async/await flow to match project conventions:
create an async function inside useEffect (or mark the effect callback async via
an inner async IIFE), await loadNamespaces(), call setNamespaces with the
result, and catch errors to setNamespaces([]); keep the existing void load('')
call and preserve the stale-response guard in load (latestRequestId,
setLoading/setError). Target symbols: useEffect block, loadNamespaces(),
setNamespaces, load, latestRequestId, and handleNamespace.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6be10365-594d-47ef-9ec2-15dec6cf7af6
📒 Files selected for processing (23)
app/src/components/intelligence/RelationshipTypesPanel.test.tsxapp/src/components/intelligence/RelationshipTypesPanel.tsxapp/src/components/intelligence/RelationshipTypesTab.test.tsxapp/src/components/intelligence/RelationshipTypesTab.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/relationshipTypes.test.tsapp/src/lib/memory/relationshipTypes.tsapp/src/pages/Intelligence.tsxapp/src/services/api/relationshipTypesApi.test.tsapp/src/services/api/relationshipTypesApi.ts
fa1bb04 to
71ca662
Compare
71ca662 to
951e2a2
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
app/src/lib/memory/relationshipTypes.ts (1)
75-75: ⚡ Quick winAvoid returning a shared mutable empty report instance.
Returning
EMPTY_REPORTby reference allows accidental external mutation (e.g.,predicates.push(...)) that affects later calls. Return a fresh object (or freeze the constant) to preserve the function’s purity contract.🤖 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/memory/relationshipTypes.ts` at line 75, The code currently returns the shared constant EMPTY_REPORT when totalEdges === 0 which allows callers to mutate that shared object; modify the early-return to return a fresh report instance instead (e.g., return { ...EMPTY_REPORT } or create a new object with the same shape) or freeze EMPTY_REPORT at its declaration (Object.freeze) and ensure nested structures are also frozen if needed; update the return at the totalEdges check (the line using EMPTY_REPORT) so callers cannot mutate shared state, and reference EMPTY_REPORT and the totalEdges check when making the change.
🤖 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/de.ts`:
- Around line 4249-4265: The German locale file contains new relationshipTypes
keys still in English; update each key (e.g., relationshipTypes.empty,
relationshipTypes.emptyHint, relationshipTypes.errorPrefix,
relationshipTypes.heading, relationshipTypes.intro, relationshipTypes.loading,
relationshipTypes.metricEdges, relationshipTypes.metricPredicates,
relationshipTypes.metricReciprocity, relationshipTypes.namespaceAll,
relationshipTypes.namespaceLabel, relationshipTypes.reciprocalShort,
relationshipTypes.reciprocalTitle, relationshipTypes.retry,
relationshipTypes.title, relationshipTypes.truncated) with proper German
translations that match the meaning and placeholders of the English originals so
German users see localized UI copy.
In `@app/src/lib/i18n/es.ts`:
- Around line 4215-4231: Replace the English values for the keys introduced here
(e.g., memory.tab.relationships, relationshipTypes.empty,
relationshipTypes.emptyHint, relationshipTypes.errorPrefix,
relationshipTypes.heading, relationshipTypes.intro, relationshipTypes.loading,
relationshipTypes.metricEdges, relationshipTypes.metricPredicates,
relationshipTypes.metricReciprocity, relationshipTypes.namespaceAll,
relationshipTypes.namespaceLabel, relationshipTypes.reciprocalShort,
relationshipTypes.reciprocalTitle, relationshipTypes.retry,
relationshipTypes.title, relationshipTypes.truncated) with correct Spanish
translations; keep the i18n keys unchanged, preserve all interpolation
placeholders like {rate}, {count}, {shown}, {total} and punctuation/ellipsis
semantics, and ensure the translations are natural Spanish (not English) and
grammatically correct for UI strings.
In `@app/src/lib/i18n/fr.ts`:
- Around line 4232-4248: The French locale file contains new keys under
relationshipTypes (e.g., 'relationshipTypes.empty', 'relationshipTypes.intro',
'relationshipTypes.title', 'relationshipTypes.reciprocalShort', etc.) that are
still in English; update app/src/lib/i18n/fr.ts by replacing each
'relationshipTypes.*' English value with correct French translations for all
added keys so the FR locale matches the EN keys and avoids mixed-language UI;
ensure placeholders like {rate}, {count}, {shown}, and {total} are preserved
exactly in the translated strings and mirror the same punctuation and
capitalization as the EN originals.
In `@app/src/lib/i18n/hi.ts`:
- Around line 4145-4161: The Hindi locale file is missing real translations for
the new i18n keys; replace the English strings for keys such as
'memory.tab.relationships' and every 'relationshipTypes.*' entry
('relationshipTypes.empty', 'relationshipTypes.emptyHint',
'relationshipTypes.errorPrefix', 'relationshipTypes.heading',
'relationshipTypes.intro', 'relationshipTypes.loading',
'relationshipTypes.metricEdges', 'relationshipTypes.metricPredicates',
'relationshipTypes.metricReciprocity', 'relationshipTypes.namespaceAll',
'relationshipTypes.namespaceLabel', 'relationshipTypes.reciprocalShort',
'relationshipTypes.reciprocalTitle', 'relationshipTypes.retry',
'relationshipTypes.title', 'relationshipTypes.truncated') with correct Hindi
translations, making sure to keep and correctly position interpolation
placeholders like {rate}, {count}, {shown}, and {total} intact. Ensure
translations are natural, context-appropriate, and follow existing Hindi locale
style/terminology.
In `@app/src/lib/i18n/id.ts`:
- Around line 4155-4171: The new relationshipTypes i18n entries are still in
English; update each key (e.g. relationshipTypes.empty,
relationshipTypes.emptyHint, relationshipTypes.errorPrefix,
relationshipTypes.heading, relationshipTypes.intro, relationshipTypes.loading,
relationshipTypes.metricEdges, relationshipTypes.metricPredicates,
relationshipTypes.metricReciprocity, relationshipTypes.namespaceAll,
relationshipTypes.namespaceLabel, relationshipTypes.reciprocalShort,
relationshipTypes.reciprocalTitle, relationshipTypes.retry,
relationshipTypes.title, relationshipTypes.truncated) with correct Indonesian
translations (preserve placeholders like {rate}, {count}, {shown}, {total}) so
the Indonesian locale shows real translations rather than English fallbacks.
In `@app/src/lib/i18n/ko.ts`:
- Around line 4108-4124: Replace the English placeholder strings for the listed
i18n keys with proper Korean translations: memory.tab.relationships,
relationshipTypes.empty, relationshipTypes.emptyHint,
relationshipTypes.errorPrefix, relationshipTypes.heading,
relationshipTypes.intro, relationshipTypes.loading,
relationshipTypes.metricEdges, relationshipTypes.metricPredicates,
relationshipTypes.metricReciprocity, relationshipTypes.namespaceAll,
relationshipTypes.namespaceLabel, relationshipTypes.reciprocalShort,
relationshipTypes.reciprocalTitle, relationshipTypes.retry,
relationshipTypes.title, relationshipTypes.truncated; keep any interpolation
tokens intact (e.g., {rate}, {count}, {shown}, {total}) and update each value to
an accurate Korean string so the Korean locale shows translated text instead of
English.
In `@app/src/lib/i18n/pl.ts`:
- Around line 4211-4227: The new relationshipTypes.* i18n keys were added in
English but not translated in pl.ts; update app/src/lib/i18n/pl.ts by adding
Polish translations for each of the keys shown (e.g., 'relationshipTypes.empty',
'relationshipTypes.emptyHint', 'relationshipTypes.errorPrefix',
'relationshipTypes.heading', 'relationshipTypes.intro',
'relationshipTypes.loading', 'relationshipTypes.metricEdges',
'relationshipTypes.metricPredicates', 'relationshipTypes.metricReciprocity',
'relationshipTypes.namespaceAll', 'relationshipTypes.namespaceLabel',
'relationshipTypes.reciprocalShort', 'relationshipTypes.reciprocalTitle',
'relationshipTypes.retry', 'relationshipTypes.title',
'relationshipTypes.truncated') replacing the English strings with correct Polish
text so the Relationships tab is fully localized.
In `@app/src/lib/i18n/pt.ts`:
- Around line 4210-4226: The new relationshipTypes i18n keys are still in
English; update the Portuguese translations for the keys
relationshipTypes.empty, relationshipTypes.emptyHint,
relationshipTypes.errorPrefix, relationshipTypes.heading,
relationshipTypes.intro, relationshipTypes.loading,
relationshipTypes.metricEdges, relationshipTypes.metricPredicates,
relationshipTypes.metricReciprocity, relationshipTypes.namespaceAll,
relationshipTypes.namespaceLabel, relationshipTypes.reciprocalShort,
relationshipTypes.reciprocalTitle, relationshipTypes.retry,
relationshipTypes.title, and relationshipTypes.truncated by replacing the
English strings with correct Portuguese translations (keep placeholders like
{rate}, {count}, {shown}, {total} intact and positioned appropriately). Ensure
the translations are idiomatic and preserve meaning/placeholder usage so the UI
displays fully localized text.
In `@app/src/lib/i18n/ru.ts`:
- Around line 4179-4195: The listed i18n keys (memory.tab.relationships and all
relationshipTypes.* entries) are still in English in ru.ts; replace each English
string with a correct Russian translation while preserving interpolation
placeholders ({rate}, {count}, {shown}, {total}) and punctuation/ellipsis
exactly as in the originals so formatting and runtime substitution remain
correct; update entries like relationshipTypes.empty, .emptyHint, .errorPrefix,
.heading, .intro, .loading, .metricEdges, .metricPredicates, .metricReciprocity,
.namespaceAll, .namespaceLabel, .reciprocalShort, .reciprocalTitle, .retry,
.title, and .truncated with proper Russian text.
---
Nitpick comments:
In `@app/src/lib/memory/relationshipTypes.ts`:
- Line 75: The code currently returns the shared constant EMPTY_REPORT when
totalEdges === 0 which allows callers to mutate that shared object; modify the
early-return to return a fresh report instance instead (e.g., return {
...EMPTY_REPORT } or create a new object with the same shape) or freeze
EMPTY_REPORT at its declaration (Object.freeze) and ensure nested structures are
also frozen if needed; update the return at the totalEdges check (the line using
EMPTY_REPORT) so callers cannot mutate shared state, and reference EMPTY_REPORT
and the totalEdges check when making the change.
🪄 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: 5837c563-95ef-4c7b-a480-38cbfa565435
📒 Files selected for processing (23)
app/src/components/intelligence/RelationshipTypesPanel.test.tsxapp/src/components/intelligence/RelationshipTypesPanel.tsxapp/src/components/intelligence/RelationshipTypesTab.test.tsxapp/src/components/intelligence/RelationshipTypesTab.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/relationshipTypes.test.tsapp/src/lib/memory/relationshipTypes.tsapp/src/pages/Intelligence.tsxapp/src/services/api/relationshipTypesApi.test.tsapp/src/services/api/relationshipTypesApi.ts
✅ Files skipped from review due to trivial changes (4)
- app/src/lib/i18n/zh-CN.ts
- app/src/lib/i18n/it.ts
- app/src/lib/i18n/bn.ts
- app/src/lib/i18n/ar.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- app/src/services/api/relationshipTypesApi.test.ts
- app/src/pages/Intelligence.tsx
- app/src/lib/i18n/en.ts
- app/src/components/intelligence/RelationshipTypesTab.tsx
- app/src/components/intelligence/RelationshipTypesTab.test.tsx
- app/src/lib/memory/relationshipTypes.test.ts
- app/src/services/api/relationshipTypesApi.ts
- app/src/components/intelligence/RelationshipTypesPanel.tsx
sanil-23
left a comment
sanil-23
left a comment
There was a problem hiding this comment.
Really nicely built feature. The engine in relationshipTypes.ts is pure, deterministic, and well-documented; the container/presentational split is clean; the monotonic request-token guard in RelationshipTypesTab correctly drops stale responses; and it adds zero new core surface by composing the existing memoryGraphQuery / memoryListNamespaces wrappers. Tests, a11y roles, and loading/empty/error states are all there. Good work.
One blocker before this can merge: the i18n is not actually localized. The PR description says "i18n across all 13 locales," but every non-English locale file ships the English strings verbatim under relationshipTypes.* and memory.tab.relationships. CodeRabbit already flagged de, es, fr, hi, id, ko, pl, pt, and ru. It missed four that have the same problem: ar, bn, it, and zh-CN — inline comments below. Per the repo guideline, every non-English locale needs a real translation, not the English fallback. Once all 13 carry genuine translations (placeholders like {rate}, {count}, {shown}, {total} preserved), I'll re-review and approve.
| Area | Assessment |
|---|---|
| Engine / logic | Clean, pure, deterministic — no issues |
| Integration | Correct; reuses existing RPC wrappers, no new core surface |
| Tests | Present across engine, api, panel, container |
| i18n | Blocking — all 13 non-English locales contain untranslated English |
| Security / breaking risk | None — read-only, additive |
951e2a2 to
02b5796
Compare
sanil-23
left a comment
sanil-23
left a comment
There was a problem hiding this comment.
@aashir-athar heads up — CI is still failing (Frontend Quality: Prettier check) on the new commit, so I'll hold a full re-review until that's green. Also, the new commit doesn't appear to address any of the locale translations — all 13 open threads (9 from CodeRabbit on de/es/fr/hi/id/ko/pl/pt/ru, and my 4 on ar/bn/it/zh-CN) remain unresolved with English placeholder copy.
Two things to fix before I re-review:
- Run
pnpm prettier --writeon the affected files and commit the formatting fix so the Prettier check passes. - Provide real translations (not English copy) for all 13 non-English locales — see the open review threads for the specific keys and files.
Once CI is green and the locale threads are addressed, I'll come back and re-review.
581fa27 to
4af0671
Compare
A new read-only "Relationships" tab that profiles the relationship VOCABULARY of the memory graph — which predicates connect facts, how often, and how often a relationship is mutual (reciprocated) — a lens distinct from the entity- centric views. - Pure deterministic engine (lib/memory/relationshipTypes.ts): per-predicate edge counts + reciprocity (an edge (s,p,o) is reciprocated when its mirror (o,p,s) exists, s != o; self-loops excluded). No clock, no RNG; duplicate triples collapse via canonical JSON keys; output sorted count desc then predicate asc (stable, order-invariant). - Zero new core surface: reuses memoryGraphQuery + memoryListNamespaces. Read-only, recomputed from the live graph. - Container guards the load with a request token; namespace selector; metric tiles (edges / predicates / overall reciprocity) + a ranked predicate list with count bars and per-predicate reciprocity, capped with a truncation note. i18n across all 13 locales. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
4af0671 to
7a751e0
Compare
sanil-23
left a comment
sanil-23
left a comment
There was a problem hiding this comment.
@aashir-athar CI is not fully green — Rust Core Coverage is failing and two Playwright lanes are failing (lanes 1 and 2 of 4), so I can not approve until those are resolved. The Playwright E2E Gate itself passed, so the E2E lane failures may be flaky, but the Rust coverage failure needs a look even though this PR does not touch Rust (could be a pre-existing breakage on the base branch, worth confirming).
Good progress on the translations. The relationshipTypes.* keys are now properly translated in all 13 locales — Arabic, Bengali, Italian, Simplified Chinese all have real translations, and the locales CodeRabbit flagged earlier appear addressed.
One outstanding question from my inline threads: memory.tab.relationships is still 'Relationships' (English) in every non-English locale — ar, bn, de, es, fr, hi, id, it, ko, pl, pt, ru, zh-CN — all 13 carry this key in English while every other key in the block is translated. Was this intentional (treating the tab label as a fixed product name), or was it missed? My open threads on ar.ts, bn.ts, it.ts, and zh-CN.ts are waiting on this. If it is intentional, just say so and I will close those threads. If it was missed, the fix is the same 1-line change in each file.
Fix CI and clarify the tab label, and I will do a proper approval pass.
|
Upstream maintainer action needed: the fresh CI workflows for this branch are queued, and GitHub is showing 3 workflows awaiting approval. Please click Approve and run workflows so the required checks can execute. The branch has been synced with the current |
sanil-23
left a comment
sanil-23
left a comment
There was a problem hiding this comment.
All previous changes addressed — approving.
The four locales I flagged (ar, bn, it, zh-CN) now have real translations with placeholders intact. All nine CodeRabbit-flagged locales were also fixed in an earlier commit. CI is green across all checks. No new issues in the translation commit.
The core feature itself (pure engine, container/presentational split, RPC reuse, 20 tests) was clean from the start. Good work getting the i18n parity sorted.
5 participants
Summary
Adds a new read-only "Relationships" tab that profiles the relationship vocabulary of the memory graph — which predicates connect facts, how often, and how often a relationship is mutual (reciprocated). Where the other Intelligence lenses focus on entities, this one focuses on the edges: high reciprocity (e.g. "knows") signals symmetric relations; low reciprocity (e.g. "works_at") signals directional facts.
Design
lib/memory/relationshipTypes.ts): per-predicate edge counts + reciprocity, where an edge(s,p,o)is reciprocated when its mirror(o,p,s)also exists ands ≠ o(self-loops are counted as edges but excluded from reciprocity). No clock, no RNG. Duplicate triples collapse via canonicalJSON.stringifykeys; output is sorted count-desc then predicate-asc (stable, relation-order-invariant);reciprocityRateguards against divide-by-zero.memoryGraphQueryandmemoryListNamespaceswrappers. Read-only — recomputed from the live graph on mount.This was adversarially reviewed (correctness / a11y / integration lenses) before opening; the one confirmed finding — a test-coverage gap where only the
predicatebranch of the malformed-row guard was exercised — is fixed (the test now pins thesubjectandobjectbranches too).Test plan
vitest— 20 tests (engine: counts/reciprocity/self-loop/dedup/tie-break/order-invariance/malformed; api facade; panel states; container load + selector + error)tsc --noEmit— cleaneslint— 0 errorsprettier --check— cleanrelationshipTypes.*keys🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests