feat(memory): route summaries through workspace paths#2527
Merged
senamakel merged 5 commits intoMay 25, 2026
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR routes Memory open/reveal/preview through shared workspace-path utilities, removes MemoryGraph's contentRootAbs prop, adds hover-driven summary preview UI with truncation/error handling, updates MemoryWorkspace to reveal the workspace base, and revises tests to assert the new workspace-path interactions. ChangesMemory workspace-path migration
Sequence Diagram(s)sequenceDiagram
participant User
participant MemoryGraph
participant summaryWorkspacePath
participant previewWorkspaceText
participant openWorkspacePath
User->>MemoryGraph: hover summary node
MemoryGraph->>summaryWorkspacePath: compute workspace path
MemoryGraph->>User: show tooltip with path + preview button
User->>MemoryGraph: click preview
MemoryGraph->>previewWorkspaceText: fetch preview for path
previewWorkspaceText-->>MemoryGraph: preview content / error / truncation
MemoryGraph->>User: render preview panel
User->>MemoryGraph: click node (open)
MemoryGraph->>openWorkspacePath: open workspace path
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/components/intelligence/MemoryGraph.test.tsx (1)
134-157: ⚡ Quick winAdd a preview-failure test for the new error surface.
MemoryGraphnow renders preview errors, but this suite only asserts the success path. Add one rejection-path test forpreviewWorkspaceTextand assert the error text is shown inmemory-graph-preview.Proposed test addition
+ it('shows preview error text when workspace preview fails', async () => { + mocks.previewWorkspaceText.mockRejectedValueOnce(new Error('preview failed')); + const nodes = [ + makeSummaryNode({ + id: 'sum-A', + tree_kind: 'topic', + tree_scope: 'workspace one', + level: 2, + file_basename: 'summary-A', + }), + ]; + render(<MemoryGraph nodes={nodes} edges={[]} mode="tree" />); + + fireEvent.mouseEnter(screen.getByTestId('memory-graph-node-sum-A')); + fireEvent.click(screen.getByTestId('memory-graph-preview-sum-A')); + + expect(await screen.findByTestId('memory-graph-preview')).toHaveTextContent('preview failed'); + });🤖 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/MemoryGraph.test.tsx` around lines 134 - 157, Add a rejection-path test for MemoryGraph that mocks mocks.previewWorkspaceText to return a rejected Promise (e.g., Promise.reject(new Error('boom'))), then render MemoryGraph with the same makeSummaryNode, fire mouseEnter on memory-graph-node-sum-A and click memory-graph-preview-sum-A, await the mock call (using waitFor) and finally assert that screen.findByTestId('memory-graph-preview') contains the error text (e.g., 'boom' or the expected error message) to verify the error surface is rendered; reference MemoryGraph, mocks.previewWorkspaceText, memory-graph-node-sum-A, memory-graph-preview-sum-A, and memory-graph-preview when locating code to modify.
🤖 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/MemoryGraph.test.tsx`:
- Around line 134-157: Add a rejection-path test for MemoryGraph that mocks
mocks.previewWorkspaceText to return a rejected Promise (e.g.,
Promise.reject(new Error('boom'))), then render MemoryGraph with the same
makeSummaryNode, fire mouseEnter on memory-graph-node-sum-A and click
memory-graph-preview-sum-A, await the mock call (using waitFor) and finally
assert that screen.findByTestId('memory-graph-preview') contains the error text
(e.g., 'boom' or the expected error message) to verify the error surface is
rendered; reference MemoryGraph, mocks.previewWorkspaceText,
memory-graph-node-sum-A, memory-graph-preview-sum-A, and memory-graph-preview
when locating code to modify.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1ed3a6e9-47c1-4319-b715-43d5ef9d5c39
📒 Files selected for processing (5)
app/src/components/intelligence/MemoryGraph.test.tsxapp/src/components/intelligence/MemoryGraph.tsxapp/src/components/intelligence/MemoryWorkspace.tsxapp/src/components/intelligence/__tests__/MemoryWorkspace.test.tsxapp/src/components/intelligence/memoryWorkspacePaths.ts
coderabbitai
Bot
added
the
feature
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/components/intelligence/memoryWorkspacePaths.test.ts (1)
22-48: ⚡ Quick winConsider adding tests for additional edge cases.
The suite provides good baseline coverage but is missing tests for:
- Gmail scope handling:
tree_kind: 'source'withtree_scope: 'gmail:user@example.com'should strip thegmail:prefix and slugify only the remainder.- Valid timestamp formatting: A realistic
time_range_start_ms(e.g.,new Date('2024-06-15').getTime()) should produce a formatted date likeglobal-2024-06-15in the path.These cases exercise important branches in
summaryScopeSlugand the happy path ofdateFromMs.📋 Suggested additional tests
it('strips gmail: prefix from source scopes', () => { expect( summaryWorkspacePath( summaryNode({ tree_kind: 'source', tree_scope: 'gmail:user@example.com' }) ) ).toBe(`${MEMORY_CONTENT_WORKSPACE_PATH}/wiki/summaries/source-user-example-com/L1/summary-file.md`); }); it('formats valid timestamps as ISO dates for global summaries', () => { const ms = new Date('2024-06-15T12:00:00Z').getTime(); expect( summaryWorkspacePath(summaryNode({ tree_kind: 'global', time_range_start_ms: ms })) ).toBe(`${MEMORY_CONTENT_WORKSPACE_PATH}/wiki/summaries/global-2024-06-15/L1/summary-file.md`); });Based on learnings from the upstream contract in memoryWorkspacePaths.ts:47-56, which shows the gmail: prefix stripping logic in
summaryScopeSlug.🤖 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/memoryWorkspacePaths.test.ts` around lines 22 - 48, Add two unit tests to cover the missing branches: one that verifies summaryWorkspacePath(summaryNode({ tree_kind: 'source', tree_scope: 'gmail:user@example.com' })) strips the "gmail:" prefix and slugifies the remainder (exercise summaryScopeSlug), and another that passes a realistic timestamp (e.g., new Date('2024-06-15T12:00:00Z').getTime()) to summaryWorkspacePath(summaryNode({ tree_kind: 'global', time_range_start_ms: ms })) and asserts the path contains the formatted date segment (exercise dateFromMs/ISO formatting); use the same MEMORY_CONTENT_WORKSPACE_PATH and summaryNode helper so the new tests match existing patterns.
🤖 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/memoryWorkspacePaths.test.ts`:
- Around line 23-26: Add tests to cover the remaining null-return cases for
summaryWorkspacePath: create summary nodes where tree_kind is null/undefined and
where level is null/undefined (use the existing summaryNode helper to build
these variants) and assert summaryWorkspacePath(...) returns null for each; keep
them alongside the existing test that checks kind !== 'summary' and missing
file_basename so all four contract conditions (!node.tree_kind, node.level ==
null, missing file_basename, kind !== 'summary') are asserted.
---
Nitpick comments:
In `@app/src/components/intelligence/memoryWorkspacePaths.test.ts`:
- Around line 22-48: Add two unit tests to cover the missing branches: one that
verifies summaryWorkspacePath(summaryNode({ tree_kind: 'source', tree_scope:
'gmail:user@example.com' })) strips the "gmail:" prefix and slugifies the
remainder (exercise summaryScopeSlug), and another that passes a realistic
timestamp (e.g., new Date('2024-06-15T12:00:00Z').getTime()) to
summaryWorkspacePath(summaryNode({ tree_kind: 'global', time_range_start_ms: ms
})) and asserts the path contains the formatted date segment (exercise
dateFromMs/ISO formatting); use the same MEMORY_CONTENT_WORKSPACE_PATH and
summaryNode helper so the new tests match existing patterns.
🪄 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: 3a287136-4df7-4f80-9f57-a631d257b68a
📒 Files selected for processing (2)
app/src/components/intelligence/MemoryGraph.test.tsxapp/src/components/intelligence/memoryWorkspacePaths.test.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/src/components/intelligence/memoryWorkspacePaths.test.ts (2)
42-50: ⚡ Quick winIncomplete coverage of gmail-prefix conditional logic.
The test validates that
gmail:is stripped for source scopes, but doesn't verify the conditional behavior: the upstream contract stripsgmail:only whentree_kind === 'source'AND the scope starts withgmail:. Add negative cases to confirm:
- A non-gmail source scope (e.g.,
dropbox:foo) is not stripped- A gmail-prefixed non-source scope (e.g.,
tree_kind: 'topic', tree_scope: 'gmail:foo') is not stripped📋 Suggested additional test cases
it('strips gmail prefixes from source scopes before slugifying', () => { expect( summaryWorkspacePath( summaryNode({ tree_kind: 'source', tree_scope: 'gmail:user@example.com' }) ) ).toBe( `${MEMORY_CONTENT_WORKSPACE_PATH}/wiki/summaries/source-user-example-com/L1/summary-file.md` ); + // Non-gmail source scope should preserve the prefix + expect( + summaryWorkspacePath( + summaryNode({ tree_kind: 'source', tree_scope: 'dropbox:Documents' }) + ) + ).toBe( + `${MEMORY_CONTENT_WORKSPACE_PATH}/wiki/summaries/source-dropbox-documents/L1/summary-file.md` + ); + // Gmail prefix on non-source should NOT be stripped + expect( + summaryWorkspacePath( + summaryNode({ tree_kind: 'topic', tree_scope: 'gmail:work' }) + ) + ).toBe( + `${MEMORY_CONTENT_WORKSPACE_PATH}/wiki/summaries/topic-gmail-work/L1/summary-file.md` + ); });🤖 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/memoryWorkspacePaths.test.ts` around lines 42 - 50, Add two negative tests to cover the conditional gmail-prefix stripping: one where tree_kind is 'source' but tree_scope is a non-gmail prefix (e.g., 'dropbox:foo') and one where tree_scope starts with 'gmail:' but tree_kind is not 'source' (e.g., tree_kind: 'topic', tree_scope: 'gmail:foo'); in both cases assert that summaryWorkspacePath(summaryNode(...)) produces a path that preserves the original scope (i.e., no removal of 'dropbox:' or 'gmail:') and matches the expected string built from MEMORY_CONTENT_WORKSPACE_PATH and the slugified scope used in the existing test pattern, reusing the same helpers summaryWorkspacePath and summaryNode to locate the behavior.
67-72: ⚡ Quick winNon-obvious edge case could use a comment.
The test correctly expects
Number.MAX_SAFE_INTEGERto produce'unknown-date', but this is non-obvious because MAX_SAFE_INTEGER appears to be a valid number. It fails because it exceeds JavaScript's Date valid range (±8.64×10¹⁵ ms), creating an Invalid Date. Adding a brief comment would help future readers understand this edge case.📝 Suggested clarifying comment
expect( summaryWorkspacePath( + // MAX_SAFE_INTEGER exceeds Date valid range (±8.64e15 ms), producing Invalid Date summaryNode({ tree_kind: 'global', time_range_start_ms: Number.MAX_SAFE_INTEGER }) ) ).toBe( `${MEMORY_CONTENT_WORKSPACE_PATH}/wiki/summaries/global-unknown-date/L1/summary-file.md` );🤖 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/memoryWorkspacePaths.test.ts` around lines 67 - 72, Add a brief clarifying comment above the assertion that uses Number.MAX_SAFE_INTEGER in memoryWorkspacePaths.test.ts to explain why MAX_SAFE_INTEGER maps to 'unknown-date' despite being a number: note that Date(Number.MAX_SAFE_INTEGER) is out of JS Date valid range (±8.64×10^15 ms) and becomes Invalid Date, so summaryWorkspacePath(summaryNode({... time_range_start_ms: Number.MAX_SAFE_INTEGER })) should produce the 'unknown-date' path; place this comment near the summaryWorkspacePath and summaryNode usage to help future readers.
🤖 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/memoryWorkspacePaths.test.ts`:
- Around line 42-50: Add two negative tests to cover the conditional
gmail-prefix stripping: one where tree_kind is 'source' but tree_scope is a
non-gmail prefix (e.g., 'dropbox:foo') and one where tree_scope starts with
'gmail:' but tree_kind is not 'source' (e.g., tree_kind: 'topic', tree_scope:
'gmail:foo'); in both cases assert that summaryWorkspacePath(summaryNode(...))
produces a path that preserves the original scope (i.e., no removal of
'dropbox:' or 'gmail:') and matches the expected string built from
MEMORY_CONTENT_WORKSPACE_PATH and the slugified scope used in the existing test
pattern, reusing the same helpers summaryWorkspacePath and summaryNode to locate
the behavior.
- Around line 67-72: Add a brief clarifying comment above the assertion that
uses Number.MAX_SAFE_INTEGER in memoryWorkspacePaths.test.ts to explain why
MAX_SAFE_INTEGER maps to 'unknown-date' despite being a number: note that
Date(Number.MAX_SAFE_INTEGER) is out of JS Date valid range (±8.64×10^15 ms) and
becomes Invalid Date, so summaryWorkspacePath(summaryNode({...
time_range_start_ms: Number.MAX_SAFE_INTEGER })) should produce the
'unknown-date' path; place this comment near the summaryWorkspacePath and
summaryNode usage to help future readers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7578dcc8-9080-454f-8fa1-2a722d91ccac
📒 Files selected for processing (1)
app/src/components/intelligence/memoryWorkspacePaths.test.ts
Contributor
Author
|
@graycyrus @senamakel Ready for review. Latest state for #2527 (feat(memory): route summaries through workspace paths):
|
12 tasks
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
2 participants
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
revealWorkspacePath.Problem
summary_rel_pathlogic.Solution
memoryWorkspacePaths.tsto derivememory_tree/content/...paths from graph summary nodes using the Rust path conventions.openWorkspacePath.previewWorkspaceText, keeping the preview button reachable after leaving the SVG node.revealWorkspacePath.Submission Checklist
diff-coverreports no coverage-relevant lines in the current test-only follow-up diff; CI coverage gate remains authoritative.## Related— N/A: no matrix feature ID change.docs/RELEASE-MANUAL-SMOKE.md) — N/A: no release smoke surface change.Closes #NNNin the## RelatedsectionImpact
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
feat/2492-memory-workspace-linksc38d11f4a8f3e3f6624fc98bdcc421c60812a4caValidation Run
pnpm --filter openhuman-app test:unit src/components/intelligence/MemoryGraph.test.tsx src/components/intelligence/memoryWorkspacePaths.test.ts src/components/intelligence/__tests__/MemoryWorkspace.test.tsx— 3 files, 33 tests passedpnpm --filter openhuman-app exec vitest run --coverage --config test/vitest.config.ts src/components/intelligence/MemoryGraph.test.tsx src/components/intelligence/memoryWorkspacePaths.test.ts src/components/intelligence/__tests__/MemoryWorkspace.test.tsx— 3 files, 33 tests passed, lcov generated/tmp/openhuman-diff-cover-venv/bin/diff-cover app/coverage/lcov.info --compare-branch=upstream/main --fail-under=80— passed; no coverage-relevant lines in the current diff after the test-only review follow-uppnpm typecheck— passedpnpm i18n:check— passed with 0 missing/extra/drifted keysgit diff --check— passedpnpm format:check— passedpnpm lint— passed with existing warnings onlyGGML_NATIVE=OFF pnpm rust:check— passed with existing warnings onlyGGML_NATIVE=OFF git pushpre-push hook — passedValidation Blocked
command:N/Aerror:N/A; local macOS/Tahoerust:checkneeds the documentedGGML_NATIVE=OFFworkaround forwhisper-rs-mcpu=native.impact:None; the workaround check passed.Behavior Changes
Parity Contract
obsidian://open?path=....Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests