mirror of
https://github.com/vectorize-io/hindsight.git
synced 2026-09-14 19:31:49 +08:00
Nicolò Boschi
11e624325b
* refactor(config): make HindsightConfig the only parser of HINDSIGHT_API_* env vars Thirty-odd call sites across the engine read HINDSIGHT_API_* out of os.environ themselves rather than off the resolved config. Two parsers for one variable is how the engine and the config drift apart: LLMProvider.from_env() had grown its own copies of the provider defaulting, the Gemini tier gating and the cache-affinity default, each carrying a comment asking the next reader not to let them disagree. Those comments are now unnecessary. Every fixed, server-level HINDSIGHT_API_* value is parsed in config.py and read as a field. Seventeen variables that worked but had no field got one, including the seven xai-oauth knobs; the five that carry secrets are registered in _CREDENTIAL_FIELDS so they stay off the API surface. Three fields become `str | None` — host, otel_service_name, xai_oauth_base_url. Each had a caller that needed to tell "the operator set this" from "this is the default" and was reading the environment a second time to find out. The default is now applied at the single point of use. requires_api_key moves to a new leaf module, engine/provider_auth.py. config.py needs it while building HindsightConfig and llm_wrapper needs the built config at import time to size its semaphores; that cycle is the reason the LLM factory had its own env parser to begin with. Both existing import paths still work. Two consequences worth knowing: * LLMProvider.from_env() now builds the full config, so an unrelated invalid setting surfaces there instead of being bypassed. The test that asserted the opposite asserts the new contract instead. * resolve_daemon_host_port() takes configured_host from its caller rather than reading HINDSIGHT_API_HOST itself. Value vocabularies are preserved exactly where they differed from _parse_boolean_env — ACCESS_LOG still accepts yes/on, XAI_OAUTH_DEBUG_HEADERS still never raises — so no working deployment turns into a start-up error. A new test walks the package AST and fails on any HINDSIGHT_API_* read outside config.py, with a short exemption list (standalone Alembic, pre-config bootstraps, the open-ended per-extension config namespaces) and a second test that fails when an exemption goes stale. tests/conftest.py resets the config cache per test: now that values are read off a cached config, a test's monkeypatch.setenv would otherwise land against whichever config the first test in that xdist worker happened to build. * fix(config): restore the DEFAULT_HOST import and keep .env authoritative Two defects from the previous commit, both caught by CI's server start rather than the suite. DEFAULT_HOST was dropped from main.py's imports during a rebase while `config.host or DEFAULT_HOST` stayed, so every entry point died with a NameError. No test caught it: each one hands _parse_cli_args a config whose host is already a string, so the fallback branch never evaluated. The new TestParseCliArgsHostDefault covers the unset-host path, and --help no longer advertises "default: None". The second is worse. HindsightConfig is cached process-wide on first build, and an entry point imports its whole module graph before main() reaches load_dotenv_for_entrypoint(). Modules reading the config at import scope (llm_wrapper sizes its semaphores there) therefore froze a config built before the .env was applied, and it stayed frozen — a discovered .env silently ignored, surfacing as "LLM API key is required" on a server that had always started. load_dotenv_for_entrypoint() now clears the cache after loading, and daemon.py's log path and poller.py's backpressure value resolve per call instead of at import.