Fix/autoreload#31
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThe development pipeline has been restructured to watch the dev task instead of build for Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
091970b to
4ab0bf0
Compare
4ab0bf0 to
29df877
Compare
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/app/src/server.ts (1)
30-43:⚠️ Potential issue | 🟡 MinorFix UI route matching for hide logic.
uiRoutes.includes(url)only matches exact strings, so/dashboard/xyz(and other subpaths) won’t be hidden. Also,/exampleand/observabilityroutes are missing. Consider prefix-based matching instead.🛠️ Proposed fix
-const uiRoutes = ["/", "/dashboard", "/dashboard/*", "/api/generate"]; +const uiExactRoutes = ["/", "/api/generate"]; +const uiRoutePrefixes = ["/dashboard", "/example", "/observability"]; ... - transform: ({ schema, url, ...rest }) => { - if (uiRoutes.includes(url)) return { schema: { hide: true }, url }; + transform: ({ schema, url, ...rest }) => { + const isUiRoute = + uiExactRoutes.includes(url) || + uiRoutePrefixes.some( + (prefix) => url === prefix || url.startsWith(`${prefix}/`) + ); + if (isUiRoute) return { schema: { hide: true }, url }; return jsonSchemaTransform({ schema, url, ...rest }); },
🤖 Fix all issues with AI agents
In `@packages/sqlite-datasource/src/dashboard-datasource.ts`:
- Around line 271-330: The restoreVersion function has a TOCTOU risk because it
runs two SELECTs (rawDashboard, rawTree) then an UPDATE (updateQ) without a
transaction; wrap the verification SELECTs and the UPDATE in a single DB
transaction so the dashboard cannot be deleted/changed between checks and
update: begin a transaction before running the selectFrom("dashboards") /
selectFrom("dashboard_ui_trees") queries, perform the same parsing/NotFound
checks on rawDashboard and rawTree inside the transaction, execute the
updateTable("dashboards") update inside it, and commit; ensure you rollback on
any error so partial changes are not persisted and re-throw the original error.
In `@packages/ui/src/client/index.html`:
- Around line 1-3: The root HTML element (<html>) is missing a language
attribute; update the <html> tag in the document to include an appropriate lang
value (e.g., lang="en" or the project's primary language) so screen readers and
search engines can correctly interpret the page language, ensuring the change is
applied in the top-level HTML markup where the <html> element is defined.
In `@packages/ui/src/components/button.tsx`:
- Around line 4-61: The Button component is only console.logging
element.props.action instead of invoking or dispatching it; replace the onClick
handler in Button (and add the missing handler in Empty) so clicks actually
perform the action: if element.props.action is a function invoke it
(element.props.action(...)), otherwise forward it to your app's action
dispatcher (e.g., call a provided dispatchAction or useAppDispatch helper) and
guard with disabled (no-op when disabled). Update the onClick logic in Button
and add the analogous onClick in Empty to use the same dispatch/invoke flow,
referencing Button, Empty, element.props.action and the onClick handlers.
In `@packages/ui/src/components/card.tsx`:
- Around line 14-18: The paddings map (paddings) is missing the "md" key which
the catalog schema allows, causing padding="md" to fall back to the default
"16px"; add an explicit "md" entry (e.g., "16px") to the paddings
Record<string,string> so the component handles the catalog values consistently
and remove reliance on the implicit default for "md".
In `@packages/ui/src/components/date-picker.tsx`:
- Around line 1-29: The label isn't associated with the input in the DatePicker
component; generate a stable id using React's useId() inside DatePicker and
apply it to the input's id and the label's htmlFor, and also set an aria-label
on the input (using label or a fallback) so assistive tech can still read it
when the visible label changes; update references near DatePicker,
element.props, label, and bindPath to wire the id properly.
In `@packages/ui/src/components/empty.tsx`:
- Around line 4-33: The rendered button in the Empty component is missing an
onClick handler; update the Empty function to wire the button's onClick to
dispatch/log the action (use the same pattern as the Button component) by
referencing element.props.action and element.props.actionLabel and calling the
handler when clicked (for example, onClick that logs "Action:" with the action
payload). Ensure you add the handler on the button element inside the Empty
component so the action is invoked/visible when clicked.
In `@packages/ui/src/components/metric.tsx`:
- Around line 16-30: The metric component renders a standalone "+" or "−" when
trend is present but trendValue is empty; update the render guard in the Metric
component so the trend span is only output when a real numeric/string value
exists (e.g., check that trendValue is non-null/non-empty) or provide a fallback
value before rendering; target the JSX around the (trend || trendValue) check
and change it to require trendValue (or a formatted fallback) so you don't
render just the symbol.
In `@packages/ui/src/components/otel-table.tsx`:
- Around line 9-18: ColumnFormat in otel-table.tsx contains "currency" and
"date" which are not present in the catalog's Table.props.columns.format enum,
causing Zod validation failures; fix by either removing "currency" and "date"
from the ColumnFormat union in otel-table.tsx (so ColumnFormat only includes
"text","timestamp","duration","truncate","badge","json",null) or by adding
"currency" and "date" to the catalog enum (Table.props.columns.format) in the
observability catalog schema so both definitions match; pick one approach and
update all usages of ColumnFormat/Table.props.columns.format to keep types in
sync.
In `@packages/ui/src/lib/component-catalog.ts`:
- Around line 108-139: createCatalog currently builds elementVariants and then
calls z.discriminatedUnion("type", ...) which crashes if elementVariants is
empty; add a guard before creating elementsUnion that checks if
elementVariants.length === 0 and throws a clear, deterministic error (or
returns/handles an empty catalog) so z.discriminatedUnion is never called with
an empty array. Update the logic around elementVariants / elementsUnion in
createCatalog to perform this check and surface a helpful message referencing
the catalog name (catalogConfig.name) and the function createCatalog.
- Around line 1-40: The componentDefinitionSchema currently allows props:
z.unknown(), which lets non-Zod values through and later causes createCatalog to
crash when it calls z.object(component.props); update componentDefinitionSchema
to require that props be a Zod schema (e.g., enforce instanceof ZodType) so
validation fails at parse time; specifically change the props definition used in
componentDefinitionSchema and ensure createCatalog (which accesses
component.props) can safely pass component.props into z.object() without runtime
errors.
In `@packages/ui/src/pages/dashboard.tsx`:
- Line 406: Replace the hardcoded baseUrl in the KopaiClient instantiation (the
line creating new KopaiClient with "http://localhost:8000/signals") with a
configurable environment-backed value (e.g. read from
process.env.NEXT_PUBLIC_SIGNALS_URL or a similar app-level config) and provide a
sensible default fallback for local dev; update the instantiation in
dashboard.tsx (where KopaiClient is constructed) to use that env/config value
and add a small runtime check that logs or throws a clear error if the value is
missing in non-dev environments.
- Around line 95-99: The useEffect is retriggering because refetchFn (from
props.refetch) is unstable; make the refetch call use a stable ref instead:
create a ref (e.g., refetchRef) that you update whenever props.refetch changes,
and in the effect that depends on currentLimit call refetchRef.current({ limit:
currentLimit }) so the effect only depends on currentLimit. Apply the same
pattern to LogsTable and MetricsTable for their refetchFn usages (update the ref
in a small effect when props.refetch/refetchFn changes, and invoke ref.current
inside the limit effect) to prevent the infinite re-fetch loop.
In `@packages/ui/src/pages/observability.tsx`:
- Around line 222-223: The KopaiClient is created at module scope with a
hardcoded baseUrl ("http://localhost:8000/signals"); change this to read from
configuration and/or accept it via props so non-local environments work: replace
the module-level instantiation of client (KopaiClient) with a factory that uses
an environment variable (e.g., process.env.NEXT_PUBLIC_SIGNALS_URL) or move
client creation into the ObservabilityPage component and accept a client or
baseUrl prop (ObservabilityPage) so tests and deployments can inject the correct
URL; ensure fallback/default behavior and update any callers to pass the client
or URL.
🧹 Nitpick comments (23)
packages/ui/README.md (2)
36-36: Consider using.optional()instead of.nullable()for optional props.The schema uses
z.string().nullable(), which requires the field to be present but allowsnullas a value. For optional React props that may not be provided,z.string().optional()orz.string().nullish()is more idiomatic and aligns with TypeScript's optional property semantics.This affects the example at line 57 where the code checks for the prop's presence.
📝 Suggested improvement
props: z.object({ - title: z.string().nullable(), + title: z.string().optional(), }),
1-137: Consider adding installation and setup sections.The README would benefit from:
- Installation instructions (
npm install@kopai/ui``)- Peer dependencies (React, Zod, etc.)
- Basic TypeScript configuration requirements
These additions would help new users get started more quickly.
packages/sqlite-datasource/package.json (1)
39-39: Consider aligning Zod version with root package.json.The root
package.jsonspecifies"zod": "^4.3.6"while this package uses"zod": "^4.0.0". While semver ranges may resolve compatibly, explicit version inconsistency across the monorepo can lead to confusion or unexpected behavior.Consider using the same version constraint as the root:
♻️ Suggested fix
- "zod": "^4.0.0" + "zod": "^4.3.6"packages/ui/src/components/text.tsx (1)
4-17: Hoist static color map to avoid per-render allocations.
Minor perf/clarity improvement.♻️ Suggested refactor
+const TEXT_COLORS: Record<string, string> = { + default: "var(--foreground)", + muted: "var(--muted)", + success: "#22c55e", + warning: "#eab308", + danger: "#ef4444", +}; + export function Text({ element, }: CatalogueComponentProps<typeof dashboardCatalog.components.Text>) { const { content, color } = element.props; - const colors: Record<string, string> = { - default: "var(--foreground)", - muted: "var(--muted)", - success: "#22c55e", - warning: "#eab308", - danger: "#ef4444", - }; return ( - <p style={{ margin: 0, color: colors[color || "default"] }}>{content}</p> + <p style={{ margin: 0, color: TEXT_COLORS[color || "default"] }}> + {content} + </p> ); }packages/ui/src/components/heading.tsx (1)
5-25: Hoist static size map to avoid per-render allocations.
Small cleanup for readability/perf.♻️ Suggested refactor
+const HEADING_SIZES: Record<string, string> = { + h1: "28px", + h2: "24px", + h3: "20px", + h4: "16px", +}; + export function Heading({ element, }: CatalogueComponentProps<typeof dashboardCatalog.components.Heading>) { const { text, level } = element.props; const Tag = (level || "h2") as keyof React.JSX.IntrinsicElements; - const sizes: Record<string, string> = { - h1: "28px", - h2: "24px", - h3: "20px", - h4: "16px", - }; return ( <Tag style={{ margin: "0 0 16px", - fontSize: sizes[level || "h2"], + fontSize: HEADING_SIZES[level || "h2"], fontWeight: 600, }} > {text} </Tag>packages/ui/src/components/stack.tsx (1)
4-31: Hoist static maps for gap/alignment to reduce per-render churn.
Minor cleanup only.♻️ Suggested refactor
+const STACK_GAPS: Record<string, string> = { + sm: "8px", + md: "16px", + lg: "24px", +}; +const STACK_ALIGNMENTS: Record<string, string> = { + start: "flex-start", + center: "center", + end: "flex-end", + stretch: "stretch", +}; + export function Stack({ element, children, }: CatalogueComponentProps<typeof dashboardCatalog.components.Stack>) { const { direction, gap, align } = element.props; - const gaps: Record<string, string> = { - sm: "8px", - md: "16px", - lg: "24px", - }; - const alignments: Record<string, string> = { - start: "flex-start", - center: "center", - end: "flex-end", - stretch: "stretch", - }; return ( <div style={{ display: "flex", flexDirection: direction === "horizontal" ? "row" : "column", - gap: gaps[gap || "md"], - alignItems: alignments[align || "stretch"], + gap: STACK_GAPS[gap || "md"], + alignItems: STACK_ALIGNMENTS[align || "stretch"], }} >packages/ui/src/components/table.tsx (1)
74-76: Prefer stable row keys over array indexes.Using the array index can cause DOM churn when rows are re-ordered or updated. The mock rows already have
id, so use it as the primary key (with a fallback if needed).♻️ Suggested change
- {mockData.map((row, i) => ( - <tr key={i}> + {mockData.map((row, i) => ( + <tr key={row.id ?? i}>packages/ui/src/components/chart.tsx (1)
27-33: Use nullish coalescing for the height default.
height || 120overrides an explicit0.??preserves valid zero values while still defaulting fornull/undefined.♻️ Suggested change
- height: height || 120, + height: height ?? 120,packages/ui/src/pages/example.tsx (1)
35-559: Type the example tree against the catalog schema.This catches key/prop mismatches at compile time and keeps the large tree safer to evolve.
♻️ Suggested change
import { dashboardCatalog } from "../lib/catalog.js"; +import type { z } from "zod"; @@ -// UITree showcasing all components -const exampleTree = { +type ExampleTree = z.infer<typeof dashboardCatalog.uiTreeSchema>; + +// UITree showcasing all components +const exampleTree = { @@ -}; +} satisfies ExampleTree;packages/ui/src/lib/renderer.test.tsx (1)
334-366: Wrap manual promise resolution inact.This avoids React state update warnings when resolving async data during tests.
♻️ Suggested change
- resolvePromise!({ data: [] }); - await waitFor(() => { + await act(async () => { + resolvePromise!({ data: [] }); + }); + await waitFor(() => {packages/ui/src/components/otel-chart.tsx (1)
28-38: Non-deterministic skeleton heights may cause visual jitter.Using
Math.random()during render generates different bar heights on each re-render (e.g., when parent state changes). Consider using deterministic pseudo-random values seeded by the index.♻️ Proposed fix using deterministic heights
{Array.from({ length: 7 }).map((_, i) => ( <div key={i} style={{ flex: 1, - height: `${30 + Math.random() * 60}%`, + height: `${30 + ((i * 17 + 13) % 60)}%`, background: "var(--border)", borderRadius: "4px 4px 0 0", opacity: 0.5, }} /> ))}packages/ui/src/components/otel-list.tsx (1)
27-41: Consider using CSS variables for error colors.The ErrorDisplay component hardcodes colors (
#fef2f2,#fecaca,#dc2626) while LoadingSkeleton uses CSS variables (var(--border)). For theming consistency, consider using CSS variables likevar(--danger)or similar.♻️ Suggested refactor
function ErrorDisplay({ error }: { error: Error }) { return ( <div style={{ padding: 16, - background: "#fef2f2", - border: "1px solid `#fecaca`", + background: "var(--danger-bg, `#fef2f2`)", + border: "1px solid var(--danger-border, `#fecaca`)", borderRadius: 8, - color: "#dc2626", + color: "var(--danger, `#dc2626`)", }} > <strong>Error:</strong> {error.message} </div> ); }packages/ui/src/components/otel-metric.tsx (2)
18-29: Use proper microseconds symbol.The microseconds unit is displayed as "us" but the standard notation is "µs" (Greek mu). This is a minor display improvement for correctness.
♻️ Suggested fix
if (ns >= 1_000) { - return `${(ns / 1_000).toFixed(2)}us`; + return `${(ns / 1_000).toFixed(2)}µs`; }
55-62: Consider using CSS variables for error color.Similar to the feedback on
otel-list.tsx, the ErrorDisplay here uses a hardcoded color (#ef4444) while other components use CSS variables. Consider usingvar(--danger)or similar for theming consistency across the codebase.packages/ui/src/lib/generate-prompt-instructions.ts (3)
92-102: Fragile detection of dataSource schema.The
isDataSourceSchemafunction specifically checks formethod?.const === "searchTracesPage"to identify the dataSource schema. This approach is fragile and will break if the dataSourceSchema definition changes or ifsearchTracesPageis no longer the first method in the discriminated union.Consider a more robust detection, such as checking for the presence of multiple expected method constants or using a schema marker.
♻️ Suggested improvement
function isDataSourceSchema(value: unknown): boolean { if (!value || typeof value !== "object") return false; const obj = value as Record<string, unknown>; if (!Array.isArray(obj.oneOf)) return false; - const first = obj.oneOf[0] as Record<string, unknown> | undefined; - if (!first?.properties) return false; - const props = first.properties as Record<string, unknown>; - const method = props.method as Record<string, unknown> | undefined; - return method?.const === "searchTracesPage"; + // Check if any variant has a method property with known dataSource methods + const knownMethods = ["searchTracesPage", "searchLogsPage", "searchMetricsPage", "getTrace", "discoverMetrics"]; + return obj.oneOf.some((variant) => { + const v = variant as Record<string, unknown>; + const props = v.properties as Record<string, unknown> | undefined; + const method = props?.method as Record<string, unknown> | undefined; + return method?.const && knownMethods.includes(method.const as string); + }); }
151-153: Role line logic may be oversimplified.The current logic assumes components either accept children OR use dataSource, but never both or neither. According to the catalog, some components like
Table,Metric, andCharthavehasChildren: falseand use dataSource, while layout components likeCard,Grid,StackhavehasChildren: true.However, the
Listcomponent hashasChildren: trueAND uses dataSource (it iterates over dataSource array and renders children for each item). The current role line would say "Accepts children: yes" but not mention dataSource capability.Consider clarifying or revising this logic if both capabilities can coexist.
170-184: Consider formatting JSON for readability.The schema and example JSON are stringified without formatting, which may be difficult to read in the generated prompt. Consider using
JSON.stringify(obj, null, 2)for better readability.♻️ Suggested improvement
## Output Schema -${JSON.stringify(unifiedSchema)} +\`\`\`json +${JSON.stringify(unifiedSchema, null, 2)} +\`\`\` --- ## Example -${JSON.stringify(exampleElements)}`; +\`\`\`json +${JSON.stringify(exampleElements, null, 2)} +\`\`\``; }packages/ui/src/hooks/use-kopai-data.ts (1)
92-110: StaleparamsOverrideRefpersists across dataSource changes.When
dataSourcechanges, theparamsOverrideRef.currentfrom a previousrefetch()call is still applied to the new fetch (line 105). This could cause unexpected behavior if the previous params aren't valid for the new dataSource.Consider clearing the override when the dataSource changes:
♻️ Proposed fix
useEffect(() => { if (!dataSource) { setData(null); setLoading(false); setError(null); return; } // Cancel previous request abortControllerRef.current?.abort(); const controller = new AbortController(); abortControllerRef.current = controller; + // Clear stale params override from previous dataSource + paramsOverrideRef.current = undefined; + fetchData(controller.signal, paramsOverrideRef.current ?? {}); return () => { controller.abort(); }; }, [dataSource, fetchData]);packages/ui/src/components/otel-table.tsx (1)
28-40:formatDurationmay produceNaNfor non-numeric strings.
parseInt(ns, 10)returnsNaNfor non-numeric strings, which would produce output like"NaNns".🛡️ Proposed defensive check
function formatDuration(ns: number | string): string { const num = typeof ns === "string" ? parseInt(ns, 10) : ns; + if (Number.isNaN(num)) return "-"; if (num >= 1_000_000_000) { return `${(num / 1_000_000_000).toFixed(2)}s`; }packages/ui/src/pages/dashboard.tsx (1)
39-55: Duplicated formatting helpers.
formatTimestamp,formatDuration, andformatAttrsduplicate similar logic fromotel-table.tsx(with slightly different implementations). Consider extracting shared formatters to a common utility module.packages/ui/src/lib/dashboard-datasource.ts (1)
4-4:UiTreetype is used but not exported.The
UiTreetype is used in method signatures (e.g.,createDashboard,updateDashboard,getDashboard) but isn't exported. Consumers implementing these interfaces will need this type.♻️ Proposed fix
-type UiTree = z.infer<typeof observabilityCatalog.uiTreeSchema>; +export type UiTree = z.infer<typeof observabilityCatalog.uiTreeSchema>;packages/ui/src/lib/renderer.tsx (1)
189-201: Child element lookup may silently skip missing children.When
tree.elements[childKey]returnsundefined(line 190-191), the child is silently skipped. This could mask configuration errors in the UI tree.Consider adding a warning similar to the missing component handler:
🔍 Proposed enhancement
const children = element.children?.map((childKey) => { const childElement = tree.elements[childKey]; - if (!childElement) return null; + if (!childElement) { + console.warn(`Missing child element in tree: ${childKey}`); + return null; + } return (packages/sqlite-datasource/src/dashboard-datasource.ts (1)
466-521: Consider whether versions of deleted dashboards should be accessible.
listVersionsandgetDashboardAtVersiondon't check if the parent dashboard is deleted. This allows fetching version history for soft-deleted dashboards.If this is intentional for auditing/recovery purposes, the behavior is fine. If not, consider adding a join or check against the dashboard's
deletedflag to maintain consistency with other read methods that exclude deleted dashboards.
| restoreVersion(dashboardId: DashboardId, uiTreeId: UiTreeId): DashboardMeta { | ||
| const now = new Date().toISOString(); | ||
|
|
||
| // Verify dashboard exists and not deleted | ||
| const selectDashboard = queryBuilder | ||
| .selectFrom("dashboards") | ||
| .select(["dashboard_id", "name", "owner_id", "pinned", "created_at"]) | ||
| .where("dashboard_id", "=", dashboardId) | ||
| .where("deleted", "=", 0) | ||
| .compile(); | ||
| const rawDashboard = this.sqliteConnection | ||
| .prepare(selectDashboard.sql) | ||
| .get(...(selectDashboard.parameters as string[])); | ||
|
|
||
| if (!rawDashboard) { | ||
| throw new SqliteDatasourceNotFoundError( | ||
| `Dashboard not found: ${dashboardId}` | ||
| ); | ||
| } | ||
| const dashboard = dashboardMetaRowSchema.parse(rawDashboard); | ||
|
|
||
| // Verify ui_tree belongs to this dashboard | ||
| const selectTree = queryBuilder | ||
| .selectFrom("dashboard_ui_trees") | ||
| .select(["ui_tree_id"]) | ||
| .where("ui_tree_id", "=", uiTreeId) | ||
| .where("dashboard_id", "=", dashboardId) | ||
| .compile(); | ||
| const rawTree = this.sqliteConnection | ||
| .prepare(selectTree.sql) | ||
| .get(...(selectTree.parameters as string[])); | ||
|
|
||
| if (!rawTree) { | ||
| throw new SqliteDatasourceNotFoundError(`Version not found: ${uiTreeId}`); | ||
| } | ||
| uiTreeIdRowSchema.parse(rawTree); | ||
|
|
||
| // Point dashboard to the existing ui_tree | ||
| const updateQ = queryBuilder | ||
| .updateTable("dashboards") | ||
| .set({ | ||
| current_ui_tree_id: uiTreeId, | ||
| updated_at: now, | ||
| }) | ||
| .where("dashboard_id", "=", dashboardId) | ||
| .compile(); | ||
| this.sqliteConnection | ||
| .prepare(updateQ.sql) | ||
| .run(...(updateQ.parameters as string[])); | ||
|
|
||
| return { | ||
| dashboardId, | ||
| uiTreeId, | ||
| name: dashboard.name, | ||
| ownerId: dashboard.owner_id, | ||
| pinned: dashboard.pinned === 1, | ||
| createdAt: dashboard.created_at, | ||
| updatedAt: now, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Missing transaction wrapper creates TOCTOU race condition.
The restoreVersion method performs two SELECT queries followed by an UPDATE without transaction protection. Between verifying the dashboard exists (line 281-283) and updating it (line 317-319), another request could delete the dashboard, leading to inconsistent state.
🔒 Proposed fix: Wrap operations in a transaction
restoreVersion(dashboardId: DashboardId, uiTreeId: UiTreeId): DashboardMeta {
const now = new Date().toISOString();
+ this.sqliteConnection.exec("BEGIN");
+ try {
// Verify dashboard exists and not deleted
const selectDashboard = queryBuilder
.selectFrom("dashboards")
.select(["dashboard_id", "name", "owner_id", "pinned", "created_at"])
.where("dashboard_id", "=", dashboardId)
.where("deleted", "=", 0)
.compile();
const rawDashboard = this.sqliteConnection
.prepare(selectDashboard.sql)
.get(...(selectDashboard.parameters as string[]));
if (!rawDashboard) {
throw new SqliteDatasourceNotFoundError(
`Dashboard not found: ${dashboardId}`
);
}
const dashboard = dashboardMetaRowSchema.parse(rawDashboard);
// Verify ui_tree belongs to this dashboard
const selectTree = queryBuilder
.selectFrom("dashboard_ui_trees")
.select(["ui_tree_id"])
.where("ui_tree_id", "=", uiTreeId)
.where("dashboard_id", "=", dashboardId)
.compile();
const rawTree = this.sqliteConnection
.prepare(selectTree.sql)
.get(...(selectTree.parameters as string[]));
if (!rawTree) {
throw new SqliteDatasourceNotFoundError(`Version not found: ${uiTreeId}`);
}
uiTreeIdRowSchema.parse(rawTree);
// Point dashboard to the existing ui_tree
const updateQ = queryBuilder
.updateTable("dashboards")
.set({
current_ui_tree_id: uiTreeId,
updated_at: now,
})
.where("dashboard_id", "=", dashboardId)
.compile();
this.sqliteConnection
.prepare(updateQ.sql)
.run(...(updateQ.parameters as string[]));
+ this.sqliteConnection.exec("COMMIT");
+
return {
dashboardId,
uiTreeId,
name: dashboard.name,
ownerId: dashboard.owner_id,
pinned: dashboard.pinned === 1,
createdAt: dashboard.created_at,
updatedAt: now,
};
+ } catch (error) {
+ this.sqliteConnection.exec("ROLLBACK");
+ throw error;
+ }
}🤖 Prompt for AI Agents
In `@packages/sqlite-datasource/src/dashboard-datasource.ts` around lines 271 -
330, The restoreVersion function has a TOCTOU risk because it runs two SELECTs
(rawDashboard, rawTree) then an UPDATE (updateQ) without a transaction; wrap the
verification SELECTs and the UPDATE in a single DB transaction so the dashboard
cannot be deleted/changed between checks and update: begin a transaction before
running the selectFrom("dashboards") / selectFrom("dashboard_ui_trees") queries,
perform the same parsing/NotFound checks on rawDashboard and rawTree inside the
transaction, execute the updateTable("dashboards") update inside it, and commit;
ensure you rollback on any error so partial changes are not persisted and
re-throw the original error.
| <!doctype html> | ||
| <html> | ||
| <head> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -path "*/packages/ui/src/client/index.html" -type fRepository: kopai-app/kopai-mono
Length of output: 99
🏁 Script executed:
head -n 10 packages/ui/src/client/index.htmlRepository: kopai-app/kopai-mono
Length of output: 324
Add a lang attribute to improve accessibility and SEO.
The <html> element is missing a language attribute, which helps screen readers and search engines properly handle the content.
🔧 Suggested change
-<html>
+<html lang="en">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <!doctype html> | |
| <html> | |
| <head> | |
| <!doctype html> | |
| <html lang="en"> | |
| <head> |
🤖 Prompt for AI Agents
In `@packages/ui/src/client/index.html` around lines 1 - 3, The root HTML element
(<html>) is missing a language attribute; update the <html> tag in the document
to include an appropriate lang value (e.g., lang="en" or the project's primary
language) so screen readers and search engines can correctly interpret the page
language, ensuring the change is applied in the top-level HTML markup where the
<html> element is defined.
| export function Button({ | ||
| element, | ||
| }: CatalogueComponentProps<typeof dashboardCatalog.components.Button>) { | ||
| const { label, variant, size, action, disabled } = element.props; | ||
|
|
||
| const variants: Record< | ||
| string, | ||
| { bg: string; color: string; border: string } | ||
| > = { | ||
| primary: { | ||
| bg: "var(--foreground)", | ||
| color: "var(--background)", | ||
| border: "var(--foreground)", | ||
| }, | ||
| secondary: { | ||
| bg: "var(--card)", | ||
| color: "var(--foreground)", | ||
| border: "var(--border)", | ||
| }, | ||
| danger: { | ||
| bg: "#ef4444", | ||
| color: "white", | ||
| border: "#ef4444", | ||
| }, | ||
| ghost: { | ||
| bg: "transparent", | ||
| color: "var(--foreground)", | ||
| border: "transparent", | ||
| }, | ||
| }; | ||
|
|
||
| const sizes: Record<string, { padding: string; fontSize: number }> = { | ||
| sm: { padding: "6px 12px", fontSize: 12 }, | ||
| md: { padding: "8px 16px", fontSize: 14 }, | ||
| lg: { padding: "12px 24px", fontSize: 16 }, | ||
| }; | ||
|
|
||
| const v = variants[variant || "primary"]!; | ||
| const s = sizes[size || "md"]!; | ||
|
|
||
| return ( | ||
| <button | ||
| disabled={disabled || false} | ||
| onClick={() => console.log("Action:", action)} | ||
| style={{ | ||
| padding: s.padding, | ||
| fontSize: s.fontSize, | ||
| fontWeight: 500, | ||
| borderRadius: "var(--radius)", | ||
| border: `1px solid ${v.border}`, | ||
| background: v.bg, | ||
| color: v.color, | ||
| cursor: disabled ? "not-allowed" : "pointer", | ||
| opacity: disabled ? 0.5 : 1, | ||
| }} | ||
| > | ||
| {label} | ||
| </button> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for existing action dispatchers or handlers to wire into Button
rg -n --type=tsx -C 3 '\b(action|onAction|dispatchAction|handleAction)\b' packages/ui/srcRepository: kopai-app/kopai-mono
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Search for action-related patterns in TypeScript files
rg -n --type=ts -C 3 'action|onAction|dispatchAction|handleAction' packages/ui/srcRepository: kopai-app/kopai-mono
Length of output: 6893
🏁 Script executed:
#!/bin/bash
# Search for dispatch, reducer, and context patterns that might handle actions
rg -n --type=ts 'dispatch|reducer|useAction|ActionContext|actionHandler' packages/ui/srcRepository: kopai-app/kopai-mono
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Search for Button component usage and TODO comments about actions
rg -n --type=ts 'Button|TODO|FIXME' packages/ui/src -A 2 | grep -A 2 -B 2 'action\|TODO\|FIXME'Repository: kopai-app/kopai-mono
Length of output: 801
🏁 Script executed:
#!/bin/bash
# View the Empty component to see if it handles action differently
cat -n packages/ui/src/components/empty.tsxRepository: kopai-app/kopai-mono
Length of output: 1368
🏁 Script executed:
#!/bin/bash
# Search for component rendering and event handling patterns
rg -n --type=ts 'onClick|onAction|handleAction' packages/ui/src -B 2 -A 2Repository: kopai-app/kopai-mono
Length of output: 1545
Action prop is not dispatched—only logged.
The Button component accepts an action prop defined in the catalog as "Clickable button with action" but only logs it to the console (line 47). This makes the action functionally inert. Wire the action to your app's action dispatcher, reducer, or accept a callback prop instead of console.log.
Note: The Empty component has the same issue—its button renders when action is present but has no onClick handler at all, leaving the action completely unused.
🤖 Prompt for AI Agents
In `@packages/ui/src/components/button.tsx` around lines 4 - 61, The Button
component is only console.logging element.props.action instead of invoking or
dispatching it; replace the onClick handler in Button (and add the missing
handler in Empty) so clicks actually perform the action: if element.props.action
is a function invoke it (element.props.action(...)), otherwise forward it to
your app's action dispatcher (e.g., call a provided dispatchAction or
useAppDispatch helper) and guard with disabled (no-op when disabled). Update the
onClick logic in Button and add the analogous onClick in Empty to use the same
dispatch/invoke flow, referencing Button, Empty, element.props.action and the
onClick handlers.
| const paddings: Record<string, string> = { | ||
| none: "0", | ||
| sm: "12px", | ||
| lg: "24px", | ||
| }; |
There was a problem hiding this comment.
Missing "md" padding value from the catalog schema.
The catalog defines padding as z.enum(["sm", "md", "lg"]).nullable() but the paddings map only includes none, sm, and lg. When padding="md" is passed, it falls through to the default "16px". Consider adding md explicitly for clarity and consistency with the catalog.
🐛 Proposed fix
const paddings: Record<string, string> = {
none: "0",
sm: "12px",
+ md: "16px",
lg: "24px",
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const paddings: Record<string, string> = { | |
| none: "0", | |
| sm: "12px", | |
| lg: "24px", | |
| }; | |
| const paddings: Record<string, string> = { | |
| none: "0", | |
| sm: "12px", | |
| md: "16px", | |
| lg: "24px", | |
| }; |
🤖 Prompt for AI Agents
In `@packages/ui/src/components/card.tsx` around lines 14 - 18, The paddings map
(paddings) is missing the "md" key which the catalog schema allows, causing
padding="md" to fall back to the default "16px"; add an explicit "md" entry
(e.g., "16px") to the paddings Record<string,string> so the component handles
the catalog values consistently and remove reliance on the implicit default for
"md".
| import { dashboardCatalog } from "../lib/catalog.js"; | ||
| import type { CatalogueComponentProps } from "../lib/component-catalog.js"; | ||
|
|
||
| export function DatePicker({ | ||
| element, | ||
| }: CatalogueComponentProps<typeof dashboardCatalog.components.DatePicker>) { | ||
| const { label, bindPath, placeholder } = element.props; | ||
|
|
||
| return ( | ||
| <div style={{ display: "flex", flexDirection: "column", gap: 4 }}> | ||
| {label && ( | ||
| <label style={{ fontSize: 14, fontWeight: 500 }}>{label}</label> | ||
| )} | ||
| <input | ||
| type="date" | ||
| placeholder={placeholder || undefined} | ||
| style={{ | ||
| padding: "8px 12px", | ||
| borderRadius: "var(--radius)", | ||
| border: "1px solid var(--border)", | ||
| background: "var(--card)", | ||
| color: "var(--foreground)", | ||
| fontSize: 16, | ||
| outline: "none", | ||
| }} | ||
| /> | ||
| <p style={{ margin: "4px 0 0", fontSize: 12, color: "var(--muted)" }}> | ||
| Bound to: {bindPath} | ||
| </p> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and examine its content
find . -path "*packages/ui/src/components/date-picker.tsx" -type f | head -5Repository: kopai-app/kopai-mono
Length of output: 108
🏁 Script executed:
# Also search for date-picker files broadly
fd "date-picker" --type fRepository: kopai-app/kopai-mono
Length of output: 106
🏁 Script executed:
# Look for package.json to check React version
fd "package.json" --type f | grep -E "(packages/ui|root)" | head -5Repository: kopai-app/kopai-mono
Length of output: 88
🏁 Script executed:
# Read the date-picker file
cat -n packages/ui/src/components/date-picker.tsxRepository: kopai-app/kopai-mono
Length of output: 1271
🏁 Script executed:
# Check React version in package.json
cat packages/ui/package.json | grep -A 5 '"react"'Repository: kopai-app/kopai-mono
Length of output: 328
Associate the label with the input for accessibility.
The <label> element is not connected to the <input> via htmlFor and id attributes. This prevents screen readers from associating them. Use useId() to generate a stable identifier and bind them together. The fallback aria-label provides a secondary text for assistive technologies when label visibility varies.
🔧 Suggested change
-import { dashboardCatalog } from "../lib/catalog.js";
-import type { CatalogueComponentProps } from "../lib/component-catalog.js";
+import { dashboardCatalog } from "../lib/catalog.js";
+import type { CatalogueComponentProps } from "../lib/component-catalog.js";
+import { useId } from "react";
export function DatePicker({
element,
}: CatalogueComponentProps<typeof dashboardCatalog.components.DatePicker>) {
const { label, bindPath, placeholder } = element.props;
+ const inputId = useId();
+ const ariaLabel = label ?? placeholder ?? "Date";
return (
<div style={{ display: "flex", flexDirection: "column", gap: 4 }}>
{label && (
- <label style={{ fontSize: 14, fontWeight: 500 }}>{label}</label>
+ <label htmlFor={inputId} style={{ fontSize: 14, fontWeight: 500 }}>
+ {label}
+ </label>
)}
<input
type="date"
+ id={inputId}
+ aria-label={ariaLabel}
placeholder={placeholder || undefined}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { dashboardCatalog } from "../lib/catalog.js"; | |
| import type { CatalogueComponentProps } from "../lib/component-catalog.js"; | |
| export function DatePicker({ | |
| element, | |
| }: CatalogueComponentProps<typeof dashboardCatalog.components.DatePicker>) { | |
| const { label, bindPath, placeholder } = element.props; | |
| return ( | |
| <div style={{ display: "flex", flexDirection: "column", gap: 4 }}> | |
| {label && ( | |
| <label style={{ fontSize: 14, fontWeight: 500 }}>{label}</label> | |
| )} | |
| <input | |
| type="date" | |
| placeholder={placeholder || undefined} | |
| style={{ | |
| padding: "8px 12px", | |
| borderRadius: "var(--radius)", | |
| border: "1px solid var(--border)", | |
| background: "var(--card)", | |
| color: "var(--foreground)", | |
| fontSize: 16, | |
| outline: "none", | |
| }} | |
| /> | |
| <p style={{ margin: "4px 0 0", fontSize: 12, color: "var(--muted)" }}> | |
| Bound to: {bindPath} | |
| </p> | |
| import { dashboardCatalog } from "../lib/catalog.js"; | |
| import type { CatalogueComponentProps } from "../lib/component-catalog.js"; | |
| import { useId } from "react"; | |
| export function DatePicker({ | |
| element, | |
| }: CatalogueComponentProps<typeof dashboardCatalog.components.DatePicker>) { | |
| const { label, bindPath, placeholder } = element.props; | |
| const inputId = useId(); | |
| const ariaLabel = label ?? placeholder ?? "Date"; | |
| return ( | |
| <div style={{ display: "flex", flexDirection: "column", gap: 4 }}> | |
| {label && ( | |
| <label htmlFor={inputId} style={{ fontSize: 14, fontWeight: 500 }}> | |
| {label} | |
| </label> | |
| )} | |
| <input | |
| type="date" | |
| id={inputId} | |
| aria-label={ariaLabel} | |
| placeholder={placeholder || undefined} | |
| style={{ | |
| padding: "8px 12px", | |
| borderRadius: "var(--radius)", | |
| border: "1px solid var(--border)", | |
| background: "var(--card)", | |
| color: "var(--foreground)", | |
| fontSize: 16, | |
| outline: "none", | |
| }} | |
| /> | |
| <p style={{ margin: "4px 0 0", fontSize: 12, color: "var(--muted)" }}> | |
| Bound to: {bindPath} | |
| </p> | |
| </div> | |
| ); | |
| } |
🤖 Prompt for AI Agents
In `@packages/ui/src/components/date-picker.tsx` around lines 1 - 29, The label
isn't associated with the input in the DatePicker component; generate a stable
id using React's useId() inside DatePicker and apply it to the input's id and
the label's htmlFor, and also set an aria-label on the input (using label or a
fallback) so assistive tech can still read it when the visible label changes;
update references near DatePicker, element.props, label, and bindPath to wire
the id properly.
| import { z } from "zod"; | ||
| import { dataFilterSchemas } from "@kopai/core"; | ||
| import type { ReactNode } from "react"; | ||
|
|
||
| // DataSource schema - discriminated union with type-safe params per method | ||
| export const dataSourceSchema = z.discriminatedUnion("method", [ | ||
| z.object({ | ||
| method: z.literal("searchTracesPage"), | ||
| params: dataFilterSchemas.tracesDataFilterSchema, | ||
| }), | ||
| z.object({ | ||
| method: z.literal("searchLogsPage"), | ||
| params: dataFilterSchemas.logsDataFilterSchema, | ||
| }), | ||
| z.object({ | ||
| method: z.literal("searchMetricsPage"), | ||
| params: dataFilterSchemas.metricsDataFilterSchema, | ||
| }), | ||
| z.object({ | ||
| method: z.literal("getTrace"), | ||
| params: z.object({ traceId: z.string() }), | ||
| }), | ||
| z.object({ | ||
| method: z.literal("discoverMetrics"), | ||
| params: z.object({}).optional(), | ||
| }), | ||
| ]); | ||
|
|
||
| export type DataSource = z.infer<typeof dataSourceSchema>; | ||
|
|
||
| export const componentDefinitionSchema = z | ||
| .object({ | ||
| hasChildren: z.boolean(), | ||
| description: z | ||
| .string() | ||
| .describe( | ||
| "Component description to be displayed by the prompt generator" | ||
| ), | ||
| props: z.unknown(), | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd packages/ui/src/lib && wc -l component-catalog.tsRepository: kopai-app/kopai-mono
Length of output: 88
🏁 Script executed:
cd packages/ui/src/lib && cat -n component-catalog.tsRepository: kopai-app/kopai-mono
Length of output: 6037
🏁 Script executed:
rg "createCatalog" packages/ui/src/lib/ -A 10 -B 2Repository: kopai-app/kopai-mono
Length of output: 18554
🏁 Script executed:
rg "z\.instanceof" packages/ -t ts -t tsxRepository: kopai-app/kopai-mono
Length of output: 91
🏁 Script executed:
rg "instanceof" packages/ -t ts -A 2 -B 2Repository: kopai-app/kopai-mono
Length of output: 4322
🏁 Script executed:
rg "ZodType" packages/ -t ts | head -20Repository: kopai-app/kopai-mono
Length of output: 984
🏁 Script executed:
cd packages/ui/src/lib && grep -n "props:" component-catalog.test.ts -A 1 | head -40Repository: kopai-app/kopai-mono
Length of output: 1117
🏁 Script executed:
rg "export.*ZodType" packages/ -t tsRepository: kopai-app/kopai-mono
Length of output: 173
🏁 Script executed:
cd packages && find . -name "*.ts" -type f -exec grep -l "z.instanceof" {} \;Repository: kopai-app/kopai-mono
Length of output: 46
🏁 Script executed:
web_search
Zod z.instanceof ZodType validationRepository: kopai-app/kopai-mono
Length of output: 156
🌐 Web query:
Zod z.instanceof ZodType documentation validation
💡 Result:
z.instanceof(Class)creates a schema that validates an input is an instance of a specific class (i.e.,value instanceof Class). Example from the docs:const TestSchema = z.instanceof(Test); TestSchema.parse(new Test())passes;TestSchema.parse("blob")throws. [1]- In Zod, all schemas extend
z.ZodType, and anyZodTypesupports validation via.parse(...),.safeParse(...), and their async counterparts. [2]
Validating that an unknown value is a Zod schema (ZodType)
At runtime you can validate “is this a Zod schema object?” by checking it’s an instance of z.ZodType:
import * as z from "zod";
const ZodSchema = z.instanceof(z.ZodType); // runtime: value instanceof z.ZodType
// or, if you want explicit typing:
const ZodSchema2 = z.custom<z.ZodTypeAny>((v) => v instanceof z.ZodType);
ZodSchema.parse(z.string()); // ok
ZodSchema.parse("not a schema"); // throwsSources: z.instanceof docs [1]; ZodType base class + parse/safeParse methods [2].
References
[1] Zod documentation — “Instanceof” section (odocs mirror) (odocs-zod.vercel.app)
[2] Zod 4 docs — ZodType base class and methods (parse, safeParse, etc.) (zod.dev)
Citations:
Enforce props as a Zod schema to prevent runtime failures in createCatalog.
The componentDefinitionSchema currently allows props: z.unknown(), which validates any value. However, createCatalog at line 131 directly uses component.props inside z.object(), expecting it to be a ZodType instance. Passing a non-Zod value will crash at runtime. Use z.instanceof(ZodType) to enforce this constraint:
import { z, ZodType } from "zod";
export const componentDefinitionSchema = z
.object({
hasChildren: z.boolean(),
description: z
.string()
.describe(
"Component description to be displayed by the prompt generator"
),
- props: z.unknown(),
+ props: z.instanceof(ZodType).describe("Zod schema for component props"),
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { z } from "zod"; | |
| import { dataFilterSchemas } from "@kopai/core"; | |
| import type { ReactNode } from "react"; | |
| // DataSource schema - discriminated union with type-safe params per method | |
| export const dataSourceSchema = z.discriminatedUnion("method", [ | |
| z.object({ | |
| method: z.literal("searchTracesPage"), | |
| params: dataFilterSchemas.tracesDataFilterSchema, | |
| }), | |
| z.object({ | |
| method: z.literal("searchLogsPage"), | |
| params: dataFilterSchemas.logsDataFilterSchema, | |
| }), | |
| z.object({ | |
| method: z.literal("searchMetricsPage"), | |
| params: dataFilterSchemas.metricsDataFilterSchema, | |
| }), | |
| z.object({ | |
| method: z.literal("getTrace"), | |
| params: z.object({ traceId: z.string() }), | |
| }), | |
| z.object({ | |
| method: z.literal("discoverMetrics"), | |
| params: z.object({}).optional(), | |
| }), | |
| ]); | |
| export type DataSource = z.infer<typeof dataSourceSchema>; | |
| export const componentDefinitionSchema = z | |
| .object({ | |
| hasChildren: z.boolean(), | |
| description: z | |
| .string() | |
| .describe( | |
| "Component description to be displayed by the prompt generator" | |
| ), | |
| props: z.unknown(), | |
| }) | |
| import { z, ZodType } from "zod"; | |
| import { dataFilterSchemas } from "@kopai/core"; | |
| import type { ReactNode } from "react"; | |
| // DataSource schema - discriminated union with type-safe params per method | |
| export const dataSourceSchema = z.discriminatedUnion("method", [ | |
| z.object({ | |
| method: z.literal("searchTracesPage"), | |
| params: dataFilterSchemas.tracesDataFilterSchema, | |
| }), | |
| z.object({ | |
| method: z.literal("searchLogsPage"), | |
| params: dataFilterSchemas.logsDataFilterSchema, | |
| }), | |
| z.object({ | |
| method: z.literal("searchMetricsPage"), | |
| params: dataFilterSchemas.metricsDataFilterSchema, | |
| }), | |
| z.object({ | |
| method: z.literal("getTrace"), | |
| params: z.object({ traceId: z.string() }), | |
| }), | |
| z.object({ | |
| method: z.literal("discoverMetrics"), | |
| params: z.object({}).optional(), | |
| }), | |
| ]); | |
| export type DataSource = z.infer<typeof dataSourceSchema>; | |
| export const componentDefinitionSchema = z | |
| .object({ | |
| hasChildren: z.boolean(), | |
| description: z | |
| .string() | |
| .describe( | |
| "Component description to be displayed by the prompt generator" | |
| ), | |
| props: z.instanceof(ZodType).describe("Zod schema for component props"), | |
| }) |
🤖 Prompt for AI Agents
In `@packages/ui/src/lib/component-catalog.ts` around lines 1 - 40, The
componentDefinitionSchema currently allows props: z.unknown(), which lets
non-Zod values through and later causes createCatalog to crash when it calls
z.object(component.props); update componentDefinitionSchema to require that
props be a Zod schema (e.g., enforce instanceof ZodType) so validation fails at
parse time; specifically change the props definition used in
componentDefinitionSchema and ensure createCatalog (which accesses
component.props) can safely pass component.props into z.object() without runtime
errors.
| export function createCatalog< | ||
| C extends Record<string, z.infer<typeof componentDefinitionSchema>>, | ||
| >(catalogConfig: { name: string; components: C }) { | ||
| const elementVariants = ( | ||
| Object.keys(catalogConfig.components) as (keyof C & string)[] | ||
| ) | ||
| .map((catalogItemName) => ({ | ||
| catalogItemName, | ||
| component: catalogConfig.components[catalogItemName], | ||
| })) | ||
| .filter( | ||
| ( | ||
| itemConfig | ||
| ): itemConfig is typeof itemConfig & { component: C[keyof C] } => | ||
| !!itemConfig.component | ||
| ) | ||
| .map(({ catalogItemName, component }) => | ||
| z.object({ | ||
| key: z.string(), | ||
| type: z.literal(catalogItemName), | ||
| children: z.array(z.string()), | ||
| parentKey: z.string(), | ||
| dataSource: dataSourceSchema.optional(), | ||
| props: component.props, | ||
| }) | ||
| ); | ||
|
|
||
| type Schemas = ElementVariantSchemas<C>; | ||
| const elementsUnion = z.discriminatedUnion( | ||
| "type", | ||
| elementVariants as unknown as [Schemas, ...Schemas[]] | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat packages/ui/src/lib/component-catalog.tsRepository: kopai-app/kopai-mono
Length of output: 4875
🌐 Web query:
zod discriminatedUnion empty array error
💡 Result:
Two common “empty array” problems around z.discriminatedUnion:
1) You passed no union options ([]) to discriminatedUnion
z.discriminatedUnion(discriminator, options) requires at least one option (its TS type is a non-empty tuple like [A, ...A[]]), so [] can lead to TS errors and/or runtime errors like “Cannot read properties of undefined (reading '0')” because Zod (and some integrations) index into options[0]. [1], [2]
Fix (guard the dynamic case):
import { z } from "zod";
function makeSchema(options: z.ZodObject<any, any, any, any, any>[]) {
return options.length
? z.discriminatedUnion("type", options as [typeof options[number], ...typeof options])
: z.never(); // or z.any().refine(() => false, "No variants configured")
}If you’re composing/merging discriminated unions, use .options (which is already a non-empty tuple) rather than rebuilding arrays unsafely. [1]
2) Your data is an empty array, but your schema expects at least one element
By default z.array(...) allows []. If you want to reject an empty input array, use .nonempty() on the array schema. [3]
const Items = z.array(Item).nonempty(); // rejects []Sources: [1] [2] [3]
Guard against empty component catalogs before building the discriminated union.
If components is empty, z.discriminatedUnion throws with "Cannot read properties of undefined (reading '0')" because it requires at least one variant. A clear guard makes failures deterministic and easier to diagnose.
Suggested guard
const elementVariants = (
Object.keys(catalogConfig.components) as (keyof C & string)[]
)
.map((catalogItemName) => ({
catalogItemName,
component: catalogConfig.components[catalogItemName],
}))
.filter(
(
itemConfig
): itemConfig is typeof itemConfig & { component: C[keyof C] } =>
!!itemConfig.component
)
.map(({ catalogItemName, component }) =>
z.object({
key: z.string(),
type: z.literal(catalogItemName),
children: z.array(z.string()),
parentKey: z.string(),
dataSource: dataSourceSchema.optional(),
props: component.props,
})
);
+ if (elementVariants.length === 0) {
+ throw new Error("Catalog must define at least one component");
+ }🤖 Prompt for AI Agents
In `@packages/ui/src/lib/component-catalog.ts` around lines 108 - 139,
createCatalog currently builds elementVariants and then calls
z.discriminatedUnion("type", ...) which crashes if elementVariants is empty; add
a guard before creating elementsUnion that checks if elementVariants.length ===
0 and throws a clear, deterministic error (or returns/handles an empty catalog)
so z.discriminatedUnion is never called with an empty array. Update the logic
around elementVariants / elementsUnion in createCatalog to perform this check
and surface a helpful message referencing the catalog name (catalogConfig.name)
and the function createCatalog.
| useEffect(() => { | ||
| if (refetchFn) { | ||
| refetchFn({ limit: currentLimit }); | ||
| } | ||
| }, [currentLimit, refetchFn]); |
There was a problem hiding this comment.
Potential infinite re-fetch loop due to unstable refetchFn reference.
refetchFn is assigned from props.refetch on each render (line 94). Since props.refetch may be a new function reference each time (from useKopaiData's useCallback which depends on fetchData), this useEffect could trigger repeatedly.
The same pattern exists in LogsTable (lines 182-186) and MetricsTable (lines 262-266).
🐛 Proposed fix - use a ref to avoid effect dependency
+ const refetchRef = useRef(props.hasData ? props.refetch : null);
+ refetchRef.current = props.hasData ? props.refetch : null;
+
- const refetchFn = props.hasData ? props.refetch : null;
useEffect(() => {
- if (refetchFn) {
- refetchFn({ limit: currentLimit });
+ if (refetchRef.current) {
+ refetchRef.current({ limit: currentLimit });
}
- }, [currentLimit, refetchFn]);
+ }, [currentLimit]);🤖 Prompt for AI Agents
In `@packages/ui/src/pages/dashboard.tsx` around lines 95 - 99, The useEffect is
retriggering because refetchFn (from props.refetch) is unstable; make the
refetch call use a stable ref instead: create a ref (e.g., refetchRef) that you
update whenever props.refetch changes, and in the effect that depends on
currentLimit call refetchRef.current({ limit: currentLimit }) so the effect only
depends on currentLimit. Apply the same pattern to LogsTable and MetricsTable
for their refetchFn usages (update the ref in a small effect when
props.refetch/refetchFn changes, and invoke ref.current inside the limit effect)
to prevent the infinite re-fetch loop.
| }; | ||
|
|
||
| // Create SDK client pointing to local server | ||
| const client = new KopaiClient({ baseUrl: "http://localhost:8000/signals" }); |
There was a problem hiding this comment.
Hardcoded localhost URL should be configurable.
The SDK client URL "http://localhost:8000/signals" is hardcoded, which will break in non-local environments.
Consider using an environment variable or configuration:
♻️ Proposed fix
-const client = new KopaiClient({ baseUrl: "http://localhost:8000/signals" });
+const client = new KopaiClient({
+ baseUrl: import.meta.env.VITE_KOPAI_API_URL ?? "http://localhost:8000/signals"
+});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const client = new KopaiClient({ baseUrl: "http://localhost:8000/signals" }); | |
| const client = new KopaiClient({ | |
| baseUrl: import.meta.env.VITE_KOPAI_API_URL ?? "http://localhost:8000/signals" | |
| }); |
🤖 Prompt for AI Agents
In `@packages/ui/src/pages/dashboard.tsx` at line 406, Replace the hardcoded
baseUrl in the KopaiClient instantiation (the line creating new KopaiClient with
"http://localhost:8000/signals") with a configurable environment-backed value
(e.g. read from process.env.NEXT_PUBLIC_SIGNALS_URL or a similar app-level
config) and provide a sensible default fallback for local dev; update the
instantiation in dashboard.tsx (where KopaiClient is constructed) to use that
env/config value and add a small runtime check that logs or throws a clear error
if the value is missing in non-dev environments.
| // Create SDK client pointing to local server | ||
| const client = new KopaiClient({ baseUrl: "http://localhost:8000/signals" }); |
There was a problem hiding this comment.
Hardcoded localhost URL will not work in non-local environments.
The KopaiClient is instantiated at module level with a hardcoded URL. This should be configurable via environment variables or passed as a prop.
🔧 Proposed fix using environment variable
-// Create SDK client pointing to local server
-const client = new KopaiClient({ baseUrl: "http://localhost:8000/signals" });
+// Create SDK client - baseUrl should be configured per environment
+const getBaseUrl = () => {
+ if (typeof window !== "undefined" && window.__KOPAI_BASE_URL__) {
+ return window.__KOPAI_BASE_URL__;
+ }
+ return import.meta.env?.VITE_KOPAI_BASE_URL ?? "http://localhost:8000/signals";
+};
+
+const client = new KopaiClient({ baseUrl: getBaseUrl() });Alternatively, consider making the client a prop to ObservabilityPage for better testability and flexibility.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Create SDK client pointing to local server | |
| const client = new KopaiClient({ baseUrl: "http://localhost:8000/signals" }); | |
| // Create SDK client - baseUrl should be configured per environment | |
| const getBaseUrl = () => { | |
| if (typeof window !== "undefined" && window.__KOPAI_BASE_URL__) { | |
| return window.__KOPAI_BASE_URL__; | |
| } | |
| return import.meta.env?.VITE_KOPAI_BASE_URL ?? "http://localhost:8000/signals"; | |
| }; | |
| const client = new KopaiClient({ baseUrl: getBaseUrl() }); |
🤖 Prompt for AI Agents
In `@packages/ui/src/pages/observability.tsx` around lines 222 - 223, The
KopaiClient is created at module scope with a hardcoded baseUrl
("http://localhost:8000/signals"); change this to read from configuration and/or
accept it via props so non-local environments work: replace the module-level
instantiation of client (KopaiClient) with a factory that uses an environment
variable (e.g., process.env.NEXT_PUBLIC_SIGNALS_URL) or move client creation
into the ObservabilityPage component and accept a client or baseUrl prop
(ObservabilityPage) so tests and deployments can inject the correct URL; ensure
fallback/default behavior and update any callers to pass the client or URL.
* feat: add vite to ui * squash this * fix: types * fix: wrap in act * fix: remove actions * chore: create prompt * feat(ui): add simple component catalog * feat: create simple renderer * feat: update uiTree schema returned from catalog * feat: add createRegistry helper function * feat: add example page showcasing all components from catalog * feat: make Renderer accept typed registry * fix: extract createRegistry * chore: remove simple- prefix from names - remove unused files and tests * feat(ui): add generatePromptInstructions fn * feat: add example of changing a limit and refetch * chore(ui): rearrange dir structure * docs(ui): add jsdoc to exported functions * chore(ui): remove createRegistry * docs(ui): update readme * fix: remove unnecessary dependencies * feat: add observability dashboard - WIP * feat(ui): add dashboard datasource * feat(ui): add observability components * fix(sqlite): parse numbers * chore(app): improve autoreload (#31) * chore(deps): bump the all-dependencies group across 1 directory with 10 updates (#32) Bumps the all-dependencies group with 10 updates in the / directory: | Package | From | To | | --- | --- | --- | | [@eslint/js](https://github.com/eslint/eslint/tree/HEAD/packages/js) | `9.39.2` | `10.0.1` | | [@types/node](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/node) | `25.2.1` | `25.2.2` | | [eslint](https://github.com/eslint/eslint) | `9.39.2` | `10.0.0` | | [@fastify/swagger](https://github.com/fastify/fastify-swagger) | `9.6.1` | `9.7.0` | | [tsdown](https://github.com/rolldown/tsdown) | `0.20.1` | `0.20.3` | | [fastify](https://github.com/fastify/fastify) | `5.7.2` | `5.7.4` | | [commander](https://github.com/tj/commander.js) | `14.0.2` | `14.0.3` | | [@bufbuild/buf](https://github.com/bufbuild/buf) | `1.64.0` | `1.65.0` | | [msw](https://github.com/mswjs/msw) | `2.12.7` | `2.12.9` | | [kysely](https://github.com/kysely-org/kysely) | `0.28.10` | `0.28.11` | Updates `@eslint/js` from 9.39.2 to 10.0.1 - [Release notes](https://github.com/eslint/eslint/releases) - [Commits](https://github.com/eslint/eslint/commits/HEAD/packages/js) Updates `@types/node` from 25.2.1 to 25.2.2 - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/node) Updates `eslint` from 9.39.2 to 10.0.0 - [Release notes](https://github.com/eslint/eslint/releases) - [Commits](eslint/eslint@v9.39.2...v10.0.0) Updates `@fastify/swagger` from 9.6.1 to 9.7.0 - [Release notes](https://github.com/fastify/fastify-swagger/releases) - [Commits](fastify/fastify-swagger@v9.6.1...v9.7.0) Updates `tsdown` from 0.20.1 to 0.20.3 - [Release notes](https://github.com/rolldown/tsdown/releases) - [Commits](rolldown/tsdown@v0.20.1...v0.20.3) Updates `fastify` from 5.7.2 to 5.7.4 - [Release notes](https://github.com/fastify/fastify/releases) - [Commits](fastify/fastify@v5.7.2...v5.7.4) Updates `commander` from 14.0.2 to 14.0.3 - [Release notes](https://github.com/tj/commander.js/releases) - [Changelog](https://github.com/tj/commander.js/blob/master/CHANGELOG.md) - [Commits](tj/commander.js@v14.0.2...v14.0.3) Updates `@bufbuild/buf` from 1.64.0 to 1.65.0 - [Release notes](https://github.com/bufbuild/buf/releases) - [Changelog](https://github.com/bufbuild/buf/blob/main/CHANGELOG.md) - [Commits](bufbuild/buf@v1.64.0...v1.65.0) Updates `msw` from 2.12.7 to 2.12.9 - [Release notes](https://github.com/mswjs/msw/releases) - [Changelog](https://github.com/mswjs/msw/blob/main/CHANGELOG.md) - [Commits](mswjs/msw@v2.12.7...v2.12.9) Updates `kysely` from 0.28.10 to 0.28.11 - [Release notes](https://github.com/kysely-org/kysely/releases) - [Commits](kysely-org/kysely@v0.28.10...v0.28.11) --- updated-dependencies: - dependency-name: "@eslint/js" dependency-version: 10.0.1 dependency-type: direct:development update-type: version-update:semver-major dependency-group: all-dependencies - dependency-name: "@types/node" dependency-version: 25.2.2 dependency-type: direct:development update-type: version-update:semver-patch dependency-group: all-dependencies - dependency-name: eslint dependency-version: 10.0.0 dependency-type: direct:development update-type: version-update:semver-major dependency-group: all-dependencies - dependency-name: "@fastify/swagger" dependency-version: 9.7.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: all-dependencies - dependency-name: tsdown dependency-version: 0.20.3 dependency-type: direct:development update-type: version-update:semver-patch dependency-group: all-dependencies - dependency-name: fastify dependency-version: 5.7.4 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: all-dependencies - dependency-name: commander dependency-version: 14.0.3 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: all-dependencies - dependency-name: "@bufbuild/buf" dependency-version: 1.65.0 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: all-dependencies - dependency-name: msw dependency-version: 2.12.9 dependency-type: direct:development update-type: version-update:semver-patch dependency-group: all-dependencies - dependency-name: kysely dependency-version: 0.28.11 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: all-dependencies ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(ui): reorg components * feat(ui): use useQuery * feat(ui): add refetch interval * feat(ui): live logs with filters, url filter state * feat(ui): add keyboard shortcuts * feat: move vite plugin to app * chore(ui,app): move page heading to app * feat(app,cli): update kopai logo * chore(ui): use short logo * feat(ui): remove span count, use minimal metrics * feat(ui): add service colors to logs and service list * fix(ui): trace link contains a span * fix(ui): minor ui problems - remove selected border outline - relative timestamp from selected log entry - add j/k shortcuts to spans * fix(ui): do not bundle zod * fix(ui): address critical issues * fix(ui): remove outline around clicked log * feat(ui): add shortcut hints * fix(ui): nitpick comments * fix(ui): span name over colored time line * feat(ui): close filter pane on Esc * chore(ui): remove unnecessary export * chore(ui): create changeset --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Summary by CodeRabbit