Skip to content

Commit 0d8537e

Browse files
THUNDER PHUMclaude
andcommitted
fix(cron): observation time = oldest source that actually contributed a stored field (codex P2)
- getLatestOpen tracks per-field provenance: fromInverter if any realtime field (gen/use/grid/batt/soc/genTotal) was overridden, fromStation if any is still station-sourced; observedAtOf() = min over contributing sources, null if a contributor has no timestamp; a device snapshot that overrides nothing no longer vouches for station values - 5 tests (110 total) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 parent df3e4c2 commit 0d8537e

4 files changed

Lines changed: 63 additions & 3 deletions

File tree

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
# Review df3e4c2 — HOLD (1 remaining P2)
2+
3+
## P2: Do not replace the bundle timestamp when station fields still contribute
4+
5+
Location: `src/worker/deye.ts:410-418` (especially line 412).
6+
7+
`getInverterFlow()` returns a truthy object for ANY nonempty `dataList`, even when every mapped power/SOC field is undefined. `out.observedAt = inv.observedAt` is unconditional, while the value overrides are conditional. Therefore `observedAt` does not yet describe the values actually stored when measure points are absent/partial/unmapped.
8+
9+
Actual `getLatest` -> `pollAndStore` -> SQLite reproduction at 2026-09-22 17:20 BKK, station reading four days old and `generationPower=123`:
10+
11+
- Device timestamp fresh, `dataList=[{key:'SOC',value:75}]`: `observedAt=now`, `genPower=123` from the stale station, current sample written, `frozen_since` cleared.
12+
- Device timestamp fresh, only `BatteryRatedCapacity=400`: same stale 123 W written as current despite **no value override at all**.
13+
- Conversely fresh station, only `BatteryRatedCapacity=400` and absent device timestamp: `observedAt=null`, drops valid station sample although the device contributes none of its values.
14+
15+
Retain station provenance when no mapped field contributes. With partial overrides, freshness must cover every source still contributing persisted realtime fields (e.g. source set / conservative oldest usable timestamp, unknown if a contributing source lacks time), or avoid mixing stale fallback fields. Merely checking that *one* mapped inverter field is present is insufficient; the SOC-only case above still fails.
16+
17+
Please add source-composition regression cases for no mapped override and partial override in both age directions, alongside the complete override tests.
18+
19+
## Verified fixes and controls
20+
21+
- All original full-override reproductions now pass: fresh/missing station + stale inverter rejected; stale station + fresh complete inverter accepted; first-deploy old observation and advancing-but-old replay rejected; millisecond normalization and missing timestamp rejected correctly.
22+
- Normal freeze/recovery and actual Hono purge authorization, dry-run, inclusive range, cache invalidation and daily preservation still pass.
23+
- Fresh 105/105 tests (14 files), typecheck, build and diff-check pass.
24+
- No app source/production changes. Harness extended in `.summary/review-088cb16.cjs`; it runs the actual current main source with fake upstream and in-memory SQLite, no external network.

‎src/lib/__tests__/freeze.test.ts‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, it, expect } from "vitest";
2-
import { isFrozenReading } from "../freeze";
2+
import { isFrozenReading, observedAtOf } from "../freeze";
33

44
const NOW = 1_790_000_000, STALE = 720;
55
describe("isFrozenReading — freshness of the value source, independent of monotonicity", () => {
@@ -14,3 +14,16 @@ describe("isFrozenReading — freshness of the value source, independent of mono
1414
expect(isFrozenReading(0, NOW, STALE).frozen).toBe(true);
1515
});
1616
});
17+
18+
describe("observedAtOf — oldest contributing source bounds freshness", () => {
19+
const S = 1_790_000_000, D = 1_789_650_000; // station fresh, device 4 days old
20+
it("full inverter override → device time only", () => expect(observedAtOf([{ used: false, ts: S }, { used: true, ts: D }])).toBe(D));
21+
it("no override (device contributed nothing) → station time only", () => expect(observedAtOf([{ used: true, ts: S }, { used: false, ts: null }])).toBe(S));
22+
it("partial override → the OLDER of both (4-day-old device drags a fresh station record to stale)", () => {
23+
expect(observedAtOf([{ used: true, ts: S }, { used: true, ts: D }])).toBe(D);
24+
});
25+
it("a contributing source without a timestamp → unprovable (null)", () => {
26+
expect(observedAtOf([{ used: true, ts: S }, { used: true, ts: null }])).toBeNull();
27+
});
28+
it("nothing contributed → null", () => expect(observedAtOf([{ used: false, ts: S }])).toBeNull());
29+
});

‎src/lib/freeze.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,14 @@ export function isFrozenReading(observedAt: number | null, nowS: number, staleS:
1616
if (age > staleS) return { frozen: true, reason: `reading observed ${Math.round(age / 60)} min ago (${new Date(observedAt * 1000).toISOString()})` };
1717
return { frozen: false, reason: null };
1818
}
19+
20+
// Observation time of a record assembled from several sources: the OLDEST
21+
// contributing source bounds its freshness, and one contributor without a
22+
// timestamp makes the whole record unprovable (null). Non-contributing sources
23+
// (a device snapshot that overrode nothing) are ignored.
24+
export function observedAtOf(contributors: { used: boolean; ts: number | null }[]): number | null {
25+
const used = contributors.filter((c) => c.used);
26+
if (!used.length) return null;
27+
if (used.some((c) => c.ts == null || !Number.isFinite(c.ts) || c.ts <= 0)) return null;
28+
return Math.min(...used.map((c) => c.ts as number));
29+
}

‎src/worker/deye.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,8 @@ export async function getStationMeta(env: Env): Promise<Station> {
331331
return picked;
332332
}
333333

334+
import { observedAtOf } from "../lib/freeze";
335+
334336
// Open API is the only data source.
335337
export async function getLatest(env: Env, stationId?: string): Promise<Latest> {
336338
return getLatestOpen(env, stationId);
@@ -407,16 +409,26 @@ async function getLatestOpen(env: Env, stationId?: string): Promise<Latest> {
407409
// (verified live): battery −=charge/+=discharge, grid +=import/−=export.
408410
try {
409411
const inv = await getInverterFlow(env, id);
412+
// Each stored realtime field keeps a provenance flag: overridden by the
413+
// inverter or left as the station value. The record's observation time is
414+
// the OLDEST source that actually contributed a field (lib/freeze.ts) — a
415+
// device snapshot that overrides nothing does not vouch for station values.
416+
const realtime: (keyof typeof out)[] = ["genPower", "usePower", "gridPower", "battPower", "soc", "genTotal"];
417+
let fromInverter = false;
410418
if (inv) {
411-
// the stored power values now come from the inverter → so must the observation time
412-
out.observedAt = inv.observedAt;
413419
if (inv.genPower != null) out.genPower = inv.genPower;
414420
if (inv.usePower != null) out.usePower = inv.usePower;
415421
if (inv.gridPower != null) { out.gridPower = inv.gridPower; out.gridStatus = inv.gridPower >= 0 ? "PURCHASE" : "REVERSE"; }
416422
if (inv.battPower != null) { out.battPower = inv.battPower; out.battStatus = inv.battPower > 20 ? "DISCHARGE" : inv.battPower < -20 ? "CHARGE" : "STATIC"; }
417423
if (inv.soc != null) { out.soc = inv.soc; out.socKnown = true; }
418424
if (inv.genTotal != null) out.genTotal = inv.genTotal; // lifetime kWh (not in station API)
425+
fromInverter = realtime.some((k) => (inv as any)[k] != null);
419426
}
427+
const fromStation = !inv || realtime.some((k) => (inv as any)[k] == null); // at least one field still station-sourced
428+
out.observedAt = observedAtOf([
429+
{ used: fromStation, ts: toSec(d.lastUpdateTime) },
430+
{ used: fromInverter, ts: inv ? inv.observedAt : null },
431+
]);
420432
} catch {}
421433
return out;
422434
}

0 commit comments

Comments
 (0)