fix(memory): stop a signed-in user's embeddings failing, and unstick the parked queue - #5427
Conversation
…the parked queue
Four defects that compounded into one symptom: a signed-in user whose Memory
Tree sat permanently on "Error", telling them to log in.
1. The keyless managed embedder read the wrong credential directory.
`default_state_dir()` returned the root `~/.openhuman`, but sign-in stores
the `app-session` token through `AuthService::from_config`, i.e. under the
user-scoped `~/.openhuman/users/<user_id>/`. The root holds no
`auth-profiles.json`, so every embed through `create_embedding_provider_with_credentials`
("managed"/"cloud") failed with "No backend session for cloud embeddings"
while the user was signed in and the socket was authenticated. Gmail sync
reported success and stored every chunk without vectors.
`default_state_dir()` now resolves `{root}/users/{active_user_id}` (falling
back to the pre-login `users/local`), keeping the `OPENHUMAN_WORKSPACE` branch
for deployments that co-locate config and credentials at one root. tinyhumansai#5363
fixed only the config-aware sibling; this covers the call sites that hold no
`Config`.
2. The status panel presented a superseded failure as the current cause.
An unrecoverable failure is terminal by design and its row keeps its
`failure_reason` forever, so `latest_failed_job_failure` kept rendering the
first diagnosis it ever saw. In practice that meant an `auth_missing` batch
from 27 days earlier producing a "log in to OpenHuman" banner for a user who
already was, while the queue completed jobs normally throughout. A failure is
now only reported as the blocking cause when no job has settled successfully
since it; the failure is still counted and still needs clearing, but the
remediation text is withheld once it stops being true.
3. There was no way to clear parked failures from the app. The
`memory_tree_retry_failed` RPC (#2 FR-011) had no caller anywhere in the
frontend, so a single bad batch pinned the panel on `error` permanently. Adds
the `memoryTreeRetryFailed` wrapper and a "Retry failed jobs" button, keyed
off the failed-job counter rather than the blocking-cause banner so it stays
reachable in exactly the superseded case above.
4. Requeue itself aborted on a duplicate `dedupe_key` (vendor/tinycortex bump).
Both the periodic self-heal and the manual retry flipped every failed row in
one UPDATE, colliding on the partial unique index and requeueing nothing.
Fixed in the submodule; see its commit for detail.
Tests: 3 for the credential scope + supersession invariants in the core, 3 in
tinycortex for the collision, 5 in the panel suite for the retry affordance.
i18n keys added to all 14 locales.
|
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 (15)
🚧 Files skipped from review as they are similar to previous changes (10)
📝 WalkthroughWalkthroughThe PR adds a Memory Tree retry flow with RPC handling, localized messages, analytics, and tests. It suppresses stale failure causes, scopes cloud embedding credentials by workspace and user, and updates the tinycortex submodule reference. ChangesMemory Tree retry flow
Failure cause supersession
Scoped embedding credentials
Tinycortex reference update
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Operator
participant MemoryTreeStatusPanel
participant memoryTreeRetryFailed
participant MemoryTreeCore
Operator->>MemoryTreeStatusPanel: click retry failed
MemoryTreeStatusPanel->>memoryTreeRetryFailed: request retry
memoryTreeRetryFailed->>MemoryTreeCore: invoke retry RPC
MemoryTreeCore-->>memoryTreeRetryFailed: return requeued count
memoryTreeRetryFailed-->>MemoryTreeStatusPanel: return result
MemoryTreeStatusPanel-->>Operator: show toast and refreshed status
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: accf6782a5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
src/openhuman/memory/tree/tree/rpc.rs (1)
694-717: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog every supersession decision.
This flow logs only the superseded branch. Add safe debug diagnostics for the active-failure and missing-timestamp branches. Include the decision and timestamps. Do not log user-authored values.
As per coding guidelines, “New or changed flows must include verbose, grep-friendly diagnostics for entry/exit, branches, external calls, retries, state transitions, and errors, while never logging secrets or full PII.”
🤖 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 `@src/openhuman/memory/tree/tree/rpc.rs` around lines 694 - 717, The pipeline_status failure-handling flow currently logs only when a failure is superseded. Add safe, grep-friendly debug logs for the active-failure decision and the missing failed_at_ms branch, including the relevant decision and timestamps while excluding the user-authored reason. Update the visible branch around failed_at_ms and the existing supersession check, preserving the current return behavior.Source: Coding guidelines
🤖 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/MemoryTreeStatusPanel.tsx`:
- Around line 391-403: The handleRetryFailed flow currently tracks only the
button interaction, not successful completion. After memoryTreeRetryFailed()
resolves in handleRetryFailed, call trackAnalyticsEvent using a stable
success-event identifier and only privacy-safe dimensions such as requeued or
none, before showing the success toast.
- Around line 391-411: Extend handleRetryFailed with grep-friendly diagnostics
for the retryBusy guard, RPC start, setRetryBusy transitions, and callback exit,
while retaining the existing entry, result, and error logs. Use fixed labels
only, avoid identifiers or user-specific values, and ensure the exit log is
emitted regardless of success or failure.
In `@app/src/lib/i18n/de.ts`:
- Line 1357: Update the translation value for memoryTree.status.retryFailedCount
to use count-neutral German wording, removing the “Job(s)” construction while
preserving the count placeholder and meaning for singular and plural counts.
In `@app/src/lib/i18n/es.ts`:
- Around line 1336-1337: Update the translation values for
memoryTree.status.retryFailedDone and memoryTree.status.retryFailedCount to use
natural Spanish; replace “vueltos a la cola” with idiomatic wording and remove
the “(s)” marker by choosing count-neutral phrasing that works for both singular
and plural values.
In `@app/src/lib/i18n/fr.ts`:
- Around line 1349-1352: Update the French translation for
memoryTree.status.retryFailedCount to describe tasks being placed back in the
queue, using natural French without the “replanifiée(s)” wording. Keep the count
placeholder and align the terminology with the nearby retryFailedDone and
retryFailedError messages.
In `@app/src/lib/i18n/pt.ts`:
- Line 1334: Update the memoryTree.status.retryFailedCount translation to avoid
the literal “tarefa(s)” construction, using plural-aware Portuguese text that
renders grammatically correct singular and plural forms based on count.
In `@src/openhuman/inference/embeddings/cloud_adapter.rs`:
- Around line 77-103: Add privacy-safe debug diagnostics at entry and successful
completion of default_state_dir, using grep-friendly messages that do not
include the resolved path or user ID. Preserve the existing branch-selection and
error logs, and ensure the completion diagnostic is emitted after the final
credential scope has been resolved.
In `@src/openhuman/memory/tree/tree/rpc.rs`:
- Around line 680-711: Update the failure lookup in latest_failed_job_failure to
derive the latest failed job and latest successful completed_at_ms within one
SQL statement or a single read transaction. Replace the separate last_success_ms
query with a value read from that same SQLite snapshot, preserving the existing
remediation decision and return behavior.
---
Nitpick comments:
In `@src/openhuman/memory/tree/tree/rpc.rs`:
- Around line 694-717: The pipeline_status failure-handling flow currently logs
only when a failure is superseded. Add safe, grep-friendly debug logs for the
active-failure decision and the missing failed_at_ms branch, including the
relevant decision and timestamps while excluding the user-authored reason.
Update the visible branch around failed_at_ms and the existing supersession
check, preserving the current return behavior.
🪄 Autofix
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: 9a721740-c033-47a4-b6b5-977b13409c6e
📒 Files selected for processing (20)
app/src/components/intelligence/MemoryTreeStatusPanel.test.tsxapp/src/components/intelligence/MemoryTreeStatusPanel.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/utils/tauriCommands/memoryTree.tssrc/openhuman/inference/embeddings/cloud_adapter.rssrc/openhuman/memory/tree/tree/rpc.rsvendor/tinycortex
…al scope, supersession race, retry UX - cloud_adapter: resolve OPENHUMAN_WORKSPACE through resolve_config_dir_for_workspace (mirrors config::load) so a legacy .../workspace override lands on the sibling .openhuman credential dir, not the workspace dir — fixes a "No backend session" regression for that env-override deployment (Codex P2). Adds entry/per-branch diagnostics and a regression test; threads the resolver through config re-exports. - rpc: read the newest failed row and the success watermark in one with_connection closure so a job settling between the two reads cannot flip the supersession decision (CodeRabbit race); log every supersession branch. - panel: track the successful requeue outcome (memory_tree_retry_succeeded, count only) and add busy-guard / rpc-start / exit diagnostics. - i18n: count-neutral wording for retryFailedCount across en/de/es/fr/pt/it; fr now uses queue wording instead of "reschedule", es drops "vueltos a la cola". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Addressed all review feedback in Correctness
Diagnostics — added grep-friendly logs to the credential-scope resolver (entry + per-branch), the supersession decision (all three branches — covers the "log every branch" nitpick), and the retry flow (busy-guard / rpc-start / exit). Analytics — the successful requeue now emits i18n — Local validation: Note: the earlier Rust Quality red was a transient |
…overage gate PR CI Gate's merged diff-cover was 73% (<80%): the memory_tree_retry_failed RPC wrapper (memoryTree.ts:926-933) had no test. Add envelope + bare-shape dispatch tests, mirroring memoryTreeFlushNow, bringing changed-line coverage to ~93%. The panel's defensive busy-guard log stays uncovered by design -- the button is disabled while a retry is in flight, so the guard is unreachable via the UI. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Pushed |
Summary
inference/embeddings/cloud_adapter.rs::default_state_dir()now resolves the user-scoped credential dir ({root}/users/{active_user}, fallbackusers/local) instead of the empty root, so the keyless managed embedder finds the signed-in session.auth_missingtombstone stops masquerading as the current problem.memoryTreeRetryFailedwrapper (thememory_tree_retry_failedRPC had no frontend caller), keyed off the failed-job counter so it's reachable even when the banner is withheld. i18n ×14.vendor/tinycortexfor the per-row requeue fix (a duplicatededupe_keyno longer aborts the whole requeue).Problem
A signed-in user's Memory Tree sat on
Errorwith a "log in" banner while they were already logged in; Gmail/chat sync reported success but stored chunks with no vectors (silently degraded recall), and the state never cleared. Root causes:default_state_dir()returned root~/.openhuman, but sign-in stores theapp-sessiontoken under~/.openhuman/users/<user_id>/. The root has noauth-profiles.json, so every keyless "managed"/"cloud" embed failed with "No backend session for cloud embeddings". fix(embeddings): read managed session token from config-scoped store #5363 fixed only the config-aware sibling; this covers the no-Configcall sites.failure_reasonforever, solatest_failed_job_failurerendered the first diagnosis it ever saw — anauth_missingbatch from 27 days earlier — while the queue completed jobs the whole time.memory_tree_retry_failedRPC (#002 FR-011) had no frontend caller, so one bad batch pinned the panel onerrorpermanently.dedupe_key. Self-heal and manual retry flipped every failed row in oneUPDATE, colliding on the partial unique index and requeueing nothing (tinycortex).Solution
default_state_dir()mirrorsconfig::load's directory choice —OPENHUMAN_WORKSPACEwhen set, else{root}/users/{active_user_id}(fallbackusers/local). Split into a pureuser_scoped_state_dirfor unit testing. Never logs the path or user id.donejob has completed since it (MAX(completed_at_ms)watermark). It is still counted (status stayserror, the "N unrecoverable failure(s) need action" reason stands); only the remediation text is withheld once untrue.memoryTreeRetryFailed+ a "Retry failed jobs" button keyed offpipeline_jobs.failed, so it stays reachable in the superseded case. Graceful error toast; button re-enables on failure.dedupe_keyrequeues, siblings settle ascancelled, keys held by a live row are skipped. Fixed invendor/tinycortex(fix(queue): requeue failed jobs per row so a dedupe collision cannot abort the batch tinycortex#139).Submission Checklist
diff-covergate not measured locally, CI enforcesN/A: behaviour-only bug fix, no new/renamed feature IDs## Related—N/A: no new feature IDsN/A: existing Memory Tree surface, no new smoke stepCloses #NNNin## RelatedImpact
default_state_dirnarrows the credential scope to the active user's dir; never logs the resolved path or user id.vendor/tinycortexgitlink to the merged tinycortex SHA before this merges.Related
vendor/tinycortexrequeue fix)AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/memory-embeddings-stuck-queueaccf6782a527aa241780cba54dfa262a5daecfc0Validation Run
pnpm --filter openhuman-app format:check— not run locallypnpm typecheck— cleanMemoryTreeStatusPanel.test.tsx32/32; coredefault_scope_*+blocking_cause_*6/6GGML_NATIVE=OFF cargo check --bin openhuman-coreexit 0 (fmt not run)N/A: no app/src-tauri changesValidation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Error/"log in" state; parked jobs can be requeued from the panel.Parity Contract
OPENHUMAN_WORKSPACEdeployments keep the root scope; failures are still counted and still gate status toerror; thederive_pipeline_statusstatus/reason path is unchanged.users/local; NULLcompleted_at_mssurfaces the failure unconditionally (legacy rows); retry error path re-enables the button.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Bug Fixes
Localization
Tests