[Bug] OLLAMA_EMBEDDING_TIMEOUT is not validated; a malformed value silently downgrades to MockEmbedder

[austintraver]

Describe the Bug

The Ollama embed request timeout (added in #656) is read from OLLAMA_EMBEDDING_TIMEOUT / REPOWISE_EMBEDDING_TIMEOUT and passed straight through float(...) with no validation, in both the CLI mapper and the embedder constructor:

• packages/cli/src/repowise/cli/providers/embedders.py — _embedder_kwargs() does kwargs["timeout"] = float(timeout)
• packages/core/src/repowise/core/providers/embedding/ollama.py — __init__ does float(env_timeout) if env_timeout else _DEFAULT_TIMEOUT

Two failure modes result:

1. A non-numeric value raises, and the CLI path swallows it into a silent MockEmbedder. OLLAMA_EMBEDDING_TIMEOUT=abc makes float("abc") raise ValueError inside _embedder_kwargs(). build_embedder() catches it with except Exception: return MockEmbedder(), so semantic search silently runs on mock vectors with no error. This is the same silent-degradation class as [Bug] Configured embedder silently falls back to MockEmbedder on init failure — broken semantic search looks healthy #306, which fix(mcp): surface embedder init failure instead of silent mock fallback #324 fixed for the MCP-server path (via the _embedder_status envelope); but the CLI build_embedder() path was not covered and still swallows to mock.

2. Non-positive / non-finite values are accepted and passed to httpx. 0, -5, inf, nan all pass the bare float() and reach httpx.AsyncClient(timeout=...). A 0 or negative timeout makes every embed request fail immediately; inf/nan are nonsensical.

Steps to Reproduce

1. Configure the Ollama embedder: export OLLAMA_EMBEDDING_MODEL=embeddinggemma
2. Set a malformed timeout: export OLLAMA_EMBEDDING_TIMEOUT=abc (also try 0, -5, inf)
3. Run any flow that builds the embedder, e.g. repowise reindex, or directly:

   from repowise.cli.providers import build_embedder
   build_embedder("ollama")   # with abc -> returns MockEmbedder() silently

Expected Behavior

A malformed, non-positive, or non-finite timeout is reported rather than silently swallowed, and the value is validated before use. Ideally consistent with the direction taken in #324 (surface the degradation, e.g. via _embedder_status, rather than silently mock). The exact shape is a maintainer call.

Actual Behavior

• OLLAMA_EMBEDDING_TIMEOUT=abc -> build_embedder("ollama") returns MockEmbedder() with only a WARNING on the CLI path; semantic search silently disabled.
• OLLAMA_EMBEDDING_TIMEOUT=0 / -5 / inf -> passed unvalidated to httpx.AsyncClient(timeout=...).

Environment

• OS: macOS (this is environment-independent; a code-level validation gap)
• Python version: 3.14.3
• Repowise version: current main (after fix(mcp): defer answer-by-union to synthesis when the mention is incidental #823, v0.31.x)
• Installation method: from source (uv)

Additional Context

Related: #656 (added the configurable timeout), and #306 / #324 (the silent-MockEmbedder-on-init class, fixed for the MCP path only).

Happy to open a PR once the intended shape is decided, specifically whether a bad embedder config should surface through the _embedder_status degradation envelope (matching #324), warn, or clamp, and whether the CLI build_embedder() fallback should distinguish configuration errors from genuine provider-unavailability.

[RaghavChamadiya]

Thanks for the careful write-up, and for tracing it back to #656 and the #306/#324 lineage. Reproduced both paths on current main:

• _embedder_kwargs() does float(timeout) (and int(dimensions) right above it, line 24) with no validation, and build_embedder()'s blanket except Exception: return MockEmbedder() turns any parse error into a silent downgrade.
• OllamaEmbedder.__init__ accepts 0, -5, inf, nan straight into httpx.AsyncClient(timeout=...).

A couple of notes on scope and shape before you pick this up:

1. Fix the whole numeric-env class, not just timeout. The dimensions parsing (int(...) in both _embedder_kwargs and the Ollama constructor) has the identical unvalidated-then-swallowed problem. A small shared parse-and-validate helper (parse, require finite and > 0, raise a typed EmbedderConfigError instead of a bare ValueError) covers both and keeps the two call sites in sync.

2. The real root cause is build_embedder conflating config errors with provider-unavailability. The except Exception -> MockEmbedder is correct for "SDK missing / Ollama not running" but wrong for "user gave a bad value." We'd like this to go the fix(mcp): surface embedder init failure instead of silent mock fallback #324 direction: surface the degradation through _embedder_status so the CLI matches the MCP path, rather than the CLI silently mocking. So a malformed config should be distinguishable from a genuinely absent provider and reported, not swallowed.

3. Prefer surfacing/raising over clamping. Clamping abc/nan/-5 to a default would hide a real user mistake. A clear error (with the offending var name and value) is better than quietly running on mock vectors or a silently-corrected timeout.

So: validate at parse time with a typed error, thread config errors through the degradation envelope in build_embedder instead of the blanket mock fallback, and cover dimensions alongside timeout. If that shape works for you, a PR would be very welcome.