Commit graph

2 commits

Author SHA1 Message Date
Matt Johnson
b694fc0c9d fix(3-L.5): per-backend settings schemas (fixes build_enrichers TypeError)
Surfaced during the 2026-05-20 NaviBackend activation: toggling
config.enrichment.backend_class to NoOpBackend while backend_settings still
held {"base_url": ...} crashed _rebuild_enrichers with
`TypeError: NoOpBackend() takes no arguments`, BEFORE invalidate() ran. Fixed
by mirroring the SourceAdapter.settings_schema pattern: each backend declares a
Pydantic settings_schema; validation happens at write-time (GUI POST) and
read-time (supervisor). A mismatch is now a clean ValidationError, never a
constructor TypeError.

Backends — each gets a `<Name>BackendSettings(BaseModel, extra="forbid")` +
`settings_schema` class attr, mirroring __init__ defaults EXACTLY (note:
timeout_s stays 10.0 — the brief's "5.0" was a transcription slip; preserve the
production default):
  NoOpBackend     -> NoOpBackendSettings        (no fields)
  NaviBackend     -> NaviBackendSettings        (base_url, timeout_s, headers, warmup)
  PhotonBackend   -> PhotonBackendSettings       (base_url, timeout_s, headers)
  NominatimBackend-> NominatimBackendSettings   (base_url, user_agent, rate_limit_per_sec, timeout_s)

GeocoderBackend Protocol (in geocoder.py, where the base actually lives — not
base.py, which only has Enricher) gains `settings_schema: type[BaseModel]`.

supervisor:
- build_enrichers validates backend_cls.settings_schema.model_validate(
  backend_settings) before instantiating, and constructs from the validated
  .model_dump(). ValidationError (not TypeError) on mismatch.
- _rebuild_enrichers builds into locals and commits to instance state only on
  success — a ValidationError leaves the previously-active enrichers/config/
  cache untouched.
- _handle_enrichment_change wraps the rebuild in try/except ValidationError:
  logs and returns, keeping the previous backend running (supervisor stays up;
  operator fixes the row; next NOTIFY applies cleanly). No cache invalidation
  on a failed change.

GUI /enrichment:
- GET skips the outer EnrichmentConfig.backend_settings field and renders a
  separate <fieldset> from describe_fields(backend_cls.settings_schema, ...)
  for the row's current backend_class. Backend fields namespaced bs_<name>.
- POST reassembles bs_<name> inputs into a backend_settings dict, validates it
  against the SUBMITTED backend_class's schema (so errors attach to the right
  fields when an operator is mid-switch), then validates the outer
  EnrichmentConfig. DB row written only if both pass; otherwise re-renders with
  field-level errors against the submitted backend.
- backend_class stays a plain text field (no <select>, no client-side reshape).

form_descriptors: generic `float -> "number"` widget (2 lines, mirrors K.5's
`dict -> "json"`), needed because backend schemas have float fields
(timeout_s, rate_limit_per_sec). Benefits any float field codebase-wide.

DB schema unchanged: backend_settings stays JSONB; validation moved to
use-site. _BACKEND_REGISTRY / _ENRICHER_REGISTRY unchanged beyond schema lookup.

Tests (test_backend_settings_schema.py, 11): schemas exist + extra='forbid';
Navi schema preserves defaults (timeout_s == 10.0); NoOp has zero fields;
build_enrichers raises ValidationError-not-TypeError for the exact 2026-05-20
case; supervisor keeps previous backend on a bad NOTIFY (the incident
scenario); valid NoOp-with-empty-settings applies + invalidates; GUI POST
rejects bad backend_settings without writing + re-renders against submitted
backend; GUI POST writes on valid settings. test_enrichment_config_plumbing
updated for the new context shape (outer_fields/backend_fields).

Verification: full pytest 546 passed, 1 skipped (was 535; +11). grep
subject_for_event/_ADAPTER_REGISTRY and grep 100.64.0./192.168.1. in src both
empty.

Does NOT touch PR L scope (events tab, remaining adapter enrichment_locations),
the DB schema, or the registries.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-20 23:10:10 +00:00
Matt Johnson
04c1d07b3f feat(3-K.5): operator-settable EnrichmentConfig (config plumbing)
Bridge PR for v0.5.0. PR J wired the supervisor with a hardcoded
EnrichmentConfig() default; PR K added real backends to the registry but
left no operator path to select one. K.5 closes that gap by mirroring the
config.adapters storage + LISTEN/NOTIFY hot-reload pattern.

config.enrichment (migration 024): single-row table (id BOOLEAN PK CHECK
(id = true), mirroring config.system). Columns enricher_class, backend_class,
backend_settings JSONB, cache_ttl_s, updated_at. Reuses the existing
config.set_updated_at + config.notify_config_change triggers (the NOTIFY
function's ELSE branch emits 'enrichment:' for this keyless single-row table).
Seeds framework DEFAULTS ONLY — GeocoderEnricher + NoOpBackend, empty
backend_settings, 24h TTL. NO URLs/IPs/auth in the seed; a fresh deploy runs
NoOp out of the box. Idempotent (CREATE IF NOT EXISTS / DROP TRIGGER IF
EXISTS / INSERT ON CONFLICT DO NOTHING).

Supervisor:
- Reads config.enrichment at startup (start() -> config_source
  .get_enrichment_config()), overriding the constructor default.
- Hot-reloads via _on_config_change(table == "enrichment"): re-reads the row,
  rebuilds the enricher set, and invalidates the enrichment cache when the
  enricher/backend/settings changed (a new backend must not keep serving the
  old backend's cached bundles until TTL). TTL-only changes retain the cache.
- build_enrichers now takes an explicit EnrichmentCache (the supervisor owns
  it so it can invalidate); cache no longer built inside build_enrichers.

ConfigStore / ConfigSource: get_enrichment_config() (falls back to defaults if
the row is somehow absent) + upsert_enrichment_config(). Mirrors the adapter
accessors.

cache.py: EnrichmentCache.invalidate(enricher_name=None) — DELETE all or
enricher-scoped; returns rows deleted.

GUI /enrichment: GET renders the EnrichmentConfig form via the generic
describe_fields machinery (no enrichment-specific Jinja); POST validates via
Pydantic, writes config.enrichment, and lets the NOTIFY trigger propagate the
hot-reload. New enrichment.html + a nav link. backend_settings (a dict field)
needed a generic "json" widget in describe_fields + the template — usable by
any dict-typed settings field, not enrichment-specific.

Necessary deviation (surfaced): PR K shipped a deployment-specific default
DEFAULT_BASE_URL = "http://192.168.1.130:8440" in navi.py. Bar (b) forbids
deployer IPs in src, and operator-settable base_url is exactly K.5's purpose,
so the default is changed to http://localhost:8440 (matching Photon/Nominatim
defaults). The live integration smoke (tests/, env-gated, skipped) now reads
the endpoint from NAVI_BASE_URL — no IP anywhere in src.

Tests (test_enrichment_config_plumbing.py, 10): ConfigStore read / default
fallback / upsert-passes-dict; cache invalidate all + scoped; supervisor builds
NaviBackend from config; hot-reload rebuilds + invalidates on backend change;
no-invalidate on TTL-only change; describe_fields json widget; /enrichment GET
render. test_firms updated for the build_enrichers signature change.

Hot-reload mechanism mirrored: Postgres LISTEN/NOTIFY on channel
'config_changed' (payload 'table:key'), same path adapters/streams use; the
supervisor's existing _on_config_change dispatch gains an "enrichment" branch.

Verification: full pytest 535 passed, 1 skipped (was 525; +10). Migration
applied cleanly on the live prod schema; SELECT * FROM config.enrichment
returns the NoOp default row. grep subject_for_event/_ADAPTER_REGISTRY and
grep 100.64.0./192.168.1. in src both empty.

Does NOT activate NaviBackend (ships NoOp default; operator action) and does
NOT declare enrichment_locations on other adapters (PR L scope).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-20 18:52:22 +00:00