Skip to content

fix(memory): classify Ollama-unavailable embed failures actionably - #5398

Merged
M3gA-Mind merged 5 commits into
tinyhumansai:mainfrom
YellowSnnowmann:fix/5354-ollama-embed-error-classification
Aug 5, 2026
Merged

fix(memory): classify Ollama-unavailable embed failures actionably#5398
M3gA-Mind merged 5 commits into
tinyhumansai:mainfrom
YellowSnnowmann:fix/5354-ollama-embed-error-classification

Conversation

@YellowSnnowmann

@YellowSnnowmann YellowSnnowmann commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Classify the two Ollama-unusable embed failures (daemon not listening, model never pulled) as local_model_unavailable instead of a generic transient fault, so the memory status panel names the actual fix.
  • Keep that code in the transient retry class so requeue_transient_failed auto-resumes ingestion once Ollama is back, with no manual "Retry failed" click.
  • Mark semantic recall degraded at classification time, so the remediation appears on the first failed embed rather than after the retry budget drains.
  • Bridge the existing Ollama health-gate signal to the UI over the metadata-only user_error web-channel path, landing a durable UserErrorCenter entry. DomainEvent::EmbeddingModelUnhealthy was published but reached no user.
  • Add the local_model_unavailable user-error kind + memory scope, and derive the panel scope from the producing domain instead of hardcoding cron.

Problem

The issue reports that the memory embedder still calls the deprecated Ollama /api/embeddings endpoint removed in v0.32.x. That root cause is not present in the tree. The embedder has called POST /api/embed with the input field since the provider port (vendor/tinyagents/src/harness/embeddings/ollama.rs), and /api/embeddings appears nowhere in the repository. So acceptance criteria 1 and 2 already held.

The real defect is criteria 3 and 4, and it is what the reporter actually experienced. When Ollama is not running, or the configured model was never pulled, the embedder produces two messages that already carry the fix in their text:

ollama embed request failed (is Ollama running at http://localhost:11434?): …
Ollama embedding model `bge-m3` is not installed at http://localhost:11434. Run `ollama pull bge-m3` …

Neither carries an Embedding API error (<status>) envelope — the first is a transport bail, the second a rewritten 404 — so classify_embed_error_str matched none of its rules and fell through to Transient. The panel then rendered memory.health.remediation.transient: "A temporary error interrupted memory processing. It will retry automatically." Retrying cannot start a daemon or pull a model, so the pipeline retried indefinitely and the user was never told what to do.

FailureCode::LocalModelUnavailable and its translated remediation ("Install/run Ollama and pull the model…") already existed in all 14 locales — with no producer anywhere in the tree.

Separately, the memory-store health gate detects an unreachable daemon and publishes DomainEvent::EmbeddingModelUnhealthy, but nothing carries the domain bus to the product UI: /events/domain is consumed only by the developer Event Log panel, and there is no app/src listener. That notice reached no user either.

Solution

Classification (memory/tree/health/mod.rs) — match the two shapes explicitly, anchored on Ollama-specific wording so a generic cloud-embedder transport failure keeps its Transient code. Placed after the client-side bails and before status parsing.

Retry classLocalModelUnavailable moves into FailureClass::Transient. This is the key design decision. requeue_transient_failed — the automatic self-healing requeue — skips rows recorded as unrecoverable. Since the condition clears from outside the app, classifying it unrecoverable would park every affected job until someone pressed "Retry failed" by hand. Transient is also consistent with derive_pipeline_status, which escalates to error only on unrecoverable failures precisely because transient ones self-heal. The panel banner renders on the presence of a typed cause, not on its class, so the remediation shows either way.

Immediate surfacing — a persisted failure_reason is only read back once a job settles terminally, which for a transient class means after the whole retry budget drains. mark_local_model_unavailable_if_applicable flips the semantic-recall flag at classification time so the remediation is on screen from the first failure; it self-clears on the next successful embed.

UI bridge (memory/store/factories.rs) — broadcast the condition as a user_error web-channel event, the same path publish_cron_user_error already uses, which socketService routes into the durable UserErrorCenter. Metadata-only: a stable kind token plus error_source, never raw provider text or the configured endpoint.

The broadcast deliberately sits above the once-per-process Sentry latch. publish_web_channel_event is an unbuffered broadcast::send — with no socket client attached the event is dropped with no redelivery — and memory is constructed early, before the renderer's socket attaches. Under the latch, that one dropped send would have been the only attempt ever made and the panel would have stayed empty for the whole outage. Re-broadcasting per failed probe is safe: the panel store dedupes on the descriptor's kind:scope:provider identity and bumps count. Only the Sentry report stays latched, which is what the latch was introduced for.

Frontend — new local_model_unavailable kind and memory scope, a classifier rule ordered last so an out-of-credits provider keeps its billing remediation, and socketService now derives scope from error_source. Unknown domains keep the historical cron default, so existing behaviour is unchanged. Each prose matcher is anchored on full producer wording rather than a bare daemon unreachable at, which backend connection-health logs also emit — the same deliberate anchoring the Rust matcher uses.

Submission Checklist

  • Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy
  • Diff coverage ≥ 80% — changed lines (Vitest + cargo-llvm-cov merged via diff-cover) meet the gate enforced by .github/workflows/ci-lite.yml. Run pnpm test:coverage and pnpm test:rust locally; PRs below 80% on changed lines will not merge.
  • Coverage matrix updated — N/A: behaviour-only change to existing memory-tree health classification; no feature rows added, removed, or renamed
  • All affected feature IDs from the matrix are listed in the PR description under ## Related
  • No new external network dependencies introduced (mock backend used per Testing Strategy)
  • Manual smoke checklist updated if this touches release-cut surfaces — N/A: no release-cut surface touched
  • Linked issue closed via Closes #NNN in the ## Related section

Tests added:

Area Tests
memory/tree/health daemon-down → LocalModelUnavailable; model-not-pulled → same; through an anyhow context chain; negative: non-Ollama transport error stays Transient; degraded flag set with its own cause; negative: other codes do not flip the flag
memory/store/factories user_error payload is metadata-only (no prose, no endpoint); broadcast survives the Sentry latch (subscribes to the bus, asserts both calls emit); kind token matches the frontend discriminator
lib/userErrors/classify kind token + both prose shapes; negative: bare daemon unreachable from another domain is not promoted; negative: a credits error naming Ollama keeps billing remediation
services/socketService memory-sourced user_error scopes to memory, no raw message forwarded

Impact

  • Platform: desktop. Rust core + React renderer. No CLI, mobile, or web surface change.
  • Behaviour change: an Ollama-unusable embed failure now reports local_model_unavailable instead of transient. The panel status moves from a generic retry notice to the actionable remediation; the pipeline status pill stays degraded rather than error, matching derive_pipeline_status' contract for self-healing failures.
  • Compatibility: no persisted rows can carry the new code — it had no producer before this change — so the class change cannot reinterpret existing job history. PipelineFailure's wire shape is unchanged; latest_failed_job_failure still trusts the persisted class column.
  • Frontend contract: the UserErrorKind and UserErrorScope unions are widened additively. Nothing branches exhaustively on kind; the UI keys on action, severity, and scope, all of which are covered. The user_error handler keeps its cron default for unknown domains.
  • Security/privacy: the new broadcast is metadata-only and carries neither the configured endpoint (which can be a private host) nor raw provider text, matching the existing no-leak contract on that path. Pinned by a test.
  • Performance: one extra atomic store per failed embed and one small socket event per failed probe, both only while the local runtime is broken.
  • Migration: none.

Related

  • Closes: Memory embedder calls deprecated Ollama endpoint — broken since Ollama v0.32 #5354
  • Affected coverage-matrix feature IDs: none. The change touches embed-failure classification and user-error surfacing, neither of which has a matrix row; the nearest rows (8.2.2 Memory Consistency, 8.3.x retrieval benchmarks) cover ingest/retrieval behaviour that this PR does not alter.
  • Follow-up PR(s)/TODOs:
    • The reported UI slowdown after multi-day uptime, SQLite WAL growth, and memory-tree pruning were listed in the issue but are not acceptance criteria and are not addressed here. The unbounded transient-retry loop this PR leaves intact is a plausible but unproven contributor to the first.
    • is_ollama_user_config_rejection (core/observability.rs) anchors every arm on ollama embed failed, which the rewritten model-not-installed message no longer contains. Possibly a stale Sentry matcher; not touched here because I could not prove that message reaches a report site from this path.

AI Authored PR Metadata (required for Codex/Linear PRs)

Linear Issue

  • Key: N/A
  • URL: N/A

Commit & Branch

  • Branch: fix/5354-ollama-embed-error-classification
  • Commit SHA: 135339ba3

Validation Run

  • pnpm --filter openhuman-app format:check
  • pnpm typecheck
  • Focused tests: cargo test --lib -- memory::tree::health memory::store::factories (56 passed), cargo test --lib memory::tinycortex (32 passed), vitest src/lib/userErrors src/services/__tests__/socketService.events.test.ts (27 passed), plus a vitest related sweep over the changed files (432 files / 4986 tests passed)
  • Rust fmt/check (if changed): cargo fmt --all --check clean; cargo check --lib clean; slim --no-default-features --features tokenjuice-treesitter clean
  • Tauri fmt/check (if changed): N/A — no app/src-tauri change. See Validation Blocked.

Validation Blocked

  • command: pnpm rust:check
  • error: failed to load source for dependency 'tauri' … unable to update app/src-tauri/vendor/tauri-cef/crates/tauri … No such file or directory (os error 2)
  • impact: Environment only — the tauri-cef submodule is not initialised in this worktree; no app/src-tauri file is touched by this PR. The risk it stands in for is the shell's default-features = false build of the core, which was checked directly and compiles clean.

Behavior Changes

  • Intended behavior change: an unusable local Ollama runtime is classified local_model_unavailable (transient class) instead of transient, sets the semantic-recall degraded flag on first failure, and raises a durable UserErrorCenter entry.
  • User-visible effect: the memory status panel shows "Install/run Ollama and pull the model, or switch this workload to a cloud provider" instead of "a temporary error… will retry automatically", and a matching entry appears in the UserErrorCenter with a deep link to provider settings. Both clear on their own once Ollama is available again.

Parity Contract

  • Legacy behavior preserved: every other embed-failure code classifies exactly as before (pinned by a negative test that a non-Ollama transport error stays Transient). The user_error socket handler keeps its cron scope for the existing producer and for payloads with no error_source, pinned by the two pre-existing tests, which are unmodified.
  • Guard/fallback/dispatch parity checks: FailureCode round-trip and the all-codes class/remediation guard both updated in lockstep with the class change. PipelineFailure serde shape unchanged. The Ollama matcher is anchored on producer-specific wording on both sides of the language boundary so a wording drift fails a test rather than silently dropping the signal.

Duplicate / Superseded PR Handling

  • Duplicate PR(s): none
  • Canonical PR: this one
  • Resolution: N/A

Summary by CodeRabbit

  • New Features

    • Added clearer notifications when local models are unavailable, with direct access to provider settings.
    • Improved detection of missing or unreachable local model services.
    • Memory-related errors now display the correct scope and context.
    • Memory processing recovers automatically after local model services are restored.
  • Localization

    • Added translations in Arabic, Bengali, Chinese, English, French, German, Hindi, Indonesian, Italian, Korean, Polish, Portuguese, Russian, and Spanish.
  • Bug Fixes

    • Prevented unrelated connectivity errors from being misidentified as local model issues.

The memory embedder reported an unusable local Ollama runtime as a
generic transient fault. Both failure shapes the embedder produces --
"is Ollama running at <base>?" (daemon not listening) and "Ollama
embedding model `<id>` is not installed at <base>" (model never pulled)
-- carry no `Embedding API error (<status>)` envelope, so
`classify_embed_error_str` fell through to `Transient` and the memory
status panel told the user "a temporary error interrupted memory
processing, it will retry automatically". Retrying cannot start a daemon
or pull a model, and the actual fix was never named.

`FailureCode::LocalModelUnavailable` and its translated remediation
already existed but had no producer anywhere in the tree. Match the two
shapes explicitly so that code is emitted, and mark the semantic-recall
surface degraded at classification time so the remediation appears on
the first failed embed rather than after the retry budget drains.

Keep the code in the transient retry class. Only transient rows are
picked up by `requeue_transient_failed`, the automatic self-healing
requeue; classifying it unrecoverable would park every affected job
until someone pressed "Retry failed" by hand, so a user who simply
restarted Ollama would never see ingestion resume.

Also bridge the existing health-gate signal to the UI. The gate
published `DomainEvent::EmbeddingModelUnhealthy`, but nothing carries
the domain bus to the product UI -- `/events/domain` is read only by the
developer Event Log panel -- so that event reached no user. Broadcast
the condition over the metadata-only `user_error` web-channel path the
cron scheduler already uses, which lands a durable UserErrorCenter entry
with a deep link to provider settings. The broadcast deliberately sits
above the once-per-process Sentry latch: `publish_web_channel_event` is
an unbuffered broadcast send, memory is constructed before the renderer
socket attaches, and a single dropped send under the latch would be the
only attempt ever made. The panel store dedupes on the descriptor
identity, so repeats collapse into one entry.

The reported root cause -- a deprecated `/api/embeddings` call -- was
not present: the embedder has used `POST /api/embed` with `input` since
the provider port, and the deprecated path appears nowhere in the tree.

Closes tinyhumansai#5354
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bd72630c-edd5-4f37-92cf-3fc3cc57b9e7

📥 Commits

Reviewing files that changed from the base of the PR and between 5667870 and f61d55a.

📒 Files selected for processing (15)
  • app/src/lib/i18n/ar.ts
  • app/src/lib/i18n/bn.ts
  • app/src/lib/i18n/de.ts
  • app/src/lib/i18n/en.ts
  • app/src/lib/i18n/es.ts
  • app/src/lib/i18n/fr.ts
  • app/src/lib/i18n/hi.ts
  • app/src/lib/i18n/id.ts
  • app/src/lib/i18n/it.ts
  • app/src/lib/i18n/ko.ts
  • app/src/lib/i18n/pl.ts
  • app/src/lib/i18n/pt.ts
  • app/src/lib/i18n/ru.ts
  • app/src/lib/i18n/zh-CN.ts
  • src/openhuman/memory/tree/health/mod.rs
🚧 Files skipped from review as they are similar to previous changes (14)
  • app/src/lib/i18n/hi.ts
  • app/src/lib/i18n/it.ts
  • app/src/lib/i18n/ar.ts
  • app/src/lib/i18n/id.ts
  • app/src/lib/i18n/en.ts
  • app/src/lib/i18n/bn.ts
  • app/src/lib/i18n/zh-CN.ts
  • app/src/lib/i18n/pl.ts
  • app/src/lib/i18n/fr.ts
  • app/src/lib/i18n/de.ts
  • app/src/lib/i18n/ko.ts
  • src/openhuman/memory/tree/health/mod.rs
  • app/src/lib/i18n/ru.ts
  • app/src/lib/i18n/pt.ts

📝 Walkthrough

Walkthrough

The change detects unavailable local Ollama models, marks memory recall as degraded, broadcasts metadata-only errors, classifies them in the frontend, adds memory error scopes, and provides localized remediation in thirteen languages.

Changes

Local model unavailability handling

Layer / File(s) Summary
Backend failure detection and degradation
src/openhuman/memory/tinycortex/*.rs, src/openhuman/memory/tree/health/mod.rs
Embedding and health-gate paths classify Ollama failures as LocalModelUnavailable, update degradation state, and deduplicate outage announcements.
User error metadata and broadcasting
src/openhuman/memory/tree/health/user_error.rs, src/openhuman/memory/store/factories.rs
The backend constructs metadata-only local_model_unavailable events and broadcasts health-gate failures without raw provider details.
Frontend error contracts and ingestion
app/src/types/userError.ts, app/src/services/socketService.ts, app/src/lib/userErrors/*
Frontend types add the error kind and memory scope. Socket ingestion maps memory events to that scope, and classification provides provider-settings remediation.
Localized remediation messages
app/src/lib/i18n/{ar,bn,de,en,es,fr,hi,id,it,ko,pl,pt,ru,zh-CN}.ts
Translations add local-model guidance and the memory error scope label.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant EmbeddingPath
  participant HealthModule
  participant UserErrorPublisher
  participant SocketService
  participant FrontendClassifier
  EmbeddingPath->>HealthModule: classify local model failure
  HealthModule->>UserErrorPublisher: publish metadata-only user_error
  UserErrorPublisher->>SocketService: broadcast memory event
  SocketService->>FrontendClassifier: ingest with memory scope
  FrontendClassifier->>FrontendClassifier: select provider-settings remediation
Loading

Possibly related PRs

Suggested reviewers: senamakel

Poem

A rabbit found a model gone,
And sent a clear alert at dawn.
Memory marked the outage right,
While translated guides shed light.
Ollama hops back into sight.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: actionable classification of Ollama-unavailable memory embedding failures.
Linked Issues check ✅ Passed The PR satisfies [#5354] by detecting unavailable Ollama, showing actionable errors, preventing silent failure, and preserving retry behavior.
Out of Scope Changes check ✅ Passed The changes remain within scope by implementing memory error classification, health signaling, UI handling, localization, and related tests.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

Comment @coderabbitai help to get the list of available commands.

@YellowSnnowmann
YellowSnnowmann marked this pull request as ready for review August 4, 2026 14:11
@YellowSnnowmann
YellowSnnowmann requested a review from a team August 4, 2026 14:11
@coderabbitai coderabbitai Bot added bug memory Memory store, memory tree, recall, summarization, and embeddings in src/openhuman/memory/. rust-core Core Rust runtime in src/: CLI, core_server, shared infrastructure. labels Aug 4, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6fb354eed7

ℹ️ 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".

Comment thread src/openhuman/memory/tree/health/mod.rs
@greptile-apps

greptile-apps Bot commented Aug 4, 2026

Copy link
Copy Markdown

Greptile Summary

This PR wires up the existing but unreachable LocalModelUnavailable failure code end-to-end: Ollama daemon-down and model-not-pulled embed failures are now correctly classified, surfaced immediately on the status panel, and routed as durable user_error web-channel events to the UserErrorCenter — rather than silently retrying and showing a generic "will retry automatically" message.

  • Rust core: classify_embed_error_str adds Ollama-specific prose matchers for the two error shapes; mark_local_model_unavailable_if_applicable flips the semantic-recall degraded flag at classification time (not after retry budget exhaustion) and broadcasts on the first transition; report_ollama_health_gate_once calls the new broadcast above the Sentry latch so socket-attach races don't silence the entry permanently. Degraded-flag mark functions are upgraded from Ordering::Relaxed to Ordering::Release/Acquire.
  • Frontend: UserErrorKind and UserErrorScope gain local_model_unavailable and memory; classify.ts adds the matching prose + token rules; socketService derives scope from error_source rather than hard-coding cron.
  • i18n: New localModelUnavailable title/body and scope.memory keys added across all 14 supported locales.

Confidence Score: 5/5

Safe to merge. Changes are additive (new failure code branch, new broadcast, widened type unions), and no existing code paths are altered beyond the Relaxed→Release/Acquire memory-ordering fix, which is strictly correct and documented.

The classification change is anchored on Ollama-specific prose so non-Ollama transport errors are unaffected. The broadcast placement above the Sentry latch is the only structurally non-obvious decision, and it is thoroughly explained and covered by a dedicated test that exercises both the suppressed and unsuppressed call. Memory ordering upgrades follow the standard flag+data Acquire/Release pattern. All changed code paths have test coverage for both happy and failure cases, including negative guards for blast-radius regression.

Files Needing Attention: No files require special attention. The FailureClass::Unrecoverable doc comment (line 44 of health/mod.rs) still lists 'missing local model' as an example despite LocalModelUnavailable now mapping to Transient — a stale example worth cleaning up in a follow-up.

Important Files Changed

Filename Overview
src/openhuman/memory/tree/health/mod.rs Core classification logic: adds LocalModelUnavailable prose matchers, mark_local_model_unavailable_if_applicable with edge detection, and upgrades degraded-flag stores from Relaxed to Release/Acquire. Logic and memory ordering are correct.
src/openhuman/memory/tree/health/user_error.rs New module: owns the metadata-only user_error payload and publisher. Pinned by three tests (payload shape, wire token, source mapping). Clean.
src/openhuman/memory/store/factories.rs Moves surface_local_model_unavailable_to_clients() above the Sentry latch so socket-attach races before renderer connection don't permanently silence the UserErrorCenter entry. Tested by a new Rust unit test.
app/src/lib/userErrors/classify.ts Adds isLocalModelUnavailable rule last (after credits rules) with token + four anchored prose matchers. Both Rust-side error shapes are covered; negative tests for bare 'daemon unreachable' and credits-error-naming-Ollama guard the blast radius.
app/src/services/socketService.ts Derives scope from error_source (memory → 'memory', else 'cron') instead of hard-coding 'cron'. The binary mapping is acknowledged as intentional; existing cron tests are unmodified and pass.
app/src/types/userError.ts Additively widens UserErrorKind and UserErrorScope unions. No exhaustive branches on kind; no breaking change.
src/openhuman/memory/tinycortex/seal.rs Adds mark_local_model_unavailable_if_applicable call and structured debug log at the seal embed failure site. Clean addition.
src/openhuman/memory/tinycortex/queue_driver.rs Same mark_local_model_unavailable_if_applicable call added at the reembed failure site, with debug log. Clean addition parallel to seal.rs.
app/src/lib/userErrors/tests/classify.test.ts Adds four new test cases: token path, both prose shapes (daemon-down and model-not-pulled), negative for bare 'daemon unreachable', and negative for credits error naming Ollama. Comprehensive coverage.
app/src/services/tests/socketService.events.test.ts New test verifies that memory-sourced user_error events get scope='memory' and sourceDomain='memory', with no raw message forwarded.

Sequence Diagram

sequenceDiagram
    participant E as EmbedderBridge / queue_driver
    participant H as health::classify_embed_error
    participant M as mark_local_model_unavailable_if_applicable
    participant F as SEMANTIC_RECALL_DEGRADED (AtomicBool)
    participant W as publish_web_channel_event
    participant S as socketService (frontend)
    participant C as classifyUserActionableError
    participant UI as UserErrorCenter

    E->>H: "classify_embed_error(&error)"
    H-->>E: "PipelineFailure { code: LocalModelUnavailable }"
    E->>M: "mark_local_model_unavailable_if_applicable(&failure)"
    M->>F: load(Acquire) → already_surfaced?
    alt "First failure in outage (already_surfaced = false)"
        M->>F: mark_semantic_recall_degraded (Release)
        M->>W: publish_local_model_unavailable_user_error(embed_classify)
        W-->>S: "user_error {error_type: local_model_unavailable, error_source: memory}"
        S->>C: "classifyUserActionableError({errorType, scope: memory})"
        C-->>UI: "UserErrorDescriptor {kind: local_model_unavailable, action: open_provider_settings}"
    else "Subsequent failures (already_surfaced = true)"
        M->>F: mark_semantic_recall_degraded (Release)
        Note over M: No re-broadcast — panel dedupes anyway
    end
    Note over W,S: health gate (factories.rs) fires surface_local_model_unavailable_to_clients() above the Sentry latch, covering daemon-down races before socket attaches
Loading

Reviews (2): Last reviewed commit: "fix(memory): surface local-model errors ..." | Re-trigger Greptile

Comment thread app/src/lib/userErrors/classify.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🤖 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/it.ts`:
- Around line 7298-7299: Update the Italian translation for
userErrors.localModelUnavailable.body to use “non è installato” instead of
wording that means “never downloaded,” while preserving the existing Ollama and
cloud-provider guidance.

In `@app/src/lib/i18n/pl.ts`:
- Around line 7270-7271: Update the translation value for
userErrors.localModelUnavailable.body to describe the required Ollama model as
not installed at the configured endpoint, rather than saying it was never
downloaded. Preserve the existing guidance to start Ollama, install the model,
or move the task to a cloud provider.

In `@app/src/lib/i18n/zh-CN.ts`:
- Around line 6779-6780: Update the translation value for
userErrors.localModelUnavailable.body to replace the narrow “从未拉取” wording with
wording that covers any model not installed at the configured Ollama endpoint,
while preserving the existing guidance to start Ollama, obtain the model, or
switch to a cloud provider.

In `@app/src/lib/userErrors/classify.ts`:
- Around line 142-149: Update the isLocalModelUnavailable matcher in
classifyUserActionableError to recognize the raw missing-model message pattern
“Ollama embedding model `<id>` is not installed at <base>” in addition to the
existing signals. Add a regression test in classify.test.ts verifying this
message is classified as the expected actionable Ollama error rather than
returning null.

In `@src/openhuman/memory/tree/health/mod.rs`:
- Around line 271-296: Move the local-model error matching from
src/openhuman/memory/tree/health/mod.rs#L271-L296 into a focused sibling module
and re-export its helper from mod.rs. Move the degradation state helpers from
src/openhuman/memory/tree/health/mod.rs#L487-L508 similarly, keeping mod.rs
export-focused. In src/openhuman/memory/store/factories.rs#L37-L60, retain only
thin factory orchestration and delegate health-gate event behavior; move the
event payload and publisher from
src/openhuman/memory/store/factories.rs#L114-L154 into that dedicated module.
Ensure each Rust file remains at or below 500 lines and preserve existing public
behavior through re-exports.
- Around line 502-507: Add debug-level [domain] or [rpc] correlated tracing for
the local-model failure branch and semantic recall state transition in
src/openhuman/memory/tree/health/mod.rs:502-507, passing safe caller correlation
from src/openhuman/memory/tinycortex/queue_driver.rs:185-187 and a safe
seal-operation correlation from src/openhuman/memory/tinycortex/seal.rs:29-33.
In src/openhuman/memory/store/factories.rs:134-139, use the required prefix and
trace the web-channel publish operation. Log only safe correlation fields; never
include raw errors, endpoints, credentials, or input text.
- Around line 487-508: Update mark_semantic_recall_degraded and
current_degraded_state to publish and read SEMANTIC_RECALL_DEGRADED with
SEMANTIC_RECALL_CAUSE as one atomic state, or under a shared lock, so readers
cannot observe a new degraded flag with a stale cause. Preserve existing
cause-clearing and recovery behavior, and add a concurrent test covering the
flag/cause interleaving.
🪄 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: 19c70dea-dd00-4979-91c4-abb5b7be44f9

📥 Commits

Reviewing files that changed from the base of the PR and between 202b313 and 6fb354e.

📒 Files selected for processing (23)
  • app/src/lib/i18n/ar.ts
  • app/src/lib/i18n/bn.ts
  • app/src/lib/i18n/de.ts
  • app/src/lib/i18n/en.ts
  • app/src/lib/i18n/es.ts
  • app/src/lib/i18n/fr.ts
  • app/src/lib/i18n/hi.ts
  • app/src/lib/i18n/id.ts
  • app/src/lib/i18n/it.ts
  • app/src/lib/i18n/ko.ts
  • app/src/lib/i18n/pl.ts
  • app/src/lib/i18n/pt.ts
  • app/src/lib/i18n/ru.ts
  • app/src/lib/i18n/zh-CN.ts
  • app/src/lib/userErrors/__tests__/classify.test.ts
  • app/src/lib/userErrors/classify.ts
  • app/src/services/__tests__/socketService.events.test.ts
  • app/src/services/socketService.ts
  • app/src/types/userError.ts
  • src/openhuman/memory/store/factories.rs
  • src/openhuman/memory/tinycortex/queue_driver.rs
  • src/openhuman/memory/tinycortex/seal.rs
  • src/openhuman/memory/tree/health/mod.rs

Comment thread app/src/lib/i18n/it.ts Outdated
Comment thread app/src/lib/i18n/pl.ts Outdated
Comment thread app/src/lib/i18n/zh-CN.ts Outdated
Comment thread app/src/lib/userErrors/classify.ts
Comment thread src/openhuman/memory/tree/health/mod.rs
Comment thread src/openhuman/memory/tree/health/mod.rs
Comment thread src/openhuman/memory/tree/health/mod.rs
Review follow-ups on tinyhumansai#5354.

The embedder health gate probes `GET /api/tags`, which succeeds whenever
the daemon is up. A running daemon whose embedding model was never pulled
therefore never tripped that gate, so the "model not installed" half of
`LocalModelUnavailable` set the degraded flag but never raised the durable
UserErrorCenter entry. The failure classifier now publishes it as well,
covering both halves from the one place that has actually classified the
failure. It fires on the transition into the state rather than per failed
embed, since the re-embed path calls it per row.

The payload and publisher move to a dedicated `health::user_error` module
so both producers emit one identical, tested shape.

Publish the degraded cause before its flag, and pair a Release store with
an Acquire load, so a concurrent status read cannot observe a freshly-set
flag alongside the previous degradation's cause and render the wrong
remediation.

Teach the frontend classifier the second Ollama prose shape ("embedding
model ... is not installed at"). It only recognised the daemon-down shape,
so a raw message carrying the model-not-pulled text fell through to null
despite the comment claiming both were covered.

Reword the remediation copy from "never pulled" to "not installed" across
all fourteen locales, English included: the backend condition also covers
a model that was removed or lives on a different endpoint.

Add correlated debug traces at both classification sites, carrying only
the embedder identity, operation, and typed outcome.
coderabbitai[bot]
coderabbitai Bot previously approved these changes Aug 4, 2026
The constant is only referenced across module boundaries from a test, so
re-exporting it from the health module left an unused import in the
non-test build, which `clippy -D warnings` rejects. Expose the module
itself instead and let the one cross-module consumer path to it.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YellowSnnowmann has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 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/bn.ts`:
- Line 7166: Update the userErrors.localModelUnavailable message to state that
the required model must be installed at the configured Ollama endpoint,
including when it exists on another Ollama instance. Apply this wording
consistently in the source translation and every locale file, including the
Bengali entry shown here.

In `@app/src/lib/i18n/en.ts`:
- Line 7563: Update the userErrors.localModelUnavailable translation to clarify
that the required model must be installed and available at the configured Ollama
endpoint, including cases where it is missing or hosted on another instance;
retain the guidance to switch to a cloud provider.

In `@app/src/lib/i18n/fr.ts`:
- Line 7346: Update the French `localModelUnavailable` translation so it is
domain-neutral for chat, cron, and memory usage, and explicitly states that the
required model must be installed on the configured Ollama endpoint. Replace the
task-specific remediation wording with endpoint-focused guidance to start the
endpoint and install the model there, or use a cloud provider.

In `@src/openhuman/memory/tree/health/mod.rs`:
- Around line 523-539: Make the check-and-update flow around
SEMANTIC_RECALL_DEGRADED, SEMANTIC_RECALL_CAUSE, and
mark_semantic_recall_degraded atomic so only one concurrent caller publishes the
local-model-unavailable user error. Serialize the state check, update, and
publish with a lock or atomic state machine, preserving the existing failure
classification and logging behavior; add a deterministic concurrent test proving
only one transition is published.
- Around line 523-539: Update the semantic-recall degradation flow around
mark_semantic_recall_degraded and publish_local_model_unavailable_user_error so
a missing-model error is delivered when no client was subscribed at the initial
failure. Persist the active LocalModelUnavailable notification for replay on
socket subscription, or implement bounded re-emission until recovery, while
retaining suppression after successful delivery. Add a test covering failure
before subscription followed by subscription and another failed embed.
🪄 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: b6767bd8-3c6c-4e61-8b6b-f48bcf7dfe6e

📥 Commits

Reviewing files that changed from the base of the PR and between 6fb354e and 5667870.

📒 Files selected for processing (21)
  • app/src/lib/i18n/ar.ts
  • app/src/lib/i18n/bn.ts
  • app/src/lib/i18n/de.ts
  • app/src/lib/i18n/en.ts
  • app/src/lib/i18n/es.ts
  • app/src/lib/i18n/fr.ts
  • app/src/lib/i18n/hi.ts
  • app/src/lib/i18n/id.ts
  • app/src/lib/i18n/it.ts
  • app/src/lib/i18n/ko.ts
  • app/src/lib/i18n/pl.ts
  • app/src/lib/i18n/pt.ts
  • app/src/lib/i18n/ru.ts
  • app/src/lib/i18n/zh-CN.ts
  • app/src/lib/userErrors/__tests__/classify.test.ts
  • app/src/lib/userErrors/classify.ts
  • src/openhuman/memory/store/factories.rs
  • src/openhuman/memory/tinycortex/queue_driver.rs
  • src/openhuman/memory/tinycortex/seal.rs
  • src/openhuman/memory/tree/health/mod.rs
  • src/openhuman/memory/tree/health/user_error.rs
🚧 Files skipped from review as they are similar to previous changes (12)
  • app/src/lib/userErrors/tests/classify.test.ts
  • app/src/lib/userErrors/classify.ts
  • app/src/lib/i18n/id.ts
  • app/src/lib/i18n/ko.ts
  • app/src/lib/i18n/ru.ts
  • app/src/lib/i18n/pt.ts
  • app/src/lib/i18n/es.ts
  • app/src/lib/i18n/it.ts
  • app/src/lib/i18n/de.ts
  • src/openhuman/memory/tinycortex/queue_driver.rs
  • app/src/lib/i18n/hi.ts
  • src/openhuman/memory/tinycortex/seal.rs

Comment thread app/src/lib/i18n/bn.ts Outdated
Comment thread app/src/lib/i18n/en.ts Outdated
Comment thread app/src/lib/i18n/fr.ts Outdated
Comment thread src/openhuman/memory/tree/health/mod.rs Outdated
Review round two on tinyhumansai#5354.

The transition check read the degraded flag and then set it, so two
concurrent embed tasks could both conclude they were first and both
publish. Claim the announcement with a compare_exchange on a dedicated
latch instead, making check-and-claim indivisible.

The latch is released by `clear_semantic_recall_degraded`, which every
write-embedder build calls once per seal or re-embed operation. That
turns the single announcement into bounded re-emission until recovery,
which is what covers a client that was not connected when the outage
began: `publish_web_channel_event` is an unbuffered broadcast with no
replay, so an announcement made before anyone subscribed is simply gone.

Make the remediation copy endpoint-aware and domain-neutral in all
fourteen locales. With more than one Ollama instance reachable, "start
Ollama and pull the model" can send the user to repair the wrong machine,
and the string is shared by the chat, cron, and memory scopes so it must
not describe the work as a task.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YellowSnnowmann has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@coderabbitai coderabbitai Bot removed bug rust-core Core Rust runtime in src/: CLI, core_server, shared infrastructure. memory Memory store, memory tree, recall, summarization, and embeddings in src/openhuman/memory/. labels Aug 4, 2026

@M3gA-Mind M3gA-Mind left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed against upstream/main: CI green on every lane that applies to the changed areas, no unresolved review threads, and the diff does what the title and body claim. Checked correctness, blast radius on shared seams, that the behaviour change is pinned by a test rather than only asserted, and i18n/em-dash compliance on any new user-facing string. Nothing blocking found.

@M3gA-Mind
M3gA-Mind merged commit ccad8f5 into tinyhumansai:main Aug 5, 2026
23 of 24 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in Team Openhuman Aug 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Memory embedder calls deprecated Ollama endpoint — broken since Ollama v0.32

2 participants