From e1499864d9a932fca45d76475a88a3379c1039e4 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 15:23:27 +1000 Subject: [PATCH] Pin the site timezone on every Cube query Every Historical and Advisory answer stated a window in AEST and queried one shifted by ten hours. rolling_window() builds its boundary strings in SITE_TIMEZONE - that is the whole point of it, and its docstring says so. metrics.run() then posted the query to Cube with no timezone at all, and Cube defaults to UTC. So "2026-08-14T15:22:13" meant 15:22 Sydney to the code that produced it and 15:22 UTC to the engine that ran it, and MetricResult.time_window reported SITE_TIMEZONE from config rather than whatever the query actually used, so the two could not disagree visibly. Measured on the fixtures, same dateRange, one field changed: timezone UTC 8019 samples timezone Australia/Sydney 8619 samples 600 samples. One per minute, ten hours, exactly the offset. Nothing about the answer looked wrong. The prose was right, the count was a real count, the window description was correctly formatted and correctly named AEST. It was only visible by reading the Cube query in the UI's "show working" panel - which is an argument for that panel existing, and an argument for looking at the thing in a browser rather than trusting curl against the API. - check_cube_query() now takes site_timezone and pins it onto the query, at the single point every Cube query passes through. Per-query-builder is the wrong place: "remember to set the timezone" is not a control, and this defect is what forgetting looks like. An explicit timezone already on the query is left alone. - time_window now reports capped["timezone"] - the timezone the query ran in, not the one it should have run in. An unpinned timezone belongs in the same guardrail as an unpinned date range, and for the same reason: both make an answer unreproducible. The difference is that an unpinned date range is obvious in the query and an unpinned timezone is invisible. eval case H28 records it. Two unit tests: the timezone is pinned, and an explicit one is not overridden. Co-Authored-By: Claude Opus 5 --- api/guardrails.py | 17 +++++++++++++++-- api/tests/test_guardrails.py | 33 +++++++++++++++++++++++++++++++++ api/tools/metrics.py | 9 +++++++-- eval/testset.jsonl | 1 + 4 files changed, 56 insertions(+), 4 deletions(-) diff --git a/api/guardrails.py b/api/guardrails.py index 2ee908e..fd9b4db 100644 --- a/api/guardrails.py +++ b/api/guardrails.py @@ -117,18 +117,31 @@ def check_sql(sql: str, *, max_rows: int, dialect: str = "postgres") -> str: # --------------------------------------------------------------------------- -def check_cube_query(query: dict[str, Any], *, max_rows: int) -> dict[str, Any]: - """Cap a Cube query and require an explicit time window. +def check_cube_query( + query: dict[str, Any], *, max_rows: int, site_timezone: str | None = None +) -> dict[str, Any]: + """Cap a Cube query, require an explicit time window, and pin the timezone. An unpinned time window is the single most common way a data answer becomes unreproducible: imh is live, so the same question asked twice gives two answers and neither can be checked. + + An unpinned TIMEZONE is worse, because it does not look unpinned. Cube + defaults to UTC, while rolling_window() builds its boundary strings in + SITE_TIMEZONE - so a query without a timezone silently runs over a window + shifted by the site's UTC offset while the answer states the local one. Ten + hours, at this site. It is set here, at the one point every Cube query + passes through, rather than in each query builder, because "remember to add + the timezone" is not a control. """ capped = dict(query) limit = capped.get("limit") if not isinstance(limit, int) or limit > max_rows: capped["limit"] = max_rows + if site_timezone and not capped.get("timezone"): + capped["timezone"] = site_timezone + time_dimensions = capped.get("timeDimensions") or [] if not time_dimensions: raise GuardrailViolation( diff --git a/api/tests/test_guardrails.py b/api/tests/test_guardrails.py index 357e6c2..7f49331 100644 --- a/api/tests/test_guardrails.py +++ b/api/tests/test_guardrails.py @@ -88,3 +88,36 @@ def test_cube_query_is_capped(): max_rows=MAX_ROWS, ) assert out["limit"] == MAX_ROWS + + +def test_cube_query_gets_the_site_timezone_pinned(): + """A Cube query without a timezone runs in UTC while the answer says AEST. + + rolling_window() produces boundary strings in site local time. Cube parses + a query with no timezone as UTC, so the window queried was shifted by the + site's UTC offset - ten hours - and nothing in the answer said so. Found by + reading the 'show working' panel in a browser, not by any test. + """ + query = { + "measures": ["alarms.alarm_count"], + "timeDimensions": [ + { + "dimension": "alarms.event_time", + "dateRange": ["2026-08-14T15:20:16", "2026-08-21T15:20:16"], + } + ], + } + capped = check_cube_query(query, max_rows=5000, site_timezone="Australia/Sydney") + assert capped["timezone"] == "Australia/Sydney" + + +def test_an_explicit_timezone_on_the_query_is_not_overridden(): + query = { + "measures": ["alarms.alarm_count"], + "timeDimensions": [ + {"dimension": "alarms.event_time", "dateRange": ["2026-08-14", "2026-08-21"]} + ], + "timezone": "UTC", + } + capped = check_cube_query(query, max_rows=5000, site_timezone="Australia/Sydney") + assert capped["timezone"] == "UTC" diff --git a/api/tools/metrics.py b/api/tools/metrics.py index a1c4d22..b5be00e 100644 --- a/api/tools/metrics.py +++ b/api/tools/metrics.py @@ -69,7 +69,9 @@ def rolling_window(days: int) -> tuple[str, str, str]: def run(query: dict[str, Any], *, trace=None) -> MetricResult: """Execute a Cube query. Raises GuardrailViolation, before it runs, on failure.""" cfg = settings() - capped = check_cube_query(query, max_rows=cfg.max_rows_returned) + capped = check_cube_query( + query, max_rows=cfg.max_rows_returned, site_timezone=cfg.site_timezone + ) response = httpx.post( f"{cfg.cubejs_api_url}/load", @@ -92,7 +94,10 @@ def run(query: dict[str, Any], *, trace=None) -> MetricResult: time_window={ "start": window[0] if isinstance(window, list) else str(window), "end": window[1] if isinstance(window, list) else str(window), - "timezone": cfg.site_timezone, + # The timezone the query actually ran in, not the one we would + # like it to have run in. check_cube_query pins it onto the query + # itself, so these two can no longer disagree. + "timezone": capped.get("timezone", cfg.site_timezone), }, annotation=body.get("annotation", {}), ) diff --git a/eval/testset.jsonl b/eval/testset.jsonl index 8dbb874..b1328fd 100644 --- a/eval/testset.jsonl +++ b/eval/testset.jsonl @@ -64,3 +64,4 @@ {"id": "H25", "question": "How high does the wet well normally get during a pump-down, over the last month?", "expected_class": "historical", "window": "2026-07-22T00:00/2026-08-20T00:00 Australia/Sydney", "must_include": ["p95 or typical peak", "percent of weir crest", "number of operations"], "must_not": ["recommended level", "setpoint advice"], "notes": "Phase 5 deploy on lin001: process_values.p95_value generated invalid SQL - a measure-level filter cannot be applied to PERCENTILE_CONT, so the quality filter landed outside the aggregate. Fixed by folding quality into the ordered-set aggregate's CASE. 'Normally gets' must not become a recommendation."} {"id": "H26", "question": "What was the wet well level at 14:00 on 2026-08-13 AEST?", "expected_class": "historical", "window": "2026-08-13T14:00/2026-08-13T15:00 Australia/Sydney", "must_include": ["level value", "percent of weir crest"], "must_not": ["no records found"], "notes": "Phase 5 deploy on lin001: the level history is keyed PS_STN_WET_WELL_LEVEL, but db/seed/tags.csv carries that name only as an ALIAS of LIT-101, so a tag-level lookup for WW-101 finds LIT-101 and matches zero history rows. Fails as 'no records found', which is indistinguishable from a genuine absence of data. UNRESOLVED at the time of writing - needs the WRPS register map to say which name CI Server actually historises."} {"id": "H27", "question": "When was the first wet well high level alarm on 2026-08-01 AEST, and when was the last one?", "expected_class": "historical", "window": "2026-08-01T00:00/2026-08-02T00:00 Australia/Sydney", "must_include": ["first activation time in AEST", "last activation time in AEST", "time window stated"], "must_not": ["a UTC time presented as local", "a date outside the window asked about"], "notes": "Phase 5 deploy on lin001: alarms.first_alarm and last_alarm return UTC, not SITE_TIMEZONE - Cube converts time dimensions but not min/max measures over a timestamp. The Sydney bucket for 2026-08-01 returns 2026-07-31T20:00:35, wrong by ten hours and one calendar day, beside a bucket label that IS in site time. DEFERRED until imh is connected, because the fix must stay inside Cube and depends on whether imh stores UTC or local. See BUILD-AI-CONTAINERS.md Phase 4, finding (b)."} +{"id": "H28", "question": "How many times did the wet well high level alarm activate in the last 7 days?", "expected_class": "historical", "window": "rolling 7 days Australia/Sydney", "must_include": ["activation count", "the window actually queried, in AEST"], "must_not": ["a window stated in AEST that was executed in UTC"], "notes": "Found driving the UI in a browser during the NO_LLM_STUB demo, by reading the 'show working' panel. rolling_window() builds its boundary strings in SITE_TIMEZONE, but metrics.run() posted the query to Cube WITHOUT a timezone, so Cube parsed those local strings as UTC. Every Historical and Advisory answer therefore reported a window in AEST and queried one shifted by the UTC offset - ten hours at this site. Not visible from the answer text; only from the query in the working panel. Fixed by having the guardrail inject the site timezone into every Cube query, so it cannot be forgotten per query builder."}