fix(memory-sync): reliable per-source sync indicators and counters#3308
Conversation
Per-source "Syncing…" indicators and the live "N syncing / N synced" counters on the Intelligence memory sources panel were unreliable: only one source could show as syncing at a time, intermediate stages never lit up the per-source bar, and progress got stuck at a placeholder. - Track syncing sources as a Set<string> (syncingIds) instead of a single syncingId so concurrent syncs each render their own progress. - Add a new optional source_id field to MemorySyncStageChanged (distinct from connection_id, which still carries the ingest-pipeline document_id). MemorySyncStageBridge extracts the originating memory-source id from the mem_src:<source_id>:<item_id> chunk encoding (first-colon split, since item ids may be URLs) so stored/queued/ingesting stages reach the right row. Frontend matches on source_id ?? connection_id (backward compatible). - Make parseSyncProgress tolerant and add per-stage fallback percentages so the bar always advances; the 5s poll reconciles missed events to done. - Tests: Rust bridge/source_id-mapping unit tests + Vitest concurrent-sync and tolerant-parsing coverage. Composio same-toolkit counter collapse and remount state recovery are intentionally out of scope (tracked separately). Closes tinyhumansai#3295
|
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 (1)
📝 WalkthroughWalkthroughThis PR implements reliable per-source memory sync indicators by extending the event bus to carry ChangesMemory Source Sync Indicators
Sequence DiagramsequenceDiagram
participant User as User (UI)
participant FrontendRegistry as MemorySourcesRegistry
participant SyncService as memorySourcesService
participant BackendSync as Backend (sync.rs)
participant EventBus as DomainEvent Bus
participant SocketIO as Socket.io Bridge
participant FrontendListener as openhuman:memory-sync-stage Listener
User->>FrontendRegistry: Click Sync on Source A
FrontendRegistry->>FrontendRegistry: Add to syncingIds, clear result
FrontendRegistry->>SyncService: syncMemorySource(source.id)
SyncService->>BackendSync: spawn sync task with source_id
BackendSync->>EventBus: emit_sync_stage("requested", source_id=A)
EventBus->>SocketIO: MemorySyncStageChanged{stage, source_id}
SocketIO->>FrontendListener: memory:sync_stage{stage, source_id=A}
FrontendListener->>FrontendRegistry: Update syncProgress[A], show spinner
BackendSync->>EventBus: emit_sync_stage("fetching", source_id=A)
EventBus->>SocketIO: MemorySyncStageChanged
SocketIO->>FrontendListener: memory:sync_stage{stage, source_id=A}
FrontendListener->>FrontendRegistry: Update progress via parseSyncProgress
BackendSync->>EventBus: emit_sync_stage("completed", detail="ingested 42 items", source_id=A)
EventBus->>SocketIO: MemorySyncStageChanged
SocketIO->>FrontendListener: memory:sync_stage{stage, source_id=A, detail}
FrontendListener->>FrontendRegistry: Remove from syncingIds, store SyncResult{itemsCount=42}
FrontendRegistry->>FrontendRegistry: Clear syncProgress[A], show "42 items synced" chip
FrontendRegistry->>User: Display terminal result chip
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested labelsmemory, rust-core Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
…icators # Conflicts: # .claude/memory.md # app/src/components/intelligence/MemorySourcesRegistry.tsx # app/src/components/intelligence/__tests__/MemorySourcesRegistry.test.tsx # src/openhuman/memory_sync/sources/github.rs
…urce (tinyhumansai#3295) A source sync that succeeded with 0 new items left no trace — the progress bar cleared on `completed` and nothing replaced it, so it read as a failure. The success toast also fired on the 4ms RPC ack (which only spawns the background sync), not on the real outcome. Drive the three states off the terminal stage event instead of the RPC ack: - Success -> persistent row chip "N items synced" (or "Up to date" when 0 new), parsed from the `completed` detail ("ingested N item(s)"). Toast moved here from the RPC ack so it reflects the actual result. - Failure -> persistent "Failed: <reason>" chip from the `failed` detail + an error toast. Internal failures are reported to Sentry in the core via `observability::report_error_or_expected` (known-expected auth/network/ rate-limit errors are logged-not-reported; never-suppress honored). - In-progress bar stays bound to non-terminal stage events; `isSyncing` now tracks real progress (syncingIds || syncProgress) and the misleading finally-clear of the syncing flag was removed. Adds parseIngestedCount + 7 tests (count parse, success/upToDate/failure chips, result-clear on re-sync, RPC-reject path). i18n keys added to all 14 locales.
…ent-driven toast (tinyhumansai#3295) The per-source Sync button test asserted a success toast on the RPC ack, which was the misleading pre-tinyhumansai#3295 behavior now removed. Success feedback fires on the terminal `completed` stage event instead, so the test emits that event before asserting the toast.
Summary
Fixes the unreliable per-source "Syncing…" indicators and the live "N syncing / N synced" counters on the Intelligence memory sources panel (
MemorySourcesRegistry). Previously only one source could show as syncing at a time, intermediate stages never lit up the per-source bar, and progress got stuck at a placeholder.Closes #3295.
Root causes addressed
MemorySourcesRegistry.tsxtracked a singlesyncingId: string | null. Now asyncingIds: Set<string>so concurrent syncs each render their own progress (immutable Set updates).MemorySyncStageBridge(src/openhuman/memory/sync.rs) emittedstored/queuedwith noconnection_idandingestingwithconnection_id = document_id, so the frontend (matching byconnection_id === source.id) never lit up intermediate stages. Added a new optionalsource_id: Option<String>field toMemorySyncStageChanged(distinct fromconnection_id, which still carries the ingest-pipelinedocument_id). The bridge extracts the originating memory-source id from themem_src:<source_id>:<item_id>chunk encoding (first-colon split, since item ids may be URLs). Frontend matches onsource_id ?? connection_id(backward compatible).parseSyncProgressnow tolerates non-N/Mdetail strings and falls back to per-stage default percentages so the bar always advances.Out of scope (intentional, tracked separately)
memory_sources_active_syncsRPC; rely on the existing 5s poll to rehydrate after a remount.Event payload change
MemorySyncStageChangedgainssource_id: Option<String>(the memory-source row id;Nonefor channel-level syncs).connection_idsemantics are unchanged. Serialized into thememory-sync-stagesocket payload.Tests
src/openhuman/memory/sync.rs):extract_mem_src_idunit tests (incl. URL item ids with colons) +MemorySyncStageBridgetests assertingsource_idis populated formem_src:syncs andNoneotherwise, withconnection_idleft unchanged. 10 lib tests pass.app/src/components/intelligence/__tests__/MemorySourcesRegistry.test.tsx): concurrent syncs render both rows,completed/failedclears only the right row,source_idpreferred overconnection_id, and tolerantparseSyncProgresscases. 11 tests pass.Checks
pnpm typecheck,pnpm lint(0 errors),pnpm format:check,pnpm build,cargo check, the new Vitest suite, and thememory::syncRust lib tests all pass. The fullpnpm test:ruste2e suite andpnpm dev:appwere not run locally due to a disk-space limit in the dev environment (ld: errno=28); the library tests covering this change pass.Notes for reviewers
connection_idmust keep carryingdocument_idfor ingest-pipeline identity — this PR addssource_idrather than repurposingconnection_id. Please sanity-check no downstream consumer depends onconnection_idbeing the source id.Summary by CodeRabbit
New Features
Documentation
Localization
Tests