Copilot commented on code in PR #1235: URL: https://github.com/apache/skywalking-banyandb/pull/1235#discussion_r3678982226
########## canopy/web/src/pipelines/topn-shared.tsx: ########## @@ -0,0 +1,277 @@ +/* + * Licensed to Apache Software Foundation (ASF) under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Apache Software Foundation (ASF) licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// topn-shared.tsx — direction<->sort mapping, the RankBadge/CondChip atoms and +// the flat-criteria<->model.v1.Criteria codec shared by TopNForms.tsx, +// TopNList.tsx and TopNDetail.tsx (docs/pipelines-design.md §1/§6). +// +// Ported from the handoff's topn-page.jsx (RankBadge, CondChip, SORT_TONE/ +// SORT_RANK helpers) and topn-form.jsx (TOPN_OPS, DEFAULT_COUNTERS), pulled +// into one module so the form and the list/detail pages don't have to import +// from each other. NEW here (the handoff had no wire-codec — its mock +// `criteria` was already a flat array): buildTopNCriteria/flattenTopNCriteria, +// which translate the UI's flat ANDed {tag,op,value} rows to/from the real +// model.v1.Criteria recursive tree BanyanDB expects. This reuses +// PropertyTagValue/encodePropertyTagValue/decodePropertyTagValue from +// data/api.ts — model.v1.Criteria's Condition.value is the same TagValue +// oneof for every catalog (property, measure, ...), so there is nothing +// TopN-specific to add there. + +import { useMemo } from 'react'; +import { useQuery, useQueries } from '@tanstack/react-query'; +import type { PropertyCriteria, PropertyTagValue, MeasureSchema, TopNAggregationSchema } from 'canopy-shared'; +import { apiDataSource, decodePropertyTagValue } from '../data/api.js'; +import { IconTopN } from '../components/icons.js'; + +export type TopNSort = 'SORT_DESC' | 'SORT_ASC' | 'SORT_UNSPECIFIED' | undefined; + +// BanyanDB defaults an unset field_value_sort to SORT_DESC (proto3 zero value +// for the Sort enum) — treat undefined the same as SORT_DESC everywhere. +export function topNTone(sort: TopNSort): 'topn' | 'bottomn' | 'bothn' { + if (sort === 'SORT_ASC') return 'bottomn'; + if (sort === 'SORT_UNSPECIFIED') return 'bothn'; + return 'topn'; +} + +export function topNRank(sort: TopNSort): 'topN' | 'bottomN' | 'both' { + if (sort === 'SORT_ASC') return 'bottomN'; + if (sort === 'SORT_UNSPECIFIED') return 'both'; + return 'topN'; +} + +export function topNSortLabel(sort: TopNSort): string { + return sort ?? 'SORT_DESC'; +} + +export const SORT_OPTS: ReadonlyArray<{ + readonly value: 'SORT_DESC' | 'SORT_ASC' | 'SORT_UNSPECIFIED'; + readonly rank: 'topN' | 'bottomN' | 'both'; + readonly label: string; + readonly hint: string; +}> = [ + { value: 'SORT_DESC', rank: 'topN', label: 'SORT_DESC', hint: 'ranks the largest values first' }, + { value: 'SORT_ASC', rank: 'bottomN', label: 'SORT_ASC', hint: 'ranks the smallest values first' }, + { value: 'SORT_UNSPECIFIED', rank: 'both', label: 'SORT_UNSPECIFIED', hint: 'tracks both top and bottom counters' }, +]; + +export const DEFAULT_COUNTERS = 1000; + +/** model.v1.Condition.BinaryOp offered in the flat criteria editor — mirrors + * property-bydbql.ts's PROP_OPS (comparison + set membership; MATCH/HAVING + * are query-time-only operators, not meaningful for a topn-agg's criteria). */ +export const TOPN_OPS: ReadonlyArray<{ readonly value: string; readonly label: string }> = [ + { value: 'BINARY_OP_EQ', label: 'equals =' }, + { value: 'BINARY_OP_NE', label: 'not equals ≠' }, + { value: 'BINARY_OP_GT', label: 'greater >' }, + { value: 'BINARY_OP_GE', label: 'greater or equal ≥' }, + { value: 'BINARY_OP_LT', label: 'less <' }, + { value: 'BINARY_OP_LE', label: 'less or equal ≤' }, + { value: 'BINARY_OP_IN', label: 'in (a, b)' }, + { value: 'BINARY_OP_NOT_IN', label: 'not in (a, b)' }, +]; + +const TOPN_OP_SYMBOL: Record<string, string> = { + BINARY_OP_EQ: '=', + BINARY_OP_NE: '≠', + BINARY_OP_GT: '>', + BINARY_OP_GE: '≥', + BINARY_OP_LT: '<', + BINARY_OP_LE: '≤', + BINARY_OP_IN: 'IN', + BINARY_OP_NOT_IN: 'NOT IN', +}; + +export function topnOpSymbol(op: string): string { + return TOPN_OP_SYMBOL[op] ?? op; +} + +/** small shared badge: DESC->topN / ASC->bottomN / UNSPECIFIED->both */ +export function RankBadge({ sort, large }: { readonly sort: TopNSort; readonly large?: boolean }) { + const cls = (large ? 'topn-rank-lg' : 'topn-rank') + ' is-' + topNTone(sort); + return ( + <span className={cls}> + <IconTopN size={large ? 14 : 11} /> + {topNRank(sort)} + </span> + ); +} + +/** a single criteria condition rendered as a chip: tag op value */ +export function CondChip({ tag, op, value }: { readonly tag: string; readonly op: string; readonly value: string }) { + return ( + <span className="topn-cond mono"> + <span className="topn-cond-tag">{tag}</span> + <span className="topn-cond-op">{topnOpSymbol(op)}</span> + <span className="topn-cond-val">{value === '' ? '∅' : value}</span> + </span> + ); +} + +// ── flat criteria <-> model.v1.Criteria codec ─────────────────────────────── + +export interface TopNCondition { + readonly tag: string; + readonly op: string; + readonly value: string; +} + +const PQ_NUM = (s: string): boolean => /^-?\d+(\.\d+)?$/.test(s.trim()); Review Comment: `PQ_NUM` treats decimal values (e.g. `1.5`) as numeric and encodes them as `TagValue.int`, but `PropertyTagValue`/`TagValue` has no float variant (int must be an integer). This can produce invalid criteria payloads for non-integer inputs and likely server-side validation errors. ########## canopy/web/src/pipelines/topn-shared.tsx: ########## @@ -0,0 +1,277 @@ +/* + * Licensed to Apache Software Foundation (ASF) under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Apache Software Foundation (ASF) licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// topn-shared.tsx — direction<->sort mapping, the RankBadge/CondChip atoms and +// the flat-criteria<->model.v1.Criteria codec shared by TopNForms.tsx, +// TopNList.tsx and TopNDetail.tsx (docs/pipelines-design.md §1/§6). +// +// Ported from the handoff's topn-page.jsx (RankBadge, CondChip, SORT_TONE/ +// SORT_RANK helpers) and topn-form.jsx (TOPN_OPS, DEFAULT_COUNTERS), pulled +// into one module so the form and the list/detail pages don't have to import +// from each other. NEW here (the handoff had no wire-codec — its mock +// `criteria` was already a flat array): buildTopNCriteria/flattenTopNCriteria, +// which translate the UI's flat ANDed {tag,op,value} rows to/from the real +// model.v1.Criteria recursive tree BanyanDB expects. This reuses +// PropertyTagValue/encodePropertyTagValue/decodePropertyTagValue from +// data/api.ts — model.v1.Criteria's Condition.value is the same TagValue +// oneof for every catalog (property, measure, ...), so there is nothing +// TopN-specific to add there. + +import { useMemo } from 'react'; +import { useQuery, useQueries } from '@tanstack/react-query'; +import type { PropertyCriteria, PropertyTagValue, MeasureSchema, TopNAggregationSchema } from 'canopy-shared'; +import { apiDataSource, decodePropertyTagValue } from '../data/api.js'; +import { IconTopN } from '../components/icons.js'; + +export type TopNSort = 'SORT_DESC' | 'SORT_ASC' | 'SORT_UNSPECIFIED' | undefined; + +// BanyanDB defaults an unset field_value_sort to SORT_DESC (proto3 zero value +// for the Sort enum) — treat undefined the same as SORT_DESC everywhere. +export function topNTone(sort: TopNSort): 'topn' | 'bottomn' | 'bothn' { + if (sort === 'SORT_ASC') return 'bottomn'; + if (sort === 'SORT_UNSPECIFIED') return 'bothn'; + return 'topn'; +} + +export function topNRank(sort: TopNSort): 'topN' | 'bottomN' | 'both' { + if (sort === 'SORT_ASC') return 'bottomN'; + if (sort === 'SORT_UNSPECIFIED') return 'both'; + return 'topN'; +} + +export function topNSortLabel(sort: TopNSort): string { + return sort ?? 'SORT_DESC'; +} + +export const SORT_OPTS: ReadonlyArray<{ + readonly value: 'SORT_DESC' | 'SORT_ASC' | 'SORT_UNSPECIFIED'; + readonly rank: 'topN' | 'bottomN' | 'both'; + readonly label: string; + readonly hint: string; +}> = [ + { value: 'SORT_DESC', rank: 'topN', label: 'SORT_DESC', hint: 'ranks the largest values first' }, + { value: 'SORT_ASC', rank: 'bottomN', label: 'SORT_ASC', hint: 'ranks the smallest values first' }, + { value: 'SORT_UNSPECIFIED', rank: 'both', label: 'SORT_UNSPECIFIED', hint: 'tracks both top and bottom counters' }, +]; + +export const DEFAULT_COUNTERS = 1000; + +/** model.v1.Condition.BinaryOp offered in the flat criteria editor — mirrors + * property-bydbql.ts's PROP_OPS (comparison + set membership; MATCH/HAVING + * are query-time-only operators, not meaningful for a topn-agg's criteria). */ +export const TOPN_OPS: ReadonlyArray<{ readonly value: string; readonly label: string }> = [ + { value: 'BINARY_OP_EQ', label: 'equals =' }, + { value: 'BINARY_OP_NE', label: 'not equals ≠' }, + { value: 'BINARY_OP_GT', label: 'greater >' }, + { value: 'BINARY_OP_GE', label: 'greater or equal ≥' }, + { value: 'BINARY_OP_LT', label: 'less <' }, + { value: 'BINARY_OP_LE', label: 'less or equal ≤' }, + { value: 'BINARY_OP_IN', label: 'in (a, b)' }, + { value: 'BINARY_OP_NOT_IN', label: 'not in (a, b)' }, +]; + +const TOPN_OP_SYMBOL: Record<string, string> = { + BINARY_OP_EQ: '=', + BINARY_OP_NE: '≠', + BINARY_OP_GT: '>', + BINARY_OP_GE: '≥', + BINARY_OP_LT: '<', + BINARY_OP_LE: '≤', + BINARY_OP_IN: 'IN', + BINARY_OP_NOT_IN: 'NOT IN', +}; + +export function topnOpSymbol(op: string): string { + return TOPN_OP_SYMBOL[op] ?? op; +} + +/** small shared badge: DESC->topN / ASC->bottomN / UNSPECIFIED->both */ +export function RankBadge({ sort, large }: { readonly sort: TopNSort; readonly large?: boolean }) { + const cls = (large ? 'topn-rank-lg' : 'topn-rank') + ' is-' + topNTone(sort); + return ( + <span className={cls}> + <IconTopN size={large ? 14 : 11} /> + {topNRank(sort)} + </span> + ); +} + +/** a single criteria condition rendered as a chip: tag op value */ +export function CondChip({ tag, op, value }: { readonly tag: string; readonly op: string; readonly value: string }) { + return ( + <span className="topn-cond mono"> + <span className="topn-cond-tag">{tag}</span> + <span className="topn-cond-op">{topnOpSymbol(op)}</span> + <span className="topn-cond-val">{value === '' ? '∅' : value}</span> + </span> + ); +} + +// ── flat criteria <-> model.v1.Criteria codec ─────────────────────────────── + +export interface TopNCondition { + readonly tag: string; + readonly op: string; + readonly value: string; +} + +const PQ_NUM = (s: string): boolean => /^-?\d+(\.\d+)?$/.test(s.trim()); + +function topnConditionValue(op: string, raw: string): PropertyTagValue { + const value = raw.trim(); + if (op === 'BINARY_OP_IN' || op === 'BINARY_OP_NOT_IN') { + const parts = value.split(',').map((x) => x.trim()).filter(Boolean); + return parts.length && parts.every(PQ_NUM) ? { intArray: { value: parts } } : { strArray: { value: parts } }; + } + return PQ_NUM(value) ? { int: { value } } : { str: { value } }; +} + +/** Build a flat AND chain of Conditions into the model.v1.Criteria tree the + * registry expects. Empty/blank-tag rows are dropped. */ +export function buildTopNCriteria(conditions: readonly TopNCondition[]): PropertyCriteria | undefined { + const parts: PropertyCriteria[] = conditions + .filter((c) => c.tag.trim()) + .map((c) => ({ condition: { name: c.tag.trim(), op: c.op, value: topnConditionValue(c.op, c.value) } })); + if (!parts.length) return undefined; + return parts.reduce((acc, part) => ({ le: { op: 'LOGICAL_OP_AND', left: acc, right: part } })); +} + +/** Flatten a model.v1.Criteria tree back into the editor's row shape. + * Documented simplification (matches the design's "flat ANDed list, not the + * recursive WHERE tree"): every `condition` leaf reachable through nested + * `le` nodes is surfaced as one AND row regardless of whether the original + * tree actually used AND or OR — a schema built outside this UI with OR'd + * criteria will round-trip lossily, same tradeoff PropertyForms' editor + * accepts for its own criteria tree. */ +export function flattenTopNCriteria(criteria: PropertyCriteria | undefined): TopNCondition[] { + const out: TopNCondition[] = []; + const walk = (node: PropertyCriteria | undefined): void => { + if (!node) return; + if (node.condition) { + out.push({ tag: node.condition.name, op: node.condition.op, value: decodePropertyTagValue(node.condition.value).value }); + return; + } + if (node.le) { walk(node.le.left); walk(node.le.right); } + }; + walk(criteria); + return out; +} + +// ── measure lookups (mirrors the handoff's data.jsx findMeasure/ +// measureFieldNames/measureTagNames, adapted to canopy's fetched-schema +// shape) ───────────────────────────────────────────────────────────────── + +/** Field names defined on a measure resource — offered as `field_name` ranking picks. */ +export function measureFieldNames(m: MeasureSchema | undefined): string[] { + return m?.fields ? m.fields.map((f) => f.name) : []; +} + +/** Tag names defined on a measure resource — offered for group-by + criteria picks. */ +export function measureTagNames(m: MeasureSchema | undefined): string[] { + if (!m) return []; + return m.tagFamilies.flatMap((f) => f.tags.map((t) => t.name)); +} + +/** Resolve a measure resource by {group,name} out of a group-name -> resources map. */ +export function findMeasureIn( + resourcesByGroup: ReadonlyMap<string, readonly MeasureSchema[]>, + ref: { readonly group: string; readonly name: string } | undefined, +): MeasureSchema | undefined { + if (!ref?.group || !ref.name) return undefined; + return resourcesByGroup.get(ref.group)?.find((m) => m.metadata.name === ref.name); +} + +// ── shared data hook ──────────────────────────────────────────────────────── +// +// Every Pipelines/TopN surface (overview counts, the cross-group list, the +// create/edit form's source pickers) needs the same two things: every measure +// group + its measures (for the source-measure picker and "does the source +// still exist" check), and every measure group's registered TopNAggregations. +// Centralizing the fetch here means the three surfaces share one TanStack +// Query cache (same query keys `['groups']` / `['resources','measures',g]` / +// `['topnAggregations',g]` already used by GroupPage/Sidebar/QueryConsole), +// so navigating between them doesn't re-fetch, and a create/edit/delete +// mutation that invalidates `['topnAggregations', group]` refreshes every Review Comment: `topn-shared.tsx` defines the TopN aggregation query key as `['topnAggregations', group]`, but `TopNForms.tsx` uses `['topNAggregations', group]` and invalidates `['topNAggregations']`. This inconsistency will prevent cache sharing/invalidations from working if `useTopNCatalog()` is adopted, and the comment above also references the wrong key. ########## canopy/web/src/pipelines/TopNList.tsx: ########## @@ -0,0 +1,315 @@ +/* + * Licensed to Apache Software Foundation (ASF) under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Apache Software Foundation (ASF) licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// TopNList.tsx — the TopN pipeline type's list page (/pipelines/topn): +// filter by name/group/source/direction, New/Edit/Delete. Ported from +// .handoff-import/banyandb/project/topn-page.jsx's TopNList (window-global +// JSX -> ES module TSX; the handoff's per-group "locked" drill-down variant +// is dropped — docs/pipelines-design.md §3 only routes the all-groups list +// and the detail page, not a per-group index). +// +// ADAPTATIONS: mock `groups` prop -> live listGroups + listTopNAggregations +// (one call per measure group, mirroring QueryConsole.tsx's group-topn-agg +// prefetch pattern); onNavigate -> useNavigate; New/Edit/Delete modals are +// owned locally (mirrors PropertyDetailPage.tsx's local ModalState, not +// App.tsx's central union — Pipelines is a self-contained route tree). + +import React, { useEffect, useMemo, useState } from 'react'; +import { useNavigate } from 'react-router'; +import { useQuery } from '@tanstack/react-query'; + +import type { TopNAggregationSchema } from 'canopy-shared'; +import { apiDataSource } from '../data/api.js'; +import { useAuth } from '../auth/AuthContext.js'; +import { + IconTopN, IconSearch, IconPlus, IconEdit, IconTrash, IconGroup, IconChevron, IconAlert, +} from '../components/icons.js'; +import { RankBadge, SORT_OPTS } from './topn-shared.js'; +import { TopNFormModal, DeleteTopNModal } from './TopNForms.js'; + +const PAGE_SIZE = 10; + +interface Row { + readonly agg: TopNAggregationSchema; + readonly group: string; +} + +type ModalState = + | { readonly kind: 'create'; readonly groupName?: string } + | { readonly kind: 'edit'; readonly groupName: string; readonly agg: TopNAggregationSchema } + | { readonly kind: 'delete'; readonly groupName: string; readonly agg: TopNAggregationSchema } + | null; + +function FilterSelect({ label, value, options, onChange }: { + label: string; + value: string; + options: ReadonlyArray<{ value: string; label: string }>; + onChange: (v: string) => void; +}) { + return ( + <label className="topn-filter"> + <span className="topn-filter-label">{label}</span> + <span className="topn-select-wrap"> + <select className="topn-select" value={value} onChange={(e) => onChange(e.target.value)} aria-label={label}> + {options.map((o) => <option key={o.value} value={o.value}>{o.label}</option>)} + </select> + <span className="topn-select-chev"><IconChevron size={13} /></span> + </span> + </label> + ); +} + +export function TopNList() { + const navigate = useNavigate(); + const { session } = useAuth(); + const canWrite = session?.role === 'admin'; + const [modal, setModal] = useState<ModalState>(null); + + const { data: groupsData } = useQuery({ queryKey: ['groups'], queryFn: () => apiDataSource.listGroups() }); + const measureGroups = useMemo(() => (groupsData?.groups ?? []).filter((g) => g.catalog === 'CATALOG_MEASURE'), [groupsData]); + const measureGroupNames = useMemo(() => measureGroups.map((g) => g.name), [measureGroups]); + + const [rows, setRows] = useState<readonly Row[]>([]); + const [loaded, setLoaded] = useState(false); + useEffect(() => { + if (measureGroupNames.length === 0) { setRows([]); setLoaded(true); return; } + let cancelled = false; + Promise.allSettled( + measureGroupNames.map((g) => apiDataSource.listTopNAggregations(g).then((aggs) => ({ group: g, aggs }))), + ).then((results) => { + if (cancelled) return; + const out: Row[] = []; + for (const r of results) if (r.status === 'fulfilled') for (const agg of r.value.aggs) out.push({ agg, group: r.value.group }); + setRows(out); + setLoaded(true); + }); + return () => { cancelled = true; }; + }, [measureGroupNames]); + + // Measure names per group, so a row can flag a source measure that no + // longer exists (like the handoff's srcExists check). + const [measureNames, setMeasureNames] = useState<ReadonlyMap<string, readonly string[]>>(new Map()); + useEffect(() => { + if (measureGroupNames.length === 0) return; + let cancelled = false; + Promise.allSettled( + measureGroupNames.map((g) => apiDataSource.listResourcesInGroup('measures', g).then((rs) => ({ group: g, names: rs.map((r) => r.metadata.name) }))), + ).then((results) => { + if (cancelled) return; + setMeasureNames((prev) => { + const next = new Map(prev); + for (const r of results) if (r.status === 'fulfilled') next.set(r.value.group, r.value.names); + return next; + }); + }); + return () => { cancelled = true; }; + }, [measureGroupNames]); + + const [nameQ, setNameQ] = useState(''); + const [groupF, setGroupF] = useState('all'); + const [sourceF, setSourceF] = useState('all'); + const [rankF, setRankF] = useState('all'); + + const groupScoped = groupF === 'all' ? rows : rows.filter((r) => r.group === groupF); + + const sourceOptions = useMemo(() => { + const out: string[] = []; + for (const r of groupScoped) { + const n = r.agg.sourceMeasure?.name; + if (n && !out.includes(n)) out.push(n); + } + return out.sort(); + }, [groupScoped]); + + const q = nameQ.trim().toLowerCase(); + const filtered = groupScoped.filter((r) => { + if (q && !r.agg.metadata.name.toLowerCase().includes(q)) return false; + if (sourceF !== 'all' && r.agg.sourceMeasure?.name !== sourceF) return false; + if (rankF !== 'all' && (r.agg.fieldValueSort ?? 'SORT_DESC') !== rankF) return false; + return true; + }); + + const totalCount = rows.length; + const filtersActive = !!q || sourceF !== 'all' || rankF !== 'all' || groupF !== 'all'; + const newTargetGroup = groupF !== 'all' ? groupF : undefined; + + // Client-side paging over the filtered list (same pattern as DocList): + // clamps when a filter change shrinks the list under the current page. + const [page, setPage] = useState(0); + const pages = Math.max(1, Math.ceil(filtered.length / PAGE_SIZE)); + const cur = Math.min(page, pages - 1); + useEffect(() => { if (page !== cur) setPage(cur); }, [page, cur]); + const start = cur * PAGE_SIZE; + const paged = filtered.slice(start, start + PAGE_SIZE); + + return ( + <div className="page-body"> + <header className="page-head"> + <div className="crumbs"> + <button className="crumb crumb-link" onClick={() => navigate('/pipelines')}>Pipelines</button> + <span className="crumb-sep">/</span> + <span className="crumb is-last">TopN</span> + </div> + <h1 className="page-title">TopN</h1> + <p className="page-meta">Offline TopN statistics computed over measures across all groups.</p> + </header> + + <div className="res-toolbar topn-toolbar"> + <div className="topn-filters"> + <div className="search-box"> + <IconSearch size={15} /> + <input placeholder="Filter by name" value={nameQ} onChange={(e) => setNameQ(e.target.value)} /> + </div> + <FilterSelect label="Group" value={groupF} + onChange={(val) => { setGroupF(val); setSourceF('all'); }} + options={[{ value: 'all', label: 'All groups' }, ...measureGroupNames.map((n) => ({ value: n, label: n }))]} /> + <FilterSelect label="Source" value={sourceF} onChange={setSourceF} + options={[{ value: 'all', label: 'All measures' }, ...sourceOptions.map((n) => ({ value: n, label: n }))]} /> + <FilterSelect label="Rank" value={rankF} onChange={setRankF} + options={[{ value: 'all', label: 'Any direction' }, ...SORT_OPTS.map((s) => ({ value: s.value, label: s.rank }))]} /> + </div> + <div className="res-toolbar-right"> + <span className="res-count"> + {filtersActive ? `${filtered.length} of ${totalCount}` : `${totalCount} ${totalCount === 1 ? 'aggregation' : 'aggregations'}`} + </span> + {canWrite && ( + <button className="btn btn-primary" onClick={() => setModal({ kind: 'create', groupName: newTargetGroup })}> + <IconPlus size={16} /> New aggregation + </button> + )} + </div> + </div> + + {!loaded ? ( + <div className="empty"> + <span className="empty-ico spin"><IconTopN size={32} /></span> + <div className="empty-title">Loading…</div> + </div> + ) : totalCount === 0 ? ( + <div className="empty"> + <span className="empty-ico"><IconTopN size={36} /></span> + <div className="empty-title">No TopN aggregations yet</div> + <p className="empty-text">Define a TopNAggregation over a measure to pre-compute ranked statistics.</p> + {canWrite && ( + <button className="btn btn-primary" onClick={() => setModal({ kind: 'create', groupName: newTargetGroup })}> + <IconPlus size={15} /> Create aggregation + </button> + )} + </div> + ) : filtered.length === 0 ? ( + <div className="empty"> + <span className="empty-ico"><IconSearch size={36} /></span> + <div className="empty-title">No matches</div> + <p className="empty-text">No aggregation matches the current filters.</p> + </div> + ) : ( + <> + <div className="idx-table topn-x"> + <div className="topn-head"> + <span>Aggregation</span> + <span>Group</span> + <span>Source · field</span> + <span>Rank</span> + <span>Group by</span> + <span className="idx-actions-h" /> + </div> + {paged.map((r) => { + const a = r.agg; + const src = a.sourceMeasure; + const srcExists = !!src && (measureNames.get(src.group) ?? []).includes(src.name); + const path = `/pipelines/topn/${r.group}/${a.metadata.name}`; + return ( + <div key={`${r.group}/${a.metadata.name}`} className="topn-row" role="button" tabIndex={0} + data-testid="topn-row" + onClick={() => navigate(path)} + onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); navigate(path); } }}> + <span className="idx-name-cell"> Review Comment: The clickable row (`role="button"`) contains nested Edit/Delete `<button>`s. Clicks are stopped from bubbling, but the row-level `onKeyDown` will still fire when those inner buttons have focus (keyboard activation bubbles), causing unintended navigation on Space/Enter for keyboard users. ########## canopy/web/src/pipelines/TopNForms.tsx: ########## @@ -0,0 +1,582 @@ +/* + * Licensed to Apache Software Foundation (ASF) under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Apache Software Foundation (ASF) licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// TopNForms.tsx — TopNAggregation CRUD: create/edit modal + delete confirm. +// Ported from .handoff-import/banyandb/project/topn-form.jsx (window-global +// JSX -> ES module TSX). Validation mirrors schema.proto TopNAggregation +// (metadata, source_measure, field_name min_len 1, field_value_sort, +// group_by_tag_names, criteria, counters_number, lru_size). +// +// ADAPTATIONS FROM THE HANDOFF: +// - Mock `groups` prop -> live queries: listGroups (source group options), +// listResourcesInGroup('measures', ...) (source measure options), +// getResource (source measure's fields/tags), listTopNAggregations +// (per-group existing names, for the create-time uniqueness check). +// - The handoff's bespoke TopNSelect/TopNChipPicker are replaced by canopy's +// existing Combobox/MultiCombobox (Combobox.tsx) for the group/measure/ +// field/group-by pickers — same fuzzy-filter UX already used by +// IndexRuleBindingForm, without introducing a second select widget. +// - CriteriaEditor's tag(name)/op/value rows build/parse the real +// model.v1.Criteria tree via topn-shared.ts's buildTopNCriteria / +// flattenTopNCriteria instead of carrying a flat array over the wire. +// - onSubmit -> createTopNAggregation / updateTopNAggregation; reuses +// useFocusTrap/useDirtyGuard from components/modal-utils.ts, matching +// every other *Form.tsx in this app (there is no shared exported +// Field/Modal component to reuse — each form locally duplicates the +// modal-overlay/modal markup, per GroupForm.tsx/PropertyForms.tsx). + +import React, { useEffect, useMemo, useState } from 'react'; +import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query'; + +import type { + MeasureSchema, TopNAggregationSchema, + CreateTopNAggregationRequest, UpdateTopNAggregationRequest, +} from 'canopy-shared'; +import { apiDataSource } from '../data/api.js'; +import { useFocusTrap, useDirtyGuard } from '../components/modal-utils.js'; +import { Combobox, MultiCombobox } from '../components/Combobox.js'; +import { IconChevron, IconPlus, IconCheck } from '../components/icons.js'; +import { + SORT_OPTS, TOPN_OPS, DEFAULT_COUNTERS, topNTone, topNRank, + buildTopNCriteria, flattenTopNCriteria, type TopNCondition, +} from './topn-shared.js'; + +const TOPN_NAME_RE = /^[a-zA-Z0-9_-]+$/; + +const IconClose = (p: React.SVGProps<SVGSVGElement>) => ( + <svg {...p} viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="1.6" strokeLinecap="round" strokeLinejoin="round"> + <path d="M6 6l12 12M18 6 6 18" /> + </svg> +); + +// Mirrors the Field wrapper every *Form.tsx in this app duplicates locally. +function Field({ label, hint, error, required, locked, children }: { + label: React.ReactNode; + hint?: string; + error?: string; + required?: boolean; + locked?: boolean; + children: React.ReactNode; +}) { + return ( + <div className={`f-field${error ? ' has-error' : ''}`}> + <label className="f-label"> + {label} + {required && <span className="f-req">*</span>} + {locked && <span className="f-lock">read-only</span>} + </label> + {children} + {error ? <div className="f-error">{error}</div> : hint ? <div className="f-hint">{hint}</div> : null} + </div> + ); +} + +/* ============ criteria editor — flat ANDed {tag, op, value} rows ============ */ + +function CriteriaEditor({ criteria, tagOptions, errors, onChange }: { + criteria: readonly TopNCondition[]; + tagOptions: readonly string[]; + errors?: ReadonlyArray<{ tag?: string; value?: string } | undefined>; + onChange: (v: TopNCondition[]) => void; +}) { + const upd = (i: number, patch: Partial<TopNCondition>) => + onChange(criteria.map((c, idx) => (idx === i ? { ...c, ...patch } : c))); + const del = (i: number) => onChange(criteria.filter((_, idx) => idx !== i)); + const add = () => onChange([...criteria, { tag: tagOptions[0] ?? '', op: 'BINARY_OP_EQ', value: '' }]); + + return ( + <div className="spec-list"> + {criteria.map((c, i) => { + const er = errors?.[i] ?? {}; + return ( + <div key={i} className="kv-row"> + <div className={'spec-cell' + (er.tag ? ' has-error' : '')}> + <Combobox + value={c.tag} + options={tagOptions} + onChange={(val) => upd(i, { tag: val })} + placeholder="— tag —" + noOptionsHint="Source measure has no tags" + ariaLabel={`Criteria tag ${i + 1}`} + /> + {er.tag && <div className="f-error">{er.tag}</div>} + </div> + <div className="spec-cell type"> + <div className="f-select-wrap"> + <select className="f-input f-select mono" aria-label={`Criteria operator ${i + 1}`} value={c.op} + onChange={(e) => upd(i, { op: e.target.value })}> + {TOPN_OPS.map((o) => <option key={o.value} value={o.value}>{o.label}</option>)} + </select> + <span className="f-select-chev"><IconChevron size={13} /></span> + </div> + </div> + <div className={'spec-cell grow' + (er.value ? ' has-error' : '')}> + <input className="f-input mono" value={c.value} placeholder="value" aria-label={`Criteria value ${i + 1}`} + onChange={(e) => upd(i, { value: e.target.value })} /> + {er.value && <div className="f-error">{er.value}</div>} + </div> + <button type="button" className="spec-del" onClick={() => del(i)} aria-label={`Remove condition ${i + 1}`}> + <IconClose width={14} height={14} /> + </button> + </div> + ); + })} + <button type="button" className="spec-add" onClick={add}><IconPlus size={14} /> Add condition</button> + </div> + ); +} + +/* ============ validation ============ */ + +export interface TopNDraft { + readonly name: string; + readonly sourceGroup: string; + readonly sourceName: string; + readonly fieldName: string; + readonly fieldValueSort: 'SORT_DESC' | 'SORT_ASC' | 'SORT_UNSPECIFIED'; + readonly groupByTagNames: readonly string[]; + readonly criteria: readonly TopNCondition[]; + readonly countersNumber: number | ''; + readonly lruSize: number | ''; +} + +export interface TopNValidationErrors { + _?: string; + name?: string; + sourceGroup?: string; + sourceMeasure?: string; + fieldName?: string; + countersNumber?: string; + lruSize?: string; + criteria?: ReadonlyArray<{ tag?: string; value?: string } | undefined>; +} + +export interface TopNValidationCtx { + readonly mode: 'create' | 'edit'; + readonly existingNames?: ReadonlySet<string>; + readonly fieldOptions?: readonly string[]; +} + +/** Pure validation — advisory only; the registry is authoritative (mirrors + * web/src/validation.ts's Principle 5 for the M3 forms). */ +export function validateTopN(v: TopNDraft, ctx: TopNValidationCtx): TopNValidationErrors { + const e: TopNValidationErrors = {}; + if (ctx.mode === 'create') { + const name = v.name.trim(); + if (!name) e.name = 'Name is required'; + else if (name.length > 255) e.name = 'Must be 255 characters or fewer'; + else if (!TOPN_NAME_RE.test(name)) e.name = "Only letters, digits, '_' and '-' are allowed"; + else if (ctx.existingNames?.has(name.toLowerCase())) e.name = `An aggregation named "${name}" already exists in this group`; + } + + if (!v.sourceGroup) e.sourceGroup = 'Select a source group'; + if (!v.sourceName) e.sourceMeasure = 'Select a source measure'; + + const fieldName = v.fieldName.trim(); + if (!fieldName) e.fieldName = 'Required'; + else if (ctx.fieldOptions && ctx.fieldOptions.length > 0 && !ctx.fieldOptions.includes(fieldName)) { + e.fieldName = 'Must be a field of the source measure'; + } + + if (v.countersNumber !== '') { + const n = Number(v.countersNumber); + if (!Number.isInteger(n) || n <= 0) e.countersNumber = 'Must be a whole number greater than 0'; + } + if (v.lruSize !== '') { + const n = Number(v.lruSize); + if (!Number.isInteger(n) || n < 0) e.lruSize = 'Must be 0 or a positive whole number'; + } + + const condErrs: Array<{ tag?: string; value?: string } | undefined> = []; + v.criteria.forEach((c, i) => { + const ce: { tag?: string; value?: string } = {}; + if (!c.tag.trim()) ce.tag = 'Required'; + if (!c.value.trim()) ce.value = 'Required'; + if (Object.keys(ce).length) condErrs[i] = ce; + }); + if (condErrs.length) e.criteria = condErrs; + + return e; +} + +function topNHasErrors(e: TopNValidationErrors): boolean { + return (Object.keys(e) as Array<keyof TopNValidationErrors>).some((k) => { + const val = e[k]; + if (Array.isArray(val)) return val.some((x) => x && Object.keys(x).length > 0); + return !!val; + }); +} + +function blankTopN(sourceGroup: string): TopNDraft { + return { + name: '', + sourceGroup, + sourceName: '', + fieldName: '', + fieldValueSort: 'SORT_DESC', + groupByTagNames: [], + criteria: [], + countersNumber: DEFAULT_COUNTERS, + lruSize: 10, + }; +} + +function draftFromSchema(agg: TopNAggregationSchema): TopNDraft { + return { + name: agg.metadata.name, + sourceGroup: agg.sourceMeasure?.group ?? agg.metadata.group, + sourceName: agg.sourceMeasure?.name ?? '', + fieldName: agg.fieldName ?? '', + fieldValueSort: agg.fieldValueSort ?? 'SORT_DESC', + groupByTagNames: agg.groupByTagNames ?? [], + criteria: flattenTopNCriteria(agg.criteria), + countersNumber: agg.countersNumber ?? DEFAULT_COUNTERS, + lruSize: agg.lruSize ?? '', + }; +} + +/* ============ create / edit modal ============ */ + +export interface TopNFormModalProps { + readonly mode: 'create' | 'edit'; + /** Locks the target measure group. Required for edit; optional for create + * (when omitted the user picks a measure group in the Identity section — + * mirrors the handoff's "New aggregation" from the all-groups TopN list). */ + readonly groupName?: string; + readonly aggregation?: TopNAggregationSchema; + readonly onClose: (created?: TopNAggregationSchema) => void; +} + +export function TopNFormModal({ mode, groupName, aggregation, onClose }: TopNFormModalProps) { + const qc = useQueryClient(); + const isEdit = mode === 'edit'; + const fixedGroup = isEdit ? (aggregation?.metadata.group ?? groupName ?? null) : (groupName ?? null); + + const { data: groupsData } = useQuery({ queryKey: ['groups'], queryFn: () => apiDataSource.listGroups() }); + const measureGroupNames = useMemo( + () => (groupsData?.groups ?? []).filter((g) => g.catalog === 'CATALOG_MEASURE').map((g) => g.name).sort(), + [groupsData], + ); + + const [targetGroup, setTargetGroup] = useState(fixedGroup ?? ''); + useEffect(() => { + if (!fixedGroup && !targetGroup && measureGroupNames.length > 0) setTargetGroup(measureGroupNames[0]); + }, [fixedGroup, targetGroup, measureGroupNames]); + + const init = useMemo<TopNDraft>( + () => (isEdit && aggregation ? draftFromSchema(aggregation) : blankTopN(fixedGroup ?? '')), + // eslint-disable-next-line react-hooks/exhaustive-deps -- snapshot on mount only + [], + ); + const [v, setV] = useState<TopNDraft>(init); + const set = (patch: Partial<TopNDraft>) => setV((c) => ({ ...c, ...patch })); + + // Existing aggregation names in the target group, for the create-time + // uniqueness check (the registry itself is authoritative either way). + const { data: existingAggs = [] } = useQuery({ + queryKey: ['topNAggregations', targetGroup], + queryFn: () => apiDataSource.listTopNAggregations(targetGroup), + enabled: !!targetGroup, + }); + const existingNames = useMemo( + () => new Set( + existingAggs + .filter((a) => !isEdit || a.metadata.name !== aggregation?.metadata.name) + .map((a) => a.metadata.name.toLowerCase()), + ), + [existingAggs, isEdit, aggregation], + ); + + const { data: sourceMeasures = [] } = useQuery({ + queryKey: ['resources', 'measures', v.sourceGroup], + queryFn: () => apiDataSource.listResourcesInGroup('measures', v.sourceGroup), + enabled: !!v.sourceGroup, + }); + const measureOptions = useMemo(() => sourceMeasures.map((m) => m.metadata.name).sort(), [sourceMeasures]); + + const { data: srcMeasure } = useQuery({ + queryKey: ['resource', 'measures', v.sourceGroup, v.sourceName], + queryFn: () => apiDataSource.getResource('measures', v.sourceGroup, v.sourceName) as Promise<MeasureSchema>, + enabled: !!v.sourceGroup && !!v.sourceName, + }); + const fieldOptions = useMemo(() => srcMeasure?.fields.map((f) => f.name) ?? [], [srcMeasure]); + const tagOptions = useMemo( + () => srcMeasure?.tagFamilies.flatMap((f) => f.tags.map((t) => t.name)) ?? [], + [srcMeasure], + ); + + // When the source measure changes, drop a field/group-by selection that no + // longer applies to the new measure. + const changeSourceMeasure = (name: string) => { + set({ sourceName: name, fieldName: '', groupByTagNames: [] }); + }; + const changeSourceGroup = (group: string) => { + set({ sourceGroup: group, sourceName: '', fieldName: '', groupByTagNames: [] }); + }; + + const [errors, setErrors] = useState<TopNValidationErrors>({}); + const [submitted, setSubmitted] = useState(false); + useEffect(() => { + if (submitted) setErrors(validateTopN(v, { mode, existingNames, fieldOptions })); + // eslint-disable-next-line react-hooks/exhaustive-deps -- re-validate on relevant state, not on the ctx object identity + }, [v, submitted, existingNames, fieldOptions, mode]); + + const dirty = useMemo( + () => JSON.stringify(v) !== JSON.stringify(init) || targetGroup !== (fixedGroup ?? init.sourceGroup), + [v, init, targetGroup, fixedGroup], + ); + const { guardedClose, resetDirty } = useDirtyGuard(dirty, () => onClose()); + const trapRef = useFocusTrap(true, guardedClose); + + const createMut = useMutation({ + mutationFn: (req: CreateTopNAggregationRequest) => apiDataSource.createTopNAggregation(req), + onSuccess: (agg) => { + void qc.invalidateQueries({ queryKey: ['topNAggregations'] }); + resetDirty(); + onClose(agg); + }, + onError: (e: Error) => setErrors({ _: e.message }), + }); + const updateMut = useMutation({ + mutationFn: (req: UpdateTopNAggregationRequest) => apiDataSource.updateTopNAggregation(targetGroup, aggregation!.metadata.name, req), + onSuccess: (agg) => { + void qc.invalidateQueries({ queryKey: ['topNAggregations'] }); + void qc.invalidateQueries({ queryKey: ['topNAggregation', targetGroup, aggregation?.metadata.name] }); + resetDirty(); + onClose(agg); + }, + onError: (e: Error) => setErrors({ _: e.message }), + }); + const isPending = createMut.isPending || updateMut.isPending; + + const submit = () => { + const e = validateTopN(v, { mode, existingNames, fieldOptions }); + setSubmitted(true); + setErrors(e); + if (topNHasErrors(e)) { + requestAnimationFrame(() => { + const first = document.querySelector<HTMLElement>('.modal .has-error .f-input, .modal .has-error input'); + first?.focus(); + }); + return; + } + // Guard against zeroing group-by tags before the source measure's schema + // has finished loading (tagOptions starts empty until that query settles). + const validGroupBy = tagOptions.length > 0 ? v.groupByTagNames.filter((t) => tagOptions.includes(t)) : v.groupByTagNames; + const payload = { + metadata: { name: (isEdit ? aggregation!.metadata.name : v.name.trim()), group: targetGroup }, + sourceMeasure: { group: v.sourceGroup, name: v.sourceName }, + fieldName: v.fieldName.trim(), Review Comment: `metadata.group` (aggregation registry scope) and `sourceMeasure.group` can diverge: the payload uses `group: targetGroup` but `sourceMeasure.group: v.sourceGroup`. If TopNAggregations must rank measures within the same group they are registered in, this allows constructing requests the server will reject (and it’s hard for users to diagnose). Consider blocking submit with a clear validation error when the groups differ (or auto-sync the source group to the target group). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
