Move configuration to a YAML file, and make the tuning knobs configurable
CI / checks (push) Skipped
CI / checks (pull_request) Successful in 51s
CI / checks (push) Skipped
CI / checks (pull_request) Successful in 51s
The thresholds Part H says to tune on the real inference hardware were hardcoded, so every adjustment would have been a PR. They now live in a `tuning:` section of a YAML config file, which also replaces flat environment variables as the home for structure (#32). - The file (NISSE_CONFIG, default ./nisse-dev.yaml) carries structure and holds no secrets: credentials are ${VAR} references resolved from the environment, and a reference to an unset variable is an error - The environment still wins: every existing NISSE_* variable overrides its key, and the file is optional, so .env and CI work unchanged - Validation stays fail-fast and reports every problem at once; unknown keys are errors, so a misspelt knob fails at startup instead of silently doing nothing - The config file joins the state store and .env in the check that refuses to start when they sit inside the workspace clone - Tuning covers the executor's turn and failure limits, the consult budget, the delegation caps, the command timeout and the tool output limit, which Workspace now takes as a setting; the defaults are the values the agent already ran with - llm.max_output_tokens reaches both LLM clients from the config docs/Tuning-Guide.md explains the knobs for someone learning local model tuning: how weights, quantization and the KV cache spend VRAM, what each knob does and what goes wrong at either extreme, and an order to tune in. nisse-dev.example.yaml documents every key and is checked by a test so it cannot drift. Closes #32 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
4a9354ae3b
commit
df449c7e34
12 files changed
+994
-67
No files matched your search
@@ -1,49 +1,234 @@
|
||||
"""Agent configuration (spec A5: from env/.env, nothing hardcoded).
|
||||
"""Agent configuration (spec A5: nothing hardcoded).
|
||||
|
||||
Configuration comes from a YAML file plus the environment:
|
||||
|
||||
- **The file carries structure.** Roles, connections, and the tuning knobs
|
||||
(see docs/Tuning-Guide.md). It holds no secrets: a credential is written as
|
||||
a reference, ``token: ${NISSE_LLM_TOKEN}``, and resolved from the
|
||||
environment, so the file can be reviewed or shared without leaking.
|
||||
- **The environment wins.** Every setting that existed as an environment
|
||||
variable still does, and overrides the file. The file is optional, so a
|
||||
deployment or CI run configured purely from the environment works as before.
|
||||
|
||||
Every value is collected and validated **before** anything is built, and a
|
||||
single error names every missing variable at once. The alternative — failing
|
||||
at first use — surfaces a missing token twenty minutes into a milestone, on
|
||||
the call that mattered.
|
||||
single error names every problem at once. The alternative — failing at first
|
||||
use — surfaces a missing token twenty minutes into a milestone, on the call
|
||||
that mattered. Unknown keys are errors too: a misspelt tuning knob that was
|
||||
silently ignored would look exactly like a knob that has no effect.
|
||||
"""
|
||||
|
||||
import os
|
||||
import re
|
||||
from collections.abc import Mapping
|
||||
from dataclasses import dataclass
|
||||
from dataclasses import dataclass, field, fields
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
from nisse_dev.llm import RoleMap
|
||||
import yaml
|
||||
|
||||
from nisse_dev.llm import DEFAULT_MAX_OUTPUT_TOKENS, RoleMap
|
||||
from nisse_dev.matrix import MatrixConfig
|
||||
|
||||
REQUIRED = (
|
||||
"NISSE_STATE_DB",
|
||||
"NISSE_WORKSPACE",
|
||||
"NISSE_PROJECT",
|
||||
"NISSE_LLM_BASE_URL",
|
||||
"NISSE_LLM_TOKEN",
|
||||
"NISSE_ROLE_EXECUTOR",
|
||||
"NISSE_ROLE_PLANNER",
|
||||
"NISSE_ROLE_DELEGATE",
|
||||
"NISSE_ROLE_CONSULTANT",
|
||||
"NISSE_MATRIX_HOMESERVER",
|
||||
"NISSE_MATRIX_USER_ID",
|
||||
"NISSE_MATRIX_TOKEN",
|
||||
"NISSE_MATRIX_ROOM_ID",
|
||||
"NISSE_MATRIX_OPERATORS",
|
||||
"NISSE_GITEA_URL",
|
||||
"NISSE_GITEA_TOKEN",
|
||||
"NISSE_GITEA_REPO",
|
||||
"NISSE_GITEA_OPERATORS",
|
||||
DEFAULT_CONFIG_FILE = "nisse-dev.yaml"
|
||||
|
||||
# Each environment variable and the key it overrides. The names are the ones
|
||||
# the agent has always used, so existing .env files and CI keep working.
|
||||
ENV_KEYS: dict[str, str] = {
|
||||
"NISSE_STATE_DB": "state_db",
|
||||
"NISSE_WORKSPACE": "workspace",
|
||||
"NISSE_PROJECT": "project",
|
||||
"NISSE_LLM_BASE_URL": "llm.base_url",
|
||||
"NISSE_LLM_TOKEN": "llm.token",
|
||||
"NISSE_MAX_OUTPUT_TOKENS": "llm.max_output_tokens",
|
||||
"NISSE_ROLE_EXECUTOR": "roles.executor",
|
||||
"NISSE_ROLE_PLANNER": "roles.planner",
|
||||
"NISSE_ROLE_DELEGATE": "roles.delegate",
|
||||
"NISSE_ROLE_CONSULTANT": "roles.consultant",
|
||||
"NISSE_MATRIX_HOMESERVER": "matrix.homeserver",
|
||||
"NISSE_MATRIX_USER_ID": "matrix.user_id",
|
||||
"NISSE_MATRIX_TOKEN": "matrix.token",
|
||||
"NISSE_MATRIX_ROOM_ID": "matrix.room_id",
|
||||
"NISSE_MATRIX_OPERATORS": "matrix.operators",
|
||||
"NISSE_GITEA_URL": "gitea.url",
|
||||
"NISSE_GITEA_TOKEN": "gitea.token",
|
||||
"NISSE_GITEA_REPO": "gitea.repo",
|
||||
"NISSE_GITEA_OPERATORS": "gitea.operators",
|
||||
"NISSE_BASE_BRANCH": "gitea.base_branch",
|
||||
}
|
||||
KEY_ENV = {key: env for env, key in ENV_KEYS.items()}
|
||||
|
||||
REQUIRED_KEYS = (
|
||||
"state_db",
|
||||
"workspace",
|
||||
"project",
|
||||
"llm.base_url",
|
||||
"llm.token",
|
||||
"roles.executor",
|
||||
"roles.planner",
|
||||
"roles.delegate",
|
||||
"roles.consultant",
|
||||
"matrix.homeserver",
|
||||
"matrix.user_id",
|
||||
"matrix.token",
|
||||
"matrix.room_id",
|
||||
"matrix.operators",
|
||||
"gitea.url",
|
||||
"gitea.token",
|
||||
"gitea.repo",
|
||||
"gitea.operators",
|
||||
)
|
||||
OPTIONAL_KEYS = ("llm.max_output_tokens", "gitea.base_branch")
|
||||
# Kept for callers that list what a pure-environment deployment must set.
|
||||
REQUIRED = tuple(KEY_ENV[key] for key in REQUIRED_KEYS)
|
||||
|
||||
_REFERENCE = re.compile(r"\$\{([A-Za-z_][A-Za-z0-9_]*)\}")
|
||||
|
||||
|
||||
class ConfigError(RuntimeError):
|
||||
pass
|
||||
|
||||
|
||||
def _people(raw: str) -> frozenset[str]:
|
||||
"""Operator allowlists are comma-separated; blanks are dropped so a
|
||||
trailing comma cannot admit an empty identity."""
|
||||
return frozenset(part.strip() for part in raw.split(",") if part.strip())
|
||||
# --- tuning ----------------------------------------------------------------
|
||||
# The defaults are the values the agent ran with before they were
|
||||
# configurable, so an absent `tuning:` section changes nothing.
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class ExecutorTuning:
|
||||
max_turns: int = 24
|
||||
failure_limit: int = 2
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class ExpertTuning:
|
||||
consult_budget: int = 3
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class DelegationTuning:
|
||||
max_turns: int = 30
|
||||
max_tokens: int = 200_000
|
||||
wall_clock_s: float = 900.0
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class ToolsTuning:
|
||||
command_timeout_s: int = 120
|
||||
max_output_chars: int = 4000
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class Tuning:
|
||||
executor: ExecutorTuning = field(default_factory=ExecutorTuning)
|
||||
expert: ExpertTuning = field(default_factory=ExpertTuning)
|
||||
delegation: DelegationTuning = field(default_factory=DelegationTuning)
|
||||
tools: ToolsTuning = field(default_factory=ToolsTuning)
|
||||
|
||||
|
||||
_TUNING_SECTIONS: dict[str, type] = {
|
||||
f.name: f.default_factory # type: ignore[misc]
|
||||
for f in fields(Tuning)
|
||||
}
|
||||
|
||||
|
||||
def _parse_tuning(raw: Any, errors: list[str]) -> Tuning:
|
||||
if raw is None:
|
||||
return Tuning()
|
||||
if not isinstance(raw, dict):
|
||||
errors.append("tuning: must be a mapping")
|
||||
return Tuning()
|
||||
sections: dict[str, Any] = {}
|
||||
for name, section in raw.items():
|
||||
cls = _TUNING_SECTIONS.get(name)
|
||||
if cls is None:
|
||||
errors.append(f"tuning.{name}: unknown section")
|
||||
continue
|
||||
if not isinstance(section, dict):
|
||||
errors.append(f"tuning.{name}: must be a mapping")
|
||||
continue
|
||||
known = {f.name: f for f in fields(cls)}
|
||||
values: dict[str, Any] = {}
|
||||
for key, value in section.items():
|
||||
spec = known.get(key)
|
||||
if spec is None:
|
||||
errors.append(f"tuning.{name}.{key}: unknown setting")
|
||||
continue
|
||||
number = _positive(value, float if spec.type in (float, "float") else int)
|
||||
if number is None:
|
||||
errors.append(f"tuning.{name}.{key}: must be a positive number")
|
||||
continue
|
||||
values[key] = number
|
||||
sections[name] = cls(**values)
|
||||
return Tuning(**sections)
|
||||
|
||||
|
||||
def _positive(value: Any, kind: type) -> int | float | None:
|
||||
# bool is an int in Python; `true` for a turn limit is a mistake.
|
||||
if isinstance(value, bool) or not isinstance(value, int | float):
|
||||
return None
|
||||
if kind is int and not float(value).is_integer():
|
||||
return None
|
||||
if value <= 0:
|
||||
return None
|
||||
return kind(value)
|
||||
|
||||
|
||||
# --- loading ---------------------------------------------------------------
|
||||
|
||||
|
||||
def _people(raw: Any) -> frozenset[str]:
|
||||
"""Operator allowlists: a YAML list, or comma-separated in the
|
||||
environment. Blanks are dropped so a trailing comma cannot admit an empty
|
||||
identity."""
|
||||
parts = raw if isinstance(raw, list) else str(raw).split(",")
|
||||
return frozenset(str(part).strip() for part in parts if str(part).strip())
|
||||
|
||||
|
||||
def _flatten(tree: Mapping[str, Any], prefix: str = "") -> dict[str, Any]:
|
||||
flat: dict[str, Any] = {}
|
||||
for key, value in tree.items():
|
||||
dotted = f"{prefix}{key}"
|
||||
if isinstance(value, dict) and dotted != "tuning":
|
||||
flat.update(_flatten(value, f"{dotted}."))
|
||||
else:
|
||||
flat[dotted] = value
|
||||
return flat
|
||||
|
||||
|
||||
def _resolve(value: Any, key: str, env: Mapping[str, str], errors: list[str]) -> Any:
|
||||
"""Replace ${VAR} references with the environment's value. A reference to
|
||||
an unset variable is an error, never an empty string: an empty token would
|
||||
pass for a value and fail at the first request."""
|
||||
if not isinstance(value, str):
|
||||
return value
|
||||
|
||||
def lookup(match: re.Match[str]) -> str:
|
||||
name = match.group(1)
|
||||
found = (env.get(name) or "").strip()
|
||||
if not found:
|
||||
errors.append(f"{key}: references ${{{name}}}, which is unset or empty")
|
||||
return found
|
||||
|
||||
return _REFERENCE.sub(lookup, value)
|
||||
|
||||
|
||||
def _read_file(path: Path, errors: list[str]) -> dict[str, Any]:
|
||||
try:
|
||||
loaded = yaml.safe_load(path.read_text())
|
||||
except yaml.YAMLError as exc:
|
||||
errors.append(f"{path}: not valid YAML ({exc})")
|
||||
return {}
|
||||
if loaded is None:
|
||||
return {}
|
||||
if not isinstance(loaded, dict):
|
||||
errors.append(f"{path}: must be a mapping at the top level")
|
||||
return {}
|
||||
return loaded
|
||||
|
||||
|
||||
def _describe(key: str) -> str:
|
||||
env = KEY_ENV.get(key)
|
||||
return f"{key} ({env})" if env else key
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
@@ -61,30 +246,102 @@ class AgentConfig:
|
||||
gitea_repo: str
|
||||
gitea_operators: frozenset[str]
|
||||
base_branch: str = "main"
|
||||
max_output_tokens: int = DEFAULT_MAX_OUTPUT_TOKENS
|
||||
tuning: Tuning = field(default_factory=Tuning)
|
||||
config_file: Path | None = None
|
||||
|
||||
@classmethod
|
||||
def load(cls, env: Mapping[str, str] = os.environ) -> "AgentConfig":
|
||||
"""The file named by NISSE_CONFIG, or ./nisse-dev.yaml if present,
|
||||
overlaid by the environment. Naming a file that does not exist is an
|
||||
error; the default file is simply optional."""
|
||||
named = env.get("NISSE_CONFIG")
|
||||
path = Path(named or DEFAULT_CONFIG_FILE).expanduser()
|
||||
if named and not path.is_file():
|
||||
raise ConfigError(
|
||||
f"cannot start: NISSE_CONFIG names {path}, which is missing"
|
||||
)
|
||||
return cls.from_sources(env, path if path.is_file() else None)
|
||||
|
||||
@classmethod
|
||||
def from_env(cls, env: Mapping[str, str] = os.environ) -> "AgentConfig":
|
||||
"""Environment only, with no file."""
|
||||
return cls.from_sources(env, None)
|
||||
|
||||
@classmethod
|
||||
def from_sources(
|
||||
cls, env: Mapping[str, str], config_file: Path | None
|
||||
) -> "AgentConfig":
|
||||
errors: list[str] = []
|
||||
tree = _read_file(config_file, errors) if config_file else {}
|
||||
tuning = _parse_tuning(tree.get("tuning"), errors)
|
||||
flat = _flatten(tree)
|
||||
flat.pop("tuning", None)
|
||||
|
||||
known = set(REQUIRED_KEYS) | set(OPTIONAL_KEYS)
|
||||
for key in sorted(set(flat) - known):
|
||||
errors.append(f"{key}: unknown setting")
|
||||
overrides = {key: env[var] for var, key in ENV_KEYS.items() if var in env}
|
||||
# A key the environment overrides is not resolved from the file, so a
|
||||
# reference there to an unset variable is not an error.
|
||||
values = {
|
||||
key: _resolve(value, key, env, errors)
|
||||
for key, value in flat.items()
|
||||
if key in known and key not in overrides
|
||||
}
|
||||
values.update(overrides)
|
||||
|
||||
# Whitespace-only counts as missing: a token of spaces would pass a
|
||||
# truthiness check and fail at the first request instead.
|
||||
missing = [name for name in REQUIRED if not (env.get(name) or "").strip()]
|
||||
missing = [
|
||||
_describe(key)
|
||||
for key in REQUIRED_KEYS
|
||||
if values.get(key) is None
|
||||
or (isinstance(values[key], str) and not values[key].strip())
|
||||
or (isinstance(values[key], list) and not _people(values[key]))
|
||||
]
|
||||
if missing:
|
||||
raise ConfigError(
|
||||
"cannot start: these are unset or empty — " + ", ".join(missing)
|
||||
errors.append("unset or empty — " + ", ".join(missing))
|
||||
|
||||
max_output = values.get("llm.max_output_tokens", DEFAULT_MAX_OUTPUT_TOKENS)
|
||||
try:
|
||||
max_output = int(max_output)
|
||||
if max_output <= 0:
|
||||
raise ValueError
|
||||
except (TypeError, ValueError):
|
||||
errors.append(
|
||||
f"{_describe('llm.max_output_tokens')}: must be a positive integer"
|
||||
)
|
||||
if errors:
|
||||
raise ConfigError("cannot start: " + "; ".join(errors))
|
||||
|
||||
config = cls(
|
||||
state_db=Path(env["NISSE_STATE_DB"]).expanduser(),
|
||||
workspace=Path(env["NISSE_WORKSPACE"]).expanduser(),
|
||||
project=env["NISSE_PROJECT"],
|
||||
llm_base_url=env["NISSE_LLM_BASE_URL"],
|
||||
llm_token=env["NISSE_LLM_TOKEN"],
|
||||
roles=RoleMap.from_env(env),
|
||||
matrix=MatrixConfig.from_env(env),
|
||||
operators=_people(env["NISSE_MATRIX_OPERATORS"]),
|
||||
gitea_url=env["NISSE_GITEA_URL"],
|
||||
gitea_token=env["NISSE_GITEA_TOKEN"],
|
||||
gitea_repo=env["NISSE_GITEA_REPO"],
|
||||
gitea_operators=_people(env["NISSE_GITEA_OPERATORS"]),
|
||||
base_branch=env.get("NISSE_BASE_BRANCH", "main"),
|
||||
state_db=Path(values["state_db"]).expanduser(),
|
||||
workspace=Path(values["workspace"]).expanduser(),
|
||||
project=str(values["project"]),
|
||||
llm_base_url=str(values["llm.base_url"]),
|
||||
llm_token=str(values["llm.token"]),
|
||||
roles=RoleMap(
|
||||
executor=str(values["roles.executor"]),
|
||||
planner=str(values["roles.planner"]),
|
||||
delegate=str(values["roles.delegate"]),
|
||||
consultant=str(values["roles.consultant"]),
|
||||
),
|
||||
matrix=MatrixConfig(
|
||||
homeserver=str(values["matrix.homeserver"]).rstrip("/"),
|
||||
user_id=str(values["matrix.user_id"]),
|
||||
token=str(values["matrix.token"]),
|
||||
room_id=str(values["matrix.room_id"]),
|
||||
),
|
||||
operators=_people(values["matrix.operators"]),
|
||||
gitea_url=str(values["gitea.url"]),
|
||||
gitea_token=str(values["gitea.token"]),
|
||||
gitea_repo=str(values["gitea.repo"]),
|
||||
gitea_operators=_people(values["gitea.operators"]),
|
||||
base_branch=str(values.get("gitea.base_branch") or "main"),
|
||||
max_output_tokens=max_output,
|
||||
tuning=tuning,
|
||||
config_file=config_file,
|
||||
)
|
||||
config._refuse_secrets_inside_the_clone(env)
|
||||
return config
|
||||
@@ -93,14 +350,20 @@ class AgentConfig:
|
||||
"""The workspace is a git clone the agent commits with ``git add -A``
|
||||
and pushes. Anything inside it reaches the project repository, so a
|
||||
state store or an env file placed there would publish credentials as a
|
||||
side effect of the agent working correctly — silently, and once.
|
||||
side effect of the agent working correctly — silently, and once. The
|
||||
config file holds no secrets itself, but it names where they come
|
||||
from and how the agent is deployed, which is not the project's
|
||||
business either.
|
||||
"""
|
||||
workspace = self.workspace.expanduser().resolve()
|
||||
offenders = []
|
||||
for label, path in (
|
||||
candidates = [
|
||||
("NISSE_STATE_DB", self.state_db),
|
||||
("NISSE_ENV", Path(env.get("NISSE_ENV", ".env"))),
|
||||
):
|
||||
]
|
||||
if self.config_file is not None:
|
||||
candidates.append(("NISSE_CONFIG", self.config_file))
|
||||
offenders = []
|
||||
for label, path in candidates:
|
||||
resolved = path.expanduser().resolve()
|
||||
if resolved == workspace or workspace in resolved.parents:
|
||||
offenders.append(f"{label} ({resolved})")
|
||||
|
||||
Reference in new issue
Block a user