Skip to content

refactor(audit): promote audit_gauges to levels audit-gauges (4C prep) - #191

Merged
mousebrains merged 2 commits into
mainfrom
promote-audit-gauges
Jun 14, 2026
Merged

refactor(audit): promote audit_gauges to levels audit-gauges (4C prep)#191
mousebrains merged 2 commits into
mainfrom
promote-audit-gauges

Conversation

@mousebrains

Copy link
Copy Markdown
Owner

What

Promotes scripts/audit_gauges.py to a levels audit-gauges CLI subcommand by moving its closure into the kayak package. 4C prep: a paired-release systemd unit must run from the release venv (/opt/kayak/current/venv/bin/levels), not a source script that's absent from immutable releases — surfaced by the clean-VM cutover rehearsal.

Changes

  • git mv the 4-file closure into src/kayak/gauge_audit/ (history preserved): audit.py, usgs_sites.py, nwps_sites.py; _gauge_metadata_cache.py deleted (→ kayak.config.GAUGE_METADATA_CACHE).
  • Fetchers' argv main() → callables; refresh_caches() calls them directly (no sys.argv/sys.path hacks).
  • New typed src/kayak/cli/audit_gauges.py (addArgs + handler → audit.run_audit), registered in cli/main.py. gauge_audit imports are lazylevels startup and other subcommands are unaffected; requests (used by the fetchers) stays a lazy, prod-locked dep, so the release venv has it.
  • systemd/kayak-audit-gauges.service: ExecStartlevels audit-gauges (literal binary path like the 5 sibling levels units; 4C re-points it to /opt/kayak/current/venv).
  • config.py: coerce blank/whitespace AUDIT_EMAILNone. Routing through KayakConfig (vs the old raw os.environ.get) means an empty AUDIT_EMAIL (systemd ${AUDIT_EMAIL} unset) would fail EmailStr validation and break every levels command at import — the validator makes "" mean "no digest".
  • mypy: gauge_audit.{audit,usgs_sites,nwps_sites} get the gradual-typing override (same precedent as kayak.tracing.trace); the CLI module is fully typed.
  • Also fixed scripts/seed_gauge_display.py (used the deleted cache module); moved tests → tests/test_gauge_audit/; swept doc refs.

Gate

ruff (full CI scope) · format · mypy (106 files) · levels audit-gauges --help · 1725 fast tests — all green. requests verified declared + in requirements-prod.lock.

🤖 Generated with Claude Code

…ges`

4C prep: a paired-release systemd unit must run from the release venv, not
a source script absent from immutable releases (a finding from the clean-VM
cutover rehearsal). Move the audit closure into the package + expose a CLI.

- git mv scripts/{audit_gauges,fetch_usgs_sites,fetch_nwps_sites}.py →
  src/kayak/gauge_audit/{audit,usgs_sites,nwps_sites}.py (history preserved);
  delete scripts/_gauge_metadata_cache.py → kayak.config.GAUGE_METADATA_CACHE.
- Fetchers' argv main() → fetch_usgs_sites()/fetch_nwps_sites() callables;
  refresh_caches() calls them directly (no sys.argv/sys.path hacks).
- New TYPED src/kayak/cli/audit_gauges.py (addArgs + handler → audit.run_audit),
  registered in cli/main.py. gauge_audit imports are lazy, so `levels` startup
  (and other subcommands) are unaffected; requests stays a lazy, prod-locked dep.
- systemd/kayak-audit-gauges.service ExecStart → /home/pat/.venv/bin/levels
  audit-gauges (literal binary path like the sibling levels units; 4C re-points
  it to /opt/kayak/current/venv).
- config.py: coerce blank/whitespace AUDIT_EMAIL → None. Routing through
  KayakConfig (vs the old raw os.environ.get) means an empty AUDIT_EMAIL
  (systemd ${AUDIT_EMAIL} unset) would fail EmailStr validation and break EVERY
  levels command at import — the validator makes "" mean "no digest".
- mypy: gauge_audit.{audit,usgs_sites,nwps_sites} get the gradual-typing
  override (same precedent as kayak.tracing.trace); cli/audit_gauges.py is typed.
- Also fixed scripts/seed_gauge_display.py (used the deleted cache module);
  moved tests → tests/test_gauge_audit/; swept doc refs.

Gate green: ruff (full scope), format, mypy (106 files), `levels audit-gauges
--help`, 1725 fast tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mousebrains

Copy link
Copy Markdown
Owner Author

Adversarial review — PR #191 (levels audit-gauges promotion)

Reviewed against the running WKCC host. This is a clean wholesale move (git mv history preserved) + a thin typed CLI wrapper, and it holds up well — no code bugs, no merge blockers. The one thing that genuinely matters is a deploy-coupling (D1) that a bare git pull would get wrong.

Verified against the live host ✅

  • requests is satisfied: 2.33.1 in the live app venv and in requirements-prod.lock — so the release venv (4C) will have it, and the lazy import keeps it off the levels startup path (other subcommands / the pipeline don't pull it).
  • Email path is safe: /usr/bin/mail is present, and _send_email_digest guards with shutil.which("mail") (warn + skip, never fail) — so a host without mail degrades gracefully.
  • DB + cache defaults resolve correctly under the unit: the unit runs User=pat, so HOME=/home/pat → the CLI's --kayak-db default (Path.home()/DB/kayak.db) lands on the real live DB, and GAUGE_METADATA_CACHE resolves to /home/pat/kayak/Gauge-metadata-cache/gauges.db (the existing 7.8 MB cache). The unit passes neither flag, so both defaults are load-bearing — both are correct here.
  • No regression in the CLI surface: the new addArgs defaults (--days 7 overridden to 16 by the unit, --cache-db, --kayak-db, --email, --candidate-miles, --include-gauged) mirror the old scripts/audit_gauges.py argparse exactly.
  • The AUDIT_EMAIL coercion is a real latent-bug fix, correctly scoped. kayak.config builds a KayakConfig at import (_config = get_config(), config.py:294; GAUGE_METADATA_CACHE = _config.gauge_metadata_cache, :326), so an empty AUDIT_EMAIL in the environment would fail EmailStr and break every levels command at import — not just audit. _blank_email_to_none (mode="before") makes ""/whitespace mean "no digest" while a genuinely malformed non-empty value still fails (right call). Belt-and-suspenders with the runtime path: run_audit uses if args.email:, so the unit's --email ${AUDIT_EMAIL} expanding to empty also yields no digest. Live AUDIT_EMAIL is set and non-empty, so today's path is unaffected either way.
  • Clean break: no deploy/systemd/drift script and no non-historical code still references scripts/audit_gauges.py, the old fetchers, or _gauge_metadata_cache; seed_gauge_display.py was correctly repointed to kayak.config.GAUGE_METADATA_CACHE.

Findings

D1 — Medium (deploy ordering; the one to not miss): this is NOT safe to deploy via a bare git pull. The PR deletes scripts/audit_gauges.py, but the installed unit /etc/systemd/system/kayak-audit-gauges.service still runs ${KAYAK_HOME}/kayak/scripts/audit_gauges.py. After a pull the script is gone, so until the unit is reinstalled (sudo install + systemctl daemon-reload) the next firing fails — and with ExecStartPost=-curl … ${HC_AUDIT_GAUGES} plus the OnFailure chain, a failed run would alert. Timing on this host: next run is Wed 2026-06-17 03:18 PDT (~3.5 days) (OnCalendar=*-*-02,17 03:00), and kayak-config-drift (Sun ~05:30) will flag the repo↔/etc mismatch before then. So deploying #191 requires reinstalling kayak-audit-gauges.service in the same step. Note this isn't 4C-only despite the framing — the ExecStart → levels audit-gauges change is correct and required for the current host the moment the script is deleted (the binary /home/pat/.venv/bin/levels audit-gauges works post-pull).

D2 — Nit (doc accuracy): "weekly" is wrong. The config.py comment ("the weekly gauge-metadata audit timer") and the PR body call it weekly; the timer is *-*-02,17 03:00 — twice monthly (2nd & 17th, ~15-day gaps). Worth correcting to avoid future confusion.

D3 — Nit (4C forward-looking, already acknowledged in code): gauge_metadata_cache is BASE_DIR-relative. In a paired release where levels runs from /opt/kayak/current/venv, BASE_DIR resolves inside the immutable release, so the generated cache would try to write into a read-only release dir. The config field comment already flags this and the field is env-overridable — so the 4C runbook just needs to point GAUGE_METADATA_CACHE (and the unit's --cache-db/env) at a stable writable path outside the release. Fine as-is for the current host; flagging so it lands on the 4C checklist.

D4 — Trivial: cli/validate_config.py:77 comment still names fetch_usgs_sites.py (now kayak/gauge_audit/usgs_sites.py). Cosmetic.

Bottom line

Solid, low-risk refactor; the laziness and the AUDIT_EMAIL hardening are both done right, and everything checks out against this host. No blockers (merge is currently BLOCKED only on the required-approval gate; I can't read CI via my token). The single must-do at deploy time is D1 — reinstall the audit unit alongside the pull, or the 6/17 run breaks.

Reviewed against the live WKCC host, 2026-06-13. 🤖 Generated with Claude Code

@mousebrains

Copy link
Copy Markdown
Owner Author

Adversarial review, including live-server context:

Finding

  • P1/P2: GAUGE_METADATA_CACHE is still implicit, so the 4C release unit will write outside the service sandbox / wrong cache path.
    systemd/kayak-audit-gauges.service:19 now runs levels audit-gauges, whose --cache-db default comes from kayak.config.GAUGE_METADATA_CACHE (src/kayak/cli/audit_gauges.py:16,45; src/kayak/config.py:158,326). The unit still only grants writes to /home/pat/DB and /home/pat/kayak/Gauge-metadata-cache (systemd/kayak-audit-gauges.service:34) and the comment says the default remains /home/pat/kayak/Gauge-metadata-cache/gauges.db (:27), but that is only true while kayak is imported from the live editable checkout.

    On this live host today, /opt/kayak and /etc/kayak/deploy.env are absent, the installed levels imports /home/pat/kayak/src/kayak, and neither /etc/kayak/env nor /home/pat/.config/kayak/.env sets GAUGE_METADATA_CACHE; in that current layout the default resolves to /home/pat/kayak/Gauge-metadata-cache/gauges.db, so the PR works in the live-tree setup. But the PR body says this is 4C prep for a paired-release unit running from /opt/kayak/current/venv/bin/levels. In an isolated PR checkout, the same code already resolves the default to /tmp/kayak-pr191-review/Gauge-metadata-cache/gauges.db; a release venv will similarly resolve under the release/package/venv location, not the live cache directory allowed by ReadWritePaths=.

    That means the first release-run audit refresh is likely to either fail with sqlite3.OperationalError: unable to open database file under ProtectSystem=strict, or silently create/read a different cache if the release path happens to be writable. Please make the cache path explicit before 4C: set GAUGE_METADATA_CACHE in the host/app env or directly in the unit to a persistent external path, and make ReadWritePaths= match that path. If the deployer needs to propagate app env into release commands, include GAUGE_METADATA_CACHE in that allowlist too.

What I verified

  • Targeted PR tests passed: pytest tests/test_config.py tests/test_gauge_audit/test_audit.py -> 52 passed.
  • PR CLI help works with AUDIT_EMAIL="": python -m kayak.cli.main audit-gauges --help.
  • Live-context dry run with refresh and email disabled succeeded against the real DB/cache when the cache path was explicit: audit-gauges --no-refresh --days 16 --cache-db /home/pat/kayak/Gauge-metadata-cache/gauges.db --kayak-db /home/pat/DB/kayak.db --email "" exited 0 and reported 2 USGS candidates, 1 NWPS candidate, and 1 stale gauge.

I did not run the network refresh path or send an email from the live server.

…dence docs

Review follow-ups on the audit_gauges promotion:

- systemd/kayak-audit-gauges.service: set GAUGE_METADATA_CACHE explicitly
  rather than relying on config.py's BASE_DIR-relative default (which only
  happens to resolve to the right path because the editable install lives at
  /home/pat/kayak). The literal value matches ReadWritePaths= so the write
  target stays inside the sandbox grant even if the install location moves.

- config.py: the audit timer is twice-monthly (2nd + 17th), not weekly —
  correct the audit_email docstring to match the unit's OnCalendar.

- cli/validate_config.py: the USGS_API_KEY reader moved from the deleted
  scripts/fetch_usgs_sites.py into gauge_audit/usgs_sites.py; update the
  stale comment to the current path.

- gauge_audit/{usgs,nwps}_sites.py: tag the `import requests` with
  `# type: ignore[import-untyped]`, matching fetch_usgs_ogc.py, so mypy
  passes with or without the types-requests stub installed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mousebrains

Copy link
Copy Markdown
Owner Author

Thanks — both reviews addressed in 41ff2e3. Summary of what landed and the one deploy-time action that stays on the human:

Fixed in code:

  • P1/P2 + D3 (cache path now explicit). systemd/kayak-audit-gauges.service sets Environment=GAUGE_METADATA_CACHE=/home/pat/kayak/Gauge-metadata-cache/gauges.db directly, so the write target no longer rides on config.py's BASE_DIR-relative default. ReadWritePaths= already names that exact dir, so they match. This makes the current live-tree unit explicit; the 4C unit renderer will carry the same env key forward (pointed at a stable writable path outside the release), which is the paired-release half of P2/D3 — tracked on the 4C checklist, not this PR.
  • D2 (cadence). config.py audit_email docstring: "weekly" → "twice-monthly (2nd + 17th)", matching OnCalendar=*-*-02,17 03:00.
  • D4 (stale comment). cli/validate_config.py now names cli/fetch_usgs_ogc.py / gauge_audit/usgs_sites.py instead of the deleted fetch_usgs_sites.py.
  • (Bonus) tagged import requests in both gauge_audit fetchers with # type: ignore[import-untyped], matching fetch_usgs_ogc.py, so mypy passes with or without the types-requests stub.

D1 — deploy-time must-do (no code change; flagging so it isn't missed): this PR deletes scripts/audit_gauges.py, but the installed /etc/systemd/system/kayak-audit-gauges.service still execs that script. scripts/deploy.sh deliberately does not touch /etc/systemd/, so a bare git pull deploy leaves the old unit pointing at a now-deleted script — the next firing (Wed 2026-06-17 ~03:18 PDT) would fail and trip the OnFailure alert chain. Deploying #191 requires reinstalling the unit in the same step (install the new .service + systemctl daemon-reload). kayak-config-drift (Sun ~05:30) would also catch the repo↔/etc mismatch first, but better to do it at deploy time. The new ExecStart=… levels audit-gauges is correct and works on the current host the moment the script is deleted (the levels binary is already installed), so this is a now-fix, not 4C-only.

Gate green locally (ruff, format, mypy, full non-slow suite 1725 passed). Not merging — yours to merge.

@mousebrains
mousebrains merged commit 06e1f1e into main Jun 14, 2026
9 checks passed
@mousebrains
mousebrains deleted the promote-audit-gauges branch June 14, 2026 00:45
mousebrains added a commit that referenced this pull request Jun 14, 2026
Batch 4C foundation. docs/PLAN_4c_renderers.md lays out the remaining work to
finish the paired-release cutover: render the systemd unit drop-ins / nginx
vhosts / FPM open_basedir from host.yaml (replacing the hand-crafted runbook
step 5), add the deployer's serving-path verification gate, and flip the live
host to SERVING_CUTOVER=yes. It also records the verified consumer
classification — note that #191's audit-gauges promotion makes it the 6th
engine consumer to re-point (the runbook predates that and lists 5).

This first increment adds only the scalar HostConfig fields the renderers need,
all defaulting to the current WKCC shape (keep-current-then-flip, like the S3
slices) so nothing changes until the cutover host.yaml ships:

- service_user / service_home — the account + home the units run as
- release_root — the /opt/kayak paired-release root
- fpm_pool_php — the PHP-FPM pool version in /etc/php/<v>/fpm

Validators: service_user must be a bare POSIX username (it's interpolated into
unit User= and shell ACL commands), the paths absolute, fpm_pool_php major.minor.
docroot (existing) is reused — it flips to /var/cache/kayak/docroot at cutover.
The vhost server_names type lands with the nginx renderer (increment 3) that
consumes it, not here.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mousebrains added a commit that referenced this pull request Jun 14, 2026
… runbook

Rehearsed the live cutover (running host → paired-release) end-to-end on a
live-equivalent VM clone (full live DB — 4.46M obs, 77 migrations) with ZERO
serving disruption, and captured it as a "Migrating a RUNNING host" section.

The validated no-downtime ordering:
- Phase 0: pre-stage the paired-release ALONGSIDE the running site (dirs, host.yaml,
  deploy.env, stage-only) — no mutation, public_html keeps serving.
- Phase 1: apply the cutover config to the FILES only (render-serving → nginx root
  + FPM open_basedir via sed; render-units → drop-ins) and stop the timers, but DO
  NOT reload nginx/FPM — the running services hold the old in-memory config, so
  public_html keeps serving. Skip install-config.sh (pool/runtime-config already
  exist; a reload now would serve the empty new docroot).
- Phase 2: activate — the gate passes on the FILES, `migrate` reports "No pending"
  against the live DB (no init-db — reuse it), the build writes the new docroot,
  symlink switch. Still serving public_html.
- Phase 3: graceful `nginx reload` + `fpm restart` → flip to the new docroot.

Confirmed: serving stayed 200 throughout (public_html → new docroot at the flip);
the live DB migrated clean; and the cutover FIXED the live audit-gauges unit (the
#191 promotion lands via render-units, which scripts/deploy.sh never did). "Still
open" trimmed to: execute the real cutover + the deferred genericization.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant