Skip to content

feat(analytics): saved-query export/import — promote experiment queries dev->prod (#2259) #2260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
cyberantonz wants to merge 3 commits into from
Closed

feat(analytics): saved-query export/import — promote experiment queries dev->prod (#2259) #2260

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
47bc952
feat(analytics): saved-query export/import — promote experiment queri…
cyberantonz Aug 6, 2026
a006d64
fix(analytics): bound import size, align PRD envelope, document colli…
cyberantonz Aug 6, 2026
18efddd
fix(analytics): preserve saved-query id on import so a promoted FE re…
cyberantonz Aug 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
257 changes: 257 additions & 0 deletions docs/components/backend/analytics/openapi.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,24 @@
],
"type": "object"
},
"ImportResponse": {
"description": "Result of `POST /v1/queries/import`: how many rows were created versus\nskipped as a same-name collision.",
"properties": {
"imported": {
"minimum": 0,
"type": "integer"
},
"skipped": {
"minimum": 0,
"type": "integer"
}
},
"required": [
"imported",
"skipped"
],
"type": "object"
},
"MetricDefinitionListResponse": {
"description": "Response body for `GET /v1/metric-definitions`. Metrics are sorted by\n`metric_key` ascending so the payload is byte-stable for caching and\ndiff tooling.",
"properties": {
Expand Down Expand Up @@ -1177,6 +1195,33 @@
],
"type": "object"
},
"PortableSavedQuery": {
"description": "One record of the export/import document.\n\nCarries the `id` — the stable handle a promoted frontend references in\n`/v1/queries/{id}/run`, so it must survive the move for the frontend to\nresolve on the target stand — plus `name`, `description`, and `sql`. It\ncarries no tenant or timestamps: the SQL is contract-relative and the tenant\nis session-injected at run time, so import preserves the `id` and re-homes\nonly the tenant to the importing session.",
"properties": {
"description": {
"type": [
"string",
"null"
]
},
"id": {
"format": "uuid",
"type": "string"
},
"name": {
"type": "string"
},
"sql": {
"type": "string"
}
},
"required": [
"id",
"name",
"sql"
],
"type": "object"
},
"Problem": {
"description": "RFC 9457 problem+json. `context` varies by error category.",
"properties": {
Expand Down Expand Up @@ -1279,6 +1324,21 @@
],
"type": "object"
},
"SavedQueryExport": {
"description": "Body of `GET /v1/queries/export` and `POST /v1/queries/import` — the portable\ndocument that carries a tenant's saved queries between stands.",
"properties": {
"queries": {
"items": {
"$ref": "#/components/schemas/PortableSavedQuery"
},
"type": "array"
}
},
"required": [
"queries"
],
"type": "object"
},
"SavedQueryListResponse": {
"description": "Response envelope for `GET /v1/queries` (`{ \"items\": [SavedQuerySummary] }`).",
"properties": {
Expand Down Expand Up @@ -1971,6 +2031,203 @@
"summary": "Create a saved query"
}
},
"/v1/queries/export": {
"get": {
"operationId": "analytics_api.queries.export",
"responses": {
"200": {
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/SavedQueryExport"
}
}
},
"description": "Portable saved-query document"
},
"400": {
"content": {
"application/problem+json": {
"schema": {
"$ref": "#/components/schemas/Problem"
}
}
},
"description": "Bad Request"
},
"401": {
"content": {
"application/problem+json": {
"schema": {
"$ref": "#/components/schemas/Problem"
}
}
},
"description": "Unauthorized"
},
"403": {
"content": {
"application/problem+json": {
"schema": {
"$ref": "#/components/schemas/Problem"
}
}
},
"description": "Forbidden"
},
"404": {
"content": {
"application/problem+json": {
"schema": {
"$ref": "#/components/schemas/Problem"
}
}
},
"description": "Not Found"
},
"409": {
"content": {
"application/problem+json": {
"schema": {
"$ref": "#/components/schemas/Problem"
}
}
},
"description": "Conflict"
},
"429": {
"content": {
"application/problem+json": {
"schema": {
"$ref": "#/components/schemas/Problem"
}
}
},
"description": "Too Many Requests"
},
"500": {
"content": {
"application/problem+json": {
"schema": {
"$ref": "#/components/schemas/Problem"
}
}
},
"description": "Internal Server Error"
}
},
"security": [
{
"bearerAuth": []
}
],
"summary": "Export saved queries"
}
},
"/v1/queries/import": {
"post": {
"operationId": "analytics_api.queries.import",
"requestBody": {
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/SavedQueryExport"
}
}
},
"description": "Portable saved-query document to bulk-create",
"required": true
},
"responses": {
"201": {
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/ImportResponse"
}
}
},
"description": "Imported and skipped counts"
},
"400": {
"content": {
"application/problem+json": {
"schema": {
"$ref": "#/components/schemas/Problem"
}
}
},
"description": "Bad Request"
},
"401": {
"content": {
"application/problem+json": {
"schema": {
"$ref": "#/components/schemas/Problem"
}
}
},
"description": "Unauthorized"
},
"403": {
"content": {
"application/problem+json": {
"schema": {
"$ref": "#/components/schemas/Problem"
}
}
},
"description": "Forbidden"
},
"404": {
"content": {
"application/problem+json": {
"schema": {
"$ref": "#/components/schemas/Problem"
}
}
},
"description": "Not Found"
},
"409": {
"content": {
"application/problem+json": {
"schema": {
"$ref": "#/components/schemas/Problem"
}
}
},
"description": "Conflict"
},
"429": {
"content": {
"application/problem+json": {
"schema": {
"$ref": "#/components/schemas/Problem"
}
}
},
"description": "Too Many Requests"
},
"500": {
"content": {
"application/problem+json": {
"schema": {
"$ref": "#/components/schemas/Problem"
}
}
},
"description": "Internal Server Error"
}
},
"security": [
{
"bearerAuth": []
}
],
"summary": "Import saved queries"
}
},
Comment on lines +2127 to +2230

@coderabbitai coderabbitai Bot Aug 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Document the actual import failure responses.

POST /v1/queries/import returns 415 for text/plain and 422 for off-schema JSON. This contract declares 400 but omits both actual responses. Clients generated from this OpenAPI document cannot rely on the documented error contract.

Add the 415 and 422 responses, or map extractor failures to the documented status code.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/components/backend/analytics/openapi.json` around lines 2122 - 2225, Add
415 Unsupported Media Type and 422 Unprocessable Entity responses to the POST
operation identified by operationId analytics_api.queries.import, using the
existing application/problem+json Problem schema pattern. Preserve the current
400 response and accurately document the actual text/plain and off-schema JSON
failure statuses.

"/v1/queries/{id}": {
"delete": {
"operationId": "analytics_api.queries.delete",
Expand Down
12 changes: 11 additions & 1 deletion docs/domain/presentation-layer/specs/DESIGN.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ Requirements that significantly influence architecture decisions.
| `cpt-presentation-fr-namespace` | New empty `presentation` database for new gold, saved-query results, and scratch; legacy gold left read-only in `insight` |
| `cpt-presentation-fr-saved-query-crud` | The saved query (`presentation.queries` logically; the `saved_queries` table physically) is a SeaORM entity in the analytics **service database (MariaDB)**, like metric definitions; CRUD mutates that metadata, not ClickHouse. Only `/run` reaches ClickHouse — it reuses the existing read path and executes the stored SQL as `presentation_ro`, so no write grant on the contract is ever needed. Shipped (#1965) |
| `cpt-presentation-fr-query-params` | Named parameters, `tenant` always injected from context (not client SQL), `period` supported |
| `cpt-presentation-fr-saved-query-export-import` | Two thin endpoints over the saved-query CRUD: `GET /v1/queries/export` dumps the tenant's queries as portable JSON (`id`/`name`/`description`/`sql`), `POST /v1/queries/import` bulk-creates from that JSON, re-gating each SQL and re-homing it to the importing session's tenant while **preserving the id** — the handle a promoted FE calls in `/run`; ids already present are skipped. Promotes tier-3 experiment queries dev to prod alongside the FE (#2259) |
| `cpt-presentation-fr-tenant-filter` | Literal leading `tenant_id = <ctx.tenant>` injected in one place — the compiler's shared `WHERE` (and the peer-cohort CTE reads) — replacing the no-op. `tenant_id` is the column the gold observation and cohort contract exposes (silver's `insight_tenant_id`, aliased to `tenant_id` in gold); filtering on it sidesteps the #1596 name drift, which affects other tables, not this read surface. Shipped for the structured `metric_results` read path (#1967). The legacy per-metric `query_ref` path (`execute_metric_query`) remains unscoped and is explicitly outside this guarantee until protected — see the component boundaries below. |
| `cpt-presentation-fr-contract-surface-doc` | Contract surface documented as the read boundary in [CONTRACT-SURFACE.md](./CONTRACT-SURFACE.md): the `class_*`/`fct_*`/`mtr_*`/`dim_*` silver families and `person.*`/`identity.*` objects, with the additive-only rules and the granted `insight` legacy gold. Shipped (#1968) |
| `cpt-presentation-fr-contract-version-stamp` | Engineering stamps `silver.contract_version` (single-row constant view, ledgerless CH migration); analytics pins `PINNED_CONTRACT_VERSION` and verifies the stamp in a periodic post-boot sweep, logging a mismatch or missing stamp without gating boot. Shipped (#1969) |
Expand Down Expand Up @@ -281,12 +282,17 @@ Plain CRUD over stored queries so a new analytics slice needs no engineering cha
- Validate SQL via the query gate (`validate_single_select`) on create, update, **and** run — the run-side re-validation keeps a stored SQL from reaching ClickHouse as anything but a single read.
- Run: execute the stored single-SELECT read-only as `presentation_ro` and return untyped JSON rows (`JSONEachRow`, same shape as the existing metric query path).
- Bind named parameters on run (#1966): `{tenant}` is always bound from the session `SecurityContext` (never client-settable); `{period}` is bound when supplied on the run request body. Values are passed as ClickHouse server-side parameters (`Query::param` → `param_<name>`), so a value can never change query structure; the gate already tolerates `{name:Type}` placeholders. A query that references a parameter left unbound (e.g. `{period}` with no period supplied) fails as a 400, not a 5xx.
- Export/import (#2259): `GET /v1/queries/export` reuses the tenant-scoped list read and returns each row as `{ id, name, description, sql }` — carrying the id but no tenant or timestamps, so the document is stand-independent. `POST /v1/queries/import` bulk-creates from that document, re-validating each `sql` through the same gate as create and binding `ctx.subject_tenant_id()` while **preserving the source `id`** (the handle a promoted FE calls in `/run`, so it must survive the move); only the tenant is re-homed. A query whose `id` already exists is skipped (not overwritten) — checked globally over the incoming ids, because the id is the sole primary key, so an already-taken id cannot be re-homed and skipping it rather than colliding on insert keeps a re-import idempotent. The response reports the imported and skipped counts.

##### Responsibility boundaries

- Does NOT carry metric metadata, thresholds, or passports — those are Phase B.
- Does NOT bypass the gate.
- Does NOT bypass the gate — import re-gates every SQL exactly as create does.
- Does NOT string-interpolate parameter values — binding is server-side only.
- Does NOT trust the tenant on an imported document — the tenant is always re-homed to the importing session, so an import can never write cross-tenant. The `id` **is** trusted and preserved (it is the FE's handle); cross-tenant isolation comes from the tenant-scoped reads (`find`/`list`/`run` all filter `insight_tenant_id`), not from changing the id.
- Does NOT overwrite on import — an id already present is skipped, never clobbered; changing an existing query stays an explicit update. Because the id is the sole primary key, a query re-homes into exactly one tenant: importing the *same* document into a second tenant skips the already-taken ids (reported in `skipped`) rather than duplicating a query the schema cannot hold twice. Multi-tenant fan-out of one experiment would need a composite-key store and is out of scope. The skip is best-effort under concurrency: two simultaneous imports of the same new id race, and the loser hits the primary-key constraint (a loud 500, not a silent duplicate).
- Does NOT accept an unbounded import — a document over the per-request query limit is rejected at the route boundary with a 400 before any load or write, so one request cannot force an unbounded batch. Export is bounded by construction: it returns only the tenant's own analyst-authored queries and never truncates.
- Does NOT seed curated/product queries — auto-seeding sanctioned definitions to every stand is definitions-as-data (semantic epic #2213), not this bespoke export/import.
- Does NOT yet inject the tenant-row filter (#1967) — the run path binds the `{tenant}` *value* but does not yet add an `insight_tenant_id = {tenant}` predicate to queries that omit it; that cross-cutting concern lands in its own sub-issue.

##### Related components (by ID)
Expand Down Expand Up @@ -476,9 +482,13 @@ Entity `presentation.queries`: `{ id, insight_tenant_id, name, description, sql,
| `PUT` | `/v1/queries/{id}` | Update (re-validates SQL) | unstable |
| `DELETE` | `/v1/queries/{id}` | Delete | unstable |
| `POST` | `/v1/queries/{id}/run` | Execute read-only as `presentation_ro`, return rows; optional body `{ "period": "<value>" }` binds `{period}`; `{tenant}` always bound from context (tenant-row *filter* deferred to #1967 — the run path binds the tenant value but adds no `insight_tenant_id` predicate yet) | unstable |
| `GET` | `/v1/queries/export` | Dump the tenant's saved queries as portable JSON `{ "queries": [{ id, name, description, sql }] }` — carries the id, but no tenant or timestamps | unstable |
| `POST` | `/v1/queries/import` | Bulk-create from `{ "queries": [{ id, name, description, sql }] }`; each `sql` re-gated, the id preserved, the tenant re-homed to the session; ids already present skipped; over `MAX_IMPORT_QUERIES` is a 400. Returns `{ imported, skipped }` counts | unstable |

`run` executes as `presentation_ro` and returns untyped JSON rows, the same shape as the existing metric query path. The request body is optional; named parameters (`tenant`/`period`, #1966) are bound as ClickHouse server-side parameters. No metric metadata, thresholds, or passports in Phase A.

`export`/`import` (#2259) move saved queries across stands so a promoted tier-3 experiment arrives with the queries it needs. The promoted FE calls each query by `id` (`/v1/queries/{id}/run`), so the document carries the `id` and import preserves it — otherwise the promoted FE would reference a query that does not exist on the target stand. The rest is portable by construction: the SQL is contract-relative and the tenant is session-injected at run time, never stored. `import` reuses the create path — it re-validates each `sql` through the single-SELECT gate and binds the importing session's tenant, so an import can never write cross-tenant (isolation is enforced by the tenant-scoped reads, not by rewriting the id). A query whose `id` already exists is skipped (not overwritten), making re-import idempotent; the response reports imported vs skipped counts.

### 3.4 Internal Dependencies

| Dependency Module | Interface Used | Purpose |
Expand Down
Loading
Loading