Make embedding server optional for vMCP optimizer

[meenacodes]

Summary

The vMCP optimizer's find_tool search only needs an embedding server for
semantic ranking — the underlying implementation already supports FTS5
keyword-only search with no embedder configured (see
pkg/vmcp/optimizer/internal/toolstore/sqlite_store.go and
pkg/vmcp/optimizer/internal/similarity/embedding_client.go, which already
treat an empty EmbeddingService as "semantic search disabled, keyword-only
still works"). However, the operator's VirtualMCPServer admission-time
validation contradicted this: validateEmbeddingServer() unconditionally
rejected any config with spec.config.optimizer set unless
spec.embeddingServerRef or spec.config.optimizer.embeddingService was
also set, even when the user only wanted keyword search. This made the
embedding server (and the extra TEI/OpenAI infrastructure it requires)
mandatory for a feature that was designed to work without it.

• Removed the hasOptimizer && !hasRef && !hasManualService error branch in
  validateEmbeddingServer() so spec.config.optimizer no longer requires an
  embedding source.
• Kept the existing, still-valid rules: embeddingServerRef.name must be
  non-empty when the ref is set, and setting embeddingServerRef without
  optimizer still auto-populates a default OptimizerConfig.
• Updated doc comments on EmbeddingServerRef and validateEmbeddingServer
  to state that omitting both fields now falls back to FTS5 keyword-only
  search instead of erroring.
• Updated TestValidateEmbeddingServer to assert success (with Optimizer
  populated) instead of an error for the no-embedding-source case.

Type of change

• Bug fix
• New feature
• Refactoring (no behavior change)
• Dependency update
• Documentation
• Other (describe):

Test plan

• Unit tests (task test)
• E2E tests (task test-e2e)
• Linting (task lint-fix)
• Manual testing (describe below)

Ran TestValidateEmbeddingServer directly — all 5 subtests pass, including
the updated optimizer_without_ref_or_service_succeeds_keyword_only case.
Full task operator-test suite run in progress at time of writing;
confirm it's green before merging.

API Compatibility

• This PR does not break the v1beta1 API, OR the api-break-allowed label is applied and the migration guidance is described above.

This only relaxes a Go-level reconcile-time validation rule (not a CEL rule
or schema marker) — no CRD field types, names, or schemas changed. Configs
that were previously rejected are now accepted; no previously-accepted
config is now rejected.

Changes

File	Change
cmd/thv-operator/api/v1beta1/virtualmcpserver_types.go	Remove mandatory embedding-source check in validateEmbeddingServer(); update doc comments
cmd/thv-operator/api/v1beta1/virtualmcpserver_types_test.go	Update test case to expect success instead of an error

Does this introduce a user-facing change?

Yes. VirtualMCPServer resources can now set spec.config.optimizer without
also setting spec.embeddingServerRef or
spec.config.optimizer.embeddingService. In that case, find_tool performs
FTS5 keyword-only search with no semantic ranking, instead of the operator
rejecting the resource at admission.

Special notes for reviewers

The openai embedding provider still requires embeddingService and
embeddingModel — that constraint is enforced separately, at optimizer
startup, in pkg/vmcp/optimizer/optimizer.go's resolveEmbeddingProvider,
and is unrelated to this fix (there's no default OpenAI-compatible endpoint
to fall back to). No CRD manifest regeneration (task operator-generate /
operator-manifests) was needed since this is a Go validation change, not a
kubebuilder marker or CEL rule change.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.42%. Comparing base (7530d35) to head (f766d6c).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6106      +/-   ##
==========================================
+ Coverage   72.41%   72.42%   +0.01%     
==========================================
  Files         734      734              
  Lines       75822    75817       -5     
==========================================
+ Hits        54903    54908       +5     
+ Misses      17019    16994      -25     
- Partials     3900     3915      +15     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[aponcedeleonch]

Verified the underlying premise end to end and it holds up: the runtime really does degrade to FTS5 keyword-only when there's no embedding source. similarity/embedding_client.go:17-19 returns (nil, nil) on an empty EmbeddingService, toolstore/sqlite_store.go:192-203 has an explicit FTS5-only branch that returns before any hybrid code runs, and the CLI already treats this as a first-class Tier 1 mode (pkg/vmcp/cli/serve.go:486-511). Nothing nil-panics, and the reconcile path is safe too: populateOptimizerEmbeddingService falls through to a no-op and readiness isn't blocked, since the requeue is gated on EmbeddingServerRef != nil. So aligning the operator with behavior that already shipped is the right call.

Signaling the degraded mode

Anchoring this here since the file isn't in the diff: cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig.go:160, the no-ref branch.

Now that this config is legal, that branch is the only place that knows the user is about to run in keyword-only mode, and it says nothing.

Everything that exists today is slog.Debug: optimizer.go:256 (semantic_search_enabled), sqlite_store.go:113, and sqlite_store.go:202. There's no Event, and no Condition either, because SetEmbeddingServerReadyCondition is only reached when EmbeddingServerRef != nil (virtualmcpserver_controller.go:1057-1074). At default log level a user gets no indication anywhere that semantic ranking is off: not in pod logs, not in kubectl describe, not in .status. "Optimizer running keyword-only" and "optimizer not enabled" are indistinguishable.

That's a rough failure mode, because the only symptom is worse find_tool results, which people tend to misdiagnose for a while before suspecting config.

Two suggestions, following the idiom already in that function:

An Info log plus a Normal event, shaped like the EmbeddingServiceManual warning just below it:

if config.Optimizer != nil && config.Optimizer.EmbeddingService == "" {
    ctxLogger.Info("optimizer configured without an embedding source; " +
        "find_tool will use FTS5 keyword-only search with no semantic ranking")
    if r.Recorder != nil {
        r.Recorder.Eventf(vmcp, nil, corev1.EventTypeNormal,
            "OptimizerKeywordOnly", "ValidateEmbeddingService",
            "No embeddingServerRef or optimizer.embeddingService set; find_tool will use "+
                "keyword-only search. Set embeddingServerRef to enable semantic ranking.")
    }
}

And promoting the runtime line at optimizer.go:256 from Debug to Info when embClient == nil, since the operator event does nothing for anyone running vMCP through the CLI.

On the Condition: I'd deliberately skip it. Setting EmbeddingServerReady=False would report an intentional configuration as a failure, and the existing False path is paired with Phase=Pending and a requeue, which isn't right for a mode that's working as designed. Event plus log feels like the better fit, but happy to be argued out of it if you'd rather it show up in .status.

The find_tool description should reflect the mode

Also not in the diff: pkg/vmcp/session/optimizerdec/decorator.go:73.

That description is unconditional: "This searches available MCP server tools using semantic and keyword-based matching." Once keyword-only becomes a reachable default, that's promising the model semantic ranking that may not be there. The description is part of the contract the LLM plans against, so it should reflect what the deployment can actually do.

Rather than watering the wording down for everyone, I think it's worth teaching the decorator which mode it's in and picking between two strings. Semantic ranking is a real capability difference, and a model that knows it's doing lexical matching can compensate by phrasing queries with terms likely to appear in tool metadata.

The constraint is that OptimizerTools() is shared: its doc comment says this decorator and the Serve path (serve_optimizer.go:81) have to advertise an identical pair, so whatever signal we add needs to reach both or the two paths will disagree about what find_tool claims.

Adding the bit to the Optimizer interface handles that cleanly. Both call sites already hold an opt at the point they build the definitions, so it needs no constructor threading:

type Optimizer interface {
    FindTool(...)
    CallTool(...)
    // SemanticSearchEnabled reports whether an embedding client was configured.
    // When false, find_tool ranks by FTS5 keyword match only.
    SemanticSearchEnabled() bool
}

func OptimizerTools(semanticEnabled bool) []vmcp.Tool { ... }

The predicate should be the same one NewEmbeddingClient uses (embedding_client.go:17), so the advertised description can't claim semantic matching in a build where the client came back nil.

[aponcedeleonch]

gofmt rejects this. Go 1.19+ gofmt reformats doc comments, and a column-0 // - bullet with an indented continuation line isn't recognized as a list, so it gets rewritten to the canonical // - form:

//   - embeddingServerRef.name must be non-empty when ref is provided
//   - optimizer does not require an embedding source: with neither embeddingServerRef
//     nor optimizer.embeddingService set, find_tool runs FTS5 keyword-only search
//   - if embeddingServerRef is set without optimizer, auto-populate optimizer with defaults

gofmt -l flags the file as it stands, so linting will fail. task lint-fix sorts it.

[aponcedeleonch]

Worth thinking about what happens to embeddingProvider and embeddingModel once this check is gone.

With no embedding source those two fields stop meaning anything, so I agree there's no reason to reject the spec over them. What we should do is tell the user they're inert, so nobody sets embeddingProvider: openai, sees keyword-only results, and spends an afternoon wondering why their gateway is never called.

One wrinkle to sort out first: today they aren't actually ignored. resolveEmbeddingProvider (pkg/vmcp/optimizer/optimizer.go:113-117) hard-errors for the openai provider when embeddingService is empty, so this spec doesn't degrade, it CrashLoopBackOffs:

config:
  optimizer:
    embeddingProvider: openai
    embeddingModel: text-embedding-3-small
    # no embeddingService, no embeddingServerRef

The CEL rule at line 22 of this file doesn't catch it, since that one only blocks embeddingServerRef combined with openai and there's no ref here. Before this PR Validate() caught it with a clear message. Now it reaches the pod and dies at startup with the reason only in pod logs, no condition or event pointing at why.

So "these fields are ignored" needs to become true before we can warn that it's true. That means short-circuiting on the absence of an embedding source before the provider switch:

func resolveEmbeddingProvider(optCfg *Config) error {
    if optCfg.EmbeddingService == "" {
        // No embedding source: semantic search is disabled entirely, so the
        // provider and model are inert. Skip provider validation rather than
        // failing startup over fields that will never be read.
        return nil
    }
    ...
}

With that in place the behavior matches the mental model (no embedding source means keyword-only, full stop, regardless of provider) and the warning is honest. Pair it with a line alongside the keyword-only event in the controller:

if config.Optimizer.EmbeddingProvider != "" || config.Optimizer.EmbeddingModel != "" {
    ctxLogger.Info("optimizer.embeddingProvider and optimizer.embeddingModel are ignored "+
        "when no embedding source is configured; set embeddingServerRef or "+
        "optimizer.embeddingService to enable semantic ranking",
        "embeddingProvider", config.Optimizer.EmbeddingProvider,
        "embeddingModel", config.Optimizer.EmbeddingModel)
}

Note embeddingProvider defaults to tei via kubebuilder, so on the operator path that check fires for essentially everyone in keyword-only mode. Probably worth folding the "provider and model ignored" sentence into the single keyword-only event rather than emitting a second one.

[aponcedeleonch]

This asserts the check is gone, which is right, but nothing yet asserts the newly-legal config produces a working deployment.

The keyword-only search path is well covered at the store layer (TestSQLiteToolStore_Search has 9 FTS5-only cases, plus _ResultsCapped, _Close, _Concurrent), but all of that predates this PR. The gaps are at the seams this PR opens:

1. NewOptimizerFactory has no tests at all. Zero references to it in any test file. Coverage stops at NewSQLiteToolStore and NewEmbeddingClient separately, so nothing asserts that NewOptimizerFactory(&Config{}) with no service succeeds and yields a store that returns FTS5 results. That's precisely the composition this PR makes reachable from the operator.

2. No operator-side test for populateOptimizerEmbeddingService in the ref-nil, svc-empty case. Worth asserting the ConfigMap still carries a non-nil config.Optimizer with an empty embeddingService, so a later refactor can't quietly drop the optimizer block. One trap: TestOptimizerEmbeddingServiceURL (controllers/virtualmcpserver_vmcpconfig_test.go:2149) looks like the natural home and already accepts esName == "", but its assertion block is wrapped in if tt.expectedURL != "" at line 2262, so a new case with an empty expectedURL would pass while asserting nothing.

3. No envtest case for spec.config.optimizer: {} with no ref being admitted. cmd/thv-operator/test-integration/virtualmcp/virtualmcpserver_embedding_cel_test.go is the place, and it's cheap to add next to the existing provider cases.

4. If resolveEmbeddingProvider gains the empty-service short-circuit, it wants a case asserting embeddingProvider: openai with no service returns cleanly instead of erroring. That's the regression pin for the whole "provider is inert without a source" contract.

Items 1 and 2 feel like they belong in this PR. 3 and 4 could be follow-ups, though 4 should land with whatever change introduces it.