diff --git a/js/packages/ui/src/api/checks.ts b/js/packages/ui/src/api/checks.ts index 9fdd2f687..df5e718d1 100644 --- a/js/packages/ui/src/api/checks.ts +++ b/js/packages/ui/src/api/checks.ts @@ -26,6 +26,7 @@ export interface Check { is_preset?: boolean; last_run?: Run; is_outdated?: boolean; + actor_type?: string; // "recce_ai" | "user" | "preset_system" — Cloud-only, undefined in OSS } /** diff --git a/js/packages/ui/src/api/types/run.ts b/js/packages/ui/src/api/types/run.ts index 21cbcc16d..9e363a46f 100644 --- a/js/packages/ui/src/api/types/run.ts +++ b/js/packages/ui/src/api/types/run.ts @@ -98,6 +98,8 @@ export interface BaseRun { error?: string; /** Current status of the run */ status?: RunStatus; + /** Who triggered this run: "user" | "recce_ai" */ + triggered_by?: string; } // ============================================================================ diff --git a/js/packages/ui/src/components/check/CheckCard.tsx b/js/packages/ui/src/components/check/CheckCard.tsx index 670f14007..be65a3879 100644 --- a/js/packages/ui/src/components/check/CheckCard.tsx +++ b/js/packages/ui/src/components/check/CheckCard.tsx @@ -57,6 +57,8 @@ export interface CheckCardData { * dbt manifest or related artifacts are regenerated (shown in outdated tooltip). */ lastRunAt?: string; + /** Who created this check: "recce_ai" | "user" | "preset_system" */ + actorType?: string; } /** @@ -176,6 +178,23 @@ function formatOutdatedTooltip(lastRunAt?: string): string { } } +/** + * Badge config for actor type display. + * Exported so downstream components (e.g. CheckDetailOss) can reuse it. + */ +export const ACTOR_BADGE_CONFIG: Record< + string, + { label: string; color: string; bg: string } +> = { + recce_ai: { label: "AI", color: "#7c3aed", bg: "rgb(124 58 237 / 0.12)" }, + user: { label: "User", color: "#059669", bg: "rgb(5 150 105 / 0.12)" }, + preset_system: { + label: "Preset", + color: "#2563eb", + bg: "rgb(37 99 235 / 0.12)", + }, +}; + /** * CheckCard Component * @@ -332,6 +351,25 @@ function CheckCardComponent({ )} + {/* Actor badge — skip for presets (already shown by Preset chip) */} + {check.actorType && + !check.isPreset && + ACTOR_BADGE_CONFIG[check.actorType] && ( + + )} + {/* Preset badge */} {check.isPreset && ( )} + {/* Actor badge — skip for presets (already shown by Preset chip) */} + {check?.actor_type && + !check.is_preset && + ACTOR_BADGE_CONFIG[check.actor_type] && ( + + )} + diff --git a/js/packages/ui/src/components/check/CheckListOss.tsx b/js/packages/ui/src/components/check/CheckListOss.tsx index cd4ca36dc..55e58e04d 100644 --- a/js/packages/ui/src/components/check/CheckListOss.tsx +++ b/js/packages/ui/src/components/check/CheckListOss.tsx @@ -80,6 +80,7 @@ const ChecklistItem = ({ isPreset: check.is_preset, isOutdated: check.is_outdated, lastRunAt: check.last_run?.run_at, + actorType: check.actor_type, }; return ( diff --git a/js/packages/ui/src/components/check/__tests__/CheckCard.actorBadge.test.tsx b/js/packages/ui/src/components/check/__tests__/CheckCard.actorBadge.test.tsx new file mode 100644 index 000000000..c5b59001b --- /dev/null +++ b/js/packages/ui/src/components/check/__tests__/CheckCard.actorBadge.test.tsx @@ -0,0 +1,44 @@ +import { render, screen } from "@testing-library/react"; +import { describe, expect, it } from "vitest"; +import { CheckCard, type CheckCardData } from "../CheckCard"; + +const baseCheck: CheckCardData = { + id: "check-1", + name: "Row Count Diff of orders", + type: "row_count_diff", +}; + +describe("CheckCard actor badge", () => { + it("renders AI badge when actorType is recce_ai", () => { + render(); + expect(screen.getByText("AI")).toBeInTheDocument(); + }); + + it("skips actor badge when isPreset is true (Preset chip already shown)", () => { + render( + , + ); + // The outlined Preset chip (from isPreset) should exist + const presetChips = screen.getAllByText("Preset"); + expect(presetChips).toHaveLength(1); // Only the isPreset chip, no actor badge + }); + + it("does not render badge when actorType is undefined", () => { + render(); + expect(screen.queryByText("AI")).not.toBeInTheDocument(); + expect(screen.queryByText("User")).not.toBeInTheDocument(); + expect(screen.queryByText("Preset")).not.toBeInTheDocument(); + }); + + it("renders both AI and Outdated badges together", () => { + render( + , + ); + expect(screen.getByText("AI")).toBeInTheDocument(); + expect(screen.getByText("Outdated")).toBeInTheDocument(); + }); +}); diff --git a/js/packages/ui/src/components/check/timeline/CheckTimelineOss.tsx b/js/packages/ui/src/components/check/timeline/CheckTimelineOss.tsx index cefb7f932..9b37cec4c 100644 --- a/js/packages/ui/src/components/check/timeline/CheckTimelineOss.tsx +++ b/js/packages/ui/src/components/check/timeline/CheckTimelineOss.tsx @@ -15,19 +15,97 @@ import Divider from "@mui/material/Divider"; import Stack from "@mui/material/Stack"; import Typography from "@mui/material/Typography"; import { useQuery } from "@tanstack/react-query"; +import { useMemo } from "react"; import { cacheKeys } from "../../../api"; +import type { CheckEvent } from "../../../api/checkEvents"; +import { listRuns } from "../../../api/runs"; import { useApiConfig, useCheckEvents, useIsDark } from "../../../hooks"; import { fetchUser } from "../../../lib/api/user"; import { CommentInput } from "../../../primitives"; +import { type RunEntry, RunTimelineEntry } from "./RunTimelineEntry"; import { TimelineEventOss as TimelineEvent } from "./TimelineEventOss"; +// ============================================================================ +// Helpers +// ============================================================================ + +/** + * Map OSS run status to display status using the error field. + * RunStatus enum values are capitalized: "Finished", "Failed", "Running", "Cancelled". + */ +function deriveRunStatus( + status: string | undefined, + error: string | undefined | null, +): string { + const s = status?.toLowerCase(); + if (s === "finished") { + return error ? "error" : "success"; + } + if (s === "failed") { + return "error"; + } + // "running", "cancelled" pass through + return status ?? "unknown"; +} + +// ============================================================================ +// Types +// ============================================================================ + +export type TimelineEntry = + | { kind: "event"; event: CheckEvent; at: string } + | { kind: "run"; run: RunEntry; index: number; at: string }; + +// ============================================================================ +// Pure merge function (exported for testing) +// ============================================================================ + +/** + * Merges check events and run entries into a single chronologically sorted + * list (descending — newest first). Runs are numbered from oldest (#1) to + * newest (#N) so the index reflects execution order. + */ +export function mergeTimelineEntries( + events: CheckEvent[], + runs: RunEntry[] | undefined, +): TimelineEntry[] { + const entries: TimelineEntry[] = []; + + for (const event of events) { + entries.push({ kind: "event", event, at: event.created_at }); + } + + if (runs) { + // Sort ascending to assign indices from oldest (#1) to newest (#N) + const sortedRuns = [...runs].sort( + (a, b) => new Date(a.run_at).getTime() - new Date(b.run_at).getTime(), + ); + for (let i = 0; i < sortedRuns.length; i++) { + entries.push({ + kind: "run", + run: sortedRuns[i], + index: i + 1, + at: sortedRuns[i].run_at, + }); + } + } + + // Sort descending (newest first) for display + entries.sort((a, b) => new Date(b.at).getTime() - new Date(a.at).getTime()); + return entries; +} + +// ============================================================================ +// Component +// ============================================================================ + interface CheckTimelineProps { checkId: string; } export function CheckTimelineOss({ checkId }: CheckTimelineProps) { const isDark = useIsDark(); - const { apiClient } = useApiConfig(); + const { apiClient, authToken } = useApiConfig(); const { events, isLoading, @@ -45,6 +123,32 @@ export function CheckTimelineOss({ checkId }: CheckTimelineProps) { retry: false, }); + // Fetch runs only in cloudMode (authToken present = Cloud) + const { data: checkRuns } = useQuery({ + queryKey: ["check-runs", checkId], + queryFn: async () => { + const allRuns = await listRuns(apiClient); + return allRuns + .filter((r) => r.check_id === checkId) + .map( + (r): RunEntry => ({ + run_id: r.run_id, + run_at: r.run_at, + status: deriveRunStatus(r.status, r.error), + summary: r.error || undefined, + triggered_by: r.triggered_by, + }), + ); + }, + enabled: !!authToken, + staleTime: 30000, + }); + + const timelineEntries = useMemo( + () => mergeTimelineEntries(events, checkRuns), + [events, checkRuns], + ); + const handleCreateComment = (content: string) => { createComment(content); }; @@ -117,21 +221,27 @@ export function CheckTimelineOss({ checkId }: CheckTimelineProps) { {/* Events List - Scrollable */} - {events.length === 0 ? ( + {timelineEntries.length === 0 ? ( No activity yet ) : ( - {events.map((event, index) => ( - - - {index < events.length - 1 && ( + {timelineEntries.map((entry, index) => ( + + {entry.kind === "event" ? ( + + ) : ( + + )} + {index < timelineEntries.length - 1 && ( diff --git a/js/packages/ui/src/components/check/timeline/RunTimelineEntry.tsx b/js/packages/ui/src/components/check/timeline/RunTimelineEntry.tsx new file mode 100644 index 000000000..592df7a33 --- /dev/null +++ b/js/packages/ui/src/components/check/timeline/RunTimelineEntry.tsx @@ -0,0 +1,104 @@ +import { Box, Typography } from "@mui/material"; +import { formatDistanceToNow } from "date-fns"; + +export interface RunEntry { + run_id: string; + run_at: string; + status: string; + summary?: string; + triggered_by?: string; // "user" | "recce_ai" +} + +const STATUS_COLORS: Record = { + success: "#059669", + failure: "#ef4444", + error: "#ef4444", + warning: "#f59e0b", +}; + +const STATUS_LABELS: Record = { + success: "PASSED", + failure: "FAILED", + error: "ERROR", + warning: "WARNING", +}; + +function getStatusColor(status: string): string { + return STATUS_COLORS[status] ?? "#6b7280"; +} + +function getStatusLabel(status: string): string { + return STATUS_LABELS[status] ?? status.toUpperCase(); +} + +interface RunTimelineEntryProps { + run: RunEntry; + index: number; + onClick?: (runId: string) => void; +} + +export function RunTimelineEntry({ + run, + index, + onClick, +}: RunTimelineEntryProps) { + const color = getStatusColor(run.status); + const label = getStatusLabel(run.status); + const timeAgo = formatDistanceToNow(new Date(run.run_at), { + addSuffix: true, + }); + const isInteractive = Boolean(onClick); + + return ( + onClick?.(run.run_id) : undefined} + aria-label={isInteractive ? `View run #${index} — ${label}` : undefined} + sx={{ + p: 1, + borderLeft: `3px solid ${color}`, + borderRadius: 1, + bgcolor: "action.hover", + cursor: isInteractive ? "pointer" : "default", + "&:hover": isInteractive ? { bgcolor: "action.selected" } : {}, + textAlign: "left", + width: "100%", + border: "none", + font: "inherit", + color: "inherit", + }} + > + + Run #{index} — {label} + {run.triggered_by === "recce_ai" && ( + + by AI + + )} + + {run.summary && ( + + {run.summary} + + )} + + {timeAgo} + + + ); +} diff --git a/js/packages/ui/src/components/check/timeline/__tests__/CheckTimelineOss.merge.test.tsx b/js/packages/ui/src/components/check/timeline/__tests__/CheckTimelineOss.merge.test.tsx new file mode 100644 index 000000000..26d5943fd --- /dev/null +++ b/js/packages/ui/src/components/check/timeline/__tests__/CheckTimelineOss.merge.test.tsx @@ -0,0 +1,44 @@ +import { describe, expect, it } from "vitest"; +import type { CheckEvent } from "../../../../api/checkEvents"; +import { mergeTimelineEntries } from "../CheckTimelineOss"; +import type { RunEntry } from "../RunTimelineEntry"; + +describe("mergeTimelineEntries", () => { + it("sorts events and runs by timestamp descending", () => { + const events: CheckEvent[] = [ + { id: "e1", created_at: "2026-03-22T10:00:00Z" } as CheckEvent, + ]; + const runs: RunEntry[] = [ + { run_id: "r1", run_at: "2026-03-21T09:00:00Z", status: "success" }, + { run_id: "r2", run_at: "2026-03-23T14:00:00Z", status: "failure" }, + ]; + + const result = mergeTimelineEntries(events, runs); + expect(result[0].kind).toBe("run"); // r2 (newest) + expect(result[1].kind).toBe("event"); // e1 + expect(result[2].kind).toBe("run"); // r1 (oldest) + }); + + it("numbers runs from oldest to newest", () => { + const runs: RunEntry[] = [ + { run_id: "r1", run_at: "2026-03-21T09:00:00Z", status: "success" }, + { run_id: "r2", run_at: "2026-03-22T10:00:00Z", status: "failure" }, + ]; + + const result = mergeTimelineEntries([], runs); + const runEntries = result.filter( + (e): e is Extract => e.kind === "run", + ); + expect(runEntries[0].index).toBe(2); // newest run = #2 (sorted desc) + expect(runEntries[1].index).toBe(1); // oldest run = #1 + }); + + it("returns events only when no runs provided", () => { + const events: CheckEvent[] = [ + { id: "e1", created_at: "2026-03-22T10:00:00Z" } as CheckEvent, + ]; + const result = mergeTimelineEntries(events, undefined); + expect(result).toHaveLength(1); + expect(result[0].kind).toBe("event"); + }); +}); diff --git a/js/packages/ui/src/components/check/timeline/__tests__/RunTimelineEntry.test.tsx b/js/packages/ui/src/components/check/timeline/__tests__/RunTimelineEntry.test.tsx new file mode 100644 index 000000000..6da2c1a4c --- /dev/null +++ b/js/packages/ui/src/components/check/timeline/__tests__/RunTimelineEntry.test.tsx @@ -0,0 +1,47 @@ +import { render, screen } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; +import { type RunEntry, RunTimelineEntry } from "../RunTimelineEntry"; + +const baseRun: RunEntry = { + run_id: "run-1", + run_at: "2026-03-23T14:30:00Z", + status: "success", +}; + +describe("RunTimelineEntry", () => { + it("renders run status and timestamp", () => { + render(); + expect(screen.getByText(/Run #1/)).toBeInTheDocument(); + }); + + it("renders summary when provided", () => { + render( + , + ); + expect(screen.getByText("30% reduction")).toBeInTheDocument(); + }); + + it("calls onClick when clicked", () => { + const onClick = vi.fn(); + render(); + screen.getByText(/Run #1/).click(); + expect(onClick).toHaveBeenCalledWith("run-1"); + }); + + it("renders as a button with aria-label when onClick is provided", () => { + const onClick = vi.fn(); + render(); + const button = screen.getByRole("button", { + name: /View run #1/, + }); + expect(button).toBeInTheDocument(); + }); + + it("renders as a div without aria-label when onClick is not provided", () => { + render(); + expect(screen.queryByRole("button")).not.toBeInTheDocument(); + }); +}); diff --git a/js/packages/ui/src/components/check/timeline/index.ts b/js/packages/ui/src/components/check/timeline/index.ts index 52798eed4..0fd49ec68 100644 --- a/js/packages/ui/src/components/check/timeline/index.ts +++ b/js/packages/ui/src/components/check/timeline/index.ts @@ -4,6 +4,7 @@ export { CheckTimelineOss } from "./CheckTimelineOss"; export { CommentInput, type CommentInputProps } from "./CommentInput"; +export { type RunEntry, RunTimelineEntry } from "./RunTimelineEntry"; export { type TimelineActor, TimelineEvent, diff --git a/recce/apis/check_api.py b/recce/apis/check_api.py index ce0cc180b..90677613e 100644 --- a/recce/apis/check_api.py +++ b/recce/apis/check_api.py @@ -162,7 +162,7 @@ async def run_check_handler(check_id: UUID, input: RunCheckIn): rerun=True, ), ) - run, future = submit_run(check.type, check.params, check_id=check_id) + run, future = submit_run(check.type, check.params, check_id=check_id, triggered_by="user") except RecceException as e: raise HTTPException(status_code=400, detail=str(e)) diff --git a/recce/apis/run_api.py b/recce/apis/run_api.py index 88f29169a..c6e2cbdee 100644 --- a/recce/apis/run_api.py +++ b/recce/apis/run_api.py @@ -56,7 +56,7 @@ async def create_run_handler(input: CreateRunIn): ), ) try: - run, future = submit_run(input.type, input.params) + run, future = submit_run(input.type, input.params, check_id=input.check_id, triggered_by="user") except RecceException as e: raise HTTPException(status_code=400, detail=str(e)) @@ -102,6 +102,8 @@ async def list_run_handler(): "params": run.params, "status": run.status, "check_id": run.check_id, + "error": run.error, + "triggered_by": run.triggered_by, } for run in runs ] diff --git a/recce/apis/run_func.py b/recce/apis/run_func.py index 9a23bea72..67db18f72 100644 --- a/recce/apis/run_func.py +++ b/recce/apis/run_func.py @@ -52,15 +52,20 @@ def generate_run_name(run): model = params.get("model") return f"profile diff of {model}".capitalize() elif run_type == RunType.ROW_COUNT_DIFF: + # MCP uses node_names (array) or node_ids (array of fully-qualified IDs) nodes = params.get("node_names") if nodes: if len(nodes) == 1: - node = nodes[0] - return f"row count diff of {node}".capitalize() + return f"row count diff of {nodes[0]}".capitalize() else: return f"row count of {len(nodes)} nodes".capitalize() - else: - return "row count of multiple nodes".capitalize() + node_ids = params.get("node_ids") + if node_ids: + if len(node_ids) == 1: + return f"row count diff of {node_ids[0].split('.')[-1]}".capitalize() + else: + return f"row count of {len(node_ids)} nodes".capitalize() + return "row count of multiple nodes".capitalize() elif run_type == RunType.TOP_K_DIFF: model = params.get("model") column = params.get("column_name") @@ -69,6 +74,24 @@ def generate_run_name(run): model = params.get("model") column = params.get("column_name") return f"histogram diff of {model}.{column} ".capitalize() + elif run_type == RunType.LINEAGE_DIFF: + return "Lineage diff" + elif run_type == RunType.SCHEMA_DIFF: + # REST API uses node_id (single), MCP uses node_names/node_ids (arrays) + node_id = params.get("node_id") + if node_id: + return f"Schema diff of {node_id.split('.')[-1]}" + node_names = params.get("node_names") + if node_names and len(node_names) == 1: + return f"Schema diff of {node_names[0]}" + elif node_names: + return f"Schema diff of {len(node_names)} nodes" + node_ids = params.get("node_ids") + if node_ids and len(node_ids) == 1: + return f"Schema diff of {node_ids[0].split('.')[-1]}" + elif node_ids: + return f"Schema diff of {len(node_ids)} nodes" + return "Schema diff" else: return f"{'run'.capitalize()} - {now}" @@ -92,7 +115,7 @@ def create_task(run_type: RunType, params: dict): return taskClz(params) -def submit_run(type, params, check_id=None): +def submit_run(type, params, check_id=None, triggered_by=None): try: run_type = RunType(type) except ValueError: @@ -111,7 +134,7 @@ def submit_run(type, params, check_id=None): if dbt_adaptor.adapter is None: raise RecceException("Recce Server is not launched under DBT project folder.") - run = Run(type=run_type, params=params, check_id=check_id, status=RunStatus.RUNNING) + run = Run(type=run_type, params=params, check_id=check_id, status=RunStatus.RUNNING, triggered_by=triggered_by) run.name = generate_run_name(run) RunDAO().create(run) diff --git a/recce/mcp_server.py b/recce/mcp_server.py index 0c8c5530b..b013fd12f 100644 --- a/recce/mcp_server.py +++ b/recce/mcp_server.py @@ -660,7 +660,7 @@ async def list_tools() -> List[Tool]: ), Tool( name="run_check", - description="Run a single check by ID and wait for completion. Returns execution status, results, and approval status.", + description="Run a single check by ID and wait for completion. Returns a Run object with fields: run_id, type, check_id, status, result, error, run_at, triggered_by.", inputSchema={ "type": "object", "properties": { @@ -668,6 +668,11 @@ async def list_tools() -> List[Tool]: "type": "string", "description": "The ID of the check to run", }, + "triggered_by": { + "type": "string", + "enum": ["user", "recce_ai"], + "description": "Who triggered this run. Defaults to 'user'.", + }, }, "required": ["check_id"], }, @@ -712,6 +717,11 @@ async def list_tools() -> List[Tool]: "type": "string", "description": "Analysis summary explaining what was found and why it matters", }, + "triggered_by": { + "type": "string", + "enum": ["user", "recce_ai"], + "description": "Who triggered this run. Defaults to 'user'.", + }, }, "required": ["type", "params", "name"], }, @@ -1617,6 +1627,28 @@ async def _tool_list_checks(self, arguments: Dict[str, Any]) -> Dict[str, Any]: return result + def _create_metadata_run(self, check_type, params, check_id, result, triggered_by): + """Create a Run record for metadata-only check types (no DB query). + + These types (lineage_diff, schema_diff) read from dbt manifest, not from + database queries. We still create a Run record so they appear in Activity. + """ + from recce.apis.run_func import generate_run_name + from recce.models import Run, RunDAO + from recce.models.types import RunStatus + + run = Run( + type=check_type, + params=params, + check_id=check_id, + status=RunStatus.FINISHED, + result=result, + triggered_by=triggered_by, + ) + run.name = generate_run_name(run) + RunDAO().create(run) + return run + async def _tool_run_check(self, arguments: Dict[str, Any]) -> Dict[str, Any]: """Run a single check by ID""" from recce.apis.run_func import submit_run @@ -1631,14 +1663,29 @@ async def _tool_run_check(self, arguments: Dict[str, Any]) -> Dict[str, Any]: if not check: raise ValueError(f"Check with ID {check_id} not found") - if check.type == RunType.LINEAGE_DIFF: - return await self._tool_lineage_diff(check.params or {}) + triggered_by = arguments.get("triggered_by", "user") - if check.type == RunType.SCHEMA_DIFF: - return await self._tool_schema_diff(check.params or {}) + if check.type in (RunType.LINEAGE_DIFF, RunType.SCHEMA_DIFF): + try: + if check.type == RunType.LINEAGE_DIFF: + result = await self._tool_lineage_diff(check.params or {}) + else: + result = await self._tool_schema_diff(check.params or {}) + run = self._create_metadata_run( + check_type=check.type, + params=check.params or {}, + check_id=check_id, + result=result, + triggered_by=triggered_by, + ) + return run.model_dump(mode="json") + except RecceException as e: + raise ValueError(str(e)) from e try: - run, future = submit_run(check.type, params=check.params or {}, check_id=check_id) + run, future = submit_run( + check.type, params=check.params or {}, check_id=check_id, triggered_by=triggered_by + ) run.result = await future return run.model_dump(mode="json") except RecceException as e: @@ -1685,12 +1732,29 @@ async def _tool_create_check(self, arguments: Dict[str, Any]) -> Dict[str, Any]: check_id = check.check_id created = True - # Auto-run for evidence. Skip for metadata-only types - # (schema_diff/lineage_diff read from manifest, not DB query) + # Auto-run for evidence run_executed = False run_error = None - if check_type not in (RunType.LINEAGE_DIFF, RunType.SCHEMA_DIFF): - run, future = submit_run(check_type, params=params, check_id=check_id) + triggered_by = arguments.get("triggered_by", "user") + if check_type in (RunType.LINEAGE_DIFF, RunType.SCHEMA_DIFF): + # Metadata-only: read from manifest, create Run record for Activity + try: + if check_type == RunType.LINEAGE_DIFF: + result = await self._tool_lineage_diff(params) + else: + result = await self._tool_schema_diff(params) + self._create_metadata_run( + check_type=check_type, + params=params, + check_id=check_id, + result=result, + triggered_by=triggered_by, + ) + run_executed = True + except Exception as e: + run_error = str(e) + else: + run, future = submit_run(check_type, params=params, check_id=check_id, triggered_by=triggered_by) await future # submit_run's future always resolves (errors caught internally). # Check run.status, not the return value. diff --git a/recce/models/check.py b/recce/models/check.py index 354e63c7c..17406db45 100644 --- a/recce/models/check.py +++ b/recce/models/check.py @@ -212,7 +212,8 @@ def create(self, check: Check) -> Check: Create a new check. In local mode: Appends check to in-memory list - In cloud mode: Creates check via Recce Cloud API + In cloud mode: Write-through — creates check via Recce Cloud API AND + keeps in local state so runs can reference it and state export includes it. Args: check: Check object to create @@ -237,7 +238,13 @@ def create(self, check: Check) -> Check: ) new_check = self._cloud_to_check(cloud_check) - logger.debug(f"Created check {new_check.check_id} in cloud") + # Write-through: also keep in local state so that: + # 1. Runs created after this check can reference it + # 2. export_persistent_state() includes checks in recce_state.json + # 3. Preview instance gets checks + runs together from S3 sync + self._checks.append(new_check) + + logger.debug(f"Created check {new_check.check_id} in cloud and local state") return new_check except Exception as e: logger.error(f"Failed to create check in cloud: {e}") diff --git a/recce/models/types.py b/recce/models/types.py index 3d7c374ea..6c6f67695 100644 --- a/recce/models/types.py +++ b/recce/models/types.py @@ -51,6 +51,7 @@ class Run(BaseModel): progress: Optional[RunProgress] = None run_id: UUID4 = Field(default_factory=uuid.uuid4) run_at: str = Field(default_factory=lambda: datetime.utcnow().strftime("%Y-%m-%dT%H:%M:%SZ")) + triggered_by: Optional[Literal["user", "recce_ai"]] = None # who triggered the run def __init__(self, **data): # Normalize status for backward compatibility (lowercase -> capitalized) diff --git a/recce/state/cloud.py b/recce/state/cloud.py index a8f223822..57c68d1b2 100644 --- a/recce/state/cloud.py +++ b/recce/state/cloud.py @@ -424,7 +424,7 @@ def _export_state_to_session(self) -> Tuple[Union[str, None], None]: # Create a copy of the state with empty artifacts for upload upload_state = RecceState() upload_state.runs = self.state.runs.copy() if self.state.runs else [] - upload_state.checks = [] + upload_state.checks = self.state.checks.copy() if self.state.checks else [] # Keep artifacts empty (don't copy self.state.artifacts) # Upload the state with empty artifacts diff --git a/recce/util/recce_cloud.py b/recce/util/recce_cloud.py index f71da5c8a..63e87feb8 100644 --- a/recce/util/recce_cloud.py +++ b/recce/util/recce_cloud.py @@ -39,6 +39,9 @@ def __init__(self, message: str, reason: str, status_code: int): pass self.reason = reason + def __str__(self): + return f"{self.args[0]} [HTTP {self.status_code}] {self.reason}" + class RecceCloud: def __init__(self, token: str): diff --git a/tests/apis/test_run_func.py b/tests/apis/test_run_func.py index 106c82038..7d935b293 100644 --- a/tests/apis/test_run_func.py +++ b/tests/apis/test_run_func.py @@ -351,6 +351,47 @@ async def test_normalized_params_propagate_to_run(self, mock_context, mock_task_ # Verify params were normalized assert run.params["primary_key"] == ["CUSTOMER_ID"] + @pytest.mark.asyncio + async def test_triggered_by_propagates_to_run(self, mock_context, mock_task_class): + """Test that triggered_by parameter is set on the created Run object.""" + from recce.apis.run_func import submit_run + + with patch("recce.apis.run_func.create_task") as mock_create_task: + mock_task = mock_task_class({"model": "customers", "primary_key": ["customer_id"]}) + mock_create_task.return_value = mock_task + + run, future = submit_run( + type="value_diff", + params={"model": "customers", "primary_key": ["customer_id"]}, + triggered_by="recce_ai", + ) + + assert run.triggered_by == "recce_ai" + + # Clean up: wait for the future to complete + await asyncio.wrap_future(future) + await asyncio.sleep(0.1) + + @pytest.mark.asyncio + async def test_triggered_by_defaults_to_none(self, mock_context, mock_task_class): + """Test that triggered_by defaults to None when not specified.""" + from recce.apis.run_func import submit_run + + with patch("recce.apis.run_func.create_task") as mock_create_task: + mock_task = mock_task_class({"model": "customers", "primary_key": ["customer_id"]}) + mock_create_task.return_value = mock_task + + run, future = submit_run( + type="value_diff", + params={"model": "customers", "primary_key": ["customer_id"]}, + ) + + assert run.triggered_by is None + + # Clean up: wait for the future to complete + await asyncio.wrap_future(future) + await asyncio.sleep(0.1) + @pytest.mark.asyncio async def test_run_params_unchanged_when_task_has_no_params(self, mock_context): """Test that run.params is unchanged when task.params is None.""" @@ -438,3 +479,101 @@ def test_nested_params_are_merged(self): # Note: dict.update() does shallow merge, so options is replaced entirely assert original["options"] == {"limit": 50} assert "offset" not in original["options"] + + +# ============================================================================= +# Tests: generate_run_name for metadata types +# ============================================================================= + + +class TestGenerateRunNameRowCountDiff: + """Tests for generate_run_name with row_count_diff node_ids fallback.""" + + def test_row_count_diff_node_names_single(self): + from recce.apis.run_func import generate_run_name + from recce.models.types import Run, RunType + + run = Run(type=RunType.ROW_COUNT_DIFF, params={"node_names": ["customers"]}) + assert generate_run_name(run) == "Row count diff of customers" + + def test_row_count_diff_node_ids_single(self): + from recce.apis.run_func import generate_run_name + from recce.models.types import Run, RunType + + run = Run(type=RunType.ROW_COUNT_DIFF, params={"node_ids": ["model.jaffle_shop.customers"]}) + assert generate_run_name(run) == "Row count diff of customers" + + def test_row_count_diff_node_ids_multiple(self): + from recce.apis.run_func import generate_run_name + from recce.models.types import Run, RunType + + run = Run( + type=RunType.ROW_COUNT_DIFF, + params={"node_ids": ["model.jaffle_shop.customers", "model.jaffle_shop.orders"]}, + ) + assert generate_run_name(run) == "Row count of 2 nodes" + + def test_row_count_diff_no_params(self): + from recce.apis.run_func import generate_run_name + from recce.models.types import Run, RunType + + run = Run(type=RunType.ROW_COUNT_DIFF, params={}) + assert generate_run_name(run) == "Row count of multiple nodes" + + +class TestGenerateRunNameMetadataTypes: + """Tests for generate_run_name with lineage_diff and schema_diff types.""" + + def test_lineage_diff_name(self): + from recce.apis.run_func import generate_run_name + from recce.models.types import Run, RunType + + run = Run(type=RunType.LINEAGE_DIFF, params={}) + assert generate_run_name(run) == "Lineage diff" + + def test_schema_diff_no_params(self): + from recce.apis.run_func import generate_run_name + from recce.models.types import Run, RunType + + run = Run(type=RunType.SCHEMA_DIFF, params={}) + assert generate_run_name(run) == "Schema diff" + + # REST API convention: node_id (single string, fully-qualified) + def test_schema_diff_node_id(self): + from recce.apis.run_func import generate_run_name + from recce.models.types import Run, RunType + + run = Run(type=RunType.SCHEMA_DIFF, params={"node_id": "model.jaffle_shop.customers"}) + assert generate_run_name(run) == "Schema diff of customers" + + # MCP convention: node_names (array of short names) + def test_schema_diff_single_node_name(self): + from recce.apis.run_func import generate_run_name + from recce.models.types import Run, RunType + + run = Run(type=RunType.SCHEMA_DIFF, params={"node_names": ["customers"]}) + assert generate_run_name(run) == "Schema diff of customers" + + def test_schema_diff_multiple_node_names(self): + from recce.apis.run_func import generate_run_name + from recce.models.types import Run, RunType + + run = Run(type=RunType.SCHEMA_DIFF, params={"node_names": ["customers", "orders"]}) + assert generate_run_name(run) == "Schema diff of 2 nodes" + + # MCP convention: node_ids (array of fully-qualified IDs) + def test_schema_diff_single_node_id_array(self): + from recce.apis.run_func import generate_run_name + from recce.models.types import Run, RunType + + run = Run(type=RunType.SCHEMA_DIFF, params={"node_ids": ["model.jaffle_shop.customers"]}) + assert generate_run_name(run) == "Schema diff of customers" + + def test_schema_diff_multiple_node_ids(self): + from recce.apis.run_func import generate_run_name + from recce.models.types import Run, RunType + + run = Run( + type=RunType.SCHEMA_DIFF, params={"node_ids": ["model.jaffle_shop.customers", "model.jaffle_shop.orders"]} + ) + assert generate_run_name(run) == "Schema diff of 2 nodes" diff --git a/tests/state/test_cloud.py b/tests/state/test_cloud.py index 5ce41145f..586d051c0 100644 --- a/tests/state/test_cloud.py +++ b/tests/state/test_cloud.py @@ -596,7 +596,7 @@ def test_export_state_to_session_success(self, mock_put): # Verify runs and checks were copied self.assertEqual(mock_upload_state.runs, [{"id": "run1"}, {"id": "run2"}]) - self.assertEqual(mock_upload_state.checks, []) + self.assertEqual(mock_upload_state.checks, [{"id": "check1"}]) def test_export_state_to_session_missing_session_id(self): loader = CloudStateLoader(cloud_options={"api_token": "token"}) diff --git a/tests/test_mcp_e2e.py b/tests/test_mcp_e2e.py index f2bbc7e8b..bf9aba333 100644 --- a/tests/test_mcp_e2e.py +++ b/tests/test_mcp_e2e.py @@ -752,8 +752,13 @@ async def test_run_check_lineage_diff(self, mcp_e2e_with_data): ) ) result = await server._tool_run_check({"check_id": str(check.check_id)}) - assert "nodes" in result - assert "edges" in result + # _tool_run_check now returns a Run object with result nested inside + assert "run_id" in result + assert "type" in result + assert result["type"] == "lineage_diff" + assert result["result"] is not None + assert "nodes" in result["result"] + assert "edges" in result["result"] @pytest.mark.asyncio async def test_run_check_schema_diff(self, mcp_e2e_with_data): @@ -770,8 +775,13 @@ async def test_run_check_schema_diff(self, mcp_e2e_with_data): ) ) result = await server._tool_run_check({"check_id": str(check.check_id)}) - assert "columns" in result - assert "data" in result + # _tool_run_check now returns a Run object with result nested inside + assert "run_id" in result + assert "type" in result + assert result["type"] == "schema_diff" + assert result["result"] is not None + assert "columns" in result["result"] + assert "data" in result["result"] @pytest.mark.asyncio async def test_run_check_not_found(self, mcp_e2e_with_data): diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py index 36965417f..251e79fcb 100644 --- a/tests/test_mcp_server.py +++ b/tests/test_mcp_server.py @@ -347,6 +347,7 @@ async def test_tool_create_check_basic(self, mcp_server): RunType.ROW_COUNT_DIFF, params={"node_names": ["orders"]}, check_id=check_id, + triggered_by="user", ) @pytest.mark.asyncio @@ -396,21 +397,36 @@ async def test_tool_create_check_idempotent_update(self, mcp_server): assert patch_in.is_checked is None # Not touching approval @pytest.mark.asyncio - async def test_tool_create_check_skips_run_for_schema_diff(self, mcp_server): - """create_check with schema_diff type does not submit a run.""" - server, _ = mcp_server + async def test_tool_create_check_metadata_run_for_schema_diff(self, mcp_server): + """create_check with schema_diff creates a metadata run (no submit_run).""" + server, mock_context = mcp_server + from uuid import uuid4 + check_id = uuid4() mock_check = MagicMock() - mock_check.check_id = MagicMock() + mock_check.check_id = check_id mock_check_dao = MagicMock() mock_check_dao.list.return_value = [] + # Mock lineage diff for schema_diff tool + mock_lineage_diff = MagicMock(spec=LineageDiff) + mock_lineage_diff.model_dump.return_value = { + "base": {"nodes": {}, "parent_map": {}}, + "current": {"nodes": {}, "parent_map": {}}, + } + mock_context.get_lineage_diff.return_value = mock_lineage_diff + mock_context.adapter.select_nodes.return_value = set() + + # Mock RunDAO to avoid default_context() call in _create_metadata_run + mock_run_dao = MagicMock() + with ( patch("recce.models.CheckDAO", return_value=mock_check_dao), patch("recce.apis.check_func.create_check_without_run", return_value=mock_check), patch("recce.apis.run_func.submit_run") as mock_submit, patch("recce.apis.check_func.export_persistent_state"), + patch("recce.models.RunDAO", return_value=mock_run_dao), ): result = await server._tool_create_check( { @@ -420,8 +436,10 @@ async def test_tool_create_check_skips_run_for_schema_diff(self, mcp_server): } ) - assert result["run_executed"] is False + assert result["run_executed"] is True mock_submit.assert_not_called() + # Verify a metadata run was created via RunDAO + mock_run_dao.create.assert_called_once() @pytest.mark.asyncio async def test_tool_create_check_run_failure(self, mcp_server): @@ -571,16 +589,22 @@ async def test_tool_run_check_with_lineage_diff(self, mcp_server): mock_context.get_lineage_diff.return_value = mock_lineage_diff mock_context.adapter.select_nodes.return_value = set() - # Mock CheckDAO + # Mock CheckDAO and RunDAO (for _create_metadata_run) mock_check_dao = MagicMock() mock_check_dao.find_check_by_id.return_value = mock_check + mock_run_dao = MagicMock() - with patch("recce.models.CheckDAO", return_value=mock_check_dao): + with ( + patch("recce.models.CheckDAO", return_value=mock_check_dao), + patch("recce.models.RunDAO", return_value=mock_run_dao), + ): result = await server._tool_run_check({"check_id": str(check_id)}) - # Verify the result is from lineage_diff tool (has nodes and edges) - assert "nodes" in result - assert "edges" in result + # Result is now a Run.model_dump() dict (not raw lineage_diff result) + assert "type" in result + assert "check_id" in result + # Verify a metadata run was persisted + mock_run_dao.create.assert_called_once() @pytest.mark.asyncio async def test_tool_run_check_with_schema_diff(self, mcp_server): @@ -609,18 +633,22 @@ async def test_tool_run_check_with_schema_diff(self, mcp_server): mock_context.get_lineage_diff.return_value = mock_lineage_diff mock_context.adapter.select_nodes.return_value = set() - # Mock CheckDAO + # Mock CheckDAO and RunDAO (for _create_metadata_run) mock_check_dao = MagicMock() mock_check_dao.find_check_by_id.return_value = mock_check + mock_run_dao = MagicMock() - with patch("recce.models.CheckDAO", return_value=mock_check_dao): + with ( + patch("recce.models.CheckDAO", return_value=mock_check_dao), + patch("recce.models.RunDAO", return_value=mock_run_dao), + ): result = await server._tool_run_check({"check_id": str(check_id)}) - # Verify the result is from schema_diff tool (has columns, data, limit, more) - assert "columns" in result - assert "data" in result - assert "limit" in result - assert "more" in result + # Result is now a Run.model_dump() dict (not raw schema_diff result) + assert "type" in result + assert "check_id" in result + # Verify a metadata run was persisted + mock_run_dao.create.assert_called_once() @pytest.mark.asyncio async def test_tool_value_diff(self, mcp_server): @@ -923,6 +951,295 @@ async def test_error_handling(self, mcp_server): await server._tool_lineage_diff({}) +class TestCreateMetadataRun: + """Test cases for the _create_metadata_run helper method.""" + + @pytest.fixture + def mcp_server(self): + mock_context = MagicMock(spec=RecceContext) + return RecceMCPServer(mock_context), mock_context + + @pytest.mark.asyncio + async def test_creates_run_with_correct_fields(self, mcp_server): + """_create_metadata_run creates a Run with correct type, params, check_id, and result.""" + server, _ = mcp_server + from uuid import uuid4 + + from recce.models.types import RunStatus, RunType + + check_id = uuid4() + params = {"select": "model_a"} + result_data = {"columns": ["col1"], "data": [["val1"]]} + + mock_run_dao = MagicMock() + with patch("recce.models.RunDAO", return_value=mock_run_dao): + run = server._create_metadata_run( + check_type=RunType.SCHEMA_DIFF, + params=params, + check_id=check_id, + result=result_data, + triggered_by="recce_ai", + ) + + assert run.type == RunType.SCHEMA_DIFF + assert run.params == params + assert run.check_id == check_id + assert run.result == result_data + assert run.status == RunStatus.FINISHED + assert run.triggered_by == "recce_ai" + assert run.name is not None # generate_run_name should set it + mock_run_dao.create.assert_called_once_with(run) + + @pytest.mark.asyncio + async def test_creates_run_for_lineage_diff(self, mcp_server): + """_create_metadata_run works for lineage_diff type.""" + server, _ = mcp_server + from uuid import uuid4 + + from recce.models.types import RunType + + check_id = uuid4() + result_data = {"nodes": {}, "edges": {}} + + mock_run_dao = MagicMock() + with patch("recce.models.RunDAO", return_value=mock_run_dao): + run = server._create_metadata_run( + check_type=RunType.LINEAGE_DIFF, + params={}, + check_id=check_id, + result=result_data, + triggered_by="user", + ) + + assert run.type == RunType.LINEAGE_DIFF + assert run.triggered_by == "user" + assert run.name == "Lineage diff" + mock_run_dao.create.assert_called_once() + + @pytest.mark.asyncio + async def test_triggered_by_none_allowed(self, mcp_server): + """_create_metadata_run accepts triggered_by=None.""" + server, _ = mcp_server + from uuid import uuid4 + + from recce.models.types import RunType + + check_id = uuid4() + + mock_run_dao = MagicMock() + with patch("recce.models.RunDAO", return_value=mock_run_dao): + run = server._create_metadata_run( + check_type=RunType.LINEAGE_DIFF, + params={}, + check_id=check_id, + result={}, + triggered_by=None, + ) + + assert run.triggered_by is None + + +class TestCreateCheckTriggeredBy: + """Test cases for triggered_by propagation in create_check and run_check.""" + + @pytest.fixture + def mcp_server(self): + mock_context = MagicMock(spec=RecceContext) + return RecceMCPServer(mock_context), mock_context + + @pytest.mark.asyncio + async def test_create_check_passes_triggered_by_to_submit_run(self, mcp_server): + """create_check passes triggered_by from arguments to submit_run.""" + server, _ = mcp_server + from uuid import uuid4 + + from recce.models.types import Check, RunStatus, RunType + + check_id = uuid4() + mock_check = MagicMock(spec=Check) + mock_check.check_id = check_id + + mock_run = MagicMock() + mock_run.status = RunStatus.FINISHED + mock_run.error = None + + mock_check_dao = MagicMock() + mock_check_dao.list.return_value = [] + + with ( + patch("recce.models.CheckDAO", return_value=mock_check_dao), + patch("recce.apis.check_func.create_check_without_run", return_value=mock_check), + patch("recce.apis.run_func.submit_run", return_value=(mock_run, asyncio.sleep(0))) as mock_submit, + patch("recce.apis.check_func.export_persistent_state"), + ): + await server._tool_create_check( + { + "type": "row_count_diff", + "params": {"node_names": ["orders"]}, + "name": "Row Count Diff", + "triggered_by": "recce_ai", + } + ) + + mock_submit.assert_called_once_with( + RunType.ROW_COUNT_DIFF, + params={"node_names": ["orders"]}, + check_id=check_id, + triggered_by="recce_ai", + ) + + @pytest.mark.asyncio + async def test_create_check_triggered_by_defaults_to_user(self, mcp_server): + """create_check defaults triggered_by to 'user' when not specified.""" + server, _ = mcp_server + from uuid import uuid4 + + from recce.models.types import Check, RunStatus, RunType + + check_id = uuid4() + mock_check = MagicMock(spec=Check) + mock_check.check_id = check_id + + mock_run = MagicMock() + mock_run.status = RunStatus.FINISHED + mock_run.error = None + + mock_check_dao = MagicMock() + mock_check_dao.list.return_value = [] + + with ( + patch("recce.models.CheckDAO", return_value=mock_check_dao), + patch("recce.apis.check_func.create_check_without_run", return_value=mock_check), + patch("recce.apis.run_func.submit_run", return_value=(mock_run, asyncio.sleep(0))) as mock_submit, + patch("recce.apis.check_func.export_persistent_state"), + ): + await server._tool_create_check( + { + "type": "row_count_diff", + "params": {"node_names": ["orders"]}, + "name": "Row Count Diff", + } + ) + + mock_submit.assert_called_once_with( + RunType.ROW_COUNT_DIFF, + params={"node_names": ["orders"]}, + check_id=check_id, + triggered_by="user", + ) + + @pytest.mark.asyncio + async def test_create_check_metadata_run_passes_triggered_by(self, mcp_server): + """create_check passes triggered_by to _create_metadata_run for schema_diff.""" + server, mock_context = mcp_server + from uuid import uuid4 + + check_id = uuid4() + mock_check = MagicMock() + mock_check.check_id = check_id + + mock_check_dao = MagicMock() + mock_check_dao.list.return_value = [] + + # Mock schema_diff dependencies + mock_lineage_diff = MagicMock(spec=LineageDiff) + mock_lineage_diff.model_dump.return_value = { + "base": {"nodes": {}, "parent_map": {}}, + "current": {"nodes": {}, "parent_map": {}}, + } + mock_context.get_lineage_diff.return_value = mock_lineage_diff + mock_context.adapter.select_nodes.return_value = set() + + mock_run_dao = MagicMock() + + with ( + patch("recce.models.CheckDAO", return_value=mock_check_dao), + patch("recce.apis.check_func.create_check_without_run", return_value=mock_check), + patch("recce.apis.check_func.export_persistent_state"), + patch("recce.models.RunDAO", return_value=mock_run_dao), + ): + await server._tool_create_check( + { + "type": "schema_diff", + "params": {}, + "name": "Schema Diff", + "triggered_by": "recce_ai", + } + ) + + # Verify the Run created via RunDAO has triggered_by="recce_ai" + created_run = mock_run_dao.create.call_args[0][0] + assert created_run.triggered_by == "recce_ai" + + @pytest.mark.asyncio + async def test_run_check_passes_triggered_by(self, mcp_server): + """run_check passes triggered_by from arguments to submit_run.""" + server, _ = mcp_server + from uuid import uuid4 + + from recce.models.types import RunStatus, RunType + + check_id = uuid4() + mock_check = MagicMock() + mock_check.check_id = check_id + mock_check.type = RunType.ROW_COUNT_DIFF + mock_check.params = {"node_names": ["orders"]} + + mock_run = MagicMock() + mock_run.status = RunStatus.FINISHED + mock_run.model_dump.return_value = {"run_id": "fake", "type": "row_count_diff"} + + mock_check_dao = MagicMock() + mock_check_dao.find_check_by_id.return_value = mock_check + + with ( + patch("recce.models.CheckDAO", return_value=mock_check_dao), + patch("recce.apis.run_func.submit_run", return_value=(mock_run, asyncio.sleep(0))) as mock_submit, + ): + await server._tool_run_check({"check_id": str(check_id), "triggered_by": "recce_ai"}) + + mock_submit.assert_called_once_with( + RunType.ROW_COUNT_DIFF, + params={"node_names": ["orders"]}, + check_id=str(check_id), + triggered_by="recce_ai", + ) + + @pytest.mark.asyncio + async def test_run_check_triggered_by_defaults_to_user(self, mcp_server): + """run_check defaults triggered_by to 'user' when not specified.""" + server, _ = mcp_server + from uuid import uuid4 + + from recce.models.types import RunStatus, RunType + + check_id = uuid4() + mock_check = MagicMock() + mock_check.check_id = check_id + mock_check.type = RunType.ROW_COUNT_DIFF + mock_check.params = {"node_names": ["orders"]} + + mock_run = MagicMock() + mock_run.status = RunStatus.FINISHED + mock_run.model_dump.return_value = {"run_id": "fake", "type": "row_count_diff"} + + mock_check_dao = MagicMock() + mock_check_dao.find_check_by_id.return_value = mock_check + + with ( + patch("recce.models.CheckDAO", return_value=mock_check_dao), + patch("recce.apis.run_func.submit_run", return_value=(mock_run, asyncio.sleep(0))) as mock_submit, + ): + await server._tool_run_check({"check_id": str(check_id)}) + + mock_submit.assert_called_once_with( + RunType.ROW_COUNT_DIFF, + params={"node_names": ["orders"]}, + check_id=str(check_id), + triggered_by="user", + ) + + class TestRunMCPServer: """Test cases for the run_mcp_server function""" @@ -1456,7 +1773,11 @@ async def test_existing_tools_dispatch_via_call_tool(self, mcp_server): mock_check_dao2 = MagicMock() mock_check_dao2.find_check_by_id.return_value = mock_check - with patch("recce.models.CheckDAO", return_value=mock_check_dao2): + mock_run_dao = MagicMock() + with ( + patch("recce.models.CheckDAO", return_value=mock_check_dao2), + patch("recce.models.RunDAO", return_value=mock_run_dao), + ): r = await self._invoke_call_tool(server, "run_check", {"check_id": str(check_id)}) assert r.root.isError is not True @@ -1724,6 +2045,29 @@ async def test_run_check_recce_exception(self, mcp_server): with pytest.raises(ValueError, match="Task execution failed"): await server._tool_run_check({"check_id": str(check_id)}) + @pytest.mark.asyncio + async def test_run_check_metadata_branch_recce_exception(self, mcp_server): + """RecceException from _tool_lineage_diff in metadata branch should be wrapped in ValueError.""" + server, _ = mcp_server + from uuid import uuid4 + + from recce.exceptions import RecceException + from recce.models.types import RunType + + check_id = uuid4() + mock_check = MagicMock() + mock_check.check_id = check_id + mock_check.type = RunType.LINEAGE_DIFF + mock_check.params = {} + + mock_check_dao = MagicMock() + mock_check_dao.find_check_by_id.return_value = mock_check + + with patch("recce.models.CheckDAO", return_value=mock_check_dao): + with patch.object(server, "_tool_lineage_diff", side_effect=RecceException("Lineage data unavailable")): + with pytest.raises(ValueError, match="Lineage data unavailable"): + await server._tool_run_check({"check_id": str(check_id)}) + class TestQueryRowCountDbtErrors: """Test _query_row_count error classification in the dbt path (lines 141, 154)."""