From 8e58b123bf0b86cc0f264397f86682defe2189a3 Mon Sep 17 00:00:00 2001 From: Abhimanyu Saharan Date: Wed, 11 Feb 2026 20:42:16 +0000 Subject: [PATCH 1/3] docs(frontend): add JSDoc for complex UI utilities --- .../src/components/BoardApprovalsPanel.tsx | 34 +++++++++++++++++++ .../src/components/BoardOnboardingChat.tsx | 22 ++++++++++++ frontend/src/components/charts/chart.tsx | 10 ++++++ .../src/components/organisms/TaskBoard.tsx | 26 ++++++++++++++ .../src/components/ui/dropdown-select.tsx | 12 +++++-- 5 files changed, 102 insertions(+), 2 deletions(-) diff --git a/frontend/src/components/BoardApprovalsPanel.tsx b/frontend/src/components/BoardApprovalsPanel.tsx index eaaeeab6..16c46872 100644 --- a/frontend/src/components/BoardApprovalsPanel.tsx +++ b/frontend/src/components/BoardApprovalsPanel.tsx @@ -148,9 +148,15 @@ const formatRubricTooltipValue = ( ); }; +/** + * Narrow unknown values to a plain record. + * + * Used for defensive parsing of `approval.payload` (schema can evolve). + */ const isRecord = (value: unknown): value is Record => typeof value === "object" && value !== null && !Array.isArray(value); +/** Safely read any value at a nested path inside an approval payload. */ const payloadAtPath = (payload: Approval["payload"], path: string[]) => { let current: unknown = payload; for (const key of path) { @@ -160,6 +166,12 @@ const payloadAtPath = (payload: Approval["payload"], path: string[]) => { return current ?? null; }; +/** + * Safely read a simple scalar value from an approval payload. + * + * The backend payload shape can evolve (camelCase vs snake_case). Keeping these + * helpers centralized makes it easier to support older approvals. + */ const payloadValue = (payload: Approval["payload"], key: string) => { const value = payloadAtPath(payload, [key]); if (typeof value === "string" || typeof value === "number") { @@ -168,12 +180,18 @@ const payloadValue = (payload: Approval["payload"], key: string) => { return null; }; +/** + * Safely read a string[] value from an approval payload. + * + * Filters non-string entries to keep UI rendering predictable. + */ const payloadValues = (payload: Approval["payload"], key: string) => { const value = payloadAtPath(payload, [key]); if (!Array.isArray(value)) return []; return value.filter((item): item is string => typeof item === "string"); }; +/** Safely read a scalar value from an approval payload at a nested path. */ const payloadNestedValue = (payload: Approval["payload"], path: string[]) => { const value = payloadAtPath(payload, path); if (typeof value === "string" || typeof value === "number") { @@ -182,6 +200,7 @@ const payloadNestedValue = (payload: Approval["payload"], path: string[]) => { return null; }; +/** Safely read a string[] value from an approval payload at a nested path. */ const payloadNestedValues = (payload: Approval["payload"], path: string[]) => { const value = payloadAtPath(payload, path); if (!Array.isArray(value)) return []; @@ -222,6 +241,15 @@ const normalizeRubricScores = (raw: unknown): Record => { const payloadRubricScores = (payload: Approval["payload"]) => normalizeRubricScores(payloadAtPath(payload, ["analytics", "rubric_scores"])); +/** + * Extract task ids referenced by an approval. + * + * Approvals can reference tasks in multiple places depending on the producer: + * - top-level `task_id` / `task_ids` fields + * - nested payload keys (task_id/taskId/taskIDs, etc.) + * + * We merge/dedupe to get a best-effort list for UI deep links. + */ const approvalTaskIds = (approval: Approval) => { const payload = approval.payload ?? {}; const linkedTaskIds = (approval as Approval & { task_ids?: string[] | null }) @@ -318,6 +346,12 @@ const approvalRelatedTasks = (approval: Approval): RelatedTaskSummary[] => { const taskHref = (boardId: string, taskId: string) => `/boards/${encodeURIComponent(boardId)}?taskId=${encodeURIComponent(taskId)}`; +/** + * Create a small, human-readable summary of an approval request. + * + * Used by the approvals panel modal: it prefers explicit fields but falls back + * to payload-derived values so older approvals still render well. + */ const approvalSummary = (approval: Approval, boardLabel?: string | null) => { const payload = approval.payload ?? {}; const taskIds = approvalTaskIds(approval); diff --git a/frontend/src/components/BoardOnboardingChat.tsx b/frontend/src/components/BoardOnboardingChat.tsx index 7b2db557..ac5c95a9 100644 --- a/frontend/src/components/BoardOnboardingChat.tsx +++ b/frontend/src/components/BoardOnboardingChat.tsx @@ -30,6 +30,12 @@ type NormalizedMessage = { content: string; }; +/** + * Normalize backend onboarding messages into a strict `{role, content}` list. + * + * The server stores messages as untyped JSON; this protects the UI from partial + * or malformed entries. + */ const normalizeMessages = ( value?: BoardOnboardingReadMessages, ): NormalizedMessage[] | null => { @@ -59,6 +65,16 @@ const FREE_TEXT_OPTION_RE = const isFreeTextOption = (label: string) => FREE_TEXT_OPTION_RE.test(label); +/** + * Best-effort parser for assistant-produced question payloads. + * + * During onboarding, the assistant can respond with either: + * - raw JSON (ideal) + * - a fenced ```json block + * - slightly-structured objects + * + * This function validates shape and normalizes option ids/labels. + */ const normalizeQuestion = (value: unknown): Question | null => { if (!value || typeof value !== "object") return null; const data = value as { question?: unknown; options?: unknown }; @@ -90,6 +106,12 @@ const normalizeQuestion = (value: unknown): Question | null => { return { question: data.question, options }; }; +/** + * Extract the most recent assistant question from the transcript. + * + * We intentionally only inspect the last assistant message: the user may have + * typed arbitrary text between questions. + */ const parseQuestion = (messages?: NormalizedMessage[] | null) => { if (!messages?.length) return null; const lastAssistant = [...messages] diff --git a/frontend/src/components/charts/chart.tsx b/frontend/src/components/charts/chart.tsx index e0056c46..46ebfc11 100644 --- a/frontend/src/components/charts/chart.tsx +++ b/frontend/src/components/charts/chart.tsx @@ -40,6 +40,12 @@ function useChart() { return context; } +/** + * Recharts wrapper that: + * - Provides a shared `ChartConfig` via context (labels/icons/colors) + * - Exposes a small legend state (hide/toggle series) + * - Injects CSS variables (`--color-*`) scoped to this chart instance + */ function ChartContainer({ id, className, @@ -108,6 +114,10 @@ function ChartContainer({ ); } +/** + * Emits scoped theme-aware CSS variables so Recharts series can use + * `var(--color-)` without hardcoding colors in every chart. + */ const ChartStyle = ({ id, config }: { id: string; config: ChartConfig }) => { const colorConfig = Object.entries(config).filter( ([, config]) => config.theme || config.color, diff --git a/frontend/src/components/organisms/TaskBoard.tsx b/frontend/src/components/organisms/TaskBoard.tsx index 00c342b6..6eb6ebee 100644 --- a/frontend/src/components/organisms/TaskBoard.tsx +++ b/frontend/src/components/organisms/TaskBoard.tsx @@ -82,15 +82,25 @@ const columns: Array<{ }, ]; +/** + * Build compact due-date UI state for a task card. + * + * - Returns `due: undefined` when the task has no due date (or it's invalid), so + * callers can omit the due-date UI entirely. + * - Treats a task as overdue only if it is not `done` (so "Done" tasks don't + * keep showing as overdue forever). + */ const resolveDueState = ( task: Task, ): { due: string | undefined; isOverdue: boolean } => { const date = parseApiDatetime(task.due_at); if (!date) return { due: undefined, isOverdue: false }; + const dueLabel = date.toLocaleDateString(undefined, { month: "short", day: "numeric", }); + const isOverdue = task.status !== "done" && date.getTime() < Date.now(); return { due: isOverdue ? `Overdue ยท ${dueLabel}` : dueLabel, @@ -103,6 +113,16 @@ type CardPosition = { left: number; top: number }; const KANBAN_MOVE_ANIMATION_MS = 240; const KANBAN_MOVE_EASING = "cubic-bezier(0.2, 0.8, 0.2, 1)"; +/** + * Kanban-style task board with 4 columns. + * + * Notes: + * - Uses a lightweight FLIP animation (via `useLayoutEffect`) to animate cards + * to their new positions when tasks move between columns. + * - Drag interactions can temporarily fight browser-managed drag images; the + * animation is disabled while a card is being dragged. + * - Respects `prefers-reduced-motion`. + */ export const TaskBoard = memo(function TaskBoard({ tasks, onTaskSelect, @@ -131,6 +151,12 @@ export const TaskBoard = memo(function TaskBoard({ [], ); + /** + * Snapshot each card's position relative to the scroll container. + * + * We store these measurements so we can compute deltas (prev - next) and + * apply the FLIP technique on the next render. + */ const measurePositions = useCallback((): Map => { const positions = new Map(); const container = boardRef.current; diff --git a/frontend/src/components/ui/dropdown-select.tsx b/frontend/src/components/ui/dropdown-select.tsx index 178fba62..51b81570 100644 --- a/frontend/src/components/ui/dropdown-select.tsx +++ b/frontend/src/components/ui/dropdown-select.tsx @@ -40,7 +40,11 @@ type DropdownSelectProps = { emptyMessage?: string; }; -// Resolve trigger placeholder text with explicit prop override first, then accessible fallback. +/** + * Derive a human-friendly trigger placeholder from an accessible `ariaLabel`. + * + * Keeps placeholder strings consistent even when callers only provide aria text. + */ const resolvePlaceholder = (ariaLabel: string, placeholder?: string) => { if (placeholder) { return placeholder; @@ -52,7 +56,11 @@ const resolvePlaceholder = (ariaLabel: string, placeholder?: string) => { return trimmed.endsWith("...") ? trimmed : `${trimmed}...`; }; -// Resolve search input placeholder from explicit override or a normalized aria label. +/** + * Search input placeholder derived from `ariaLabel`. + * + * Example: ariaLabel="Select agent" -> "Search agent...". + */ const resolveSearchPlaceholder = ( ariaLabel: string, searchPlaceholder?: string, From 44e4499a2613308dbbfcf47fc1a94d59d5b9ca76 Mon Sep 17 00:00:00 2001 From: Abhimanyu Saharan Date: Wed, 11 Feb 2026 21:45:46 +0000 Subject: [PATCH 2/3] docs(frontend): document SSE reconnect cursor + backoff reset rationale --- frontend/src/app/board-groups/[groupId]/page.tsx | 12 ++++++++++-- frontend/src/app/boards/[boardId]/page.tsx | 10 ++++++++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/frontend/src/app/board-groups/[groupId]/page.tsx b/frontend/src/app/board-groups/[groupId]/page.tsx index fb8db638..e2325fe4 100644 --- a/frontend/src/app/board-groups/[groupId]/page.tsx +++ b/frontend/src/app/board-groups/[groupId]/page.tsx @@ -339,6 +339,12 @@ export default function BoardGroupDetailPage() { [], ); + /** + * Computes the newest `created_at` timestamp in a list of memory items. + * + * We pass this as `since` when reconnecting SSE so we don't re-stream the + * entire chat history after transient disconnects. + */ const latestMemoryTimestamp = useCallback((items: BoardGroupMemoryRead[]) => { if (!items.length) return undefined; const latest = items.reduce((max, item) => { @@ -405,11 +411,13 @@ export default function BoardGroupDetailPage() { while (!isCancelled) { const { value, done } = await reader.read(); if (done) break; + + // Consider the stream healthy once we receive any bytes (including pings) + // and reset the backoff so a later disconnect doesn't wait the full max. if (value && value.length) { - // Consider the stream healthy once we receive any bytes (including pings), - // then reset the backoff for future reconnects. backoff.reset(); } + buffer += decoder.decode(value, { stream: true }); buffer = buffer.replace(/\r\n/g, "\n"); let boundary = buffer.indexOf("\n\n"); diff --git a/frontend/src/app/boards/[boardId]/page.tsx b/frontend/src/app/boards/[boardId]/page.tsx index e1a0cd00..297edb08 100644 --- a/frontend/src/app/boards/[boardId]/page.tsx +++ b/frontend/src/app/boards/[boardId]/page.tsx @@ -1315,6 +1315,12 @@ export default function BoardDetailPage() { return () => window.clearTimeout(timeout); }, [chatMessages, isChatOpen]); + /** + * Returns an ISO timestamp for the newest board chat message. + * + * Used as the `since` cursor when (re)connecting to the SSE endpoint so we + * don't re-stream the entire chat log. + */ const latestChatTimestamp = (items: BoardChatMessage[]) => { if (!items.length) return undefined; const latest = items.reduce((max, item) => { @@ -1373,9 +1379,9 @@ export default function BoardDetailPage() { while (!isCancelled) { const { value, done } = await reader.read(); if (done) break; + // Consider the stream healthy once we receive any bytes (including pings) + // and reset the backoff so a later disconnect doesn't wait the full max. if (value && value.length) { - // Consider the stream "healthy" once we receive any bytes (including pings), - // then reset the backoff for future reconnects. backoff.reset(); } buffer += decoder.decode(value, { stream: true }); From 691fd11d10b17993598ea4404b90a117eba74665 Mon Sep 17 00:00:00 2001 From: Abhimanyu Saharan Date: Sat, 14 Feb 2026 14:26:04 +0000 Subject: [PATCH 3/3] docs(frontend): document skills marketplace + packs data-flow --- frontend/src/app/skills/marketplace/page.tsx | 19 +++++++++++ frontend/src/app/skills/packs/page.tsx | 11 ++++++ .../skills/MarketplaceSkillForm.tsx | 13 +++++++ .../src/components/skills/table-helpers.tsx | 7 ++++ frontend/src/lib/skills-source.ts | 34 +++++++++++++++++++ 5 files changed, 84 insertions(+) diff --git a/frontend/src/app/skills/marketplace/page.tsx b/frontend/src/app/skills/marketplace/page.tsx index 95d1f3c3..bd76a4d6 100644 --- a/frontend/src/app/skills/marketplace/page.tsx +++ b/frontend/src/app/skills/marketplace/page.tsx @@ -145,6 +145,18 @@ function parsePageSizeParam(value: string | null) { return MARKETPLACE_DEFAULT_PAGE_SIZE; } +/** + * Skills Marketplace admin page. + * + * Data-flow notes: + * - URL query params are treated as the shareable source-of-truth for filters + * and pagination (we mirror local state -> URL via `router.replace`). + * - We keep a *separate* query (without pagination) for building filter option + * lists (categories/risks) so options don't disappear just because the current + * page is filtered/paginated. + * - Total row count is best-effort: when the API provides `x-total-count`, we + * use it; otherwise we infer whether there is a next page from page-size. + */ export default function SkillsMarketplacePage() { const queryClient = useQueryClient(); const router = useRouter(); @@ -247,6 +259,8 @@ export default function SkillsMarketplacePage() { resolvedGatewayId, selectedPackId, ]); + // Fetch a non-paginated (or minimally constrained) slice for building filter options. + // Keeping this separate avoids "missing" categories/risks when the main list is paginated. const filterOptionsParams = useMemo(() => { const params: MarketplaceSkillListParams = { gateway_id: resolvedGatewayId, @@ -314,6 +328,8 @@ export default function SkillsMarketplacePage() { ); const filteredSkills = useMemo(() => skills, [skills]); + // Prefer the API total-count header when available (true pagination). + // When missing, we fall back to a heuristic for "has next page". const totalCountInfo = useMemo(() => { if (skillsQuery.data?.status !== 200) { return { hasKnownTotal: false, total: skills.length }; @@ -435,6 +451,9 @@ export default function SkillsMarketplacePage() { } }, [currentPage, totalCountInfo.hasKnownTotal, totalPages]); + // Mirror local filter/page state back into the URL so the view is shareable and + // back/forward navigation works. We use `replace` (not push) to avoid filling + // browser history with every keystroke / filter tweak. useEffect(() => { const nextParams = new URLSearchParams(searchParams.toString()); const normalizedSearchForUrl = searchTerm.trim(); diff --git a/frontend/src/app/skills/packs/page.tsx b/frontend/src/app/skills/packs/page.tsx index 8b35eb94..2f07a567 100644 --- a/frontend/src/app/skills/packs/page.tsx +++ b/frontend/src/app/skills/packs/page.tsx @@ -32,6 +32,15 @@ const PACKS_SORTABLE_COLUMNS = [ "updated_at", ]; +/** + * Skill packs admin page. + * + * Notes: + * - Sync actions are intentionally serialized (per-pack) to avoid a thundering herd + * of GitHub fetches / backend sync jobs. + * - We keep UI state (`syncingPackIds`, warnings) local; the canonical list is + * still React Query (invalidate after sync/delete). + */ export default function SkillsPacksPage() { const queryClient = useQueryClient(); const { isSignedIn } = useAuth(); @@ -141,6 +150,8 @@ export default function SkillsPacksPage() { try { let hasFailure = false; + // Run sequentially so the UI remains predictable and the backend isn't hit with + // concurrent sync bursts (which can trigger rate-limits). for (const pack of packs) { if (!pack.id) continue; setSyncingPackIds((previous) => { diff --git a/frontend/src/components/skills/MarketplaceSkillForm.tsx b/frontend/src/components/skills/MarketplaceSkillForm.tsx index 541e97e2..89e31c27 100644 --- a/frontend/src/components/skills/MarketplaceSkillForm.tsx +++ b/frontend/src/components/skills/MarketplaceSkillForm.tsx @@ -48,6 +48,12 @@ const extractErrorMessage = (error: unknown, fallback: string) => { return fallback; }; +/** + * Form used for creating/editing a marketplace skill source. + * + * Intentionally keeps validation lightweight + client-side only: + * the backend remains the source of truth and returns actionable errors. + */ export function MarketplaceSkillForm({ initialValues, sourceUrlReadOnly = false, @@ -80,6 +86,13 @@ export function MarketplaceSkillForm({ ); const [errorMessage, setErrorMessage] = useState(null); + /** + * Basic repo URL validation. + * + * This is strict by design (https + github.com + at least owner/repo) + * to catch obvious mistakes early. More complex URLs (subpaths, branches) + * are handled server-side. + */ const isValidSourceUrl = (value: string) => { try { const parsed = new URL(value); diff --git a/frontend/src/components/skills/table-helpers.tsx b/frontend/src/components/skills/table-helpers.tsx index bbbf3592..241275ce 100644 --- a/frontend/src/components/skills/table-helpers.tsx +++ b/frontend/src/components/skills/table-helpers.tsx @@ -26,6 +26,13 @@ export const SKILLS_TABLE_EMPTY_ICON = ( ); +/** + * Small helper for supporting both controlled and uncontrolled table sorting. + * + * TanStack Table expects a `sorting` state + `onSortingChange` callback. + * Some pages want to control this from the URL (shareable links), while others + * are fine letting the table manage it internally. + */ export const useTableSortingState = ( sorting: SortingState | undefined, onSortingChange: OnChangeFn | undefined, diff --git a/frontend/src/lib/skills-source.ts b/frontend/src/lib/skills-source.ts index 5c9f0fb6..df1d32bf 100644 --- a/frontend/src/lib/skills-source.ts +++ b/frontend/src/lib/skills-source.ts @@ -1,8 +1,24 @@ +/** + * Normalize a repository-ish URL for UI usage. + * + * - Trims whitespace + * - Removes trailing slashes + * - Strips a trailing ".git" (common for clone URLs) + */ export const normalizeRepoSourceUrl = (sourceUrl: string): string => { const trimmed = sourceUrl.trim().replace(/\/+$/, ""); return trimmed.endsWith(".git") ? trimmed.slice(0, -4) : trimmed; }; +/** + * Extract the base repo URL from a skill source URL. + * + * We support skill source URLs that may include a `/tree//...` suffix + * (e.g. a skill living inside a monorepo). In those cases, we want to link to + * the repo root (the "pack") rather than the nested path. + * + * Returns `null` for invalid/unsupported URLs. + */ export const repoBaseFromSkillSourceUrl = ( skillSourceUrl: string, ): string | null => { @@ -26,11 +42,23 @@ export const repoBaseFromSkillSourceUrl = ( } }; +/** + * Derive the pack URL from a skill source URL. + * + * Prefer the repo base when the URL points at a nested `/tree/...` path. + * Otherwise, fall back to the original source URL. + */ export const packUrlFromSkillSourceUrl = (skillSourceUrl: string): string => { const repoBase = repoBaseFromSkillSourceUrl(skillSourceUrl); return repoBase ?? skillSourceUrl; }; +/** + * Create a short, stable label for a pack URL (used in tables and filter pills). + * + * - For GitHub-like URLs, prefers `owner/repo`. + * - For other URLs, falls back to the host. + */ export const packLabelFromUrl = (packUrl: string): string => { try { const parsed = new URL(packUrl); @@ -44,6 +72,12 @@ export const packLabelFromUrl = (packUrl: string): string => { } }; +/** + * Build a packs page href filtered to a specific pack URL. + * + * We use a query param instead of path segments so the packs list can be + * shareable/bookmarkable without additional route definitions. + */ export const packsHrefFromPackUrl = (packUrl: string): string => { const params = new URLSearchParams({ source_url: packUrl }); return `/skills/packs?${params.toString()}`;