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."}