feat/hoymiles

seed-metrics-catalog.ts credentialFields has 'token' but manifest/worker use 'api_key'

packages/database/src/seed-metrics-catalog.ts:159

The Integration seed row for hoymiles declares credentialFields: [{ name: 'token' }]. The manifest (hoymiles.ts:47) declares { name: 'api_key' } and the worker's loadCredentialFromArn reads parsed['api_key']. Any org provisioned via the UI wizard will have { token: '...' } written to their secret; the worker finds no 'api_key' field, returns MissingSecret (400), and data collection is permanently broken. The manifest is correct — fix the seed to { name: 'api_key', label: 'Hoymiles API key', secret: true } and re-run db:seed:metrics-catalog on all envs before SG-3.

CALL_BUFFER_MS=100ms is ~6.7× too fast for the 90 calls/min vendor cap

services/metrics/integrations/hoymiles/src/engine/hoymiles-client.ts:40

90 calls/min = one call every 667ms minimum. With HTTP round-trips averaging 200–400ms, the effective cycle is 300–500ms = 2–3 calls/sec = 120–180 calls/min. A 15-day intraday window makes 15 sequential power-day fetches + up to 2 range calls = 17 calls. CALL_BUFFER_MS must be raised to at least 600ms (or implement 429-triggered retry-after back-off). The UpstreamRateLimited path exists and maps correctly, so 429s are not silently lost — but they cause invocation failure for the entire plant window.

dailyRowsToBatu: empty-after-clip returns silent ok with zero points

services/metrics/integrations/hoymiles/src/translation/points-to-batu.ts:337

channel-not-found fires only when rowsWithField === 0. If vendor returns daily rows carrying the channel's Wh field but all localMidnightUtcMs anchors fall outside [fromUtc, toUtc), the function returns { ok: true, payload: { points: [] } }. This empty-success payload advances metric_coverages without writing any data — silent coverage inflation. powerDaysToBatu handles the analogous case as channel-not-found when allPoints.length === 0 after clipping. Add a guard: if (rowsWithField > 0 && points.length === 0) return channel-not-found.

loadCredentialFromArn swallows all AWS SDK errors as MissingSecret (400)

services/metrics/integrations/hoymiles/src/engine/hoymiles-client.ts:257

The bare catch { return null } converts AccessDenied, ResourceNotFoundException, network timeouts, and malformed ARNs all to null, which both handlers surface as MissingSecret (400). An IAM misconfiguration on the Lambda execution role looks identical to a legitimately absent secret. The fix: catch specific AWS exception types — ResourceNotFoundException → keep as null/MissingSecret; AccessDeniedException/UnauthorizedException → rethrow or map to a distinct CredentialAccessDenied tag so CloudWatch shows the root cause immediately.

dominantSpacingMinutes tie-break (equal vote counts → smaller delta) is undocumented

services/metrics/integrations/hoymiles/src/translation/points-to-batu.ts:177

When two spacings have equal vote counts, the smaller delta wins. In the hoymiles mixed-fleet context this is correct (5-min DTU wins), but the behavior on a genuinely ambiguous day (e.g. a 15-min plant with a firmware glitch producing equal 5/15-min spacing counts) is to report 5-min — causing a granularity-mismatch failure on a correctly provisioned 15-min source. The tie-break is safe (fails rather than silently mislabels) but worth a comment.

IAM wildcard {env}/hoymiles/* grants read on portal credentials workers never need

infra/cdk/src/stacks/services/metrics/integrations/hoymiles/iam.ts:43

The first ARN {targetEnv}/hoymiles/* covers both {env}/hoymiles/oid={oid} (API key, needed) and {env}/hoymiles/portal/oid={oid} (human portal username/password, never needed). Least-privilege scope: {env}/hoymiles/oid=* — this pattern excludes the /portal/ path without an additional statement. A compromised worker Lambda could exfiltrate human portal passwords for all orgs via a single GetSecretValue call. The over-grant is bounded to the same dev/stg/prod account.

Overly broad /key check/i regex may misclassify business rejections as auth failures

services/metrics/integrations/hoymiles/src/engine/hoymiles-client.ts:110

The condition status === '2' || /key check/i.test(message) treats any vendor message containing 'key check' as UpstreamAuthFailed — regardless of status value. A future Hoymiles business-rejection message containing that substring would trigger spurious rotation warnings. The regex was added as a defensive fallback; since status === '2' is probe-verified, the regex should be narrowed to a last-resort guard (e.g. apply only when status is absent or unrecognized) to avoid false positives.

API key in URL query param — vendor-mandated but worth noting for future egress changes

services/metrics/integrations/hoymiles/src/engine/hoymiles-client.ts:61

The Hoymiles OpenAPI requires ?key= in the query string. No current log statement leaks the URL or key (confirmed). The key is visible in: VPC Flow Logs (if a VPC NAT gateway is added), any future egress inspection layer, and Hoymiles' own server logs. The code is correct today; document this in INFRA_DESIGN.md so future infra changes (VPC egress, WAF) don't inadvertently enable key logging.

dominantSpacingMinutes: all-negative-delta code path not tested

services/metrics/integrations/hoymiles/src/__tests__/points-to-batu.test.ts

The function skips deltas <= 0 (points-to-batu.ts:170). If a vendor sends 2+ points with reversed or duplicate timestamps, all deltas are non-positive, counts stays empty, and the function returns null — bypassing cadence detection in powerDaysToBatu. This is a distinct path from the < 2 points guard. A malformed vendor response with reversed timestamps would then silently pass cadence validation instead of failing or detecting null. Add a test: dominantSpacingMinutes([{tm:'10:00'}, {tm:'09:55'}]) === null.

dailyRowsToBatu: all-rows-clipped silent success not tested

services/metrics/integrations/hoymiles/src/__tests__/points-to-batu.test.ts

No test exercises the case where rows carry the channel's Wh field (rowsWithField > 0) but all localMidnight anchors fall outside the requested [fromUtc, toUtc) window, causing empty points and a silent { ok: true, payload: { points: [] } } return. This is the asymmetric empty-success path described in the correctness finding above. Add a test: rows for 2026-06-01 requested with window 2026-07-01..2026-07-08 → the result should either error or return empty success (establishing the expected behavior).

metrics handler: fetchProductionDaily and fetchEnergyStatsDaily failure paths not directly tested

services/metrics/integrations/hoymiles/src/__tests__/metrics-handler.test.ts

Tests cover fetchPowerProductionDay auth-failure and the all-channels-dead case, but there is no dedicated test for fetchProductionDaily returning { ok: false, reason: 'timeout' } on a solar_generation@1d channel, or fetchEnergyStatsDaily failing on a grid_import channel. These are independent fetch surfaces (lines 215–231 of metrics.lambda.ts) that return early on failure. Add tests exercising each 1d fetch failure path in isolation.

enumerateLocalDays: DST fall-back transition not tested

services/metrics/integrations/hoymiles/src/__tests__/points-to-batu.test.ts

Spring-forward DST is tested (Tijuana 2026-03-08). Fall-back is not. The 36h advance trick handles fall-back correctly (an extra hour means +36h always overshoots to the next day), but a test asserting no duplicate or skipped days across America/Mexico_City's fall-back (first Sunday of November) would prevent regression if the helper is modified.

Connection handler: rate-limited and timeout GPW responses not tested

services/metrics/integrations/hoymiles/src/__tests__/connection-handler.test.ts

auth-failed, vendor-rejected, and unreachable GPW cases are tested. rate-limited and timeout are not. These map to reachable: false, credentialValid: false in probeFailurePayload — plausible in production given the 90 calls/min cap shared with the metrics worker. Add two tests: gpw returns { ok: false, reason: 'rate-limited' } → InvocationSuccess with reachable:false, credentialValid:false; and { ok: false, reason: 'timeout' } → same.

Retention boundary (startDate === oldestServed) not tested

services/metrics/integrations/hoymiles/src/__tests__/metrics-handler.test.ts

The handler uses strict less-than: if (startDate < oldestServed). A window whose startDate equals oldestServed is allowed. Tests only cover the clearly-too-old case. Add a test where the window's first local day exactly equals the oldest served day to confirm it proceeds to the fetch rather than triggering the retention rejection.

assetMetadata.plantId override path not tested in either handler

services/metrics/integrations/hoymiles/src/__tests__/metrics-handler.test.ts

Both metrics.lambda.ts:121 and connection.lambda.ts:54 support an optional params.assetMetadata.plantId override over asset.externalId — a platform-controlled nesting that mirrors eGauge's pattern. All tests use asset.externalId directly. Low risk (simple string substitution) but worth a single smoke test.

Cross-org concurrent Lambda invocations defeat per-Lambda pacing

services/metrics/integrations/hoymiles/src/handlers/metrics.lambda.ts:190

The SFN Device Collection Map fan-out runs one Lambda per (org, plant). All plants in the same org share one API key. 10 concurrent plant invocations each doing 5 paced calls = 50 near-simultaneous calls against the same key's 90/min quota. No reserved concurrency is set (lambda.stack.ts). The pacing inside one Lambda does not protect the org-level rate. At minimum this should be documented in INFRA_DESIGN.md before the fleet scales. Full mitigation: reserved concurrency of 1 per Lambda, or an org-level SQS queue with consumer rate limiting.

Timezone helpers duplicated verbatim from victron integration

services/metrics/integrations/hoymiles/src/translation/points-to-batu.ts:65

Points-to-batu.ts line 65 explicitly says 'Copied verbatim from victron's translation layer'. A third copy exists in growatt/src/engine/historical-data.ts:302. The 10+ upcoming brand ports will copy them again. These are ideal candidates for @batu/metrics-engine/lib/local-time.ts — a leaf lib export already used (claim-check.ts, correlation-id.ts). Consolidating eliminates copy-drift risk.

manifest.variables[k].externalVariableId ('e', 'i_e', 'o_e') is unused by the worker at runtime

packages/integration-manifests/src/manifests/hoymiles.ts:52

The worker routes on the outer manifest variable key (solar_generation, grid_import, grid_export) and maps to vendor fields internally via dailyWhOf — not via manifest.variables[k].externalVariableId. The field documents the legacy vendor channel name but is not dispatched through the SFN pipeline (resolveExternalVariableId stores the outer key). A brief comment clarifying this in the manifest JSDoc would prevent a future engineer from assuming the inner externalVariableId is what the worker routes on.

probeFailurePayload: vendor-rejected maps to credentialValid:true — semantically subtle

services/metrics/integrations/hoymiles/src/handlers/connection.lambda.ts:97

When status '1' (business rejection: 'No operating authority for this plant'), credentialValid is set to true — correctly reflecting the key was accepted. However credentialValid:true alongside reachable:false could be read as 'key is fine, plant is offline' when it may mean 'key is fine but this plant is not in this org's account'. The ConnectionPayload contract doesn't have a field for this distinction. Worth noting in INFRA_DESIGN.md alongside the last_at unreliability note.