Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
|
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 lowReview details
Scope
Findings
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 threadsThe 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. |
|
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
Scope of verification
Anchor check: Findings
Dropping Review threadsThe listing returned 0 of my own threads (single page, |
|
Claude finished @mngr's task in 2m 9s —— View job Docs review — 3 low-severity findings, all posted inline. No blockers.Review detail
Findings
What I could and couldn't verifyThe behavioural claims are all about What I did check here:
Review threadsThe listing returned 0 unresolved threads authored by me — no prior threads, so none resolved and none re-affirmed. The four |
|
Claude finished @mngr's task in 2m 33s —— View job Docs change is sound — 3 low findings, all editorial (no blockers)Review details
Scope of verificationThe behaviour this page describes lives in What I did check:
Findings
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 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 threadsThe 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 |
|
Claude finished @mngr's task in 2m 13s —— View job Docs review: no blockers — 4 low findings posted inline, 1 general observationReview detailsTodo
What I could and could not verifyThe behaviour this page describes lives in Anchors check out: Findings
General observation (not posted inline)Length. 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. PraiseThe Review threadsThe listing returned 0 threads — no prior threads of mine on this PR (the earlier review comments in the timeline are |
|
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 citationsReview detail
Posted inlineMedium — Not posted (low, editorial)
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 verifyEvery code citation in the PR body is to Review threadsThe unresolved-thread listing came back empty — 0 threads, mine or anyone's. None resolved, none re-affirmed. |
… URL reference (CUB-4769)
|
Claude finished @mngr's task in 2m 34s —— View job Review complete — 1 medium, 1 low-medium, both inlineDetails
Findings
Not posted: the Checked and clean
Review threadsThe unresolved-thread listing came back empty — no prior threads of mine on this PR, so none to resolve and no duplicates to skip. |
…aseSensitive to what Cube emits (CUB-4769)
|
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
FindingsAll three are internal-consistency gaps in the two new Sharing the current selection paragraphs and the widened
What checks out
Review threads
Not doneThe claims rest on |
…what the nothing-to-scope-by list is a list of (CUB-4769)
|
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
FindingsMedium — The new list-value paragraph ends with "a list in their Low — The row never says a Low — not posted, settled by an earlier round. What checks out
Review threadsThe 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. |
… charts do narrow on (CUB-4769)
|
Claude finished @mngr's task in 1m 55s —— View job 1 medium, 0 high — one inline finding on the Review detail
Findingmedium — What checked out
Review threadsListing 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. |
…e charts do narrow (CUB-4769)
|
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
ScopeOne file, What holds
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.
Review threadsThe unresolved-thread listing came back empty on its only page ( |
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..HEADon the branch lists one commit after this page's last edit,c5f73077a4, which only rewords a comment inplaywright/pages/workbook.page.ts).Re-based once more onto today's master, which gained #11960 — that PR touches
widgets/index.mdxandwidgets/layout.mdxonly, 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 namescaseSensitive, and the filter-JSON reference it links to documents that key besidestartInclusive/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 acustomrow in itstypetable, and a clause placed after thecaseSensitiveparagraph: outsidebetween, avaluemay still be a list —isandis notread 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
customrow 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-233lists'custom'invalidTypesand applies no parse, permission check or allow-list to itsvalue, andsemantic-sql-generator.ts:320-321iscase 'custom': return filterItem.value as string;, thecolumnRefabove 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 nothingand describes thevaluenot at all; that is the reviewer's own narrower option, and it keeps thetypestring 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
caseSensitiveparagraph claiming something about the warehouse rather than about Cube — "withtrueit matches the case of the value exactly", where the key's whole effect (semantic-sql-generator.ts:197-198) isLIKEinstead ofILIKE, which a MySQL_cior a case-insensitive Snowflake collation still matches case-insensitively. The sentence now qualifies the guarantee where it makes it: withtruethe match is made withLIKErather thanILIKE, 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, sincesubstringBuilderdrops a case-sensitive negation unconditionally, Cube'snotContainsfamily 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-
valueparagraph namingis/is notand 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. ProbinggenerateSemanticSql(a throwaway vitest, one filter per case) showed the omission was the opposite of an omission:{"type":"contains","value":["ship","proc"]}emitsILIKE '%ship,proc%'— comma, no space —{"type":"not_equals","value":[...]}emitsNOT IN (...), and a list onin_month/greater_thanthrowsvalue.replace is not a function, because those branches handfilterItem.valuetoformatFilterValue, 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 andbetweenon 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_BUILDERScarriesbetween: numberBetweenBuilder, which mirrors the chart'sBETWEEN 10 AND 20as agte/ltepair 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 samevalue.replacethrow. 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.tspredates 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-valueparagraph at line 426 and the<Warning>bullet at line 127 disagreed about whatf_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 overgenerateSemanticSqlhasin_monthandgreater_thanwith a list both throwvalue.replace is not a function, so no query is built, whilenot_equalswith a list emitsorders.status NOT IN ('a', 'b'). Thein 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 keepsis noton several dates, which is the leg the code really does put in this state (NOT INin the chart,nullfromtimeEqualityBuilder, which drops a negated list), and says in the same breath why thein 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 saysis/is notonly.This push answers the round's medium finding, r4075788212, in
90721e86a, and it changes one line. The previous push put thein the …/not in the …list case into the<Warning>as a parenthetical on theis notbullet, 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: thein_month/not_in_month/in_quarter/ … branch ofsemantic-sql-generator.tshandsfilterItem.valuetoformatFilterValue, whose only string path isvalue.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-valueparagraph of the filter-JSON reference ("Every other condition reads exactly one value — the comparisons, and thein the …andnot in the …family — and a list in theirvalueleaves 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
customrow, 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 notand 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 anot contains/not starts with/not ends withcarrying"caseSensitive": true— both settable only through a hand-written URL parameter, both leaving the list alone rather than scoping it differently from the charts. ThecaseSensitivekey is documented where that caveat sends the reader: the filter-JSON reference under Sharing the current selection now carries it beside thebetweenparagraph'sstartInclusive/endInclusive, with the examplef_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:presenceBuilderemits a valuelessnotSet/setUnaryFilter, 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 theis not-on-a-single-moment warning bullets.What I verified against the code
FACET_FILTER_BUILDERSinpackages/console-ui/src/modules/d3/components/Workbook/DashboardBuilder/utils/facet-scope-utils.tscovers every(member data type, condition)pair the operator picker can produce — string, number, time, boolean — andfacet-scope-utils.spec.tsasserts that exhaustively.isFacetingAvailableis just a lookup in that table.presenceBuilder(is_null→notSet,is_not_null→set, on every member type including the unresolved-type fallback) andemptyStringBuilder(is_empty→equals [''],is_not_empty→notEquals [''], string only) take noitem.value, and they passgenerateCubeFilter'sisFilterCompletegate — the same gate that drops a source still waiting on a value, matching what the chart's SQL does.DASHBOARD_FILTER_TYPES_BY_MEMBER_TYPE(utils/dashboard-filter-operators.ts) offers nocustomfor any member type, whileurl-filter-params.ts'svalidTypesaccepts it — so a URL parameter is the only way to set one, andFACET_FILTER_BUILDERShas nocustomcell for any type.isFilterAvailableForFacetissame semantic view && isFacetingAvailable, so with the table full the only reason a pickable filter greys out is the view;FilterEditSidebar.tsxrenders theIconAlertTrianglerightIcon(and adangertheme on the picker) for an item that is selected and no longer available, rather than removing it.convertFilterValue(WorkbookReportFilters/filter-utils.tsx) collapses the value toextractFirstValuewhen the operator changes to aCONTAINS_FILTERSone, 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;substringBuilderreturnsnullfor an array rather than emitting a Cube OR.caseSensitive: true(url-filter-params.tsreads it off the parameter;semantic-sql-engine.tssets it only when parsing a report's own SQL), the chart's negations becomeNOT LIKEwhen it is set (semantic-sql-generator.ts:198), and Cube'snotContainsfamily is ILIKE-only — sosubstringBuilderdrops a case-sensitive negation instead of scoping tighter than the chart.caseSensitivekey as documented: declared on the URL filter shape besidestartInclusive/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-198picksLIKE/NOT LIKEonly when it istrue, so a filter without the key is case-insensitive, and the operator is the only place it applies (equalsuses=/IN). The claim that the editor cannot set it: the Case Sensitive switch inContainsFilterControl.tsx:138is commented out (// TODO: restore Case Sensitive switch once backend supports it), so the popover renders the value input and Apply alone.customtypeas documented, and what the page deliberately stops short of:customis invalidTypes(packages/console-ui/src/modules/d3/utils/url-filter-params.ts:208-233), so the parameter parses, and it is not inVALUE_LESS_FILTER_TYPES, sovalidateUrlFilterrequires itsvalue;semantic-sql-generator.ts:320-321iscase 'custom': return filterItem.value as string, i.e. the value becomes theWHEREpredicate verbatim, with no parse and no permission check anywhere on that path. The row therefore names thetypeand what a facet source set to it does, and says nothing about thevalue— see the push note above and CUB-4950.valueas documented, probed rather than read: the array branches insemantic-sql-generator.ts:199-232belong toequalsandnot_equalsalone (IN/NOT IN, on a time dimension as much as a string or number one); the substring cases at:258-270interpolate%${filterItem.value}%with no array branch, and a run ofgenerateSemanticSqlover{"type":"contains","value":["ship","proc"]}emitsorders_view.status ILIKE '%ship,proc%'— the array stringified by the template literal, so comma with no space. Every other condition passes the value toformatFilterValue/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 throwvalue.replace is not a function, so no query is built at all.betweenon a Number member is the one array-taking condition outside those:orders_view.amount BETWEEN 10 AND 20in the chart,{ and: [gte, lte] }fromnumberBetweenBuilderin the facet.staging-mngr-6by 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