Repository navigation
Conversation
The module asks the host to embed with provider "custom", but
EmbeddingCallbacks::embed only looked the endpoint up in
config.cloud_providers, where nothing is ever named "custom". The
endpoint came back empty, so every seal and reembed_backfill failed with
"custom embedding provider endpoint must not be empty" and the memory
tree never got a vector.
Read the endpoint from memory.embedding_provider ("custom:<url>") when no
cloud provider matches, the same way embedding_host::rpc::embed does.
Cloud provider lookup still wins, and a blank "custom:" keeps failing in
the endpoint validation.
Closes tinyhumansai#6984
Tiny Sweeper reviewTiny Sweeper reviewed this change across 6 lane(s) and found 1 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below. State: Incomplete Review snapshot
Completeness: Incomplete What changedThe review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below. FeaturesNone identified with supported citations. TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. FindingsPreviously reported and still active
Pending checks: Rust E2E (mock backend), Build Playwright E2E Artifact, E2E (Playwright / web lane), Desktop E2E (full suite, 3 OS) Could not review: crates/openhuman-core/src/modules/memory_host.rs, crates/openhuman-core/src/modules/memory_host_tests.rs Before merge
How this fits togetherflowchart LR
n0["...uments_in_the_order_the_module_sends_them<br/>changed"]:::changed
n1["scoped_config"]:::impacted
n2["call"]:::impacted
n3["expect"]:::impacted
n4["api_key_is_the_hosts_own_credential_store"]:::impacted
n5["...config_key_when_credential_store_is_empty"]:::impacted
n0 -->|calls| n1
n0 -->|tests| n1
n0 -->|calls| n2
n0 -->|tests| n2
n0 -->|calls| n3
n2 -->|calls| n3
n4 -->|calls| n1
n4 -->|tests| n1
n4 -->|calls| n2
n4 -->|tests| n2
n4 -->|calls| n3
n5 -->|calls| n2
n5 -->|tests| n2
n5 -->|calls| n3
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe memory host now falls back to the configured custom embedding endpoint when cloud-provider lookup finds no endpoint. Tests cover successful embedding and rejection of a whitespace-only endpoint. ChangesCustom embedding resolution
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to The configured custom endpoint reaches the memory host, while blank endpoints remain rejected. No concrete merge-blocking risk is established. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to Changing a custom embedding profile without restarting may leave requests using the previous address. Because credentials are looked up separately, memory text—and potentially a replacement API key—could continue going to the previous provider. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation For
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
A rabbit checked the custom route, Comment |
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
tinysweeper found nothing blocking, but could not review everything, so this is not an approval: crates/openhuman-core/src/modules/memory_host.rs, crates/openhuman-core/src/modules/memory_host_tests.rs, tinysweeper/description.
$0.0006 · 17,390 in / 1,554 out · 0 cached (0%) · deepseek/deepseek-v4-flash
tests: $0.0001 · 5,115 in / 81 out · 0 cached (0%) · deepseek/deepseek-v4-flash
e2e: $0.0003 · 9,025 in / 514 out · 0 cached (0%) · deepseek/deepseek-v4-flash
| /// `memory.embedding_provider = "custom:<url>"`, never as a `cloud_providers` | ||
| /// entry, so the slug lookup in `embed` cannot find it (#6984). This reads it | ||
| /// the way `embedding_host::rpc::embed` already does. | ||
| fn custom_embedding_endpoint<'a>(config: &'a Config, provider: &str) -> Option<&'a str> { |
There was a problem hiding this comment.
Cover the custom embedding endpoint fallback with an end-to-end test
The custom_embedding_endpoint function and its call site in embed are new behavioural code that resolves a user-configured custom embedding endpoint. No end-to-end test exercises this path: the only E2E candidate mentions (tests/agent_harness_e2e.rs and app/test/e2e/specs/memory-roundtrip.spec.ts) do not set up an EmbeddingCallbacks with memory.embedding_provider = "custom:<url>" or call embed through the module hosting path. Without an E2E test that configures a custom endpoint (e.g. via a mock server), this code could regress silently in the deployed app. A Playwright or Rust E2E test should spin up a mock embedding server, set the config key, and verify that a vector is produced.
[RULE] e2e-uncovered ·
tinysweeper
Bot
added
the
priority: p2
There was a problem hiding this comment.
tinysweeper found nothing blocking, but could not review everything, so this is not an approval: crates/openhuman-core/src/modules/memory_host.rs, crates/openhuman-core/src/modules/memory_host_tests.rs.
$0.0010 · 34,130 in / 1,369 out · 0 cached (0%) · deepseek/deepseek-v4-flash
tests: $0.0002 · 8,517 in / 95 out · 0 cached (0%) · deepseek/deepseek-v4-flash
description: $0.0003 · 9,528 in / 98 out · 0 cached (0%) · deepseek/deepseek-v4-flash
e2e: $0.0004 · 12,427 in / 469 out · 0 cached (0%) · deepseek/deepseek-v4-flash
| .iter() | ||
| .find(|candidate| candidate.slug == provider) | ||
| .map(|candidate| candidate.endpoint.as_str()) | ||
| .or_else(|| custom_embedding_endpoint(&self.0, &provider)) |
There was a problem hiding this comment.
Cover the custom embedding endpoint fallback with an end-to-end test
The change adds custom_embedding_endpoint as a fallback when a custom embedding provider is configured via memory.embedding_provider. This path has unit tests but no end-to-end test. An E2E test (e.g., in a Playwright spec or Rust integration test that drives the core via RPC) should configure a custom endpoint, embed a memory, and verify that vectors are produced. Without it, a future refactor could silently break the fallback and the CI suite would not catch it.
[RULE] e2e-uncovered ·
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
tinysweeper found nothing blocking, but could not review everything, so this is not an approval: crates/openhuman-core/src/modules/memory_host.rs, crates/openhuman-core/src/modules/memory_host_tests.rs.
$0.0011 · 34,355 in / 1,657 out · 0 cached (0%) · deepseek/deepseek-v4-flash
tests: $0.0002 · 8,543 in / 115 out · 0 cached (0%) · deepseek/deepseek-v4-flash
description: $0.0003 · 9,627 in / 111 out · 0 cached (0%) · deepseek/deepseek-v4-flash
e2e: $0.0004 · 12,453 in / 561 out · 0 cached (0%) · deepseek/deepseek-v4-flash
|
hey bro. dropping this since we're now completly changing the memory engine to 3rd party implementations that have their own embeddign providers |
Summary
"custom"embedding provider inmemory.embedding_provider(custom:<url>), the same placeembedding_host::rpc::embedalready reads it from.reembed_backfillfailed with "custom embedding provider endpoint must not be empty".Problem
EmbeddingCallbacks::embedinmodules/memory_host.rslooked the endpoint up only inconfig.cloud_providersby slug. The module calls it withprovider = "custom", and no cloud provider is ever namedcustom, so the endpoint wasNoneand tinyinference'svalidate_custom_endpointrefused the call.mem_tree_jobs.last_error, so the memory tree silently ends up with no vectors and semantic recall doesn't work. Details and repro in Memory host ignores the custom embedding endpoint, so nothing gets embedded #6984.Solution
custom_embedding_endpointhelper. When the provider is"custom"and no cloud provider matches, it returns the URL after thecustom:prefix inmemory.embedding_provider.custom:still resolves to no endpoint, so the existing validation error is kept for a misconfigured profile instead of guessing.[custom_embeddings]on purpose. The RPC embed path doesn't use it either, and keeping both paths identical seemed safer. Happy to add it if you want it.Submission Checklist
embed_resolves_a_custom_endpoint_from_the_memory_embedding_provider(wiremock OpenAI-compatible server, asserts the returned vector) andembed_with_a_blank_custom_endpoint_still_fails_in_the_host.memory_host.rsare covered (100%), measured locally withcargo llvm-cov -p openhuman --lib -- modules::memory_host. The CIrust-covlane did not get as far as the diff-cover step on this run because two unrelated suites fail onmain(tokenjuice::repl_module_tests::a_large_result_becomes_a_handle_the_repl_tools_can_queryandagent_prompt_comprehension_e2e::summarizer_advertises_no_tools), see test: fix lib tests failing on main #6982.## Related: N/A, no matrix rows affected.wiremockserver (already a dev-dependency).Closes #NNNin the## Relatedsection.Impact
validate_custom_endpoint).Related
mem_tree_jobs.last_error. Logging them at warn level or surfacing them in Memory health would make this class of bug much easier to spot. Left out to keep this PR focused.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
bodharma:fix/memory-host-custom-embedding-endpointb1cfbeef13b16fee8f06eb2e9b146a579d3a2605Validation Run
pnpm --filter openhuman-app format:check, no frontend files changed. The Rust part ran ascargo fmt --all -- --check, clean.pnpm typecheck, no TypeScript changed.cargo test -p openhuman --lib modules::memory_host(17 passed, including the two new ones). The new happy-path test failed onmainwith the exact error from Memory host ignores the custom embedding endpoint, so nothing gets embedded #6984 before the fix.cargo fmt --all -- --checkandcargo clippy -p openhuman -- -D warnings, both clean. The pre-push hook passed too.pnpm test:rust(scripts/test-rust-with-mock.sh) gave 9158 passed, 5 failed. None of the failures touch the memory host, see below.Validation Blocked
command:pnpm test:ruston macOS 26.6 arm64error:5 failures unrelated to this change:inference::ops::tests::inference_get_client_config_returns_safe_snapshot,tools::implementations::system::shell::tests::runtime_and_sandbox_tests::shell_sandboxed_mode_routes_through_sandbox_backendandshell_uses_cached_python_path_in_native_modefail the same way on a clean checkout ofmain(d0d1e51ea) on this machine.inference::tokenjuice::repl_module_tests::a_large_result_becomes_a_handle_the_repl_tools_can_queryandsandbox::docker::exec_tests::timeout_reports_timed_out_resultfailed only inside the full run. Each passes when run on its own with this branch (twice each).impact:none on this PR as far as I can tell. They look environment-specific (local sandbox / python / docker setup) or flaky under full-suite load.Lanes (checks)fails onstatic:module-pins(vendor/tinyboxis atv0.1.14-5-ge7a04c8awhileregistry.rspins0.1.14) andstatic:linux-tls-policy(crates/openhuman-app/Cargo.lockneeds an update under--locked). This PR touches neither, and Fix composer routing for selected and persisted models #6978 fails the same lane.Behavior Changes
memory.embedding_provider = "custom:<url>".Parity Contract
embedding_host::rpc::embed(stripcustom:frommemory.embedding_provider).Duplicate / Superseded PR Handling