Skip to content

refactor(config): single pydantic-settings model (closes #84)#98

Merged
hadamrd merged 1 commit into
trunkfrom
refactor/pydantic-settings
May 28, 2026
Merged

refactor(config): single pydantic-settings model (closes #84)#98
hadamrd merged 1 commit into
trunkfrom
refactor/pydantic-settings

Conversation

@hadamrd
Copy link
Copy Markdown
Owner

@hadamrd hadamrd commented May 28, 2026

Summary

  • Closes refactor(config): single pydantic-settings model replaces scattered config.py + os.environ.get sites #84
  • Replaces scattered config.py + 35 ad-hoc os.environ.get(...) callsites with one pydantic-settings Settings tree (src/forge_loop/settings.py).
  • Precedence is uniform: env > yaml > defaults. Validation errors raise ConfigError at load time with field name + source hint.
  • Legacy Config dataclass kept as a thin shim built from Settings — 80+ existing call sites (cfg.worker.model, cfg.critic.timeout_s, etc) work unchanged.
  • forge-loop config now dumps resolved Settings as round-trippable YAML; --json preserves the legacy summary surface.
  • Architectural regression test asserts no os.environ.get sites outside settings.py/config.py (modulo a small documented allowlist for dynamic-key callers).

Env-var → settings field mapping

Env var Settings path
LOOP_GH_REPO repo.github
LOOP_BASE_BRANCH repo.base_branch
LOOP_REPOS_DIR repo.repos_dir
LOOP_PARALLEL scheduling.parallel
LOOP_TICK_INTERVAL_S scheduling.tick_interval_s
LOOP_MAX_TICKS scheduling.max_ticks
LOOP_WORKER_TIMEOUT_S scheduling.worker_timeout_s
LOOP_MAINTENANCE_EVERY_N scheduling.maintenance_every_n_ticks
LOOP_DEPLOY_TASK deploy.task
LOOP_DEPLOY_DRIFT_HALT deploy.drift_halt
LOOP_QUERY_LABEL labels.ready
LOOP_CRITIC_* (5 knobs) critic.*
LOOP_PO_* (3 knobs) po.*
LOOP_WORKER_* (7 knobs) worker.*
LOOP_LUMEN_TOP_K lumen.top_k
LOOP_RETRY_COOLDOWN_S attempts.cooldown_s
LOOP_WORKER_MAX_ITERATIONS iteration.max_iterations
LOOP_PIPELINE_DRIVEN iteration.pipeline_driven
LOOP_OPERATOR_* (4 knobs) operator.*
LOOP_DASHBOARD_PORT/TOKEN dashboard.port/token
LOOP_COAUTHOR misc.coauthor
LOOP_EVENTS_ROTATE_BYTES misc.events_rotate_bytes
LOOP_MCP_CAP_DEFAULT misc.mcp_cap_default
LOOP_QUEUE_URL misc.queue_url
FORGE_LOOP_EXPERIMENTAL misc.experimental_enabled

Documented exceptions (kept reading env directly — dynamic key or out-of-scope)

  • briefs/__init__.py — env-var name is a function arg (dynamic)
  • mcp_server.py — per-tool cap LOOP_MCP_CAP_<TOOL_NAME> (dynamic)
  • observability/__init__.py — experimental extras, separate cleanup pass
  • runner_async.py — dynamic env-keyed helper
  • cli_tui.py — FORGE_LOOP_TUI_FORCE single op-mode flag

Test plan

  • pytest tests/test_settings.py -v — 12 new tests (precedence matrix, validation, yaml round-trip, codex defaults, regression gate)
  • pytest tests/test_config.py — 22 existing assertions still green (monkeypatches repointed to forge_loop.settings._repo_root)
  • pytest tests/ — 613 passed, 0 regressions vs trunk baseline (10 pre-existing failures unchanged)
  • forge-loop config round-trips: write yaml → load → re-dump = same shape

@hadamrd
refactor(config): single pydantic-settings model — kills scattered os…
6d36f8d 
….environ.get sprawl (closes #84)

Before: config.py + 35 ad-hoc os.environ.get() callsites across 15 modules.
Inconsistent precedence (some env vars beat yaml, some didn't), no schema
validation at load, no `forge-loop config` output worth trusting.

After:
- src/forge_loop/settings.py: one pydantic-settings Settings tree with
  nested groups (RepoSettings, WorkerSettings, OperatorSettings, ...)
  mirroring the legacy Config shape. Uniform precedence: env > yaml >
  defaults. Validation errors raise ConfigError at startup with the field
  name + source hint so operators can fix typos without grepping.
- ENV_MAP is data-driven (env_var → dotted_path → coercer) so a future
  `forge-loop config --show-env` can dump the whole mapping.
- config.py: kept as a thin backwards-compat shim. Config dataclass is
  materialised from Settings via _from_settings(); existing 80+ call
  sites (cfg.worker.model, cfg.critic.timeout_s, ...) work unchanged.
  ModelConfigError aliased to ConfigError.
- Ported standalone env reads (operator.py, attempts.py, state.py,
  cli.py drift_halt/dashboard/repos_dir, mcp_server cap default,
  drift.py, boot.py queue_url, _pipeline_driver.py, _extras.py
  experimental, dashboard/app.py token).
- `forge-loop config` defaults to dumping resolved Settings as YAML
  (round-trippable). --json preserves the legacy summary surface.
- Architectural regression gate: tests/test_settings.py asserts no
  os.environ.get sites outside settings.py + config.py (modulo a small
  allowlist of dynamic-key callers documented inline).

Tests:
- +12 new tests in test_settings.py (precedence matrix, validation,
  yaml round-trip, codex provider defaults, regression gate).
- test_config.py: monkeypatches repointed to forge_loop.settings._repo_root;
  all 22 existing assertions still pass.
- Full suite: 613 passed, baseline 10 pre-existing failures unchanged.
- 0 regressions introduced.

Legitimate exceptions (NOT ported, listed in test allowlist):
- briefs/__init__.py: env_var name passed as function arg (dynamic key)
- mcp_server.py per-tool cap (LOOP_MCP_CAP_<TOOL_NAME> — dynamic key)
- observability/__init__.py: experimental-gated, separate cleanup pass
- runner_async.py: dynamic env-keyed helper
- cli_tui.py: FORGE_LOOP_TUI_FORCE single op-mode flag
@hadamrd hadamrd merged commit 158cc0f into trunk May 28, 2026
2 checks passed
@hadamrd hadamrd deleted the refactor/pydantic-settings branch May 28, 2026 09:14
This was referenced May 28, 2026
Closed
Merged
hadamrd added a commit that referenced this pull request May 28, 2026
@hadamrd
feat(manifestos): seed quality+testing manifestos for forge-loop (#135)…
1d3f057 
… (#139)

Dogfood the manifestos system on forge-loop itself by writing the seed
quality and testing manifestos that every future forge-loop change is
gated against.

quality-manifesto.md codifies five rules drawn from this week's
persistent-worker work: no shared module-level state (#100), typed
Protocol+Fake at every I/O boundary (#104), single Settings source of
truth (#98), typed events instead of untyped **fields (#99), and no
subprocess.run for SDK-able services (#103, #105). Each rule names the
concrete issue it came from so future contributors know the *why*.

testing-manifesto.md codifies six rules drawn from this week's
iteration-probe bugs: one test per state-machine edge plus a fallthrough
adversarial (would have caught #97/#120/#128), an adversarial test for
the false case of every external-dep assumption, both ==0 and !=0
branches for every subprocess.returncode (specifically #128), a
contract test pinning every Fake to its Real, hypothesis property
tests on >4-branch / user-input functions (#102), and an adversarial
test that every infinite-loop guard actually fires.

tests/test_manifestos_discovery.py is the meta-validation gate: it
discovers and parses both files, asserts each rule has a rationale,
asserts the spec-mandated issue references are present, and includes
adversarial tests that stubs and missing files are detectable. 22
tests, all pass.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

refactor(config): single pydantic-settings model replaces scattered config.py + os.environ.get sites

1 participant

@hadamrd