Skip to content

Commit df3e4c2

Browse files
THUNDER PHUMclaude
andcommitted
fix(cron): freshness gate uses the value source's own observation time, judged on age alone (codex P2 x2)
- Latest.observedAt = inverter collectionTime when the inverter's live points override the station fields, else station lastUpdateTime (toSec normalises s/ms); null when the source gave none — updatedAt stays display-only - freeze.ts: frozen = observedAt missing OR now-observedAt > STALE_AFTER_S; no monotonic marker (a 4-day-old reading that advances 5 min is still 4 days old); last_reading_ts dropped - tests updated (105 total) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 088cb16 commit df3e4c2

6 files changed

Lines changed: 86 additions & 30 deletions

File tree

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
# Review 088cb16 — HOLD (2 P2)
2+
3+
Reviewed incident guard, health reporting, operator purge and adjacent ingestion/recovery paths. No application source, production data, branch, deploy or purge was changed. Existing main untracked `.mcp.json.bak-httptest` preserved.
4+
5+
## P2: Gate on the timestamp of the data actually persisted
6+
7+
Location: `src/worker/index.ts:167` (integration with `src/worker/deye.ts:390-408`).
8+
9+
`Latest.updatedAt` always comes from station `lastUpdateTime`, or server `now` when absent. But `getLatestOpen` then overwrites watts/SOC with `/device/latest` values and never carries their `collectionTime` into that timestamp. The new guard therefore checks a different data source from the measurements it admits/rejects.
10+
11+
Actual `getLatest` -> `pollAndStore` -> in-memory SQLite reproductions at 2026-09-22 17:20 BKK:
12+
13+
- Station timestamp absent; inverter timestamp four days old with 2,631 W: writes a current `samples` row and `device_samples` row, deletes `frozen_since`, health healthy.
14+
- Fresh station timestamp; same four-day-old inverter: same fabricated current rows and false recovery.
15+
- Four-day-old station timestamp equal to marker; fresh inverter reports 4,000 W: drops the genuinely fresh sample and telemetry, remains frozen.
16+
17+
Carry trustworthy observation provenance from the selected source(s), retaining unknown as unknown instead of substituting poll time for freshness evidence. If values can be mixed between sources, do not label the whole bundle fresh merely because one timestamp is fresh. Test both divergence directions and station timestamp absent. Normalize timestamp units at the provenance boundary; a synthetic millisecond station timestamp also bypassed the current seconds-based age guard.
18+
19+
## P2: Reject known-stale observations even without a prior marker or when they advance
20+
21+
Location: `src/lib/freeze.ts:13`.
22+
23+
Known age >720 seconds is not sufficient to reject: rejection additionally requires a non-null stored marker and `readingTs <= lastStoredReadingTs`. Consequently:
24+
25+
- First poll after deploying this new metadata key accepts a four-day-old reading, stamps it as current in `samples`/`device_samples`, and clears frozen state.
26+
- A delayed replay that advances from four days old to four days old +5 minutes also writes current rows and clears frozen state. A stream of delayed observations can continue doing this on each tick without ever being fresh.
27+
28+
Both reproduced through the real worker and SQLite, not just the helper. Treat freshness separately from monotonic advancement/deduplication: a known-old observation cannot be written under today's poll timestamp or prove recovery. If historical ingestion is desired, use the original observation time in an explicit history path. Test missing marker + stale observation, advancing-but-stale observation, and genuinely fresh recovery.
29+
30+
## Verified / non-blocking
31+
32+
- Fresh run: 105/105 tests (14 files), typecheck, build, and `git diff --check` passed. Existing tests cover only the helper, not source composition or the purge route.
33+
- Established frozen station+device timestamps: no sample or telemetry written; repeated frozen ticks keep the first `frozen_since`; `_health` remains collector-healthy. Genuinely fresh recovery clears the marker and writes.
34+
- Self-heal is reached on genuine recovery. Existing cap is **2 days**, so a four-day gap needs explicit older-history backfill if wanted. It did not fabricate frames when the mocked history was empty.
35+
- Actual Hono purge endpoint: no login ->401; no configured PIN ->403; reversed or >31-day range ->400; default dry-run leaves data untouched; confirmed purge deletes only inclusive boundaries in both tables. Day/battery caches cleared; daily and year cache preserved.
36+
- No production purge executed. After deploying a corrected admission policy, dry-run and confirm exact incident bounds; especially preserve the last valid reading / first real recovery sample if endpoints are inclusive.
37+
38+
Reproducer: `/home/l22/Desktop/deyecloud/.tmx-worktrees/codex/.summary/review-088cb16.cjs` (run with Node; transpiles actual TypeScript in memory, intercepts upstream requests, uses SQLite `:memory:`; external network forbidden).

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,15 @@ import { describe, it, expect } from "vitest";
22
import { isFrozenReading } from "../freeze";
33

44
const NOW = 1_790_000_000, STALE = 720;
5-
describe("isFrozenReading", () => {
6-
it("first ever poll: nothing stored → not frozen", () => expect(isFrozenReading(NOW - 60, null, NOW, STALE).frozen).toBe(false));
7-
it("reading advanced → not frozen even if old-ish", () => expect(isFrozenReading(NOW - 600, NOW - 900, NOW, STALE).frozen).toBe(false));
8-
it("same reading, still within the stale window → not frozen yet (one slow upload is fine)", () => {
9-
expect(isFrozenReading(NOW - 600, NOW - 600, NOW, STALE).frozen).toBe(false);
5+
describe("isFrozenReading — freshness of the value source, independent of monotonicity", () => {
6+
it("fresh reading → stored", () => expect(isFrozenReading(NOW - 60, NOW, STALE).frozen).toBe(false));
7+
it("exactly at the window edge → still stored", () => expect(isFrozenReading(NOW - STALE, NOW, STALE).frozen).toBe(false));
8+
it("4-day-old reading → frozen, even if it 'advanced' by 5 min since last poll", () => {
9+
const r = isFrozenReading(NOW - 4 * 86400 + 300, NOW, STALE);
10+
expect(r.frozen).toBe(true); expect(r.reason).toContain("min ago");
1011
});
11-
it("same reading and older than the window → FROZEN", () => {
12-
const r = isFrozenReading(NOW - 3600, NOW - 3600, NOW, STALE);
13-
expect(r.frozen).toBe(true); expect(r.reason).toContain("60 min");
12+
it("no observation timestamp → cannot prove freshness → frozen", () => {
13+
expect(isFrozenReading(null, NOW, STALE).frozen).toBe(true);
14+
expect(isFrozenReading(0, NOW, STALE).frozen).toBe(true);
1415
});
15-
it("no device timestamp → cannot judge → not frozen", () => expect(isFrozenReading(0, NOW - 3600, NOW, STALE).frozen).toBe(false));
1616
});

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, it, expect } from "vitest";
2-
import { pointValue } from "../../worker/deye";
2+
import { pointValue, toSec } from "../../worker/deye";
33

44
describe("pointValue — a reading is a finite number, nothing else", () => {
55
it("absent forms → undefined (never 0)", () => {
@@ -10,3 +10,11 @@ describe("pointValue — a reading is a finite number, nothing else", () => {
1010
expect(pointValue("79")).toBe(79); expect(pointValue(" -107.8 ")).toBeCloseTo(-107.8);
1111
});
1212
});
13+
14+
describe("toSec — Deye timestamps in s or ms, absent → null", () => {
15+
it("normalises", () => {
16+
expect(toSec(1_790_000_000)).toBe(1_790_000_000);
17+
expect(toSec("1790000000123")).toBe(1_790_000_000);
18+
expect(toSec("")).toBeNull(); expect(toSec(0)).toBeNull(); expect(toSec(undefined)).toBeNull();
19+
});
20+
});

‎src/lib/freeze.ts‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
1-
// Frozen-reading detection — pure, unit-tested.
1+
// Reading-freshness gate — pure, unit-tested.
22
//
3-
// When the site's data logger drops off Deye Cloud, /station/latest keeps
4-
// answering with the LAST reading it has (same watts, same SOC, same
5-
// lastUpdateTime) for hours or days. Storing that every 5 minutes fabricates a
6-
// flat 4-day "production curve" (observed 2026-09-18→22: PV 2,631 W × 96 h).
7-
// A reading is only new evidence if its device timestamp advanced past the one
8-
// we last stored; if it hasn't and it is older than the stale window, the poll
9-
// carries no information about the present and must not be written as a sample.
3+
// When the site's data logger drops off Deye Cloud, /station/latest and
4+
// /device/latest keep answering with the LAST reading they have (same watts,
5+
// same SOC, same timestamp) for hours or days. Storing that every 5 minutes
6+
// fabricates a flat multi-day "production curve" (observed 2026-09-18→22:
7+
// PV 2,631 W × 96 h). A reading is evidence about NOW only if the timestamp of
8+
// the source that produced its values is inside the stale window — freshness is
9+
// judged on its own, never on whether the timestamp moved since last time (a
10+
// 4-day-old reading that "advances" by 5 min is still 4 days old), and a missing
11+
// timestamp cannot prove freshness at all.
1012
export interface FreezeCheck { frozen: boolean; reason: string | null }
11-
export function isFrozenReading(readingTs: number, lastStoredReadingTs: number | null, nowS: number, staleS: number): FreezeCheck {
12-
if (!Number.isFinite(readingTs) || readingTs <= 0) return { frozen: false, reason: null }; // no device timestamp → cannot judge, keep old behaviour
13-
if (lastStoredReadingTs != null && readingTs <= lastStoredReadingTs && nowS - readingTs > staleS) {
14-
return { frozen: true, reason: `Deye reading unchanged since ${new Date(readingTs * 1000).toISOString()} (${Math.round((nowS - readingTs) / 60)} min)` };
15-
}
13+
export function isFrozenReading(observedAt: number | null, nowS: number, staleS: number): FreezeCheck {
14+
if (observedAt == null || !Number.isFinite(observedAt) || observedAt <= 0) return { frozen: true, reason: "source gave no observation timestamp" };
15+
const age = nowS - observedAt;
16+
if (age > staleS) return { frozen: true, reason: `reading observed ${Math.round(age / 60)} min ago (${new Date(observedAt * 1000).toISOString()})` };
1617
return { frozen: false, reason: null };
1718
}

‎src/worker/deye.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,8 @@ export async function getStationId(env: Env): Promise<string> {
267267
// A measure-point / field value is a reading only if it is a finite number.
268268
// null, "", " ", "N/A" are ABSENT — Number("") is 0, which is how a missing SOC
269269
// used to become a fake 0 % reading.
270+
// Deye timestamps arrive in seconds or milliseconds depending on endpoint.
271+
export const toSec = (t: unknown): number | null => { const v = pointValue(t); if (v == null || v <= 0) return null; return v > 1e12 ? Math.floor(v / 1000) : Math.floor(v); };
270272
export function pointValue(raw: unknown): number | undefined {
271273
if (raw == null) return undefined;
272274
if (typeof raw === "string" && raw.trim() === "") return undefined;
@@ -282,6 +284,11 @@ export interface Latest {
282284
// false when Deye's day-history call failed: the *Today fields are then unknown
283285
// (0 placeholders) and must not be persisted as the day's totals.
284286
totalsOk: boolean;
287+
// Observation time (unix s) of the SOURCE whose power values this record
288+
// carries: the inverter's collectionTime when its live points overrode the
289+
// station fields, else the station's lastUpdateTime. null = the source gave no
290+
// usable timestamp — the reading's freshness is then unprovable.
291+
observedAt: number | null;
285292
// false when neither the station nor the inverter reported SOC — soc is then a
286293
// 0 placeholder for the UI and must be persisted as NULL, never as a reading.
287294
socKnown: boolean;
@@ -387,7 +394,8 @@ async function getLatestOpen(env: Env, stationId?: string): Promise<Latest> {
387394
warningStatus: "NORMAL",
388395
totalsOk,
389396
selfSufficiency: selfSuff,
390-
updatedAt: n(d.lastUpdateTime) || Math.floor(Date.now() / 1000),
397+
updatedAt: n(d.lastUpdateTime) || Math.floor(Date.now() / 1000), // display only — never a freshness proof
398+
observedAt: toSec(d.lastUpdateTime),
391399
raw: d,
392400
};
393401

@@ -400,6 +408,8 @@ async function getLatestOpen(env: Env, stationId?: string): Promise<Latest> {
400408
try {
401409
const inv = await getInverterFlow(env, id);
402410
if (inv) {
411+
// the stored power values now come from the inverter → so must the observation time
412+
out.observedAt = inv.observedAt;
403413
if (inv.genPower != null) out.genPower = inv.genPower;
404414
if (inv.usePower != null) out.usePower = inv.usePower;
405415
if (inv.gridPower != null) { out.gridPower = inv.gridPower; out.gridStatus = inv.gridPower >= 0 ? "PURCHASE" : "REVERSE"; }
@@ -429,7 +439,7 @@ async function getInverterSn(env: Env, stationId: string): Promise<string> {
429439
async function getInverterFlow(
430440
env: Env,
431441
stationId: string
432-
): Promise<{ genPower?: number; usePower?: number; gridPower?: number; battPower?: number; soc?: number; genTotal?: number } | null> {
442+
): Promise<{ genPower?: number; usePower?: number; gridPower?: number; battPower?: number; soc?: number; genTotal?: number; observedAt: number | null } | null> {
433443
const sn = await getInverterSn(env, stationId);
434444
if (!sn) return null;
435445
const res = await deviceLatest(env, [sn]);
@@ -443,6 +453,7 @@ async function getInverterFlow(
443453
battPower: num("BatteryPower"),
444454
soc: num("SOC"),
445455
genTotal: num("TotalActiveProduction"),
456+
observedAt: toSec(((res.deviceDataList && res.deviceDataList[0]) || {}).collectionTime),
446457
};
447458
}
448459

‎src/worker/index.ts‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -163,13 +163,12 @@ async function pollAndStore(env: Env) {
163163
const day = bkkDay();
164164
// Frozen reading (logger offline, Deye re-serving its last value): record that
165165
// the poll itself succeeded, but write NO sample/daily — see lib/freeze.ts.
166-
const lastReading = (await env.DB.prepare("SELECT v FROM meta WHERE k='last_reading_ts'").first()) as { v: string } | null;
167-
const fz = isFrozenReading(Number(l.updatedAt) || 0, lastReading ? Number(lastReading.v) : null, Math.floor(Date.now() / 1000), STALE_AFTER_S);
166+
const fz = isFrozenReading(l.observedAt, Math.floor(Date.now() / 1000), STALE_AFTER_S);
168167
if (fz.frozen) {
169-
console.warn("poll: frozen reading — not stored:", fz.reason);
168+
console.warn("poll: stale/frozen reading — not stored:", fz.reason);
170169
await env.DB.batch([
171170
env.DB.prepare("INSERT INTO meta (k,v) VALUES ('last_poll_ok',?) ON CONFLICT(k) DO UPDATE SET v=excluded.v").bind(String(ts)),
172-
env.DB.prepare("INSERT INTO meta (k,v) VALUES ('frozen_since',?) ON CONFLICT(k) DO UPDATE SET v=v").bind(String(l.updatedAt)), // keep the FIRST frozen timestamp
171+
env.DB.prepare("INSERT INTO meta (k,v) VALUES ('frozen_since',?) ON CONFLICT(k) DO UPDATE SET v=v").bind(String(l.observedAt ?? ts)), // keep the FIRST frozen timestamp
173172
env.DB.prepare("DELETE FROM meta WHERE k='last_poll_error'"),
174173
]);
175174
// Alerts still run so the offline rule can fire from the device snapshot.
@@ -210,7 +209,6 @@ async function pollAndStore(env: Env) {
210209
env.DB.prepare("DELETE FROM meta WHERE k='last_poll_error'"),
211210
env.DB.prepare("DELETE FROM meta WHERE k='frozen_since'"),
212211
env.DB.prepare("INSERT INTO meta (k,v) VALUES ('last_poll_ok',?) ON CONFLICT(k) DO UPDATE SET v=excluded.v").bind(String(ts)),
213-
env.DB.prepare("INSERT INTO meta (k,v) VALUES ('last_reading_ts',?) ON CONFLICT(k) DO UPDATE SET v=excluded.v").bind(String(Number(l.updatedAt) || ts)),
214212
];
215213
if (l.totalsOk) stmts.push(
216214
env.DB.prepare(

0 commit comments

Comments
 (0)