fix(embedding): recover truncated responses and validate vectors

[loveRhythm1990]

What type of PR is this?

• fix (bug fix)

Which issue(s) this PR fixes

Related: #275 (HTTP client portion); owner-scoped idempotency and a structured precommit outcome remain separate work.

What this PR does / why we need it

An interrupted embedding response previously exited the retry loop after HTTP headers arrived, aborting memory ingestion even when the next embedding attempt could succeed. Read the body separately from JSON parsing and retry body-read failures and JSON EOF/IO within the existing three-attempt budget, before memory persistence. This includes partial JSON in responses without Content-Length and in chunked responses.

JSON Syntax/Data failures and nonretryable HTTP errors fail promptly. Existing 5xx/429 retries remain unchanged. Return stable timeout, connection, request-body, response-body, and parse classifications without exposing upstream bodies, unexpected JSON values, request URLs, or credentials. HTTP status remains available for non-success responses; raw or truncated provider-body logging is intentionally omitted because it may reveal sensitive values.

Validate response count and every vector's configured dimension before returning single or batch results. When indexes are supplied, require a complete, unique 0..N-1 set and return vectors in input order. Providers that omit all indexes remain compatible and retain response order; their semantic ordering cannot be checked without indexes. Mixed, duplicate, and out-of-range indexes fail promptly.

The retry budget is three attempts with a 30-second request timeout and 200/400 ms backoff (approximately 90.6 seconds of request/backoff budget); semaphore acquisition is separate, defaulting to five seconds. This is not a new end-to-end deadline.

This addresses the confirmed HTTP-client retry gap and response validation. It does not replay memory writes, add an atomic idempotency/precommit contract, or establish the cause of the original live provider response.

Validation

• cargo test --offline -p memoria-embedding --lib: 37 passed, including 18 loopback HTTP/client tests.
• Coverage: single/batch recovery with Content-Length, close-delimited and chunked responses; JSON EOF recovery and exhaustion; incomplete chunk framing; invalid JSON/schema without retry or value disclosure; connection failures; nonretryable authentication; 503/429 retries; normal success; response count/dimension validation; indexed ordering and invalid indexes.
• Targeted Clippy (--all-targets -- -D warnings), rustfmt, and git diff --check: passed.
• Both modified Rust files across the two related PR updates were reviewed (2/2 files, no skipped files).
• No live service deployment or benchmark rerun in this PR. CI results are reported by the checks on the current head.

[loveRhythm1990]

Review

The issue is valid, and the root-cause analysis checks out. In reqwest 0.12.28 (the version in our Cargo.lock), Response::bytes() maps body-read errors through crate::error::decode, the same kind that json() uses for a serde failure. Both print as error decoding response body, so the log line in #275 can't tell truncation from invalid JSON. Either way, the old loop returned straight out of the retry loop. Splitting bytes() from serde_json::from_slice is the right fix.

cargo test -p memoria-embedding --lib passes on the PR head (27 tests plus one probe I added locally).

Main concern: a truncated body without Content-Length is still not retried

The truncation fixtures always send Content-Length: body.len() + 100, so hyper detects the short body and bytes() fails. Many gateways and proxies send close-delimited or chunked responses instead. When such a connection ends early, bytes() can succeed with a partial buffer, and the failure appears as a serde EOF error. I checked this locally with a fixture that sends the same truncated body without a Content-Length header:

body = {"data":[{"embedding":[0.1,     (no Content-Length, Connection: close)
→ Err("Embedding error: invalid embedding response JSON"), requests = 1   // not retried

That is the same truncation scenario #275 suspects, but it fails immediately. Suggested fix: treat serde_json::error::Category::Eof (and Io) as retryable, the same way as a bytes() failure, and keep Syntax/Data non-retryable:

match serde_json::from_slice::<EmbedResponse>(&bytes) {
    Ok(r) => return Ok(r),
    Err(e) if matches!(e.classify(), Category::Eof | Category::Io) => {
        last_err = "embedding response body was truncated".into();
        continue;
    }
    Err(e) => return Err(/* schema / JSON as now */),
}

Please add a test for this case too, e.g. a Reply variant with no Content-Length header.

Minor / optional

• last_err = "embedding request failed" drops the error kind entirely. Some operator signal would help without leaking the URL, e.g. e.is_connect() → "connect failed", e.is_request(), e.is_body(), or e.without_url().to_string().
• A 4xx body is no longer included anywhere. Not echoing it to the API caller is right, but a truncated tracing::debug!/warn! of the body or status would still help diagnose problems like 400 "input too long".
• The issue asks to validate the response contract. embed_batch still doesn't check data.len() == texts.len(), and neither path checks embedding.len() == self.dimension. A short or misordered batch would be persisted silently. A follow-up is fine.
• The worst case is 3 × 30 s request timeout plus backoff, about 90 s. That's acceptable, but worth noting against the "total time budget" in the issue.

The part of #275 about the precommit outcome contract and idempotency stays out of scope, and the PR description already says so with Related:. 👍

[loveRhythm1990]

Re-review of d341891

This addresses my main concern. Category::Eof is now retried within the same attempt budget, and the new fixtures cover close-delimited, chunked (both clean terminal chunk and short chunk), and fully-received-{ truncation. Syntax and schema errors still fail on the first attempt. cargo test -p memoria-embedding --lib passes on the new head (37 tests), and clippy with -D warnings is clean.

What I checked in the new code:

• Connection error classes (is_timeout / is_connect / is_body): this gives operators a useful signal without the URL, and there's a test that checks neither the URL nor the key leaks. 👍
• validate_response:
  • The dimension is checked against the same cfg.embedding_dim that SqlMemoryStore::connect uses for the vector column. A mismatch would have failed at insert anyway, so failing at the embedding step is strictly earlier and clearer.
  • Rejecting a wrong count, including a single-text response with extra items, is correct.
  • If a provider sends index, the data is sorted by it and must cover exactly 0..n. Duplicate, missing, or mixed indexes are rejected. If no item has an index, the response order is kept, so existing providers still work.
• RoundRobinEmbedder compatibility: is_non_retryable still matches on "HTTP 4" and the new message is HTTP {status}, so failover behavior is unchanged. Validation and JSON errors rotate to the next backend, which is the right call for a backend-specific bad response.

Nit: serde_json::from_slice never produces Category::Io, so that arm is unreachable. It's harmless; keep it if you'd like it to be future-proof.

LGTM. The precommit-outcome/idempotency part of #275 is still tracked separately via Related:.