Review fixes: deploy/onboarding, schema & doc drift, CI hardening - #18
Merged
Conversation
Front-door docs had drifted from reality (flagged in the branch review): - schema: 24 ORM tables (25 live), not 25/26; drop the maintainer_credential ghost (dropped in migration 0022); add reach.gradient_profile + gradient_unreliable to the reach table. - pipeline: document the check-reaches step that wires in after orphan-check. - timers: reconcile the count to 15 and add kayak-status/fetch-osmb/recap to the README + operations lists. - quick-start: load the metadata snapshot so a fresh DB renders data instead of failing orphan-check on ~300 orphan sources. - hardware: Hetzner CPX11 (2 vCPU/2 GB/40 GB), not the stale CPX21. - operations: add the /_internal/status operator-dashboard row; populate the CHANGELOG [Unreleased] section. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The CI and pre-commit comments claimed 'Level 7' grandfathering '123' pre-existing issues. The gate is actually level 8 with a 79-error phpstan-baseline.neon. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
These four migrations lacked @no_transaction yet carried their own BEGIN TRANSACTION/COMMIT. migrate.py wraps each non-@no_transaction file in one transaction and splits on ';', so the inner COMMIT ended that wrapper early -- a failure afterward would leave data committed but the version unstamped. Removed the explicit transaction statements (the runner owns the transaction) and left a comment to prevent regression. UPDATEs unchanged and idempotent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…v, specs - SETUP.md now deploys conf/sites/ + conf/snippets/levels-common.conf (the live config) instead of the retired deploy/levels monolith; HSTS and fastcgi_pass references repointed; the historical Oracle Cloud section removed. - install.service.sh installs all 15 timers (was missing kayak-fetch-osmb and kayak-status). - nginx-editor-env.conf: hostname-aware $site_url (the 2026-05-20 cutover is done). - engine.py: the SQLite PRAGMA comment reflects the 2 GB CPX11 + swap, not 4 GB. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Default --db to the configured DATABASE_URL (matching how levels resolves it) so init-db and the metadata load can't target different files when the operator has set DATABASE_URL. Docstring now uses 'init-db --no-seed' to avoid duplicate-by-name source rows on import. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
docs/done/ held 18 completed plans with no README, so a reader couldn't tell them from in-flight plans — and several are load-bearing (cited by systemd unit comments, operations.md, slo.md). Added an index that lists each with a one-line summary and frames them as rationale/provenance, and notes why PLAN_production_discipline.md stays in docs/ root (live cross-reference target). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- biome: replace the hand-maintained includes allowlist with globs over static/ and src/kayak/web/static/ (excluding vendored leaflet.*), so new JS is linted automatically instead of silently escaping CI. The allowlist had already dropped gradient-profile.js, internal-sort.js, and scroll-indicator.js. - internal-sort.js: var -> const/let + optional chaining (the noVar debt the glob surfaced); 'biome check' is now clean (0 errors). - lint job: pin ruff==0.15.11 so the format gate can't drift on a new release (the churn behind df6e422). - add .github/dependabot.yml (github-actions / npm / composer, monthly + grouped, low-noise for a solo maintainer; pip omitted since uv owns uv.lock). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per docs/one-offs/README's own lifecycle rule (move a script here once its outcome is captured in the DB -- migration 0046 did): git mv the five run-once gradient scripts (fetch_dem_tiles, sample_reach_elevations, compute_reach_gradient, emit_max_gradient_migration, snap_reach_to_channel_min) from scripts/ to docs/one-offs/ and add their rows to the index. Repointed the intra-script cross-references and the one live consumer comment (php/includes/svg_plot.php) to the new path. Migration-header provenance comments are left as historical (git log --follow resolves them). The geom / M_TO_FT duplication the review flagged lived in these now-archived scripts, so it's out of the active tree. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Records the self-contained-rebuild decision in export_metadata.py so a future maintainer doesn't exclude gradient_profile (or trim huc_name) for size: `levels init-db --no-seed` + import_metadata.py restores a fully-populated DB (gradients + basin names) without replaying migration 0046 or re-running assign-huc. The reach.csv gradient columns themselves landed via prod's nightly snapshot (255d6e9), which a drift check confirmed byte-identical to a fresh dev export, so no separate reach.csv change is needed here. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
deploy/levels and conf/levels.nginx were the pre-split monolithic nginx vhost, retired by the conf/sites/ + conf/snippets/ split and already drifted ~20 lines from each other; check-config-drift.sh excluded them as retired. Removed both. Repointed the references that cited them by line number: - Living docs (posture, controls-map, editor-surface, incident-response) now point at the real files: HSTS -> conf/security-headers.conf; access_log -> conf/sites/levels-wkcc-org; rate-limit bindings / security.txt / editor location blocks -> conf/snippets/levels-common.conf. - A note atop docs/security/README.md maps deploy/levels -> the split files so the point-in-time refs in the dated tier-audit logs and F-1 findings history stay interpretable without rewriting the audit record. - check-config-drift.sh comment updated (removed, not just retired). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
scripts/regression/gauge_pair_linear.py and its test import numpy, but numpy was only present transitively via the [geo] extra (rasterio -> numpy). A [dev]-only venv (the documented `pip install -e .[dev]`) therefore lacked it, so test_gauge_pair_linear hard-failed locally while CI (uv sync --all-extras) stayed green. Declared numpy in [dev] and refreshed uv.lock so the base test suite runs without the heavy [geo]/GDAL stack. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The from-scratch VM install surfaced two SETUP.md bugs: - §3 set only SQLITE_PATH (the PHP variable), pointed inside the repo. But `levels` (Python) reads DATABASE_URL, so init-db fell back to its default /home/pat/DB/kayak.db -- whose directory didn't exist -> sqlite3 'unable to open database file'. Now: mkdir /home/pat/DB and set DATABASE_URL + SQLITE_PATH to the same prod path (matching the nginx fastcgi_param). - §4 ran `init-db` (seeded), which leaves ~300 orphan sources and an empty site. Switched to init-db --no-seed -> import_metadata.py -> pipeline so the gauge_source links load and orphan-check passes (same flow already in README/CLAUDE; SETUP.md had been missed). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Remediates the findings from a deep review of the repo. Scope is the deploy-critical and drift/bloat findings; the trace-geometry work is a deliberate separate follow-up PR.
Deploy & onboarding — the #1 finding was a broken documented quick-start
init-db --no-seed → import_metadata.py → pipeline, so a fresh DB renders real data instead of dying atorphan-checkwith ~300 orphan sources (README, CLAUDE.md, SETUP.md §3/§4).import_metadata.pyresolves its DB path fromDATABASE_URL(matchinglevels), so it can't target a different file thaninit-db./home/pat/DBand setsDATABASE_URL+SQLITE_PATH— fixes the "unable to open database file" a real fresh install hit (Python readsDATABASE_URL, notSQLITE_PATH).install.service.shinstalls all 15 timers (was missingkayak-fetch-osmb+kayak-status).conf/sites/+conf/snippets/nginx split instead of the retireddeploy/levels; the historical Oracle Cloud section is removed.nginx-editor-env.conf$site_urlis hostname-aware (post-2026-05-20 cutover).engine.pySQLite-PRAGMA comment.Migrations
BEGIN/COMMITfrom migrations 0052/0054/0055/0056 — they ran inside the migrate runner's own transaction and defeated its atomicity.Schema & documentation drift
maintainer_credentialtable removed (dropped back in migration 0022),gradient_profile/gradient_unreliablecolumns documented.check-reaches; systemd timer counts reconciled to 15; CHANGELOG[Unreleased]populated; CI/pre-commit PHPStan comments corrected (level 8, 79-item baseline).docs/done/README.mdindex; archived the five run-once gradient scripts intodocs/one-offs/per the repo's own lifecycle rule.CI
biome.jsonnow lints all project JS/CSS via glob (closes the gap where new JS — incl.gradient-profile.js— escaped CI) and the surfacedinternal-sort.jslint debt is fixed.ruffin the lint job (was floating); added.github/dependabot.yml; declarednumpyin[dev]so the regression test runs in any dev venv.Removed
deploy/levels+conf/levels.nginxmonoliths (~682 drifted lines); repointed the livedocs/security/references to theconf/split, with a note covering the dated tier-audit refs.Validation
Exercised end-to-end on a fresh Debian 13 arm64 VM: the corrected
init-db --no-seed → import_metadata → pipelineproduced a populated site (orphan-check: clean,check-reaches: clean), and the PHP layer — including the gradient chart — renders. The deploy docs were followed verbatim, which is how the SETUP.md §3/§4 bug was caught and fixed.Out of scope (separate follow-up PR)
Trace-geometry simplification (~5 m Douglas–Peucker, 11.8 → 2.3 MB), the
reaches.jsonsnapshot + import wiring, and thefind_huc4→reach.huc[:4]determination fix.🤖 Generated with Claude Code