Skip to content

docs: Snowflake cancellation, KPI block styling, filter operators, Python-on-dashboards - #11454

Open
keydunov wants to merge 2 commits into
masterfrom
claude/gallant-ramanujan-0euq2c
Open

docs: Snowflake cancellation, KPI block styling, filter operators, Python-on-dashboards#11454
keydunov wants to merge 2 commits into
masterfrom
claude/gallant-ramanujan-0euq2c

Conversation

@keydunov

@keydunov keydunov commented Aug 3, 2026

Copy link
Copy Markdown
Member

Check List

  • Docs have been added / updated if required
  • Tests have been run in packages where changes have been made if available
  • Linter has been run for changed code
  • Tests for the changes have been added if not covered yet

Description of Changes Made

A batch of small documentation gaps found by cross-checking recent shipped changes against docs-mintlify:

  • Snowflake driver query cancellation (feat(snowflake-driver): Support query cancellation #11428): Cube-initiated cancellation (orphaned/timeout or the /v1/running-query/{requestId} endpoint) now cancels the underlying Snowflake statement so the warehouse stops billing for it. Added a short note to the Snowflake data source page.
  • KPI block styling (cubejs-enterprise #13162, #13113): the KPI chart builder shipped per-block Background fill color (Number, Progress bar, Sparkline, HTML blocks), a Neutral comparison color alongside Positive/Negative, and a per-block value-format override for Progress bar and Sparkline blocks. Added the missing rows to the KPI chart-type reference.
  • Dashboard filter is empty / is not empty operators (cubejs-enterprise #13456): first-class value-less operators for string dimensions, distinct from the null checks. Added to the filter widget's operators-by-dimension-type table.
  • Python analysis on dashboards (cubejs-enterprise #13499): the "On dashboards" section was factually stale. Python widgets on dashboards (builder, published, and embedded) now run live per viewer — like SQL widgets — instead of replaying the last saved Run. Corrected the behavior description and its row-level-security implications.

…ouse query

Cube-initiated cancellation (orphaned/timeout or the running-query REST
endpoint) now cancels the underlying Snowflake statement via the driver's
new query-cancellation support (#11428), instead of leaving it running in
the warehouse.
@mintlify

mintlify Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
cubed3 🟢 Ready View Preview Aug 3, 2026, 12:19 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@claude

claude Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Claude finished @keydunov's task in 1m 40s —— View job


Review: docs-only change, accurate — 0 high, 1 medium, 2 low

Docs-only, 10 added lines. The claim is backed by the code. Two inline comments posted (best-effort wording, anchor validity) plus one gap between the PR description and the diff.

Full review

Checklist

  • Gather context (diff, target file, linked docs, driver implementation)
  • Verify the documented behavior against SnowflakeDriver
  • Check link target / anchor and heading placement
  • Check for stale prior Claude review threads (none existed)
  • Post inline comments and summary

Verification

The behavior described is real. SnowflakeDriver.query() returns a CancelablePromise (packages/cubejs-snowflake-driver/src/SnowflakeDriver.ts:524), executeCancelable wires promise.cancel to cancelStatement (:1058), and cancelStatement calls the Snowflake SDK's stmt.cancel() (:1008). The streaming path reuses the same abort (:878, :910), so the doc's blanket "when Cube cancels a query" framing holds.

One note on the PR description rather than the diff: it says the driver cancels "via ALTER SESSION ... CANCEL". The driver calls the Node SDK's statement.cancel(); the exact statement the SDK issues is an SDK detail. The docs text wisely doesn't mention the mechanism — no change needed, just flagging the description.

Findings

Sev Location Issue
Medium snowflake.mdx:231-236 Reads as a hard guarantee that the warehouse stops billing; the implementation is best-effort (no statement issued yet ⇒ nothing cancelled; failed/unacked cancel is logged as a warning after a 30s CANCEL_ACK_TIMEOUT and then swallowed). Suggestion posted inline.
Low snowflake.mdx:238 The #base_path/v1/running-query/requestid anchor is copied from same-page links in reference.mdx; worth confirming in the Mintlify preview that it actually resolves rather than landing at page top.
Low snowflake.mdx:229 ### Query cancellation sits under ## Setup, which is about connecting/configuring rather than runtime behavior. Consistent with the neighboring ### Query tagging, so this is fine as-is — but both arguably belong in their own top-level section.

Gap vs. PR description

The description says the note is "cross-linked from the REST API's running-query cancellation endpoint," but docs-mintlify/reference/core-data-apis/rest-api/reference.mdx is unchanged — the link is one-way. The /v1/running-query/{requestId} section currently implies cancellation is uniform across data sources; a one-line note there that whether the upstream warehouse query actually stops is driver-dependent (with a link back to the Snowflake page) would be the more useful half of the cross-link.

Other checks

Security, performance, and testing sections of the review checklist don't apply — no executable code changed. Frontmatter, reference-style link definitions, and root-relative internal links all follow docs-mintlify/CLAUDE.md conventions, and the change edits an existing page rather than adding a new one, as the conventions prefer. No local lint/test run: docs-only, and the Mintlify preview build covers it.

• branch [`claude/gallant-ramanujan-0euq2c`](https://github.com/cube-js/cube/tree/claude/gallant-ramanujan-0euq2c)

Comment on lines +231 to +236
When Cube cancels a query against Snowflake — for example, an orphaned or
timed-out query, or a request cancelled through the
[`/v1/running-query/{requestId}`][ref-rest-api-cancel] REST (JSON) API
endpoint — the Snowflake driver also cancels the underlying statement, so the
warehouse stops running (and billing for) it instead of continuing in the
background.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cancellation is best-effort in the implementation, but this reads as a guarantee. In SnowflakeDriver.executeCancelable / cancelStatement (packages/cubejs-snowflake-driver/src/SnowflakeDriver.ts:1058, :972):

  • if the cancel is requested before the statement was issued, nothing is sent to the warehouse (the driver just rejects locally) — nothing to stop;
  • the cancel ack is bounded by CANCEL_ACK_TIMEOUT (30s) and an SDK error/timeout is only logged as a warning, then resolved — Cube does not retry or surface a failure.

Suggest softening slightly so users don't read it as a hard billing guarantee:

Suggested change
When Cube cancels a query against Snowflake — for example, an orphaned or
timed-out query, or a request cancelled through the
[`/v1/running-query/{requestId}`][ref-rest-api-cancel] REST (JSON) API
endpoint — the Snowflake driver also cancels the underlying statement, so the
warehouse stops running (and billing for) it instead of continuing in the
background.
When Cube cancels a query against Snowflake — for example, an orphaned or
timed-out query, or a request cancelled through the
[`/v1/running-query/{requestId}`][ref-rest-api-cancel] REST (JSON) API
endpoint — the Snowflake driver also asks Snowflake to cancel the underlying
statement, so the warehouse stops running (and billing for) it instead of
continuing in the background. Cancellation is best-effort: if the statement
hasn't been issued yet there's nothing to cancel, and a failed or unacknowledged
cancel is logged as a warning.

warehouse stops running (and billing for) it instead of continuing in the
background.

[ref-rest-api-cancel]: /reference/core-data-apis/rest-api/reference#base_path/v1/running-query/requestid

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Worth confirming this anchor resolves in the Mintlify preview. The target heading is ## `{base_path}/v1/running-query/{requestId}` (docs-mintlify/reference/core-data-apis/rest-api/reference.mdx:470), and the #base_path/v1/running-query/requestid form is copied from the same-page links in that file (lines 29, 420) — so if those in-page anchors are already stale, this cross-page link inherits the bug and silently lands at the top of the reference page instead of the section.

…erators, and correct Python-on-dashboards behavior

- KPI blocks: document the Background fill color (Number, Progress bar,
  Sparkline, HTML), the Neutral comparison color, and the Sparkline/Progress
  bar value-format override — all shipped in cubejs-enterprise#13162 and
  cubejs-enterprise#13113 but undocumented.
- Dashboard filter widgets: add the is empty / is not empty string operators
  (cubejs-enterprise#13456), missing from the operators-by-dimension-type table.
- Python analysis: the 'On dashboards' section was factually stale —
  cubejs-enterprise#13499 made dashboard Python widgets run live per viewer
  (like SQL widgets) instead of replaying the last saved Run. Corrected the
  behavior description and the row-level-security implications.
@keydunov keydunov changed the title docs: document Snowflake driver query cancellation now stops the warehouse query docs: Snowflake cancellation, KPI block styling, filter operators, Python-on-dashboards Aug 3, 2026
@claude

claude Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Claude finished @keydunov's task in 2m 2s —— View job


Review: docs-only, 4 files — 0 high, 1 medium, 3 low

Commit 4ecbdce broadened this PR well past its title: three Cloud UI docs pages (KPI blocks, control filter operators, Python-on-dashboards) now ride along with the Snowflake note. The one finding that matters is in python-analysis.mdx — the new dashboard behavior contradicts a security <Warning> that was left unchanged. 3 new inline comments; the 2 prior Snowflake threads still apply.

Full review

Checklist

  • Gather full diff (4 files now)
  • Verify KPI per-block styling claim
  • Verify is empty / is not empty filter operators claim
  • Verify Python-on-dashboards behavior claim
  • Re-check Snowflake note against prior threads
  • Resolve stale threads / dedupe, post findings

Verifiability

The Snowflake claim is backed by code in this repo and I confirmed it last round (SnowflakeDriver.query()CancelablePromise, executeCancelable wiring cancelcancelStatement → SDK stmt.cancel()).

The three new pages document Cube Cloud UI surfaces whose source is not in this repo — no KPI block, dashboard control, or Python-sandbox code exists under packages/ (grep for sparkline returns nothing). So the substantive claims — Background as a per-block setting, a Neutral comparison color, a sparkline Format override, is empty/is not empty operators, and dashboards running Python live per viewer — cannot be verified from this checkout. They need a maintainer who can confirm against the Cloud build; my review below is limited to internal consistency.

Findings

Sev Location Issue
Medium python-analysis.mdx:150-152 The dashboard RLS exception sits after the <Warning> it contradicts. The callout still says unconditionally that Python runs as the person who pressed Run, that the result is persisted on the report, and that RLS is applied at run time — none of which now holds for dashboard widgets. A skimmed callout is a security misunderstanding. Suggestion posted inline.
Low controls.mdx:21 is empty / is not empty land next to is null / is not null with no explanation of the distinction that justifies having both. Also reference/data-modeling/view.mdx:372 lists filter-bar operator labels and omits the two new ones — if they exist in the workbook filter bar too, that list is now incomplete.
Low kpi.mdx:99-102 **Background** is repeated across four block tables, and the new HTML table exists solely to hold that row — while Text still has no table, leaving it ambiguous whether Text lacks Background or was missed. One sentence under ## Block types covers it.
Low python-analysis.mdx:151-152, 156-163 "cached per viewer identity and isn't persisted back to the report" leaves the Limits table ambiguous: it's framed in terms of persistence ("Maximum output rows persisted", "cannot be saved to a report"), so it's unclear whether the 10,000-row / 2 MB caps still bound a live dashboard run. Worth one clarifying clause.

Also on python-analysis.mdx: line 28 still says a saved-from-chat report "renders the saved output without re-running." That's correct for a workbook tab, so it reads fine in place — noting it only so it's a deliberate keep rather than an oversight.

Prior threads

Both earlier Snowflake threads are still unresolved and still apply — snowflake.mdx is byte-identical to what I reviewed, so neither was resolved and no duplicate was posted:

  • best-effort wording vs. what reads as a hard billing guarantee (snowflake.mdx:236)
  • #base_path/v1/running-query/requestid anchor validity (snowflake.mdx:238)

The one-way cross-link gap I raised last round also stands: reference/core-data-apis/rest-api/reference.mdx is still unchanged, so nothing points from the cancellation endpoint back to the driver-dependent caveat.

Other checks

Security / performance / testing sections of the review checklist don't apply — no executable code changed. Frontmatter, root-relative internal links, and Mintlify component usage follow docs-mintlify/CLAUDE.md; all four changes edit existing pages rather than adding new ones. No local lint or test run: docs-only, and the Mintlify preview build covers rendering.

Meta

The commit message and PR title/description cover only the Snowflake change. If the three Cloud UI pages are intentional, the PR description should name them so a reviewer knows to verify those claims; if they were picked up accidentally, they belong in a separate PR.

• branch [`claude/gallant-ramanujan-0euq2c`](https://github.com/cube-js/cube/tree/claude/gallant-ramanujan-0euq2c)

Comment on lines +150 to +152
On dashboards, this changes: each viewer's live run applies row-level security for
**their own** identity, the same as a SQL widget — it isn't the Run author's access
persisted for everyone who views the dashboard.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The dashboard exception is placed after the <Warning> that it contradicts, which is the one spot in this page where a skimmed read is a security misunderstanding.

The callout above (lines 128–144) still asserts unconditionally that Python "runs with the security context of the person who pressed Run", that the result "is then persisted on the report", and that "Row-level security is applied at run time, not at view time." With this PR, none of that holds for dashboard widgets — and the callout is exactly the block a reader is most likely to read instead of the surrounding prose, not in addition to it.

Suggest scoping the warning to workbooks and pointing at the dashboard case from inside it, e.g.:

Suggested change
On dashboards, this changes: each viewer's live run applies row-level security for
**their own** identity, the same as a SQL widget — it isn't the Run author's access
persisted for everyone who views the dashboard.
On dashboards, this changes: each viewer's live run applies row-level security for
**their own** identity, the same as a SQL widget — it isn't the Run author's access
persisted for everyone who views the dashboard.

…plus a one-line addition inside the <Warning> itself (outside this suggestion's range), such as "This describes Python in a workbook tab. On dashboards the widget runs live per viewer — see below." Fix this →

| Dimension type | Operators |
|---|---|
| **String** | `is`, `is not`, `contains`, `not contains`, `starts with`, `not starts with`, `ends with`, `not ends with`, `is null`, `is not null` |
| **String** | `is`, `is not`, `contains`, `not contains`, `starts with`, `not starts with`, `ends with`, `not ends with`, `is empty`, `is not empty`, `is null`, `is not null` |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two follow-ups on adding is empty / is not empty next to is null / is not null:

  1. The row now offers four operators whose difference (empty string vs. SQL NULL) is the whole reason both pairs exist, but the page doesn't say which is which. One clause under the table — e.g. "is empty matches empty strings; is null matches missing values" — saves a support round-trip.
  2. reference/data-modeling/view.mdx:372 lists the operator labels for filter_bar and prefaces them with "what you see in the filter bar is what you type" — that list omits is empty / is not empty. If these labels also exist in the workbook filter bar (and not only in dashboard control widgets), that reference list is now incomplete and should get the same two entries.

Comment on lines +99 to +102
| Setting | Description |
|---|---|
| **Background** | Fill color for the block |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

**Background** | Fill color for the block is now repeated in four of the six block tables, and this HTML table exists only to carry that one row. The Text block (line 89) still has no settings table at all — so a reader can't tell whether Text has no Background or whether it was just missed.

If Background is universal across block types, a single line under ## Block types ("Every block type has a Background setting that sets the block's fill color.") is both shorter and answers the Text question. If Text genuinely lacks it, worth saying so explicitly. Fix this →

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants