* Fix layout shift when switching tabs on the transform job page
The job Overview and Run history pages each rendered their own header:
Overview used an editable name input plus a `...` menu, while Run history
used a static title with no menu. The differing heights shifted the tab
row, so the clicked tab jumped when switching between the two.
Extract a shared JobHeader component (and a useJobHeaderState hook for the
readOnly/name-change logic) rendered identically by both pages, mirroring
the MetricHeader pattern. Run history now also shows the `...` menu and the
editable name, and Metabot is consistent across both tabs.
* Unify JobPage header logic with useJobHeaderState
JobPage duplicated the readOnly computation and name-change handler that
now live in useJobHeaderState. Source them from the hook instead; keep the
local useTransformPermissions call only for the page-level loading gate.
* Use strict undefined comparisons in job header code
Address review: job/jobId are never null, so compare with undefined
strictly and derive jobId from job.id in the JobHeader render.
Fixes GIT-10551: connecting an MCP client to a Metabase instance served
under a subpath (site-url like https://example.com/metabase behind a
prefix-stripping reverse proxy) failed at the consent step.
The OAuth flow already derives every advertised URL from site-url —
discovery metadata, authorization/token/registration endpoints, and the
WWW-Authenticate resource_metadata header are all subpath-correct. But
three browser-facing pieces assumed Metabase lives at the domain root:
1. The consent form posted to root-relative /oauth/authorize/decision,
so the browser sent the POST outside the subpath and the proxy
returned 404 (the reported bug). The form action is now the absolute
site-url-based URL, via the same absolute-url helper the page already
uses for the logo. The bundled @font-face URLs on the page get the
same treatment.
2. The CSRF cookie was scoped to Path=/oauth/authorize. Under a subpath
the consent page lives at /metabase/oauth/authorize, so the browser
would never send the cookie back and the decision POST would fail
CSRF validation even with the form action fixed. The cookie path now
carries the path component of site-url.
3. The logged-out flow: the backend bounces to the login page with a
basename-relative redirect target (the server sits behind the
prefix-stripping proxy and never sees the subpath). The frontend
guard resolved that target against the bare origin, losing the
subpath on the full-page redirect back to /oauth/authorize. The
guard now normalizes targets through the router basename in both
directions: paths without an explicit origin get the basename joined
(browser-real href), and pathnames get it stripped (router path, so
the backend-only-path prefix check still matches).
Why this is safe:
- Root deployments are bit-for-bit unchanged. site-url has no path
component there, so the cookie path is still /oauth/authorize, the
form action still targets /oauth/authorize/decision, and with an
empty basename the guard resolves every input shape (relative path,
bare path, absolute URL) to exactly the URLs it produced before.
- Under a subpath, the only outputs that change are ones that were
previously unroutable (404 through the proxy or a path the SPA
router would double-prefix).
- The server never sees the subpath, so no routes, parameters, or
validation logic change; every fix is to a browser-visible artifact
derived from site-url.
Tests pin both halves: existing root-deployment assertions are
untouched and still pass, and new tests cover the subpath form action,
cookie path, login redirect target, and the full consent -> decision
round-trip with a subpath site-url. Also verified live against a local
prefix-stripping Caddy proxy with a real MCP client flow (DCR, consent,
PKCE token exchange, tool calls).
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* Rewrite image-buffer pool on dirigiste (same pool as the GraalVM contexts)
Replace the hand-rolled ConcurrentLinkedDeque pool, stats counters, sweeper
daemon, and logging with the dirigiste Pool already used for static-viz
contexts, with the same semantics: 0-3 pooled int[] arrays of the fixed
oversized size, held exclusively per render, idle arrays dropped after ~10
minutes. acquire blocks until an array is free when the requested size fits;
larger requests get a fresh unpooled BufferedImage.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Extract the dirigiste pool factory into metabase.util.pool
create-pool was duplicated verbatim between the static-viz graal renderer
(channel.render.js.common) and the sqlglot graal runner (sql-parsing.common),
and image-buffer had just picked up a third use. Move it to metabase.util.pool
and use it from all three; the pool behavior tests move from
sql-parsing.common-test to util.pool-test with it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Add prometheus counters for image-buffer pool usage
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Rename image-buffer metrics to metabase-notification namespace
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* Migrate nav search and search result components from Emotion to Mantine
* Address review feedback: close button label, valid footer markup, footer role and hover transition
* Restore search dropdown footer ref by returning a ref callback from useListKeyboardNavigation
* Improve page/container margins when printing document
* do not call flushSync while printing
* Revert "do not call flushSync while printing"
This reverts commit 37c6fe2b50.
* Resize ECharts synchronously for print layout changes
* Refactor ECharts print resize tests
* Clean up
* remove compile-with-inline for clickhouse, no longer needed since the driver got fixed
* re-enable full tests
* fix: for max behavior on clickhouse
* fix: insert for clickhouse
* add temporal reg test
* Generate expression parser as TypeScript
Add --typeScript to lezer-generator so the parser emits typed output; convert lezer.js to lezer.ts.
* Migrate NotebookStepPreview to TypeScript
* Reuse QuestionResultLoaderChildrenProps for VisualizationPreview props
* Add element prop support to facade Route
* Compose route guards as element wrappers
* Memoize the element bridge by component type and render off the injected route
* Clarify that index routes accept component or element
* Test that the element bridge reuses shared components across sibling routes
* Migrate DataSelector SavedEntityPicker to TypeScript
* Align DataSelector picker prop types with actual usage
* Migrate DataSelector to TypeScript and adapt consumers
* Migrate DataSelector unit tests to TypeScript
* Remove unused DataSelector props
* Add TODO for DataSelector api/metabase-lib structural-type cleanup
* Remove unused hasBackButton/onBack from DataSelectorSchemaPicker
* Use setState updater form in setStateWithComputedState
* Guard currentUser instead of faking a zero user id
* Avoid passing a null database to SchemaAndTableDataSelector
* Convert SavedQuestionHeaderButton unit spec to TypeScript
* Justify or remove DataSelector type casts for no-unjustified-type-casts
Different endpoints dealing with destination databases handled checks differently, causing confusing error messages depending on code path. This consolidates and standardizes both the messages and the HTTP code.
* Remove table_id from measures and segments
* Clear stale Card table_id when the query loses its source table
populate-query-fields only wrote table_id when the derived value was
truthy, so converting an MBQL question to native SQL (or pointing it at
a native model) left table_id referencing the old table. Make it
unconditional, the same fix#68080 applied to source_card_id.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Address review: validate derived table on segment/measure updates
- PUT /api/segment/:id and PUT /api/measure/:id now derive the table from an
updated definition: table-less definitions are rejected (consistent with
POST), and if the definition implicitly moves the entity to another table,
the user must also pass the create permission check for the new table
(write-check alone only covered the old table).
- Drop the dead :table_id insert arg in POST /api/measure — before-insert
already derives it from the definition.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Derive segment/measure perms from the definition, not table_id
Per review: instead of API-handler checks against a separately derived table
id, the mi/can-read?, mi/can-write?, and mi/can-create? implementations for
:model/Segment and :model/Measure now derive the source table from the
definition via lib/primary-source-table-id (same admin-or-analyst check as
before, rejecting definitions with no source table), and the API handlers no
longer deal with table ids at all — table_id only remains in what we return
to the client.
Segment definitions are now normalized to MBQL5 at the API boundary (like
measures already were), so the perms and model hooks always see full queries.
Updating a definition re-runs the create check with the new definition, since
the write-check only covers the existing one.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Fix remote-sync segment permission tests for definition-derived perms
The three remote-sync segment tests used empty {} definitions and relied on
table_id/hydrated :table for the permission checks. Perms now derive the
source table from the definition, so give the segments a real MBQL5
definition on the venues table. Also coerce the can-manage helpers to
booleans so callers asserting false? keep working.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Strip stray :aggregation from MBQL4 segment fragments at the API boundary
The model migration always dropped :aggregation from legacy segment
fragments (segments cannot have one). When definition normalization moved to
the API layer, that behavior was lost, so fragments with a stray aggregation
(which several Cypress specs post) started failing schema validation on
insert.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Reject invalid segment definitions instead of stripping :aggregation
Per review: don't silently fix up segment definitions with a stray
:aggregation — validate the normalized definition against the segment schema
at the API boundary and return a 400. Remove the aggregation key from the
Cypress specs that were posting it (it was always dropped by the old model
migration, so those specs never exercised it anyway).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Simplify: keep table_id derivation at the API, revert the perms refactor
Restore the segment/measure permission methods, search filtering, and the
associated tests to their previous table_id-based behavior. The PR keeps to
its core change: the segment/measure APIs no longer accept table_id — it is
derived from the definition and used for the permission checks (including
re-checking the newly derived table when an update moves the entity).
Also update compare-to-the-rest-15655-test for the card change in this PR:
a wrong table_id can no longer be persisted (it is always derived from the
query), so the comparison dashboard for a nil-table card is built from the
field-distribution path now.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Only clear a stale card table_id when the query itself changes
populate-query-fields' unconditional :table_id meant any card update
(rename, archive, collection move) would wipe table_id whenever the
derivation came back nil — e.g. when the query's source card had been
hard-deleted. Gate the nil-overwrite on :dataset_query actually being in
t2/changes; inserts and query edits still clear stale values.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* Persistent queue: concurrent batches, per-queue error handler, config
Generic improvements to the mq persistent-queue subsystem, extracted from the
explorations work so they can land independently:
- max-concurrent-batches: a queue can process multiple batches in parallel,
bounded by a (optionally dynamic) limit (metabase.mq.queue.concurrency).
- on-error: per-queue handler invoked when a message exhausts its retries,
so callers can record terminal failures instead of silently dropping them.
- Reworked polling, quartz scheduling, and registry for the above.
- Expanded mq/README.md and tests (concurrency, on-error, quartz affinity).
* Fix Eastwood reflection warnings on Scheduler.checkExists in quartz-test
`Scheduler.checkExists` is overloaded (JobKey vs TriggerKey), so the un-hinted
`nudge-key` made the three call sites reflective. Import `JobKey` and hint the
local; CI's Eastwood step fails the build on reflection warnings.
* Code fixes
* Migrate remaining ModalContent Emotion styles to Mantine and fix dead EmbedModal header overrides
* Make the modal header back button keyboard-accessible with a descriptive label
* fix orphan semantic search index table cleanup
* Harden orphan index cleanup: schema-scope the scan, skip views, isolate drops
The orphan scan now only considers base tables (a view in the DROP TABLE
batch would abort every nightly cleanup) and, in dedicated-pgvector mode,
only current_schema() — where the unqualified DROP actually resolves;
previously it scanned all schemas and logged drops that never happened.
Stale/orphan tables are dropped one statement at a time with per-table
error handling so a single undroppable table (e.g. one with a dependent
view) cannot permanently block stale, tombstone, and repair cleanup.
Also anchors index-table-name-pattern (\A/\z) so it stays substring-proof
independent of re-matches, documents that legacy pre-BOT-337
index_table_* names must keep matching, and derives the force-reset test
case from model-table-name/model-table-suffix instead of a literal.
* Count actual index-table drops and reap legacy index_table_ stragglers
drop-index-table! now reports :dropped/:missing/:failed — dropping without
IF EXISTS so a vacuous drop (metadata row whose table is already gone)
does not count — and cleanup logs dropped-of-found. The name pattern
gains an index_table_<anything> alternative: the prefix belongs
exclusively to the pre-BOT-337 era, whose digit-less stragglers were
otherwise unreachable.
---------
Co-authored-by: Chris Truter <chris@metabase.com>
* Do not duplicate user context between system prompt and last user message
In #71045 we moved most of the user context out of internal.selmer system prompt and injected it into the last user
message. However, other system prompts still injected user context into the system prompt, so we wound up formatting
and including it twice. If the `:user_is_viewing` context happens to be large (many related tables / fields), then the
memory and token cost can be significant (#76493).
This commit removes everything from system prompts that was already being injected into the last user message, namely:
- `current_time`
- `first_day_of_week`
- `current_user_info`
- `viewing_context`
It also moves `recent_views` into the last user message (not previously included there).
In addition to saving input tokens and memory, this should also help with caching on providers like openai that do
automatic prefix caching, since we now always inject the current date/time near the end of the prompt.
This also means all system prompts can drop the `<<<METABOT_CACHE_BREAKPOINT>>>` since they no longer have any dynamic
content after the breakpoint.
* Add :slackbot and :document-generate-content to profiles-excluding-recent-views
Their system prompt never included recent_views, so preserve the behavior now that we inject recent_views in the last
user message.
A failed CREATE USER used to surface the generated workspace password
verbatim in the JDBC exception message, which provisioning then logged
(GHY-3660). Scrubbing landed in #74657; this adds a regression test that
reproduces the failure for real: init-workspace-isolation! runs over a
connection authenticated as a low-privilege workspace user (granted
database-level CREATE on postgres/redshift so execution reaches the
password-bearing CREATE USER), and every piece of text a logger could
print from the thrown exception - stack trace, Throwable->map, causes,
suppressed, ex-data, and SQLException nextException links - must not
contain the password. A postgres-only assertion requires the redaction
marker in the echoed statement so the test cannot pass vacuously.
JDK 25 changed the empty file path to resolve to the current directory
(File("").exists() is now true), so a long-standing MB_CONFIG_FILE_PATH=''
made the config-from-file loader try to parse the current directory as YAML
and fail at boot. Ignore blank values like metabase.config.core/config-str
does everywhere else, and don't count them as config-text-file being enabled
in anonymous stats.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* initial migration
* initial migration
* remove deadcode loaders
* one more site is migrated. remove dead code
* make data-testid="loading-indicator" to be by default for Loader component
* fix unit test
* Omit negative values from treemap instead of producing wrong percentages
Treemaps summed metric values as signed totals and divided by that signed
total, so rows of 30 and -50 rendered a surviving leaf as -150%. Negative
tiles also have no positive area, so ECharts dropped them with no warning.
Mirror the pie chart: omit negative values (with a warning) during
aggregation, or fall back to absolute values when every value is negative.
With all node values non-negative the downstream percentage math is correct.
Fixes GDGT-2760
* Tighten null check and document per-row negative filtering
* Bump mise JDK to temurin 25 to match CI
CI defaults to Java 25 (.github/actions/prepare-backend and the backend
workflow matrix), but mise.toml still pinned temurin 21.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Update remaining Java 21 leftovers to 25
The e2e matrix builder and docs still referenced JDK 21, while CI,
Docker images, and release requirements for v60+ are on Java 25.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* Pin MBQL transform source schema in transforms e2e spec
The many_schemas Postgres fixture creates an "Animals" table in every
schema (Schema A through Schema Z). createMbqlTransform resolved its
source table via getTableId by name only, so Array.find returned an
arbitrary schema's "Animals" depending on /api/table ordering. The
"convert an MBQL transform to a SQL transform" test asserts the compiled
SQL contains "Schema A", so a non-Schema-A match made it flake.
Add an optional sourceSchema to createMbqlTransform and pin the spec's
source to Schema A so the resolved table and its SQL are deterministic.
* Wait for MBQL->SQL transform save to settle before clicking Run tab
The Run tab only exists in read-only view mode; after clicking Save the
test must wait for the update PUT and the navigation off /edit, otherwise
it races the still-in-flight save and times out looking for the Run tab.
* only pin source schema for the default source table
---------
Co-authored-by: Ngoc Khuat <qn.khuat@gmail.com>
* Add index-route support and redirect() to the metabase/router facade
* Adapt ModalRoute and DataPermissionsPage to the function-based Route
* Replace IndexRoute/IndexRedirect/Redirect with index routes and redirect()
* Convert createEntityIdRedirect to a facade-hook wrapper component
* Update route test scaffolding for index routes and redirect()
* Build the Route shim via createRoutes instead of augmenting RouteUtils
* Point the guards spec at the relocated Route export
* Stabilize transforms permission navigation in e2e specs
* Freeze redirect() target at mount to fix chained index-redirect race
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
* Don't GZIP sync-flush per object when serializing QP cache entries
Sync-flushing after every cached row meant one native Deflater call in a
JNI critical region per row; on JDK <= 21 (no G1 region pinning, JEP 423)
that stalls GC via the GCLocker under concurrent cache writes. Let the
deflater buffer normally instead, and have result-fn finish the stream
(draining the deflater and writing the GZIP trailer) before returning the
bytes. Old cache entries (unterminated gzip) still deserialize since the
reader already treats EOFException as end-of-stream.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Don't flush the JSON download writer per row
Each per-row flush cascaded into a sync-flush of the gzipped HTTP response:
one native Deflater call in a JNI critical region per exported row (GCLocker
stalls on JDK <= 21) plus one TCP packet per row (#34795, fixed for the :api
writer but not :json downloads) and a worse compression ratio. Let the
BufferedWriter push rows through as it fills, like the CSV writer does;
finish! still flushes the tail.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Make Snowflake CI runs use a randomly-generated, isolated DB names.
Something is still randomly calling `destroy-db!` on the test-data
dataset and wrecking things for CI runs:
```
(pprint (jdbc/query (no-db-connection-spec)
["SELECT query_text, end_time
FROM SNOWFLAKE.ACCOUNT_USAGE.QUERY_HISTORY
WHERE query_text LIKE '%_c1bae%'
AND (query_text LIKE '%DROP%' OR query_text LIKE '%drop%')
ORDER BY end_time DESC limit 128"]))
[...]
"DROP DATABASE \"sha_rel_c1baee7db240aa419104c2d925a07a4d4faeeb24_test-data\";"
```
This particular query (without `IF EXISTS`) only exists on master in
the `destroy-db!` method. However, this method already has protection
against being run on the `test-data` dataset and will throw an
exception if this is tried! And the main call to `destroy-db!` is
gated behind a feature check for `:test/dynamic-dataset-loading`. So
there's nowhere this could be coming from on master.
Rather than trying to track it down, I think it's better to just treat
all snowflake DBs as isolated and disposable. Update `qualified-db-name`
to just prefix the DB name with a random prefix, and use the
`after-run` hook to drop it when done. This also allows us to get rid
of the janky and weird locking system, because no two jobs should ever
try to hit the same DB.
* Add per-version download link to Security Center advisories
Thread optional download_jar_url through backend schema/fetch and render
a Download link next to each fixed version in AdvisoryCard.
* fix aligning
* Read instance version via useSetting; simplify affected-version lookup
* rollback a redundant change
* addressing PRs comments
* use sequence to nil-coerce download_jar_urls
* addressing the PR's comments
* add download-click analytics
* fix test: add download_jar_urls to Advisory factory
* add e2e test
* initial migration
* fix link input size
* remove dead files
* Use Input.ClearButton for textinput clear icon
Replace hand-rolled Tooltip+ActionIcon+Icon clear pattern w/ Mantine
Input.ClearButton across 5 sites; c=text-secondary + rightSectionPointerEvents=all.
Comment why LinkViz needs pointer-events on both root+input.
* fix unit test
* Update loki refs for parameter search Input->TextInput
* Use Input.ClearButton in PermissionsEditorContent; rm blank line in SearchBar styled
* addressing the PR's comments
* validate client sent parent_message_id
* implement more restrictive retry logic
* move history to the backend
* migate state from conversation to message records
* client nows generates message ids to shrink case of client not knowing id of written message record on aborted requests
* pr feedback
* [Metabot] Implement conversation branches in usage analytics (#77542)
* implement branches in usage analtyics
* sol pass at making code terser / cleaner
* handle error messages correctly + allow branching after assistant message
* pr feedback
* fix test failures
* remove migration logicalFilePath
* Hide write-only share actions when entity is not writable (#72752)
The Question and Dashboard sharing menus, and the public-link popover,
offered write actions to any admin regardless of write access. On a
read-only remote-synced instance a card/dashboard in a synced collection
is not writable (can_write=false), so these actions 403'd or authored
state that doesn't stick — the "updating embed fails on read-only
instance" symptom in #72752.
Gate the write actions on can_write, matching how the normal edit
affordances already disappear when the entity can't be written:
- Embed (publish): hidden when not writable.
- Create a public link: hidden when not writable; an existing public link
stays visible so it can still be viewed/copied (those are reads).
- Remove public link: hidden when not writable.
* Disable embed Publish instead of hiding the option on read-only entities (#72752)
Keep the Embed sharing option visible for a read-only remote-synced
card/dashboard, but disable the Publish/Unpublish buttons in the embed
modal with a tooltip explaining the entity is synced and read-only.
This reverts the earlier hiding of the Embed menu item; the public-link
gating (hide create / remove, keep an existing link viewable) is
unchanged.
* Translate the read-only embed tooltip per resource type (#72752)
Interpolating resourceType left the noun untranslated and prevented the
rest of the sentence from agreeing with its gender.
* Add ai-tracing: eval-time trace capture for the Metabot agent
Introduce a new `ai-tracing` module that captures complete, per-run agent
traces during benchmark/eval runs and exports them to an external eval
backend (Confident AI) over OTLP — separate from production observability.
- Capture is gated by a per-run dynamic binding (`*capture*`), orthogonal to
MB_TRACING_*, so it never fires on organic traffic. Enabled via the
env-only MB_AI_EVAL_CAPTURE setting; the API path emits the trace as an
`eval_trace` data part and exports it.
- Captures the full agent.turn -> llm.call -> tool.* tree with prompts,
completions, tool args/results, durations, and concurrent-tool nesting
(conveyed across the agent's virtual threads).
- OTLP export uses a dedicated TracerProvider (SimpleSpanProcessor +
force-flush, never the production one), replaying the in-memory tree
post-hoc. The domain -> Confident `confident.*` semconv mapping lives in a
single chokepoint (`node->semconv-attrs`); vendor/schema swaps are one edit.
- Wires capture into run-agent-loop, loop-step, call-llm, and run-tool.
- Trace-level input/output (user question + final answer) set on the root.
- Export failures are logged loudly instead of swallowed.
- Tests cover the gate, nesting, enrichment, timing, concurrency, the
semconv mapping, and a metadata-size regression guard.
* Make ai-tracing export more idiomatic; refresh stale docs
- export.clj: replace node->semconv-attrs's nested cond-> of repeated
type checks + json/encode with :ai/keys destructuring and a flat
medley/assoc-some table; compose final-llm-output as transducers;
extract event->attributes; tidy parse-headers (for-comprehension),
build-provider! (doto), and ensure-provider! (fresh? helper).
- core.clj: drop the stale "seam not wired" comment block and the
outdated ns docstring (it referenced the abandoned gen_ai/OpenInference
schema and "single namespace" framing); describe the shipped export.
No behavior change; 14 tests / 56 assertions still pass.
* Replace ai-tracing OTLP export with per-session JSONL log files
Switch the eval-trace collection method from OpenTelemetry/OTLP export to a
dedicated file-based logger, and extend instrumentation to the MCP server and
agent-api in addition to the Metabot agent loop.
- Rip out all OTLP/Confident code (export.clj, the OTLP settings, the OTel
provider/semconv mapping, the eval_trace SSE part).
- New metabase.ai-tracing.log sink: each span streams one JSON line on a
dedicated logger, routed per session to its own <session-id>.jsonl by a
Log4j2 RoutingAppender keyed on mb-eval-session-id. Pure JSONL (%m%n), full
payloads, no size limits.
- Session model = OTel trace-id analog: *session-id* minted at a fresh
capture's root, or supplied by the caller, conveyed across virtual threads
by bound-fn*. Nodes carry :id/:parent-id for tree reconstruction.
- Generic with-eval-session entrypoint (gated by MB_AI_EVAL_CAPTURE, inherits
when nested) reused by run-agent-loop, MCP dispatch-request (keyed on the
Mcp-Session-Id so a conversation lands in one file), and agent-api routes.
- MB_AI_EVAL_CAPTURE is the single on/off switch; the logger sits at info and
writes nothing unless a capture is active.
- run-agent-loop emits a tiny eval_session SSE part so the HTTP harness can
locate the session file.
Verified end-to-end: real agent run writes a per-session JSONL file with the
correct span tree (parent chain, full structured + normal tool output);
sessions route to separate files. 14 tests / 51 assertions pass.
* ai-tracing: capture full request/response context and add trace read API
Enrich eval spans so a captured trace is a self-contained record of a run:
- agent-api: record HTTP request body, user id, and the response status/body
via a wrapping respond fn that fires inside the span.
- mcp: record JSON-RPC params, user id, scopes, and the materialized result.
- metabot agent: thread an optional eval-session-id through run-agent-loop so
the harness can name and read back the per-session trace file; record the
full request (messages/context/state), turn-level token usage, finish reason,
and final agent state (link registry / generated entities). Surface the loop
finish-reason out of loop-step instead of only logging it.
- log: guard span serialization so a non-encodable attribute can never break a
traced run.
Add metabase.ai-tracing.api: a superuser-only GET /api/eval-trace/:session-id
that returns the captured JSONL, mounted only when MB_AI_EVAL_CAPTURE is on.
Filenames are validated against path traversal. Wire the module into routes and
the kondo module config.
* ai-tracing: preserve OpenAPI spec on agent-api routes; harden trace sink
The eval-tracing wrapper turned agent-api `routes` into a plain fn, which no
longer implemented OpenAPISpec — full-API spec generation (openapi.json,
endpoint-dox, the openapi-all-routes test) threw when the walk reached
/api/agent. Wrap it in `handler-with-open-api-spec`, delegating the spec to the
underlying ns-handler.
Also: narrow the trace-sink catch to Exception so Errors/InterruptedException
propagate; stream the trace file instead of slurping it into heap; and correct
the "single nil-check" docstring claim.
* ai-tracing: validate eval-session-id at mint time to prevent path traversal
The trace read endpoint sanitized the session-id before touching disk, but the
write path fed the raw, caller-supplied eval_session_id straight into the
RoutingAppender's `<session-id>.jsonl` file name — a value like `../../foo` on
an eval instance escaped the trace dir (arbitrary .jsonl write).
Hoist the safety regex to a single `safe-session-id-re` in ai-tracing.core and
add `checked-session-id`, which mints a uuid when none is supplied and throws on
an unsafe supplied id. `with-eval-session` now validates every caller-supplied
id before it can reach the log-file path; ai-tracing.api references the shared
regex so read/write validation can't drift.
* ai-tracing: gate span-macro arg construction on capture-active?
The span macros expanded to an eval-span* call, so span-name and attrs
were built on every invocation and discarded on the nil-check when no
capture was live. Hoist the capture-active? guard into a `spanning`
helper macro (and an inlined variant in with-tool-call, whose name
derives from attrs) so organic traffic never builds the span name string
or attrs map — it pays only the *capture* nil-check plus one thunk
closure. with-tool-call now evaluates attrs exactly once, and only when
capturing.
* ai-tracing: test cross-module span nesting; make agent tests capture-agnostic
Add eval-tracing-nesting-test: drives the real run-agent-loop under
capture-reducible with only the LLM + search backend mocked, and asserts
the emitted span tree nests agent.turn -> llm.call -> tool.search. This
covers the wiring the ai-tracing docstrings promise — the turn/llm spans
opened in metabot.agent.core, the tool span in metabot.self.core, and
*parent* conveyed across bound-fn* into the virtual-thread tool executor
— which the ai-tracing unit tests exercised only at the primitive level.
Also pin eval-capture off for the namespace via a fixture. run-agent-loop
appends an eval_session data part to the stream whenever capture is active,
so every exact-match assertion on its output broke under MB_AI_EVAL_CAPTURE
(which dedicated eval instances set). The fixture keeps those assertions on
the production stream shape; the new test forces capture independently.
* ai-tracing: key MCP eval session on UUID correlator; complete finish-reason
Key the MCP eval-trace session on the session id's UUID prefix rather than
the full <uuid>.<base64> id: the prefix is stable across the conversation and
always filesystem/URL-safe, so a base64 payload that require-valid-session
accepts but safe-session-id-re rejects (e.g. `=` padding) can no longer make
checked-session-id throw ahead of the JSON-RPC try/catch and 500 the request.
Record :finish-reason on the empty-response and reduced (early-termination)
loop terminations too, so the turn span's :ai/finish-reason is populated on
every path, not just natural completion.
* ai-tracing: harden trace sink serialization and error capture
- emit!: fall back to a JSON-safe copy when an attribute value isn't
encodable, so one bad value no longer drops the span (and, for a root
span, orphans its children).
- span error events now carry the exception class and ex-data, not just
the message.
- rename JSONL :start-ns/:end-ns to :start-epoch-ns/:end-epoch-ns (they
hold wall-clock epoch nanos, not System/nanoTime).
- test the api trace-file path-traversal guard directly.
* ai-tracing: bound session ids, gate user-input build, snapshot finish-node
- Cap supplied eval-session-id at 200 chars on the API schemas and in
checked-session-id, so a supplied id can't produce a filename that fails
creation and silently drops the trace (charset was already enforced).
- Compute the turn span's :ai/user-input inside the capture-gated attrs map
instead of unconditionally on every run-agent-loop call.
- finish-node takes a single consistent snapshot of the node atom.
* ai-tracing: enforce session-id cap+charset at API boundaries
Route the 200-char cap through ait/max-session-id-length everywhere (read
endpoint, run-agent-loop opts, POST /api/metabot params) instead of
hardcoding the literal, and bound the previously-unbounded read-endpoint
:session-id path segment.
Validate the charset via safe-session-id-re in the Malli schemas so a bad
eval-session-id 400s at the boundary rather than throwing deep in
checked-session-id and surfacing as a generic agent error. Anchor
safe-session-id-re with \A/\z so it stays exact under both re-matches (mint/
read checks) and re-find (Malli :re) — unanchored, :re accepted any string
containing a safe run (e.g. ../../etc/passwd).
* ai-tracing: key MCP eval trace on a harness-supplied session id
Let an eval harness name and later fetch an MCP conversation's trace. The MCP
endpoint honors an `x-eval-session-id` header — `dispatch-request` keys
`with-eval-session` on it (validated through `checked-session-id`, the single
source of truth for the safe-id contract), falling back to the `Mcp-Session-Id`
correlator when absent. This is the MCP analogue of metabot's `eval_session_id`,
needed because the client owns the `Mcp-Session-Id` and never exposes it.
Also record the resolved query on the construct-query endpoints as `ai/query`
(inert unless capturing): the MCP `construct_query` tool returns only an opaque
handle, so this is the only place a harness can recover the agent's query off
the trace to grade it.
* ai-tracing: address review — guard :reduced append, cover the read API
- run-agent-loop: on a :reduced stop (client disconnect / cancellation) the
reducing fn has already terminated, so don't step it again to append the
eval-session pointer or debug part — that violates the transducer contract.
Fold both trailing appends into one capture-gated cond-> that no-ops on
:reduced (also fixes the latent debug-branch case). The harness reads the
JSONL file, not the stream, so a skipped pointer on disconnect costs nothing.
- agent-api: record :http/user-id in the respond closure, where +auth has bound
*current-user-id* — it is unbound when the span opens, so direct HTTP callers
previously recorded nil.
- ai-tracing.api: add endpoint integration tests — capture-gated 404,
superuser-gated 403, missing-trace 404, and existing-trace 200.
- log: mark node->entry ^:private; document that trace-file retention is the
operator's responsibility (append-only, no rollover, re-runs accumulate).
- mcp: extract the dispatch lambda (line length) and note the fresh-file-per-
request fallback when neither session id is present. Mark pure tests ^:parallel.
* ai-tracing: drop dead exports, document capture id, harden json-safe
- Remove the `sanitize-sql` re-export and `event!`: neither has a
non-test caller. Dropping `sanitize-sql` also removes ai-tracing's only
dependency on the `tracing` module (trimmed from `:uses`). The sites that
put a query in a span record the raw value on purpose (a scorer wants the
real SQL/MBQL), so the sanitizer was advertising an inconsistency the
design doesn't want.
- Document that `capturing`/`capture-reducible` is return-value-based: a
session id supplied to an inner `with-eval-session` is ignored because
capture is already active, so no `<id>.jsonl` is written on that path.
- `json-safe` now stringifies non-scalar map keys too, so the fallback can
never itself throw and orphan the root span it exists to save. Covered by
a vector-keyed-map regression test.
* ai-tracing: address review — gate pointers, bound json-safe, drop indirection
Correctness:
- capturing/capture-reducible no longer mint a session id, so the in-process
path is truly file-less (read :trace) instead of writing a stray <uuid>.jsonl
- gate the trailing eval_session pointer on a non-nil *session-id* so the
file-less path doesn't emit a pointer to a <nil>.jsonl
- add *retain-tree* so the in-memory span tree is only built when a caller
reads it back (capturing); the file-routed path no longer pins heap for the
whole run
- depth-bound json-safe so a deep/cyclic attr degrades to a string instead of a
StackOverflowError escaping emit!'s catch
- agent-api trace wrapper: only record a plain-data :http/response (skip a
streaming/opaque body rather than stringifying it); fix the stale comment
Cleanup:
- drop the eval-capture-enabled? indirection; entrypoints and tests consult the
ai-eval-capture setting directly
- with-eval-session: inline supplied-id, ^:once, generic 404 gate message
- extract mcp dispatch-method to de-nest the JSON-RPC case
- remove the dead ai-tracing.core/event! kondo entry
- keep eval JSONL spans out of the shared test log (log4j2-test.xml)
Tests: cover the agent-api trace wrapper, the file-less no-pointer/no-file
behaviour, and drive the log-sink assertions through with-eval-session.
* ai-tracing: fix route sort order and wire settings through init
Two structural test failures on the branch:
- Move the `/eval-trace` route into sorted position in the route-map
(keep-routes-sorted-test reads the file and asserts the keys are sorted).
- Add `metabase.ai-tracing.init` (requiring the settings ns), require it from
`metabase.core.init`, and add it to the module's `:api` — so
settings-namespaces-should-be-required-by-init-namespaces-test passes and the
`MB_AI_EVAL_CAPTURE` setting is registered deterministically at startup rather
than only when some instrumented namespace happens to load `ai-tracing.core`.
* ai-tracing: store span wall-clock timestamps as epoch millis, not nanos
Review: the epoch fields were nanos synthesized from currentTimeMillis (ms-
granular anyway), and precise sub-ms duration already lives in :dur-ms — so nanos
bought nothing. Rename :start-epoch-nanos/:end-epoch-nanos → :start-epoch-ms/
:end-epoch-ms (and the JSONL keys :start-epoch-ns/:end-epoch-ns → :start-epoch-ms/
:end-epoch-ms). End is still start + the monotonic duration (rounded to ms), so it
never reads a second, possibly backward-jumping wall clock.
visitDashboard waits on every dashcard query with cy.wait's default 5s
per-alias window; with 20+ cards the tail queries stagger behind the HTTP/1
concurrency limit and miss it (the dashcardQuery### "no request occurred"
flake). Add a dashcardTimeout option to visitDashboard and use it for the
large-dashboard visits in this test, so the downstream card assertions can
stay at their default timeouts. Also give the 20-request bulk move a longer
window, since its tail lags under CPU constraint.
The ci-conductor scripts are shared CI infrastructure: every supported
release branch runs them, and they keep evolving on master. A copy
committed to a branch goes stale, and syncing six branches by hand on
every change does not scale.
Add a prepare-ci-conductor action that overlays release/ci-conductor and
e2e/support/ci_conductor.ts from master, so a release branch runs the
current scripts while its workflow wiring stays branch-historical.
Anything headed for master uses its own sources, so a PR can still test
its own changes.
quarantine-gate and report-failures-to-ci-conductor delegate their bun
setup to it. prepare-cypress calls it too, because e2e/support/config.js
imports the reporter at Cypress bundle time and it has to be in place
before the run starts.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Implement schema, lib helpers, viz-settings + query -> query
* Implement grouping-set compilation
* Implement query nesting to support breakouts with parameters
For example, when selecting nested JSON fields in postgres,
the generated SQL query contains prepared query parameter
placeholders and the engine cannot determine that all copies
are the same expression.
* Keep dominant treemap share honest instead of rounding to 100%
The treemap percent formatter used a fixed 2 decimal places, so a dominant
leaf's share rounded up to a misleading 100.00% (e.g. 100000 vs 1 rendered
as 100.00% instead of 99.999%).
Mirror the pie chart: derive the decimal count from every node share via
computeMaxDecimalsForValues, so a tiny sibling forces enough precision to
keep the dominant share honest. Floored at the existing two decimals so
ordinary treemaps are unchanged, and a genuine whole still shows a clean 100%.
Fixes GDGT-2761
* Pass tree to test setup via SetupOpts object
* Delete react-router-redux and redux-auth-wrapper
* Dedupe redux to a single v5 copy
Removing the @types packages dropped a transitive redux@3 pin that kept a
typeless redux at the top level and masked a redux@4-vs-RTK-v5 type conflict.
Force a single redux@5 so types stay consistent.
* Migrate remaining react-router-redux references
Drop the now-dead eslint import bans for the deleted package, remove the
obsolete redux-action cast in auth logout, and reword stale comments. Keep the
metabase/router provenance docs that name the package as their reference point.
* First pass for updating Lib
* Update metabase.lib.query.test-spec
* All Lib + Lib BE tests are passing
* Fix remaining tests in Lib + QP
* Update Lib JS template tag functions
* cljfmt
* Another round of test fixes 🔧
* Leave NativeQuerySnippet :template-tags alone for now; test fixes 🔧
* cljfmt
* kondo
* Test fixes 🔧
* FE test fix? 🔧
* Another round of test fixes 🔧
* Test fix 🔧
* Test fixes 🔧
* Test fix 🔧
* Test fixes 🔧
* PR feedback
* PR feedback
* 🔧
* Test fix 🔧
* Use qualified db names for locks in snowflake.
We're seeing some timeouts in Snowflake on CI. There are a few reasons
for this, but part of it is that we are locking a lot waiting for
access to the datasets. These locks are currently based on unqualified
database names, not hashed and branch-qualified, so they lock more
often than they need to. Using the qualified names spreads things out
a bit.
In order to do this, we need to change the `dataset-lock` method to
take the whole dataset (to hash it) not just the dataset's database-name.
* Fix upload test to not assume table set is stable.
The purpose of this test is to ensure that the created table is gone,
but the assertion it makes is far too wide. This breaks in snowflake
where we have other tests randomly creating tables in the shared cloud
DB across jobs.
* Rename snippets and application_permissions plugin modules to TS extensions
* Convert snippets and application_permissions plugin modules to TypeScript
* [pr-3] Move SDK error types to embedding-sdk-shared
* [pr-3] Move SDK auth-config types to embedding-sdk-shared
* remove un-useful usage of SdkCollectionId to remove import of sdk code in core app
we don't really need that type and they both end up being `number | string`
* [pr-3] Move PLUGIN_NOTIFICATIONS_SDK to the core app plugins
I don't particularly like this: this is one of those "plugins" used
both to activate enterprise code and to inject behavior from the SDK
into the core app. We don't have an alternative at the moment, so this
is the change with the smallest blast radius. The plugin was already
used in the core app, so this just moves it to the right place.
* [pr-3] Move onEditQuestion from the SDK dashboard context to DashboardContext
DashCardMenuItems (feature/dashboard) reached into the SDK bundle's
SdkDashboardContext just for the onEditQuestion override.
I moved the onEditQuestion to the DashboardContext instead, as
it's used in the core app dashboard.
* [pr-3] Remove CommonSdkStoryWrapper from SimpleDataPicker stories
It was not needed
* [pr-3] Move useLazySelector to the SDK package
The hook exists to read the lazy-loaded bundle's redux store from the
npm package, so the package is its natural home. Its only bundle-side
consumer, useInitDataInternal, already receives the redux store as a
parameter and now subscribes to it directly instead of going through
the window props-store singleton.
* [pr-3] Introduce SdkSharedStoreState
Some core app code currently needs to read _one_ piece of information
from the SDK redux state, I moved that piece of info into a new type
called `SdkSharedState` which I documented as being something we
should avoid adding to in the future.
* [pr-3] Promote embedding-sdk-bundle to the app tier
Promotes the sdk bundle to app tier, it needs a special rule for the
bridge
module that we'll hopefully clean up in the future
* [pr-4] Promote embedding-iframe-sdk to the app tier
EAJS becomes app tier, but we need to whitelist the theme preview from
importing it
* [pr-5] Move DashboardSharingEmbeddingModal to the dashboard module
The modal is dashboard UI living under metabase/embedding, giving the
embedding module an outgoing edge into dashboard (it imports
metabase/dashboard/selectors). Move it next to its domain so the next
commit can enforce the embedding module's outgoing boundaries.
* [pr-5] Move UserForm and group helpers to the common module
UserForm is used by the new "invite people to view" modal
(InviteToViewModal, which lives in metabase/embedding, shared tier),
so it moves out of feature/admin down to metabase/common together with
GroupsMultiSelect and the group utilities it depends on. Import paths
updated, no logic changes.
* [pr-5] Move DashCardMenuItem to the embedding-sdk plugins contract
It's imported by shared/embedding-sdk but it lived in feature/dashboard,
moved it down to remove violation that will be enforced on one of the next
commits.
* [pr-5] Enforce outgoing boundaries for shared/embedding and shared/embedding-sdk
* [pr-1] Define embedding modules
Defines the embedding modules without promoting them yet: modules whose
back-edges still need cleanup stay at their current tier (shared, outgoing
unenforced). Each module is then promoted to app in its own follow-up PR,
together with the fixes its back-edges require.
* [pr-1] Promote embedding-sdk-package to the app tier
The npm package is an entrypoint and should be able to import from
anything, and nothing should import from it.
I raised it up as app tier
* [pr-1] Promote embedding-sdk-ee to the app tier
This should probably be "embedding-features" in the future, but for now
I'm moving it to app to be above enterprise (which it imports from) and
to prohibit other things from importing from it
* [pr-2] Move LegacyStaticEmbeddingModalProps to the plugins contract
Module `embedding-iframe-sdk-setup` will be moved to app tier, so we move
`LegacyStaticEmbeddingModalProps` to the oss plugin file to avoid an import
going up to app
* [pr-2] Promote embedding-iframe-sdk-setup to the app tier
`embedding-iframe-sdk-setup` imports EAJS, which is app tier
so this also needs to be brought up to app tier
TBD how we want to structure this, but it kinda makes sense that some
parts of the core app are above EAJS as they use it (we also use it
to render theme previews in the admin settings for themes)
* Move root route onEnter/onChange/onLeave into components and effects
* Replace data-studio index onEnter redirect with a component
* Address review: settle LoadCurrentUser on rejection, drop test any-cast
* Point test Route import at the metabase/router facade
* Freeze has-user-setup on entry so the setup wizard is not interrupted
* Colocate route lifecycle components with the modules that own them
* Drop onEnter/onChange/onLeave from the facade's Route props
* Update to typescript 7.0 (beta)
* Switch to tsgo in scripts
* Remove NODE_OPTIONS from typecheck script
* Use typescript 7 instead of native-preview, but keep 6 around for lint
* Use typescipt 7.0.2
* Use real typescript 6.0.3 for eslint, run tsc 7 by explicit path
* Fix type errors surfaced by typescript 7
* refactor(sql-parsing): protocol + graal split, render.js-style context pool
Mirror the static-viz renderer structure (channel/render/js) for the sqlglot
parser:
- protocol.clj: strongly-typed SqlParser protocol for the sql_tools operations
- graal.clj (replaces pool.clj): GraalPy implementation on a 0..2 dirigiste
pool; contexts share one refcounted Engine and one bootstrap Source, created
with the first context and closed with the last, so an idle pool releases
all native memory (replacing the old TTL/min-size-1 leak mitigation);
timed-out contexts are interrupted and disposed instead of released
- common.clj: create-pool copied as-is from render.js.common (replaces the
PythonEval protocol); the future external-python impl will share it
- core.clj: public API unchanged; keeps JVM-side pre/post-processing
(VALUES stripping, field/error conversion) and delegates through the
protocol
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* tune(sql-parsing, static-viz): 0..3 pooled graal contexts, 10-minute idle reap
:idle-minutes is now an explicit create-pool option in both pool factories
(no default): graal pools (sqlglot + static-viz) keep idle workers for up to
10 minutes, the node renderer pool stays at 1.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(sql-parsing): native CPython parser, mirroring the static-viz node renderer
- python.clj: :python SqlParser running sqlglot in a pool of external CPython
child processes speaking newline-delimited JSON over stdin/stdout
(sql_tools_cli.py); in dev the child imports straight from
resources/python-sources, from the jar the sources are written to a fresh
temp zip per spawn (zipimport) and deleted with the process; single-process
pool (0..1, 1-minute idle reap), timeout kills a wedged process, one retry
when an acquired process turns out dead, call-level Python errors release
the healthy process back
- settings.clj + parser.clj: sql-parsing-mode setting (:graalvm default |
:python) and the mode-selection namespace, like static-viz-mode/renderer
- common.clj: python-sources locations + dev sqlglot install move here from
graal.clj (shared by both transports), plus make-parser building the
protocol reify over a transport call fn (marshalling written once)
- fix latent nil-schema bug in validate-query arg encoding: (json/encode
(or schema "{}")) double-encoded, making Python treat the string "{}"
as a strict-mode schema; encode an empty map instead
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(sql-parsing): transport-agnostic parse errors; 10-minute idle for process pools
Both parser transports now throw the same ex-info for Python-side failures
(common/call-failed-ex: :sql-parsing/error + extracted Python error type) —
graal wraps PolyglotException, the CPython transport wraps its ok:false
responses — and core exposes parse-error? so callers stop catching
PolyglotException (sql-tools.sqlglot updated; a python-mode instance
previously bypassed its parse-error fallbacks entirely).
Also raise the external-process pools (sql-parsing python, static-viz node)
to a 10-minute idle reap, matching the graal context pools.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(sql-parsing): un-nest testing blocks
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* build(docker): install python3 for the :python sql-parsing mode
Both images already carry nodejs for the :node static-viz renderer; add
python3 (~45MB on alpine) so MB_SQL_PARSING_MODE=python works out of the box.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* build(docker): default sql-parsing to python and static-viz to node in the entrypoint
The image ships python3 and node, so prefer the external-process backends
(lighter and faster than in-process GraalVM) unless MB_SQL_PARSING_MODE /
MB_STATIC_VIZ_MODE are set explicitly, following the MB_JETTY_HOST pattern.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* tune(static-viz): keep the node process pool at a 1-minute idle reap
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(sql-parsing): wire settings into module init
metabase.core.init-test requires every .settings namespace to be reachable
from an init namespace; add metabase.sql-parsing.init and register it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* Resolve the default search engine from capability precedence
The search-engine setting's :appdb default meant the engine precedence
list never chose semantic, no matter how many of its dependencies were
met. Engine selection now works in three layers:
- supported-engine? is a pure capability check (app-db type, premium
features, infrastructure) and no longer reads the search-engine
setting.
- The default engine is the configured override when servable,
otherwise the first supported engine in the precedence
(semantic > appdb > in-place). Unknown configured values warn once
and degrade instead of breaking all of search.
- Indexes are maintained for the default engine, the engines it
declares as dependencies (semantic mixes in appdb results), and any
engines opted in via the new additional-search-engines setting.
Reading the search-engine setting now resolves the engine that
actually serves search, so the admin UI needs no changes; the raw
override is available via configured-search-engine.
An explicitly passed per-request search_engine that cannot serve now
returns a 400 naming the cause (unknown / not supported / not enabled)
instead of silently falling back; legacy engine cookies keep the
fallback. The engine-active prometheus metric now reports active-set
membership rather than capability.
MB_SEMANTIC_SEARCH_ENABLED remains a kill switch, folded into the one
canonical semantic availability predicate (so repair/cleanup tasks now
respect it too); it will be removed once Cloud stops setting it.
Part of BOT-1832
* Tighten search engine docstrings and comments
* Validate cookie-carried search engines before use
A stale engine cookie naming an unmaintained index (e.g. appdb on an
in-place-default instance) now degrades to the default engine instead
of querying a missing index. The in-place exemption in engine-active?
uses = rather than isa?, so derived engines don't inherit a free pass.
Updates the prometheus search-engine-metrics-test to the new
engine-active semantics.
Part of BOT-1832
* Serve per-request engines through validated query-params; gate semantic writers on activity
Review follow-ups from roborev job 2600 and the workflow review:
- The engine middleware now reads and writes :query-params - the
:params injection had been dead since the api.macros migration, so
engine cookies had no effect on the served engine. Cookie values are
validated (stale/unknown degrade to the default), and param values
are normalized: blank means absent, the qualified form the API
returns in :engine is accepted, and the legacy fulltext name maps to
appdb at the boundary so activity checks use exact matches.
- engine-status in search.engine is the one classification of whether
an engine can serve (:ok/:unknown/:unsupported/:inactive); the API
layer maps it to 400s and no longer re-encodes engine invariants.
- Semantic fallback (EE core and the metabot search tool) prefers
engines with a maintained index over merely supported ones, so an
in-place default with additional-search-engines=semantic mixes with
the warm appdb index instead of in-place.
- The semantic indexer and index-repair tasks gate on the new
semantic-search-active? instead of availability, so a supported but
inactive semantic index no longer accrues hourly embedding spend.
- active-engines guards the pre-init nil default and only
capability-checks dependency entries.
Part of BOT-1832
* Canonicalize legacy engine names at every boundary; tolerate repeated engine params
search.engine/canonical-engine resolves legacy names (fulltext -> appdb)
for configured values, additional-search-engines, and request
params/cookies alike, so active-engine membership compares exactly.
A repeated search_engine query param parses as a vector; the middleware
now passes it through for schema validation to reject with a 400
instead of throwing.
Part of BOT-1832
* Review cleanups: single evaluation of engine resolution values
Bind configured-engine, the parsed param engine, and active-engines
once per expression instead of re-evaluating (Copilot); remove blank
lines inside the check-engine-serves! case form (CodeRabbit).
Part of BOT-1832
* Engine-active metric means can-serve; harden param parsing and dependency filtering
engine-active shares engine-status's definition: in-place is always
active while it exists, indexed engines while their index is
maintained. Fixes the prometheus test assertion that compared a metric
to itself.
param->engine strips the search.engine/ prefix rather than splitting on
slashes, so junk values 400 with the full value named instead of
validating their last segment, and degenerate values like // cannot
produce an empty or nil keyword.
active-engines filters dependency engines through known-engine? before
supported-engine?, matching the startup-ordering guard in
supported-engines.
Part of BOT-1832
* Reject malformed explicit engine values instead of treating them as absent
A non-blank search_engine that strips to a blank suffix (e.g.
search.engine/) parses to an unknown engine and 400s naming the raw
value; only genuinely blank values fall through to the cookie/default.
Part of BOT-1832
* Clarify the unknown-engine filter comment in supported-engines
* Fix review findings: runtime kill switch, csv hygiene, cookie unpinning, shared fallback
- Semantic Quartz jobs are scheduled when the instance is capable
(pgvector + feature) and check availability/activity per execution,
so the kill switch works at runtime again instead of gating job
scheduling at boot.
- semantic-search-active? is plain active-set membership; the HNSW
build gates on it like every other index writer.
- additional-search-engines values are trimmed, blanks dropped, and
qualified/legacy names canonicalized (prefix handling now lives in
canonical-engine, the one boundary fn).
- An explicit blank search_engine clears the engine cookie (the unpin
mechanism this branch had removed) instead of being ignored.
- fallback-engine lives in search.engine; the EE semantic core and the
metabot search tool share it.
- The runtime-activation backfill note moved from the
additional-search-engines description to a comment.
Part of BOT-1832
* Initialize the semantic index from repair when none is active
Semantic can become active at runtime (kill switch re-enabled, or added
to additional-search-engines) without init! ever having run; the hourly
repair task now initializes and backfills in that case instead of
no-oping until a manual /re-init.
Also renames warn-once to warn-once! per the side-effect convention and
adds end-to-end coverage for the pinned-cookie and blank-unpin paths.
Part of BOT-1832
* Cover the repair-initializes path; unpin test asserts the resolved default
The blank-unpin api test compared against a hard-coded appdb default,
which does not hold on MySQL/MariaDB app dbs. New repair test starts
from a blank pgvector db and asserts repair-index! creates an active
index and gates the documents.
Part of BOT-1832
* Trigger search index init when additional engines are set at runtime
Registers the (previously dangling) init-job-key as a durable,
trigger-less quartz job wrapping the search init, and the
additional-search-engines setter fires it when it exists. In tests the
scheduler is not running, so the setter stays side-effect free there;
env-var configuration is covered by the startup init.
Part of BOT-1832
* Rename the engine cookie and cover the pin/unpin round trip
metabase.SEARCH_ENGINE becomes metabase.SEARCH_ENGINE_OVERRIDE, so the
population of long-lived cookies written before engine validation
existed stays inert.
The round-trip test covers pinning via an explicit param, serving from
the cookie, and clearing it with a blank param.
Part of BOT-1832
* Order semantic capability gates cheapest first
The license check short-circuits before the pgvector URL check, whose
slot becomes a DB probe under BOT-1796.
Part of BOT-1832
* Only trigger search init when the setting change activates an engine
Semantic init re-gates every searchable document, so an unconditional
trigger made any write to additional-search-engines cost a full app-db
sweep. The setter now diffs active-engines across the set and triggers
only for newly activated engines.
Part of BOT-1832
* Defer the engine-activation trigger to after commit
The activation diff and quartz trigger run via do-after-commit in a
future (extracted to trigger-init-for-newly-active-engines!), observing
the final committed settings state - a surrounding set-many! may
include the setting that makes the engine supported - without I/O on
the transaction's thread.
The cookie round-trip test uses the in-process client so the handler
runs inside the fixture's engine and index bindings.
Part of BOT-1832
* Review nits: log async trigger failures, quote the cookie regex, fix a docstring
The post-commit init trigger logs exceptions instead of dropping them
in the future; the cookie test quotes the cookie name before building
its regex; build-hnsw-index-async! says active, matching its gate.
Part of BOT-1832
* Schedule semantic jobs on the boot-static gate; indexer initializes missing indexes
Job scheduling happens once at startup, so it now gates on the only
boot-static input: whether a pgvector DB is configured. The runtime
gates (license, kill switch, engine activity) are checked per
execution, so applying a license or re-enabling the kill switch at
runtime no longer strands semantic as an unindexed default until
restart.
The indexer's 20-second tick initializes the index when it is missing,
so every runtime activation path (license, kill switch, setter on a
stale cluster node) converges within seconds instead of waiting for the
hourly repair or a restart.
Also: the blank-param unpin deletes the engine cookie (max-age 0)
instead of writing an empty 20-year one; the activation trigger helper
lives in search.task.search-index with static requires; the setter test
uses poll-until for the async positive case and a direct synchronous
call for the negative.
Part of BOT-1832
* Runtime self-heal initializes all active engines and the HNSW index
The indexer's missing-index branch runs the generic search init, which
covers newly active dependencies, and applies the same HNSW-strategy
safety net as task registration.
Part of BOT-1832
* Ensure the HNSW index on the indexer's active path
An index-backed strategy configured while semantic was inactive misses
the setter's build event; the indexer tick now fires the async build
when the active index lacks its HNSW index. The pg_indexes existence
check is self-limiting: CONCURRENTLY registers the index when the build
starts.
Part of BOT-1832
* Fix engine tests on app dbs without appdb search support
Explicitly requesting an unservable engine now 400s instead of silently
falling back, so tests must not hard-code appdb: the data-studio search
tests iterate only supported engines, the cookie staleness test pins
in-place (servable everywhere), and the not-enabled case runs only
where appdb is supported.
Also realigns warn-once! call args (cljfmt).
Part of BOT-1832
* Keep semantic active under the search test fixtures
active-engines now derives from the default engine, so fixtures that
pin default-engine were silently deactivating semantic: semantic
queries in the metabot search tool fell through to the term engine.
Both fixtures pin active-engines to the set the old derivation gave
them - appdb plus semantic when supported, or semantic alone for
legacy search.
Part of BOT-1832
* Rename with-new-search-* test fixtures to with-appdb-search-*
The appdb engine predates semantic search; new no longer disambiguates.
Part of BOT-1832
* Update the last supported-implies-active consumers
The metabot semantic-search feature predicate gates tool schemas on
engine-status :ok rather than capability, so semantic_queries is only
advertised when a semantic index is actually maintained.
The metric collector runs only when semantic is active, and index
cleanup no-ops until the bookkeeping tables exist: available but never
initialized is now a steady state, and both jobs would otherwise warn
or throw about missing tables on every run. Cleanup still runs while
merely available so an opted-out instance's leftover indexes get
dropped.
Part of BOT-1832
---------
Co-authored-by: Tyler Plude <tsplude@gmail.com>
* Migrate query_builder/analytics to TypeScript
* Type Card.creationType and drop analytics cast
Address review: type Card.creationType as a union instead of a plain
string, narrow Question.creationType(), and make the new_question_saved
type field optional/nullable to match the Snowplow schema. Removes the
cast in the analytics tracker.
* Drop as const in load-question via Card annotation
* Re-own react-router-redux mechanisms behind the metabase/router facade
* Wire store, app and store types onto the re-owned router facade
* Re-point react-router-redux call sites onto metabase/router
* Update router tests and harness for the re-owned facade
* Ban react-router-redux imports outside metabase/router
* Model routing state's null location contract instead of casting
* Flatten CallHistoryMethodAction into per-method history action types
* Match history actions with ts-pattern in routerMiddleware
* Preserve explicit null location in createMockRoutingState
Adds `bun run email-dev`: a small Bun server + static page that renders
the Handlebars email templates in src/metabase/channel/email/ with
editable sample data, so designers and copywriters can iterate on email
copy and layout without running Metabase or sending real emails.
- live-reloads when a .hbs file changes on disk (1s polling)
- Fields/JSON panels to fill in template values; edits persist in
localStorage per browser
- Desktop/Narrow/Phone width toggles and Copy HTML
- sample-contexts.js mirrors the real send-time contexts (traced from
messages.clj and the notification handlers)
- mirrors the jknack custom helpers (equals, format-date, now, initials,
card-url, dashboard-url, trash-url) in handlebars.js
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* Migrate Link, ListItem, and LoadingSpinner from Emotion to Mantine + CSS modules
* Fix TableList disabled-state assertions by exposing data-disabled instead of relying on Emotion-injected styles
* Name the referring entity in remote sync not-found errors
When an import references content that doesn't exist on this instance, the
error named only the missing entity's raw serdes id. It now also names the
entity holding the dangling reference, and backtick-quotes names and ids so
leading and trailing whitespace is visible.
`load-one!` takes the referrer as an explicit parameter rather than carrying
it on the load context: `load-deps!` reduces over sibling deps and each
recursive `load-one!` returns its own context, so a context key would leave
later siblings labelled with the deepest child's referrer.
Message construction lives in `missing-reference-message` so the format can
change without touching the throw sites.
* Attach the referrer by catch-and-rethrow instead of threading it
Replaces the extra `load-one!`/`load-deps!` parameter with a try/catch around
the dependency load. The nearest enclosing `load-one!` unwinds first, so it is
the frame that attaches itself as the referrer; outer frames see the key
already present and rethrow unchanged. The original exception is kept as the
cause, preserving the stack trace of the detection site.
Adds an end-to-end assertion in bare-import-test that a real import of a Card
referencing a missing Database carries the Card as `:referrer`, which the
hand-built ex-info unit tests could not cover.
* Key the database-not-found error off ex-data, and name the referring card
`source-error-message` detected this case by matching "database not found"
against the cause's message text, then interpolated that raw internal string
into the user-facing error:
Import failed: A referenced database does not exist on this instance.
table id present, but database not found: [clickhouse nil some_table]
`import-table-fk` now tags the throw with `:error ::database-not-found` and
`:db-name`, and the render walks the whole cause chain looking for that key
rather than checking only the immediate cause. `::load-failure` carries the
referring entity, so both missing-content paths now render through
`missing-reference-message`:
Import failed: Card `Some card` (`abc123`) references Database (`clickhouse`),
which does not exist on this instance. ...
Also fixes `:database-names` in that ex-data, which selected names from
:model/Table rather than :model/Database. It has no consumers.
The end-to-end test uses a result_metadata `:table_id`, since that is the path
that actually reaches `import-table-fk` with an undeclared database:
`result-metadata-deps` derives deps only from `:field_ref`, and a Card's own
top-level `table_id` is recomputed from `dataset_query` by
`populate-query-fields` on insert.
* Backtick-quote file paths in remote sync ingest-error messages
GHY-3992 asks for content names to be quoted so leading and trailing
whitespace is visible. The ::not-found path already did this; the
ingest-errors branch still joined raw paths, and the message already uses
": " between path and reason and "; " between entries, so a path containing
punctuation or trailing space was ambiguous to read.
No fallback is added for a nil :file. All three ingest-error producers
(ingest.clj file-hierarchy!, ingestable.clj read and parse failures) always
set it, and only YamlIngestion and IngestableSnapshot produce a non-empty
ingest-errors; the other Ingestables delegate.
Splits source-error-message-entity-not-found-test into three deftests, one
per error branch, which kondo requires now that each has grown assertions.
* Render load-failure errors with the entity name, reason, and stripped keys
::load-failure fell through to the generic "Failed to reload from git
repository: <ex-message>" fallback, showing a NanoID and burying the
stripped-keys note. It now reads:
Import failed: could not save Card `Orders by Month` (`abc123`).
NOT NULL constraint failed: report_card.display.
and, when the entity was part of a dependency loop, adds "It may have been
saved without: `dashcards`, `parameters`." — a partial row for it survived
the import, which the user previously had no way to learn.
Renames the ::load-failure ex-data key `:referrer` to `:entity`. It names the
entity that failed to load, which is a referrer only relative to the missing
database in the ::database-not-found case; that branch now reads `:entity` and
passes it as the referrer. ::not-found keeps `:referrer`, where it really is
the parent.
The new branch is ordered after the tenant-collection and database-not-found
branches, both of which match on causes that a ::load-failure wraps.
::circular is left on the fallback. Rendering its cycle needs an ordered
expansion chain — `expanding` is a set — and it only fires when the
key-stripping recovery has already failed.
* Address CodeRabbit review
- Remove a duplicated `testing` block in impl_test.clj. It was byte-identical
to the one above it, introduced when the deftest was split. Replaced with a
doseq covering trailing, leading, and no surrounding whitespace.
- Mark `missing-reference-message` and `load-failure-message` private; neither
is used outside impl.clj.
- Capture the exception once in bare-import-test's not-found case instead of
running the same failing import twice, and flatten the resulting nested let.
* Fix metabase.metabot.api/openrouter-model-group
Fallback to parsing the id if the display_name has no ":"-separated prefix.
* Add a default OpenRouter model so connect-without-model works
Without an entry in default-llm-metabot-model-by-provider, a PUT /api/metabot/settings that connects openrouter with
only an API key would save the key but silently fail to switch llm-metabot-provider.
* Use real dotted OpenRouter model IDs in defaults and docs
OpenRouter model IDs use dots in version numbers, e.g. anthropic/claude-haiku-4.5. The hyphenated
anthropic/claude-haiku-4-5 referenced by the default args and docstrings does not exist.
* FE: enable openrouter provider in metabot admin UI config
* Whitelist openrouter models
Filter openrouter list-models down to the same curated Anthropic and OpenAI model set the direct providers support,
instead of returning the full ~340-model catalog.
* Throw an exception if callers pass :ai-proxy true for openrouter
Similar to bedrock and openai, supporting this would require work on the ai-service side, and we already reject
non-anthropic providers in validate-metabase-managed-provider!
* Remove explicit credentials check in list-all-models
We want to allow callers to omit the creds and fallback to the configured api key from settings. core/resolve-auth
still handles the auth check and will throw if no credentials are provided or configured.
* Support prompt caching for openrouter
Report `:cacheCreationTokens` and `:cacheReadTokens` in the `:usage`.
For anthropic models, add the `:cache_control` breakpoint on the system message to enable caching (openai caching is
automatic).
* Apply code review suggestion in AIProviderSettingsSection.unit.spec.tsx
Co-authored-by: Maksym Yakubych <undrfined@gmail.com>
* Add gpt-5.6 and drop gpt-5 from openrouter supported-models
See also #77450
* Apply code review suggestion in fe tests
Co-authored-by: Maksym Yakubych <undrfined@gmail.com>
---------
Co-authored-by: Maksym Yakubych <undrfined@gmail.com>
* Move remote-sync branch switching into Settings with guard rails (GHY-4019)
Branch switching ran a full import that hard-deletes any synced-collection
content absent from the target branch, and it lived as a routine app-bar
dropdown item. A forced switch could permanently delete never-pushed content.
Backend:
- spec: detect collection-scoped deletion conflicts (Cards/metrics, Dashboards,
Documents, Timelines), not just the transform family; wire into get-conflicts
- import: expose force_deletion on POST /import so a forced version/dirty
override no longer silently forces deletions
- async-import dirty guard now returns the dirty objects (what would be lost)
- PUT /settings rejects a read-write branch switch (must use the guarded import);
read-only changes and first-time config still allowed
Frontend:
- remove the Switch branch affordance from the app-bar sync controls (branch
name stays as a read-only label; push/pull intact); drop the switch-branch
conflict variant
- add a guarded branch switcher to the read-write Settings panel: destructive
warning copy, hard block when there are unsaved changes (lists them, no force
path), destructive confirm when clean, stale-tab handling
* Refine Settings branch switcher: choice modal, admin gating, direct clean switch
Follow-up to the branch-switching rework, from review feedback:
- Offer a choice modal (push / new branch / discard) when switching with
unsaved changes, instead of a hard block — restores the switch-branch
conflict variant, now driven from the Settings panel BranchSwitcher.
- Drop the destructive confirmation on a clean switch: with everything pushed
the switch is reversible, so it runs directly.
- Show a red "N unsaved change(s)" label beside the dropdown when dirty.
- Gate the switcher on superuser: non-admins (e.g. settings managers) see the
section with an "admin required" message instead of a control they can't use;
the section no longer depends on the admin-only branch setting being readable.
- Match the section subtitle styling to the other settings sections.
- Strengthen the import superuser test to cover a branch switch.
* Update remote-sync E2E for Settings-panel branch switching
Branch switching moved from the app bar to the Settings panel, so the
branching specs and helpers drive it there now:
- Replace the app-bar switch helpers with Settings-panel ones
(visitRemoteSyncSettings, getSettingsBranchSwitcher,
createBranchViaSettings, switchBranchViaSettings).
- Rewrite the branching tests to create/switch branches on the Settings
page, navigating back to the app for content changes.
- Switching with unsaved changes opens the choice modal (wait for the
unsaved-changes indicator first); a clean switch goes directly.
- Snowplow triggered_from: app-bar -> admin-settings.
* cljfmt
* Address review: guard-bypass fix, anti-join, shared removal predicate
- api: block blanking the branch in read-write mode too, so it can't be used to
reset the branch-switch guard and then switch freely; regression test added.
- spec: compute collection deletion conflicts with a SQL anti-join + count/limit
instead of loading every candidate and an exists? per row, so it scales when a
switch would drop a large synced collection.
- Extract spec/removal-where-clauses as the single definition of what an import
reconcile removes for a model; remove-unsynced! and the deletion-conflict check
both build their WHERE from it (removes the duplicate build-entity-id-where-clause).
- Add a parity test asserting the conflict warning is exactly the unsynced subset
of what remove-unsynced! removes.
- Nits: be.visible for the dirty-warning indicator; wrap a long test string.
* Build the deletion-conflict WHERE with a thread-first instead of nested into/conj
* Give check-deletion-conflicts the same anti-join as the collection check
Was doing a t2/exists? per candidate (and materializing every candidate).
Now counts via one SQL anti-join, sharing removal-where-clauses and the new
model-id-column / unsynced-anti-join helpers with check-collection-deletion-conflicts.
Adds a direct spec-level test for it.
* Build collection deletion conflicts with cond-> instead of if/into
* Cover NativeQuerySnippet in the deletion-conflict warning
Generalize check-collection-deletion-conflicts -> check-content-deletion-conflicts:
warn before an import deletes any unsynced entity-id content, not just collection-
scoped models. This pulls in NativeQuerySnippet (global, no collection scope), which
otherwise fell through both conflict checks and could be silently deleted on a forced
import/branch switch. Collection stays excluded (structural; its contents are covered).
Adds a snippet test case.
* Address branch-switch review: drop force_deletion param, refetch dirty at switch, add tests
- Remove the unused force_deletion param from POST /import (schema, destructure,
and async-import! call); internal force-deletion? threading is unchanged.
- BranchSwitcher refetches dirty state at switch time so a stale settings-page
snapshot can't route a switch down the wrong (delete-without-warning) path.
- Add BranchSwitcher unit tests: non-admin message, dirty count label,
clean direct switch, and the choose-what-to-do modal on the dirty path.
* Left-align the branch value in the settings switcher for consistency with other selects
* Show a 'Using <env var>' note when the sync branch is pinned by an env var
* Fix branch-switcher e2e: scroll the switcher into view, drop spurious branch-switched event
The Sync branch section sits below the fold at the default viewport, and Cypress treats an element
clipped by a scrollable ancestor as hidden, so the switcher assertions failed (surfaced by
testing-library as a misleading "Unable to find an element"). Scroll to it before asserting.
BranchSwitcher also reported a branch switch when creating a branch, emitting a spurious
remote_sync_branch_switched alongside the branch picker's remote_sync_branch_created. Creating a
branch forks the current one without a reconcile, so only the creation is reported now.
Introduces a `QUARANTINE_ENGINE` variable that acts as a control knob. Either Trunk enforces a quarantine or CI conductor does. So far wired only in three places:
1. fe
2. be
3. e2e
* Preserve native model column types during serialization (GHY-4043)
export-result-metadata checked (model? card) before the native branch, so
native models were routed through lib/model-preserved-keys — a metadata-merge
soft-key set that omits :base_type/:effective_type. On a read-only Remote Sync
target the column types could not be re-derived without executing the SQL, so
computed columns (e.g. STRING_AGG(...) AS names) fell back to type/* and their
filter UI collapsed to Is empty / Not empty.
Reorder the cond so native cards — models included — go through the branch that
preserves structural type info; only MBQL models, whose types are re-derivable
from the query at import, use the soft-key set.
* Update native-model extract test for preserved column types
extract-result-metadata-native-model-test asserted the old behavior — that a
native model dropped :base_type on export. Update it to expect structural type
info to survive, matching the GHY-4043 fix. Drop the now-unused clojure.set
require.
* Ensure openai reports `:cacheReadTokens` in `:usage`
Add `openai-usage->aisdk-usage` to convert openai usage format to aisdk format and report `:cacheReadTokens`.
* Add undocumented :cache_write_tokens as well
`parse-grant` lowercased role names parsed out of `SHOW GRANTS FOR <user>`.
Those names are fed back into `SHOW GRANTS FOR <user> USING <roles>`, and
MySQL 8 treats backticked role identifiers as case-sensitive, so any role
with uppercase characters made the follow-up query fail with error 3530 and
took the whole metadata sync step down with it.
Lowercasing is still correct for the privilege types in the sibling
`:privileges` branch; it was never right for role identifiers.
Also add the missing space before `USING` in the follow-up query, which
only parsed because the user identifier ends in a backtick.
* Static-viz: dispose ECharts SSR instances + faster SVG dominant-baseline patch
Two isolated, output-preserving fixes to the static-viz render path:
- Dispose the ECharts/zrender SSR instance after renderToSVGString() in every
chart component. These instances were never released, leaking ~MBs per render
and OOMing a long-lived renderer process; memory is flat after.
- Replace patchDominantBaseline's full hast parse+serialize round-trip over the
whole SVG with a targeted string transform (like the sibling
transformSvgForOutline), cutting a large share of big-chart render time with
byte-identical output.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Update BoxPlotChart.tsx
* Update ComboChart.tsx
* Update PieChart.tsx
* Update SankeyChart.tsx
* Update ScatterPlot.tsx
* Update TreemapChart.tsx
* Update WaterfallChart.tsx
* Drop hast deps entirely; svg test uses DOMParser
The SVG dominant-baseline patch no longer uses hast, and its test was the last
user of hast-util-from-html. Rewrite the test with the jsdom DOMParser and
remove hast-util-from-html and hast-util-to-html from the deps.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Fix formatting in ComboChart.tsx
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* [Metabot] Migrate BE to v2 message format (#75672)
* migrate codebase to writing v2 format
* move to camelCase to match upstream schema changes
* improvements
* improvements
* correction
* metadata corrections
* handle string outputs correctly
* put add error code to message metadata in finish message
* self-review pass
* record step start to db
* correct issue in claude provider that sent too long of message ids
* record text state to open up future ability to write in progress work
* increase malli usage
* limit client streamed output for tool calls
* emit cache token usage correctly
* ensure started before error is emited
* update migration version
* [Metabot] FE read new message format (#76429)
* initial impl
* self review
* update e2e tests
* copilot feedback
* pr feedback
* fix lint
* feedback - test helper ergonomics
* misc. fixes to metabot chat improvements integration branch (#77321)
* fixes due to stale branches, correct data- prefix not getting recorded, remove unused code
* clean up debug visuals
* ci fixes
* fix couple more tests
* pr feedback
* test: add repro for SDK viz-settings column edits being dropped
Reproduces EMB-2057 — in modular embedding (embed.js), removing a
column via Visualization settings > Add or remove columns silently
no-ops. Fails against the current QuestionSettings.tsx onChange,
which drops the second question argument carrying the query edit.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: forward query edits from viz settings in SDK QuestionSettings
QuestionSettings.tsx's onChange dropped the second `question`
argument that ChartSettings forwards for query-level edits (e.g.
add/remove columns), applying only the settings to the stale closure
question and never re-running the query. Column-removal edits landed
in the query object but the table kept rendering the old result.
Forward `nextQuestion` when present and re-run only in that case, so
pure viz-settings-only edits (no query change) keep their existing
no-run behavior.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Fix code format
* move test to where it belongs
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Currently they are timing out with some regularity in CI. We've done
this same thing to bigquery and redshift already.
Not only will this get us pass/fail results more quickly; it also
reduces the scope of failures so retries don't have to redo as much
wasted work.
* docs: add page for treemap visualizations
* Optimized 1 images, saved 237.8 KB (44.8% smaller)
* fix broken link
* Edits from Jeff
* Edits from Robert
* Edits from Jeff 2
---------
Co-authored-by: Metabase Automation <github-automation@metabase.com>
Stop running metabase-enterprise.transforms-inspector-test in cloud DBs.
These tests create new tables in the default schema, which is fine for
most drivers, but not for cloud DBs like snowflake and bigquery, because
concurrent CI jobs share the same jobs, which means this will cause test
failures in other unrelated tests which list tables, such as
metabase-enterprise.impersonation.driver-test/resilient-connection-options-test
We need to get these tests to stop creating tables in the default schema,
then we can drop the requirement for the :test/dynamic-dataset-loading feature.
* Replace the group picker in the user form with a multiselect
* Port the tenant group-picker e2e steps to the new multiselect
* Add group-manager controls and a section separator to the multiselect, with chevron and Backspace fixes
* Fix external-default group ordering in the multiselect dropdown, with manager-flag and divider tests
* Update frontend/src/metabase/admin/people/components/GroupsMultiSelect/GroupsMultiSelect.tsx
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* Update remove-button test assertions for the group-specific aria-label
---------
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* Serialize static-viz GraalJS context creation (GHY-4077)
Even with the shared-Source code cache (#77180), evaluating the bundle
in a fresh context churns through ~500 MB of allocation. That memory is
transient — allocated and discarded during the eval, with only ~10 MB
retained per extra live context — but it all still has to pass through
the heap. The dirigiste pool grows during render bursts, and nothing
stopped 2-3 context creations from running concurrently: ~1-1.5 GB of
allocation churn stacked on top of in-flight renders, which on a 2 GB
heap outruns the collector and drives GC thrash / OOM.
Serialize creation on a private monitor so at most one generate runs at
a time; waiting acquires just queue on the pool. Also jitter the
10-minute expiry timestamps (+/- 3 min) so pool members created in the
same burst don't all expire and regenerate in the same window.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Document static-viz pool memory model in namespace docstring
Capture what the OOM investigation measured so the pool's design
choices stay legible: engine + parsed sources ~220 MB retained (paid
once); fresh-context eval ~500 MB of collectable churn with only
~16 MB retained per live context (first-in-JVM cold parse ~1.2 GB);
renders ~1 MB/row transient. Also spell out why creation is serialized
on a monitor rather than time-throttled: bursts genuinely need
multiple contexts, so the pool still grows — just one creation at a
time.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* Make test stats upload more robust
* Fix trailing whitespace
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* Fix rendering of tenant avatars
* Remove unused Avatar code, cleanup initials logic, more explicit types
* Fix type used in isUser
* Stricter types to avoid initial bug in the future
* Wire embedding tests to ci-conductor reporting
* Add ci-conductor quarantine gate to embedding Cypress jobs
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Add withRouteProps facade shim for the router migration
* Convert DatabaseListContainer to a function component on facade hooks
* Put route-prop reference containers on withRouteProps
* Require route props in withRouteProps generic constraint
* Convert ScrollToTop from withRouter to useLocation hook form
* Fix merge fallout: rename RouteProps shim and repoint react-router imports
* Facade re-exports full react-router surface via a single leaf seam
* Re-point all react-router imports onto the metabase/router facade
* Ban react-router imports outside the metabase/router facade (eslint)
* Migrate InviteToViewModal Link import onto metabase/router facade
* Add serialization-dependencies multimethod + Card impl (GHY-4010)
Foundation for export-time dependency validation. serialization-dependencies
runs on a raw appdb entity and returns its references tagged :content (must be
in the export) or :data (database/table/field that must exist in the appdb).
Unlike the load-time `dependencies` multimethod, it operates pre-serialization
and includes tables/fields, which import otherwise creates on the fly.
The Card impl derives content deps from `descendants` (so the two can't drift)
and adds data-model deps from the query via the lib walkers. Not yet wired into
the export pipeline.
* Replace escape analysis with general dependency validation (GHY-4010)
extract-subtrees now runs a dry-run pass (collect-dependencies) that shares the
real export's extract queries but neither serializes nor writes. For every entity
that will be extracted it derives serialization-dependencies and checks them:
content references must be in the visited/extracted set (or be analytics cards),
and data-model references (database/table/field) must exist in the appdb. Any
unsatisfied reference aborts the export (unless continue-on-error), before any
archive is produced.
This generalizes part A's Card-only, collection-id escape analysis: it now also
catches non-Card content escapees and dangling data-model references (a deleted
table/field that would otherwise serialize to a malformed [nil nil nil] ref).
Adds a Dashboard serialization-dependencies impl. Tests not yet updated.
* Update serialization tests for dependency validation (GHY-4010)
The dependency-validation warnings replace the old "Failed to export
Dashboard/Cards ... outside requested collections" messages with a uniform
"<entity> references <model> <id> which is not included in the export /
is missing from the source database" form, so the escape tests now assert
on the new wording. Adds dependency-validation-missing-data-ref-test, which
covers the new data-model case: a card referencing a deleted field aborts the
export (and continue-on-error still exports the rest).
* Fold dependency collection over a monoid, with batched queries (GHY-4010)
Replaces the nested reduce in collect-dependencies with a {:visited :deps}
monoid: entity-deps maps one entity to its contribution, merge-deps is the
associative combine (0/1/2-arity, usable directly as a transduce reducing fn),
and collect-dependencies folds every entity into the whole via a single
transduce. content-id-batches yields [model id-batch] pairs so each extract
query's :in :id clause stays bounded (query-batch-size 1000), and the data-dep
existence check is batched the same way. Behavior is unchanged.
* Flatten dependency collection into four transducer stages (GHY-4010)
entity-deps now reads its model off the entity (extract-query returns modeled
rows) instead of taking it as an argument, so collect-dependencies becomes a
flat pipeline: filter content models, resize into id batches, mapcat each batch
through extract-query into an entity stream, then map entity-deps — no eduction
nested inside the mapcat. Behavior unchanged.
* Move dependency validation into its own namespace (GHY-4010)
Extract the export-time dependency-satisfaction check out of
serialization.v2.extract into serialization.v2.dependency-validation. extract
now just calls dependency-validation/validate-dependencies! and keeps ownership
of the analytics-card set, which it passes in. The moved code carries this
session's cleanups (map-kv-vals for the existence lookup, set-based :deps).
Tests move to a matching dependency-validation-test namespace; the escape
assertion embedded in selective-serialization-basic-test stays with extract as
an extraction-level integration check.
* Address review: drop :kind, transients, cover segment/measure (GHY-4010)
Alex's PR comments on the raw dependency validation:
- serialization-dependencies no longer tags deps with :kind; it returns plain
{:model :id} and the validator classifies each by model (serdes.models
content vs data-model), since serdes already knows which is which.
- collect-dependencies accumulates into transient sets — an export can walk a
lot of entities.
- Card serialization-dependencies now covers every reference in the query,
including :segment and :measure (both data-model, existence-checked), via the
lib walkers, closing the gap the field/table/card-only version had.
Refs embedded in parameter_mappings / visualization_settings / result_metadata
still aren't covered, and the raw serialization walker still duplicates the
serialized *-deps knowledge; both are tracked for the follow-up unification.
* Cover result_metadata and visualization_settings field refs (GHY-4010)
serialization-dependencies "Card" now gathers Field references from every
routine-backed source, not just the query:
- query: lib walkers (fields, tables, segments, measures, template-tag fields)
- result_metadata: each column's :id
- visualization_settings: the JSON-encoded column-setting keys, parsed with
mb.viz/db->norm (the same routine the dependency-analysis project uses)
New test covers a field referenced only in visualization_settings. Field refs
buried in parameter_mappings / parameters value-fields are bare [:field ...]
clauses the lib walkers reject; those remain for the dep-walker unification.
* Rename `dependencies` multimethod to `deserialization-dependencies`
Makes the load-time dep multimethod parallel with the export-time
`serialization-dependencies`. `dependencies` runs on the serialized/ingested
form for load ordering and omits tables/fields (import synthesizes them);
`serialization-dependencies` runs on the raw entity and includes them. The
name now says which is which.
Mechanical rename of the defmulti + default + 19 impls across 18 models + 3
callers (load, metabot, remote-sync) + tests + doc-links. No behavior change.
* Address CodeRabbit: batch analytics lookup, fix test targeting (GHY-4010)
- analytics-card-ids batches the card-id :in via a partition-all transducer, so
a large closure doesn't exceed database parameter limits (matches the batching
already used in dependency collection).
- escape-report-test's ":parameters" case targeted coll2 (re-testing the
source-table card); target coll3 so it actually exercises the parameter card.
- Correct the continue-on-error test wording: it suppresses the abort but does
not skip the affected card at export (the malformed card is skipped on import).
* Unify serialization/deserialization dependency walkers (GHY-4010)
Make the mbql-deps walker family format-aware so a single traversal handles
both raw (numeric ids, export/serialization) and serialized (portable refs,
load/deserialization) forms, gated by *serialization?* / raw-ref-id?. Card and
Dashboard now share one deps helper across both dependency multimethods.
Implement serialization-dependencies for the remaining content models
(Action, Collection, Document, NativeQuerySnippet, Timeline), computing deps
from the raw entity: Action fetches its query_action child row; Collection
derives its parent from the raw :location path.
Refine export-time validation: a content reference to a deleted entity is not a
failure (export can't emit a portable id for a missing row, so fk-elide drops
it), and structural Collection references are ignored (a selective export may
omit the containing collection tree).
Tests: mbql-deps format-parity test, direct per-model serialization-dependencies
coverage, and the full baseline load->extract round-trip over every model.
* Trim duplicated dependency docstrings and consolidate abort tests (GHY-4010)
Dedupe the "content refs must be in export / data-model refs must exist"
explanation that appeared in four docstrings, and collapse the three
near-identical "delete a data-model ref -> export aborts" tests into one
parameterized test over Field/Segment/viz-settings references.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Share query-batch-size constant; use pos-int? for raw ref ids (GHY-4010)
Hoist the `:in`-clause batch size into a single serdes/query-batch-size
constant instead of a private def plus a bare 1000 literal in extract, and
tighten raw-ref-id? to pos-int? (entity ids are always positive).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Replace transient accumulator with a named merge-entity-deps monoid (GHY-4010)
The set-building is not the hot path (batched DB queries and the mbql walk
dominate), so drop the transients for a plain `merge-with into` monoid,
extracted as `merge-entity-deps` — the same idiom as u/traverse and
remote-sync's merge-incremental-export-plans.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Fold partition-all into the existing-ids transducer (GHY-4010)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Fold distinct into the existing-ids transducer too (GHY-4010)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Use peek and set-as-predicate in dependency validation (GHY-4010)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* SPIKE: thread allow-int-ids? param instead of *serialization?* dyn var (GHY-4010)
Replaces the *serialization?* dynamic var with an explicit allow-int-ids?
parameter threaded through the mbql walkers (mbql-deps/-map/-vector,
parameters-deps, visualization-settings-deps, viz-link-card-deps) and the
shared model helpers (card-deps, dashboard-deps, serdes-deps-dashcard,
result-metadata-deps). The name states the behavior: integer ids count as
references only when set (raw appdb entities), never on the serialized/load
form where a stray numeric must be ignored.
The parameter is mandatory everywhere — no default arities. Load-side callers
(measure/segment/transform/action deserialization impls) pass false; the three
serialization-dependencies impls pass true.
Spike to compare against the dyn-var approach; drop if we prefer that.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Fix cljfmt: align card-deps destructuring continuation lines (GHY-4010)
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Don't compute related-tables or metrics in get-report-details
`get-report-details` drops `:related_tables` and `:metrics` via a `select-keys`, so don't ask `card-details` for them.
* Migrate remaining color tokens to new names (GDGT-2535)
Migrate the remaining legacy design-system color tokens to their new
names per the GDGT-2517 migration guide:
- feedback: error/danger -> feedback-negative, success ->
feedback-positive, warning/gold -> feedback-warning
- misc/core: info -> core-info, white -> core-white, shadow ->
shadow-default, overlay -> modal-overlay, focus -> input-focus,
metabase-brand -> core-metabase_brand
- admin: admin-navbar* -> navbar-admin*
- upsell/illustration: per guide
- remove unused bronze/copper/silver
Covers all reference forms: CSS `var(--mb-color-*)`, `color()`/helper
calls, Mantine `c`/`bg`/`color`/`iconColor` props (string and ternary),
object-literal `color:`/`iconColor:` keys, `ColorName` return values,
arrays, and `Record<_, ColorName>` value maps. Tightens `Undo.toastColor`
from `string` to `ColorName` so future token removals fail at compile
time. Maps the `Alert` component's semantic `color` prop to feedback
tokens internally so its call sites stay unchanged.
Also sweeps up brand/filter/summarize consumer stragglers left over from
the earlier (merged) migration. SDK/whitelabel input keys, the
derive-theme alias definitions, and static-viz's local palette are left
on their existing names by design.
* Migrate missed success-secondary props in StepperWithCards
The .module.css was migrated to feedback-positive-selected but the two
inline c="success-secondary" props (Done check icon + label) were missed.
* Update Loki Snapshots
* Fix toastColor type widening in metabot toast-prop helpers
Undo.toastColor tightened to ColorName in this branch; the two get*ToastProps
helpers returned object literals where toastColor widened to string (missing
as const, unlike their iconColor/dismissIconColor siblings).
* Update TransformListPage sync-color spec to migrated feedback token names
utils.ts color map was migrated (success/warning/danger -> feedback-*) but
the spec assertions still expected the old names.
* Update SDK theme + treemap specs for feedback/misc token mirrors
- get-embedding-theme: SDK mapping now fans text-white->core-white and
focus->input-focus, so the expected override gains those keys.
- treemap option: getColor("white") became getColor("core-white"); add
core-white to the test's MOCK_COLORS palette.
* Update Loki Snapshots
* Update toast color-attr assertions to migrated feedback token names
* Cleanup
* Migrate remaining color tokens in Button and Table stories
* Add explainer comment for colors naming convention
---------
Co-authored-by: Metabase Automation <github-automation@metabase.com>
* fix: Toaster crash on long-running questions from color-mix tokens
The Toaster's styled-components computed colors at render time with the
JS `color` library (via the deprecated lighten()/alpha() helpers). One
of the tokens they parsed, `background_page-tertiary-inverse`, is now
defined as a color-mix() value, which the `color` library can't parse —
so it threw "Unable to parse color from string" and crashed.
Replace those runtime helper calls with CSS color-mix on the variables
directly, so the mixing happens in CSS where color-mix()/var() resolve.
* Adjust dismiss color
* Add react-router v7 as a test-only aliased devDependency
* Add differential conformance harness for the router facade
* Add v7 conformance test for useLocation
* Add v7 conformance test for useParams
* Add v7 conformance test for useNavigate
* Add v7 conformance test for useSearchParams
* Add v7 conformance test for Navigate
* Add v7 conformance test for useSearchParams defaultInit
* Pin react-router v7 to 7.18.1 for conformance
* Add type-level conformance tests against react-router v7
* Guarantee cleanup between facade and v7 renders in runBoth
* Migrate ChartSettingsSidebar unit test to TypeScript
* Migrate DatasetMetadataStrengthIndicator unit test to TypeScript
* Migrate QuestionDataSource unit test to TypeScript
* Migrate ViewTitleHeader unit test to TypeScript
* Correct onSave return type to Promise<void> and relax Toaster className
* Migrate query builder View components to TypeScript
* Preserve collection type on move and tighten timeline callback types
* Remove dead onCloseQuestionInfo prop from structured sidebar
`enhance-context-with-schema` resolved each `:user_is_viewing` item's used tables and then ran them through
`table-utils/enhanced-database-tables`, which fetched and formatted the full visible-columns set for every used table, including all implicitly-joinable columns from related tables. That work was discarded anyway since columns were recalculated via `entity-details/get-table-details` when rendering the context. On large, highly-connected schemas this could load thousands of columns into the request metadata cache and exhausted the heap (metabase#76493).
`:used_tables` entries now carry only `{:id :type :name :database_schema :description}`. name/schema/description are kept for the `simple-entity` error fallback. `enhanced-database-tables` had no other callers, so it is removed.
Fixes UXW-4704.
Support emoji in PDF rendering - in card titles, text cards, and the dashboard title.
These are monochrome only, since monochrome emoji are regular font glyphs. Colour emoji requires unusual bitmap-backed font support, and the PDFBox engine doesn't support that. It could be done, but it would require rendering the emoji glyphs to PNGs, then embedding those in the PDF. It's not worth the effort or including a bulky bitmap emoji font for a cosmetic feature.
This includes the Noto Emoji font in the JAR, and adds the Open Font License notice file covering all the PDF fonts.
The "Transforms" group header in the push/pull changes list is backed by a
virtual collection with the sentinel id -1 (TRANSFORMS_ROOT_ID). Every -1 case
is special-cased in displayGroups.ts except the header breadcrumb, so the
sentinel flowed into collectionUrl({ id: -1 }) and produced a dead
/collection/-1-transforms link (a hyphen at the start of the last path segment).
Link the transforms-root segment to the transforms list (/data-studio/transforms)
instead. Covers both the virtual-root group and the transforms-namespace prefix
on sub-collections, since the guard sits at the single render site.
* Whitelist bedrock models
* Whitelist anthropic models
* Add claude-fable-5 to supported-models for bedrock and claude providers
* Include a display_name for all supported-models
* Filter to available-model? in bedrock's list-models
Snippet template-tag ids were regenerated on every serdes import: YAML
ingest keywordizes the tag-name keys, so the NativeQuerySnippet load hook
could not match them to preserve the existing :id and minted a fresh UUID
instead. In a remote-sync import/export loop this produced a spurious id
diff for every snippet with variables on every cycle.
Normalize :template_tags on import in make-spec so the tag-name keys are
strings before the model hooks run; export is unchanged. Add a round-trip
regression test asserting template-tag ids survive export/import.
* [GHY-3974] Add failing tests for Usage Analytics upgrade failure modes
Two invariant tests covering the audit-DB upgrade breakage, asserted
through ensure-audit-db-installed! so they're independent of the fix
mechanism and leave the generic sync path untouched:
- Mode A (audit-db-self-heals-duplicate-rows-from-stale-schema-test):
an interrupted host-adjust + schema sync duplicates a v_* view row and
retires the original, breaking customer content. ensure-audit-db-installed!
must reconcile back to one active row per view and keep dependent content
on an active table.
- Mode B (checksum-not-advanced-until-host-adjust-completes-test):
last-analytics-checksum must not advance if adjust-audit-db-to-host! is
interrupted, so the next boot re-runs the load and the host-adjust.
Both fail on master.
* [GHY-3974] Fix Usage Analytics upgrade failure modes
Mode B (checksum committed before host-adjust): in maybe-load-analytics-content!
advance last-analytics-checksum only after adjust-audit-db-to-host! completes,
so an interrupted host-adjust leaves the checksum behind and the next boot
re-runs the load + adjust. Also force a reload, regardless of the checksum, when
audit tables are stuck at the postgres "source" schema on a non-postgres host
(audit-db-in-source-state?) so already-corrupted instances self-heal.
Mode A (schema-renamed view duplicated + retired by sync): add
reconcile-audit-db-duplicates!, run on every ensure-audit-db-installed! so
already-corrupted instances (whose checksum already matches, skipping the load)
still recover. For each duplicate same-name group in the audit DB it keeps the
row dependent content points at, repoints stray report_card.table_id refs to it,
normalizes it to the host-canonical name/schema, and deletes the orphans. Schema
is derived from the host engine, not the surviving row, so a duplicate left at
the postgres source schema (Mode B masking Mode A) is corrected too. Scoped
entirely to the audit DB; the generic sync path is untouched.
Both audit-test regression tests pass on h2 and postgres.
* [GHY-3974] Serialize audit DB load/adjust/sync across nodes (1b)
with-duplicate-ops-prevented is per-process only, so on a rolling upgrade
two pods could run the audit load/adjust/sync concurrently and produce the
half-applied schema states behind both failure modes. Wrap the
load+adjust+sync+reconcile in an app-DB cluster lock (metabase_cluster_lock)
so only one node runs it at a time. If another node already holds the lock it
is doing the same work, so skip rather than fail the boot.
The sync must run inside the lock to be serialized, so maybe-sync-audit-db!
now runs synchronously instead of in a background future: on a transactional
appdb a sync on another thread would escape the lock's transaction and
deadlock against its uncommitted metabase_table writes.
Full audit-app namespace passes on h2 (in-process RW lock) and postgres
(transaction + SELECT FOR UPDATE), 52 assertions each.
* [GHY-3974] Filter audit view lookups in SQL, not in memory
audit-view-table and the Mode A heal assertion were selecting every active
audit table and filtering by lower(name) in Clojure. Push the predicate into
the query with a lower(name) where clause.
* Better comments
* [GHY-3974] Don't force a reload for an inactive public orphan (MySQL fix)
audit-db-in-source-state? treated any `public`-schema audit table on a
non-postgres host as the interrupted-host-adjust state and forced a reload.
But a Mode A duplicate leaves an *inactive* `public` orphan, and on MySQL
(canonical schema NULL) forcing a reload made adjust-audit-db-to-source!
move the active NULL row to `public`, colliding with the orphan on the
(db_id, schema, name) unique index (Duplicate entry '…-public-v_content').
Only active rows signal the genuine stuck source state; inactive public rows
are retired orphans handled by reconcile-audit-db-duplicates!. Restrict the
check to active rows. Surfaced by the MySQL/MariaDB app-db CI run of
audit-db-self-heals-duplicate-rows-from-stale-schema-test.
* [GHY-3974] Update modules config: audit-app no longer uses config
Removing the metabase.config.core require from the audit-app module (the
unused config dep dropped when the sync future was removed) leaves config
out of enterprise/audit-app :uses. Fixes modules-config-up-to-date-test.
* [GHY-3974] Only swallow lock-contention failures for our own audit lock
A different cluster lock's acquisition failure raised from inside the locked
body (e.g. the permissions lock taken when serdes inserts a Database) also
carries :lock-names; it must propagate rather than be swallowed as audit
contention and silently abandon the load.
* [GHY-3974] Bail faster on audit lock contention
Drop the lock-acquisition timeout to 5s and retries to 2, so a node that loses
the boot-time audit lock skips in ~17s instead of ~150s. The work is idempotent
and the lock holder is doing it, so a losing node need not win; keeping a couple
retries still lets a node pick up the work if the holder crashes mid-load.
* [GHY-3974] Serialize audit DB install/adjust under the cluster lock too
The install-time adjust-audit-db-to-host! + schema sync (the ::installed and
::updated branches of maybe-install-audit-db!) previously ran outside the
cluster lock, so two nodes detecting an engine mismatch could adjust and sync
concurrently against a half-adjusted schema. Move install inside the lock so the
whole install+adjust+load+sync+reconcile is serialized across nodes; a node that
acquires the lock after the installer commits sees the DB already exists and
no-ops instead of colliding on the audit DB primary key.
* [GHY-3974] Harden reconcile-audit-db-duplicates! and Mode B test
- Sort rows by id before grouping so survivor selection (including the
referenced? tiebreak) is deterministic regardless of appdb row order.
- Clear is_defective_duplicate on the survivor so it re-enters idx_unique_table
(a flagged row has a NULL unique_table_helper and is excluded from the index).
- Comment why orphans are deleted before the survivor is re-canonicalized.
- Mode B test: move cleanup into a finally and reinstall the audit DB so an
unexpected throw can't leave sibling tests without an audit DB.
* [GHY-3974] Make reconcile lossless: move content off orphans before deleting
The previous reconcile did (t2/update! :model/Card {:table_id orphan} {:table_id survivor})
then deleted the orphan. That update is a no-op — Card's before-update populate-query-fields
re-derives table_id from the unchanged dataset_query — and report_card.table_id is ON DELETE
CASCADE, so deleting the orphan destroyed any customer card whose query pointed at it. This
happens in real Mode A: bundled audit cards reference the retired row (making it the survivor)
while a customer question built during the corrupted window references the freshly-synced active
row (the deleted orphan).
Now repoint-cards-to-survivor! rewrites each referencing card's dataset_query (source-table)
and result_metadata, remapping field ids onto the survivor's same-named fields, before the
orphan is deleted. Handles both pMBQL and legacy field refs.
Adds audit-db-reconcile-preserves-content-on-the-deleted-duplicate-test, which asserts the card
survives and its field refs resolve to active fields on the survivor (fails on the old code).
* [GHY-3974] Use lib field-ref helpers instead of hand-indexing refs
repoint-refs now reads/writes the field id via lib/field-ref-id and
lib/with-field-ref-id rather than indexing the ref vector by position. Expose
with-field-ref-id from metabase.lib.core (the setter alongside the already-public
field-ref-id getter) so it's reachable through the lib module's public API.
* [GHY-3974] Use lib clause predicate / field-ref-id for remaining MBQL access
Replace the hand-rolled field-ref? predicate with lib/clause-of-type?, and the
positional field-id extraction in the test with lib/field-ref-id.
* better docstring
* [GHY-3974] Order reconcile rows in the query instead of in memory
Address review: let the DB sort (:order-by [[:id :asc]]) rather than sort-by; group-by
preserves the order within each group, keeping the survivor tiebreak deterministic.
* [GHY-3974] Move query id-remap into lib; handle result-metadata thoroughly
Review (@metamben): the ref manipulation should live in lib and be general-purpose,
and queries vs result metadata shouldn't share one generic walker.
- Add metabase.lib.walk.util/replace-field-ids and replace-table-ids (general, built on
lib.walk): field refs everywhere via walk-clauses (filters/aggregations/breakouts/joins/
nested stages), :source-field/:fk-field-id opts, native :template-tags :dimension refs, and
:source-table per stage/join. Exposed via metabase.lib.core.
- audit-app repoint-cards-to-survivor! now rewrites dataset_query via the lib helpers, and
handles result_metadata separately (non-generic): :id, :fk_target_field_id, and :field_ref
covering both legacy ([:field id opts]) and pMBQL ([:field opts id]) shapes.
- Test asserts legacy result_metadata :field_ref is remapped onto the survivor.
Covers review threads: field-ref manipulation in lib (#383), FK/source-field refs (#418),
legacy result-metadata refs (#392).
* [GHY-3974] Batch the reconcile referenced? check into one query
Address review: instead of a t2/exists? per row in a duplicate group, fetch the set of
card-referenced table_ids for the whole group in a single query and test membership.
* [GHY-3974] Review cleanups: cond-> in replace-field-ids; update-existing in result-metadata remap
- lib replace-field-ids: use cond-> for the template-tags/:dimension updates instead of if/else.
- audit remap-result-metadata: use m/update-existing with an identity-for-unmapped fn instead of
a cond-> with duplicate lookups; drop the redundant pos-int? guard in remap-result-metadata-ref
(field-id-remap is always a map, so it already returns nil for names/nil).
Fixes UXW-3770.
The default limit of 50 levels of nesting is too small for deeply nested
MBQL expressions, such as a `replace(replace(replace(...)))` tower.
This PR raises the limit to 10k, and includes a test that 500 levels
parses fine. 10k works at my REPL but I don't want to commit a test like
that in case other JVMs hit a `StackOverflowError`. Rejecting the import
is a better UX than crashing Metabase!
Fixes UXW-4108.
When no `database`, `dashboard` or `question` parameter is supplied, the
endpoint does not invalidate anything. It previously returned a 404 in
that case, indicating that nothing was found to invalidate.
But since no attempt to invalidate anything is made without at least one
of those parameters, the 404 was misleading. Instead, this malformed
request should return a 400, which it now does.
No impact to the Metabase FE, which only ever calls this endpoint with
the parameters supplied.
* Add an Invite someone to view this action to dashboard and question share menus (#76209)
* Add an Invite someone to view this action to dashboard and question Share menus
* Update admin create-user e2e to reveal the temporary password via the new toggle
* Add a shareable item link to the invite modal's no-email states
* Remove the stale password-toggle aria-label and reword the temp-password invite copy
* Land invited users on the shared dashboard or question after signup (#76505)
* Land invited users on the shared dashboard or question after signup
* Scope the invite email to the shared dashboard or question
* Code cleanup
* Polish the invite email's join button spacing, label, and padding
* Remove stale TODO about synchronous invite email
* Add tests for invite someone to view (unit + e2e) (#76589)
* Add unit and e2e tests for the invite-to-view feature
* Fix the invite-to-view e2e signup button and no-access selectors
---------
Co-authored-by: Nathan Voxland <nathan@metabase.com>
* Add user_invited and invite_to_view_opened analytics events (#76646)
* Add user_invited and invite_to_view_opened analytics events
* Reword the external-invite tracking comment in NewUserModal
* Type isEmailAlreadyInUse's error with a guard instead of a cast
* Return an error_code for the duplicate-email 400 and key isEmailAlreadyInUse off it
* Move invite-events analytics into common/analytics and assert events fire once
---------
Co-authored-by: Nathan Voxland <nathan@metabase.com>
* fix(custom-viz): allow map tile server host in img-src CSP
When Custom Visualizations are enabled, csp-img-enabled is forced on,
switching img-src from a permissive wildcard to a restricted allowlist.
Map tiles (OpenStreetMap by default, or a custom map-tile-server-url)
were never added to that allowlist, so map visualizations broke.
Automatically add the configured tile server's origin to img-src,
mirroring how custom font files are added to font-src. The {s} subdomain
placeholder becomes a * wildcard and the {z}/{x}/{y} path is dropped.
* docs: note map tile server is always allowed under restricted image domains
* Missing update to setting description
* Update comment
* Fix whitespace
* Rename router useRouter to kebab-case use-router
* Add router facade shared types and path/search utils
* Add useNavigate facade hook
* Add useLocation facade hook
* Add useParams facade hook
* Add useSearchParams facade hook
* Add Navigate facade component
* Add Outlet facade shim
* Add Link facade re-export
* Reference react-router v7 docs from each facade helper
* Lock facade tests to react-router v7 behaviour
* Import useRouter from the metabase/router barrel
* Sort metabase/router barrel exports
* Test useNavigate splits query from a string destination with state
* Test setSearchParams clears the query when called with no argument
* Use router/<name> pattern for facade test describe blocks
* Link react-router v7 API reference from facade types
* Use react-router's official descriptions for facade type comments
* Use useMount instead of a bare useEffect in Navigate
* Expand inline JSDoc in router utils to multiline
* Implement useNavigate over v3 history instead of redux dispatch
* Navigate via react-router's router API instead of history directly
* Re-navigate on prop change in Navigate to match v7
* Default useLocation state to null and note v3 helper limitations
* Pass Navigate state by reference and clone search-param updater arg
* Expose v3 splat param under v7's * key in useParams
* Drop the existing hash in setSearchParams to match v7
* Churn useNavigate identity per navigation to match v7
* Support defaultInit in useSearchParams to match v7
* Match v7's URLSearchParamsInit pair-tuple type
* Match v7's useParams generic signature
The work done in #76355 and #76504 to fix some OOMs while generating
subscriptions modified the core rendering pipeline (static viz charts to
PNGs) concurrently with the work happening on PDFs.
This adapts the PDF generation to reuse both in-memory and on-disk query
results, and to use the whole-dashboard budget of results in memory before
spilling to disk. Cleans up all temporary files when finished.
Take existing tables into account when determining already-loaded bigquery.
We no longer have bigquery tables that constantly think that they need
to be recreated, but we have fallen a little too far in the other
direction; datasets that get their DB created but fail midway thru
will still be treated as loaded.
In snowflake we have a way of checking this down to the individual row
counts and counting as not-already-loaded if there's a mismatch. I
don't see a way to do this in bigquery, but we can at least check that
all the tables exist. If we get a failed partial load that gets all
the tables created but fails the rows that will still require manual
intervention.
Also stop using (config/current-major-version) when qualifying dataset
names; turns out this is not at all reliable in CI. This unfortunately
means that we don't actually have cross-major-version isolation; the
best we can do is isolate master/release from all other branches,
which is still an improvement over what we have now.
`get-day-of-week` accepts a positional week-mode, and the ::week-mode
schema admits :iso, :us, and :instance (shared with get-week). But only
the default (no-mode) and :iso forms have a lowering in lib.filter.desugar
— :us and :instance desugar to a nil temporal-extract unit and 500 at SQL
compile time (observed: an LLM emitting "instance" for "day of week on
weekends"). Drop the unsupported mode, leaving the start-of-week-aware
default. :iso and the bare form are untouched.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Prompt-snippet updates, independent of any code change:
- discovery: search to choose a source, navigate to drill in (don't skip
the curated layer just because a raw table name is recognizable)
- data-sources: a source the user explicitly names is authoritative
- grounding: when several columns plausibly match, read descriptions and
sample values rather than picking by name resemblance
- construct_notebook_query: a "translating the request" section (constraint
vs breakout, apply every constraint, absolute vs relative dates)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(custom-viz): make dev hot-reload resilient to transient failures
When the dev server is mid-rebuild its bundle endpoint can transiently
fail, and rapid edits / overlapping reloads could leave the viz showing a
stale bundle or a spurious "unavailable" toast.
- Backend: fetch-dev-bundle returns nil on upstream failure (mirroring
fetch-dev-manifest) so the endpoint serves its existing retryable 503
instead of a hard 500.
- Frontend: retry the dev bundle fetch on transient 5xx (fail fast on
4xx), and tag each load with a monotonic sequence so a slower/older
overlapping load can't overwrite a newer applied bundle or fire a stale
toast (latest-wins).
* fix(custom-viz): render the freshest bundle after overlapping dev reloads
Overlapping dev reloads (rapid edits) could apply the newest bundle to the
visualizations registry but leave the chart rendering an older one: the
visualization only re-reads the registry when it remounts on the loading
toggle, and a plain boolean cleared after the first load resolved.
- Refcount in-flight loads so `loading` stays true until the last settles,
so the single remount reads the freshest registered bundle.
- Stop retrying a load once a newer one has started (it would be discarded
by the latest-wins guard anyway, and would keep `loading` held).
* fix(e2e): restore custom-viz dev plugin source between retries
The development-mode custom-viz test mutates the scaffolded plugin
source in place ("Above threshold" -> "Way above!") to verify hot
reload, but never reverts it. Since `before` does not re-run on a
Cypress retry, a retried attempt started from the mutated file and the
opening "Above threshold" assertion timed out against stale state.
Capture the pristine source in `before` (after the clean re-scaffold)
and rewrite it in `afterEach` so each attempt starts from the default.
* fix(e2e): use frozen lockfile for custom-viz SDK bun install
Skip lockfile resolution during the dev-mode `before` hook so the
install step is deterministic and faster, reducing the chance of the
120s setup timeout.
* fix(e2e): start custom-viz dev server per test, not per suite
The dev-mode test stops the dev server as its final assertion (revert to
default), but the server was started once in `before`. Since `before`
does not re-run on a Cypress retry or during burn-in, every run after the
first hit a dead server and failed at "verify the dev plugin is
registered".
Move the server start to `beforeEach` and the stop to `afterEach` (the
stop task is idempotent), keeping the heavy build/scaffold/install in
`before`.
* chore(custom-viz): trim verbose comments per review
* refactor(custom-viz): reuse retry util for dev bundle fetch
* fix(custom-viz): use > not >= in superseded check
The == case is impossible (loadAppliedSeq is only set to a load's own seq
on success, which returns before the catch; seqs are unique), so >= and >
are equivalent here. > matches the sibling loadStartedSeq check and reads
as "a strictly newer load exists".
* fix(custom-viz): only the latest-started load registers its bundle
The apply-guard bailed only when a newer load had already applied; bail
when a newer load has merely started too (applying implies starting, so
this subsumes the old check). Reuses isLatest(), consistent with the
retry loop and the superseded check.
* refactor(custom-viz): name load-sequence maps by plugin id
* test(custom-viz): scaffold dev plugin per attempt instead of capturing source
Move the plugin scaffold (init + npm i) into beforeEach so each attempt
starts from a pristine source, dropping the pristineSrc capture/restore.
The expensive SDK build stays one-time in before.
* fix(embedding-sdk): gate query builder controls on create-queries permission
Filter/Summarize/Breakout dropdowns and the notebook editor toggle rendered
regardless of query permissions, showing non-working controls for users
without create-queries access. Mirrors main app's FilterHeaderButton/
QuestionSummarizeWidget/QuestionNotebookButton gating (isEditable, !isNative,
!isArchived) via a new shouldRenderQueryBuilderEditingControl.
Fixes EMB-1963
* test(embedding-sdk): unit test shouldRenderQueryBuilderEditingControl
Covers editable/native/archived/no-question cases, plus the missing
create-queries case by building a query against metadata with the source
table excluded (mirrors what the backend does when that permission is
missing) rather than mocking metabase-lib.
* fix(embedding-sdk): hide decorative icon from ToolbarButton accessible name
Icon set role="img" aria-label="<name> icon" unconditionally, so a button's
accessible name became "filter icon Filter" instead of just "Filter",
breaking cy.button()/getByRole name lookups.
* test(embedding-sdk): e2e test for query builder permission gating
Confirms Filter/Summarize/notebook editor toggle are hidden for a user
without create-queries permission on the underlying table, and shown for
one with it, using existing readonly/readonlynosql fixture users whose
default group permissions already match each scenario.
Fixes EMB-1963
* docs(embedding-sdk): tighten comments, scope test fixtures per-test
Bullet the three omitted main-app gating conditions instead of run-on
prose, shorten the missing-permission test comment, and move each
question fixture into the test that uses it instead of module-level
constants shared across unrelated tests.
* fix(embedding-sdk): guard BreakoutDropdown on missing question
Matches the !question || !shouldRenderQueryBuilderEditingControl(question)
pattern used by FilterDropdown and EditorButton.
* Let fetchDashboard accept a prefetched dashboard
(cherry picked from commit 63425d8754777a96f023c3be6028c5a45b4607d5)
* Use save response to refresh dashboard instead of refetching
(cherry picked from commit 8f5aff044beb8290f9c63f7ebb041ffbe8c2b392)
* Test save reuses response without refetching the dashboard
(cherry picked from commit a90b2885f7ac19ed472bed1018a88fa29fee60be)
* Update e2e save helper for dropped post-save dashboard refetch
(cherry picked from commit 41bba584ae41640903c06fcd76022a1569d6f087)
* Update e2e dashboard reload expectations for save-response reuse
(cherry picked from commit 97fabf4f8df7d0c1345ea33a6136a1c3ba76933f)
* Wait on dashboard UI instead of phantom reload when returning from drill
(cherry picked from commit 19181e1345af6af1fe46f323c3b0362a5286e1b4)
* Add waitForDashcardsToLoad e2e helper and use it in #32444 test
(cherry picked from commit fa3a0e989f2166f50af10a0294fcda9bfef968d9)
* Settle saveDashboard on dashcards loaded instead of fixed sleep
(cherry picked from commit b7a2e157b6db12a2f4f8ab10b7c19f68437b94ed)
* Drop redundant saveDashboard waitMs args now that it settles deterministically
(cherry picked from commit e8827b06955f581dde739491d3aef2bee5f90be9)
* Drop redundant post-save sleep now covered by saveDashboard settle
(cherry picked from commit c3ae1c65997165fffc0fa1c394d1d2765b4fb2b8)
* Drop tombstone comments per review
* Make waitForDashcardsToLoad tolerant of dashboards without a grid
* Anchor dashcard settle on dashboard body to avoid early return
* Revert broad saveDashboard auto-settle; keep save-reuse and opt-in waitForDashcardsToLoad
* Settle saveDashboard on dashcards loaded + grid layout stable; drop waitMs sleeps
* Wait for filter widgets to settle after save (parameter panel re-render)
* Make editDashboard resilient to a dropped Edit click after save
* Make editDashboard retry work inside cy.within (use cy.document not cy.get body)
* auto stress test
* better options
* fix(e2e): restore 30s timeout on findByText for impersonated permission test
The timeout was removed in a8cc638d2f when updating the error message text.
The impersonated write path takes 15-20s, far exceeding the default 4s timeout,
causing consistent test failures (quarantined for 33 days).
---------
Co-authored-by: Ryan Laurie <iethree@gmail.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Ryan Laurie <30528226+iethree@users.noreply.github.com>
* Fix flaky pivot-table dashboard filter spec: wait for dashcard load before opening question
The "does not use extra filtering stage for pivot tables" spec clicked the pivot
dashcard's legend title immediately after exiting edit mode. The title's
navigation handler is only wired once the card series has reloaded, so an early
click was a no-op and the /api/dataset/pivot request never fired, timing out
cy.wait("@datasetPivot"). Wait for the dashcard to finish loading (visible title
+ no loading indicator) before clicking.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Give pivot-table dashboard filter spec a throttle-tolerant pivot wait
Clicking the pivot dashcard title navigates to an ad-hoc QB that must boot
the route, load table metadata, and only then issue /api/dataset/pivot. Under
network throttling that chain can exceed cy.wait's default 5s timeout, so the
wait occasionally fails even though the request does fire moments later. Wait
on the @datasetPivot alias with a throttle-tolerant timeout.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* WIP: support for merge strategy via delete + insert
* PoC FE
* fix
* overridable method
* support for py
* refactor
* FE
* changelog
* FE fixes
* validate UK
* chore
* update FE
* tests
* remove compile-merge multi
* FE tests
* remove feature
* move component to its own file
* chore: test composite keys
* fix: override for sqlserver
* fix(embedding-sdk): open SQL editor when Metabot navigates to native hash URL
When Metabot sends navigate_to with /question#<native_hash>, SdkAdHocQuestion
was returning questionId=null (no numeric slug), so isNewQuestion=false and the
SQL editor never opened. Now detects dataset_query.type==='native' and returns
'new-native', which makes SdkQuestionProvider set isNewQuestion=true and
useNewQuestionEditorSync open the editor.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dwi9BjxKNSJsztkkVrsU8d
* test(sdk): use real embedding Metabot stream for SQL editor test
Replaces hand-rolled mock payload with a navigate_to event captured
verbatim from an actual embedding Metabot response, so the test
fixture matches real wire format (db id adjusted to match env).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012wNQmcGH7PCXMD1PqFLiBS
* refactor(sdk): call resolveQuestionId inline instead of in useMemo
It's a cheap derivation, not worth complicating the memo for.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012wNQmcGH7PCXMD1PqFLiBS
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* GDGT-2534 Migrate icon tokens (disabled/secondary swap + text aliases)
Per design, the icon tokens are corrected: `icon-disabled` and
`icon-secondary` swap meaning, and all four icon tokens become runtime
aliases of their text counterparts.
- Swap consumers: `icon-disabled` <-> `icon-secondary` call sites.
- Redefine in both themes:
- icon-primary -> var(--mb-color-text-primary) (value unchanged)
- icon-secondary -> var(--mb-color-text-secondary)
- icon-disabled -> var(--mb-color-text-disabled)
- icon-brand -> var(--mb-color-text-brand) (brand[40] -> brand[50])
Token names are unchanged, so there's nothing to remove at cleanup. Icon
tokens aren't SDK public theme keys and aren't used in static-viz, so the
var() aliases are safe everywhere.
* Update Loki Snapshots
---------
Co-authored-by: Metabase Automation <github-automation@metabase.com>
* refactor: add basename module for app-wide basename state
* refactor: move basename off ApiClient into the basename module
* refactor: make setBasename tolerate nullish input
* refactor: drop redundant basename guard in sdk init
* test: capture original basename per-test in setupBasename
* fix: strip basename only as a prefix in getDatasetDownloadUrl
* Measure SDK/app bundle sizes from both uberjars, apples-to-apples
* Always report brotli bundle size, estimating when no .br is shipped
* Bundle-size stats: upload per-bundle deltas, skip unchanged commits
* Bundle-size stats: always record releases, gate/report on brotli delta
* Scope app to app-main and SDK totals to reachable code via the chunk graph
* Fall back to whole app/dist when stats lack reachableAssets
* Stats logger: record master only, drop release tags from the live series
* Measure SDK async chunks as loadable; split legacy initial/total
* Point bundle-size logger at table 79702
* Skip SDK bundle-size gate when base ref predates reachable-chunk stats
* Split bundle-size scripts into pure cores + I/O wrappers, add unit tests
* Bundle-size: extract legacy embedding-sdk.js fallback; fix stale as-served comments
* Retry stats upload with backoff; keep bundle-size run green if it still fails
* Add default-openai-llm-metabot-model gpt-4.1-mini
* Filter openai list-models by supported-models
OpenAI's /vi/models endpoint returns ~140 models, many of which are not relevant (text embeddings, audio/video models)
or else deprecated. Filter this down to a whitelisted set of known supported models.
* Add grouping for openai models in provider-models-response
* Throw an exception if callers pass `:ai-proxy true` for openai
Similar to bedrock and azure, supporting this would require some work on the ai-service side, and we already reject
non-anthropic in validate-metabase-managed-provider!
* FE: enable openai provider in metabot admin UI config
* Drop gpt-4.1-mini from supported-models and change default to gpt-5.4
* Replace gpt-4.1 with gpt-5 in supported-models
* Bump metabase.metabot.example-question-generator/max-tokens to 4096 for gpt-5
GPT-5 produces a lot of reasoning tokens and sometimes exceeded the old limit.
* Remove reference to gpt-4.1-mini in docstring example
* Remove references to gpt-4.1 and gpt-4.1-mini in FE unit tests
* Skip empty remote-sync commits after a no-op export (GHY-3934)
A dirty RemoteSyncObject row whose serialized content already matches the
repo stages a net-zero tree change, so finishing the commit produced an
empty commit that still got pushed. GHY-3933's content-hash gating catches
the no-op-update-event case, but not the others: a full-export fallback
that re-serializes byte-identical content, a rename/delete whose old path
isn't known yet, a dedup name collision, or path/hybrid models.
Add an `empty-commit?` query to the CommitBuilder protocol (git layer
answers whether the staged tree equals the parent's; it does not decide
policy). full-export!/incremental-export! branch on it: when empty, abort
the commit, leave the version unadvanced, and report push-skipped — while
still reconciling RemoteSyncObject state so the stale dirty flags that
triggered the no-op are cleared.
* Docstring
* Unify remote-sync export commit lifecycle and skip empty merges
Extract commit-staged! — the open/stage/empty-commit?/finish-or-abort
lifecycle — and route full, incremental, and merge exports through it.
merge-and-store! moves from source.clj into impl.clj (next to the other
export paths) so it shares commit-staged! instead of duplicating the
lifecycle via replace-all-files!; source.clj keeps the merge computation
(compute-merge, preview-merge).
commit-staged! returns the :remote-sync/empty-commit sentinel when the
staged tree matches the parent, so the merge path no longer pushes an
empty commit either. An empty merge folds in any remote changes and
advances to the remote tip, reporting "pulled" (or "push-skipped" when
nothing changed on either side) rather than a misleading "merged, pushed 0".
* Inline merge-and-store! into export-merged!
The intermediate {:status ...} map that export-merged! re-dispatched via
case added nothing: inline the merge computation and commit so the merge
path reads like full-export!/incremental-export! — compute, commit-staged!,
reconcile, outcome.
Move the test seam down from merge-and-store! to source/compute-merge: the
export! tests stub the pure merge function and exercise the real commit
staging, and the three dedicated merge-and-store! unit tests are dropped as
redundant with merge-test (algorithm) and the commit-staged!/export coverage.
* Better docstrings
* Test empty-commit? against a real git repo
Exercise empty-commit? through the real JGit commit builder (not a mock):
identical re-stage and delete-of-absent-path read empty, real edits and new
files don't, aborted no-op checks leave the branch untouched, and the
memoized tree still yields a correct commit when finish-commit! follows.
Serdes stamped every non-Setting entity (and the Card column) with the
instance build version on extract. Since the value differs between
instances, it produced spurious uncommitted changes during remote sync.
Compatibility is now governed by the versioned representation spec
(:card_schema + upgrade-card-schema-to), so the per-entity version stamp
no longer carries useful information. Stop serializing it:
- extract: remove the metabase_version stamp
- Card serdes spec: move :metabase_version from :copy to :skip
- load: remove the now-dead version-mismatch warning machinery
Legacy baseline fixtures still carry the field, exercising that older
exports import cleanly now that it's skipped.
Replace e2e's local ConductorTest with the shared NormalizedTest wire shape
(plus a transient screenshotPath) and import toNumber from the module, so the
report path and the quarantine path can't drift on a test's identity. The
transport stays forked: e2e posts mid-run from Cypress's after:spec hook on
node-fetch, with screenshots and a dry-run mode the module's post-run transport
doesn't cover.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(sdk): strip controlled parameters from drill-through target dashboard
When InteractiveDashboard renders a drill-through target via
SdkInternalNavigationProvider, the full dashboardProps spread included
the host's controlled `parameters` and `onParametersChange` props.
getEffectiveParameterValues makes the controlled prop win over
initialParameters, so click-behavior filter values were silently
discarded and the target dashboard opened with empty filters.
Introduce getDrillThroughDashboardProps to explicitly strip controller
props before forwarding to the drill-through target. Display props
(withDownloads, withTitle, etc.) pass through unchanged. The target
is seeded from click-behavior initialParameters as intended.
Fixes EMB-1946.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fwn7CNjPBZAg4KLVMaV7HW
* fix(sdk): also strip initialParameters from drill-through target props
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fwn7CNjPBZAg4KLVMaV7HW
* test(sdk): clean up EMB-1946 e2e test
Rename Dashboard A/B to Origin/Target for readability, fix parameter id,
remove cy.intercept/cy.wait in favour of visible UI assertions only, and
rename the describe block to cover all three stripped props.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fwn7CNjPBZAg4KLVMaV7HW
* test(sdk): remove describe.only from EMB-1946 test
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fwn7CNjPBZAg4KLVMaV7HW
* fix(sdk): address review feedback on getDrillThroughDashboardProps
- shorten comment to focus on why (root-scoped props override drill-through target)
- tighten return type to Omit<> reflecting the stripped keys
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fwn7CNjPBZAg4KLVMaV7HW
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* Limit metabot FK related-tables to bound context size
When bulding metabot's context we expand one related-table entry per FK column and fetch and format the full column
set for each related table. On a large, highly-connected schema this fetches a large number of columns into the
metadata cache, exhausting the heap (metabase#76493).
We now limit the number of related-tables we attempt to fetch to metabase.metabot.tools.entity-details/max-related-tables
* Report truncated related tables to the LLM
When the related-table list in the metabot context gets truncated due to exceeding the limit, leave a note in the
context that truncation occurred. For tables that were dropped, include them in a lighter weight list with only
id/name/related-by (also capped), so the LLM knows more related tables exist and can look any of them up individually.
`entity-details/related-tables` now returns the detailed list plus a total count and a list of truncated items; the
result surfaces related_tables_truncated / related_tables_total / related_table_refs (+ related_table_refs_truncated),
and llm-shape renders a <related-tables-truncated> note with the truncated items.
* Rename max-related-table-{,truncated-}refs and add table descriptions
* Don't :include-implicitly-joinable? when looking up FKs for metabot related-tables
Otherwise we fetch and cache the full column set for every FK-target table when looking up realted-tables for
metabot's context. Instead, just do a bulk metadata lookup on the FK target columns only so we don't blow up the
metadata cache for tables with a huge number of columns.
* Cleanup docstrings and formatting in related-tables->xml
* Exclude tables in :related_tables from :related_table_refs
The refs list doesn't need to duplicate the tables from :related_tables. This way, the LLM potentially gets more
info about truncated tables, since we also cap the :related_table_refs list.
* Drop :related_tables_truncated and :related_table_refs_truncated
They can be inferred from :related_tables_total and the size of two lists.
* Use render-llm-template for related-tables->xml and related-table-ref->xml
* Fold related-tables-result into related-tables
* Reduce max-related-tables from 20 to 10
* Make metabase.metabot.tools.entity-details/related-tables private
* Fix docstring typo
* Ensure that :related_tables_total = :realted_tables + :related_table_refs
Don't exclude entries from :related_table_refs just because the table id appears in :realted_tables. We specifically
want to allow duplicate table-ids as long as the (table-id, fk) pairs are distinct to preserve the total = related +
refs invariant.
* Cleanup realted-tables
* Return field id from fk-related-table-groups
To avoid having to call lib.metadata/field in the caller for each fk just to extract the :name.
* Include the field id in related-tables :related_by to distinguish fk fields with the same :name
* Simplify handling for related-tables without fields
- rename max-related-tables to max-related-tables-with-fields
- rename max-related-table-truncated-refs to max-related-tables
- rename :related_table_refs to :related_tables_without_fields
- return up to max-related-tables-with-fields (10) in :related_tables
- return up to 40 tables in :related_tables_without_fields
- 40 = 50 - 10 = max-related-tables - max-related-tables-with-fields
- always use table-details to format related-tables, just pass :with-fields? false once max-related-tables-with-fields
is exceeded
* Don't say roster
* Only include :related_tables_without_fields if the caller requested fields
If the caller did not pass `with-fields? true`, then there is no distinction between :related_tables and
:related_tables_without_fields, so don't include it.
* Fix docstring
Adjust dataset hashing for snowflake and bigquery.
We are frequently seeing some rogue process dropping the test-data
database which is shared across all tests, causing a good deal of
instability. We have not been able to track down what code is doing
this. Changing the way we calculate database names based on dataset
will help us find the culprit and also help isolate branches for
stability.
When we are on master or a release branch, splice the current major
version into the qualified database name. When we aren't, use "sha__"
as a prefix instead of "sha_", which will keep backport branches from
affecting dev branches going forward and help us identify whether it's
backports or not that are causing the database drops we're seeing.
Stop checking the tracking table to determine whether the dataset is
loaded. The dataset will be tracked regardless, so this should not
factor into the question of whether it needs to be created.
Unrelated change: update get-or-create/create-database! to print when
it encounters a failure and deletes the database it was trying to
create. This should never happen for snowflake/bigquery because they
are flagged as not supporting dynamic dataset loading, but it's still
better to have this printed visibly instead of the error logs which
are hidden.
* Fail serialization export when cards escape requested collections (#75176)
When a dashboard or card in the requested collections referenced a card
saved outside them, escape analysis logged a warning but the extract
stream was silently emptied. The CLI still printed "Export complete!" and
exited 0, and the API returned HTTP 200 with an archive containing only
export.log, so automated pipelines could not detect the empty export.
The default export now aborts loudly: the eager extract phase throws, so
the CLI exits non-zero and the API returns a 4xx before streaming begins
(the status can't change once the tar.gz starts). continue-on-error is
unchanged - it still proceeds, skipping the escaped entities, and the
escape warnings still land in export.log.
* Unify escape reporting and abort into handle-escapees!
The escape warning report and the abort throw were two adjacent branches
over the same data. Fold them into one handle-escapees! function: it logs
the per-entity report and, unless continue-on-error? is set, throws to
abort. Behavior and the thrown message/status are unchanged.
* Better docstrings
* Track remote-sync dirty state by serialized content hash (GHY-3933)
The remote-sync "yellow dot" was driven purely by update events: any
:event/<model>-update flipped the RemoteSyncObject row to "update",
regardless of whether the entity's serialized form actually changed. So
editing a non-serialized field (e.g. a transform's schedule) or any no-op
re-save marked the entity dirty, and the subsequent push produced a commit
with no files.
Record a content_hash (SHA-256 of the entity's serialized YAML, taken from
the live DB entity) on each RemoteSyncObject at every sync point — full
export, incremental export, and import. On an update event, the consumer
recomputes the hash and keeps the row "synced" when it matches the recorded
baseline; a null baseline is treated as dirty so new content still exports
once. A reverted edit that returns to the synced content clears the stale
dirty flag.
Works for all tracked model types (entity-id, hybrid, and path identities).
* Add export/import integration tests for content-hash dirty detection
Beyond the consumer-level per-type tests (which seed the baseline hash
directly), add end-to-end tests that exercise the baseline-recording paths:
- export integration: run a real export! to a mock source, then fire a no-op
update and assert it stays synced — covers the incremental export
synced-loop hash recording for Card, Dashboard, Document, Timeline,
Collection, TransformTag, and PythonLibrary.
- import integration: run a real import! from a mock source (the "right after
a pull" scenario), then fire a no-op update — covers record-content-hashes!
on the import path for Transform and TransformTag.
* Better comments in migration
* Batch record-content-hashes! to one extraction and update per model
Instead of re-extracting and updating each RemoteSyncObject one entity at a
time, group rows by model type and: run a single extract-query (keeping each
instance's local id so it correlates to its row), serialize each in-process,
and write all hashes with a single CASE update. This works for every identity
flavor — including the extract-query overrides (Collection, NativeQuerySnippet),
hybrids, and path models (Table/Field) that have no entity_id — since
correlation is by the instance's primary key, not entity_id.
Adds a unit test asserting the batched result matches per-row row->content-hash
across entity-id, override, hybrid, and path models.
* Chunk record-content-hashes! into batches of 500
Bound the IN-list and CASE expression sizes: partition each model type's rows
into chunks of content-hash-batch-size (500) and run one extract-query + one
CASE update per chunk, so very large synced sets don't produce a single
oversized statement. Adds a test (with a small batch size) covering rows that
span multiple chunks.
* Use t2/query CASE update for cross-driver portability
Switch the batched content_hash write from t2/update! (with a honeysql value)
to the t2/query {:update :set :where} form used by
queries.models.query/update-rolling-average-execution-times! — the established
cross-driver (H2/Postgres/MySQL) pattern for a single atomic CASE update.
* Use t2/update! for the batched content_hash CASE update
RemoteSyncObject has no transforms or update hooks, so t2/update! passes the
honeysql CASE expression straight through to the same dialect-aware compilation
as t2/query — equally portable across H2/Postgres/MySQL — while reading more
idiomatically with its conditions/changes maps.
* Make record-content-hashes! no-arg, selecting all rows itself
Both callers passed the same select-all of RemoteSyncObject rows, so move that
query into the function. It is always invoked after every row is synced.
* Run record-content-hashes! under serdes/with-cache
Extraction resolves foreign keys (collections, databases, users, …) while
serializing each entity. Wrapping the pass in serdes/with-cache memoizes those
resolvers so shared references are looked up once instead of once per entity.
The full-export caller was already inside a cache; the import caller was not.
* Record export/import metadata in one serialize + one batched write
Collapses the post-export/import RemoteSyncObject bookkeeping. Previously a full
export serialized the whole synced set twice (store! to write files, then
record-content-hashes! to re-serialize for hashes) and wrote each row up to
three times (blanket status, per-row file_path, batched content_hash); import
was similar.
Now:
- source/store-and-record! drives serialization from extract-query so it keeps
each entity's primary key, writing files and returning {:model_type :model_id
:path :content_hash} — so file_path and content_hash come from the single
serialization store! already does, for every model type (including path
models with no entity_id).
- record-exported-metadata! writes file_path and content_hash for all models in
one batched CASE update per chunk (not per model, not per row), matched by
primary key.
- import-content-metadata re-serializes the loaded entities once (import has no
store!), overriding file_path with the actual repo path for entity-id models;
used by both full and incremental import.
Extraction (extract-one) runs inside the extract-query reduction so the JDBC
ResultSet stays open. Replaces record-exported-paths! and record-content-hashes!.
* Batch the incremental export RSO updates
The incremental export fast-path wrote each synced row with its own t2/update!.
Replace with one batched CASE update per chunk (status/status_changed_at
constant, file_path/content_hash per-row CASE), keyed by the RSO :id the synced
plan already carries — no extra select. Fewer round-trips on the fast path.
* Fold import metadata into the insert (one batched insert per row)
Import previously wrote each RemoteSyncObject row twice: sync-objects! inserted
status/name, then record-exported-metadata! updated file_path/content_hash.
Compute the metadata before the insert and merge it into the insert maps
(merge-content-metadata), so the whole import reconcile is one delete-all + one
batched insert (full import) / one delete + one insert (incremental). Drops the
follow-up update pass and leaves record-exported-metadata! export-only.
* Make full export one RSO write per row
record-exported-metadata! now reconciles the whole table in one write per row:
one select of all rows, then chunked updates that set status for every row in
the chunk and file_path/content_hash for the exported ones (CASE with ELSE
keep-existing). full-export! drops the separate blanket status update. Each row
is written once; removed/delete rows just get their status reset.
* Chunk the import pipeline and export IN clauses
Bound the work that could exceed DB param limits / memory on large syncs:
- insert-with-metadata! processes the import in chunks of content-hash-batch-size
(extract + hash + enrich + insert per chunk, under one shared serdes cache),
so insert/IN params and the maps held at once stay bounded — replaces the
whole-table merge + unbounded insert. Used by full and incremental import.
- store-and-record! chunks each model's extract-query IN (restores bounding the
refactor had dropped from record-content-hashes!).
Removes the now-unused sync-objects!.
* Flatten store-and-record! into one transduction pipeline
Replace the nested mapcat>mapcat>map with a single (into [] (comp ...) targets):
expand each model into bounded [model-type id-chunk] work items, then stream
each chunk's extract-query through a serialize helper via an eduction — so
extract-one still runs while the ResultSet is open, with no intermediate
per-chunk sequence realized.
* Flatten import-content-metadata to a single transduction
Mirror store-and-record!: pull the per-instance work into a serialize helper and
stream each model's extract-query through it via an eduction instead of an inner
(into [] (keep ...)), so the whole function is one transduction over the model
groups with no intermediate per-model sequence realized. extract-one still runs
inside the extract-query reduction (ResultSet open).
* Fix events_test references to the removed sync-objects!
The export/import refactor removed sync-objects!, but events_test still called it
(2-arg form) to set up a synced baseline before firing move-out events, failing
clj-kondo (unresolved var) and the enterprise test load. Replace with a local
mark-synced! helper that does the same via spec/sync-all-entities!. Also refresh
two stale comments referencing removed functions.
* Use empty? instead of not
* kick ci
* Remove no-op (filter identity) from export root-targets
query-export-roots is consumed by mapcat (so nil returns contribute nothing) and
the [model-type id] tuples it produces are never nil, so the filter could never
remove anything. (PR review #76098)
* export-targets returns {} instead of nil
Removes a nil-as-empty signal: export-targets always returns a map, so callers
test emptiness (full-export! already uses empty?; extract-entities-for-export
uses not-empty) rather than nil. (PR review #76098)
* refactor
* Refactor!
* commment
* Stream full-export serialization; fix disabled-files deref
- store-and-record! feeds write-files! a lazy tee'd eduction so the full
serialized content is never held at once; only the lightweight per-entity
entries accumulate (GHY-3933 / PR review #11).
- Fix export!: pass @disabled-files (was the raw delay) to export-incremental!,
which reduces it via into.
- Fix resolve-status: return "synced" (was "synched").
* Stream the incremental export in two passes (bound memory)
The incremental plan used to serialize every dirty entity and hold all their YAML
in :upserts. Split it so the dirty set's content is never all in memory:
- Pass 1 (incremental-plan / incremental-updates-for-row / dep-writes): validate
the batch is syncable and compute each write's target path — using the new
source/entity->path, which renders no YAML. Produces a content-free plan of
{:writes [{:id :model_type :model_id :file_path}] :delete-paths :removed-ids}.
- Pass 2 (export-incremental!): stream-re-serialize each write's content (via
source/entity->content), using the pass-1 path, and feed it lazily to
apply-changes!; tee the per-row synced metadata. One entity's content is live
at a time.
stream-extract is a single comp'd eduction over the model groups (not a nested
eduction over the reduce-only extract-query) so it survives reduce- or
seq-consumption. entity->file-spec is now path+content composed.
* Batch the incremental pass-1 extraction (one query per model, not per row)
incremental-updates-for-row used to extract its entity itself, so incremental-plan
did one DB read per dirty row. Move extraction up into incremental-plan: batch-
extract the create/update rows on entity-id models via stream-extract (one query
per model/chunk), computing each row's path + existing-repo-file info once, then
run incremental-updates-for-row as a pure decision over the precomputed info.
removed/delete and non-entity-id rows need no extraction; a create/update row
whose entity is gone gets no info and is unsyncable; an extraction error sends the
whole batch to a full export (which self-heals). Paths are still computed with the
shared storage context (rows before deps) so dedup stays consistent.
* Simplify stream-extract: one key->row lookup, no tuple threading
Compute the [model-type model-id]->row lookup once instead of threading a per-model
id->row through the work-item tuple. Same two-mapcat structure (which keeps the
reduce-only extract-query on the reduce path); just less to read.
* Full export drops tracking rows for entity-id entities that left the set
Reconcile the removed-row handling between the two export paths. The incremental
path deletes the RemoteSyncObject row for a removed/delete entity; full export
left it as a synced row pointing at a file the wholesale re-serialize had already
dropped. Full export now deletes those rows too — but only for entity-id entities
that genuinely left (removed/delete status AND not in this export). Path/hybrid
removed rows (which can't go incremental) and removed rows whose entity is still
exported are still kept and reconciled to synced, as before.
* remove unnecessary function
* Rework export onto a streaming git commit-builder + chunk model
Reshape the export path around two ideas the reviewer asked for: an incremental
git commit API, and a uniform chunk-based extract pipeline shared by full and
incremental export.
Git protocol: add a stateful CommitBuilder (open-commit / stage-upsert! /
stage-delete! / finish-commit! / abort-commit!). The caller stages one file at a
time and the blob is inserted immediately, so the writer never receives or holds a
whole collection — streaming is structural, not a fragile single-pass contract.
write-files!/apply-changes! are kept, re-expressed on top of the builder. All
SourceSnapshot implementers (git, mocks, read-only views) implement it.
Export core: a chunk is [model-name rows<=500]; ->chunks (pure) groups+partitions
and extract-chunk runs one query per chunk. full-export! and incremental-export!
share stage-and-commit!, which doseq-stages each chunk's entities and accumulates
the synced metadata locally. After the push, set-version! + the RSO writes
(mark-synced! / record-exported-metadata! / removed deletes) run in one
transaction so the version can't advance past stale RSO state. Incremental still
validates first (incremental-plan) and aborts to a full export when unsyncable.
Supersedes the store-and-record! helper and the reduce-once write hardening (now
moot — no collection is passed to git).
* Tighten docstrings on the reworked export functions
One-line intent + args/return; move the how (batched CASE, select-all, streaming,
dedup, validation rules) into body comments where it matters. Fix the open-commit
docstring that still referenced the old :replace-dirs option.
* Batch enable-snippet-tracking! into one insert
Was one t2/insert! per snippets-namespace collection and per snippet (N+M
inserts); build the rows and do a single batched insert instead.
* Fix snippets export test for the removed-row reconciliation
A full export now drops the tracking row for an entity-id entity that left the
synced set (archived snippet, 'delete' status, not in export-targets) instead of
marking it synced — matching the incremental path. Update the assertion to expect
the row to be deleted.
* BEtter names
* Consistent capped chunking + shared path-free? predicate
- ->sized-chunks (group by model + cap at content-hash-batch-size) is now used by
stage-and-commit!, incremental-plan, and dep-writes, so every extract-chunk gets
a bounded chunk (the ≤500 cap was missing from the first two).
- dep-writes validates one chunk at a time and short-circuits on the first
colliding chunk, instead of realizing all decisions then scanning.
- path-free? is a pure predicate over the {:eid :file-exists? :file-eid} info,
shared by dep-chunk-writes (via repo-file-info) and incremental-updates-for-row's
create/rename branches — one collision check instead of three copies.
* docs and names
* Calculate the incremental plan one chunk at a time
incremental-export-plan now folds the create/update rows over ->sized-chunks
(chunk->updates per chunk), short-circuiting on the first unsyncable chunk like
dep-writes and stage-and-commit! do — instead of building an id->info map for all
rows and then deciding in a separate pass. Rows that need no entity (removed/delete,
non-entity-id, bad status) are decided up front. A gone entity is detected by the
found-vs-requested count; an extraction error falls back to a full export. Reuses
repo-file-info + path-free?.
* lots of doc improvements, some refactoring
* More renames
* Row is now map
* Chunk-as-map, deps return plans, and a pure plan combiner
- A chunk is {:model_type :rows}; export-closure/untracked deps speak
{:model_type :model_id} maps throughout.
- dep validation returns plan fragments ({:writes [...]}) merged like every other
fragment; row/chunk/dependency all produce plans combined by one function.
- Split merge-incremental-export-plans into a pure combiner (safe for direct
calls, e.g. the deps merge) and a -reducer wrapper that adds the reduced
short-circuit at fold sites — fixing a bug where the direct deps merge returned
a Reduced and broke the fall-back path.
- Move the chunk-plan try/catch into chunk->plan where the throwing happens.
* REfactor
* Nice!
* Go!
* Guard no-source export at the API call; add exportable-entity-chunks
- async-export! refuses up front (HTTP 400) when remote sync isn't configured,
instead of a nil-snapshot check inside the async task (which never fired) or a
cryptic clone error. export! no longer guards its snapshot.
- spec/exportable-entity-chunks: exportable-entities in chunk form.
* Decompose stage-writes into stage-write/chunk-stage-writes
Split per-entity staging (stage-write) and per-chunk staging (chunk-stage-writes)
out of stage-writes; synced metadata flows out as return values.
* Go for it!
* Self-contained exportable-chunks; tighten docstrings
- exportable-chunks reads its own export set and full-export!/delete-departed-entities!
flow over chunks; delete-departed derives the exported set from the chunk rows.
- Tighten prose-heavy docstrings to intent + Returns shape (content-metadata trio,
untracked-content-deps, incremental-export-plan, dep plan, row->content-hash).
* Refactor!
* Go for it!
* Retire the legacy write API (write-files!/apply-changes!, store!)
The CommitBuilder (open-commit → replace-all!/stage-upsert!/stage-delete! →
finish-commit!) is now the sole SourceSnapshot write path:
- merge-and-store! (the export-merged! path) writes via the commit builder
(replace-all-files!) instead of write-files!.
- Removed the dead store! and its tests/mocks.
- Removed write-files! and apply-changes! entirely — protocol methods, git
impls + helpers (and the now-unused with-staged), and the read-only stubs.
- Test fixtures (git_test, test_helpers_test) seed via local commit-builder
helpers, so they still exercise the same wholesale/incremental write paths.
* Untrack content_hash_bench.clj (committed by mistake)
* cljfmt: re-align test helper call sites
* Review fixes: drop unused fn, log skipped serialization, document commit lifecycle
- Remove the unused spec/exportable-entity-chunks.
- Log a warning when an entity fails to serialize for its content hash (was a
silent catch in import-content-metadata).
- Document the CommitBuilder resource/lifecycle contract (caller must finish/abort).
* Align RSO txn ordering (delete then update); use u/index-by in merge-content-metadata
- incremental-export! now deletes departed rows before updating RSO metadata,
matching full-export!'s order (the ops are disjoint within the txn, so this is
a consistency change, not a behavior change).
- merge-content-metadata uses u/index-by instead of an into/map/juxt build.
* Inline expand-nested-collections
* Bgrabow/ghy 3933 suggestions (#76640)
PR feedback
---------
Co-authored-by: Ben Grabow <ben.grabow@metabase.com>
* Move OnBeforeRequest* types into the api client middleware module
The `OnBeforeRequestHandler` / `OnBeforeRequestHandlerConfig` types lived in
`metabase/plugins/oss/api` but describe the request-manipulation middleware,
which lives under `metabase/api/client`. Relocate the type definitions to
`metabase/api/client/middleware` and re-export them from the `metabase/api/client`
barrel, so the middleware contract is fully contained in its own module. Pure
move plus import updates; no behavior change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Make the request middleware a single static plugin-slot list
The request-manipulation pipeline was split across two mechanisms: a hardcoded
list of `PLUGIN_*` slots in `middleware.ts`, plus a dynamic
`api.beforeRequestHandlers` array that three features pushed onto at runtime
(the embed-preview rewrite, the React-SDK embed referrer, and the iframe embed
referrer). That made the full set of handlers — and their order — impossible to
see in one place.
Promote the three dynamic handlers to plugin slots and drop the dynamic array,
so `middleware.ts` lists the entire ordered pipeline statically:
- `PLUGIN_API.onBeforeRequestHandlers.rewriteEmbedPreviewUrl`
- `PLUGIN_EMBEDDING_SDK.onBeforeRequestHandlers.reactSdkEmbedReferrer`
- `PLUGIN_EMBEDDING_IFRAME_SDK.onBeforeRequestHandlers.embedReferrer` (new
`onBeforeRequestHandlers` section on that plugin)
The owning features now install their handler into the slot instead of pushing.
`ApiClient` loses its `beforeRequestHandlers` field; `apiRequestManipulationMiddleware`
reads the static list and delegates to an exported `runBeforeRequestHandlers`
(kept pure so the merge/ordering semantics stay unit-testable). Slot resets on
plugin `reinitialize` now cover these handlers too, and the dead
`overrideRequestsForStaticEmbeds` slot is removed.
Because the slot positions are fixed, the public/static preview-rewrite no
longer depends on render-vs-effect registration timing.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Move the embedded-app header to a plugin handler
`client.ts` imported `isEmbeddingSdk` from `metabase/embedding-sdk/config` to
decide whether to send the `X-Metabase-Embedded` header — the last SDK import in
the api HTTP client. Move that logic into a `PLUGIN_API.onBeforeRequestHandlers`
slot (`setEmbeddedHeader`) whose default implementation does the
`isWithinIframe() && !isEmbeddingSdk()` check, and add it to the static pipeline
in `middleware.ts`. The plugin layer may import SDK config; `metabase/api/client`
no longer does.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Route param-remapping endpoints through the embed middleware
`api/card.ts` and `api/dashboard.ts` built the parameter-value remapping URL via
`PLUGIN_API.getRemapped*Url` — the last reason these endpoint files imported
`metabase/plugins`. Switch them to the same `:tag` template + params convention
the sibling `values`/`search` endpoints already use, so the existing embed
override middleware (which already has remapping transforms) does the rewrite.
Embed requests pass `entityIdentifier` instead of the real id, mirroring
`parameters/actions` — non-embed fills `:cardId`/`:dashId`, embed swaps to
`:entityIdentifier` and the guest handler injects the token. `substituteUrlTags`
encodes tag values, matching the getter's prior `encodeURIComponent`.
NOTE: verify the embedding e2e suite (guest/public/static parameter remapping)
before merge — this changes outward-facing embed request URLs and is not covered
by unit tests.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Remove the now-unused remapping-URL plugin getters
With the card/dashboard endpoints routed through the embed override middleware,
nothing reads `PLUGIN_API.getRemappedCardParameterValueUrl` /
`getRemappedDashboardParameterValueUrl` anymore. Drop the getters from the
plugin default and delete `setupRemappingUrls` (and its calls in the guest /
public-static embed setup), which only existed to override them.
`metabase/api/card.ts` and `metabase/api/dashboard.ts` no longer import
`metabase/plugins`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Move embedding auth/client request headers off the api client
The api client carried embedding-only request state (apiKey, sessionToken,
requestClient) and emitted the embedding request headers (X-Api-Key /
X-Metabase-Session / X-Metabase-Client / -Version / -Embedded-Preview). Move
these off the generic client into embedding request-header plugin handlers built
as closure factories: setApiKeyHeader / setSessionTokenHeader installed onto the
setEmbeddingRequestAuthHeaders slot as the active auth strategy, and
setRequestClientHeaders installed onto its own slot per embed mode. The embedding
setup sites install the handlers they need, each closing over its value, so no
embedding state or header logic remains in metabase/api and there is no shared
mutable module state.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Send the SSO session token on the initial embed requests
In the SSO/SAML/JWT path the session-token header was only installed by the
request-pipeline refresh handler, which runs after the auth-header step, so the
user/settings requests fired during init went out without X-Metabase-Session
(the bootstrap path already installed it eagerly). Install the strategy eagerly
right after the initial session refresh, sharing one refresh-and-install helper
with the pipeline handler.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Simplify RequestClientInfo type
* Split the embed-preview header off the request-client handler
X-Metabase-Embedded-Preview signals preview mode (IFRAMED_IN_SELF), which is
orthogonal to which client is embedding. It was riding inside
setRequestClientHeaders, where the IFRAMED_IN_SELF branch was unreachable from
the static and full-app install sites (they skip installing the handler in
preview). Move it to its own setEmbedPreviewHeader slot, installed only by the
public and SDK flows so static/full-app previews keep not emitting it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Set the SSO session header from the refresh handler itself
getOrRefreshSessionHandler re-installed setEmbeddingRequestAuthHeaders with the
fresh token, but that was too late for the triggering request: the handler list
is snapshotted per request, and setEmbeddingRequestAuthHeaders runs earlier in
the pipeline, so the request that detected expiry still went out with the stale
token. Have the handler return the session header itself instead — it runs after
the static auth-header slot, so the refreshed token applies to the same request
(mirroring getOrRefreshGuestSessionHandler).
Drop the three dynamic setSessionTokenHeader installs (SSO + bootstrap) in favour
of the returning handler; keep setSessionTokenHeader for MCP's static token via a
shared sessionTokenHeaders helper. Widen the getOrRefreshSessionHandler slot type
to allow returning a header config.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Type request handlers with OnBeforeRequestHandler
Replace the repeated inline `async (): Promise<Partial<OnBeforeRequestHandlerConfig> | void>`
annotations (and the local EmbeddingRequestHandler alias) with the existing
OnBeforeRequestHandler type. The plugin no-op defaults collapse to a shared
`noop` handler, and the embedding-request-auth factories and the two embed-
referrer handlers gain their parameter and return types from the annotation.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Skip null querystring params, not just undefined
appendQueryParameters only skipped undefined, so a param explicitly set to
null was serialized as the literal string "null" (e.g. ?cursor=null). Skip
nullish values so an absent param drops out of the querystring entirely.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Drop the cardId/entityIdentifier switch into the embed override
The param-value/remapping endpoints chose between cardId/dashId and
entityIdentifier at six call sites to keep the unused id out of the request.
Pass both unconditionally instead: the embed override now drops the real
cardId/dashId from the params bag when it rewrites the path to
:entityIdentifier, so the real id never leaks to an embed endpoint, and a null
entityIdentifier falls out of the normal-app querystring via the nullish skip.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Settle the JWT popover test's auth flow to stop a cross-test leak
The 'usage problem popover' test rendered the auth flow but never awaited it,
so its user/current + session/properties requests were still in flight when the
next test (the SSO subpath test) ran, tripping the strict unmocked-routes
afterEach. Await the user request after the synchronous assertion so the flow
settles within the test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Omit null entityIdentifier from parameter-value requests
The parameter-value dropdowns key their cache on stableStringify(request).
Always including entityIdentifier (null when not embedding) changed that key
from {dashId, paramId} to {dashId, entityIdentifier: null, paramId} — stable-
Stringify keeps null — so every cache lookup missed and the dropdown rendered
empty (caught by the PublicOrEmbeddedDashboardView Loki stories). Only include
entityIdentifier when it exists; cardId/dashId stay unconditional (the embed
override still strips them). Add actions tests pinning the cache-key shape.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Use camelCase ids in the remapped-parameter request types
The getRemapped{Card,Dashboard}ParameterValue requests used snake_case
(card_id/dashboard_id/parameter_id) while the url tags and the embed override
use camelCase (:cardId/:dashId/:paramId), so each query function had to remap
the keys. Align the request types (and their callers) to camelCase like the
sibling values/search endpoints, so the query is a plain passthrough. Rewriting
the url template to snake_case instead would have pushed mixed casing into the
shared override (its embed-url outputs and the cardId/dashId strip), so the
request types are the right place to converge.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Migrate bubble Radio usages off the deprecated component
Replace the two `bubble`-variant usages of the deprecated
`metabase/common/components/Radio`:
- EntityViewSwitch (Groups/Databases view switch) -> Mantine `Tabs`
`variant="pills"`. Drops the accent7 color scheme for the standard brand.
- FormFieldEditor "Field type" -> a horizontal Mantine `Radio.Group`. It's a
single-select choice, so a radio group fits better than tabs.
Centralize the pills active style in the shared `metabase/ui` Tabs so
`variant="pills"` renders our soft "selected" pill out of the box.
Update the EntityViewSwitch e2e selectors (label/radio -> tab). The Field type
tests are unchanged: Mantine `Radio.Group` keeps the radio/radiogroup roles.
Part of GDGT-2588 (PR 3 of 3).
* Replace data studio hand-rolled tabs with Mantine pill tabs
PaneHeaderTabs (the data-studio entity-header navigation) was a hand-rolled
Group of Button-as-Link pills. Render it with the shared Mantine `Tabs`
`variant="pills"` instead, deriving the active tab from the existing
`isSelected` logic. Tabs keep real `<Link>` anchors (cmd/middle-click, href)
via a small cast, since Mantine's `Tabs.Tab` isn't polymorphic.
Drops the unused `withBackground` prop and refines the shared pills styling.
* Remove the deprecated common/components/Radio component
All usages have been migrated to metabase/ui (Radio, Tabs, SegmentedControl)
across the normal/underlined/bubble variants, so the deprecated component is
now unused. Delete it along with its stories and spec.
* Document pills variant in Tabs stories and docs
Add Pills (and Pills + icons) stories plus a variant control, and document the
pills variant and the listBorder prop in Tabs.mdx.
* Fix tests for the Tabs/Radio migration
- TransformHeader and JobRunListPage specs: PaneHeaderTabs are now role="tab",
not "link".
- FormFieldEditor: give the "Field type" Radio.Group a real label (via the
Subtitle) so the radiogroup has an accessible name again — Mantine doesn't
name it from a sibling label + aria-label. Fixes the actions e2e specs that
query the radiogroup by name, and add a positive assertion for it.
* Simplify comment
* feat: convert SDK analytics events to use trackSimpleEvent
Replace the custom Snowplow "sdk" tracker approach with trackSimpleEvent
so SDK analytics go through both Snowplow (simple_event schema) and
Metaplow automatically — no custom schema registration or Metaplow
migration needed.
Two events via trackSimpleEvent:
- embedding_sdk_initialized: fires once per JS load from ComponentProvider
- embedding_sdk_component_rendered: fires per component mount via
triggered_from (component name) and event_detail (JSON with sdk_version,
auth_method, locale_used, and component-specific properties)
Removes: snowplow.ts (custom tracker), sdkTrackerReady Redux state
(no longer needed — trackSimpleEvent needs no async init). Dedup for
component events switches to instance-based (useRef + nextInstanceId)
from entity-based, matching the reference branch design.
setSdkTrackingContext() is called synchronously during ComponentProvider
render so child component effects can read auth_method on the first commit
without needing a Redux gate.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: pass live store getState to Snowplow instance-context plugin
getSdkStore() calls configureStore() each invocation — no memoization —
so the plugin's lazy getSdkStore().getState() was always reading from a
fresh empty store, leaving analytics-uuid, version, and token_features
undefined on every event. Fix: pass store.getState from useSdkStore()
(the actual ComponentProvider store) to initSdkTracker, which threads it
into the plugin closure so it reads the live state at event-send time.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VagN42CYBMhd1MHSDhbxTJ
* test: guard tracker-before-event ordering with an integration test
Uses real tracker.ts, snowplow.ts, component-events.ts (only
@snowplow/browser-tracker mocked) to assert that newTracker is called
before the first trackSelfDescribingEvent across a parent+child tree.
Falsifiability verified: reverting initSdkTracker to useEffect causes
the assertion to fail (invocationCallOrder 9 > 6), confirming the test
is exercising real render/effect ordering, not just mocked call counts.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VagN42CYBMhd1MHSDhbxTJ
* fix: address review findings for simple_event schema migration
- Cache sdkMetaplowEnabled at init instead of storing the full store ref
- Unexport trackSdkEvent; only trackSdkSimpleEvent is public
- Move SdkComponentName back to component-events.ts (SDK-only type)
- Remove triggered_from from SDK event types; component goes in event_detail
- Nest event_detail as { component, properties, global } envelope
- Restore field order auth_method/sdk_version/locale_used in global block
- Add sdkTrackerReady Redux flag; gates per-mount events until tracker is up
- Initialize SDK Snowplow tracker synchronously to prevent first-event drop
- Read metaplow gate from Redux store, not Settings singleton
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018HWtepijYZu4qkYB3XAgwT
* fix: tighten trackSdkSimpleEvent to EmbeddingSdkEvent union type
Narrows the parameter from the open SimpleEventSchema to the closed
EmbeddingSdkEvent union so only known SDK events are accepted at
call sites. Updates two Metaplow gate tests to supply the now-required
event_detail field and runs Prettier on format-checked files.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jux5AAEEsYVNUNSUEK8zBF
* refactor: remove implementation-detail tests and unnecessary diffs
- Delete reducer.unit.spec.ts (Redux plumbing; behavior covered by events
and tracker specs)
- Delete ComponentProvider test 1 (call arity check); update tests 2 & 3
to use toHaveBeenCalledWith instead of magic index access
- Extract parseEventDetail/findEventByComponent helpers in e2e spec to
eliminate repeated try-catch JSON.parse blocks
- Restore setSdkTrackerReady position in reducer.ts to append after
setInitialDashboardTabId (matches master, removes unnecessary diff)
- Restore JSDoc on sdkTrackerReady in types.ts
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jux5AAEEsYVNUNSUEK8zBF
* refactor: simplify event_detail assertions in events spec
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jux5AAEEsYVNUNSUEK8zBF
* refactor: move SDK event types and schema URI out of metabase-types
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jux5AAEEsYVNUNSUEK8zBF
* fix: pass metaplow-url and analytics-uuid explicitly to trackMetaplowEvent
Convert trackMetaplowEvent to object-style signature with metaplowUrl and
analyticsUuid params that default to Settings.get. The SDK caches both
values from the Redux store at initSdkTracker time and passes them
explicitly, bypassing the Settings singleton which is absent in a
customer's page.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jux5AAEEsYVNUNSUEK8zBF
* refactor: convert all metaplow.ts functions to object-style params
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jux5AAEEsYVNUNSUEK8zBF
* refactor: revert core app metaplow to positional params
Settings.setAll already runs in SDK auth flow, so trackMetaplowEvent and
trackMetaplowPageView can read metaplow-url and analytics-uuid via Settings.get
directly. No need to change the core app function signatures for SDK's sake.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NQTnAAmpqsWDQqNymVmnck
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Fixes UXW-4614.
There are effectively three "sizes" of query results for rendering
dashboard subscriptions. The PDF renderer had so far only supported the
smallest of them ("fits in memory").
The QP always *streams* query results, but the subscriptions sink
collects them in memory up to a threshold, then spills them to a temp
file and streams the rest there.
If the temp files get too large, we don't risk an OOM by reading them and
trying to render the dashcard. The files are discarded and the card's
results are marked `:too-large? true`.
This PR makes the PDF renderer correctly read from temp files, and
properly surfaces a "results too large" message just like the FE if the
results are too big to render in one chart.
Fixes UXW-4040.
The timezone being dropped caused it to default to UTC, causing the
bug. With the `results_timezone` preserved the correct timezone is used
to render the x-axis and tooltip labels, achieving correct results.
Migrate the "{user} accepted their Metabase invite" admin notification
email from the legacy _header/_footer layout to the newer card-style
template (_dashsub_alert_header) already used by the new-device login
and transform-failure emails. Updates the sender to build the nested
context map the new template expects.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Make Snowflake data loaded checks look at row count.
We have some unidentified process that destroys the test-data dataset
and causes failures such as this:
https://github.com/metabase/metabase/actions/runs/28206004397/job/83561786972#step:5:381
Sometimes the database exists but the tables have double or triple the
expected number of rows.
First change: refuse to destroy the test-data database. This should
just never happen. If this ends up throwing, it should allow us to
identify where the deletions are coming from. (Unless the culprit is
someone calling DROP DATABASE directly.) Of course, we really should
be blocking *all* deletions of static datasets, but this is by far the
most commonly-used, and we don't have a good predicate for identifying
static datasets anyway.
Second change: don't consider a dataset loaded if it's been
double-loaded or triple-loaded. Previously we would just look at
whether the dataset's database existed and ignore the tables inside
it, but now we'll be able to see when it's been double-loaded and
treat this the same as if it had not been created at all.
Third change: when creating a database, ensure it gets dropped first
before creating. Without this, the previous change would just cause
the datasets to get loaded increasingly many times. This actually
reverts a change made previously in #74464, but we have a lot more
data now about what's happening, and making create-db into a no-op
when it already exists has turned out to be counterproductive because
of the double-loading issue.
* Remove parallel from database-local-settings-test
This fails in CI sometimes with the following error:
clojure.lang.ExceptionInfo: Error rolling back after previous error: (conn=362) Deadlock found when trying to get lock; try restarting transaction
This indicates it is probably not safe to do in parallel with other tests.
* GDGT-2533 Migrate text and syntax tokens
Sweep all consumer references from the legacy text/syntax tokens to their
new names (added in the foundation PR):
- text-tertiary -> text-disabled (and -inverse) — value shifts from solid
orion to orion-alpha (intended recolor)
- text-hover -> text-brand-hover (value unchanged)
- syntax-parameters(-active) -> text-syntax-parameter(-active)
- saturated-{blue,green,yellow,red,purple} -> core-{blue,green,yellow,red,
purple}-saturated (value unchanged)
`text-tertiary` and `text-hover` are also SDK public theme keys, so the
SDK_TO_MAIN_APP_COLORS_MAPPING right-hand sides gain the new internal names
as second targets and the public keys are left unchanged.
Per design review, the five saturated colors are renamed to
`core-{color}-saturated` palette primitives rather than `text-syntax-{role}`;
this PR adds those token definitions, removes the superseded text-syntax-*
ones, and keeps the legacy saturated-* definitions emitting until cleanup.
`text-secondary-opaque` is left untouched (open design question).
* Formatting
* Update Loki Snapshots
* Keep public SDK theme key text-tertiary in embedding examples/tests
text-tertiary is an embedder-facing SDK theme key (MetabaseColors), not an
internal token. The blanket sweep wrongly renamed it to the internal
text-disabled in three embedding spots that live outside the type-check
scope (e2e specs + the CLI scaffold snippet). Revert those to the public
text-tertiary; the SDK maps it internally via SDK_TO_MAIN_APP_COLORS_MAPPING.
---------
Co-authored-by: Metabase Automation <github-automation@metabase.com>
* Migrate underlined Radio usages to Mantine Tabs
Replace the `underlined`-variant usages of the deprecated
`metabase/common/components/Radio` with Mantine `Tabs` from `metabase/ui`:
AccountHeader, the admin PermissionsTabs, and the BaseChartSettings section
picker. Mantine's default tab styling already matches the old underlined look
(neutral list divider + brand active underline).
Add a thin `metabase/ui` Tabs wrapper (replacing the bare Mantine re-export)
with a typed `listBorder?: boolean` prop. When `false` it suppresses the tab
list's own divider (default variant only), so a full-width divider can instead
be drawn on a parent container — avoiding full-width hacks and absolute
positioning. The active tab's underline is unaffected.
Adopt that pattern everywhere the list divider is customized, and migrate the
tabs that previously hid the divider with one-off CSS to use it, deleting their
CSS modules:
- PeopleListingApp, TenantsListingApp, SourceReplacementModal (were
`::before { display: none }`)
- AccountHeader and BaseChartSettings now draw the divider on a parent
Also tidy the permissions header layout (parent-drawn divider, toolbar back in
normal flow) and convert a leftover utility-class `div` to a `Box`.
Update BaseChartSettings tests from radio `toBeChecked()` assertions to tab
`aria-selected`.
Part of GDGT-2588 (PR 2 of 3).
* Update tests for radio -> tab structure
Fix tests that queried the migrated tab bars as radios:
- section picker (Scalar, viz-charts e2e): role="radio" -> "tab",
toBeChecked() -> aria-selected
- permissions tabs (CollectionPermissionsPage, admin-permissions e2e,
command-palette e2e, transforms e2e): role="radio"/"radiogroup" and
label-based selectors -> role="tab"
* Fix data-permissions e2e for radio -> tab structure
The "data permissions > warns about leaving with unsaved changes" test
selected the Collections permissions tab via a label selector; the tab is
now a role="tab" button.
* Reflect listBorder in Tabs stories; show focus outline
- Add the `listBorder` prop to the Tabs Storybook stories (boolean control +
a dedicated "Without list border" story).
- Show a keyboard focus outline on tabs via `:focus-visible` instead of
suppressing it, for accessibility.
Consolidate dashboard sidebar-switch repros, drop flaky 27356 block
Fold the parameterized -> non-parameterized sidebar-switch coverage into
the better-asserted 26230 sticky-positioning test by making its switch
target a param-less bookmarked dashboard, and remove the redundant,
weakly-asserting flaky block.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Update E2E test timings with averaged data
Collected timing data from 7 days of test runs.
All specs with timing data were included (simple averages).
🤖 Generated automatically by update-e2e-timings workflow
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: GitHub Actions <actions@github.com>
* Add transient creationType field to UnsavedCard type
* Type STRUCTURED_QUERY_TEMPLATE and allow null source-table
* Declare dev-only __lib_query/__lib_metadata/Lib window globals
* Accept null/undefined collection in collection type guards
* Remove @ts-nocheck from metabase-lib Question
Open the navbar inside assertTrashSelectedInNavigationSidebar
Routes like a dashboard or question collapse the navbar, so the assertion
needs it open first. Two of the four call sites opened it manually while the
other two relied on the route not collapsing it. Fold the open into the
assertion helper so every call site is covered uniformly, and drop the now
redundant standalone calls. openNavigationSidebar is idempotent and self-heals
against a pending collapse, so calling it unconditionally is safe.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Navigating to a dashboard/question collapses the navbar, and that collapse
can land a beat after the page content renders. The previous helper took a
single non-retrying snapshot of the sidebar's open state, so it could read a
lingering-open navbar, skip the toggle click, then watch it collapse out from
under the be.visible assertion. Re-check after acting and re-open if it
collapsed, bounded to a few attempts.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Add ExplicitSize test for measuring a child that renders content after mount
PivotTable returns null on its first render, so ExplicitSize has nothing
to measure at mount. The regression was that once it rendered content,
its size was never recalculated without a resize event, leaving the
child stuck at null dimensions. Cover that contract directly at the unit
layer rather than through a flaky e2e viz-switch repro.
* Delete e2e repro for 51926
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The test forced email-configured?=true with a response-modifying intercept
on GET /api/session/properties; a reload aborting an in-flight refetch made
the callback throw "Socket closed before finished writing response". Set
email-smtp-host server-side instead so email-configured? is genuinely true.
* Add monitor dir in metabase-enterprise
* GDGT-2676: Migrate Dependency Diagnostics under Monitor space
* GDGT-2677: Migrate Admin -> Tools under Monitor space
* GDGT-2676: Fix paths
* GDGT-2676: Create separate high-level directories for migrated features
---------
Co-authored-by: Kamil Mielnik <kamil@kamilmielnik.com>
After publishing, the active-path effect in useExpandedState auto-expands
the database. The test then clicked the (already-expanded, active) database
row, which only toggles expansion and collapses it again — so the schema
fetch never re-fired and the table rows disappeared, causing the getSchema
wait timeout and the missing [data-type="table"] failures.
Re-open Data Studio for a clean, collapsed tree before expanding the
database, matching the reliable pattern the sibling tests already use.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Make workspaces deletable from any database state (GHY-3954)
delete-workspace! only deprovisioned :provisioned rows, but the model-level
guard rejects any non-:unprovisioned row. A WorkspaceDatabase stuck in
:provisioning or :deprovisioning (crash mid-flight, or a failed warehouse
teardown) made the workspace permanently undeletable with a 409.
delete-workspace! now drives every active row through a best-effort,
delete-only teardown (force-teardown-for-delete!) that works from any state:
it reconstructs the iso schema/user names deterministically from the row id,
drops them idempotently, always clears app-DB TableRemapping rows so query
routing is never left dangling, and forces the row to :unprovisioned. When the
warehouse is unreachable the workspace is still deleted; the 200 response then
carries :orphaned_resources and a :message naming the inert schema/user left
behind and why.
Recovering a stuck row without deleting (startup reconciliation / admin
force-unstick) remains GHY-3550.
* Surface orphaned warehouse resources in logs and the delete UI
Log a per-row warning (with the exception) when warehouse teardown fails
during delete, plus a consolidated warning carrying the full human-readable
orphan summary.
On the frontend, type the delete response (DeleteWorkspaceResponse) and, when
the workspace was deleted but warehouse cleanup partly failed, show a warning
toast naming the leftover schema/user objects and why they couldn't be removed
— the response is otherwise silently discarded since delete returns 200.
* Gate workspace delete on pending databases with an ignore-pending flag
A database still :provisioning/:deprovisioning may have warehouse work in
flight, so delete-workspace! now refuses by default (409, :pending_databases
lists them) rather than tearing such rows down. The new ignore-pending? arg
(ignore-pending query param, default false) leaves those databases' warehouse
resources untouched and removes only their app-DB rows. :provisioned rows are
still torn down, and the orphaned-resources reporting for unreachable warehouses
is unchanged.
On the frontend, the delete modal fetches the workspace's databases, and when
any are pending it swaps in a warning that lists them by name and confirms with
ignore-pending=true.
* Mock get-workspace in WorkspaceItem delete-modal test
The delete modal now fetches the hydrated workspace when opened, so the
'opens the delete modal' test hit an unmocked GET /api/ee/workspace-manager/:id.
Check dynamic dataset in do-with-dataset-definition/with-transform-cleanup!
Monitoring failures for bigquery/snowflake on master, we're seeing
these are still trying to do creation/deletion of datasets and
occasionally failing on bigquery and snowflake.
These are specific to the tests for transforms and actions.
Long term we want to make it so this can work reliably. Short term we
just need the tests to fail less when blocking releases and allow us
to unquarantine sooner.
* Consolidate usage-auditing row-chart drill-through e2e flake fixes
clickRowChartBarForLabel coordinate-clicked the chart container at a blind
labelRect.right + 30 offset, which intermittently landed on empty SVG so the
drill-through never fired (cy.wait("@conversations") timed out) or a near-miss
drilled a neighboring group. Two things conspired: the page only waits for the
audit metadata so the chart's dataset query + render could still be in flight,
and row charts measure their size asynchronously (ExplicitSize, throttled) and
first render with zero-width bars — assertChartRendered passes in both states.
Anchor on assertChartRendered, then click the measured (non-zero-width) bar
whose vertical band contains the label's midpoint, asserting exactly one match.
Cypress re-resolves the element's center at click time and retries for
actionability, so the click can't miss.
selectUserFilter anchored on `url includes "user="`, a no-op after a
drill-through since the URL already carries the drilled user, so it didn't gate
on the filter actually switching. Anchor on the select's value instead,
mirroring selectDateFilter.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Use graphics-symbol role query for row-chart bar drill
Swap the ad-hoc `[aria-roledescription="bar"]` attribute selector in
clickRowChartBarForLabel for the a11y query cy.findAllByRole("graphics-symbol"),
matching the conventions' preference for role queries over CSS attribute
selectors in spec files. Functionally equivalent — both resolve the same ECharts
row-chart bars within the scoped chart card.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Give the chart-render assertion a budget for the ECharts mount
The usage-stats page waits only for the audit metadata before rendering; each of
its six charts then fires its own adhoc /api/dataset query and mounts ECharts
independently. getChartCard resolves as soon as the chart title paints (a
Skeleton placeholder), but the [data-testid="chart-container"] only mounts once
that card's data query returns. With all six mounting at once under CI CPU
contention, the container mount routinely overran Cypress's default 4s budget,
surfacing as "Unable to find an element by [data-testid=chart-container]".
Give the chart-container readiness assertion a 12s budget (CHART_RENDER_TIMEOUT)
so it waits out the ECharts mount instead of racing it. This hardens every chart
assertion in the spec, including the assertChartRendered anchor the drill-through
helpers rely on.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Fix model filter dropdown lost on serdes sync (GHY-3946)
A dashboard/model filter mapped to an MBQL model column rendered as a
text-search box instead of a value dropdown after the content was synced
via serdes (git sync).
Root cause: a model's result_metadata is serialized with overrides only
(display_name, semantic_type, ...); the structural :id that links each
column to its underlying Field is dropped on export and never restored on
import, because populate-result-metadata stores passed-in metadata
verbatim during deserialization. Without that link the column can't
resolve has_field_values, so the widget falls back to text search.
Fix: add a generic serdes load-finalize! lifecycle hook that runs after
every entity is loaded (when the referenced Tables/Fields are present),
and implement it for Card to re-derive an MBQL model's result_metadata
field links from its query while preserving the serialized overrides.
This keeps the serialized form unchanged (no :id in the YAML) and avoids
the load-ordering fragility of re-inferring at insert time.
* Drive model metadata restore through model-preserved-keys
Rework the Card load-finalize! to mirror the export: recompute the fresh
columns from the query, then overlay the serialized overrides using the
same lib/model-preserved-keys set (with this model's native flag) that the
export uses to decide what to keep. This replaces the opaque
infer-metadata-with-model-overrides call and removes the separate native
guard — native-ness now flows through model-preserved-keys, so native
models (whose infer-metadata yields nil) keep their already-complete
serialized metadata untouched.
* Remove comments on native models in card.clj
Removed comments about native models and infer-metadata.
* Switch to serializing model result_metadata :id as a portable ref
Carry each model column's :id through serdes as a portable field
reference (via *export-field-fk*), instead of recomputing it on load.
serdes already knows how to remap it on import (import-result-metadata),
and its FK resolution handles load ordering. This drops the load-finalize!
hook (and its serialization/load plumbing) in favor of a one-key change to
the model result_metadata keep-set.
* Re-derive model result_metadata :id on deserialization
Instead of serializing the model column :id (reverted), restore it on
import: when deserializing a model, populate-result-metadata re-derives
the result_metadata from the query (recovering each column's :id link to
its underlying Field) and blends the serialized overrides on top. This
assumes the referenced Fields are already present on the destination; if
they aren't resolvable, it falls back to the passed-in metadata.
Add a mandatory closing section to the e2e-test-review skill that classifies
each it() block as load-bearing e2e, narrowable, or a unit/component-test
candidate, grounded in which layers the test actually exercises. Wire it into
the review process and final-check steps so every report includes it.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The 'should create collections and save transforms to them' e2e test
created a nested transform collection and then clicked the parent row's
name to expand it. The row only toggles when row.getCanExpand() is true,
i.e. once the post-create collection-tree refetch has landed and
re-rendered with the new child. Clicking the name before that re-render
was a no-op, leaving 'Q4 Reports' hidden until the 4s findByText timed
out — a race that surfaced on faster (unthrottled) runs.
Waiting on the GET /api/collection/tree refetch wasn't enough: the
re-render can lag the network response. Instead, wait for the row's
Expand control, which the TreeTable only renders once the row is
actually expandable, and click that. Its presence is the deterministic
signal the child has landed, and it only ever expands (never toggles a
collapse), removing both the readiness race and the toggle ambiguity.
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Replace deprecated Radio with metabase/ui Radio (normal variant)
Migrate the four `normal`-variant usages of the deprecated
`metabase/common/components/Radio` to Mantine's `Radio.Group` + `Radio`
from `metabase/ui`:
- query_builder LimitPopover
- ActionCreator FieldSettingsPopover (input type)
- ValuesSourceTypeModal (parameter value source)
- enterprise EditSandboxingModal
Mantine's `Radio.Group` only handles string values, so the two non-string
cases are adapted at the radio boundary:
- EditSandboxingModal: boolean options -> "column"/"saved-question"
- ValuesSourceTypeModal: the null source type ("From connected fields")
-> a sentinel string via serialize/deserialize helpers, keeping
`ValuesSourceType` (incl. null) as the domain type everywhere else
Part of GDGT-2588 (PR 1 of 3).
* Adjust visual alignment
* Preserve search relevance order for command palette actions (#76055)
The command palette re-sorted basic action results by a static
BASIC_ACTION_ORDER array, discarding the relevance order kbar's
useMatches already computes via Fuse.js. Since basic actions only
render when there is a search term, this static sort never served as a
useful default ordering and only buried the best match (e.g. typing
"New c" pushed "New collection" below "New question").
Drop the static re-sort so kbar's relevance order is preserved, and
remove the now-unused BASIC_ACTION_ORDER constant and processSection
sortOrder parameter.
* Update e2e test
* Make command palette nav test independent of action relevance order
* Try fixing e2e
* Fix test
* Tie-break command palette actions by default order via kbar priority
Rather than dropping the default action order entirely, encode it as a
small kbar `priority` so it only orders actions whose relevance scores
are equal. Typing "New" still surfaces "New question" first, while a
stronger match like "New c" surfaces "New collection" (metabase#76055).
Adds unit coverage for both cases and restores the concrete navigation
assertions in the palette specs.
* Rank palette actions by relevance with registration-order tie-break
Type a query and the command palette now orders basic actions by kbar's
search relevance, falling back to registration order only to break ties
between equally-relevant matches (via a tiny per-action priority). This
fixes 'New c' burying 'New collection' while keeping 'New question' first
for the bare 'New' query.
Also drop the redundant 'new embed' and 'new SQL' keywords: each merely
duplicated words already in the action's name, but distorted Fuse scoring
for short prefixes (ranking 'New embed'/'New SQL query' above the plain
create actions for 'New'). They remain findable by name.
* Restore PageDown-based palette nav assertions with updated order
* Widen isDimension/isMetric and formatField parameter types
* Fold Base into Field and remove @ts-nocheck
* Type the Field constructor input and keep runtime-only fields on the class
* Use the API field directly in MappedFieldPicker; document Field hydration
* Move Field instance fields onto the class; drop vestigial JSDoc type comments
* Remove _comesFromEndpoint reference in clojurescript
* Take a NormalizedField in the Field constructor (removes constructor casts and lies)
* Move comment
* Drop dead SOFT_RELOAD_CARD branch from questions reducer
* Hydrate questions slice via RTK endpoints, remove INJECT reducer
* Hydrate metadata store from entity mutation endpoints
* Revert "Hydrate metadata store from entity mutation endpoints"
This reverts commit fc20ae580a.
* Fix flaky data-studio bulk "syncing multiple tables" e2e test
The "syncing multiple tables" test restores the postgres-writable snapshot
in its body but, unlike every sibling test in this spec, never re-signs in
afterward. Visiting Data Studio could then land in an unauthenticated state
where the TablePicker never issues the GET .../schema/public request, so
`cy.wait("@getSchema")` timed out after 5s with "No request ever occurred".
Add `cy.signInAsAdmin()` after the restore — the same deterministic
re-authentication the reliable sibling tests already perform — so the
database expands and the schema request fires before the wait.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Fix flaky "syncing multiple tables" e2e test: read table IDs deterministically
The test derived the expected table IDs from the intercepted `@getSchema`
UI response. Under stress that response body is occasionally a truthy
non-array (e.g. an error map), so `tables.find` threw
`TypeError: tables.find is not a function`. Read the table list from a
direct `cy.request` instead, which deterministically returns the array,
and keep `cy.wait("@getSchema")` purely as a UI-readiness wait.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Fix flaky resize-column-width e2e test with real-events drag
The read-only column-resize test hand-rolled a synthetic mousedown/
mousemove/mouseup sequence plus a magic cy.wait(10). The TanStack resize
handle re-renders while the table measures itself, so the synthetic
events intermittently landed on a stale node and the drag was lost (or
only partly applied), leaving the column width unchanged.
Drive the drag with cypress-real-events (CDP-level input) and re-query
the resize handle for every event so each one targets the live node,
matching the proven pattern in notebook-native-preview-sidebar. Drop the
numeric wait in favor of the existing retrying assertion.
* Harden flaky read-only column resize e2e with bounded drag retry
The synthetic TanStack column-resize drag in saved.cy.spec.js was
occasionally lost or only partially applied because the resize handle
re-renders while the data grid measures/auto-sizes itself, so the
width assertion would time out (e.g. expected 72 to be at least 82).
A prior cypress-real-events rewrite did not resize the 4px, hover-only
handle at all. Revert to the proven synthetic drag and re-run it until
the column has actually widened (bounded, gte-checked, idempotent), so
a dropped drag becomes a non-event instead of a failure.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Stabilize pivot dashcard drill flake in dashboard-filters-misc
The 'does not use extra filtering stage for pivot tables' test drilled into
the pivot dashcard via its legend caption immediately after a raw
cy.button("Save").click(), without waiting for the dashboard to leave edit
mode. The drill only navigates in view mode, so when the click landed during
the save/edit-to-view transition it did nothing and /api/dataset/pivot never
fired, timing out cy.wait("@datasetPivot").
Use H.saveDashboard(), which waits for the save PUT, dashboard reload, and the
edit bar to disappear (view mode) before proceeding, so the drill reliably
triggers the pivot request.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Exit dashboard edit mode without awaiting a save PUT in pivot filter test
The 'does not use extra filtering stage for pivot tables' test only inspects
parameter mapping options (verifyDashcardMappingOptions opens the picker and
clicks away); it never creates a mapping, so the dashboard stays clean and
clicking Save issues no PUT /api/dashboard/*. The prior H.saveDashboard() call
waited unconditionally on the saveDashboardCards request, which only fired on
the runs where the dashboard happened to be dirty, timing out otherwise.
Pass awaitRequest: false so saveDashboard clicks save and waits for the edit bar
to disappear (view mode) without awaiting a PUT that may never occur. This still
guarantees the following legend-caption drill runs in view mode, where it
reliably triggers the pivot dataset query.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The usage-auditing seed timestamps the "today" conversations at
`today - {15min, 30min, 2h}` where `today` is the current instant in
UTC. When the e2e suite runs shortly after UTC midnight, those events
roll into the previous UTC day, while the reported `:date` (the UTC
calendar day) advances to the new day. The "Conversations by day"
chart's last dot is then yesterday, so drilling navigates to
`date=<yesterday>` and the assertion `search` includes `date=<today>`
times out (e.g. `'?date=2026-05-25' to include 'date=2026-05-26'`).
Anchor `today` at noon UTC so every seeded "today" event stays firmly
inside the current UTC day regardless of run time. Relative offsets and
all per-hour relationships between the seeded conversations are
preserved.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fix click-behavior tab-fallback flake: await target dashboard load before save
The "fall back to the first tab" test saved the dashboard immediately after
opening the click behavior sidebar, gated only by a negative assertion that
the tab selector is absent. That assertion passes trivially before the target
dashboard's fetch resolves, so the save raced ahead of the `migrateDeletedTab`
effect that falls the invalid tabId back to the first tab — the only thing
that dirties the dashboard. With no change registered, the PUT never fired and
`cy.wait("@saveDashboard-saveDashboardCards")` timed out.
Anchor on a positive signal (the click-mappings heading, rendered once the
target dashboard has loaded) before asserting the tab selector is absent, so
the migration has run and the dashboard is dirty by the time we save.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The "allows publishing and unpublishing multiple tables" test clicked the database row to expand it and then waited on the schema fetch. The picker tree is still mounting when the databases request resolves, so the click on the row landed before its expand handler was wired and was dropped — the GET .../schema/public?include_hidden=true request that populates the tables never fired, timing out cy.wait("@getSchema") with "No request ever occurred" (and downstream, the [data-type=table] rows never rendered).
Click the database's dedicated expand toggle once it has rendered in its collapsed state (aria-expanded=false), then assert it reaches the expanded state. The pre-click assertion anchors on a positive readiness signal so the toggle is fully interactive before we click it, making the schema fetch fire deterministically.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Fix flaky dashboard filter card-reload e2e test
The "should not reload dashboard cards not connected to a filter" test
re-aliases the dashcard query intercept after saving the dashboard, then
asserts exactly one filter-triggered card query fires. Saving exits edit
mode and reloads both dashcards, but the test waited for only one of those
two post-save queries. The unwaited second query, slow under network
throttling, arrived after the intercept was re-registered on the same URL
and got counted as a filter-triggered query, so the assertion saw 2 instead
of 1.
Wait for both post-save card queries (total of four) to settle before
re-aliasing, so only the genuine filter-triggered query is counted.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Settle dashboard reload by loading state, not query count
The flaky dashboard filter card-reload test re-aliased the card-query
intercept right after saving, then asserted only one filter-triggered
query fired. A post-save reload query could still be in flight and get
captured by the new intercept, inflating the count.
The previous attempt waited for a fixed number (4) of post-save queries,
but how many fire is not deterministic — cached card results may be
reused, so the total varied (3 vs 4) and the wait failed.
Wait instead for both dashcards' loading indicators to clear before
re-aliasing, which drains the post-save reload regardless of how many
queries it issued.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The dependency graph is recomputed asynchronously on entity create/update,
so waitForBackfillComplete (global one-time flag) doesn't guarantee freshly
seeded entities have surfaced in the unreferenced list. Visiting too early
renders an incomplete list and findByText times out.
Replace the per-test ad-hoc waits with a single waitForUnreferencedAnalysis
helper that polls the unreferenced endpoint until every seeded entity (table
included, via the slower DB-sync path) is present. Apply it to all ten
presence-asserting tests, including five that shared the same race but were
never individually patched. The lone absence-asserting test keeps the global
backfill wait, since presence can't be polled for entities that should be
absent.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Revert to returning unimported fk when importing fk fails
* Only ask for dashboard_id, for perf and to avoid pulling in problematic dangling refs in visualizer settings
* Create HNSW semantic-search index just-in-time, only when configured
The semantic-search index table always got an HNSW index on its embedding
column, regardless of whether anything used it. HNSW indexes are expensive
(build time, memory, disk), and a cluster hosting thousands of instances paid
that cost for every one even though ~99% are fast enough with an exact scan.
Flip the default vector-search strategy to :brute-force (exact, no index) and
only build the HNSW index when an instance is actually configured for :hnsw:
- create-index-table-if-not-exists! builds the HNSW index only when the
configured strategy is :hnsw (covers env-var config and restarts). Extracted
into idempotent create-hnsw-index-if-not-exists! (supports CONCURRENTLY).
- Setting the strategy to :hnsw at runtime triggers an async background build
on the active index table (via core/build-hnsw-index-async!), so the settings
call returns promptly.
- A query that requests :hnsw (e.g. a per-request override) when no HNSW index
exists now fails fast with a clear error, rather than silently doing a slow
exact scan.
* Add clj-kondo ignore marker to index-relfilenode test helper
Matches the sibling pgvector test helpers; suppresses
:metabase/test-helpers-use-non-thread-safe-functions.
* Build HNSW index only on the transition into :hnsw; test trigger and build
The setter previously kicked off a background build on every set to :hnsw,
including the restore that with-temporary-setting-values does. Gate it on the
actual transition (old != :hnsw) so redundant sets don't spawn stray builds.
Tests, split so neither relies on a real (flaky) future:
- index-test asserts the setter *triggers* a build only on the transition into
:hnsw (spying on the async build).
- pgvector-api-test asserts ensure-active-hnsw-index! actually *builds* a missing
index, and is a genuine noop when the index already exists (stable relfilenode)
or when there is no active index.
* Build HNSW index via event instead of requiring-resolve; add startup safety net
* Test the indexer task's startup HNSW safety net
The "allows to edit attributes for tables" test clicked the database to
expand it and then immediately queried for table rows, without waiting
for the schema/tables to load. The tree picker is populated by an async
GET .../schema/public request, so the `[data-type="table"]` rows were
sometimes not yet rendered when the assertion ran, timing out after
4000ms.
Add `cy.wait("@getSchema")` after expanding the database — the same
deterministic wait the sibling tests in this spec already use — so the
tables are guaranteed to be present before selection.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The two auto-wire suggestion toasts (Orders Question + Reviews Question)
stack and briefly overlap while the undo list re-measures heights and
animates them into their final positions. During that window a sibling
toast can cover the precisely-scoped Auto-connect button, so cy.click()
fails its actionability check with "covered by another element".
Force the two Auto-connect clicks past the transient obstruction; the
target buttons are already uniquely scoped via
.contains(...).closest("toast-undo").
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The "preserve zoom and pan after resize" test reads the first leaflet
marker position before and after a 1px viewport resize and asserts the
two are within 5px. The settle helper only required two consecutive
should() retries to land within 0.5px. Cypress retries faster than the
browser repaints, so two reads can fall in the same animation frame and
return an identical getBoundingClientRect() mid-animation, a false settle
that captured the after-zoom baseline before leaflet finished animating,
producing the historical 8-29px gaps.
Anchor the settle on elapsed wall-clock time (performance.now) instead of
read-count: require the marker to hold within 0.5px of an anchor for at
least 200ms before sampling, which a single-frame coincidence cannot
satisfy.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The picker resolves its initial path asynchronously before auto-navigating
into the data source's collection: a card-metadata fetch plus one
listCollectionItems request per collection level. On a throttled CI runner
that serial chain outlasts Cypress's default 4 s command timeout, so the
clickPickerItem lookup gives up while the picker is still showing the root
collections — the dominant "cannot find menuitem 'QA Postgres12 Orders
Model'" failure.
Give the picker-item lookup a generous timeout so it waits for the path to
finish resolving, and assert visibility before clicking. Also drop the stale
comment referencing a force-click that was already removed.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* GDGT-2532 Migrate background-* tokens to background_page / background_surface
Sweep all consumer references from the legacy `background-*` semantic
tokens to their `background_page-*` / `background_surface-*` counterparts
(added in the foundation PR):
- Page: background-primary/secondary/tertiary (+ -inverse), -highlight
(-> -highlighted), -filter -> background_page-*
- Surface: background-disabled (+ -inverse), -hover, -selected, -brand
(-> -brand-subtle), -success, -warning (+ -secondary -> -strong),
-error (+ -secondary -> -subtle) -> background_surface-*
- Replaces: background-info -> background_page-tertiary;
background-success-secondary -> feedback-positive-selected
SDK public theme keys (background, background-secondary, -hover, -disabled,
-error, -light) are part of the embedder-facing contract and are left
unchanged; the SDK_TO_MAIN_APP_COLORS_MAPPING right-hand sides gain the new
internal names as second targets so SDK/whitelabel overrides reach consumers
that now read the new tokens. Alert.config.tsx's `background-${color}`
synthesis is replaced with an explicit lookup map.
Drive-by fixes (pre-existing, surfaced by the sweep):
- EmbedFrame.module.css: fix `-inverseer` typo (undefined var since #65855).
- Bare `var(--mb-color-background)` was never a real token (rendered
transparent at 8 sites); point each at the correct token by role and
remove the dead `--mb-color-background` definition in LogoBadge that was
silencing find-never-defined-css-variables.
* Use surface background tokens for Switch component
The Switch is a control (a surface on the page), so its track/thumb
backgrounds should use `background_surface-*` rather than `background_page-*`.
Disabled states now use the purpose-built `background_surface-disabled`;
the enabled off-track uses `background_surface-secondary`.
* Remove redundant comment
* Use text-secondary for RowChart axis text
The axis tick + title text was borrowing `background_page-tertiary-inverse`
(a background token) for its color. That token recolors as part of the
background migration, which shifted the axis text. Point it at the proper
`text-secondary` token instead — also consistent with the goal label.
* Update Loki Snapshots
* Formatting
* Update Loki Snapshots
* Update dark-mode body background assertion for recolored page-secondary
The body background uses `background_page-secondary`, which this migration
intentionally recolors in dark mode (color-mix off the page primary). Update
`assertDarkMode` from rgb(5, 14, 21) to rgb(5, 17, 25) (#051119) to match.
Light mode is unchanged.
* Assert dark-mode body background as color(srgb ...) computed value
The body background is a color-mix(in srgb, ...), which browsers serialize
in computed style as color(srgb ...). Assert the exact computed value
instead of an approximated rgb().
* Formatting
---------
Co-authored-by: Metabase Automation <github-automation@metabase.com>
* Migrate SDK execute-action to api.request
* Use updateSetting RTK hook in test render helper
* Remove legacy API client
* Flatten ApiClient request dispatch into a single _send
* Call updateSetting hook inside the test store provider
* Use RTK query for executeAction in embedding SDK bundle
* Fix embedding SDK on-demand chunk loading via runtime publicPath
The backend-hosted embedding SDK bundle shipped with output.publicPath: ""
and never set a runtime publicPath, so any dynamic import()/React.lazy chunk
URL resolved against the host app's origin (where SDK chunks don't exist) and
failed with ChunkLoadError.
Set webpack's runtime publicPath to the Metabase-hosted asset directory
(app/embedding-sdk/) so on-demand chunks load correctly:
- sdk-public-path.ts resolves the asset base and assigns __webpack_public_path__;
it is the first import in the entry so it runs before any import().
- The chunked bootstrap exposes the base on
window.METABASE_EMBEDDING_SDK_ASSET_BASE_URL (the entry runs asynchronously,
so document.currentScript is unreliable there).
- The legacy monolithic bundle has no bootstrap, so the resolver falls back to
document.currentScript.src, which is valid during its synchronous evaluation.
The backend already serves any app/embedding-sdk/chunks/*.js generically, so no
backend change is needed.
* Load jspdf and html2canvas lazily in the embedding SDK bundle
These export-only dependencies (~180KB brotli) were force-imported eagerly in
sdk-specific-imports.ts to work around the broken on-demand chunk loading. Now
that the runtime publicPath is set, drop the eager imports so they load on
demand (PDF / image export) instead of weighing down the SDK's critical path.
* Load pivot table grid lazily in the embedding SDK bundle
* Load map renderer lazily in the embedding SDK bundle
* Count async SDK chunks toward total bundle size now that they are loadable
The "back to the dashboard button when navigating to a question" test
clicked "Edit question" from a dashcard and immediately clicked the
"Back to <dashboard>" button without waiting for the notebook editor to
finish loading. Clicking back before the query builder settles produces
a racy back-navigation that can briefly leave the dashboard without a
mounted app bar, so the later `appBar()` lookup ("Navigation bar")
intermittently fails while the dashboard header text is already visible.
Wait for the notebook editor's "Visualize" button to be visible before
clicking back, mirroring the sibling native-editor test and the
dashboard.cy.spec navigate-to-notebook test.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The "drills through from the conversations by hour/day chart" tests
interact with the timeseries chart immediately after visiting the usage
stats page, which only waits for the audit metadata request. The chart's
dataset query and ECharts render can still be in flight, so
cartesianChartCircle() races the render and intermittently times out
looking for [data-testid="chart-container"].
Anchor clickLastTimeseriesChartDot on assertChartRendered so the chart is
fully rendered (container visible with an svg child) before drilling.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The test clicked a result cell immediately after saving a new model,
racing the table that renders/animates in after the redirect to the
model view (Cypress reported the cell as still animating). Wait for the
model create (POST /api/card) and its query (POST /api/dataset) to
resolve and assert the table is visible before clicking the cell, so the
click no longer depends on animation timing.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The "should duplicate filters and mappings when duplicating a dashcard"
test began interacting with the dashboard parameters (clicking the
"Count" pill) immediately after entering edit mode. Entering edit mode
re-mounts the dashcards and re-runs the card query, so the parameter
header was still re-rendering asynchronously when the click fired,
detaching the target element mid-click ("cy.click() failed because the
page updated while this command was executing").
Gate the first interaction on the dashcard's chart being visible so the
page has settled before editing parameters.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The "unsubscribe from an alert when the user has not created it" test
scoped its post-unsubscribe negative assertion inside the
notifications-list container and never waited for the unsubscribe
request. Since admin was the only recipient and not the creator,
unsubscribing empties the list and the notifications-list container
unmounts into the empty state. When the refetch won the race,
findByTestId("notifications-list") timed out.
Wait deterministically on the unsubscribe POST, anchor on the success
toast, then assert the notifications-list is gone (the real
post-condition) instead of querying inside a container that disappears.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When export! or import! falls back from the incremental fast-path to a
full re-serialize/reload, log which guard forced it so operators can see
why a full sync ran:
- export: forced overwrite, or a pending change can't be applied incrementally
- import: forced, local changes pending, first import, or changes not
incrementally loadable
Bind import dirty? as a delay so the fast-path check and the reason
derivation share one query without forcing it on the no-op/merge paths.
* Add metabase.agent-lib.representations.repair/rewrite-lib-type-aliases*
Rewrite the hallucinated lib/type mbql.join/join to the corrected mbql/join. The LLM was observed making this error in BOT-1657.
Similar to the existing rewrite passes:
- rewrite-operator-name-aliases*
- rewrite-temporal-bucket-aliases*
- rewrite-direction-aliases*
* Fill in a missing lib/type for join clauses
We already did this for stages and the top-level query.
* Validate lib/type for ::join clause in metabase.agent-lib.representations/validate-query
* Mark all deftests ^:parallel in metabase.agent-lib.representations-test
* account for extra padding in bar width
* add loki test
* Update Loki Snapshots
---------
Co-authored-by: Metabase Automation <github-automation@metabase.com>
* restrict permissions for glossary
* disable on UI
* adjust FE code + removed hover effect if the row is readOnly
* rename FieldId to GlossaryField
* address comments
---------
Co-authored-by: Egor Grushin <egor@metabase.com>
Serializing the **import** of a very large data model (~1M+ fields) can OOM. The entire retained-memory cost of an import is the **ingest index**, built up-front before any entity loads (a flat memory plateau through the whole load phase).
Fixes:
1. **Drop the dead structures.** The cache value becomes the `File` directly (`{stripped-path File}`); delete `:from-ids` and its `medley` require, removing the second `ingest-list` pass.
2. **Intern path segments while building the index.** Each `{:model … :id …}` segment and its strings are canonicalized through a shared interner so index keys share their `[db schema table]` prefix structure. Value equality is unchanged, so cache lookups and `:seen`-set comparisons behave identically. Interning goes through a new `metabase.util.memoize/fast-interner` - a thread-safe `ConcurrentHashMap`-backed interner that stores the *canonical* value as the key (so the retained object is the string-pooled copy, not the parsed input).
* Lazy load Echarts renderer
* Load Echarts as the vizualisation loads
* Use correct fallback for echarts renderer
* Fix placeholders for some skeletons
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Eagerly load echarts in sdk bundle
* Reenable brush after chart loads
---------
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
2026-06-23 16:24:56 +10:00
5621 changed files with 542921 additions and 395269 deletions
@@ -22,10 +22,11 @@ Before starting, determine which mode to use:
3.**(Conditional)** If after reading the spec a specific assertion or selector is **ambiguous** — you can't tell if it's the right anchor, or whether the test actually verifies what its title claims — briefly grep the related component for the test ID / role / text. **Don't read components by default.** Only do this when there's a real signal of confusion in the spec.
4. Scan against the checklist + pattern table below.
5. Number all issues sequentially. Skip nits — only flag what's worth fixing.
6.**Always** finish with the honest e2e-vs-unit breakdown (see the section of that name below). It's part of every report, not an optional add-on.
### When to hand off instead of review
If the spec references an issue (`metabase#NNNNN`) and the user wants to **fix** flakiness or assess whether the test still reproduces the original bug, that's outside the review skill's scope. Point them at `/fix-flakey-test` (or the dedicated flake-fixing workflow), which knows to fetch the issue and the resolving PR's diff. The review skill stays focused on "is this test well-written and conformant" — it doesn't fetch external issue context by default.
If the spec references an issue (`metabase#NNNNN`) and the user wants to **fix** flakiness or assess whether the test still reproduces the original bug, that's outside the review skill's scope. Point them at the dedicated flake-fixing workflow, which knows to fetch the issue and the resolving PR's diff. The review skill stays focused on "is this test well-written and conformant" — it doesn't fetch external issue context by default.
## Review checklist
@@ -194,8 +195,41 @@ Use the pending review workflow:
Each comment body starts with `**Issue N: [Brief title]**`.
## Honest e2e-vs-unit breakdown (always include)
Every report ends with this section. The numbered issues above are about whether the test is *well-written*; this section is about whether it should be an **e2e test at all**. They're orthogonal — a flawless spec can still be paying e2e prices for component-test value, and that's worth saying out loud.
The bar is simple: **an `it()` earns its place in e2e only if it exercises a layer you cannot fake without gutting the test's value.** In this codebase that means one or more of:
- a **real backend query against seeded data** (the assertion depends on the server actually filtering/sorting/bucketing rows, not on a mocked response);
- **real rendering you then interact with** — clicking a charted ECharts SVG element at computed coordinates, drag/hit-testing, drill-through;
- **routing across screens** — navigation, URL param round-trips that the backend then honors, browser back/forward;
- **cross-screen traversal** — list → detail → back, where the value is the seam between screens.
Conversely, a test belongs in **Jest + RTL** (the cheaper layer this repo already uses for component/unit coverage) when its real subject is frontend-only:
- **pure frontend logic** — a bucketing/formatting/derivation decision (e.g. "single-day range → bucket by hour") that has one right answer independent of the backend;
- **render assertions** — "the page mounts N titled cards and an SVG exists." A mocked dataset response + RTL verifies the titles; that an ECharts SVG renders is library behavior, not your logic;
- **state/wiring** — tab → `aria-selected` → active-state, or select → querystring mapping. The *URL gets the param* half is component-level; only the *backend honors the param* half needs e2e.
Write the breakdown in three honest buckets. Ground each call in **which layers the test actually touches**, not in how the test is named:
1. **Genuinely must-be-e2e (keep)** — load-bearing. Name the layer(s) only a browser + backend can verify together.
2. **Defensible-but-narrowable e2e** — justified as integration, but over-split or doing more boots than the coverage requires. Say what to merge.
3. **Should be unit / component tests** — where the real savings are. Name the cheaper layer and what it would assert (often *more precisely* than the e2e test does indirectly).
Then a one-paragraph **bottom line**: what to keep as e2e, what to push down, and — if you pulled CI timing from `e2e/support/timings.json` for the Performance check — roughly how much of that wall time is *earned* by the must-be-e2e set versus paid by the rest.
Rules of engagement so this stays honest, not performative:
- **Don't inflate the keep bucket to look balanced.** If most of the spec is component-test value paying e2e prices, say exactly that. If every test is load-bearing, say that too — a spec where all 12 tests must be e2e is a legitimate outcome, not a failure to find candidates.
- **Be specific.** Cite the `it()` titles and line ranges. "Several tests could be unit tests" is useless; "tests 1/17/18 assert only that N titled cards mount — RTL with a mocked dataset" is actionable.
- **This is advice to the author, not a blocking issue.** It's a recommendation, separate from the numbered issues. Don't renumber it into the issue list.
- **Stay rooted in what exists.** The cheaper layer here is Jest + RTL component/unit tests. Don't invent infrastructure the repo doesn't have, and don't recommend deleting backend-dependent coverage just because part of the test is frontend-only — split it, keep the integration half.
## Final check
1. Trim issues that won't materially help the author.
2. Verify sequential numbering with no gaps.
3. In PR mode, verify each issue was posted as a separate review comment.
4. Verify the honest e2e-vs-unit breakdown is present — it's required in every report, even when the answer is "all of these need to be e2e."
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.