feat(onboarding): guided memory & vault setup with health checklist (#2910)#3160
Conversation
…surface The MemoryWindowControl h3 had no text-color class, so it inherited the onboarding flow's dark default and rendered near-invisible when the panel is embedded in the vault setup step. Pin it to text-stone-900 dark:text-neutral-100 to match the sibling section headings.
…e explainer The vault step rendered the read/write explainer and the health checklist twice — once inline in VaultSetupContent and again via the embedded MemoryDataPanel. Render only MemoryDataPanel embedded so each appears once, and rework its prose explainer into a structured write/read/internal definition list matching the checklist card style.
Collapse multi-line object literals and rewrap per the repo prettier config so `format:check` passes on the vault-health checklist, its test, and the memory data panel.
|
Warning Review limit reached
More reviews will be available in 31 minutes and 4 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (22)
📝 WalkthroughWalkthroughThis PR introduces vault health monitoring and guided onboarding for the Memory and vault setup. A new backend RPC probes directory accessibility, Obsidian registration, and pipeline health; a React component displays status with recovery actions; and a wizard step integrates the checklist into onboarding with conditional UI for local vs managed sessions. ChangesVault Health Monitoring and Onboarding Setup
Sequence Diagram(s)sequenceDiagram
participant Browser as User / Browser
participant Component as VaultHealthChecklist
participant Tauri as memoryTreeVaultHealthCheck
participant Backend as vault_health_check RPC
participant FS as Filesystem<br/> + Registry
Browser->>Component: Mount / Click Refresh
Component->>Tauri: memoryTreeVaultHealthCheck()
Tauri->>Backend: invoke RPC
Backend->>FS: Probe memory_tree/content
FS-->>Backend: Health status
Backend-->>Tauri: VaultHealthCheckResponse
Tauri-->>Component: Unwrapped data + debug log
Component->>Component: Render checklist
Browser->>Component: Click Reveal / Open / Install
Component->>Tauri: openUrl() or revealPath()
Tauri->>FS: Navigate / reveal
FS-->>Browser: File manager / URL handler
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
Actionable comments posted: 4
🧹 Nitpick comments (2)
app/src/pages/onboarding/steps/CustomWizardStep.tsx (1)
117-117: 💤 Low valueConsider simplifying the nested
t()fallback pattern.Line 117 uses a nested
t()call:t('onboarding.custom.stepperVault', t('onboarding.custom.stepperMemory', 'Vault')). While technically valid, this pattern is unconventional and may confuse future maintainers. The innert()call resolves to either thestepperMemorytranslation or'Vault', which is then passed as the default value for the outert()call.♻️ Simpler alternative
If the intent is to use
stepperVaultwith a fallback to'Vault', simplify to:- t('onboarding.custom.stepperVault', t('onboarding.custom.stepperMemory', 'Vault')), + t('onboarding.custom.stepperVault', 'Vault'),If both keys should be checked in sequence (first try
stepperVault, thenstepperMemory, then literal'Vault'), document the intent or extract to a helper:const getVaultLabel = () => { // Try new key first, fall back to legacy key, then literal return t('onboarding.custom.stepperVault') || t('onboarding.custom.stepperMemory') || 'Vault'; };(Note: this assumes your
useT()returns empty string for missing keys; adjust based on actual behavior.)🤖 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/pages/onboarding/steps/CustomWizardStep.tsx` at line 117, The nested t() fallback is confusing: inside the component or file replace the expression t('onboarding.custom.stepperVault', t('onboarding.custom.stepperMemory', 'Vault')) with a clearer fallback strategy such as either calling t('onboarding.custom.stepperVault', 'Vault') if you only want a literal fallback, or implement a small helper (e.g., getVaultLabel) that returns t('onboarding.custom.stepperVault') || t('onboarding.custom.stepperMemory') || 'Vault' and use that helper where the current nested t() is used; update the reference where the label is computed so maintainers see intent clearly.app/src/pages/onboarding/customWizardSteps.ts (1)
20-20: 💤 Low valueReassess the “memory” custom-wizard route (currently unreachable in the active flow).
CustomStepKeystill includes'memory'andCustomMemoryPage.tsxexists (STEP_KEY = 'memory').app/src/pages/onboarding/customWizardSteps.tsstill mapsmemory: '/onboarding/custom/memory', butCUSTOM_WIZARD_STEPSno longer includes'memory'(it goes fromembeddings→vault).- The routed onboarding flow in
app/src/pages/onboarding/Onboarding.tsxmountscustom/vaultand has nocustom/memoryroute; there are no navigations to/onboarding/custom/memory.If the “memory” step was truly replaced by “vault”, remove
'memory'fromCustomStepKeyand pruneCustomMemoryPage+ thememoryentries inCUSTOM_WIZARD_ROUTES/CUSTOM_WIZARD_SETTINGS_ROUTES(can’t drop only the route string without adjusting theRecord<CustomStepKey, ...>types).🤖 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/pages/onboarding/customWizardSteps.ts` at line 20, Custom "memory" step is orphaned: remove all references or restore it consistently. If "vault" replaced "memory", delete the CustomMemoryPage (and its STEP_KEY = 'memory'), remove 'memory' from the CustomStepKey union, and remove the 'memory' entries from CUSTOM_WIZARD_ROUTES and CUSTOM_WIZARD_SETTINGS_ROUTES in customWizardSteps.ts; then update any Record<CustomStepKey, ...> maps and imports so types line up (no leftover keys) and ensure Onboarding.tsx only references the remaining steps (e.g., 'embeddings' -> 'vault'). If you intend to keep memory instead, add it back into CUSTOM_WIZARD_STEPS and mount a route in Onboarding.tsx to /onboarding/custom/memory and restore navigation.
🤖 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/VaultHealthChecklist.tsx`:
- Around line 14-26: The function formatRelativeTime currently returns
hard-coded English strings; update it to use the i18n hook useT() from
app/src/lib/i18n/I18nContext and replace every literal ("Never", "just now",
"min ago", "hr ago", "day(s) ago") with translation keys (e.g., 'vault.never',
'vault.justNow', 'vault.minutesAgo', 'vault.hoursAgo', 'vault.daysAgo') and
proper pluralization where needed, by calling useT() in the VaultHealthChecklist
component (or surrounding module) and passing the translated strings into
formatRelativeTime or refactor formatRelativeTime to accept the translator
function (t) as an argument; apply the same localization pattern to other
user-visible strings in this component (lines ~37-225) so all UI text flows
through useT().
In `@app/src/components/settings/panels/MemoryDataPanel.tsx`:
- Around line 57-91: The hardcoded UI strings in MemoryDataPanel.tsx must be
changed to use the i18n hook: import and call useT() from I18nContext and
replace the literal strings ("How memory storage works", "Workspace vault ·
write", the three descriptions, and title="Vault setup health") with t(...)
lookups; add corresponding keys under a sensible namespace (e.g.,
memory.howItWorks.title, memory.howItWorks.workspaceVault.label,
memory.howItWorks.workspaceVault.description,
memory.howItWorks.connectedSources.label,
memory.howItWorks.connectedSources.description,
memory.howItWorks.internalFiles.label,
memory.howItWorks.internalFiles.description, memory.vaultHealthTitle) to
app/src/lib/i18n/en.ts and mirror them in other locale files so
VaultHealthChecklist onToast={addToast} uses
title={t('memory.vaultHealthTitle')}.
In `@app/src/pages/onboarding/pages/VaultSetupStep.tsx`:
- Around line 14-15: Replace hardcoded user strings with i18n keys using useT()
from I18nContext: remove the constant LOCAL_DEFAULT_DISABLED_REASON and stop
passing literal strings to CustomWizardStep props (title, subtitle,
defaultDescription, configureDescription); instead call useT() in VaultSetupStep
and reference translation keys (e.g., onboarding.custom.vault.title, subtitle,
defaultDescription, configureDescription, localDisabledReason), add those keys
and English values to app/src/lib/i18n/en.ts and update other locale files
accordingly so all user-visible text is localized.
- Around line 29-36: The effect is synchronously calling setChoice and setDraft
when isLocalSession becomes true, which triggers the
react-hooks/set-state-in-effect lint; refactor by deriving the forced choice at
render or guarding the update with a ref so no state setter is called
synchronously in the effect: e.g., compute choice = isLocalSession ? 'configure'
: choiceState (and initialize choiceState accordingly) or add a ref like
appliedLocalSessionRef used inside the effect to only perform setDraft
asynchronously/once (or better, move the draft update into the code path that
sets isLocalSession) — update references to setChoice, setDraft, isLocalSession
and STEP_KEY in VaultSetupStep to implement one of these patterns.
---
Nitpick comments:
In `@app/src/pages/onboarding/customWizardSteps.ts`:
- Line 20: Custom "memory" step is orphaned: remove all references or restore it
consistently. If "vault" replaced "memory", delete the CustomMemoryPage (and its
STEP_KEY = 'memory'), remove 'memory' from the CustomStepKey union, and remove
the 'memory' entries from CUSTOM_WIZARD_ROUTES and CUSTOM_WIZARD_SETTINGS_ROUTES
in customWizardSteps.ts; then update any Record<CustomStepKey, ...> maps and
imports so types line up (no leftover keys) and ensure Onboarding.tsx only
references the remaining steps (e.g., 'embeddings' -> 'vault'). If you intend to
keep memory instead, add it back into CUSTOM_WIZARD_STEPS and mount a route in
Onboarding.tsx to /onboarding/custom/memory and restore navigation.
In `@app/src/pages/onboarding/steps/CustomWizardStep.tsx`:
- Line 117: The nested t() fallback is confusing: inside the component or file
replace the expression t('onboarding.custom.stepperVault',
t('onboarding.custom.stepperMemory', 'Vault')) with a clearer fallback strategy
such as either calling t('onboarding.custom.stepperVault', 'Vault') if you only
want a literal fallback, or implement a small helper (e.g., getVaultLabel) that
returns t('onboarding.custom.stepperVault') ||
t('onboarding.custom.stepperMemory') || 'Vault' and use that helper where the
current nested t() is used; update the reference where the label is computed so
maintainers see intent clearly.
🪄 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: ef49f7ac-9440-497f-a2fc-961a46ea9018
📒 Files selected for processing (14)
app/src/components/intelligence/VaultHealthChecklist.test.tsxapp/src/components/intelligence/VaultHealthChecklist.tsxapp/src/components/settings/components/MemoryWindowControl.tsxapp/src/components/settings/panels/MemoryDataPanel.tsxapp/src/pages/onboarding/Onboarding.tsxapp/src/pages/onboarding/OnboardingContext.tsxapp/src/pages/onboarding/customWizardSteps.tsapp/src/pages/onboarding/pages/VaultSetupStep.test.tsxapp/src/pages/onboarding/pages/VaultSetupStep.tsxapp/src/pages/onboarding/steps/CustomWizardStep.tsxapp/src/utils/tauriCommands/memoryTree.tssrc/openhuman/memory/read_rpc.rssrc/openhuman/memory/read_rpc_tests.rssrc/openhuman/memory/schema.rs
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
@oxoxDev heads up — CI is failing (PR Submission Checklist) and several coverage + E2E checks are still pending, so I'll hold off on approval until those are green. I did a pass through the diff and found a few things beyond what CodeRabbit caught:
The stepper label fallback in CustomWizardStep.tsx is going to show the wrong text. t('onboarding.custom.stepperVault', t('onboarding.custom.stepperMemory', 'Vault')) — if stepperVault is missing from the locale files (which it almost certainly is, since there's no corresponding key added to en.ts in this diff), the t() call falls back to the second argument. That second argument is another t() call for stepperMemory, which is an existing key and will resolve to whatever the Memory step label currently says — probably "Memory". So the vault step in the stepper will read "Memory". You want t('onboarding.custom.stepperVault', 'Vault') with a string fallback, and you need to add stepperVault to the locale files.
The completeAndExit failure in VaultSetupStep.tsx is swallowed silently. The try/catch around it logs to console.error and then... nothing. If completeAndExit throws, the user is stuck on the final onboarding step with no feedback and no way to proceed. This needs a toast or an inline error state so the user knows something went wrong and can retry.
CodeRabbit already caught the i18n gaps (hardcoded strings in VaultHealthChecklist, MemoryDataPanel, and VaultSetupStep) and the useEffect/setState lint violation — those are all blocking too and need to be addressed before this is ready.
Smaller things: the memory key is still in CUSTOM_WIZARD_ROUTES and CUSTOM_WIZARD_SETTINGS_ROUTES in customWizardSteps.ts even though memory was removed from CUSTOM_WIZARD_STEPS. It's inert but it's dead config that should be cleaned up. In probe_directory_writable, the remove_file failure is silently ignored — a leak of .openhuman-vault-writecheck-*.tmp in the user's vault is unlikely but Obsidian would surface it as an unexpected file if it ever stuck around. Worth at least logging the failure at debug level.
The core RPC logic itself is solid — the spawn_blocking pattern is correct, path redaction in the log is good, and the Rust tests cover the states that matter. The component structure and the way VaultHealthChecklist is shared between onboarding and Settings is the right call.
Fix the stepper key, the completeAndExit error path, and the i18n/lint issues CodeRabbit flagged, get CI green, and this is ready.
| t('onboarding.custom.stepperSearch'), | ||
| t('onboarding.custom.stepperEmbeddings'), | ||
| t('onboarding.custom.stepperMemory'), | ||
| t('onboarding.custom.stepperVault', t('onboarding.custom.stepperMemory', 'Vault')), |
There was a problem hiding this comment.
[major] Nested t() as fallback resolves to the wrong label. If stepperVault is not in the locale files (no entry is added in this PR), this falls back to t('onboarding.custom.stepperMemory', 'Vault') — which is an existing key that returns the Memory step label, not "Vault".
Change to:
t('onboarding.custom.stepperVault', 'Vault')And add stepperVault to en.ts (and other locale files).
| } | ||
| }} | ||
| continueLabel={t('onboarding.custom.finish')} | ||
| /> |
There was a problem hiding this comment.
[major] Error from completeAndExit is logged to console but otherwise swallowed. The user has no feedback that onboarding failed to complete and no way to retry. Need to surface this — a toast, an inline error state on the button, anything. As written, a failed exit leaves the user stuck.
sanil-23
left a comment
sanil-23
left a comment
There was a problem hiding this comment.
@oxoxDev nice work on this — the read/write explainer plus the health checklist with concrete recovery actions is exactly what the issue was asking for, and I appreciate that you backed it with both Rust (vault_health_check_*) and Vitest coverage rather than leaving it UI-only. The shared MemoryDataPanel between onboarding and Settings is the right call too.
The code itself looks clean on my read — the new RPC avoids .unwrap() in the request path, the log line redacts the content-root path so no filesystem PII leaks, and the write probe (create-temp then remove under content-root) is a sensible way to detect writability without side effects.
A couple of things to sort before I can come back and approve:
- CI isn't green yet —
Rust Core Coverage, the two failing E2E lanes, andPR Submission Checklist(the diff-coverage box is still unchecked) all need to pass. Worth confirming the new lines clear the >= 80% diff-cover gate. - CodeRabbit has open threads on localizing the user-visible strings in the new components (they should flow through
useT()like the rest ofapp/src/**). Those are the main code change left — once they're addressed and CI is green, ping me and I'll do the approval pass.
One small optional note: the readable field is computed in the RPC and carried through the TS type, but the checklist UI only surfaces exists/writable/obsidian/pipeline — so readable is effectively dead on the wire right now. Either surface it or drop it, your call.
# Conflicts: # app/src/pages/onboarding/Onboarding.tsx # app/src/pages/onboarding/OnboardingContext.tsx # app/src/pages/onboarding/customWizardSteps.ts # app/src/pages/onboarding/steps/CustomWizardStep.tsx
- VaultHealthChecklist: replace all hardcoded strings with useT() i18n keys - MemoryDataPanel: replace hardcoded strings with useT() i18n keys - VaultSetupStep: replace hardcoded strings with useT() i18n keys, surface completeAndExit errors via inline error banner instead of swallowing them, refactor useEffect setState to render-time ref guard Addresses @coderabbitai and @graycyrus review comments.
Add stepperVault, onboarding.custom.vault.*, vaultHealth.*, and memoryData.* translation keys for the new vault onboarding step and health checklist components.
Add real translations for stepperVault, onboarding.custom.vault.*, vaultHealth.*, and memoryData.* keys across ar, bn, de, es, fr, hi, id, it, ko, pl, pt, ru, zh-CN locale files.
The remove_file call after the vault writability probe was silently discarding errors, which could leave .openhuman-vault-writecheck-*.tmp files in the user's vault. Log at debug level so the failure is traceable. Addresses @graycyrus review comment.
4 participants
Summary
/onboarding/custom/vault) explaining what OpenHuman reads, what it writes, and where generated notes live.VaultHealthChecklistcomponent (also surfaced in Settings → Memory) showing path-exists / writable / Obsidian-registered / pipeline-healthy with recovery actions (reveal folder, open/register in Obsidian, install Obsidian).openhuman.memory_tree_vault_health_checkreturning{ exists, readable, writable, obsidian_registered, pipeline_healthy, last_sync_ms, content_root_abs }with a real write probe (create-temp → remove under the content root).Problem
New users hit dead ends setting up vaults and memory: it is unclear what is read-only vs writable, where generated notes are stored, whether the workspace vault opens, or whether the sync pipeline is healthy. Without onboarding they cannot tell read sources from the write vault, and silent setup failures erode confidence before the upgrade prompt.
Solution
VaultSetupStepto the custom onboarding wizard (CUSTOM_WIZARD_STEPS+ routes), with a Default (OpenHuman-managed) vs Configure (review + tune now) choice; local-only sessions are forced to Configure.MemoryDataPanel(read/write explainer + health checklist + memory-window control + workspace), so the same surface backs both onboarding and Settings → Memory — no duplicated panels.vault_health_check_rpcinmemory/read_rpc.rs), reusing existing workspace-path resolution + Obsidian-status + pipeline-status logic; failed checks render explicit recovery guidance (no silent failures).Submission Checklist
vault_health_check_*tests (read_rpc_tests.rs) cover fresh/missing-root and writable/registered states; VitestVaultHealthChecklist.test.tsx+VaultSetupStep.test.tsxcover loaded, error, and local-session states.pr-ci.ymldiff-cover); focused Vitest +cargo test memory::read_rpcpass locally.## Related— N/A: no matrix feature IDs affected.Closes #NNNin the## Relatedsection.Impact
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
feat/2910-vault-onboardinge882aa5ddda15a8735e2ce3190357b8cdd1461aeValidation Run
pnpm --filter openhuman-app format:check— prettier clean on changed filespnpm typecheck— passespnpm test:unit VaultHealthChecklist.test.tsx VaultSetupStep.test.tsx(6 passed);cargo test --lib openhuman::memory::read_rpc(36 passed, incl. 2 new vault_health_check tests)cargo fmt --checkclean,cargo check -p openhumancleanpnpm rust:checkclean (warnings pre-existing)Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
Duplicate / Superseded PR Handling
Verification
exists/writablepass, Obsidian/pipeline need-attention) with working Reveal Folder / Open in Obsidian actions.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Tests