Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 51 additions & 4 deletions src/conclave/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,46 @@ def _read_yaml(path: Path) -> dict[str, Any]:
return {}


def _resolve_path(path: Path | None) -> Path:
"""Resolve the effective config path from an explicit arg or the environment."""
if path is not None:
return path
env_path = os.environ.get("CONCLAVE_CONFIG")
return Path(env_path) if env_path else DEFAULT_CONFIG_PATH


def _cache_key(path: Path) -> tuple[str, float]:
"""A (path, mtime) cache key. mtime is 0.0 when the file is absent.

Keying on mtime means an edited or newly created config is picked up on the
next call, while an unchanged file is served from the in-process cache --
eliminating the repeated disk read + YAML parse that ``call_model`` used to
incur on every single model call (issue #15), without ever serving stale data.
"""
try:
return (str(path), path.stat().st_mtime)
except OSError:
return (str(path), 0.0)


# In-process memo: (path, mtime) -> merged config. Cleared automatically when the
# underlying file changes (mtime moves) and explicitly via ``clear_config_cache``.
_CONFIG_CACHE: dict[tuple[str, float], ConclaveConfig] = {}


def clear_config_cache() -> None:
"""Drop the memoized config. Intended for tests and long-lived processes."""
_CONFIG_CACHE.clear()


def load_config(path: Path | None = None) -> ConclaveConfig:
"""Load and merge configuration.
"""Load and merge configuration, memoized by (path, mtime).

The result is cached in-process keyed on the resolved path and its
modification time, so repeated calls within a run (e.g. one per model call)
do not re-read disk or re-parse YAML. The cache self-invalidates when the
file's mtime changes, preserving the previous always-fresh behavior across
edits while removing the redundant hot-path reads (issue #15).

Args:
path: Optional override path. Defaults to ``~/.conclave/config.yml`` or
Expand All @@ -115,10 +153,19 @@ def load_config(path: Path | None = None) -> ConclaveConfig:
A fully merged ``ConclaveConfig``. Built-in model defaults are always
present; file entries override or extend them.
"""
if path is None:
env_path = os.environ.get("CONCLAVE_CONFIG")
path = Path(env_path) if env_path else DEFAULT_CONFIG_PATH
resolved = _resolve_path(path)
key = _cache_key(resolved)
cached = _CONFIG_CACHE.get(key)
if cached is not None:
return cached

config = _load_config_uncached(resolved)
_CONFIG_CACHE[key] = config
return config


def _load_config_uncached(path: Path) -> ConclaveConfig:
"""Read + merge config from ``path`` with no caching (the real disk work)."""
raw = _read_yaml(path)

merged_models = dict(DEFAULT_MODELS)
Expand Down
14 changes: 11 additions & 3 deletions src/conclave/providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ async def call_model(
*,
temperature: float = 0.7,
timeout: float = 120.0,
config: ConclaveConfig | None = None,
) -> ModelAnswer:
"""Call a single model and return a structured :class:`ModelAnswer`.

Expand All @@ -62,17 +63,24 @@ async def call_model(
messages: OpenAI-style message list.
temperature: Sampling temperature.
timeout: Per-call timeout in seconds.
config: Pre-resolved config to use for adapter resolution (custom
``endpoints:``). A caller that already holds the config -- e.g.
``Council`` -- threads it in so this hot path does not re-read it.
When ``None`` (a standalone call) the config is resolved via the
memoized :func:`conclave.config.load_config`, so even repeated
standalone calls avoid redundant disk reads (issue #15).

Returns:
A ``ModelAnswer`` with either ``answer`` populated or ``error`` set.
"""
started = time.perf_counter()

# Resolve the adapter (config-aware for user-declared custom endpoints). A
# bad/unknown provider id surfaces as a clean, non-raising error.
# bad/unknown provider id surfaces as a clean, non-raising error. Prefer the
# injected config; fall back to the memoized loader only when called standalone.
try:
config: ConclaveConfig = load_config()
adapter = resolve_adapter(model_id, config)
resolved_config = config if config is not None else load_config()
adapter = resolve_adapter(model_id, resolved_config)
except ProviderError as exc:
latency = time.perf_counter() - started
logger.warning("%s (%s) unresolved: %s", name, model_id, exc)
Expand Down
165 changes: 152 additions & 13 deletions src/conclave/registry.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,69 @@
"""Provider registry: friendly names, LiteLLM model ids, and key presence.

This module is the single source of truth for which environment variable a
given provider needs. It NEVER reads or returns a key value -- only whether the
relevant variable is set and non-empty. That keeps secrets out of every code
path and out of any serialized output.
"""Provider registry: the single source of truth for built-in provider metadata.

This module owns, in ONE place, everything that defines a built-in provider:

* its LiteLLM provider prefix,
* the env var(s) that satisfy its key,
* (for OpenAI-compatible providers) its full ``/chat/completions`` URL,
* the friendly-name -> default model id mapping.

Previously the env-var names lived here while the OpenAI-compatible URLs lived in
``adapters.openai_compat.OPENAI_COMPAT_URLS``. The two tables could silently
drift: a URL present without a matching env var (or vice versa) surfaced only as a
runtime ``KeyError`` deep in the call path. Now the per-provider metadata is
declared once (see :data:`OPENAI_COMPAT_PROVIDERS` and :data:`NATIVE_PROVIDERS`)
and the derived tables (:data:`PROVIDER_ENV_VARS`) are built from it, so adding or
removing a provider can never desync. A module-level consistency check
(:func:`_assert_metadata_consistent`) runs at import and fails loudly if the
adapter layer's URL table ever drifts from this source of truth -- turning a
silent per-call KeyError into an immediate, diagnosable startup error.

It NEVER reads or returns a key value -- only whether the relevant variable is
set and non-empty. That keeps secrets out of every code path and out of any
serialized output.

### Adding a built-in provider (single edit step)

* **OpenAI-compatible provider** -> add one entry to
:data:`OPENAI_COMPAT_PROVIDERS` (prefix -> URL + env vars) **and** the matching
URL to ``adapters.openai_compat.OPENAI_COMPAT_URLS``. The import-time check
guarantees you cannot forget one half.
* **Native (non-compatible) provider** -> add its env var(s) to
:data:`NATIVE_PROVIDERS` and register its adapter in
``adapters._NATIVE_BUILDERS``.
* **Zero-code custom provider** -> declare it under ``endpoints:`` in
``~/.conclave/config.yml``; no registry edit needed.
"""

from __future__ import annotations

import os
from dataclasses import dataclass


class RegistryError(RuntimeError):
"""Raised at import time when provider metadata is internally inconsistent.

A loud, immediate failure here replaces the former silent drift between the
env-var table and the OpenAI-compatible URL table, which used to escape as a
runtime ``KeyError`` deep inside the per-call adapter resolution.
"""


@dataclass(frozen=True)
class OpenAICompatProvider:
"""Authoritative metadata for one built-in OpenAI-compatible provider.

Attributes:
completions_url: Full POST URL of the provider's ``/chat/completions``
endpoint.
env_vars: Candidate env var NAMES (never values); the first present one is
the active key. Order matters for fallbacks.
"""

completions_url: str
env_vars: tuple[str, ...]


# Friendly name -> default LiteLLM model id. Overridable via ~/.conclave/config.yml.
DEFAULT_MODELS: dict[str, str] = {
Expand All @@ -19,19 +74,96 @@
"openai": "openai/gpt-4.1",
}

# LiteLLM provider prefix -> the env var(s) that satisfy it. The first present
# var in the list is considered the active key. Order matters for fallbacks.
# SINGLE SOURCE OF TRUTH for built-in OpenAI-compatible providers: prefix ->
# (URL + env vars) in one place. The adapter layer's OPENAI_COMPAT_URLS is kept in
# lockstep with this table by the import-time consistency check below.
OPENAI_COMPAT_PROVIDERS: dict[str, OpenAICompatProvider] = {
"openai": OpenAICompatProvider(
completions_url="https://api.openai.com/v1/chat/completions",
env_vars=("OPENAI_API_KEY",),
),
"xai": OpenAICompatProvider(
completions_url="https://api.x.ai/v1/chat/completions",
env_vars=("XAI_API_KEY",),
),
# Perplexity has NO /v1 segment; that detail lives here, once.
"perplexity": OpenAICompatProvider(
completions_url="https://api.perplexity.ai/chat/completions",
env_vars=("PERPLEXITY_API_KEY",),
),
}

# Native (non OpenAI-compatible) providers: prefix -> candidate env var names.
# These have bespoke adapters in adapters._NATIVE_BUILDERS rather than a URL here.
NATIVE_PROVIDERS: dict[str, tuple[str, ...]] = {
"gemini": ("GEMINI_API_KEY", "GOOGLE_API_KEY"),
"anthropic": ("ANTHROPIC_API_KEY",),
}

# Derived: LiteLLM provider prefix -> the env var(s) that satisfy it. Built from
# the single-source tables above so it can never drift from them. The first
# present var in the list is the active key. Order matters for fallbacks.
PROVIDER_ENV_VARS: dict[str, list[str]] = {
"xai": ["XAI_API_KEY"],
"gemini": ["GEMINI_API_KEY", "GOOGLE_API_KEY"],
"anthropic": ["ANTHROPIC_API_KEY"],
"perplexity": ["PERPLEXITY_API_KEY"],
"openai": ["OPENAI_API_KEY"],
**{prefix: list(meta.env_vars) for prefix, meta in OPENAI_COMPAT_PROVIDERS.items()},
**{prefix: list(env_vars) for prefix, env_vars in NATIVE_PROVIDERS.items()},
}

DEFAULT_SYNTHESIZER = "claude"


def _assert_metadata_consistent() -> None:
"""Fail loudly at import if provider metadata has drifted.

Two invariants are enforced:

1. The adapter layer's ``OPENAI_COMPAT_URLS`` must have exactly the same
prefixes as :data:`OPENAI_COMPAT_PROVIDERS`, with identical URLs. This is
the drift that used to escape as a runtime ``KeyError`` in
``adapters._openai_compat_adapter`` (issue #19).
2. Every prefix in :data:`OPENAI_COMPAT_PROVIDERS` must declare at least one
env var, so an OpenAI-compatible provider can never be half-declared.

Raises:
RegistryError: with a message naming the drifting prefixes / URLs.
"""
# Imported lazily inside the function to avoid any import-order coupling:
# openai_compat does not import registry, so this edge is safe and acyclic.
from .adapters.openai_compat import OPENAI_COMPAT_URLS

source_prefixes = set(OPENAI_COMPAT_PROVIDERS)
adapter_prefixes = set(OPENAI_COMPAT_URLS)

missing_in_adapter = source_prefixes - adapter_prefixes
missing_in_source = adapter_prefixes - source_prefixes
if missing_in_adapter or missing_in_source:
raise RegistryError(
"OpenAI-compatible provider tables have drifted: "
f"prefixes only in registry.OPENAI_COMPAT_PROVIDERS={sorted(missing_in_adapter)}, "
f"prefixes only in adapters.OPENAI_COMPAT_URLS={sorted(missing_in_source)}. "
"Add the missing entry to both (registry is the source of truth)."
)

mismatched_urls = {
prefix: (OPENAI_COMPAT_PROVIDERS[prefix].completions_url, OPENAI_COMPAT_URLS[prefix])
for prefix in source_prefixes
if OPENAI_COMPAT_PROVIDERS[prefix].completions_url != OPENAI_COMPAT_URLS[prefix]
}
if mismatched_urls:
raise RegistryError(
"OpenAI-compatible URL drift between registry and adapters: "
f"{mismatched_urls} (registry is the source of truth)."
)

half_declared = [
prefix for prefix, meta in OPENAI_COMPAT_PROVIDERS.items() if not meta.env_vars
]
if half_declared:
raise RegistryError(
f"OpenAI-compatible providers declare a URL but no env var: {half_declared}. "
"Every provider needs at least one env var name."
)


def provider_prefix(model_id: str) -> str:
"""Extract the LiteLLM provider prefix from a model id.

Expand Down Expand Up @@ -75,3 +207,10 @@ def key_source(model_id: str) -> str | None:
if os.environ.get(var, "").strip():
return var
return None


# Enforce the single-source-of-truth invariant at import time. Any drift between
# this registry and the adapter layer's URL table is a programming error in a
# provider definition; surfacing it here (loudly, once) is strictly better than
# letting it escape as a per-call KeyError deep in the call path (issue #19).
_assert_metadata_consistent()
46 changes: 46 additions & 0 deletions tests/test_council.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,3 +222,49 @@ async def test_ask_sync_raises_inside_loop(monkeypatch):
council = Council(models=["grok"], config=_config())
with pytest.raises(RuntimeError, match="running event loop"):
council.ask_sync("hi")


async def test_config_disk_read_at_most_once_per_ask(monkeypatch, tmp_path):
"""A full Council.ask run hits the config file on disk at most once (issue #15).

Exercises the REAL call path (transport patched, not call_model) so every
member call plus synthesis flows through providers.call_model -> load_config.
With the memoized loader, the underlying disk read happens at most once across
the whole run rather than once per model call.
"""
import conclave.config as config_mod

_all_keys(monkeypatch)

# Synthesizer is openai so the OpenAI-shaped transport stub serves both the
# members and the synthesis call (all OpenAI-compatible).
config_file = tmp_path / "conclave.yml"
config_file.write_text("synthesizer: openai\n", encoding="utf-8")
monkeypatch.setenv("CONCLAVE_CONFIG", str(config_file))

config_mod.clear_config_cache()

reads = {"n": 0}
real_read_yaml = config_mod._read_yaml

def counting_read_yaml(path):
reads["n"] += 1
return real_read_yaml(path)

monkeypatch.setattr(config_mod, "_read_yaml", counting_read_yaml)

async def fake_post(url, headers, json_body, timeout):
return 200, {"choices": [{"message": {"content": "answer"}}]}

monkeypatch.setattr("conclave.transport.post_json", fake_post)

# Council built with no injected config -> resolves via load_config; every
# member + synthesis call then also calls load_config from providers.
council = Council(models=["grok", "perplexity", "openai"], synthesizer="openai")
result = await council.ask("what is 2+2?")

assert result.synthesis == "answer"
assert len(result.answers) == 3
assert reads["n"] <= 1, f"expected at most one disk read for the run, got {reads['n']}"

config_mod.clear_config_cache()
Loading
Loading