Skip to content

perf(skills): use embed_batch in embed_skills_with_timeout#6532

Merged
bug-ops merged 1 commit into
mainfrom
fix/6481-embed-batch-skills
Jul 20, 2026
Merged

perf(skills): use embed_batch in embed_skills_with_timeout#6532
bug-ops merged 1 commit into
mainfrom
fix/6481-embed-batch-skills

Conversation

@bug-ops

@bug-ops bug-ops commented Jul 20, 2026

Copy link
Copy Markdown
Owner

Summary

  • embed_skills_with_timeout (crates/zeph-skills/src/embedding.rs) now tries LlmProvider::embed_batch first, wrapped in a single timeout for the whole batch, instead of embedding each skill sequentially — providers with a native batch endpoint (e.g. OpenAI) now cost one round trip instead of N.
  • Falls back to the original per-item sequential loop (extracted unchanged into embed_skills_sequential) on batch error, timeout, or a mismatched result count, preserving the existing partial-failure tolerance (one bad or slow skill no longer loses the rest).
  • Both callers (SkillMiner, trace-based skill extraction) are unaffected — the public function signature is unchanged.

Adversarial critique confirmed the result set is byte-identical to the original in every traced case; two minor, non-blocking notes worth surfacing:

  • Worst-case latency on the timeout→fallback path is now (N+1)×timeout (fresh budget for the fallback) instead of N×timeout.
  • A flaky provider could be double-charged on the failure-path re-embedding (batch attempt + fallback retry).

Note: found a pre-existing, unrelated rustdoc broken-intra-doc-link issue in crates/zeph-skills/src/matcher.rs ([Self::Qdrant] referencing a variant absent under some feature combinations), confirmed to exist on unmodified main via git stash. Out of scope here; filing as a separate follow-up issue.

Closes #6481

Test plan

  • cargo nextest run -p zeph-skills — 573/573 passed (5 new tests, including a regression test proving partial-failure tolerance survives the batch→fallback transition)
  • cargo +nightly fmt --check clean
  • cargo clippy --profile ci --workspace --all-targets --features "desktop,ide,server,chat,pdf,scheduler,testing" -- -D warnings clean
  • cargo nextest run --config-file .github/nextest.toml --workspace --features "desktop,ide,server,chat,pdf,scheduler" --lib --bins — 14805/14805 passed
  • RUSTFLAGS="-D warnings" RUSTDOCFLAGS="--deny rustdoc::broken_intra_doc_links" cargo doc --no-deps --workspace --features "desktop,ide,server,chat,pdf,scheduler" clean
  • gitleaks protect --staged --no-banner --redact — no leaks

@github-actions github-actions Bot added performance Performance improvements documentation Improvements or additions to documentation skills zeph-skills crate rust Rust code changes size/L Large PR (201-500 lines) and removed performance Performance improvements labels Jul 20, 2026
embed_skills_with_timeout looped over skills one at a time, paying N
sequential network round trips even against providers with a native
batch embeddings endpoint. It now tries LlmProvider::embed_batch first
under a single timeout for the whole batch, falling back to the
original per-item sequential loop on batch error, timeout, or a
mismatched result count, so the existing partial-failure tolerance is
preserved.

Closes #6481
@bug-ops
bug-ops enabled auto-merge (squash) July 20, 2026 00:59
@bug-ops
bug-ops force-pushed the fix/6481-embed-batch-skills branch from 4560edd to 6df9ef7 Compare July 20, 2026 00:59
@github-actions github-actions Bot added the performance Performance improvements label Jul 20, 2026
@bug-ops
bug-ops merged commit 2362ca9 into main Jul 20, 2026
121 of 128 checks passed
@bug-ops
bug-ops deleted the fix/6481-embed-batch-skills branch July 20, 2026 01:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation performance Performance improvements rust Rust code changes size/L Large PR (201-500 lines) skills zeph-skills crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

zeph-skills embed_skills_with_timeout reimplements sequential per-item embedding instead of using LlmProvider::embed_batch

1 participant