Skip to content

docs: every filter condition can source a dashboard facet (CUB-4769) - #11936

Open
mngr wants to merge 11 commits into
masterfrom
docs/cub-4769
Open

mngr wants to merge 11 commits into
masterfrom
docs/cub-4769

Conversation

@mngr

@mngr mngr commented Sep 19, 2026

Copy link
Copy Markdown
Collaborator

Rebased onto master now that #11938 (CUB-2842) has merged — that PR rewrote this same Faceted filters section, so this branch is no longer a parallel rewrite of it but a delta on top of its merged wording, and the diff is down to four paragraphs. It must not merge before its own feature PR, cubedevinc/cubejs-enterprise#15177.

Re-based again onto today's master, which additionally carries #11956 (CUB-2842's follow-up). That PR appended a paragraph after the section's <Warning>, so this branch's pointer at it — "the warning closing this section" — no longer described the page and now reads "the warning below". No other line needed re-stating: the feature PR has gained no product change since this page was written (git log --format='%ai %h %s' origin/master..HEAD on the branch lists one commit after this page's last edit, c5f73077a4, which only rewords a comment in playwright/pages/workbook.page.ts).

Re-based once more onto today's master, which gained #11960 — that PR touches widgets/index.mdx and widgets/layout.mdx only, so no line on this page needed re-stating. The push itself answers r4075433005: the case-sensitivity caveat rested on a URL key the page never named, leaving a reader whose value list stopped narrowing with no way to recognise the flag in their own link or take it out. The caveat now names caseSensitive, and the filter-JSON reference it links to documents that key beside startInclusive / endInclusive.

This push answers r4075488788, which found the other two "only a hand-written URL parameter can" claims pointing at a reference that carried neither key: the custom-SQL bullet's link and the <Warning>'s several-values-on-a-substring case. Of the reviewer's two remedies it takes the first — give the URL section the missing keys — because the second would have retracted half of the single fact the previous round (r4075382469) asked line 104 to state. The filter-JSON reference under Sharing the current selection now carries a custom row in its type table, and a clause placed after the caseSensitive paragraph: outside between, a value may still be a list — is and is not read its entries as alternatives, a substring condition matches them joined into a single term. The Faceted filters section itself is unchanged by this push.

This push answers the round's two findings, both on the filter-JSON reference this branch added, and it changes two lines. r4075550624 (medium) objected that the custom row published an arbitrary-SQL surface and asked either for the gate to be named or for the row to be narrowed. I checked the enterprise source rather than answering from the row: there is no gate — url-filter-params.ts:208-233 lists 'custom' in validTypes and applies no parse, permission check or allow-list to its value, and semantic-sql-generator.ts:320-321 is case 'custom': return filterItem.value as string;, the columnRef above it discarded. So the row's claim held, which is exactly why it could not stay. The row now reads `custom` — a facet source set to it scopes nothing and describes the value not at all; that is the reviewer's own narrower option, and it keeps the type string named where the faceting bullet at line 104 sends the reader, which is what r4075382469 asked for. The ungated surface itself is neither new nor this PR's: both lines predate the feature branch by about a year (#10133, #9215) and cubedevinc/cubejs-enterprise#15177 touches neither file, so it is filed as CUB-4950 and linked from that PR's description rather than held against this page.

r4075553248 (low-medium) found the caseSensitive paragraph claiming something about the warehouse rather than about Cube — "with true it matches the case of the value exactly", where the key's whole effect (semantic-sql-generator.ts:197-198) is LIKE instead of ILIKE, which a MySQL _ci or a case-insensitive Snowflake collation still matches case-insensitively. The sentence now qualifies the guarantee where it makes it: with true the match is made with LIKE rather than ILIKE, so it respects case wherever the column's collation is case-sensitive, and without the key case is ignored. The <Warning> bullet at line 129 is deliberately unchanged — the product's behaviour there is not collation-dependent, since substringBuilder drops a case-sensitive negation unconditionally, Cube's notContains family having no case-sensitive form at all.

This push answers the round's two remaining findings, both medium, and it changes two lines. r4075614348 found the new list-value paragraph naming is / is not and the substring conditions only, reading as a closed set that denies what the <Warning> bullet at line 127 rests on, and never naming the separator the joined term uses. Probing generateSemanticSql (a throwaway vitest, one filter per case) showed the omission was the opposite of an omission: {"type":"contains","value":["ship","proc"]} emits ILIKE '%ship,proc%' — comma, no space — {"type":"not_equals","value":[...]} emits NOT IN (...), and a list on in_month / greater_than throws value.replace is not a function, because those branches hand filterItem.value to formatFilterValue, which handles a string or a number only. The paragraph now names all three behaviours, the separator and the example pattern.

r4075616013 asked for two more bullets in the "nothing to scope by" <Warning> — a non-scalar numeric bound and between on a Number dimension — or for the list to be declared illustrative. Neither belongs, and both were checked in the source rather than taken from this description: NUMBER_BUILDERS carries between: numberBetweenBuilder, which mirrors the chart's BETWEEN 10 AND 20 as a gte/lte pair with inclusivity honoured, so it is an acceptance item of the feature PR rather than a drop case; and a non-scalar numeric bound never reaches the state the <Warning> is about, because the charts do not narrow either — the same value.replace throw. So the third form makes the list's membership rule explicit instead: it is the cases "where the charts match on something the value list has no way to ask for", which every bullet is an instance of and neither numeric case is. The two crashes are a pre-existing product defect — semantic-sql-generator.ts predates this branch by a year and neither PR touches it — filed as CUB-4951; the page documents neither list as a thing to write.

This push answers the round's medium finding, r4075722498, in cce556edb, and it changes two lines. The list-value paragraph at line 426 and the <Warning> bullet at line 127 disagreed about what f_orders.created_at={"type":"in_month","value":["2026-01","2026-02"]} does. The paragraph is the line that matches the code, re-probed at this head rather than taken from the earlier round's note — a throwaway vitest over generateSemanticSql has in_month and greater_than with a list both throw value.replace is not a function, so no query is built, while not_equals with a list emits orders.status NOT IN ('a', 'b'). The in the … / not in the … family was therefore never a "nothing to scope by" case at all: that list is the cases where the charts narrow and the value list cannot follow, and here the charts do not narrow either. Rather than revert either line, the bullet now states the third form — it keeps is not on several dates, which is the leg the code really does put in this state (NOT IN in the chart, null from timeEqualityBuilder, which drops a negated list), and says in the same breath why the in the … family is out of the list. The Time row of the selection table at line 45 leaned the same way, offering a hand-written URL parameter as the route to several values on every time condition; it now says is / is not only.

This push answers the round's medium finding, r4075788212, in 90721e86a, and it changes one line. The previous push put the in the … / not in the … list case into the <Warning> as a parenthetical on the is not bullet, explaining there why that family is absent from the list — but the explanation itself fails the membership rule the same push added one line above ("cases where the charts match on something the value list has no way to ask for", over a premise of a value list left wide while the charts narrow), since in that case the charts build no query at all. Re-checked at this head rather than taken from the earlier round: the in_month / not_in_month / in_quarter / … branch of semantic-sql-generator.ts hands filterItem.value to formatFilterValue, whose only string path is value.replace(/'/g, "''") — an array throws, so nothing narrows either way. The parenthetical is gone; the fact stays where it holds without a claim about the charts, in the list-value paragraph of the filter-JSON reference ("Every other condition reads exactly one value — the comparisons, and the in the … and not in the … family — and a list in their value leaves the widget unable to build its query at all"), which is the reviewer's own second remedy. That is not a revert of r4075722498: the claim it objected to — those conditions reading one period from one value while the charts narrow — left the bullet in that push and has not come back, and r4075614348's ask that the reference name all three families is untouched. The <Warning> now lists only cases where the charts do narrow.

The round's other finding, r4075723616 on the custom row, is low — replied to and resolved without a page change, since only medium- and high-severity findings are fixed on this docs PR.

Faceted dashboard filters used to narrow a value list from is / is not and the date conditions alone — every other condition was greyed out in the source picker, and the product tooltip said so. CUB-4769 (stacked on CUB-2842's date conditions) makes every condition a dashboard filter can be set to a usable facet source, so the page has to stop naming the ones that no longer fail.

Changes, all in docs/explore-analyze/dashboards/widgets/controls.mdx, inside Faceted filters:

  • The source-condition list and the "every other condition scopes nothing" paragraph are replaced by one lead sentence — a source can be set to any condition — followed by the three rules that qualify it, one bullet each: the four valueless conditions (is null, is not null, is empty, is not empty) narrow the moment they are set while every other condition leaves the list alone until it has a value (the same rule that leaves the charts unfiltered); a condition and value that together leave nothing to scope by narrow nothing, as the section's <Warning> sets out; and a source set to custom SQL scopes nothing — no operator menu offers that condition, so only a hand-written URL parameter can set a filter to it.

  • The source picker now greys out only a filter on another semantic view, since the condition-based reason is gone; the selected-but-no-longer-usable case (its dimension re-pointed at another view) is named, because that one is flagged rather than dropped.

  • The "nothing to scope by" <Warning> gains the two cases this change introduces, each qualified where it is made by how it is reached: several values on a substring condition, and a not contains / not starts with / not ends with carrying "caseSensitive": true — both settable only through a hand-written URL parameter, both leaving the list alone rather than scoping it differently from the charts. The caseSensitive key is documented where that caveat sends the reader: the filter-JSON reference under Sharing the current selection now carries it beside the between paragraph's startInclusive / endInclusive, with the example f_orders.status={"type":"not_contains","value":"ship","caseSensitive":true} and the note that the value editor has no switch for it.

  • The date-source window table's lead-in places is null / is not null. Making every condition a source makes the presence pair readable on a time dimension too, and the table is keyed on "which window" — neither names one, so the lead-in now says they scope by presence instead, to the rows with no date and to the rows with one. The table itself gains no row: presenceBuilder emits a valueless notSet / set UnaryFilter, so a "window" cell for either would have to read "none".

Everything else CUB-2842 merged and this change does not contradict is left exactly as it stands — the Operators by dimension type table (which already carries is empty / is not empty, the Boolean row and the time labels), the window table's own rows, the whole-day bound paragraph and the is not-on-a-single-moment warning bullets.

What I verified against the code

  • Every condition sources a facet: FACET_FILTER_BUILDERS in packages/console-ui/src/modules/d3/components/Workbook/DashboardBuilder/utils/facet-scope-utils.ts covers every (member data type, condition) pair the operator picker can produce — string, number, time, boolean — and facet-scope-utils.spec.ts asserts that exhaustively. isFacetingAvailable is just a lookup in that table.
  • The four valueless conditions narrow as soon as they are set: presenceBuilder (is_nullnotSet, is_not_nullset, on every member type including the unresolved-type fallback) and emptyStringBuilder (is_emptyequals [''], is_not_emptynotEquals [''], string only) take no item.value, and they pass generateCubeFilter's isFilterComplete gate — the same gate that drops a source still waiting on a value, matching what the chart's SQL does.
  • Custom SQL is not in any operator menu: DASHBOARD_FILTER_TYPES_BY_MEMBER_TYPE (utils/dashboard-filter-operators.ts) offers no custom for any member type, while url-filter-params.ts's validTypes accepts it — so a URL parameter is the only way to set one, and FACET_FILTER_BUILDERS has no custom cell for any type.
  • Greying and flagging: isFilterAvailableForFacet is same semantic view && isFacetingAvailable, so with the table full the only reason a pickable filter greys out is the view; FilterEditSidebar.tsx renders the IconAlertTriangle rightIcon (and a danger theme on the picker) for an item that is selected and no longer available, rather than removing it.
  • Several values on a substring condition are URL-only: convertFilterValue (WorkbookReportFilters/filter-utils.tsx) collapses the value to extractFirstValue when the operator changes to a CONTAINS_FILTERS one, and the page's own selection table already says those conditions hold one value. The chart interpolates %${filterItem.value}% with no array branch (semantic-sql-generator.ts), i.e. the values joined into one term; substringBuilder returns null for an array rather than emitting a Cube OR.
  • Case-sensitivity is URL-only too: nothing in the filter widget writes caseSensitive: true (url-filter-params.ts reads it off the parameter; semantic-sql-engine.ts sets it only when parsing a report's own SQL), the chart's negations become NOT LIKE when it is set (semantic-sql-generator.ts:198), and Cube's notContains family is ILIKE-only — so substringBuilder drops a case-sensitive negation instead of scoping tighter than the chart.
  • The caseSensitive key as documented: declared on the URL filter shape beside startInclusive / endInclusive (packages/console-ui/src/modules/d3/utils/url-filter-params.ts:32-34), parsed in (:168) and serialized back out (:316, :400); semantic-sql-generator.ts:197-198 picks LIKE / NOT LIKE only when it is true, so a filter without the key is case-insensitive, and the operator is the only place it applies (equals uses = / IN). The claim that the editor cannot set it: the Case Sensitive switch in ContainsFilterControl.tsx:138 is commented out (// TODO: restore Case Sensitive switch once backend supports it), so the popover renders the value input and Apply alone.
  • The custom type as documented, and what the page deliberately stops short of: custom is in validTypes (packages/console-ui/src/modules/d3/utils/url-filter-params.ts:208-233), so the parameter parses, and it is not in VALUE_LESS_FILTER_TYPES, so validateUrlFilter requires its value; semantic-sql-generator.ts:320-321 is case 'custom': return filterItem.value as string, i.e. the value becomes the WHERE predicate verbatim, with no parse and no permission check anywhere on that path. The row therefore names the type and what a facet source set to it does, and says nothing about the value — see the push note above and CUB-4950.
  • The list value as documented, probed rather than read: the array branches in semantic-sql-generator.ts:199-232 belong to equals and not_equals alone (IN / NOT IN, on a time dimension as much as a string or number one); the substring cases at :258-270 interpolate %${filterItem.value}% with no array branch, and a run of generateSemanticSql over {"type":"contains","value":["ship","proc"]} emits orders_view.status ILIKE '%ship,proc%' — the array stringified by the template literal, so comma with no space. Every other condition passes the value to formatFilterValue / formatComparisonValue, which take a string or a number: {"type":"in_month","value":["2026-01-01","2026-02-01"]} and {"type":"greater_than","value":[10,20]} both throw value.replace is not a function, so no query is built at all. between on a Number member is the one array-taking condition outside those: orders_view.amount BETWEEN 10 AND 20 in the chart, { and: [gte, lte] } from numberBetweenBuilder in the facet.
  • In the app: the Facet block, its tooltip and the source picker were exercised on the deployed branch at staging-mngr-6 by the CUB-4769 acceptance walk (12/12 items, 3 passed).

Feature PR: https://github.com/cubedevinc/cubejs-enterprise/pull/15177
Linear: https://linear.app/cube-d3/issue/CUB-4769/facet-sources-numeric-comparisons-containsstartswithendswith-is-nullis

@mngr
mngr requested a review from keydunov as a code owner September 19, 2026 00:12
@mintlify

mintlify Bot commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

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

Project Status Preview Updated
cubed3 🟢 Ready View Preview Sep 22, 2026, 7:52 PM

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

@github-actions github-actions Bot added docs Issues that require a documentation improvement pr:community Contribution from Cube.js community members. labels Sep 19, 2026
@claude

claude Bot commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

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


Docs review: accurate on the new behaviour, but the section lead still states the old rule — 1 medium, 3 low

Review details
  • Read the diff
  • Read the full page for consistency
  • Check claims for internal consistency
  • Resolve/dedupe prior review threads
  • Post findings

Scope

packages/console-ui isn't in this repo (the feature lives in cubejs-enterprise), so the code claims in the PR description couldn't be re-verified here. What I could check is internal consistency, and the operator counts line up with the description exactly: String 12, Number 8, Time 15, Boolean 1 — matching the FACET_FILTER_BUILDERS per-type counts quoted there.

Findings

# Severity Where Concern
1 Medium controls.mdx:82, :84 (surfaced on :101) The section lead — "you can mark them as faceted", "marking both as faceted" — reads as mutual opt-in, while :101 says a faceted filter is narrowed by every other filter on its view whether or not that one is faceted. Only the narrowed filter needs Enable facet. The PR set out to stop the page implying the old limit and left the first two paragraphs stating it.
2 Low controls.mdx:28 The Boolean row puts two sentences of prose in the Operators column; every other row is a comma-separated list of code-formatted operators. Suggestion posted to move the operator-picker note under the table.
3 Low controls.mdx:86 vs :101 The toggle is Enable facet in one sentence and the gear sits "beside Facet" in the next. One label is wrong, and a reader hunting the UI has two strings to try.
4 Low controls.mdx:90103 Against docs-mintlify/CLAUDE.md's concision rule: :90 says "every condition" and then re-enumerates them all; :94's "A source is read on the same rule the charts read it by" is oblique where "an unfilled filter narrows nothing, just as it doesn't filter the charts" is direct; :103's "rather than quietly dropping out of the arrangement" is padding.

None of these are correctness problems with the new behaviour the PR documents — finding 1 is the only one that leaves the page self-contradictory.

Review threads

The unresolved-thread listing returned 0 threads of mine (no prior threads). Nothing resolved, nothing re-affirmed, no findings skipped as duplicates. Four inline comments posted this round.

· branch [`docs/cub-4769`](https://github.com/cube-js/cube/tree/docs/cub-4769)

Comment thread docs-mintlify/docs/explore-analyze/dashboards/widgets/controls.mdx Outdated
Comment thread docs-mintlify/docs/explore-analyze/dashboards/widgets/controls.mdx Outdated
Comment thread docs-mintlify/docs/explore-analyze/dashboards/widgets/controls.mdx Outdated
Comment thread docs-mintlify/docs/explore-analyze/dashboards/widgets/controls.mdx Outdated
@claude

claude Bot commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

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


Docs-only change, no blockers — 1 medium, 4 low, all posted inline. The operator table and the facet rewrite hang together; the medium is an internal contradiction over valueless operators.

Full review
  • Read the PR diff and the full changed page
  • Check claims against the source
  • Check cross-page consistency (anchors, related docs)
  • Resolve stale review threads / dedupe findings
  • Post findings and final summary

Scope of verification

packages/console-ui is not in this repo — DASHBOARD_FILTER_TYPES_BY_MEMBER_TYPE, FACET_FILTER_BUILDERS, isFilterAvailableForFacet, handleToggleFacetSettings and RELATIVE_PRESETS all live in cubejs-enterprise, so none of the behavioral claims could be checked against code here. The PR body documents each of them against a named symbol, which is the right level of evidence; this review covers internal consistency, anchors and wording only.

Anchor check: #operators-by-dimension-type (line 92) resolves to the ### Operators by dimension type heading. No other page links #faceted-filters, so the section rewrite breaks no inbound links. The only other facet hit in the docs is an MDX comment in small-multiples.mdx — chart faceting, not published prose, so no term collision to resolve.

Findings

Site Concern
Medium controls.mdx:9296 "Every condition narrows" vs "a filter with no value contributes nothing" contradict for is null / is not null / is empty / is not empty, which never take a value. Suggestion posted scoping the no-value rule to conditions that take one.
Low controls.mdx:103 "Closing the gear drops the picks" also applies to an already-saved source list — silent widening of what viewers see, stated as if it only concerned a fresh selection.
Low controls.mdx:90 "source filter" / "source picker" used from this heading onward; source is never defined, and 84–88 talk only about narrowing.
Low controls.mdx:99 "narrows by whole days" doesn't say the rounding is outward; the guarantee in the next sentence implies it, but the rule itself is ambiguous.
Low controls.mdx:30 Relative-value examples mix bold (Today) and backticks (3 months ago) in one list, implying a distinction the sentence doesn't make.

Dropping relative date from the Time row is the right call — it was never an operator — and the new sentence covering it is the correct replacement. The Boolean row plus the "no operator picker" sentence is a small say-it-twice against docs-mintlify/CLAUDE.md, but the sentence carries the true/false value and the absent picker, so it earns its place.

Review threads

The listing returned 0 of my own threads (single page, hasNextPage: false). Nothing resolved, nothing re-affirmed — no prior threads on this PR.

· branch [`docs/cub-4769`](https://github.com/cube-js/cube/tree/docs/cub-4769)

Comment thread docs-mintlify/docs/explore-analyze/dashboards/widgets/controls.mdx Outdated
Comment thread docs-mintlify/docs/explore-analyze/dashboards/widgets/controls.mdx Outdated
Comment thread docs-mintlify/docs/explore-analyze/dashboards/widgets/controls.mdx Outdated
Comment thread docs-mintlify/docs/explore-analyze/dashboards/widgets/controls.mdx Outdated
Comment thread docs-mintlify/docs/explore-analyze/dashboards/widgets/controls.mdx Outdated
@claude

claude Bot commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

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


Docs review — 3 low-severity findings, all posted inline. No blockers.

Review detail
  • Read the changed file in full
  • Verify claims against available source
  • Check anchors, links, Mintlify conventions
  • Handle prior review threads
  • Post findings

Findings

# Location Concern
1 controls.mdx:103 A saved source list being discarded on closing the gear is the section's most consequential behaviour and sits in a nested subordinate clause. Suggested splitting it into its own sentence.
2 controls.mdx:97 "the source picker below" reads as a UI position, not a page position. Suggested linking #choosing-which-filters-narrow-this-one, as every other forward reference on the page does.
3 controls.mdx:94 "Two cases contribute nothing" is stated as closed, but the PR description mentions time builders that "return null rather than approximate" — from the reader's side a third case of a set, same-view source that narrows nothing. Asked which of two readings holds.

What I could and couldn't verify

The behavioural claims are all about console-ui, which lives in cubedevinc/cubejs-enterprise, not this repo (packages/ has no console-ui). So dashboard-filter-operators.ts, facet-scope-utils.ts and FilterEditSidebar.tsx were unavailable to me — I took the PR description's code citations at face value and reviewed the prose on its own terms. Finding 3 is the one place where that gap produced a question instead of a verdict.

What I did check here:

  • #operators-by-dimension-type resolves — ### Operators by dimension type is on line 19.
  • "greyed out" matches the repo's spelling; google-sheets.mdx:99, dbt.mdx:894 and funnel.mdx:32 all use "greyed". No change wanted.
  • No other page in docs-mintlify describes faceted dashboard filters, so nothing else went stale with this change. (color-and-stacking.mdx "Color faceting" and small-multiples.mdx are a different feature.)
  • Heading depth, table syntax and internal-link form follow docs-mintlify/CLAUDE.md. The prose is denser than that file's "be concise" guidance would suggest in isolation, but it matches the register of the surrounding parent-control sections on the same page, so I'm not raising it.
  • Boolean row reading just is while line 32 says there is no operator picker is the resolution from the earlier round, and it reads correctly now.

Review threads

The listing returned 0 unresolved threads authored by me — no prior threads, so none resolved and none re-affirmed. The four mngr review comments visible on the PR are human-authored and outside my remit either way.

· branch [`docs/cub-4769`](https://github.com/cube-js/cube/tree/docs/cub-4769)

Comment thread docs-mintlify/docs/explore-analyze/dashboards/widgets/controls.mdx Outdated
Comment thread docs-mintlify/docs/explore-analyze/dashboards/widgets/controls.mdx Outdated
Comment thread docs-mintlify/docs/explore-analyze/dashboards/widgets/controls.mdx Outdated
@claude

claude Bot commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

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


Docs change is sound — 3 low findings, all editorial (no blockers)

Review details
  • Read the PR diff and the full changed file
  • Verify claims against available source
  • Reconcile prior review threads
  • Post findings

Scope of verification

The behaviour this page describes lives in packages/console-ui in cubedevinc/cubejs-enterprise, which is not part of this checkout — dashboard-filter-operators.ts, facet-scope-utils.ts and FilterEditSidebar.tsx are all unavailable here, so I could not independently re-verify the operator lists, VALUE_LESS_FILTER_TYPES, the upperDayBound rounding or the handleToggleFacetSettings behaviour. The PR body and the author's own review replies cite file and line for each of those; I reviewed the prose on internal consistency, cross-page consistency and repo docs conventions instead, and say so rather than implying a code check I didn't run.

What I did check:

  • Internal consistency of the operator table. String 12, Number 8, Time 15, Boolean 1 — matches the counts the PR body claims from FACET_FILTER_BUILDERS. relative date is gone from the Time row and re-explained as a value mode at controls.mdx:30, which is the right split.
  • Cross-page. No other .mdx mentions faceted dashboard filters or carries a stale copy of the operator table; the only other relative date hits are REST query-format and pre-aggregation pages, unrelated. Nothing else to keep in sync.
  • Anchors. #operators-by-dimension-type and #choosing-which-filters-narrow-this-one both resolve to headings on the page; the new #### nesting under ### Faceted filters is valid.

Findings

# Site Finding
1 controls.mdx:92 "reads every condition its sources can be set to" is stated flat and contradicted by the is not-on-time bullet six lines down
2 controls.mdx:98 "rather than a period such as 3 months ago" attaches the example to period, filing 3 months ago on the wrong side of the distinction the bullet is drawing
3 controls.mdx:106 the gear-close data loss — a saved source list dropped, All published to every viewer — is body prose, where the page uses <Warning> for the structurally identical rename trap at controls.mdx:331

All three posted inline with suggestions. None block the merge; 1 and 2 are the two places a reader can come away with a wrong rule, 3 is a convention point from docs-mintlify/CLAUDE.md.

The gear behaviour itself is a product wart, and recording it for a follow-up rather than fixing it in a docs PR is the right call.

Review threads

The listing returned 0 unresolved threads on this PR, so none of mine to reconcile: 0 resolved, 0 re-affirmed. The author's own earlier review comments were all self-resolved in commits 852f141, 9377b3b and f50b4fc.

· branch `docs/cub-4769`

Comment thread docs-mintlify/docs/explore-analyze/dashboards/widgets/controls.mdx Outdated
Comment thread docs-mintlify/docs/explore-analyze/dashboards/widgets/controls.mdx Outdated
Comment thread docs-mintlify/docs/explore-analyze/dashboards/widgets/controls.mdx Outdated
@claude

claude Bot commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

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


Docs review: no blockers — 4 low findings posted inline, 1 general observation

Review details

Todo

  • Read the PR diff
  • Read the full page for coherence
  • Check anchors, cross-references and Mintlify component usage
  • Resolve stale review threads / de-duplicate findings
  • Post findings

What I could and could not verify

The behaviour this page describes lives in console-ui, which is not in this repo (packages/ has no console-ui; the feature PR is cubedevinc/cubejs-enterprise#15177). So none of the factual claims — the operator labels per dimension type, FACET_FILTER_BUILDERS coverage, VALUE_LESS_FILTER_TYPES, the whole-day rounding, the gear's clearing behaviour — could be checked against source from here. The PR description cites each of them with file and line, and the review replies re-derive several; I'm taking those on trust rather than confirming them. Everything below is about the prose, the structure, and the page's internal consistency.

Anchors check out: #operators-by-dimension-type and #choosing-which-filters-narrow-this-one both match headings on the page, and the <Warning> follows the same no-blank-line form as the other callouts in the file (controls.mdx:159, :333).

Findings

# Location Concern
1 controls.mdx:92 "the last of the three cases below" is an ordinal forward reference — the reader counts bullets, and the count breaks if a fourth case appears. It also conflates two kinds of case: bullets 1–2 are about filter state and hold for every operator; only bullet 3 is an exception to "every condition".
2 controls.mdx:98 The is not / single-moment bullet is the longest sentence on the page and describes something no author can act on; two thirds of it is rationale that belongs in the feature PR. The reader-facing rule is already at :100.
3 controls.mdx:32 The new Boolean row adds a fourth dimension type that "Single vs. multiple selection" (:36) doesn't cover — it still names only string, time and number defaults.
4 controls.mdx:107 The <Warning>'s "otherwise" has no antecedent: Cancel works whether or not you closed the gear, so it isn't an alternative to leaving it open. Also flags a maintenance liability — the callout documents a bug as expected behaviour and goes silently wrong when the gear is fixed; worth linking the follow-up issue from it.

General observation (not posted inline)

Length. docs-mintlify/CLAUDE.md asks for "the shortest version that fully answers 'how do I use this?'", and specifically: do not pad, say it once. Faceted filters went from two paragraphs to seven plus two #### subsections and a callout, for a change that is fundamentally "the source picker no longer greys out most operators". The single point that faceting affects only suggested values is made three times — :84 ("scopes the values it suggests"), :88 ("changes only the values this filter suggests"), :100 ("never narrowed past what the charts show"). Findings 1 and 2 each cut in this direction; a further pass over :88 and :100 would likely fold three sentences into one.

This is a judgement call rather than a defect — the section is accurate and well-organized, and the two subsections are genuinely separate topics. But the page is now the longest in the dashboards tree, and the increment is a mid-size feature.

Praise

The #### Choosing which filters narrow this one split is the right cut — "what a source contributes" and "which sources apply" are different questions and a reader arrives with one or the other. Replacing "marking both as faceted" with the one-directional explanation at :86 fixes a real misreading in the old text, and naming source in the example before the heading that leans on it is the correct ordering.

Review threads

The listing returned 0 threads — no prior threads of mine on this PR (the earlier review comments in the timeline are mngr's own, which I never resolve). Nothing resolved, nothing re-affirmed, no findings skipped as duplicates. All four findings above were posted fresh.

· branch [`docs/cub-4769`](https://github.com/cube-js/cube/tree/docs/cub-4769)

Comment thread docs-mintlify/docs/explore-analyze/dashboards/widgets/controls.mdx Outdated
@claude

claude Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Claude finished @mngr's task in 1m 57s —— View job


1 medium, 3 low — docs-only change, factual claims consistent with the PR's own code citations

Review detail
  • Read the diff and the surrounding page
  • Check internal consistency and anchors
  • Reconcile prior review threads
  • Post findings

Posted inline

Medium — controls.mdx:104, dead-end cross-reference. The new custom-SQL bullet routes the reader to Sharing the current selection as the only way to set a filter to custom SQL, but that section's type table (:409-417) has no custom row and the prose after it (:419-423) covers only Boolean values, between and caseSensitive. The same gap hits the new warning bullet at :128 — it cites a URL parameter for several values on a substring condition, while the URL section documents an array value for between alone. :129's caseSensitive case is the only one of the three that got its counterpart, at :423.

Not posted (low, editorial)

  • :100 vs :104 — the lead promises a source can be set to "whatever condition", and bullet 3 then takes custom SQL back out. The "That comes with qualifications:" frame carries it, but the lead is literally false on its own.
  • :110 — "A date source never scopes the value list to less than the window the charts are showing" now runs into the appended clause saying is null / is not null name no window at all; the guarantee is about windows and the clause is about their absence.
  • :106 — the picker's tooltip is still quoted as "not available for this semantic view or filter type" while the prose now gives the view as the only reason. Correct as a UI literal, but the reader has no way to tell that from the page.

These three are the low-severity editorial kind the PR has said it isn't taking this round; they're recorded rather than pressed.

What I could not verify

Every code citation in the PR body is to packages/console-ui/... in cubedevinc/cubejs-enterprise, which isn't in this checkout — facet-scope-utils.ts, FilterEditSidebar.tsx, semantic-sql-generator.ts and the rest. The behavioural claims (every (member type, condition) pair builds a facet filter, presenceBuilder emitting a valueless notSet / set, the notContains family being ILIKE-only, custom absent from DASHBOARD_FILTER_TYPES_BY_MEMBER_TYPE) are taken as stated. What I checked is that the page is internally consistent with them and with the sections merged from CUB-2842 — it is, apart from the cross-reference above.

Review threads

The unresolved-thread listing came back empty — 0 threads, mine or anyone's. None resolved, none re-affirmed.

· [`docs/cub-4769`](https://github.com/cube-js/cube/tree/docs/cub-4769)

Comment thread docs-mintlify/docs/explore-analyze/dashboards/widgets/controls.mdx
@claude

claude Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

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


Review complete — 1 medium, 1 low-medium, both inline

Details
  • Read the diff and the surrounding page
  • Check internal consistency and anchors
  • Check prior review threads (stale / duplicate)
  • Post findings

Findings

Severity Site Concern
Medium controls.mdx:418 The new table row documents custom as a URL-settable condition whose value is "a SQL condition, used in the query as written" — an arbitrary-SQL surface published without naming any permission or validation gate, and with an "as written" claim the page can't support if the engine re-parses the SQL. Asked for the gate to be named or the row narrowed to "custom exists, a facet source set to it scopes nothing".
Low-medium controls.mdx:424 "With true it matches the case of the value exactly" is a claim about the warehouse: the key only swaps ILIKE for LIKE, which is case-insensitive under MySQL _ci and Snowflake case-insensitive collations. Suggested wording that says what Cube emits and defers to the collation. It also makes the <Warning> bullet at line 129 collation-dependent.

Not posted: the custom row sits under a sentence framing the table as "the strings for the conditions in Operators by dimension type", where Custom SQL does not appear — editorial, and the standing instruction on this PR is that only medium and high findings are acted on.

Checked and clean

  • #sharing-the-current-selection (line 391) and #operators-by-dimension-type both resolve; every new link on the page points at a real heading.
  • The faceting lead at line 100 and its three qualifying bullets agree with the <Warning> at 123–130: the valueless four narrow on being set, everything else waits for a value, and the nothing-to-scope-by cases are stated once, in the warning.
  • The two new <Warning> bullets are internally consistent with the new URL-reference paragraphs — several values on a substring condition joined into one term (128 ↔ 426), and the case-sensitive negation narrowing tighter than the charts (129 ↔ 424).
  • The date-window table's new lead-in scopes the presence pair out of the "which window" frame rather than adding a row that would have to read "none" — correct given presenceBuilder emits a valueless notSet / set.
  • Code claims about packages/console-ui (the FACET_FILTER_BUILDERS table, url-filter-params.ts, semantic-sql-generator.ts) could not be verified here — that source lives in cubedevinc/cubejs-enterprise, not this repo. Both findings above are argued from the page's own text and from database semantics, not from a reading of that code.

Review threads

The unresolved-thread listing came back empty — no prior threads of mine on this PR, so none to resolve and no duplicates to skip.

· branch [`docs/cub-4769`](https://github.com/cube-js/cube/tree/docs/cub-4769)

Comment thread docs-mintlify/docs/explore-analyze/dashboards/widgets/controls.mdx Outdated
Comment thread docs-mintlify/docs/explore-analyze/dashboards/widgets/controls.mdx Outdated
…aseSensitive to what Cube emits (CUB-4769)
@claude

claude Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Claude finished @mngr's task in 3m 9s —— View job


Reviewed — 3 medium, 0 high. Docs-only; nothing blocks merge on this repo's side.

Review detail
  • Read the diff against origin/master
  • Read the whole changed page for context
  • Cross-check the new reference paragraphs against the page's own claims
  • Handle prior review threads (resolve stale / dedupe)
  • Post findings

Findings

All three are internal-consistency gaps in the two new Sharing the current selection paragraphs and the widened <Warning> — no factual claim on the page contradicts the code as the PR description documents it.

# Site Concern
1 controls.mdx:426 The new list-value paragraph names is / is not and the substring conditions only, while controls.mdx:114 and the <Warning> bullet at controls.mdx:127 both treat the in the … / not in the … conditions as taking several values — so the reference denies by omission what the warning rests on. Same line: "joined into a single term" never names the separator, so the emitted LIKE pattern can't be predicted. (discussion)
2 controls.mdx:128-129 The two added bullets abandon the "unreachable from the filter UI" rule that kept the other URL-only drop cases out, but leave those cases out anyway — a non-scalar numeric bound and between on a Number dimension. Both are reachable from a hand-written link the page elsewhere encourages. (discussion)
3 controls.mdx:418 The new custom row omits that its value is the WHERE predicate — the one value contract in the table that differs — and uses the cell to restate the facet fact already at controls.mdx:104. Suggestion posted. (discussion)

What checks out

  • The caseSensitive paragraph at controls.mdx:424 and the <Warning> bullet at controls.mdx:129 agree on direction: a case-sensitive negation excludes fewer rows in the charts than a case-insensitive facet query would, so dropping the source is the only option that doesn't hide visible values. The collation hedge ("wherever the column's collation is case-sensitive") is the right one for a LIKE / ILIKE split.
  • The date-window table's new lead-in at controls.mdx:110 resolves the presence pair before the table rather than adding a row whose window cell would read "none" — consistent with the table staying keyed on "which window".
  • The three qualification bullets at controls.mdx:102-104 cover everything the removed "every other condition scopes nothing" paragraph used to, and controls.mdx:106 correctly drops the condition-based greying reason while keeping the flagged-source case.

Review threads

gh list-review-threads cube-js cube 11936 returned 0 unresolved threads across a single page — no prior threads, so none resolved and none re-affirmed. All three findings posted fresh.

Not done

The claims rest on packages/console-ui/** in cubedevinc/cubejs-enterprise, which isn't checked out here — I verified them against each other and against the page, not against that source. The PR description's own citations (facet-scope-utils.ts, semantic-sql-generator.ts, url-filter-params.ts) are the record for those. No build or lint run: the change is .mdx prose with no docs.json edit.

· branch [`docs/cub-4769`](https://github.com/cube-js/cube/tree/docs/cub-4769)

Comment thread docs-mintlify/docs/explore-analyze/dashboards/widgets/controls.mdx Outdated
Comment thread docs-mintlify/docs/explore-analyze/dashboards/widgets/controls.mdx
Comment thread docs-mintlify/docs/explore-analyze/dashboards/widgets/controls.mdx
…what the nothing-to-scope-by list is a list of (CUB-4769)
@claude

claude Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Claude finished @mngr's task in 3m 8s —— View job


Docs-only change, internally consistent apart from one cross-reference conflict — 0 high, 1 medium, 2 low (2 posted inline).

Review detail
  • Read the changed file in full, not just the diff
  • Check internal consistency of the new claims across the page
  • Verify anchors / links resolve
  • Reconcile prior review threads
  • Post findings

Findings

Medium — controls.mdx:426 contradicts controls.mdx:127 (thread)

The new list-value paragraph ends with "a list in their value leaves the widget unable to build its query at all" for the in the … / not in the … family, while the <Warning> bullet at line 127 says those same conditions "on several values — each reads one period from one value" while the charts narrow. Both cannot hold for {"type":"in_month","value":["2026-01","2026-02"]}. Whichever way the code goes, the other line needs restating in the same push; controls.mdx:45 leans the same way as 127.

Low — controls.mdx:418, the custom row (thread)

The row never says a value is required (the row above it does say the null/empty family takes none), so {"type":"custom"} is rejected with nothing on the page explaining it. The clause it does carry — "a facet source set to it scopes nothing" — repeats line 104.

Low — not posted, settled by an earlier round. controls.mdx:106 now names the semantic view as the only greying reason while the tooltip it quotes still reads "not available for this semantic view or filter type". The round at 02:12:57Z decided deliberately to leave the member-data-type gate undocumented, since it is unreachable through the operator picker; raising it again would just re-litigate that.

What checks out

  • Every anchor the diff adds or moves resolves to a heading on the page: #sharing-the-current-selection, #operators-by-dimension-type.
  • The rewritten lead at line 100 and its three qualifying bullets no longer leave a condition unaccounted for, and the "greyed out" prose at 106 matches the picker reason the bullets left standing.
  • The caseSensitive paragraph at 424 states the guarantee where it holds (LIKE vs ILIKE, collation-dependent) rather than as a warehouse-wide claim, and the <Warning> bullet at 129 stays unqualified, which is right — the dropped case-sensitive negation is not collation-dependent.
  • The date-window table's new lead-in at 110 places is null / is not null without adding a row that would have to read "none".

Review threads

The unresolved-thread listing returned 0 threads of mine across all pages, so none to resolve and none re-affirmed. Both findings posted as new threads.

· branch `docs/cub-4769`

Comment thread docs-mintlify/docs/explore-analyze/dashboards/widgets/controls.mdx
Comment thread docs-mintlify/docs/explore-analyze/dashboards/widgets/controls.mdx
@claude

claude Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Claude finished @mngr's task in 1m 55s —— View job


1 medium, 0 high — one inline finding on the <Warning>; everything else in the diff checks out.

Review detail
  • Read the PR diff and the full changed file
  • Verify claims against the page's own surrounding text
  • Check prior review threads (resolve stale / dedupe)
  • Post findings

Finding

mediumdocs-mintlify/docs/explore-analyze/dashboards/widgets/controls.mdx:127 (thread). The parenthetical added to the is not bullet describes a case where "a list in theirs stops the charts building a query at all" — no query, no narrowing — inside a <Warning> whose premise is a value list left wide "while the charts narrow", under a membership rule this same push added at line 124 ("cases where the charts match on something the value list has no way to ask for"). It is the same test the PR description uses to keep the two numeric crash cases out of the list; the in the … list case is that state exactly. The fact itself is already stated for the whole family at line 426, without the false claim about the charts.

What checked out

  • Line 45 (several only on is/is not``) agrees with the new list-value paragraph at line 426 and with line 34's `equals` example; `between`'s two-entry array is still correctly described as "one per field".
  • Line 110's new lead-in scopes the window table to the conditions that name a window and sends is null / is not null to presence, which matches the no-value bullet at line 102 — no orphaned row.
  • The custom row (line 418) names the type and what a facet source set to it does, and says nothing about the value; the faceting bullet at line 104 resolves to it.
  • The caseSensitive paragraph (line 424) qualifies the guarantee at LIKE vs ILIKE rather than asserting warehouse behaviour, and the <Warning> bullet at line 129 correctly does not inherit that qualification.

Review threads

Listing returned 0 of my own threads (0 unresolved threads on the PR at all). Nothing to resolve, nothing re-affirmed; the finding above is not a duplicate.

· branch [`docs/cub-4769`](https://github.com/cube-js/cube/tree/docs/cub-4769)

Comment thread docs-mintlify/docs/explore-analyze/dashboards/widgets/controls.mdx Outdated
@claude

claude Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

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


No medium or high findings — 0 high, 0 medium, 4 low (nothing posted inline)

Review detail
  • Fetch PR diff (gh pr diff 11936)
  • Read controls.mdx in full for internal consistency
  • Check the four changed regions against the rest of the page
  • Resolve stale review threads / dedupe findings
  • Report

Scope

One file, docs-mintlify/docs/explore-analyze/dashboards/widgets/controls.mdx, four regions: the Time row of the selection table (line 45), the Faceted filters source paragraph (100–106), the date-window table's lead-in and the "nothing to scope by" <Warning> (110, 123–130), and the filter-JSON reference (418, 424, 426). The product source this page describes lives in cubedevinc/cubejs-enterprise, which is not checked out here, so the behavioural claims were checked for internal consistency across the page rather than re-derived from the code.

What holds

  • Line 45's new is / is not restriction agrees with line 426 (equals / not_equals are the two conditions that read a list) and with the <Warning> bullet at 127, which is the one place the page puts is not on several dates in the nothing-to-scope-by state. The three lines no longer disagree.
  • The date table's new lead-in resolves the gap it was meant to: is null / is not null now have an answer before the table, and adding no row is the right call — the pair resolves to no window, so a Window the value list is scoped to cell for either would have to read "none".
  • The <Warning>'s membership rule ("the charts match on something the value list has no way to ask for") is true of all four bullets, including the caseSensitive one — a case-insensitive notContains excludes rows a case-sensitive NOT LIKE chart keeps, i.e. drops values the charts still show.
  • Every anchor the diff adds or moves resolves: #sharing-the-current-selection (line 391), #operators-by-dimension-type (line 19).

Low findings (not posted inline)

Only medium- and high-severity findings are actioned on this PR per the maintainer's standing note, and these are all editorial or marginal. Listed for the record, not as change requests.

  1. controls.mdx:426 — "Every other condition reads exactly one value — the comparisons, and the in the … and not in the … family — and a list in their value leaves the widget unable to build its query at all." The set isn't closed: custom, which line 418 documents as a writable type eight lines above, is in neither the "reads a list" group nor the enumerated one, and a list in its value doesn't fail to build a query — the value reaches the SQL verbatim.
  2. controls.mdx:34 — "a time filter carries several values for one condition only when a hand-written URL parameter passes a list in its value" carries no is / is not restriction, so it reads more permissively than line 45 now does. Its own example uses equals, which keeps it from being wrong, but a reader taking the rule rather than the example gets in_month with a list.
  3. controls.mdx:100 — "whatever condition that sibling is set to" is contradicted by the third bullet at line 104. "That comes with qualifications:" covers a qualification; custom SQL is an exception, and naming it in the lead is the shape the earlier rounds settled on for the is not/single-moment case.
  4. controls.mdx:128, :129 — "which only a [URL parameter] can give one" leaves "one" reaching back past "several values" to the condition, and "narrowed case-insensitively alone" uses "alone" where the sentence means "only".

Review threads

The unresolved-thread listing came back empty on its only page (hasNextPage: false) — no prior threads, so none resolved and none re-affirmed.

· branch [`docs/cub-4769`](https://github.com/cube-js/cube/tree/docs/cub-4769)

This branch was successfully deployed

1 active deployment
staging - docs-mintlify 90721e86 Deployed Sep 22, 2026 by mintlify[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Issues that require a documentation improvement pr:community Contribution from Cube.js community members.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant