diff --git a/src/components/admin/review-row.tsx b/src/components/admin/review-row.tsx index 78b5381..21de9e6 100644 --- a/src/components/admin/review-row.tsx +++ b/src/components/admin/review-row.tsx @@ -5,8 +5,14 @@ import { createPortal } from "react-dom" import { useRouter } from "next/navigation" import { ArrowRight, ArrowRightLeft, CheckCircle2, ChevronRight, GitMerge, Layers, Loader2, Network, Pencil, PlusCircle, PlusSquare, Share2, Trash2, Users, type LucideIcon } from "lucide-react" import { formatDateRelative } from "@/lib/date-format" -import type { Review, ReviewStatus } from "@/lib/graph-api" +import type { + PromotionSummary, + Review, + ReviewStatus, + SchemaTypeOverride, +} from "@/lib/graph-api" import { approveReview, dismissReview, triggerMergeWorkflow } from "@/lib/graph-api" +import { SchemaPromotionDialog } from "@/components/admin/schema-promotion-dialog" import { useStakworkRunStatus } from "@/lib/hooks/use-stakwork-run-status" import { cn, displayNodeType } from "@/lib/utils" import { @@ -669,9 +675,52 @@ export function ReviewRow({ return false }, [review.action_name, direction, canonicalId, checkedSources]) + // ── Scratchpad promotion (add_schema_node_type on a scratchpad_entry) ─────── + // The type has to be created WITH the attributes the parked payloads need — + // a bare type can't hold their data, so the entries would fail to replay. + // Approving therefore goes through a confirmation dialog rather than straight + // to the API, and its result comes back as the approve override_payload. + const isSchemaPromotion = + review.action_name === "add_schema_node_type" && + review.type === "scratchpad_entry" + const [promotionOpen, setPromotionOpen] = useState(false) + const [promotionSummary, setPromotionSummary] = + useState(null) + + async function submitApproval(override?: SchemaTypeOverride | { from: string[]; to: string }) { + setApproving(true) + setInlineError(null) + try { + const res = await approveReview(review.ref_id, override) + if (res.error_message || res.status === "failed") { + setInlineError(res.error_message ?? "Approval failed") + onCountRefresh?.() + return + } + // Surface partial promotion outcomes before the row is refetched away — + // "approved" alone doesn't say whether the entries actually landed. + if (res.promotion_summary && res.promotion_summary.failed.length > 0) { + setPromotionSummary(res.promotion_summary) + onCountRefresh?.() + return + } + setPromotionOpen(false) + onRefresh() + onCountRefresh?.() + } catch { + setInlineError("Approval request failed") + } finally { + setApproving(false) + } + } + async function handleApprove() { if (!isAdmin) return if (mergeError) return + if (isSchemaPromotion) { + setPromotionOpen(true) + return + } setApproving(true) setInlineError(null) try { @@ -792,17 +841,35 @@ export function ReviewRow({
e.stopPropagation()}> {isPending && isAdmin ? (
- handleApprove()} - minWidthClass="min-w-[68px]" - disabled={mergeError !== null} - disabledReason={mergeError ?? undefined} - /> + {/* A schema promotion needs the property table filled in before + anything is created, so it opens the dialog directly — a + confirm popover in front of it would be a second prompt for + the same decision. */} + {isSchemaPromotion ? ( + + ) : ( + handleApprove()} + minWidthClass="min-w-[68px]" + disabled={mergeError !== null} + disabledReason={mergeError ?? undefined} + /> + )} )} + {/* Partial promotion: the type was created but some entries did not + replay. Reported here because the row's status ("approved") can't + express it. */} + {promotionSummary && promotionSummary.failed.length > 0 && ( +
+ Type created. {promotionSummary.promoted.length} of{" "} + {promotionSummary.attempted} entries promoted;{" "} + {promotionSummary.failed.length} failed: +
    + {promotionSummary.failed.map((f) => ( +
  • + {f.entry_ref_id.slice(0, 8)}{" "} + — {f.error} +
    • + ))} +
+
+ )} + + {isSchemaPromotion && ( + { + setPromotionOpen(next) + if (!next) setInlineError(null) + }} + reviewRefId={review.ref_id} + submitting={approving} + onConfirm={submitApproval} + /> + )} + {/* Expanded detail */} {expanded && (
diff --git a/src/components/admin/schema-promotion-dialog.tsx b/src/components/admin/schema-promotion-dialog.tsx new file mode 100644 index 0000000..3707754 --- /dev/null +++ b/src/components/admin/schema-promotion-dialog.tsx @@ -0,0 +1,462 @@ +"use client" + +import { useCallback, useEffect, useMemo, useState } from "react" +import { AlertTriangle, Loader2, Plus, Trash2 } from "lucide-react" + +import { Button } from "@/components/ui/button" +import { Checkbox } from "@/components/ui/checkbox" +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from "@/components/ui/dialog" +import { Input } from "@/components/ui/input" +import { SelectNative } from "@/components/ui/select-native" +import { + SCHEMA_ATTRIBUTE_TYPES, + getSchemaProposal, + type SchemaAttributeType, + type SchemaProposal, + type SchemaTypeOverride, +} from "@/lib/graph-api" +import { cn } from "@/lib/utils" + +/** + * A row in the editable property table. + * + * `origin` distinguishes properties inferred from the parked payloads from ones + * the admin added by hand — an added property has no sample and no presence + * count, and excluding an inferred one drops it from the replayed payload. + */ +interface PropertyRow { + id: string + name: string + type: SchemaAttributeType + required: boolean + included: boolean + origin: "inferred" | "added" + presentIn?: number + sample?: unknown +} + +function formatSample(sample: unknown): string { + if (sample === null || sample === undefined) return "—" + if (typeof sample === "string") return sample + return JSON.stringify(sample) +} + +function rowsFromProposal(proposal: SchemaProposal): PropertyRow[] { + return proposal.properties.map((p) => ({ + id: p.name, + name: p.name, + type: p.inferred_type, + required: p.suggested_required, + included: true, + origin: "inferred" as const, + presentIn: p.present_in, + sample: p.sample, + })) +} + +export interface SchemaPromotionDialogProps { + open: boolean + onOpenChange: (open: boolean) => void + reviewRefId: string + /** Called with the confirmed override; the caller performs the approve. */ + onConfirm: (override: SchemaTypeOverride) => Promise + submitting?: boolean +} + +export function SchemaPromotionDialog({ + open, + onOpenChange, + reviewRefId, + onConfirm, + submitting = false, +}: SchemaPromotionDialogProps) { + // One state object keyed by review so a stale response can never be mistaken + // for the current one, and loading is derived rather than set synchronously + // in the effect body (matches the fetch pattern used elsewhere in the app). + const [fetched, setFetched] = useState<{ + refId: string + proposal: SchemaProposal | null + error: string | null + } | null>(null) + + const [rows, setRows] = useState([]) + const [nodeKey, setNodeKey] = useState("") + const [parent, setParent] = useState("Thing") + + // Fetch on open. Aborting on close stops a slow response from landing on top + // of edits the admin has already started making after re-opening. + useEffect(() => { + if (!open) return + const controller = new AbortController() + getSchemaProposal(reviewRefId, controller.signal) + .then((result) => { + if (controller.signal.aborted) return + setFetched({ refId: reviewRefId, proposal: result, error: null }) + setRows(rowsFromProposal(result)) + setParent("Thing") + setNodeKey(result.node_key_candidates[0] ?? "") + }) + .catch((err: unknown) => { + if (controller.signal.aborted) return + setFetched({ + refId: reviewRefId, + proposal: null, + error: + err instanceof Error ? err.message : "Could not load the proposal", + }) + }) + return () => controller.abort() + }, [open, reviewRefId]) + + const current = fetched?.refId === reviewRefId ? fetched : null + const proposal = current?.proposal ?? null + const loadError = current?.error ?? null + const loading = open && current === null + + const updateRow = useCallback((id: string, patch: Partial) => { + setRows((prev) => + prev.map((r) => (r.id === id ? { ...r, ...patch } : r)) + ) + }, []) + + const addRow = useCallback(() => { + setRows((prev) => [ + ...prev, + { + id: `added-${prev.length}-${Date.now()}`, + name: "", + type: "string", + required: false, + included: true, + origin: "added", + }, + ]) + }, []) + + const removeRow = useCallback((id: string) => { + setRows((prev) => prev.filter((r) => r.id !== id)) + }, []) + + const includedRows = useMemo(() => rows.filter((r) => r.included), [rows]) + + // node_key components must be required attributes: an optional component + // makes every future write of this type fail validation. + const nodeKeyOptions = useMemo( + () => + includedRows.filter((r) => r.required && r.type === "string" && r.name), + [includedRows] + ) + + // Derived, not reset via an effect: when the row the selection points at is + // excluded, renamed, or made optional it simply stops being a valid choice, + // and submitting a stale value would fail validation server-side. + const effectiveNodeKey = + nodeKey && nodeKeyOptions.some((r) => r.name === nodeKey) ? nodeKey : "" + + const validationError = useMemo(() => { + if (!proposal) return null + if (!proposal.intended_type) return "This review has no intended type" + const names = includedRows.map((r) => r.name.trim()) + if (names.some((n) => !n)) return "Every included property needs a name" + const duplicates = names.filter((n, i) => names.indexOf(n) !== i) + if (duplicates.length > 0) { + return `Duplicate property name: ${duplicates[0]}` + } + const blocked = new Set(proposal.blocked_names.map((b) => b.name)) + const offending = names.find((n) => blocked.has(n)) + if (offending) { + return `"${offending}" is reserved — rename it` + } + if (includedRows.length === 0) { + return "Include at least one property" + } + return null + }, [proposal, includedRows]) + + const droppedNames = useMemo( + () => + rows + .filter((r) => r.origin === "inferred" && !r.included) + .map((r) => r.name), + [rows] + ) + + function handleConfirm() { + if (!proposal?.intended_type || validationError) return + const attributes: Record = {} + for (const row of includedRows) { + attributes[row.name.trim()] = row.required ? row.type : `?${row.type}` + } + const override: SchemaTypeOverride = { + type: proposal.intended_type, + parent: parent.trim() || "Thing", + attributes, + entries: proposal.entry_ref_ids, + } + if (effectiveNodeKey) { + override.node_key = `${proposal.intended_type.toLowerCase()}-${effectiveNodeKey}` + } + void onConfirm(override) + } + + return ( + + + + + Create type{" "} + + {proposal?.intended_type ?? "…"} + + + + {proposal + ? `Confirm the properties this type needs. ${proposal.entry_count} parked ${ + proposal.entry_count === 1 ? "entry" : "entries" + } will be replayed as ${ + proposal.entry_count === 1 ? "a node" : "nodes" + } of this type.` + : "Loading the proposed properties…"} + + + + {loading && ( +
+ + Inferring properties from the parked entries… +
+ )} + + {loadError && ( +
+ ✕ {loadError} +
+ )} + + {proposal && !loading && ( +
+ {proposal.conflicts.length > 0 && ( +
+ +
+ {proposal.conflicts.map((c) => ( +
+ {c.name} appeared as{" "} + {c.types_seen.join(" and ")} — widened to{" "} + {c.resolved_to}. +
+ ))} +
+
+ )} + + {proposal.blocked_names.length > 0 && ( +
+ +
+ {proposal.blocked_names.map((b) => ( +
+ {b.name} cannot be used:{" "} + {b.reason}. +
+ ))} +
+
+ )} + +
+ + + + + + + + + + + + {rows.map((row) => ( + + + + + + + + + ))} + +
UsePropertyType + Required + Sample +
+ + updateRow(row.id, { included: next }) + } + ariaLabel={`Include ${row.name || "new property"}`} + /> + + {row.origin === "added" ? ( + + updateRow(row.id, { name: e.target.value }) + } + /> + ) : ( + {row.name} + )} + {row.origin === "inferred" && + row.presentIn !== undefined && + row.presentIn < proposal.entry_count && ( + + in {row.presentIn}/{proposal.entry_count} + + )} + + ({ + value: t, + label: t, + }))} + onChange={(e) => + updateRow(row.id, { + type: e.target.value as SchemaAttributeType, + }) + } + className="h-7 text-[12px]" + /> + + + updateRow(row.id, { required: next }) + } + ariaLabel={`${row.name || "New property"} required`} + /> + + {row.origin === "added" + ? "—" + : formatSample(row.sample)} + + {row.origin === "added" && ( + + )} +
+
+ + + +
+ + +
+ + {droppedNames.length > 0 && ( +

+ Excluded from the type, and dropped when the parked entries are + replayed: {droppedNames.join(", ")} +

+ )} + + {proposal.unresolved_subject_ids.length > 0 && ( +

+ {proposal.unresolved_subject_ids.length} of this review's + subjects are no longer parked entries and will be skipped. +

+ )} + + {validationError && ( +
+ ✕ {validationError} +
+ )} +
+ )} + + + + + + + + ) +} diff --git a/src/lib/__tests__/schema-promotion-dialog.test.tsx b/src/lib/__tests__/schema-promotion-dialog.test.tsx new file mode 100644 index 0000000..d80bf6b --- /dev/null +++ b/src/lib/__tests__/schema-promotion-dialog.test.tsx @@ -0,0 +1,221 @@ +import { describe, it, expect, vi, beforeEach } from "vitest" +import { render, screen, waitFor } from "@testing-library/react" +import userEvent from "@testing-library/user-event" +import React from "react" + +import type { SchemaProposal } from "@/lib/graph-api" + +const { mockGetSchemaProposal } = vi.hoisted(() => ({ + mockGetSchemaProposal: vi.fn(), +})) + +vi.mock("@/lib/graph-api", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + getSchemaProposal: (...args: unknown[]) => mockGetSchemaProposal(...args), + } +}) + +import { SchemaPromotionDialog } from "@/components/admin/schema-promotion-dialog" + +function makeProposal(overrides: Partial = {}): SchemaProposal { + return { + review_ref_id: "rev-1", + intended_type: "PurchaseOrder", + status: "pending", + entry_count: 2, + entry_ref_ids: ["entry-1", "entry-2"], + unresolved_subject_ids: [], + properties: [ + { + name: "supplier", + inferred_type: "string", + present_in: 2, + suggested_required: true, + sample: "Acme Corp", + }, + { + name: "notes", + inferred_type: "string", + present_in: 1, + suggested_required: false, + sample: "rush", + }, + ], + conflicts: [], + node_key_candidates: ["supplier"], + blocked_names: [], + ...overrides, + } +} + +function renderDialog( + onConfirm = vi.fn().mockResolvedValue(undefined), + props: Partial> = {} +) { + render( + + ) + return onConfirm +} + +describe("SchemaPromotionDialog", () => { + beforeEach(() => { + vi.clearAllMocks() + mockGetSchemaProposal.mockResolvedValue(makeProposal()) + }) + + it("renders the inferred properties with their samples", async () => { + renderDialog() + expect(await screen.findByLabelText("Include supplier")).toBeInTheDocument() + expect(screen.getByText("notes")).toBeInTheDocument() + expect(screen.getByText("Acme Corp")).toBeInTheDocument() + }) + + it("shows how many entries supplied a partially-present property", async () => { + renderDialog() + expect(await screen.findByText("in 1/2")).toBeInTheDocument() + }) + + it("submits required properties bare and optional ones with a ? prefix", async () => { + const onConfirm = renderDialog() + await screen.findByLabelText("Include supplier") + + await userEvent.click(screen.getByTestId("schema-promotion-confirm")) + + await waitFor(() => expect(onConfirm).toHaveBeenCalled()) + const override = onConfirm.mock.calls[0][0] + expect(override.type).toBe("PurchaseOrder") + expect(override.attributes).toEqual({ + supplier: "string", + notes: "?string", + }) + }) + + it("sends the entry ref ids so the entries get promoted", async () => { + const onConfirm = renderDialog() + await screen.findByLabelText("Include supplier") + + await userEvent.click(screen.getByTestId("schema-promotion-confirm")) + + await waitFor(() => expect(onConfirm).toHaveBeenCalled()) + expect(onConfirm.mock.calls[0][0].entries).toEqual(["entry-1", "entry-2"]) + }) + + it("prefixes the node_key with the lowercased type", async () => { + const onConfirm = renderDialog() + await screen.findByLabelText("Include supplier") + + await userEvent.click(screen.getByTestId("schema-promotion-confirm")) + + await waitFor(() => expect(onConfirm).toHaveBeenCalled()) + expect(onConfirm.mock.calls[0][0].node_key).toBe("purchaseorder-supplier") + }) + + it("excludes a deselected property from the submitted attributes", async () => { + const onConfirm = renderDialog() + await screen.findByLabelText("Include supplier") + + await userEvent.click(screen.getByLabelText("Include notes")) + await userEvent.click(screen.getByTestId("schema-promotion-confirm")) + + await waitFor(() => expect(onConfirm).toHaveBeenCalled()) + expect(onConfirm.mock.calls[0][0].attributes).toEqual({ + supplier: "string", + }) + }) + + it("warns that excluded properties are dropped from the replay", async () => { + renderDialog() + await screen.findByLabelText("Include supplier") + + await userEvent.click(screen.getByLabelText("Include notes")) + + expect( + await screen.findByText(/dropped when the parked entries are replayed/i) + ).toBeInTheDocument() + }) + + it("surfaces type conflicts found across entries", async () => { + mockGetSchemaProposal.mockResolvedValue( + makeProposal({ + conflicts: [ + { name: "total", types_seen: ["int", "string"], resolved_to: "string" }, + ], + }) + ) + renderDialog() + expect(await screen.findByText(/widened to/i)).toBeInTheDocument() + }) + + it("blocks submission while a reserved name is still in the table", async () => { + mockGetSchemaProposal.mockResolvedValue( + makeProposal({ + properties: [ + { + name: "status", + inferred_type: "string", + present_in: 2, + suggested_required: true, + sample: "live", + }, + ], + blocked_names: [{ name: "status", reason: "reserved" }], + node_key_candidates: [], + }) + ) + renderDialog() + await screen.findByText(/cannot be used/i) + + expect(screen.getByTestId("schema-promotion-confirm")).toBeDisabled() + }) + + it("blocks submission when every property is excluded", async () => { + renderDialog() + await screen.findByLabelText("Include supplier") + + await userEvent.click(screen.getByLabelText("Include supplier")) + await userEvent.click(screen.getByLabelText("Include notes")) + + expect( + await screen.findByText(/include at least one property/i) + ).toBeInTheDocument() + expect(screen.getByTestId("schema-promotion-confirm")).toBeDisabled() + }) + + it("omits node_key when the chosen property is made optional", async () => { + const onConfirm = renderDialog() + await screen.findByLabelText("Include supplier") + + // supplier is the node_key candidate; making it optional disqualifies it + await userEvent.click(screen.getByLabelText("supplier required")) + await userEvent.click(screen.getByTestId("schema-promotion-confirm")) + + await waitFor(() => expect(onConfirm).toHaveBeenCalled()) + expect(onConfirm.mock.calls[0][0].node_key).toBeUndefined() + }) + + it("reports a load failure instead of rendering an empty table", async () => { + mockGetSchemaProposal.mockRejectedValue(new Error("boom")) + renderDialog() + expect(await screen.findByText(/boom/)).toBeInTheDocument() + expect(screen.getByTestId("schema-promotion-confirm")).toBeDisabled() + }) + + it("notes subjects that are no longer parked entries", async () => { + mockGetSchemaProposal.mockResolvedValue( + makeProposal({ unresolved_subject_ids: ["gone-1"] }) + ) + renderDialog() + expect( + await screen.findByText(/no longer parked entries and will be skipped/i) + ).toBeInTheDocument() + }) +}) diff --git a/src/lib/graph-api.ts b/src/lib/graph-api.ts index b1c7023..3de4356 100644 --- a/src/lib/graph-api.ts +++ b/src/lib/graph-api.ts @@ -867,6 +867,76 @@ function buildMockOntologyReviews(runRef: string, instruction: string): Review[] export type ReviewStatus = "pending" | "approved" | "dismissed" | "failed" +// ── Scratchpad promotion ───────────────────────────────────────────────────── +// A scratchpad_entry review creates a node type AND replays the parked entries +// as real nodes of it. Those are separate outcomes: the type can be created +// while individual entries fail to replay, so the summary reports per entry. + +/** Attribute type vocabulary accepted by the schema. "?" prefix = optional. */ +export const SCHEMA_ATTRIBUTE_TYPES = [ + "string", + "int", + "float", + "boolean", + "datetime", + "list", + "complex", +] as const + +export type SchemaAttributeType = (typeof SCHEMA_ATTRIBUTE_TYPES)[number] + +export interface ProposedProperty { + name: string + inferred_type: SchemaAttributeType + /** How many of the parked payloads supplied this property. */ + present_in: number + suggested_required: boolean + sample: unknown +} + +export interface SchemaProposal { + review_ref_id: string + intended_type: string | null + status: ReviewStatus + entry_count: number + entry_ref_ids: string[] + unresolved_subject_ids: string[] + properties: ProposedProperty[] + conflicts: Array<{ + name: string + types_seen: string[] + resolved_to: SchemaAttributeType + }> + node_key_candidates: string[] + blocked_names: Array<{ name: string; reason: string }> +} + +export interface PromotionSummary { + attempted: number + promoted: Array<{ + entry_ref_id: string + node_ref_id: string + dropped_properties: string[] + retired?: boolean + retire_error?: string + }> + failed: Array<{ entry_ref_id: string; error: string }> + skipped: Array<{ entry_ref_id: string; reason: string }> +} + +/** The admin's confirmed property table, sent back as the approve override. */ +export interface SchemaTypeOverride { + type: string + parent?: string + node_key?: string + attributes: Record + entries: string[] +} + +export type ReviewOverridePayload = + | { from: string[]; to: string } + | SchemaTypeOverride + export interface Review { ref_id: string type: string @@ -884,6 +954,7 @@ export interface Review { priority: number dismissal_reason?: string error_message?: string + promotion_summary?: PromotionSummary | null run_ref_id?: string created_at: string decided_at?: string @@ -1011,11 +1082,32 @@ export async function getReviewNodeTypeCounts( ) } +/** + * Fetch the property table proposed for a scratchpad_entry review's new type. + * + * Inference happens server-side: the review's rationale is redacted, so the UI + * has no access to the parked payloads and samples arrive already masked. + */ +export async function getSchemaProposal( + refId: string, + signal?: AbortSignal +): Promise { + return api.get( + `/v2/reviews/${refId}/schema_proposal`, + undefined, + signal + ) +} + export async function approveReview( refId: string, - overridePayload?: { from: string[]; to: string }, + overridePayload?: ReviewOverridePayload, signal?: AbortSignal -): Promise<{ status: string; error_message?: string }> { +): Promise<{ + status: string + error_message?: string + promotion_summary?: PromotionSummary | null +}> { if (isMocksEnabled()) { const store = getMockReviewsStore() const review = store.find((r) => r.ref_id === refId)