Skip to content

Send the provider name first on the Embed wire (module-mode embeddings were refused at decode) - #111

Merged
YellowSnnowmann merged 2 commits into
tinyhumansai:mainfrom
YellowSnnowmann:fix/5820-embed-wire-order
Aug 27, 2026
Merged

Send the provider name first on the Embed wire (module-mode embeddings were refused at decode)#111
YellowSnnowmann merged 2 commits into
tinyhumansai:mainfrom
YellowSnnowmann:fix/5820-embed-wire-order

Conversation

@YellowSnnowmann

@YellowSnnowmann YellowSnnowmann commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

What

Every embed a module-mode engine performs goes over the bus to the host's EmbeddingHost::embed(provider, model, dimensions, texts). The module sent three arguments, (model, dimensions, texts), so dimensions landed where the host reads model and every batch was refused at decode:

Embed: bad arguments: invalid type: integer, expected a string

Nothing ingested in module mode has had a vector since the host grew the provider argument (openhuman 3ee5a3cad, 2026-08-12): the reembed_backfill job burns its three attempts, the tree reports "Degraded", every source shows "Stored without vectors". Found while testing openhuman#5820 end to end; the fix is the openhuman-side half of tinyhumansai/openhuman#5823.

Changes

  • BusEmbeddingProvider::embed sends (name, model, dimensions, texts). The host resolves credential and endpoint from the name, which is why it comes first.
  • The default provider names itself cloud rather than the invented module-bus. The host's default is its managed-cloud embedder built from the same cloud_embedding_model/cloud_embedding_dimensions it sent, and cloud is the factory arm that builds it; an unknown slug fails every batch just as surely as the wrong arity did.

Tests

The test fake now declares the host's real four-argument Embed, in the host's order, and records what it was asked for; a new case pins the order for the managed, local and BYO-key providers. A fake with the wrong arity passed every test here while the real host refused every call, which is how this shipped.

cargo test --lib embedding in the module's own workspace: 15 passed.

Verified live against openhuman fix/5820-memory-corruption-reporting with TINYMEMORY_TEST_MODULE pointing at this build (results in the openhuman PR).

🤖 Generated with Claude Code

https://claude.ai/code/session_01Ufhq47VCos7Tw9zCyYEXmR

Summary by CodeRabbit

  • Bug Fixes
    • Corrected cloud embedding provider identification for more reliable host-side routing.
    • Fixed embedding requests so provider, model, dimensions, and text values are sent in the correct order.
    • Improved compatibility across cloud, Ollama, and custom-key embedding providers.

…`cloud`

Every embed a module-mode engine performs goes over the bus to the host's
`EmbeddingHost::embed(provider, model, dimensions, texts)`. The module sent
three arguments, `(model, dimensions, texts)`, so `dimensions` landed where the
host reads `model` and every batch was refused at decode with

    Embed: bad arguments: invalid type: integer, expected a string

Nothing ingested in module mode has had a vector since the host grew the
`provider` argument (openhuman 3ee5a3cad, 2026-08-12): the reembed backfill
job burns its three attempts, the tree reports "Degraded", and every source
shows "Stored without vectors" (openhuman#5820).

Two changes:

* `BusEmbeddingProvider::embed` sends `(name, model, dimensions, texts)`. The
  host resolves credential and endpoint from the name, which is why it comes
  first.
* The default provider names itself `cloud` rather than the invented
  `module-bus`. The host's default is its managed-cloud embedder built from
  the same `cloud_embedding_model`/`cloud_embedding_dimensions` it sent, and
  `cloud` is the factory arm that builds it; an unknown slug fails every
  batch just as surely as the wrong arity did.

The test fake now declares the host's real four-argument `Embed`, in the
host's order, and records what it was asked for; a new case pins the order
for the managed, local and BYO-key providers. A fake with the wrong arity
passed every test here while the real host refused every call, which is how
this shipped.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ufhq47VCos7Tw9zCyYEXmR
@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown

Review Change Stack

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a127be15-8484-4d94-9a3a-7e649b8ef5b1

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2ef9193e-f723-4545-96b6-445f4e055e31

📥 Commits

Reviewing files that changed from the base of the PR and between e26383d and bad09f5.

📒 Files selected for processing (2)
  • crates/tinymemory-module/src/embedding.rs
  • crates/tinymemory-module/src/embedding_test.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The embedding module now identifies the default provider as cloud and sends provider, model, dimensions, and texts to the host in the declared order. Tests update the fake host and verify the argument sequence across supported providers.

Changes

Embedding host contract

Layer / File(s) Summary
Provider routing and bus call contract
crates/tinymemory-module/src/embedding.rs
The default provider uses cloud. Bus embedding requests include provider, model, dimensions, and texts in the host-defined order.
Host contract regression coverage
crates/tinymemory-module/src/embedding_test.rs
FakeHostEmbedder records call metadata. Constructors replace struct literals. Tests verify the cloud identity and argument order for cloud, ollama, and voyage providers.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to bad09

This localized change corrects the embedding wire argument order and provider name, with focused coverage for supported provider types. No actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: senamakel

Poem

A rabbit checks the cloud-bound trail
Provider first, then model’s detail
Dimensions hop behind
Texts arrive aligned
The host now reads the tale!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 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: sending the provider name first in the Embed wire arguments. It also states the resulting decode failure that motivated the fix.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Warning

Your free Security trial is over. An organization admin can activate Security or dismiss this notice.


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

@YellowSnnowmann

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@tinysweeper

tinysweeper Bot commented Aug 27, 2026

Copy link
Copy Markdown

How this change flows

7 changed behaviours across 24 relationships. 4 surrounding behaviours are shown (60 graph nodes walked). 39 further behaviours left out to keep the diagram readable.

flowchart LR
  n0["..._request_still_builds_and_ignores_the_key<br/>changed"]:::changed
  n1["a_wrong_vector_count_is_refused<br/>changed"]:::changed
  n2["a_wrong_width_is_refused_rather_than_written<br/>changed"]:::changed
  n3["...est_answered_with_real_vectors_is_refused<br/>changed"]:::changed
  n4["config_with_dims<br/>changed"]:::changed
  n5["...rovider_factories_preserve_their_identity<br/>changed"]:::changed
  n6["...on_support_is_answered_from_configuration<br/>changed"]:::changed
  n7["assert"]:::impacted
  n8["FakeHostEmbedder"]:::impacted
  n9["bus_with_host"]:::impacted
  n10["embed"]:::impacted
  n0 -->|calls| n4
  n0 -->|uses| n8
  n0 -->|calls| n9
  n0 -->|calls| n10
  n1 -->|calls| n4
  n1 -->|calls| n7
  n1 -->|uses| n8
  n1 -->|calls| n9
  n1 -->|calls| n10
  n2 -->|calls| n4
  n2 -->|calls| n7
  n2 -->|uses| n8
  n2 -->|calls| n9
  n2 -->|calls| n10
  n3 -->|calls| n4
  n3 -->|calls| n7
  n3 -->|uses| n8
  n3 -->|calls| n9
  n3 -->|calls| n10
  n5 -->|calls| n7
  n5 -->|uses| n8
  n5 -->|calls| n9
  n6 -->|calls| n4
  n6 -->|calls| n7
  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
Loading

Green: changed behaviour. Grey: surrounding behaviour. Arrows name the call, use, implementation, or test relationship. Orange: has findings. Red: has a finding that blocks the merge.

tinysweeper 0.1.0

@tinysweeper tinysweeper 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.

tinysweeper found nothing blocking. Approving.

$0.0000 · 0 in / 0 out

@tinysweeper tinysweeper Bot added the priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. label Aug 27, 2026
YellowSnnowmann added a commit to YellowSnnowmann/openhuman that referenced this pull request Aug 27, 2026
… cloud defaults

Two more findings from testing the module path end to end, both host-side;
the module half is tinyhumansai/tinymemory#111.

Source syncs timed out at 30 s while still running. `RunSourceSync`,
`RunConnectionSync` and `BootstrapConnection` rode tinybus's 30 s default
deadline, and one Gmail page alone is ~31 s end to end: the caller was
released with "call to `RunSourceSync` timed out after 30000ms" while the
module kept fetching and ingesting, and finished. The UI reported a failure
for work that then succeeded. The frontend's own default per-call timeout is
30 s as well, so the fetch aborted first. Both layers now carry a budget that
covers a whole run: `SOURCE_SYNC_BUS_TIMEOUT` (the client's 600 s clamp
ceiling plus the grace `IngestCodingSessions` already uses, so the client's
abort with its clean message is the one that fires if a run wedges) and
`MEMORY_SYNC_RPC_TIMEOUT_MS` on `memory_sources_sync` and
`memory_sources_apply_all_in` (All In runs every enabled source to completion
before answering). Same failure class and same fix as tinyhumansai#5802.

The module's cloud fallback asked the managed embedder for the user's local
model. `module_config` sent `config.memory.embedding_model` as
`cloud_embedding_model`, but that field is the module's
`EmbeddingHost::default_cloud_embedding_model`: what its engine switches to
when the opted-in Ollama model is unreachable. The in-process host answers
`DEFAULT_CLOUD_EMBEDDING_MODEL`; the module was told `nomic-embed-text` and
built a reembed job signed `provider=cloud;model=nomic-embed-text`. The host
constants travel now, along with the `text-embedding-3-*` list the module's
synchronous `model_supports_dimensions` needs (`MODELS_SUPPORTING_DIMENSIONS`).

Together with tinymemory#111 (the module sent `Embed(model, dims, texts)`
where the host reads `(provider, model, dims, texts)`, so every batch was
refused at decode) this is what stood between a module-mode install and any
vector at all: verified live, `mem_tree_chunk_embeddings` went 0 -> 124 and the
stuck `reembed_backfill` job completed on first attempt.

Tests: `Embed` arity/order pinned host-side the way `Execute` already is;
`module_config` asserts the cloud defaults and that the user's model still
travels as `memory.embedding_model`; the service test carries the new budget.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ufhq47VCos7Tw9zCyYEXmR
`recall_reaches_the_host_embedder` dlopens the real module against a fake
host whose `Embed` still declared the module's old `(model, dimensions,
texts)`. With the module now sending `(provider, model, dimensions, texts)`
the fake refused the call, which is the same mismatch the previous commit
fixed in the other direction. The fake now declares the host's signature and
asserts the slug that arrives is `cloud`, the arm the host builds its managed
embedder from.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ufhq47VCos7Tw9zCyYEXmR
@YellowSnnowmann
YellowSnnowmann merged commit 51f3930 into tinyhumansai:main Aug 27, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant