feat(data-warehouse): warn when querying stale or failed warehouse syncs#59436
Conversation
|
🎭 Playwright didn't run on this PR — your changes touch code that could affect E2E behavior, but Playwright is opt-in via label now to keep CI cost down. Add the Most PRs don't need this. Real regressions still get caught on master and fix-forward. |
|
Size Change: +6.09 kB (+0.01%) Total Size: 80 MB 📦 View Changed
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Pull request overview
This PR adds end-to-end “data warehouse sync health” warnings to HogQL query responses, so queries that reference warehouse tables backed by failed/paused/billing-limited/stale syncs can surface user-visible warnings across the SQL editor, MCP tools, and Max AI.
Changes:
- Introduces a backend helper to detect unhealthy/stale
ExternalDataSchemasync states and emit structuredDataWarehouseSyncWarningobjects. - Extends the HogQL query response schema to include
warnings, and plumbs warnings through HogQL resolution/execution into API responses (and caches). - Surfaces warnings in the SQL editor UI and forwards/prefixes them in MCP + HogAI/LLM-facing formatting.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| services/mcp/src/tools/query/run.ts | Forwards HogQL warnings from the query API response through the query-run tool output. |
| services/mcp/src/api/generated.ts | Updates generated API types to include DataWarehouseSyncWarning and warnings on HogQL responses. |
| services/mcp/src/api/client.ts | Adds MCP client-side types for warnings on the query endpoint response. |
| products/product_analytics/frontend/generated/api.schemas.ts | Updates generated frontend API schema types to include warnings on HogQL responses. |
| products/data_warehouse/backend/test/test_sync_status.py | Adds parameterized backend tests covering warning trigger conditions and negative cases. |
| products/data_warehouse/backend/sync_status.py | New warning computation helper (get_warehouse_sync_warnings) for warehouse tables/schemas. |
| posthog/schema.py | Adds DataWarehouseSyncWarning schema model and warnings field on HogQL query response models (incl cached variants). |
| posthog/hogql/resolver.py | Records precomputed warehouse sync warnings into the HogQL context when resolving warehouse tables. |
| posthog/hogql/query.py | Clears warning state between executions and attaches collected warnings to HogQLQueryResponse. |
| posthog/hogql/database/database.py | Precomputes warnings per warehouse table during table loading; optimizes schema preloading with select_related('source'). |
| posthog/hogql/context.py | Adds storage + dedupe helper for collected warehouse sync warnings in HogQLContext. |
| frontend/src/scenes/data-warehouse/editor/OutputPane.tsx | Displays a warning banner in the SQL editor output pane when response.warnings is present. |
| frontend/src/queries/schema/schema-general.ts | Extends TypeScript query schema types to include warnings on HogQLQueryResponse. |
| frontend/src/queries/schema.json | Updates JSON schema to include DataWarehouseSyncWarning and warnings on HogQL responses. |
| ee/hogai/context/insight/query_executor.py | Prepends a warnings block to LLM-formatted outputs when warehouse sync warnings exist. |
| ee/hogai/context/insight/format/init.py | Adds format_warehouse_sync_warnings helper and integrates it into LLM formatting paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
products/data_warehouse/backend/sync_status.py:40
The fallback DB query is missing `select_related("source")`. In `_build_warning_for_schema`, `schema.source.source_type` is accessed whenever `schema.source_id` is truthy — without `select_related`, this triggers a separate DB query per schema (N+1). The preloaded path (set up by `_preload_active_external_data_schemas` in `database.py`) already uses `select_related("source")`, but any caller that skips preloading will hit the extra queries on every non-`unknown` source type.
```suggestion
return list(ExternalDataSchema.objects.filter(_NOT_DELETED, table_id=warehouse_table.id).select_related("source"))
```
### Issue 2 of 3
products/data_warehouse/backend/sync_status.py:83-86
When `should_sync=False` but `status` is not `PAUSED` (e.g., `COMPLETED`), `build(message)` is called with `status=status or ExternalDataSchema.Status.PAUSED`. Since `"Completed"` is truthy, the `or` short-circuits and the returned `DataWarehouseSyncWarning.status` is `"Completed"` — while the message says "is paused". Consumers (e.g., MCP clients) that parse the structured `status` field will see a contradiction. The test `test_should_not_sync_treated_as_paused` only checks `warning.message`, so this inconsistency isn't caught.
```suggestion
if status == ExternalDataSchema.Status.PAUSED or not schema.should_sync:
return DataWarehouseSyncWarning(
table_name=table_name,
schema_name=schema.name,
source_type=source_type,
status=ExternalDataSchema.Status.PAUSED,
message=f"Sync of `{table_name}` (from {source_type}) is paused. Results reflect the last successful sync.",
)
```
### Issue 3 of 3
products/data_warehouse/backend/sync_status.py:67-68
`latest_error` is appended verbatim to both the user-visible `LemonBanner` in the SQL editor and to LLM-facing outputs (Max AI, MCP). Sync job errors can include internal stack traces, database hostnames, or other infrastructure details that are appropriate for an audit log but not for surfacing in a UI banner or LLM context. Consider truncating or omitting `latest_error` here and instead directing users to the warehouse management screen where they already have access to the full error.
Reviews (1): Last reviewed commit: "chore(data-warehouse): address review fe..." | Re-trigger Greptile |
PR overviewData warehouse sync warnings added to query resultsThis PR adds generic stale/failed warehouse sync warnings to HogQL responses and surfaces them in AI/MCP formatting and the SQL editor. I checked the backend warning construction, tenant-scoped table resolution, and rendering paths; the warning messages avoid raw sync errors and are rendered as text in the frontend. Security review
Risk: 2/10 |
mariusandra
left a comment
There was a problem hiding this comment.
Cool stuff, though some AI comments to review before we can tick it off
| /** Query metadata output */ | ||
| metadata?: HogQLMetadataResponse | ||
| /** | ||
| * Warnings about data warehouse sources referenced by the query whose latest sync failed, | ||
| * is paused, hit a billing limit, or is otherwise stale. Results may not reflect current source data. | ||
| */ | ||
| warnings?: DataWarehouseSyncWarning[] |
There was a problem hiding this comment.
Should these be their own top level response field, or somehow part of the metadata response object?
There was a problem hiding this comment.
Fixed in #60058, promoted these to AnalyticsQueryResponseBase (not metadata, which is debug-only) so Trends/Funnels/etc backed by warehouse tables can get warnings too
This stack of pull requests is managed by Graphite. Learn more about stacking. |
c6c0b9d to
6bce634
Compare
Surface a warning whenever a HogQL query references a data warehouse table whose latest sync is failed, paused, billing-limit-reached, billing-limit-too-low, or running but stale (last_synced_at older than 2x sync_frequency_interval). Adds a new top-level `warnings: DataWarehouseSyncWarning[]` field to HogQLQueryResponse, populated during table resolution and surfaced in the SQL editor (yellow banner above results), MCP execute-sql output (text prefix), MCP query-run output (structured field), and Max AI compressed results.
- Reset HogQLContext.data_warehouse_sync_warnings at start of each
HogQLQueryExecutor._prepare_execution to prevent warning bleed between
executions that share a context.
- sync_status.py: replace hand-rolled _format_relative with humanize.naturaltime,
inline _active_external_data_schemas to break the circular import with
database.py, drop redundant str(status) wrapping, require now as a kwarg.
- database.py: hoist datetime.now(UTC) out of the per-table loop; add
select_related("source") to _preload_active_external_data_schemas so the
warning builder doesn't lazy-load ExternalDataSource per unhealthy schema.
- resolver.py: rename _emit_warehouse_sync_warnings to
_record_warehouse_sync_warnings — it copies, not emits.
- format/__init__.py: move format_warehouse_sync_warnings call inside the
success branch to avoid running it when the query type is unsupported.
- services/mcp: declare DataWarehouseSyncWarning and QueryEndpointResponse
once in api/client.ts; import the shared type in tools/query/run.ts.
Collapse the duplicate ...(warnings && warnings.length > 0 ? ... : {})
spread into a single warningsField const.
…ings
- Security: stop inlining ExternalDataSchema.latest_error into the warning
message. Raw error text can carry DB hostnames/credentials/stack traces and
the message reaches LLM contexts via MCP/Max; it also opened a prompt-injection
vector through attacker-controlled source errors. The message now points to the
data warehouse source screen, which is access-scoped.
- Correctness: when should_sync=False but the schema status is e.g. Completed,
the structured warning status now reports Paused, consistent with the message.
build() takes an explicit status per branch instead of falling back to the raw
schema status.
- Performance: add select_related("source") to the non-preloaded
_active_external_data_schemas query to avoid an N+1 on schema.source.
- MCP: QueryEndpointResponse.warnings now allows null, matching what the backend
serializes for responses with no warnings.
6bce634 to
360c99d
Compare

Problem
When a HogQL query references a data warehouse table whose latest sync failed, hit a billing limit, was paused, or has fallen behind its schedule, the user silently gets stale (or empty) results with no indication. This is true across the SQL editor, MCP (
execute-sql/query-run), and Max AI, so a stale Postgres or Stripe source can quietly skew any query that touches it.Changes
Detects unhealthy warehouse syncs during HogQL table resolution and surfaces a structured warning to every query consumer.
Tested with a Postgres DB I had lying around from a side project:
products/data_warehouse/backend/sync_status.py:get_warehouse_sync_warnings(table)returns aDataWarehouseSyncWarningper schema attached to a warehouse table whose status isFAILED,BILLING_LIMIT_REACHED,BILLING_LIMIT_TOO_LOW,PAUSED/should_sync=False, orRUNNINGwithlast_synced_atolder than 2×sync_frequency_interval.DataWarehouseSyncWarningand a top-levelwarnings?: DataWarehouseSyncWarning[]field onHogQLQueryResponse(and the cached variant). Regeneratedschema.json/schema.pyviahogli build:schema.Databaseprecomputes warnings per warehouse table during table loading; the HogQL resolver emits them into theHogQLContextwhenever it resolves aDataWarehouseTable.HogQLQueryExecutor.execute()attaches the collected warnings to the response.LemonBannerabove the Results grid and the Visualization tab (frontend/src/scenes/data-warehouse/editor/OutputPane.tsx).execute-sql+ Max AI:format_warehouse_sync_warnings()prepends a `[Data warehouse sync warnings — …]` block to LLM-facing output.query-run: forwards the structuredwarningsarray through to the LLM (previously it only returnedresults/columns, throwing the field away).How did you test this code?
I'm an agent (Claude Code) — manual UI testing was done by the human author.
Automated:
products/data_warehouse/backend/test/test_sync_status.py(13 tests, all passing locally).
Manual (by author):
ExternalDataSchema.statusvia Django shell on a real warehouse table and confirmed the yellow banner appears in the SQL editor for each trigger status, plus the stale-running case (backdatedlast_synced_at).execute-sqlformatter prepends the warning block.query-runtool was dropping the field — fixed by passing it through inservices/mcp/src/tools/query/run.ts.Publish to changelog?
Yes
Docs update
skip-inkeep-docs
🤖 Agent context
Authored by Claude Code (Opus 4.7) with the human author. Notable decisions: