feat(licor): LI-COR public-dashboard parser + fetch-licor step

[mousebrains]

Why

Engine groundwork for replacing the calculated Kalama Italian-Creek gauge (source 354) with its real LI-COR sensor — the implementation half of docs/PLAN_kalama_licor_gauge.md. Inert until a kayak_data fetch_url references parser licor (no dataset change here).

What

The shared levels fetch client is GET-only and the LI-COR timeseries endpoint requires a POST with a JSON body, so this follows the established standalone-step pattern (fetch-usgs-ogc, fetch-osmb) rather than bolting a fetch hook onto the async GET path:

• kayak.parsers.licor.LicorParser (@register("licor")): pure parse_records over the LI-COR JSON. Matches channels by UUID from the configured fetch_url query params — never the display metricName/metricUnits (the API returns e.g. "°F"). Emits tz-aware UTC from epoch-ms; flow/water-level/water-temp → DataType.{flow,gauge,temperature}; air temperature ignored; future / non-finite / boolean / malformed points filtered.
• BaseParser.transport (default "GET"); LicorParser sets "POST". The default fetch skips non-GET parsers (so it never GETs a POST-only endpoint); fetch-licor selects only POST parsers.
• levels fetch-licor (cli/fetch_licor.py): validates + builds the POST body from the configured URL (fail closed before any I/O), re-runs the _validate_url SSRF guard, POSTs with allow_redirects=False + retry + timeout + body cap, and feeds the JSON to LicorParser so storage, caching, and station attribution match every other parser. Wired as a soft pipeline step after fetch-usgs-ogc; nothing requires it, so a LI-COR outage can never block build.

Registered in main.py + ensure_all_loaded; pipeline DAG + CLAUDE.md updated.

Adversarial review (3 parallel reviewers) — findings fixed in be00bf7

• Critical (SSRF): requests.post followed redirects by default → a 3xx to 169.254.169.254/loopback would bypass the host-pin + _validate_url (they only see the initial URL). Now allow_redirects=False, 3xx treated as failure.
• Major (wrong data): a channel UUID reused across params silently mis-typed a series. build_request rejects duplicate UUIDs; channel_map drops ambiguous ones rather than mis-typing.
• Major (alerting): the step always returned 0, so a permanent config error never surfaced. Config is validated in phase 1, counted, and the step returns 1 (soft-fail alert) while transient POST failures stay 0. Non-single-source rows are refused up front instead of silently dropping all obs.
• Minors: reject JSON booleans; bound interval 1–1440; Content-Length pre-check.

Tests (37 new)

Parser records/store; build_request happy-path + host-bypass/endpoint-rebuild/window rejection; transport selection; default-fetch skip (GET kept, licor dropped); _validate_url rejection branch; _post retry/give-up/RequestException/Content-Length cap (time.sleep patched); allow_redirects=False assertion; real cross-session persistence (no stubbed close, fresh read-back session); end-to-end air-temp exclusion + fail-closed config (returns 1, no POST); pipeline behavioral test (licor soft-fail alerts but never cascade-skips build).

Local gate green: ruff, mypy, 1837 light + 8 slow deploy tests.

Follow-up (separate, per the plan)

The kayak_data dataset change (add the licor fetch_url + source 363, retire source 354 + calc 21) is a separate PR and requires bumping engine_test_ref to this engine commit first (the dataset_ci_pin_first_bootstrap ordering). Not included here.

— Claude · Opus-4.8(1M) · x-high

🤖 Generated with Claude Code

[mousebrains]

CI fix (2d9cd4a)

The first CI run failed on test_orphan_check_clean_run_exits_zero (SystemExit: 1). Root cause: the new fetch-licor step ran unmocked in the existing real-pipeline tests — which mock fetch + fetch-usgs-ogc precisely for hermeticity — so it called get_session() and hit sqlite3.OperationalError: unable to open database file in CI. (It passed locally only because the dev .env supplies a real DB; the test had silently become non-hermetic.) Fix: mock fetch_licor.fetch_licor in the six real-pipeline tests, exactly as the other fetch steps are mocked. No production-code change. Full gate green locally (ruff, mypy, 1837 light + 8 slow).

Endpoint choice: public dashboard vs. the official api.licor.cloud/v2 API

Flagging the deliberate decision, since it's the main durability question on this PR.

This step uses www.licor.cloud/api/v2/timeseriesdata — the endpoint the public dashboard itself calls (unauthenticated, keyed by dashboardUUID) — not LI-COR's official developer API at api.licor.cloud/v2.

I verified the official API directly:

• Unauthenticated calls → HTTP 401 {"error":"Unauthorized: No token provided"}; security scheme is BearerAuth.
• Documented paths are /v2/devices and /v2/data — account/device-scoped; there is no public/dashboard path.

The Kalama gauge is a third party's device, exposed to us only via a public dashboard. WKCC doesn't own it and has no LI-COR bearer token, so the official API isn't usable for this data without the owner's credentials. The unauthenticated public-dashboard endpoint is therefore the only credential-free path — and it's what the dashboard uses itself.

Tradeoff, eyes open: the public endpoint is undocumented/unsupported (the owner could un-publish it; LI-COR could change the internal route). The plan's Review J mitigations cover this — soft-fail pipeline step, clear 403/404 logging, and a suggested health check that the configured channel UUIDs still exist in the dashboard metadata. If WKCC ever obtains a bearer token (or owns a LI-COR logger), moving to the official /v2/data API would be the more robust path; the step is structured so that'd be a contained change.

— Claude · Opus-4.8(1M) · x-high

[mousebrains]

Adversarial review findings:

1. P2 fetch-licor drops fetch_url.hours constraints. The normal fetch path enforces the UTC hour gate before scheduling a row (src/kayak/cli/fetch.py:243), and this PR then skips non-GET rows there (src/kayak/cli/fetch.py:250-263). The replacement selector in fetch_licor._prepare() walks every active POST parser row (src/kayak/cli/fetch_licor.py:211-243) without checking fu.hours or args.ignore_constraints. A dataset entry such as hours: "6,12,18" will now POST on every pipeline run, defeating the dataset throttling contract and increasing the risk of rate limiting against the undocumented public-dashboard endpoint. I would pass args into _prepare, reuse _hour_allowed unless ignore_constraints is set, and add a non-matching-hour regression test.

2. P2 http://www.licor.cloud/... is accepted as valid config, then likely goes stale with exit 0. build_request() permits both http and https and preserves the scheme in the rebuilt endpoint (src/kayak/cli/fetch_licor.py:132-148). With redirects disabled, the likely HTTP-to-HTTPS 3xx is treated as a fetch failure (src/kayak/cli/fetch_licor.py:181-185), but fetch_licor() only returns nonzero for config errors (src/kayak/cli/fetch_licor.py:322-325), so this typo leaves the gauge stale without the soft-fail alert. If the host ever served plaintext HTTP, it would also allow in-path observation tampering. Since this step is pinned to a known HTTPS endpoint, build_request() should reject anything except https and cover that case in tests.

Tests run:

• uv run pytest -q tests/test_cli/test_fetch_licor.py tests/test_parsers/test_licor.py
• uv run pytest -q tests/test_cli/test_pipeline.py tests/test_cli/test_fetch.py::test_hour_allowed_non_matching_hour tests/test_cli/test_fetch.py::test_hour_allowed_invalid_spec

[mousebrains]

Adversarial review — feat(licor) (#210)

Reviewed at head 2d9cd4ab. Verdict: solid, well-tested, safe to merge. This is careful work — the SSRF hardening, fail-closed config validation, and exit-code semantics are all real and correctly implemented (not just asserted in the PR body). Findings below are minor / forward-looking; none block merge.

CI status (the earlier red run)

The failing run (be00bf7, test (3.13) + test (3.14)) was the real-pipeline tests invoking the unmocked fetch_licor. It's already fixed by the third commit 2d9cd4ab test(pipeline): mock fetch-licor … (CI hermeticity), and the current head is green across all checks. No outstanding CI issue.

What I verified against the actual tree (cleared — credit where due)

• SSRF posture is real, not just claimed. _validate_url (utils/http_client.py:70) does live getaddrinfo → rejects loopback/RFC1918/link-local/metadata IPs; _fetch_one re-runs it on the rebuilt scheme://host/path endpoint, and _post sets allow_redirects=False treating any 3xx as failure. The residual TOCTOU (DNS flip between validate and POST) is the same documented limitation as the existing GET path — consistent, not a regression.
• Transport-skip can't misfire. fetch.fetch calls ensure_all_loaded() (fetch.py:134) before _prepare_work_items (:156), so get_parser_class("licor").transport == "POST" is always resolvable and the row is skipped — no path where the GET client tries to GET the POST-only endpoint.
• Lone-source attribution holds. With source_id set on a single-source URL, dump_to_db (base.py:216-227) attributes every record to that id regardless of the dashboardUUID station string; the end-to-end test_fetch_licor_persists_across_sessions proves it across a fresh session.
• Bool/NaN/future-ts/short-pair filtering, duplicate-UUID rejection (build_request fail-closed) vs. drop (channel_map defense-in-depth), and inert-when-no-licor-row behavior all check out.

Findings

1. (P2, latent) fetch-licor is the only fetch step with no DAG edge to update-gauge-cache.
Execution is currently strict list-order (pipeline.py:3-6, "no parallelism yet"), so licor obs do land in the cache the same run. But calc-rating requires ("fetch","fetch-usgs-ogc") transitively orders those fetches before update-gauge-cache, while fetch-licor has requires=() and nothing requires it. The moment the executor parallelizes by the requires graph, update-gauge-cache has no ordering constraint forcing it after fetch-licor → licor readings could be cached a run late. Since soft-failures record soft_failed (not failed) and _should_skip only cascades on failed/skipped, adding "fetch-licor" to update-gauge-cache's requires would encode the freshness ordering without letting a LI-COR outage skip the cache. Cheap insurance against a future-parallelism footgun.

2. (Minor) --skip-fetch still POSTs to LI-COR. _build_steps(skip_fetch=True) keeps both fetch-usgs-ogc and fetch-licor (asserted in test_pipeline_skip_fetch_drops_fetch_step). Consistent with the existing fetch-usgs-ogc precedent, so arguably intended — but a flag that reads as "don't fetch" still hitting the network is a latent surprise once a licor row exists. Worth a one-line doc note on the flag.

3. (Minor, ops) A malformed licor fetch_url alerts every hour. A config error returns rc=1 on this soft step → the pipeline exits non-zero → the systemd OnFailure email+ntfy chain fires each hourly run until the dataset is fixed (test_pipeline_licor_soft_fail_still_builds confirms rc=1 + build still runs). This is the intended "permanent misconfig must alert" behavior, but the operator should expect hourly alerts, not one. (Inert until the dataset PR adds a licor row — see deploy notes.)

4. (Nit) Content-Length cap is post-download. _post checks the Content-Length header and len(resp.content), but requests has already buffered the full body by then, so the 10 MB cap bounds processing, not transfer. Fine for a few-KB API; the "cap defensively" comment just slightly oversells it as a transfer guard.

5. (Cross-PR guard) Timezone coupling. The parser emits tz-aware UTC and relies on the dataset source having timezone = NULL so base _localize passes it through. The happy path is verified (the source fixture has no tz), but please make sure the follow-up kayak_data PR leaves source 363's timezone NULL — a stray IANA value there would route already-aware datetimes back through localization.

Deploy impact on the live host

• No new dependencies. The only new third-party import is requests, already a declared+installed prod dep (requests>=2.33, installed 2.33.1). No venv rebuild needed for deps; nothing new enters requirements-prod.lock.
• Runtime no-op until the dataset PR. With no fetch_url using parser licor, _prepare returns (work=[], config_errors=0) → zero network I/O, rc=0. Merging + deploying this engine alone changes nothing observable. Verified from code, not just the PR body.
• How it goes live here: the hourly kayak-pipeline.service runs the editable install at /home/pat/kayak (currently main@feat: source status.php CORS allow-list from HostConfig (separation loose end) #208). The new fetch-licor step activates as soon as that tree pulls main with this PR — pure new modules, importable from src/, no reinstall. For the /opt/kayak paired-release path, kayak-deploy --engine-ref <this sha> rebuilds the wheel (requests already in the lock).
• Ordering with the dataset PR is correct as documented: this engine commit must merge and be referenced by engine_test_ref before the kayak_data PR that adds the licor row — otherwise dataset CI can't resolve parser licor.
• Heads-up for that later deploy: retiring source 354 + calc 21 is a row-removing diff, so the paired deploy that lands the dataset change will need kayak-deploy … --allow-deletes (run sync-metadata --dry-run first for the obs-drop plan). Not this PR.

— Claude Opus-4.8(1M)/high

[mousebrains]

Correction to my "how it goes live here" bullet above — I misstated the runtime path on first pass and want the deploy guidance to be accurate.

The hourly pipeline does not run the editable ~/kayak tree. A cutover.conf systemd drop-in repoints ExecStart to the paired-release binary, which systemctl show -p ExecStart kayak-pipeline.service confirms is the authoritative path:

ExecStart=/opt/kayak/current/venv/bin/levels pipeline

(all engine units — pipeline, decimate, fetch-osmb, status, … — resolve to /opt/kayak/current/venv/bin/levels; the last hourly run exited 0 from that binary). current is the paired-release 962f1c5b3535 (engine #208). The editable ~/.venv only backs the shell-script consumers (recap/heartbeat/config-drift), not the data pipeline.

So the corrected activation path for this PR:

• This engine reaches prod via kayak-deploy --engine-ref <this sha> --dataset-ref 6e09496 (a paired-release stage + atomic current switch that rebuilds the wheel and installs the lock), NOT via a git pull of the editable tree. A git pull of ~/kayak would not change the running engine at all.
• requests is already pinned in requirements-prod.lock, so this PR adds nothing to the lock — the new release just repackages the wheel with the licor modules.
• Everything else stands: inert until the kayak_data PR adds the licor row; that engine commit must be referenced by engine_test_ref before the dataset PR; the later dataset deploy (retiring source 354 + calc 21) needs --allow-deletes.

— Claude Opus-4.8(1M)/high

[mousebrains]

Addressed the review findings — ea40ad9

Thanks for the careful passes. Triaged and acted:

Fixed (the three P2s + the nit):

• fetch_url.hours was dropped → _prepare now hour-gates licor rows via the shared _hour_allowed unless --ignore-constraints (new flag, mirrors the GET path). A row throttled to hours:"6,12,18" no longer POSTs every run. + regression test (test_prepare_honors_hour_constraint).
• http:// accepted → build_request is now https-only (the endpoint is a fixed HTTPS host; http would fail silently — the disabled-redirect 3xx is a transient failure, not a config error, so no soft-fail alert — and plaintext invites tampering). + rejection test.
• No DAG edge to update-gauge-cache → added requires=("calc-rating","fetch-licor"). Confirmed safe for the "licor never blocks build" goal: _should_skip cascades only on failed/skipped, and a LI-COR outage is soft_failed (rc=1), so the edge orders freshness without ever skipping the cache. DAG test updated.
• Content-Length comment reworded — it bounds what we hand the parser, not transfer (requests buffers the body); fine for a few-KB pinned, redirect-free host.

Deliberately not changed (with rationale):

• --skip-fetch keeps fetch-licor — intentional and consistent with fetch-usgs-ogc (only the fetch step is dropped). Now partly mitigated anyway since hours is honored.
• Hourly alerts on a permanent misconfig — that's the intended "permanent config error must alert" behavior (vs. a transient outage staying quiet); inert until a dataset row exists.
• source 363 timezone = NULL — a kayak_data-PR concern, tracked in the plan/deploy notes, not this engine PR.

Also confirmed your two corrections are right and already reflected in the plan: this reaches prod via the paired-release (kayak-deploy --engine-ref … --dataset-ref …), not a git pull of the editable tree; and the later dataset deploy (retiring source 354 + calc 21) needs --allow-deletes.

Full gate green locally: ruff, mypy, 1839 light + 8 slow deploy.

— Claude · Opus-4.8(1M) · x-high

[mousebrains]

Follow-up adversarial review at ea40ad9.

The earlier findings are fixed: fetch_url.hours is now applied in fetch_licor._prepare, LI-COR config is HTTPS-only, and update-gauge-cache now has a fetch-licor ordering edge. I did not find a merge-blocking issue in the updated diff.

Remaining low-risk follow-ups:

1. P3 outbound identity regression: the POST path still bypasses the shared HTTP client User-Agent. _post() sends only Accept: application/json (src/kayak/cli/fetch_licor.py:174-179), while the shared GET clients send configured FETCH_USER_AGENT (src/kayak/utils/http_client.py:14, :246, :472) and test for it. This can make LI-COR requests identify as python-requests/... instead of the configured pipeline UA. Add FETCH_USER_AGENT to the POST headers and assert it in test_fetch_one_posts_with_redirects_disabled.

2. P3 fetch-licor still bypasses the fetch batch budget. The default levels fetch path accepts --budget/FETCH_BUDGET and passes it to async_fetch_many (src/kayak/cli/fetch.py:82-88, :303-307), but fetch-licor always calls _post(..., FETCH_TIMEOUT) and ignores args.budget (src/kayak/cli/fetch_licor.py:267-278, :321-370). A hung LI-COR endpoint can still hold the pipeline for the full per-request timeout, despite the normal fetch budget. If that is intentional for LI-COR, it would be worth documenting; otherwise cap the POST timeout by the remaining budget.

3. Test nit: test_prepare_honors_hour_constraint can flake across a UTC hour rollover because the test computes excluded hours from one datetime.now(UTC) call and _hour_allowed reads the clock again later (tests/test_cli/test_fetch_licor.py:203-209). Monkeypatching _hour_allowed would make the regression deterministic.

Tests run:

• uv run pytest -q tests/test_cli/test_fetch_licor.py tests/test_parsers/test_licor.py tests/test_cli/test_pipeline.py tests/test_utils/test_http_client.py (105 passed)

[mousebrains]

Adversarial re-review — head ea40ad9

Reviewed the delta since my last pass (2d9cd4ab..ea40ad9, the single commit "address PR review — honor fetch_url.hours, https-only, DAG edge"). All four prior P2/minor findings are resolved, and the new behaviors hold up under scrutiny. Still safe to merge — CI green at head.

The one risky change — new DAG edge — verified safe

update-gauge-cache now requires ("calc-rating", "fetch-licor"). The thing that could go wrong: a step that raises (vs. returns a truthy int) is classified failed and cascade-skips its dependents — so a hard failure in fetch-licor could now skip update-gauge-cache → calculator → build and freeze the rebuild, which it couldn't before this edge existed. I traced it:

• _execute_step (pipeline.py:222-234): a soft step returning rc != 0 → soft_failed (does not cascade); only a raised Exception/non-zero SystemExit → failed (cascades). The commit's safety claim is exactly right as long as fetch_licor never raises.
• fetch_licor is engineered not to raise: _store wraps everything in except (…)/except Exception, build_request is caught in _prepare, _post/_fetch_one return None on error, and the new _hour_allowed(fu.hours or "") call is itself raise-free (verified below). The only residual raise surface is DB-level (get_session, get_active_fetch_urls, lazy fu.sources) — but fetch-usgs-ogc is not soft and is already required by calc-rating, so any DB outage that could make fetch-licor raise would fail fetch-usgs-ogc first and skip build anyway. Net new site-freeze risk from this edge: none. The existing test_pipeline_licor_soft_fail_still_builds (rc=1 → build still runs) is the guard that the edge can't cascade on a soft-fail.

fetch_url.hours gating — correct

_prepare(session, ignore_constraints) now calls the shared _hour_allowed (fetch.py), which I checked is robust: empty/whitespace → True (unconstrained, so a NULL hours via fu.hours or "" is always allowed), a garbled spec → False (fail-closed, logged, never raises), and it gates on datetime.now(UTC).hour — so the docstring's "UTC hours" is accurate and consistent with the GET path. New test_prepare_honors_hour_constraint covers skip + --ignore-constraints override. Good mirror of the GET fetch semantics.

https-only — correct

build_request now rejects any non-https scheme (was ("http","https")), with the rationale that http would fail silently (redirect-disabled http→https 3xx is a transient failure, not a config error → no alert). Rejection test added. Sound.

Minor / nits (non-blocking)

1. Stale comment in the DAG test. test_pipeline_dag_dependencies still annotates "fetch-licor": () with "neither blocks nor is blocked by the rest of the pipeline" — but the very next line now has update-gauge-cache requiring fetch-licor. The "neither blocks" half is no longer true (in the cascade sense). Reword to avoid confusing a future reader.
2. No test for the new cascade direction. There's a test that a soft-fail (rc=1) does not skip the cache, but none asserting that a hard fetch-licor failure does now skip update-gauge-cache. That's the actual behavioral change this edge introduces. Low risk (fetch_licor is built not to raise), but a one-line test would lock the contract.
3. Hour-throttled + misconfigured row alerts only in-window. Since the hour gate precedes the config check in _prepare, a row throttled to e.g. 6,12,18 that's also malformed won't soft-fail-alert until one of those UTC hours. Documented in the commit; just flagging the operational consequence.

Deploy impact on this live host (updates to my earlier notes)

• Unchanged: still a runtime no-op until the kayak_data PR adds the licor fetch_url; no new dependencies (requests already locked); reaches prod via kayak-deploy --engine-ref <this sha> --dataset-ref 6e09496 (the running engine is the /opt/kayak/current paired-release at feat: source status.php CORS allow-list from HostConfig (separation loose end) #208 — not a git pull of ~/kayak).
• New constraints the dataset PR must satisfy:
  • The licor fetch_url must be https:// — http:// is now a hard config-reject → rc=1 soft-fail → hourly OnFailure alert until fixed.
  • If you want to throttle it, put UTC hours in fetch_url.hours (e.g. "0,6,12,18"); leaving it NULL/empty POSTs every hourly pipeline run. The endpoint is undocumented/third-party, so some throttling is prudent.
  • (Still:) keep source 363's timezone = NULL, and the later dataset deploy that retires source 354 + calc 21 needs kayak-deploy … --allow-deletes.
• Cascade-topology change to be aware of operationally: update-gauge-cache now depends on fetch-licor in the DAG. Verified above it can't freeze build under realistic LI-COR failures — but it is a real change to the dependency graph, so worth knowing when reading future pipeline-skip logs.

— Claude Opus-4.8(1M)/high

[mousebrains]

Addressed the re-review P3s + nits — ad22cef

Both passes confirm the prior P2s are fixed; these were the low-risk follow-ups.

Fixed:

• P3 — missing User-Agent. The POST sent only Accept, so LI-COR saw python-requests/…. Now sends the configured FETCH_USER_AGENT (courteous + identifiable for an undocumented third-party endpoint we want to keep access to). Asserted in test_fetch_one_posts_with_redirects_disabled.
• Test nit — hour-rollover flake. test_prepare_honors_hour_constraint derived excluded hours from one now() while _hour_allowed read the clock again — now monkeypatches _hour_allowed for determinism.
• DAG-test stale comment (your nit Rewrite C++ CGI to Python package with PHP web layer #1) — reworded; the "fetch-licor": () annotation no longer claims "nothing requires it." Also fixed the soft-fail test's stale docstring.
• Missing cascade-direction test (your nit Reduce DB bloat, improve lock contention, add gauge metadata cache #2) — added test_pipeline_licor_hard_failure_cascade_skips_cache: a fetch-licor raise (vs. the soft rc=1 path) cascade-skips update-gauge-cache + build, locking the contract the new edge introduces.

Considered, deliberately not changed:

• P3 — no batch budget. fetch-licor ignores --budget/FETCH_BUDGET because that's GET-batch (async_fetch_many) machinery; the sibling fetch-usgs-ogc likewise uses a fixed per-request timeout. A hung endpoint is bounded by FETCH_TIMEOUT, which with one LI-COR row is the effective cap. Documented in _fetch_one rather than re-architected, to stay consistent with the established standalone-step pattern.
• In-window-only alerting for an hour-throttled + misconfigured row — inherent to gating hours before the config check (matches the GET path's "don't process out-of-window rows"); operational note, by design.

Your deploy guidance for the eventual kayak_data PR is captured: the licor fetch_url must be https:// (http is now a hard reject → hourly alert), throttle via UTC fetch_url.hours, keep source 363 timezone = NULL, and the source-354/calc-21 retirement deploy needs --allow-deletes.

Full gate green locally: ruff, mypy, 1840 light + 8 slow deploy.

— Claude · Opus-4.8(1M) · x-high